[ARVADOS] updated: 1c3d0033b49be2c81ce4e3d05e96ad287890638c

git at public.curoverse.com git at public.curoverse.com
Sat Nov 7 04:36:12 EST 2015


Summary of changes:
 .../app/controllers/collections_controller.rb      |  14 ++-
 apps/workbench/config/application.default.yml      |  11 +-
 apps/workbench/test/integration_helper.rb          |   1 +
 .../install-compute-node.html.textile.liquid       |   4 +
 .../install-crunch-dispatch.html.textile.liquid    |   2 +-
 doc/install/install-keep-web.html.textile.liquid   |  13 ++-
 doc/install/install-keepproxy.html.textile.liquid  |   2 +-
 sdk/cli/bin/crunch-job                             |   7 +-
 sdk/go/arvadosclient/arvadosclient.go              |  20 ++++
 sdk/go/arvadosclient/arvadosclient_test.go         |  78 +++++++------
 sdk/go/arvadostest/fixtures.go                     |   2 +
 sdk/go/arvadostest/run_servers.go                  |  18 +--
 sdk/go/keepclient/discover.go                      | 130 +++++++++++++++++++++
 sdk/go/keepclient/discover_test.go                 |  18 +++
 sdk/go/keepclient/support.go                       |  81 -------------
 sdk/python/tests/run_test_server.py                |  34 +++---
 .../test/fixtures/api_client_authorizations.yml    |  12 ++
 services/datamanager/datamanager_test.go           |  27 +++--
 services/dockercleaner/arvados_docker/cleaner.py   |  40 ++++++-
 services/dockercleaner/tests/test_cleaner.py       |  74 +++++++++++-
 services/keep-web/anonymous.go                     |  23 +++-
 services/keep-web/doc.go                           |  18 +--
 services/keep-web/handler.go                       |  19 ++-
 services/keep-web/main.go                          |   4 +-
 services/keep-web/server.go                        |   2 +-
 services/keepproxy/keepproxy.go                    |  39 +------
 services/keepproxy/keepproxy_test.go               |  45 ++++---
 services/keepstore/keepstore.go                    |  14 +--
 .../arvnodeman/computenode/dispatch/__init__.py    |  22 ++--
 .../arvnodeman/computenode/dispatch/slurm.py       |   4 +-
 services/nodemanager/arvnodeman/daemon.py          |  14 ++-
 .../nodemanager/tests/test_computenode_dispatch.py |  14 ++-
 services/nodemanager/tests/test_daemon.py          |  20 ++++
 tools/keep-rsync/keep-rsync_test.go                |  10 +-
 34 files changed, 572 insertions(+), 264 deletions(-)
 create mode 100644 sdk/go/keepclient/discover.go
 create mode 100644 sdk/go/keepclient/discover_test.go

       via  1c3d0033b49be2c81ce4e3d05e96ad287890638c (commit)
       via  9546de1a0c3512d4f8209d89b86873de2729d9e7 (commit)
       via  fb1127878ebbafd3e1aaa1fa21862f7d3c661be6 (commit)
       via  c42c5968dd554a96bf0b67ea52a0bab3528035e2 (commit)
       via  6ccb59197f1ad3ba831c7936e8f5c0e9adccb963 (commit)
       via  100b485f4295cbb414ed743b2fdba990591272e7 (commit)
       via  745cfb876277f0c65244d570c1736fb936446f35 (commit)
       via  b1ef43a1dbb4a66e1646fc68ac88ff6d54026ac1 (commit)
       via  4ec9919ea6cce4ada252f1e0a0dc521fe27e508c (commit)
       via  49fccd5b00230418a0918a33d4fb9154af63220a (commit)
       via  8e1e7e6bf355eb0f1defc7278f1434c393a75a75 (commit)
       via  05a0e777cdef513f5365d5319fea7af01ae9e7f7 (commit)
       via  b85cb1ba40385444f9494bbb88248ab65d700c85 (commit)
       via  28425a5cf55a26113bdbea53381f86b779d84bf9 (commit)
       via  6a8d940762f1b9be1a3a273e13b070d1f75ef8f1 (commit)
       via  fa96bcea706312be52f1488da3dad34452f5ccf4 (commit)
       via  6b606a655410d9ed5116c3a3b6b6971bd90f9666 (commit)
       via  c74d29f1a53a1020520ffe6992ce4a39a350b5ef (commit)
       via  57d3837ca9a8c5e4e71f9741931cc67e908fb3dd (commit)
       via  0a43beae9c546b356220b281a432e1fa1986a305 (commit)
       via  21bcf992a7e4eb806ea45c851356b2a50015fa65 (commit)
       via  3c0ef3fc0cdb3f56d90442cc389ce4c2c94e8831 (commit)
       via  3bef97ac46da8f5a7c322ec31c6bba4affb8ad80 (commit)
       via  0a365f3d1a19fc27b402c9c742debf8975424b3f (commit)
       via  242d3272391baeb95eb0a5e4e51627c2d54e7bc6 (commit)
       via  59b22379a693d1fc15b4b77e58e47bac89e62660 (commit)
       via  1d1c6de3c842a33a57b7d469fdaaaa1b873433dc (commit)
       via  21006cfaf6d4d0ac3884a72803a8723bc4bb76fb (commit)
       via  9b48b17eddea5e366e0c59ed9f3540793550256c (commit)
       via  82b7adbaf524d6e7fd2b9a6403f9e490dcb3ac85 (commit)
       via  bd9722cb7b6f92b752ac786797d884ea711944cb (commit)
       via  7bde93de3260e08503c7bf9c06fd3448972d378a (commit)
       via  1057d0912fef22aeaa707fd428521a76806a21d0 (commit)
       via  e10ccaba824b4f60ddc516903304351496b5fdca (commit)
       via  b11cc5c782e4e94a422b8133be42bbb263d48dba (commit)
      from  767e1199d0e1bdf2b564b5c58a91d29141eb67d7 (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 1c3d0033b49be2c81ce4e3d05e96ad287890638c
Author: Tom Clegg <tom at curoverse.com>
Date:   Sat Nov 7 04:36:01 2015 -0500

    5824: Fix disposition=attachment handling.
    
    Propagate disposition=attachment from Workbench to keep-web when
    redirecting.
    
    Include a filename in the Content-Disposition header if the request
    URL contains "?", so UAs don't mistakenly include the query string as
    part of the default filename.

diff --git a/apps/workbench/app/controllers/collections_controller.rb b/apps/workbench/app/controllers/collections_controller.rb
index 38b58a1..cadef38 100644
--- a/apps/workbench/app/controllers/collections_controller.rb
+++ b/apps/workbench/app/controllers/collections_controller.rb
@@ -148,6 +148,7 @@ class CollectionsController < ApplicationController
         # to read the collection.
         opts[:query_token] = usable_token
       end
+      opts[:disposition] = params[:disposition]
       return redirect_to keep_web_url(params[:uuid], params[:file], opts)
     end
 
@@ -332,9 +333,18 @@ class CollectionsController < ApplicationController
     end
     uri.path += '_/'
     uri.path += CGI::escape(file)
-    if opts[:query_token]
-      uri.query = 'api_token=' + CGI::escape(opts[:query_token])
+
+    query_params = []
+    { query_token: 'api_token',
+      disposition: 'disposition' }.each do |opt, param|
+      if opts[opt]
+        query_params << param + '=' + CGI::escape(opts[opt])
+      end
     end
+    unless query_params.empty?
+      uri.query = query_params.join '&'
+    end
+
     uri.to_s
   end
 
diff --git a/apps/workbench/config/application.default.yml b/apps/workbench/config/application.default.yml
index 5504fd2..0a9ee9f 100644
--- a/apps/workbench/config/application.default.yml
+++ b/apps/workbench/config/application.default.yml
@@ -230,6 +230,13 @@ common:
   # download facility.
   #
   # Examples:
-  # keep_web_url: https://%{uuid_or_pdh}.dl.zzzzz.your.domain
-  # keep_web_url: https://%{uuid_or_pdh}--dl.zzzzz.your.domain
+  # keep_web_url: https://%{uuid_or_pdh}.collections.uuid_prefix.arvadosapi.com
+  # keep_web_url: https://%{uuid_or_pdh}--collections.uuid_prefix.arvadosapi.com
+  #
+  # Example supporting only public data and collection-sharing links:
+  # keep_web_url: https://collections.uuid_prefix.arvadosapi.com/c=%{uuid_or_pdh}
+  #
+  # Example supporting only download/attachment: (using keep-web
+  # -attachment-only-host collections.uuid_prefix.arvadosapi.com):
+  # keep_web_url: https://collections.uuid_prefix.arvadosapi.com/c=%{uuid_or_pdh}
   keep_web_url: false
diff --git a/apps/workbench/test/integration_helper.rb b/apps/workbench/test/integration_helper.rb
index 5750a1b..1cda127 100644
--- a/apps/workbench/test/integration_helper.rb
+++ b/apps/workbench/test/integration_helper.rb
@@ -27,6 +27,7 @@ Capybara.register_driver :selenium_with_download do |app|
   profile['browser.download.folderList'] = 2 # "save to user-defined location"
   profile['browser.download.manager.showWhenStarting'] = false
   profile['browser.helperApps.alwaysAsk.force'] = false
+  profile['browser.helperApps.neverAsk.saveToDisk'] = 'text/plain,application/octet-stream'
   Capybara::Selenium::Driver.new app, profile: profile
 end
 
diff --git a/services/keep-web/handler.go b/services/keep-web/handler.go
index 3e38cc3..e8678fa 100644
--- a/services/keep-web/handler.go
+++ b/services/keep-web/handler.go
@@ -9,6 +9,7 @@ import (
 	"net/http"
 	"net/url"
 	"os"
+	"strconv"
 	"strings"
 
 	"git.curoverse.com/arvados.git/sdk/go/arvadosclient"
@@ -304,8 +305,24 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) {
 	if rdr, ok := rdr.(keepclient.ReadCloserWithLen); ok {
 		w.Header().Set("Content-Length", fmt.Sprintf("%d", rdr.Len()))
 	}
+
+	disposition := "inline"
 	if attachment {
-		w.Header().Set("Content-Disposition", "attachment")
+		disposition = "attachment"
+	}
+	if strings.ContainsRune(r.RequestURI, '?') {
+		// Help the UA realize that the filename is just
+		// "filename.txt", not
+		// "filename.txt?disposition=attachment".
+		//
+		// TODO(TC): Follow advice at RFC 6266 appendix D
+		if basenamePos < 0 {
+			basenamePos = 0
+		}
+		disposition += "; filename=" + strconv.QuoteToASCII(filename[basenamePos:])
+	}
+	if disposition != "inline" {
+		w.Header().Set("Content-Disposition", disposition)
 	}
 
 	w.WriteHeader(http.StatusOK)

commit 9546de1a0c3512d4f8209d89b86873de2729d9e7
Author: Tom Clegg <tom at curoverse.com>
Date:   Sat Nov 7 04:06:47 2015 -0500

    5824: Fixup new keepproxy tests to use simplified test setup.
    
    See 813d35123538b00ab70719e247b6bb0881269460

diff --git a/services/keepproxy/keepproxy_test.go b/services/keepproxy/keepproxy_test.go
index 9c33948..7c5b362 100644
--- a/services/keepproxy/keepproxy_test.go
+++ b/services/keepproxy/keepproxy_test.go
@@ -73,6 +73,10 @@ func (s *ServerRequiredSuite) TearDownSuite(c *C) {
 
 func (s *NoKeepServerSuite) SetUpSuite(c *C) {
 	arvadostest.StartAPI()
+	// We need API to have some keep services listed, but the
+	// services themselves should be unresponsive.
+	arvadostest.StartKeep(2, false)
+	arvadostest.StopKeep(2)
 }
 
 func (s *NoKeepServerSuite) SetUpTest(c *C) {
@@ -103,7 +107,7 @@ func runProxy(c *C, args []string, bogusClientToken bool) *keepclient.KeepClient
 	kc.Arvados.External = true
 	kc.Using_proxy = true
 
-	return &kc
+	return kc
 }
 
 func (s *ServerRequiredSuite) TestPutAskGet(c *C) {
@@ -407,8 +411,7 @@ func (s *ServerRequiredSuite) TestGetIndex(c *C) {
 }
 
 func (s *ServerRequiredSuite) TestPutAskGetInvalidToken(c *C) {
-	kc := runProxy(c, []string{"keepproxy"}, 28852, false)
-	waitForListener()
+	kc := runProxy(c, nil, false)
 	defer closeListener()
 
 	// Put a test block
@@ -446,7 +449,7 @@ func (s *ServerRequiredSuite) TestAskGetKeepProxyConnectionError(c *C) {
 	// keepclient with no such keep server
 	kc := keepclient.New(&arv)
 	locals := map[string]string{
-		"proxy": "http://localhost:12345",
+		TestProxyUUID: "http://localhost:12345",
 	}
 	kc.SetServiceRoots(locals, nil, nil)
 
@@ -467,22 +470,24 @@ func (s *ServerRequiredSuite) TestAskGetKeepProxyConnectionError(c *C) {
 }
 
 func (s *NoKeepServerSuite) TestAskGetNoKeepServerError(c *C) {
-	kc := runProxy(c, []string{"keepproxy"}, 29999, false)
-	waitForListener()
+	kc := runProxy(c, nil, false)
 	defer closeListener()
 
-	// Ask should result in temporary connection refused error
 	hash := fmt.Sprintf("%x", md5.Sum([]byte("foo")))
-	_, _, err := kc.Ask(hash)
-	c.Check(err, NotNil)
-	errNotFound, _ := err.(*keepclient.ErrNotFound)
-	c.Check(errNotFound.Temporary(), Equals, true)
-	c.Assert(strings.Contains(err.Error(), "HTTP 502"), Equals, true)
-
-	// Get should result in temporary connection refused error
-	_, _, _, err = kc.Get(hash)
-	c.Check(err, NotNil)
-	errNotFound, _ = err.(*keepclient.ErrNotFound)
-	c.Check(errNotFound.Temporary(), Equals, true)
-	c.Assert(strings.Contains(err.Error(), "HTTP 502"), Equals, true)
+	for _, f := range []func()error {
+		func() error {
+			_, _, err := kc.Ask(hash)
+			return err
+		},
+		func() error {
+			_, _, _, err := kc.Get(hash)
+			return err
+		},
+	} {
+		err := f()
+		c.Assert(err, NotNil)
+		errNotFound, _ := err.(*keepclient.ErrNotFound)
+		c.Check(errNotFound.Temporary(), Equals, true)
+		c.Check(err, ErrorMatches, `.*HTTP 502.*`)
+	}
 }

commit fb1127878ebbafd3e1aaa1fa21862f7d3c661be6
Author: Tom Clegg <tom at curoverse.com>
Date:   Sat Nov 7 04:03:27 2015 -0500

    5824: Move "periodically refresh Keep services" func from keepproxy to SDK.

diff --git a/sdk/go/keepclient/discover.go b/sdk/go/keepclient/discover.go
new file mode 100644
index 0000000..650c2b5
--- /dev/null
+++ b/sdk/go/keepclient/discover.go
@@ -0,0 +1,130 @@
+package keepclient
+
+import (
+	"encoding/json"
+	"fmt"
+	"log"
+	"os"
+	"os/signal"
+	"reflect"
+	"strings"
+	"syscall"
+	"time"
+)
+
+// DiscoverKeepServers gets list of available keep services from api server
+func (this *KeepClient) DiscoverKeepServers() error {
+	var list svcList
+
+	// Get keep services from api server
+	err := this.Arvados.Call("GET", "keep_services", "", "accessible", nil, &list)
+	if err != nil {
+		return err
+	}
+
+	return this.loadKeepServers(list)
+}
+
+// LoadKeepServicesFromJSON gets list of available keep services from given JSON
+func (this *KeepClient) LoadKeepServicesFromJSON(services string) error {
+	var list svcList
+
+	// Load keep services from given json
+	dec := json.NewDecoder(strings.NewReader(services))
+	if err := dec.Decode(&list); err != nil {
+		return err
+	}
+
+	return this.loadKeepServers(list)
+}
+
+// RefreshServices calls DiscoverKeepServers to refresh the keep
+// service list on SIGHUP; when the given interval has elapsed since
+// the last refresh; and (if the last refresh failed) the given
+// errInterval has elapsed.
+func (kc *KeepClient) RefreshServices(interval, errInterval time.Duration) {
+	var previousRoots = []map[string]string{}
+
+	timer := time.NewTimer(interval)
+	gotHUP := make(chan os.Signal, 1)
+	signal.Notify(gotHUP, syscall.SIGHUP)
+
+	for {
+		select {
+		case <-gotHUP:
+		case <-timer.C:
+		}
+		timer.Reset(interval)
+
+		if err := kc.DiscoverKeepServers(); err != nil {
+			log.Println("Error retrieving services list: %v (retrying in %v)", err, errInterval)
+			timer.Reset(errInterval)
+			continue
+		}
+		newRoots := []map[string]string{kc.LocalRoots(), kc.GatewayRoots()}
+
+		if !reflect.DeepEqual(previousRoots, newRoots) {
+			log.Printf("Updated services list: locals %v gateways %v", newRoots[0], newRoots[1])
+			previousRoots = newRoots
+		}
+
+		if len(newRoots[0]) == 0 {
+			log.Printf("WARNING: No local services (retrying in %v)", errInterval)
+			timer.Reset(errInterval)
+		}
+	}
+}
+
+// loadKeepServers
+func (this *KeepClient) loadKeepServers(list svcList) error {
+	listed := make(map[string]bool)
+	localRoots := make(map[string]string)
+	gatewayRoots := make(map[string]string)
+	writableLocalRoots := make(map[string]string)
+
+	// replicasPerService is 1 for disks; unknown or unlimited otherwise
+	this.replicasPerService = 1
+	this.Using_proxy = false
+
+	for _, service := range list.Items {
+		scheme := "http"
+		if service.SSL {
+			scheme = "https"
+		}
+		url := fmt.Sprintf("%s://%s:%d", scheme, service.Hostname, service.Port)
+
+		// Skip duplicates
+		if listed[url] {
+			continue
+		}
+		listed[url] = true
+
+		localRoots[service.Uuid] = url
+		if service.SvcType == "proxy" {
+			this.Using_proxy = true
+		}
+
+		if service.ReadOnly == false {
+			writableLocalRoots[service.Uuid] = url
+			if service.SvcType != "disk" {
+				this.replicasPerService = 0
+			}
+		}
+
+		// Gateway services are only used when specified by
+		// UUID, so there's nothing to gain by filtering them
+		// by service type. Including all accessible services
+		// (gateway and otherwise) merely accommodates more
+		// service configurations.
+		gatewayRoots[service.Uuid] = url
+	}
+
+	if this.Using_proxy {
+		this.setClientSettingsProxy()
+	} else {
+		this.setClientSettingsDisk()
+	}
+
+	this.SetServiceRoots(localRoots, writableLocalRoots, gatewayRoots)
+	return nil
+}
diff --git a/sdk/go/keepclient/discover_test.go b/sdk/go/keepclient/discover_test.go
new file mode 100644
index 0000000..eab01f5
--- /dev/null
+++ b/sdk/go/keepclient/discover_test.go
@@ -0,0 +1,18 @@
+package keepclient
+
+import (
+	"fmt"
+	"time"
+
+	"git.curoverse.com/arvados.git/sdk/go/arvadosclient"
+)
+
+func ExampleRefreshServices() {
+	arv, err := arvadosclient.MakeArvadosClient()
+	if err != nil {
+		panic(err)
+	}
+	kc := MakeKeepClient(arv)
+	go kc.RefreshServices(5*time.Minute, 3*time.Second)
+	fmt.Printf("LocalRoots: %#v\n", kc.LocalRoots())
+}
diff --git a/sdk/go/keepclient/support.go b/sdk/go/keepclient/support.go
index 8414afa..90ac3bc 100644
--- a/sdk/go/keepclient/support.go
+++ b/sdk/go/keepclient/support.go
@@ -2,7 +2,6 @@ package keepclient
 
 import (
 	"crypto/md5"
-	"encoding/json"
 	"errors"
 	"fmt"
 	"git.curoverse.com/arvados.git/sdk/go/streamer"
@@ -87,86 +86,6 @@ type svcList struct {
 	Items []keepService `json:"items"`
 }
 
-// DiscoverKeepServers gets list of available keep services from api server
-func (this *KeepClient) DiscoverKeepServers() error {
-	var list svcList
-
-	// Get keep services from api server
-	err := this.Arvados.Call("GET", "keep_services", "", "accessible", nil, &list)
-	if err != nil {
-		return err
-	}
-
-	return this.loadKeepServers(list)
-}
-
-// LoadKeepServicesFromJSON gets list of available keep services from given JSON
-func (this *KeepClient) LoadKeepServicesFromJSON(services string) error {
-	var list svcList
-
-	// Load keep services from given json
-	dec := json.NewDecoder(strings.NewReader(services))
-	if err := dec.Decode(&list); err != nil {
-		return err
-	}
-
-	return this.loadKeepServers(list)
-}
-
-// loadKeepServers
-func (this *KeepClient) loadKeepServers(list svcList) error {
-	listed := make(map[string]bool)
-	localRoots := make(map[string]string)
-	gatewayRoots := make(map[string]string)
-	writableLocalRoots := make(map[string]string)
-
-	// replicasPerService is 1 for disks; unknown or unlimited otherwise
-	this.replicasPerService = 1
-	this.Using_proxy = false
-
-	for _, service := range list.Items {
-		scheme := "http"
-		if service.SSL {
-			scheme = "https"
-		}
-		url := fmt.Sprintf("%s://%s:%d", scheme, service.Hostname, service.Port)
-
-		// Skip duplicates
-		if listed[url] {
-			continue
-		}
-		listed[url] = true
-
-		localRoots[service.Uuid] = url
-		if service.SvcType == "proxy" {
-			this.Using_proxy = true
-		}
-
-		if service.ReadOnly == false {
-			writableLocalRoots[service.Uuid] = url
-			if service.SvcType != "disk" {
-				this.replicasPerService = 0
-			}
-		}
-
-		// Gateway services are only used when specified by
-		// UUID, so there's nothing to gain by filtering them
-		// by service type. Including all accessible services
-		// (gateway and otherwise) merely accommodates more
-		// service configurations.
-		gatewayRoots[service.Uuid] = url
-	}
-
-	if this.Using_proxy {
-		this.setClientSettingsProxy()
-	} else {
-		this.setClientSettingsDisk()
-	}
-
-	this.SetServiceRoots(localRoots, writableLocalRoots, gatewayRoots)
-	return nil
-}
-
 type uploadStatus struct {
 	err             error
 	url             string
diff --git a/services/keepproxy/keepproxy.go b/services/keepproxy/keepproxy.go
index 2efc9ce..7b5cd2b 100644
--- a/services/keepproxy/keepproxy.go
+++ b/services/keepproxy/keepproxy.go
@@ -14,7 +14,6 @@ import (
 	"net/http"
 	"os"
 	"os/signal"
-	"reflect"
 	"regexp"
 	"sync"
 	"syscall"
@@ -104,7 +103,7 @@ func main() {
 
 	kc.Want_replicas = default_replicas
 	kc.Client.Timeout = time.Duration(timeout) * time.Second
-	go RefreshServicesList(kc, 5*time.Minute, 3*time.Second)
+	go kc.RefreshServices(5*time.Minute, 3*time.Second)
 
 	listener, err = net.Listen("tcp", listen)
 	if err != nil {
@@ -135,42 +134,6 @@ type ApiTokenCache struct {
 	expireTime int64
 }
 
-// Refresh the keep service list on SIGHUP; when the given interval
-// has elapsed since the last refresh; and (if the last refresh
-// failed) the given errInterval has elapsed.
-func RefreshServicesList(kc *keepclient.KeepClient, interval, errInterval time.Duration) {
-	var previousRoots = []map[string]string{}
-
-	timer := time.NewTimer(interval)
-	gotHUP := make(chan os.Signal, 1)
-	signal.Notify(gotHUP, syscall.SIGHUP)
-
-	for {
-		select {
-		case <-gotHUP:
-		case <-timer.C:
-		}
-		timer.Reset(interval)
-
-		if err := kc.DiscoverKeepServers(); err != nil {
-			log.Println("Error retrieving services list: %v (retrying in %v)", err, errInterval)
-			timer.Reset(errInterval)
-			continue
-		}
-		newRoots := []map[string]string{kc.LocalRoots(), kc.GatewayRoots()}
-
-		if !reflect.DeepEqual(previousRoots, newRoots) {
-			log.Printf("Updated services list: locals %v gateways %v", newRoots[0], newRoots[1])
-			previousRoots = newRoots
-		}
-
-		if len(newRoots[0]) == 0 {
-			log.Printf("WARNING: No local services (retrying in %v)", errInterval)
-			timer.Reset(errInterval)
-		}
-	}
-}
-
 // Cache the token and set an expire time.  If we already have an expire time
 // on the token, it is not updated.
 func (this *ApiTokenCache) RememberToken(token string) {

commit c42c5968dd554a96bf0b67ea52a0bab3528035e2
Author: Tom Clegg <tom at curoverse.com>
Date:   Sat Nov 7 04:00:50 2015 -0500

    5824: Fix server shutdown code.
    
    * Pay attention to --num-keep-servers in stop_keep.
    
    * Wait for processes to exit, to avoid start/stop races.
    
    * Bring back exception handling for getpgid() erroneously removed in
      6b1b4d80445f0e03f89c46a167bebefe7bcf97c0 (and scope it correctly
      this time).

diff --git a/sdk/python/tests/run_test_server.py b/sdk/python/tests/run_test_server.py
index 8d1c202..25cc6a7 100644
--- a/sdk/python/tests/run_test_server.py
+++ b/sdk/python/tests/run_test_server.py
@@ -83,11 +83,15 @@ def kill_server_pid(pidfile, wait=10, passenger_root=False):
         with open(pidfile, 'r') as f:
             server_pid = int(f.read())
         while now <= timeout:
+            try:
+                os.getpgid(server_pid)
+            except OSError:
+                # process has already disappeared
+                break
             if not passenger_root or timeout - now < wait / 2:
                 # Half timeout has elapsed. Start sending SIGTERM
+                print("Sending TERM to {} ({})".format(server_pid, pidfile))
                 os.kill(server_pid, signal.SIGTERM)
-            # Raise OSError if process has disappeared
-            os.getpgid(server_pid)
             time.sleep(0.1)
             now = time.time()
     except EnvironmentError:
@@ -388,7 +392,7 @@ def run_keep(blob_signing_key=None, enforce_permissions=False, num_servers=2):
         os.kill(int(open(proxypidfile).read()), signal.SIGHUP)
 
 def _stop_keep(n):
-    kill_server_pid(_pidfile('keep{}'.format(n)), 0)
+    kill_server_pid(_pidfile('keep{}'.format(n)))
     if os.path.exists("{}/keep{}.volume".format(TEST_TMPDIR, n)):
         with open("{}/keep{}.volume".format(TEST_TMPDIR, n), 'r') as r:
             shutil.rmtree(r.read(), True)
@@ -436,7 +440,7 @@ def run_keep_proxy():
 def stop_keep_proxy():
     if 'ARVADOS_TEST_PROXY_SERVICES' in os.environ:
         return
-    kill_server_pid(_pidfile('keepproxy'), wait=0)
+    kill_server_pid(_pidfile('keepproxy'))
 
 def run_arv_git_httpd():
     if 'ARVADOS_TEST_PROXY_SERVICES' in os.environ:
@@ -461,7 +465,7 @@ def run_arv_git_httpd():
 def stop_arv_git_httpd():
     if 'ARVADOS_TEST_PROXY_SERVICES' in os.environ:
         return
-    kill_server_pid(_pidfile('arv-git-httpd'), wait=0)
+    kill_server_pid(_pidfile('arv-git-httpd'))
 
 def run_keep_web():
     if 'ARVADOS_TEST_PROXY_SERVICES' in os.environ:
@@ -486,7 +490,7 @@ def run_keep_web():
 def stop_keep_web():
     if 'ARVADOS_TEST_PROXY_SERVICES' in os.environ:
         return
-    kill_server_pid(_pidfile('keep-web'), wait=0)
+    kill_server_pid(_pidfile('keep-web'))
 
 def run_nginx():
     if 'ARVADOS_TEST_PROXY_SERVICES' in os.environ:
@@ -526,7 +530,7 @@ def run_nginx():
 def stop_nginx():
     if 'ARVADOS_TEST_PROXY_SERVICES' in os.environ:
         return
-    kill_server_pid(_pidfile('nginx'), wait=0)
+    kill_server_pid(_pidfile('nginx'))
 
 def _pidfile(program):
     return os.path.join(TEST_TMPDIR, program + '.pid')
@@ -678,7 +682,7 @@ if __name__ == "__main__":
     elif args.action == 'start_keep':
         run_keep(enforce_permissions=args.keep_enforce_permissions, num_servers=args.num_keep_servers)
     elif args.action == 'stop_keep':
-        stop_keep()
+        stop_keep(num_servers=args.num_keep_servers)
     elif args.action == 'start_keep_proxy':
         run_keep_proxy()
     elif args.action == 'stop_keep_proxy':

commit 6ccb59197f1ad3ba831c7936e8f5c0e9adccb963
Author: Tom Clegg <tom at curoverse.com>
Date:   Sat Nov 7 03:54:03 2015 -0500

    5824: Fix Keep server shutdown, check errors, simplify stderr redirection.
    
    (Oops, we forgot to actually Run() the python command for stop_keep.)

diff --git a/sdk/go/arvadostest/run_servers.go b/sdk/go/arvadostest/run_servers.go
index 27c552a..00dc98f 100644
--- a/sdk/go/arvadostest/run_servers.go
+++ b/sdk/go/arvadostest/run_servers.go
@@ -3,7 +3,6 @@ package arvadostest
 import (
 	"bufio"
 	"bytes"
-	"fmt"
 	"io"
 	"io/ioutil"
 	"log"
@@ -113,14 +112,10 @@ func StartKeep(numKeepServers int, enforcePermissions bool) {
 	}
 
 	cmd := exec.Command("python", cmdArgs...)
-
-	stderr, err := cmd.StderrPipe()
-	if err != nil {
-		log.Fatalf("Setting up stderr pipe: %s", err)
-	}
-	go io.Copy(os.Stderr, stderr)
+	cmd.Stderr = os.Stderr
+	cmd.Stdout = os.Stderr
 	if err := cmd.Run(); err != nil {
-		panic(fmt.Sprintf("'python run_test_server.py start_keep' returned error %s", err))
+		log.Fatalf("%+v: %s", cmd.Args, err)
 	}
 }
 
@@ -132,5 +127,10 @@ func StopKeep(numKeepServers int) {
 	defer os.Chdir(cwd)
 	chdirToPythonTests()
 
-	exec.Command("python", "run_test_server.py", "stop_keep", "--num-keep-servers", strconv.Itoa(numKeepServers))
+	cmd := exec.Command("python", "run_test_server.py", "stop_keep", "--num-keep-servers", strconv.Itoa(numKeepServers))
+	cmd.Stderr = os.Stderr
+	cmd.Stdout = os.Stderr
+	if err := cmd.Run(); err != nil {
+		log.Fatalf("%+v: %s", cmd.Args, err)
+	}
 }

commit 100b485f4295cbb414ed743b2fdba990591272e7
Author: Tom Clegg <tom at curoverse.com>
Date:   Sat Nov 7 02:22:07 2015 -0500

    5824: Use fifo2stderr for arv-git-httpd and keep-web logs, too.

diff --git a/sdk/python/tests/run_test_server.py b/sdk/python/tests/run_test_server.py
index afc78ce..8d1c202 100644
--- a/sdk/python/tests/run_test_server.py
+++ b/sdk/python/tests/run_test_server.py
@@ -447,11 +447,12 @@ def run_arv_git_httpd():
     gitport = find_available_port()
     env = os.environ.copy()
     env.pop('ARVADOS_API_TOKEN', None)
+    logf = open(_fifo2stderr('arv-git-httpd'), 'w')
     agh = subprocess.Popen(
         ['arv-git-httpd',
          '-repo-root='+gitdir+'/test',
          '-address=:'+str(gitport)],
-        env=env, stdin=open('/dev/null'), stdout=sys.stderr)
+        env=env, stdin=open('/dev/null'), stdout=logf, stderr=logf)
     with open(_pidfile('arv-git-httpd'), 'w') as f:
         f.write(str(agh.pid))
     _setport('arv-git-httpd', gitport)
@@ -470,12 +471,13 @@ def run_keep_web():
     keepwebport = find_available_port()
     env = os.environ.copy()
     env['ARVADOS_API_TOKEN'] = auth_token('anonymous')
+    logf = open(_fifo2stderr('keep-web'), 'w')
     keepweb = subprocess.Popen(
         ['keep-web',
          '-allow-anonymous',
          '-attachment-only-host=localhost:'+str(keepwebport),
          '-listen=:'+str(keepwebport)],
-        env=env, stdin=open('/dev/null'), stdout=sys.stderr)
+        env=env, stdin=open('/dev/null'), stdout=logf, stderr=logf)
     with open(_pidfile('keep-web'), 'w') as f:
         f.write(str(keepweb.pid))
     _setport('keep-web', keepwebport)

commit 745cfb876277f0c65244d570c1736fb936446f35
Author: Tom Clegg <tom at curoverse.com>
Date:   Fri Nov 6 16:58:32 2015 -0500

    5824: Sync test suite to new keep-web argument names.

diff --git a/sdk/python/tests/run_test_server.py b/sdk/python/tests/run_test_server.py
index b58c611..afc78ce 100644
--- a/sdk/python/tests/run_test_server.py
+++ b/sdk/python/tests/run_test_server.py
@@ -469,12 +469,12 @@ def run_keep_web():
 
     keepwebport = find_available_port()
     env = os.environ.copy()
-    env.pop('ARVADOS_API_TOKEN', None)
+    env['ARVADOS_API_TOKEN'] = auth_token('anonymous')
     keepweb = subprocess.Popen(
         ['keep-web',
-         '-anonymous-token='+fixture('api_client_authorizations')['anonymous']['api_token'],
+         '-allow-anonymous',
          '-attachment-only-host=localhost:'+str(keepwebport),
-         '-address=:'+str(keepwebport)],
+         '-listen=:'+str(keepwebport)],
         env=env, stdin=open('/dev/null'), stdout=sys.stderr)
     with open(_pidfile('keep-web'), 'w') as f:
         f.write(str(keepweb.pid))

commit b1ef43a1dbb4a66e1646fc68ac88ff6d54026ac1
Merge: 767e119 4ec9919
Author: Tom Clegg <tom at curoverse.com>
Date:   Fri Nov 6 16:53:01 2015 -0500

    5824: Merge branch 'master' into 5824-keep-web-workbench


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


hooks/post-receive
-- 




More information about the arvados-commits mailing list