[ARVADOS] updated: b20590222beddb52c8c89294ed3a324c8c7190a2

git at public.curoverse.com git at public.curoverse.com
Wed Feb 4 21:36:12 EST 2015


Summary of changes:
 apps/workbench/test/test_helper.rb   |  9 +---
 sdk/go/keepclient/support.go         |  9 ----
 sdk/python/tests/run_test_server.py  | 90 ++++++++++++++++++++++++++++++------
 services/keepproxy/keepproxy_test.go | 13 ++++--
 4 files changed, 88 insertions(+), 33 deletions(-)

       via  b20590222beddb52c8c89294ed3a324c8c7190a2 (commit)
       via  13f83b9374e66e4609aff661b467d747067d66c2 (commit)
       via  64416e4751edfe6c49c0bed8a7e38071200282d8 (commit)
       via  0c8f599d598f36d67daf0e0e39756ba4d064cbd0 (commit)
       via  d1957808f6e3ccece499ac2f4048d4ef850b262c (commit)
       via  1f8fcb0279a7bb2aa9cf1386ff9516da58216d53 (commit)
       via  2cf42c27a7e8b37e29462d0b695e24cb6f3ad5ce (commit)
       via  bd6f17515de33e6eee9631723730fc65125ebad2 (commit)
      from  4470ba26b332cb92d347af00cdb26c716b1a6953 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit b20590222beddb52c8c89294ed3a324c8c7190a2
Author: Tom Clegg <tom at curoverse.com>
Date:   Wed Feb 4 21:28:52 2015 -0500

    3021: In start(), if a stale server is already running (but we can't reset() it), kill it.

diff --git a/sdk/python/tests/run_test_server.py b/sdk/python/tests/run_test_server.py
index dbad488..ab1c7ff 100644
--- a/sdk/python/tests/run_test_server.py
+++ b/sdk/python/tests/run_test_server.py
@@ -153,6 +153,14 @@ def run(leave_running_atexit=False):
         except:
             pass
 
+    # Before trying to start up our own server, call stop() to avoid
+    # "Phusion Passenger Standalone is already running on PID 12345".
+    # We want to kill it if it's our own _or_ it's some stale
+    # left-over server. But if it's been deliberately provided to us
+    # by a parent process, we don't want to force-kill it. That'll
+    # just wreck things for the next test suite that tries to use it.
+    stop(force=('ARVADOS_TEST_API_HOST' not in os.environ))
+
     restore_cwd = os.getcwd()
     api_src_dir = os.path.join(SERVICES_SRC_DIR, 'api')
     os.chdir(api_src_dir)

commit 13f83b9374e66e4609aff661b467d747067d66c2
Author: Tom Clegg <tom at curoverse.com>
Date:   Wed Feb 4 21:27:17 2015 -0500

    3021: Check whether a randomly selected port is available before using it.

diff --git a/sdk/python/tests/run_test_server.py b/sdk/python/tests/run_test_server.py
index 4f7cbb1..dbad488 100644
--- a/sdk/python/tests/run_test_server.py
+++ b/sdk/python/tests/run_test_server.py
@@ -10,6 +10,7 @@ import re
 import shutil
 import signal
 import subprocess
+import string
 import sys
 import tempfile
 import time
@@ -92,6 +93,31 @@ def kill_server_pid(pidfile, wait=10, passenger_root=False):
     except OSError:
         pass
 
+def find_available_port():
+    """Return a port number that is not in use right now.
+
+    Some opportunity for races here, but it's better than choosing
+    something at random and not checking at all. If all of our servers
+    (hey Passenger) knew that listening on port 0 was a thing, the OS
+    would take care of the races, and this wouldn't be needed at all.
+    """
+    port = None
+    while port is None:
+        port = random.randint(20000, 40000)
+        port_hex = ':%04x ' % port
+        try:
+            with open('/proc/net/tcp', 'r') as f:
+                for line in f:
+                    if 0 <= string.find(line, port_hex):
+                        port = None
+                        break
+        except OSError:
+            # This isn't going so well. Just use the random port.
+            pass
+        except IOError:
+            pass
+    return port
+
 def run(leave_running_atexit=False):
     """Ensure an API server is running, and ARVADOS_API_* env vars have
     admin credentials for it.
@@ -149,7 +175,7 @@ def run(leave_running_atexit=False):
             '-days', '3650',
             '-subj', '/CN=0.0.0.0'])
 
-    port = random.randint(20000, 40000)
+    port = find_available_port()
     env = os.environ.copy()
     env['RAILS_ENV'] = 'test'
     env['ARVADOS_WEBSOCKETS'] = 'yes'
@@ -219,7 +245,7 @@ def stop(force=False):
 
 def _start_keep(n, keep_args):
     keep0 = tempfile.mkdtemp()
-    port = random.randint(20000, 40000)
+    port = find_available_port()
     keep_cmd = ["keepstore",
                 "-volumes={}".format(keep0),
                 "-listen=:{}".format(port),
@@ -288,7 +314,7 @@ def run_keep_proxy():
     stop_keep_proxy()
 
     admin_token = auth_token('admin')
-    port = random.randint(20000,40000)
+    port = find_available_port()
     env = os.environ.copy()
     env['ARVADOS_API_TOKEN'] = admin_token
     kp = subprocess.Popen(

commit 64416e4751edfe6c49c0bed8a7e38071200282d8
Author: Tom Clegg <tom at curoverse.com>
Date:   Wed Feb 4 21:25:53 2015 -0500

    3021: Don't worry about env vars, run_test_server.py does that.

diff --git a/apps/workbench/test/test_helper.rb b/apps/workbench/test/test_helper.rb
index a763b2b..078190b 100644
--- a/apps/workbench/test/test_helper.rb
+++ b/apps/workbench/test/test_helper.rb
@@ -122,12 +122,7 @@ class ApiServerForTests
   def run_test_server
     env_script = nil
     Dir.chdir PYTHON_TESTS_DIR do
-      env = {
-        'RAILS_ENV' => 'test',
-        'ARVADOS_WEBSOCKETS' => 'yes'
-      }
-      cmd = ['python', './run_test_server.py', 'start', '--auth', 'admin']
-      env_script = check_call({}.merge(ENV).merge(env), cmd)
+      env_script = check_call %w(python ./run_test_server.py start --auth admin)
     end
     test_env = {}
     env_script.each_line do |line|
@@ -144,7 +139,7 @@ class ApiServerForTests
   def stop_test_server
     Dir.chdir PYTHON_TESTS_DIR do
       # This is a no-op if we're running within run-tests.sh
-      check_call ['python', './run_test_server.py', 'stop']
+      check_call %w(python ./run_test_server.py stop)
     end
     @@server_is_running = false
   end

commit 0c8f599d598f36d67daf0e0e39756ba4d064cbd0
Author: Tom Clegg <tom at curoverse.com>
Date:   Wed Feb 4 17:35:26 2015 -0500

    3021: Fix use of inaccessible global in atexit handler.

diff --git a/sdk/python/tests/run_test_server.py b/sdk/python/tests/run_test_server.py
index 2998f64..4f7cbb1 100644
--- a/sdk/python/tests/run_test_server.py
+++ b/sdk/python/tests/run_test_server.py
@@ -61,17 +61,17 @@ def find_server_pid(PID_PATH, wait=10):
 
     return server_pid
 
-def kill_server_pid(pidfile, wait=10, passenger=False):
+def kill_server_pid(pidfile, wait=10, passenger_root=False):
     # Must re-import modules in order to work during atexit
     import os
     import signal
     import subprocess
     import time
     try:
-        if passenger:
+        if passenger_root:
             # First try to shut down nicely
             restore_cwd = os.getcwd()
-            os.chdir(os.path.join(SERVICES_SRC_DIR, 'api'))
+            os.chdir(passenger_root)
             subprocess.call([
                 'bundle', 'exec', 'passenger', 'stop', '--pid-file', pidfile])
             os.chdir(restore_cwd)
@@ -80,7 +80,7 @@ def kill_server_pid(pidfile, wait=10, passenger=False):
         with open(pidfile, 'r') as f:
             server_pid = int(f.read())
         while now <= timeout:
-            if not passenger or timeout - now < wait / 2:
+            if not passenger_root or timeout - now < wait / 2:
                 # Half timeout has elapsed. Start sending SIGTERM
                 os.kill(server_pid, signal.SIGTERM)
             # Raise OSError if process has disappeared
@@ -128,7 +128,8 @@ def run(leave_running_atexit=False):
             pass
 
     restore_cwd = os.getcwd()
-    os.chdir(os.path.join(SERVICES_SRC_DIR, 'api'))
+    api_src_dir = os.path.join(SERVICES_SRC_DIR, 'api')
+    os.chdir(api_src_dir)
 
     # Either we haven't started a server of our own yet, or it has
     # died, or we have lost our credentials, or something else is
@@ -167,7 +168,7 @@ def run(leave_running_atexit=False):
         env=env)
 
     if not leave_running_atexit:
-        atexit.register(kill_server_pid, pid_file, passenger=True)
+        atexit.register(kill_server_pid, pid_file, passenger_root=api_src_dir)
 
     match = re.search(r'Accessible via: https://(.*?)/', start_msg)
     if not match:

commit d1957808f6e3ccece499ac2f4048d4ef850b262c
Author: Tom Clegg <tom at curoverse.com>
Date:   Wed Feb 4 17:28:29 2015 -0500

    3021: Remove ARVADOS_KEEP_PROXY support. (If you need to interfere
    with the discovery mechanism from out-of-process, use some combination
    of HTTP_PROXY, NO_PROXY, and a generic proxy server.)

diff --git a/sdk/go/keepclient/support.go b/sdk/go/keepclient/support.go
index c24849e..9db6ebc 100644
--- a/sdk/go/keepclient/support.go
+++ b/sdk/go/keepclient/support.go
@@ -11,7 +11,6 @@ import (
 	"log"
 	"net"
 	"net/http"
-	"os"
 	"strings"
 	"time"
 )
@@ -78,14 +77,6 @@ func (this *KeepClient) setClientSettingsStore() {
 }
 
 func (this *KeepClient) DiscoverKeepServers() error {
-	if prx := os.Getenv("ARVADOS_KEEP_PROXY"); prx != "" {
-		sr := map[string]string{"proxy": prx}
-		this.SetServiceRoots(sr)
-		this.Using_proxy = true
-		this.setClientSettingsProxy()
-		return nil
-	}
-
 	type svcList struct {
 		Items []keepDisk `json:"items"`
 	}

commit 1f8fcb0279a7bb2aa9cf1386ff9516da58216d53
Author: Tom Clegg <tom at curoverse.com>
Date:   Wed Feb 4 17:15:53 2015 -0500

    3021: Set up a proxy-only keepclient manually, instead of communicating with global/env vars.

diff --git a/services/keepproxy/keepproxy_test.go b/services/keepproxy/keepproxy_test.go
index 0b75bb6..ccbd7d8 100644
--- a/services/keepproxy/keepproxy_test.go
+++ b/services/keepproxy/keepproxy_test.go
@@ -107,17 +107,22 @@ func runProxy(c *C, args []string, port int, bogusClientToken bool) keepclient.K
 	if bogusClientToken {
 		os.Setenv("ARVADOS_API_TOKEN", "bogus-token")
 	}
-	os.Setenv("ARVADOS_KEEP_PROXY", fmt.Sprintf("http://localhost:%v", port))
 	arv, err := arvadosclient.MakeArvadosClient()
 	c.Assert(err, Equals, nil)
-	kc, err := keepclient.MakeKeepClient(&arv)
-	c.Assert(err, Equals, nil)
+	kc := keepclient.KeepClient{
+		Arvados: &arv,
+		Want_replicas: 2,
+		Using_proxy: true,
+		Client: &http.Client{},
+	}
+	kc.SetServiceRoots(map[string]string{
+		"proxy": fmt.Sprintf("http://localhost:%v", port),
+	})
 	c.Check(kc.Using_proxy, Equals, true)
 	c.Check(len(kc.ServiceRoots()), Equals, 1)
 	for _, root := range kc.ServiceRoots() {
 		c.Check(root, Equals, fmt.Sprintf("http://localhost:%v", port))
 	}
-	os.Unsetenv("ARVADOS_KEEP_PROXY")
 	log.Print("keepclient created")
 	if bogusClientToken {
 		arvadostest.ResetEnv()

commit 2cf42c27a7e8b37e29462d0b695e24cb6f3ad5ce
Author: Tom Clegg <tom at curoverse.com>
Date:   Wed Feb 4 17:13:25 2015 -0500

    3021: Tidy up (and document) the choice of exactly which server we expect to run/reset/stop.

diff --git a/sdk/python/tests/run_test_server.py b/sdk/python/tests/run_test_server.py
index c52ba8e..2998f64 100644
--- a/sdk/python/tests/run_test_server.py
+++ b/sdk/python/tests/run_test_server.py
@@ -95,6 +95,17 @@ def kill_server_pid(pidfile, wait=10, passenger=False):
 def run(leave_running_atexit=False):
     """Ensure an API server is running, and ARVADOS_API_* env vars have
     admin credentials for it.
+
+    If ARVADOS_TEST_API_HOST is set, a parent process has started a
+    test server for us to use: we just need to reset() it using the
+    admin token fixture.
+
+    If a previous call to run() started a new server process, and it
+    is still running, we just need to reset() it to fixture state and
+    return.
+
+    If neither of those options work out, we'll really start a new
+    server.
     """
     global my_api_host
 
@@ -107,8 +118,10 @@ def run(leave_running_atexit=False):
     pid_file = os.path.join(SERVICES_SRC_DIR, 'api', SERVER_PID_PATH)
     pid_file_ok = find_server_pid(pid_file, 0)
 
-    if pid_file_ok:
+    existing_api_host = os.environ.get('ARVADOS_TEST_API_HOST', my_api_host)
+    if existing_api_host and pid_file_ok:
         try:
+            os.environ['ARVADOS_API_HOST'] = existing_api_host
             reset()
             return
         except:
@@ -170,17 +183,33 @@ def run(leave_running_atexit=False):
     os.chdir(restore_cwd)
 
 def reset():
+    """Reset the test server to fixture state.
+
+    This resets the ARVADOS_TEST_API_HOST provided by a parent process
+    if any, otherwise the server started by run().
+    """
+    existing_api_host = os.environ.get('ARVADOS_TEST_API_HOST', my_api_host)
     token = auth_token('admin')
     httpclient = httplib2.Http(ca_certs=os.path.join(
         SERVICES_SRC_DIR, 'api', 'tmp', 'self-signed.pem'))
     httpclient.request(
-        'https://{}/database/reset'.format(os.environ['ARVADOS_API_HOST']),
+        'https://{}/database/reset'.format(existing_api_host),
         'POST',
         headers={'Authorization': 'OAuth2 {}'.format(token)})
 
 def stop(force=False):
-    """Stop the API server, if one is running. If force==True, kill it
-    even if we didn't start it ourselves.
+    """Stop the API server, if one is running.
+
+    If force==False, kill it only if we started it ourselves. (This
+    supports the use case where a Python test suite calls run(), but
+    run() just uses the ARVADOS_TEST_API_HOST provided by the parent
+    process, and the test suite cleans up after itself by calling
+    stop(). In this case the test server provided by the parent
+    process should be left alone.)
+
+    If force==True, kill it even if we didn't start it
+    ourselves. (This supports the use case in __main__, where "run"
+    and "stop" happen in different processes.)
     """
     global my_api_host
     if force or my_api_host is not None:

commit bd6f17515de33e6eee9631723730fc65125ebad2
Author: Tom Clegg <tom at curoverse.com>
Date:   Wed Feb 4 17:11:57 2015 -0500

    3021: Fix misspelled variable.

diff --git a/sdk/python/tests/run_test_server.py b/sdk/python/tests/run_test_server.py
index ddc2799..c52ba8e 100644
--- a/sdk/python/tests/run_test_server.py
+++ b/sdk/python/tests/run_test_server.py
@@ -154,7 +154,7 @@ def run(leave_running_atexit=False):
         env=env)
 
     if not leave_running_atexit:
-        atexit.register(kill_server_pid, pidfile, passenger=True)
+        atexit.register(kill_server_pid, pid_file, passenger=True)
 
     match = re.search(r'Accessible via: https://(.*?)/', start_msg)
     if not match:

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list