[ARVADOS] updated: feff3b35134c95961d9846836491a3ad1600969c

Git user git at public.curoverse.com
Tue Sep 27 13:46:50 EDT 2016


Summary of changes:
 .../app/views/work_units/_show_log.html.erb        |   8 +-
 .../container_requests_controller_test.rb          |  17 ++-
 crunch_scripts/cwl-runner                          |   4 +-
 sdk/go/arvadosclient/arvadosclient.go              |  18 ++--
 sdk/go/arvadosclient/pool.go                       |   2 +-
 sdk/go/crunchrunner/crunchrunner.go                |   2 +-
 sdk/go/dispatch/dispatch.go                        |   2 +-
 sdk/go/keepclient/collectionreader_test.go         |   4 +-
 sdk/go/keepclient/discover_test.go                 |   6 +-
 sdk/go/keepclient/keepclient_test.go               |  62 +++++------
 sdk/go/logger/logger.go                            |   6 +-
 sdk/go/util/util.go                                |   4 +-
 services/api/app/models/container.rb               |  11 +-
 services/api/app/models/container_request.rb       |   5 +-
 services/api/config/application.default.yml        |   6 ++
 .../api/lib/tasks/delete_old_container_logs.rake   |  14 +++
 services/api/lib/tasks/delete_old_job_logs.rake    |  11 +-
 services/api/test/fixtures/container_requests.yml  |   2 +-
 services/api/test/fixtures/containers.yml          |  63 +++++++++++-
 services/api/test/fixtures/logs.yml                | 114 +++++++++++++++++++++
 .../test/tasks/delete_old_container_logs_test.rb   |  50 +++++++++
 services/api/test/unit/container_request_test.rb   |  15 ++-
 services/api/test/unit/container_test.rb           |  18 ++--
 services/api/test/unit/repository_test.rb          |   8 +-
 .../crunch-dispatch-local_test.go                  |   2 +-
 .../crunch-dispatch-slurm_test.go                  |   2 +-
 services/crunch-run/crunchrun.go                   |  27 +++--
 services/crunch-run/crunchrun_test.go              |  10 ++
 services/datamanager/collection/collection.go      |   2 +-
 services/datamanager/collection/collection_test.go |   2 +-
 services/datamanager/datamanager.go                |   6 +-
 services/datamanager/datamanager_test.go           |   4 +-
 services/datamanager/keep/keep.go                  |   8 +-
 services/datamanager/keep/keep_test.go             |  16 +--
 services/keep-balance/integration_test.go          |   2 +-
 services/keep-web/server_test.go                   |   4 +-
 services/keepproxy/keepproxy_test.go               |   4 +-
 services/keepstore/pull_worker_integration_test.go |   2 +-
 services/keepstore/pull_worker_test.go             |   2 +-
 39 files changed, 415 insertions(+), 130 deletions(-)
 create mode 100644 services/api/lib/tasks/delete_old_container_logs.rake
 create mode 100644 services/api/test/tasks/delete_old_container_logs_test.rb

  discards  0211e7c4e89dd855c6bb2628465902bc1f397ee9 (commit)
       via  feff3b35134c95961d9846836491a3ad1600969c (commit)
       via  c803477b8025556c8235c033c69468b852b1bfe3 (commit)
       via  35ff079469606b58e2ec17631045d7f7ec0e3b53 (commit)
       via  7bc11e7efb6e742f412dc24ec9394f3c9f098de4 (commit)
       via  69972d44f4806ea5ff89b9ba1afd6f51848968c9 (commit)
       via  91528dbaf47e8b89670b733f6cb5ce3709722b60 (commit)
       via  e0f4124bd0156f7cb029e5330e256086962f9e8e (commit)
       via  e1d958bc57d64055bcd08d84bd3b86823f1ebc5d (commit)
       via  f40544f6523bf2d54b288a64af7cab7469741512 (commit)
       via  89b46374d746fe785e3ba0088f0886caa17893db (commit)
       via  9f179a8b8acf6fa23091445fc1332357d1fe1644 (commit)
       via  8d61948fecccfc60db2f18ba4daf7c01ddf3d3c8 (commit)
       via  e3de0a09618c8aa46833357e6eacabe15bb68f6f (commit)

This update added new revisions after undoing existing revisions.  That is
to say, the old revision is not a strict subset of the new revision.  This
situation occurs when you --force push a change and generate a repository
containing something like this:

 * -- * -- B -- O -- O -- O (0211e7c4e89dd855c6bb2628465902bc1f397ee9)
            \
             N -- N -- N (feff3b35134c95961d9846836491a3ad1600969c)

When this happens we assume that you've already had alert emails for all
of the O revisions, and so we here report only the revisions in the N
branch from the common base, B.

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 feff3b35134c95961d9846836491a3ad1600969c
Author: Tom Clegg <tom at curoverse.com>
Date:   Tue Sep 27 12:49:49 2016 -0400

    9848: Set expiry time on container output and log collections.

diff --git a/services/crunch-run/crunchrun.go b/services/crunch-run/crunchrun.go
index 02f2ee9..d804c01 100644
--- a/services/crunch-run/crunchrun.go
+++ b/services/crunch-run/crunchrun.go
@@ -29,8 +29,9 @@ import (
 type IArvadosClient interface {
 	Create(resourceType string, parameters arvadosclient.Dict, output interface{}) error
 	Get(resourceType string, uuid string, parameters arvadosclient.Dict, output interface{}) error
-	Update(resourceType string, uuid string, parameters arvadosclient.Dict, output interface{}) (err error)
-	Call(method, resourceType, uuid, action string, parameters arvadosclient.Dict, output interface{}) (err error)
+	Update(resourceType string, uuid string, parameters arvadosclient.Dict, output interface{}) error
+	Call(method, resourceType, uuid, action string, parameters arvadosclient.Dict, output interface{}) error
+	Discovery(key string) (interface{}, error)
 }
 
 // ErrCancelled is the error returned when the container is cancelled.
@@ -93,6 +94,7 @@ type ContainerRunner struct {
 	SigChan        chan os.Signal
 	ArvMountExit   chan error
 	finalState     string
+	trashLifetime  time.Duration
 
 	statLogger   io.WriteCloser
 	statReporter *crunchstat.Reporter
@@ -601,18 +603,25 @@ func (runner *ContainerRunner) CaptureOutput() error {
 	err = runner.ArvClient.Create("collections",
 		arvadosclient.Dict{
 			"collection": arvadosclient.Dict{
+				"expires_at":    time.Now().Add(runner.trashLifetime).Format(time.RFC3339),
+				"name":          "output for " + runner.Container.UUID,
 				"manifest_text": manifestText}},
 		&response)
 	if err != nil {
 		return fmt.Errorf("While creating output collection: %v", err)
 	}
-
-	runner.OutputPDH = new(string)
-	*runner.OutputPDH = response.PortableDataHash
-
+	runner.OutputPDH = &response.PortableDataHash
 	return nil
 }
 
+func (runner *ContainerRunner) loadDiscoveryVars() {
+	tl, err := runner.ArvClient.Discovery("defaultTrashLifetime")
+	if err != nil {
+		log.Fatalf("getting defaultTrashLifetime from discovery document: %s", err)
+	}
+	runner.trashLifetime = time.Duration(tl.(float64)) * time.Second
+}
+
 func (runner *ContainerRunner) CleanupDirs() {
 	if runner.ArvMount != nil {
 		umount := exec.Command("fusermount", "-z", "-u", runner.ArvMountPoint)
@@ -665,15 +674,14 @@ func (runner *ContainerRunner) CommitLogs() error {
 	err = runner.ArvClient.Create("collections",
 		arvadosclient.Dict{
 			"collection": arvadosclient.Dict{
+				"expires_at":    time.Now().Add(runner.trashLifetime).Format(time.RFC3339),
 				"name":          "logs for " + runner.Container.UUID,
 				"manifest_text": mt}},
 		&response)
 	if err != nil {
 		return fmt.Errorf("While creating log collection: %v", err)
 	}
-
 	runner.LogsPDH = &response.PortableDataHash
-
 	return nil
 }
 
@@ -858,6 +866,7 @@ func NewContainerRunner(api IArvadosClient,
 	cr.Container.UUID = containerUUID
 	cr.CrunchLog = NewThrottledLogger(cr.NewLogWriter("crunch-run"))
 	cr.CrunchLog.Immediate = log.New(os.Stderr, containerUUID+" ", 0)
+	cr.loadDiscoveryVars()
 	return cr
 }
 

commit c803477b8025556c8235c033c69468b852b1bfe3
Author: Tom Clegg <tom at curoverse.com>
Date:   Tue Sep 27 12:48:16 2016 -0400

    9848: Use pointer receiver for all arvadosclient.ArvadosClient methods.
    
    This makes it possible for all methods to satisfy a single interface.

diff --git a/sdk/go/arvadosclient/arvadosclient.go b/sdk/go/arvadosclient/arvadosclient.go
index e3cbfcf..5f24c71 100644
--- a/sdk/go/arvadosclient/arvadosclient.go
+++ b/sdk/go/arvadosclient/arvadosclient.go
@@ -125,12 +125,12 @@ func New(c *arvados.Client) (*ArvadosClient, error) {
 // environment variables ARVADOS_API_HOST, ARVADOS_API_TOKEN,
 // ARVADOS_API_HOST_INSECURE, ARVADOS_EXTERNAL_CLIENT, and
 // ARVADOS_KEEP_SERVICES.
-func MakeArvadosClient() (ac ArvadosClient, err error) {
+func MakeArvadosClient() (ac *ArvadosClient, err error) {
 	var matchTrue = regexp.MustCompile("^(?i:1|yes|true)$")
 	insecure := matchTrue.MatchString(os.Getenv("ARVADOS_API_HOST_INSECURE"))
 	external := matchTrue.MatchString(os.Getenv("ARVADOS_EXTERNAL_CLIENT"))
 
-	ac = ArvadosClient{
+	ac = &ArvadosClient{
 		Scheme:      "https",
 		ApiServer:   os.Getenv("ARVADOS_API_HOST"),
 		ApiToken:    os.Getenv("ARVADOS_API_TOKEN"),
@@ -166,7 +166,7 @@ func MakeArvadosClient() (ac ArvadosClient, err error) {
 
 // CallRaw is the same as Call() but returns a Reader that reads the
 // response body, instead of taking an output object.
-func (c ArvadosClient) CallRaw(method string, resourceType string, uuid string, action string, parameters Dict) (reader io.ReadCloser, err error) {
+func (c *ArvadosClient) CallRaw(method string, resourceType string, uuid string, action string, parameters Dict) (reader io.ReadCloser, err error) {
 	scheme := c.Scheme
 	if scheme == "" {
 		scheme = "https"
@@ -312,7 +312,7 @@ func newAPIServerError(ServerAddress string, resp *http.Response) APIServerError
 // Returns a non-nil error if an error occurs making the API call, the
 // API responds with a non-successful HTTP status, or an error occurs
 // parsing the response body.
-func (c ArvadosClient) Call(method, resourceType, uuid, action string, parameters Dict, output interface{}) error {
+func (c *ArvadosClient) Call(method, resourceType, uuid, action string, parameters Dict, output interface{}) error {
 	reader, err := c.CallRaw(method, resourceType, uuid, action, parameters)
 	if reader != nil {
 		defer reader.Close()
@@ -331,22 +331,22 @@ func (c ArvadosClient) Call(method, resourceType, uuid, action string, parameter
 }
 
 // Create a new resource. See Call for argument descriptions.
-func (c ArvadosClient) Create(resourceType string, parameters Dict, output interface{}) error {
+func (c *ArvadosClient) Create(resourceType string, parameters Dict, output interface{}) error {
 	return c.Call("POST", resourceType, "", "", parameters, output)
 }
 
 // Delete a resource. See Call for argument descriptions.
-func (c ArvadosClient) Delete(resource string, uuid string, parameters Dict, output interface{}) (err error) {
+func (c *ArvadosClient) Delete(resource string, uuid string, parameters Dict, output interface{}) (err error) {
 	return c.Call("DELETE", resource, uuid, "", parameters, output)
 }
 
 // Modify attributes of a resource. See Call for argument descriptions.
-func (c ArvadosClient) Update(resourceType string, uuid string, parameters Dict, output interface{}) (err error) {
+func (c *ArvadosClient) Update(resourceType string, uuid string, parameters Dict, output interface{}) (err error) {
 	return c.Call("PUT", resourceType, uuid, "", parameters, output)
 }
 
 // Get a resource. See Call for argument descriptions.
-func (c ArvadosClient) Get(resourceType string, uuid string, parameters Dict, output interface{}) (err error) {
+func (c *ArvadosClient) Get(resourceType string, uuid string, parameters Dict, output interface{}) (err error) {
 	if !UUIDMatch(uuid) && !(resourceType == "collections" && PDHMatch(uuid)) {
 		// No object has uuid == "": there is no need to make
 		// an API call. Furthermore, the HTTP request for such
@@ -358,7 +358,7 @@ func (c ArvadosClient) Get(resourceType string, uuid string, parameters Dict, ou
 }
 
 // List resources of a given type. See Call for argument descriptions.
-func (c ArvadosClient) List(resource string, parameters Dict, output interface{}) (err error) {
+func (c *ArvadosClient) List(resource string, parameters Dict, output interface{}) (err error) {
 	return c.Call("GET", resource, "", "", parameters, output)
 }
 
diff --git a/sdk/go/arvadosclient/pool.go b/sdk/go/arvadosclient/pool.go
index 26b3518..6b3171f 100644
--- a/sdk/go/arvadosclient/pool.go
+++ b/sdk/go/arvadosclient/pool.go
@@ -23,7 +23,7 @@ type ClientPool struct {
 func MakeClientPool() *ClientPool {
 	proto, err := MakeArvadosClient()
 	return &ClientPool{
-		Prototype: &proto,
+		Prototype: proto,
 		lastErr:   err,
 	}
 }
diff --git a/sdk/go/crunchrunner/crunchrunner.go b/sdk/go/crunchrunner/crunchrunner.go
index 040d7c2..5c3d65c 100644
--- a/sdk/go/crunchrunner/crunchrunner.go
+++ b/sdk/go/crunchrunner/crunchrunner.go
@@ -379,7 +379,7 @@ func main() {
 	}
 
 	var kc IKeepClient
-	kc, err = keepclient.MakeKeepClient(&api)
+	kc, err = keepclient.MakeKeepClient(api)
 	if err != nil {
 		log.Fatal(err)
 	}
diff --git a/sdk/go/dispatch/dispatch.go b/sdk/go/dispatch/dispatch.go
index a486132..4987c01 100644
--- a/sdk/go/dispatch/dispatch.go
+++ b/sdk/go/dispatch/dispatch.go
@@ -25,7 +25,7 @@ const (
 // Dispatcher holds the state of the dispatcher
 type Dispatcher struct {
 	// The Arvados client
-	Arv arvadosclient.ArvadosClient
+	Arv *arvadosclient.ArvadosClient
 
 	// When a new queued container appears and is either already owned by
 	// this dispatcher or is successfully locked, the dispatcher will call
diff --git a/sdk/go/keepclient/collectionreader_test.go b/sdk/go/keepclient/collectionreader_test.go
index 2cc2373..be4f386 100644
--- a/sdk/go/keepclient/collectionreader_test.go
+++ b/sdk/go/keepclient/collectionreader_test.go
@@ -19,7 +19,7 @@ import (
 var _ = check.Suite(&CollectionReaderUnit{})
 
 type CollectionReaderUnit struct {
-	arv     arvadosclient.ArvadosClient
+	arv     *arvadosclient.ArvadosClient
 	kc      *KeepClient
 	handler SuccessHandler
 }
@@ -30,7 +30,7 @@ func (s *CollectionReaderUnit) SetUpTest(c *check.C) {
 	c.Assert(err, check.IsNil)
 	s.arv.ApiToken = arvadostest.ActiveToken
 
-	s.kc, err = MakeKeepClient(&s.arv)
+	s.kc, err = MakeKeepClient(s.arv)
 	c.Assert(err, check.IsNil)
 
 	s.handler = SuccessHandler{
diff --git a/sdk/go/keepclient/discover_test.go b/sdk/go/keepclient/discover_test.go
index ee31ea0..379d44c 100644
--- a/sdk/go/keepclient/discover_test.go
+++ b/sdk/go/keepclient/discover_test.go
@@ -17,7 +17,7 @@ func ExampleKeepClient_RefreshServices() {
 	if err != nil {
 		panic(err)
 	}
-	kc, err := MakeKeepClient(&arv)
+	kc, err := MakeKeepClient(arv)
 	if err != nil {
 		panic(err)
 	}
@@ -53,9 +53,9 @@ func (s *ServerRequiredSuite) TestOverrideDiscovery(c *check.C) {
 	// arv2 should use our stub servers, but one created for arv1
 	// should not.
 
-	kc1, err := MakeKeepClient(&arv1)
+	kc1, err := MakeKeepClient(arv1)
 	c.Assert(err, check.IsNil)
-	kc2, err := MakeKeepClient(&arv2)
+	kc2, err := MakeKeepClient(arv2)
 	c.Assert(err, check.IsNil)
 
 	_, _, _, err = kc1.Get(hash)
diff --git a/sdk/go/keepclient/keepclient_test.go b/sdk/go/keepclient/keepclient_test.go
index 4ba1d7c..bd36d9d 100644
--- a/sdk/go/keepclient/keepclient_test.go
+++ b/sdk/go/keepclient/keepclient_test.go
@@ -62,7 +62,7 @@ func (s *ServerRequiredSuite) TestMakeKeepClient(c *C) {
 	arv, err := arvadosclient.MakeArvadosClient()
 	c.Assert(err, Equals, nil)
 
-	kc, err := MakeKeepClient(&arv)
+	kc, err := MakeKeepClient(arv)
 
 	c.Assert(err, Equals, nil)
 	c.Check(len(kc.LocalRoots()), Equals, 2)
@@ -75,15 +75,15 @@ func (s *ServerRequiredSuite) TestDefaultReplications(c *C) {
 	arv, err := arvadosclient.MakeArvadosClient()
 	c.Assert(err, Equals, nil)
 
-	kc, err := MakeKeepClient(&arv)
+	kc, err := MakeKeepClient(arv)
 	c.Assert(kc.Want_replicas, Equals, 2)
 
 	arv.DiscoveryDoc["defaultCollectionReplication"] = 3.0
-	kc, err = MakeKeepClient(&arv)
+	kc, err = MakeKeepClient(arv)
 	c.Assert(kc.Want_replicas, Equals, 3)
 
 	arv.DiscoveryDoc["defaultCollectionReplication"] = 1.0
-	kc, err = MakeKeepClient(&arv)
+	kc, err = MakeKeepClient(arv)
 	c.Assert(kc.Want_replicas, Equals, 1)
 }
 
@@ -125,7 +125,7 @@ func UploadToStubHelper(c *C, st http.Handler, f func(*KeepClient, string,
 	arv, _ := arvadosclient.MakeArvadosClient()
 	arv.ApiToken = "abc123"
 
-	kc, _ := MakeKeepClient(&arv)
+	kc, _ := MakeKeepClient(arv)
 
 	reader, writer := io.Pipe()
 	upload_status := make(chan uploadStatus)
@@ -269,7 +269,7 @@ func (s *StandaloneSuite) TestPutB(c *C) {
 		make(chan string, 5)}
 
 	arv, _ := arvadosclient.MakeArvadosClient()
-	kc, _ := MakeKeepClient(&arv)
+	kc, _ := MakeKeepClient(arv)
 
 	kc.Want_replicas = 2
 	arv.ApiToken = "abc123"
@@ -310,7 +310,7 @@ func (s *StandaloneSuite) TestPutHR(c *C) {
 		make(chan string, 5)}
 
 	arv, _ := arvadosclient.MakeArvadosClient()
-	kc, _ := MakeKeepClient(&arv)
+	kc, _ := MakeKeepClient(arv)
 
 	kc.Want_replicas = 2
 	arv.ApiToken = "abc123"
@@ -361,7 +361,7 @@ func (s *StandaloneSuite) TestPutWithFail(c *C) {
 		make(chan string, 1)}
 
 	arv, err := arvadosclient.MakeArvadosClient()
-	kc, _ := MakeKeepClient(&arv)
+	kc, _ := MakeKeepClient(arv)
 
 	kc.Want_replicas = 2
 	arv.ApiToken = "abc123"
@@ -419,7 +419,7 @@ func (s *StandaloneSuite) TestPutWithTooManyFail(c *C) {
 		make(chan string, 4)}
 
 	arv, err := arvadosclient.MakeArvadosClient()
-	kc, _ := MakeKeepClient(&arv)
+	kc, _ := MakeKeepClient(arv)
 
 	kc.Want_replicas = 2
 	kc.Retries = 0
@@ -480,7 +480,7 @@ func (s *StandaloneSuite) TestGet(c *C) {
 	defer ks.listener.Close()
 
 	arv, err := arvadosclient.MakeArvadosClient()
-	kc, _ := MakeKeepClient(&arv)
+	kc, _ := MakeKeepClient(arv)
 	arv.ApiToken = "abc123"
 	kc.SetServiceRoots(map[string]string{"x": ks.url}, nil, nil)
 
@@ -504,7 +504,7 @@ func (s *StandaloneSuite) TestGet404(c *C) {
 	defer ks.listener.Close()
 
 	arv, err := arvadosclient.MakeArvadosClient()
-	kc, _ := MakeKeepClient(&arv)
+	kc, _ := MakeKeepClient(arv)
 	arv.ApiToken = "abc123"
 	kc.SetServiceRoots(map[string]string{"x": ks.url}, nil, nil)
 
@@ -524,7 +524,7 @@ func (s *StandaloneSuite) TestGetFail(c *C) {
 	defer ks.listener.Close()
 
 	arv, err := arvadosclient.MakeArvadosClient()
-	kc, _ := MakeKeepClient(&arv)
+	kc, _ := MakeKeepClient(arv)
 	arv.ApiToken = "abc123"
 	kc.SetServiceRoots(map[string]string{"x": ks.url}, nil, nil)
 	kc.Retries = 0
@@ -554,7 +554,7 @@ func (s *StandaloneSuite) TestGetFailRetry(c *C) {
 	defer ks.listener.Close()
 
 	arv, err := arvadosclient.MakeArvadosClient()
-	kc, _ := MakeKeepClient(&arv)
+	kc, _ := MakeKeepClient(arv)
 	arv.ApiToken = "abc123"
 	kc.SetServiceRoots(map[string]string{"x": ks.url}, nil, nil)
 
@@ -573,7 +573,7 @@ func (s *StandaloneSuite) TestGetNetError(c *C) {
 	hash := fmt.Sprintf("%x", md5.Sum([]byte("foo")))
 
 	arv, err := arvadosclient.MakeArvadosClient()
-	kc, _ := MakeKeepClient(&arv)
+	kc, _ := MakeKeepClient(arv)
 	arv.ApiToken = "abc123"
 	kc.SetServiceRoots(map[string]string{"x": "http://localhost:62222"}, nil, nil)
 
@@ -609,7 +609,7 @@ func (s *StandaloneSuite) TestGetWithServiceHint(c *C) {
 	defer ks.listener.Close()
 
 	arv, err := arvadosclient.MakeArvadosClient()
-	kc, _ := MakeKeepClient(&arv)
+	kc, _ := MakeKeepClient(arv)
 	arv.ApiToken = "abc123"
 	kc.SetServiceRoots(
 		map[string]string{"x": ks0.url},
@@ -652,7 +652,7 @@ func (s *StandaloneSuite) TestGetWithLocalServiceHint(c *C) {
 	defer ks.listener.Close()
 
 	arv, err := arvadosclient.MakeArvadosClient()
-	kc, _ := MakeKeepClient(&arv)
+	kc, _ := MakeKeepClient(arv)
 	arv.ApiToken = "abc123"
 	kc.SetServiceRoots(
 		map[string]string{
@@ -699,7 +699,7 @@ func (s *StandaloneSuite) TestGetWithServiceHintFailoverToLocals(c *C) {
 	defer ksGateway.listener.Close()
 
 	arv, err := arvadosclient.MakeArvadosClient()
-	kc, _ := MakeKeepClient(&arv)
+	kc, _ := MakeKeepClient(arv)
 	arv.ApiToken = "abc123"
 	kc.SetServiceRoots(
 		map[string]string{"zzzzz-bi6l4-keepdisk0000000": ksLocal.url},
@@ -736,7 +736,7 @@ func (s *StandaloneSuite) TestChecksum(c *C) {
 	defer ks.listener.Close()
 
 	arv, err := arvadosclient.MakeArvadosClient()
-	kc, _ := MakeKeepClient(&arv)
+	kc, _ := MakeKeepClient(arv)
 	arv.ApiToken = "abc123"
 	kc.SetServiceRoots(map[string]string{"x": ks.url}, nil, nil)
 
@@ -770,7 +770,7 @@ func (s *StandaloneSuite) TestGetWithFailures(c *C) {
 		content}
 
 	arv, err := arvadosclient.MakeArvadosClient()
-	kc, _ := MakeKeepClient(&arv)
+	kc, _ := MakeKeepClient(arv)
 	arv.ApiToken = "abc123"
 	localRoots := make(map[string]string)
 	writableLocalRoots := make(map[string]string)
@@ -816,7 +816,7 @@ func (s *ServerRequiredSuite) TestPutGetHead(c *C) {
 	content := []byte("TestPutGetHead")
 
 	arv, err := arvadosclient.MakeArvadosClient()
-	kc, err := MakeKeepClient(&arv)
+	kc, err := MakeKeepClient(arv)
 	c.Assert(err, Equals, nil)
 
 	hash := fmt.Sprintf("%x", md5.Sum(content))
@@ -863,7 +863,7 @@ func (s *StandaloneSuite) TestPutProxy(c *C) {
 	st := StubProxyHandler{make(chan string, 1)}
 
 	arv, err := arvadosclient.MakeArvadosClient()
-	kc, _ := MakeKeepClient(&arv)
+	kc, _ := MakeKeepClient(arv)
 
 	kc.Want_replicas = 2
 	arv.ApiToken = "abc123"
@@ -891,7 +891,7 @@ func (s *StandaloneSuite) TestPutProxyInsufficientReplicas(c *C) {
 	st := StubProxyHandler{make(chan string, 1)}
 
 	arv, err := arvadosclient.MakeArvadosClient()
-	kc, _ := MakeKeepClient(&arv)
+	kc, _ := MakeKeepClient(arv)
 
 	kc.Want_replicas = 3
 	arv.ApiToken = "abc123"
@@ -964,7 +964,7 @@ func (s *StandaloneSuite) TestPutBWant2ReplicasWithOnlyOneWritableLocalRoot(c *C
 		make(chan string, 5)}
 
 	arv, _ := arvadosclient.MakeArvadosClient()
-	kc, _ := MakeKeepClient(&arv)
+	kc, _ := MakeKeepClient(arv)
 
 	kc.Want_replicas = 2
 	arv.ApiToken = "abc123"
@@ -1002,7 +1002,7 @@ func (s *StandaloneSuite) TestPutBWithNoWritableLocalRoots(c *C) {
 		make(chan string, 5)}
 
 	arv, _ := arvadosclient.MakeArvadosClient()
-	kc, _ := MakeKeepClient(&arv)
+	kc, _ := MakeKeepClient(arv)
 
 	kc.Want_replicas = 2
 	arv.ApiToken = "abc123"
@@ -1054,7 +1054,7 @@ func (s *StandaloneSuite) TestGetIndexWithNoPrefix(c *C) {
 	defer ks.listener.Close()
 
 	arv, err := arvadosclient.MakeArvadosClient()
-	kc, _ := MakeKeepClient(&arv)
+	kc, _ := MakeKeepClient(arv)
 	arv.ApiToken = "abc123"
 	kc.SetServiceRoots(map[string]string{"x": ks.url}, nil, nil)
 
@@ -1080,7 +1080,7 @@ func (s *StandaloneSuite) TestGetIndexWithPrefix(c *C) {
 	defer ks.listener.Close()
 
 	arv, err := arvadosclient.MakeArvadosClient()
-	kc, _ := MakeKeepClient(&arv)
+	kc, _ := MakeKeepClient(arv)
 	arv.ApiToken = "abc123"
 	kc.SetServiceRoots(map[string]string{"x": ks.url}, nil, nil)
 
@@ -1106,7 +1106,7 @@ func (s *StandaloneSuite) TestGetIndexIncomplete(c *C) {
 	defer ks.listener.Close()
 
 	arv, err := arvadosclient.MakeArvadosClient()
-	kc, _ := MakeKeepClient(&arv)
+	kc, _ := MakeKeepClient(arv)
 	arv.ApiToken = "abc123"
 	kc.SetServiceRoots(map[string]string{"x": ks.url}, nil, nil)
 
@@ -1128,7 +1128,7 @@ func (s *StandaloneSuite) TestGetIndexWithNoSuchServer(c *C) {
 	defer ks.listener.Close()
 
 	arv, err := arvadosclient.MakeArvadosClient()
-	kc, _ := MakeKeepClient(&arv)
+	kc, _ := MakeKeepClient(arv)
 	arv.ApiToken = "abc123"
 	kc.SetServiceRoots(map[string]string{"x": ks.url}, nil, nil)
 
@@ -1148,7 +1148,7 @@ func (s *StandaloneSuite) TestGetIndexWithNoSuchPrefix(c *C) {
 	defer ks.listener.Close()
 
 	arv, err := arvadosclient.MakeArvadosClient()
-	kc, _ := MakeKeepClient(&arv)
+	kc, _ := MakeKeepClient(arv)
 	arv.ApiToken = "abc123"
 	kc.SetServiceRoots(map[string]string{"x": ks.url}, nil, nil)
 
@@ -1186,7 +1186,7 @@ func (s *StandaloneSuite) TestPutBRetry(c *C) {
 			make(chan string, 5)}}
 
 	arv, _ := arvadosclient.MakeArvadosClient()
-	kc, _ := MakeKeepClient(&arv)
+	kc, _ := MakeKeepClient(arv)
 
 	kc.Want_replicas = 2
 	arv.ApiToken = "abc123"
@@ -1226,7 +1226,7 @@ func (s *ServerRequiredSuite) TestMakeKeepClientWithNonDiskTypeService(c *C) {
 	defer func() { arv.Delete("keep_services", blobKeepService["uuid"].(string), nil, nil) }()
 
 	// Make a keepclient and ensure that the testblobstore is included
-	kc, err := MakeKeepClient(&arv)
+	kc, err := MakeKeepClient(arv)
 	c.Assert(err, Equals, nil)
 
 	// verify kc.LocalRoots
diff --git a/sdk/go/logger/logger.go b/sdk/go/logger/logger.go
index 3b2db3a..6dd7fb3 100644
--- a/sdk/go/logger/logger.go
+++ b/sdk/go/logger/logger.go
@@ -37,9 +37,9 @@ const (
 )
 
 type LoggerParams struct {
-	Client          arvadosclient.ArvadosClient // The client we use to write log entries
-	EventTypePrefix string                      // The prefix we use for the event type in the log entry
-	WriteInterval   time.Duration               // Wait at least this long between log writes
+	Client          *arvadosclient.ArvadosClient // The client we use to write log entries
+	EventTypePrefix string                       // The prefix we use for the event type in the log entry
+	WriteInterval   time.Duration                // Wait at least this long between log writes
 }
 
 // A LogMutator is a function which modifies the log entry.
diff --git a/sdk/go/util/util.go b/sdk/go/util/util.go
index 6bc8625..ac510de 100644
--- a/sdk/go/util/util.go
+++ b/sdk/go/util/util.go
@@ -6,7 +6,7 @@ import (
 	"git.curoverse.com/arvados.git/sdk/go/arvadosclient"
 )
 
-func UserIsAdmin(arv arvadosclient.ArvadosClient) (is_admin bool, err error) {
+func UserIsAdmin(arv *arvadosclient.ArvadosClient) (is_admin bool, err error) {
 	type user struct {
 		IsAdmin bool `json:"is_admin"`
 	}
@@ -21,7 +21,7 @@ func UserIsAdmin(arv arvadosclient.ArvadosClient) (is_admin bool, err error) {
 // return
 //   count - the number of items of type resource the api server reports, if no error
 //   err - error accessing the resource, or nil if no error
-func NumberItemsAvailable(client arvadosclient.ArvadosClient, resource string) (count int, err error) {
+func NumberItemsAvailable(client *arvadosclient.ArvadosClient, resource string) (count int, err error) {
 	var response struct {
 		ItemsAvailable int `json:"items_available"`
 	}
diff --git a/services/crunch-dispatch-local/crunch-dispatch-local_test.go b/services/crunch-dispatch-local/crunch-dispatch-local_test.go
index ff4fa8a..bcb406e 100644
--- a/services/crunch-dispatch-local/crunch-dispatch-local_test.go
+++ b/services/crunch-dispatch-local/crunch-dispatch-local_test.go
@@ -152,7 +152,7 @@ func testWithServerStub(c *C, apiStubResponses map[string]arvadostest.StubRespon
 	api := httptest.NewServer(&apiStub)
 	defer api.Close()
 
-	arv := arvadosclient.ArvadosClient{
+	arv := &arvadosclient.ArvadosClient{
 		Scheme:    "http",
 		ApiServer: api.URL[7:],
 		ApiToken:  "abc123",
diff --git a/services/crunch-dispatch-slurm/crunch-dispatch-slurm_test.go b/services/crunch-dispatch-slurm/crunch-dispatch-slurm_test.go
index 6692f7f..b278e1c 100644
--- a/services/crunch-dispatch-slurm/crunch-dispatch-slurm_test.go
+++ b/services/crunch-dispatch-slurm/crunch-dispatch-slurm_test.go
@@ -193,7 +193,7 @@ func testWithServerStub(c *C, apiStubResponses map[string]arvadostest.StubRespon
 	api := httptest.NewServer(&apiStub)
 	defer api.Close()
 
-	arv := arvadosclient.ArvadosClient{
+	arv := &arvadosclient.ArvadosClient{
 		Scheme:    "http",
 		ApiServer: api.URL[7:],
 		ApiToken:  "abc123",
diff --git a/services/crunch-run/crunchrun.go b/services/crunch-run/crunchrun.go
index 0b4eb2b..02f2ee9 100644
--- a/services/crunch-run/crunchrun.go
+++ b/services/crunch-run/crunchrun.go
@@ -877,7 +877,7 @@ func main() {
 	api.Retries = 8
 
 	var kc *keepclient.KeepClient
-	kc, err = keepclient.MakeKeepClient(&api)
+	kc, err = keepclient.MakeKeepClient(api)
 	if err != nil {
 		log.Fatalf("%s: %v", containerId, err)
 	}
diff --git a/services/crunch-run/crunchrun_test.go b/services/crunch-run/crunchrun_test.go
index 644e9e9..0ce6587 100644
--- a/services/crunch-run/crunchrun_test.go
+++ b/services/crunch-run/crunchrun_test.go
@@ -202,6 +202,12 @@ func (client *ArvTestClient) Update(resourceType string, uuid string, parameters
 	return nil
 }
 
+var discoveryMap = map[string]interface{}{"defaultTrashLifetime": float64(1209600)}
+
+func (client *ArvTestClient) Discovery(key string) (interface{}, error) {
+	return discoveryMap[key], nil
+}
+
 // CalledWith returns the parameters from the first API call whose
 // parameters match jpath/string. E.g., CalledWith(c, "foo.bar",
 // "baz") returns parameters with parameters["foo"]["bar"]=="baz". If
@@ -307,6 +313,10 @@ func (ArvErrorTestClient) Update(resourceType string, uuid string, parameters ar
 	return nil
 }
 
+func (ArvErrorTestClient) Discovery(key string) (interface{}, error) {
+	return discoveryMap[key], nil
+}
+
 type KeepErrorTestClient struct{}
 
 func (KeepErrorTestClient) PutHB(hash string, buf []byte) (string, int, error) {
diff --git a/services/datamanager/collection/collection.go b/services/datamanager/collection/collection.go
index 5fcacff..05e7a5f 100644
--- a/services/datamanager/collection/collection.go
+++ b/services/datamanager/collection/collection.go
@@ -42,7 +42,7 @@ type ReadCollections struct {
 
 // GetCollectionsParams params
 type GetCollectionsParams struct {
-	Client    arvadosclient.ArvadosClient
+	Client    *arvadosclient.ArvadosClient
 	Logger    *logger.Logger
 	BatchSize int
 }
diff --git a/services/datamanager/collection/collection_test.go b/services/datamanager/collection/collection_test.go
index b23ef2c..1bf6a89 100644
--- a/services/datamanager/collection/collection_test.go
+++ b/services/datamanager/collection/collection_test.go
@@ -184,7 +184,7 @@ func testGetCollectionsAndSummarize(c *C, testData APITestData) {
 	api := httptest.NewServer(&apiStub)
 	defer api.Close()
 
-	arv := arvadosclient.ArvadosClient{
+	arv := &arvadosclient.ArvadosClient{
 		Scheme:    "http",
 		ApiServer: api.URL[7:],
 		ApiToken:  "abc123",
diff --git a/services/datamanager/datamanager.go b/services/datamanager/datamanager.go
index 8e12835..5250d17 100644
--- a/services/datamanager/datamanager.go
+++ b/services/datamanager/datamanager.go
@@ -81,7 +81,7 @@ func main() {
 
 var arvLogger *logger.Logger
 
-func singlerun(arv arvadosclient.ArvadosClient) error {
+func singlerun(arv *arvadosclient.ArvadosClient) error {
 	var err error
 	if isAdmin, err := util.UserIsAdmin(arv); err != nil {
 		return errors.New("Error verifying admin token: " + err.Error())
@@ -142,7 +142,7 @@ func singlerun(arv arvadosclient.ArvadosClient) error {
 			rlbss.Count)
 	}
 
-	kc, err := keepclient.MakeKeepClient(&arv)
+	kc, err := keepclient.MakeKeepClient(arv)
 	if err != nil {
 		return fmt.Errorf("Error setting up keep client %v", err.Error())
 	}
@@ -185,7 +185,7 @@ func singlerun(arv arvadosclient.ArvadosClient) error {
 }
 
 // BuildDataFetcher returns a data fetcher that fetches data from remote servers.
-func BuildDataFetcher(arv arvadosclient.ArvadosClient) summary.DataFetcher {
+func BuildDataFetcher(arv *arvadosclient.ArvadosClient) summary.DataFetcher {
 	return func(
 		arvLogger *logger.Logger,
 		readCollections *collection.ReadCollections,
diff --git a/services/datamanager/datamanager_test.go b/services/datamanager/datamanager_test.go
index 0ff6b77..7a8fff5 100644
--- a/services/datamanager/datamanager_test.go
+++ b/services/datamanager/datamanager_test.go
@@ -19,7 +19,7 @@ import (
 	"time"
 )
 
-var arv arvadosclient.ArvadosClient
+var arv *arvadosclient.ArvadosClient
 var keepClient *keepclient.KeepClient
 var keepServers []string
 
@@ -40,7 +40,7 @@ func SetupDataManagerTest(t *testing.T) {
 
 	// keep client
 	keepClient = &keepclient.KeepClient{
-		Arvados:       &arv,
+		Arvados:       arv,
 		Want_replicas: 2,
 		Client:        &http.Client{},
 	}
diff --git a/services/datamanager/keep/keep.go b/services/datamanager/keep/keep.go
index 651c869..39d2d5b 100644
--- a/services/datamanager/keep/keep.go
+++ b/services/datamanager/keep/keep.go
@@ -66,7 +66,7 @@ type ReadServers struct {
 
 // GetKeepServersParams struct
 type GetKeepServersParams struct {
-	Client arvadosclient.ArvadosClient
+	Client *arvadosclient.ArvadosClient
 	Logger *logger.Logger
 	Limit  int
 }
@@ -215,7 +215,7 @@ func GetKeepServers(params GetKeepServersParams) (results ReadServers, err error
 // GetServerContents of the keep server
 func GetServerContents(arvLogger *logger.Logger,
 	keepServer ServerAddress,
-	arv arvadosclient.ArvadosClient) (response ServerResponse) {
+	arv *arvadosclient.ArvadosClient) (response ServerResponse) {
 
 	err := GetServerStatus(arvLogger, keepServer, arv)
 	if err != nil {
@@ -247,7 +247,7 @@ func GetServerContents(arvLogger *logger.Logger,
 // GetServerStatus get keep server status by invoking /status.json
 func GetServerStatus(arvLogger *logger.Logger,
 	keepServer ServerAddress,
-	arv arvadosclient.ArvadosClient) error {
+	arv *arvadosclient.ArvadosClient) error {
 	url := fmt.Sprintf("http://%s:%d/status.json",
 		keepServer.Host,
 		keepServer.Port)
@@ -298,7 +298,7 @@ func GetServerStatus(arvLogger *logger.Logger,
 // CreateIndexRequest to the keep server
 func CreateIndexRequest(arvLogger *logger.Logger,
 	keepServer ServerAddress,
-	arv arvadosclient.ArvadosClient) (req *http.Request, err error) {
+	arv *arvadosclient.ArvadosClient) (req *http.Request, err error) {
 	url := fmt.Sprintf("http://%s:%d/index", keepServer.Host, keepServer.Port)
 	log.Println("About to fetch keep server contents from " + url)
 
diff --git a/services/datamanager/keep/keep_test.go b/services/datamanager/keep/keep_test.go
index 6698849..ca8797e 100644
--- a/services/datamanager/keep/keep_test.go
+++ b/services/datamanager/keep/keep_test.go
@@ -45,8 +45,8 @@ func (s *KeepSuite) TestSendTrashLists(c *C) {
 	tl := map[string]TrashList{
 		server.URL: {TrashRequest{"000000000000000000000000deadbeef", 99}}}
 
-	arv := arvadosclient.ArvadosClient{ApiToken: "abc123"}
-	kc := keepclient.KeepClient{Arvados: &arv, Client: &http.Client{}}
+	arv := &arvadosclient.ArvadosClient{ApiToken: "abc123"}
+	kc := keepclient.KeepClient{Arvados: arv, Client: &http.Client{}}
 	kc.SetServiceRoots(map[string]string{"xxxx": server.URL},
 		map[string]string{"xxxx": server.URL},
 		map[string]string{})
@@ -72,8 +72,8 @@ func sendTrashListError(c *C, server *httptest.Server) {
 	tl := map[string]TrashList{
 		server.URL: {TrashRequest{"000000000000000000000000deadbeef", 99}}}
 
-	arv := arvadosclient.ArvadosClient{ApiToken: "abc123"}
-	kc := keepclient.KeepClient{Arvados: &arv, Client: &http.Client{}}
+	arv := &arvadosclient.ArvadosClient{ApiToken: "abc123"}
+	kc := keepclient.KeepClient{Arvados: arv, Client: &http.Client{}}
 	kc.SetServiceRoots(map[string]string{"xxxx": server.URL},
 		map[string]string{"xxxx": server.URL},
 		map[string]string{})
@@ -132,14 +132,14 @@ func testGetKeepServersFromAPI(c *C, testData APITestData, expectedError string)
 	api := httptest.NewServer(&apiStub)
 	defer api.Close()
 
-	arv := arvadosclient.ArvadosClient{
+	arv := &arvadosclient.ArvadosClient{
 		Scheme:    "http",
 		ApiServer: api.URL[7:],
 		ApiToken:  "abc123",
 		Client:    &http.Client{Transport: &http.Transport{}},
 	}
 
-	kc := keepclient.KeepClient{Arvados: &arv, Client: &http.Client{}}
+	kc := keepclient.KeepClient{Arvados: arv, Client: &http.Client{}}
 	kc.SetServiceRoots(map[string]string{"xxxx": "http://example.com:23456"},
 		map[string]string{"xxxx": "http://example.com:23456"},
 		map[string]string{})
@@ -233,14 +233,14 @@ func testGetKeepServersAndSummarize(c *C, testData KeepServerTestData) {
 	api := httptest.NewServer(&apiStub)
 	defer api.Close()
 
-	arv := arvadosclient.ArvadosClient{
+	arv := &arvadosclient.ArvadosClient{
 		Scheme:    "http",
 		ApiServer: api.URL[7:],
 		ApiToken:  "abc123",
 		Client:    &http.Client{Transport: &http.Transport{}},
 	}
 
-	kc := keepclient.KeepClient{Arvados: &arv, Client: &http.Client{}}
+	kc := keepclient.KeepClient{Arvados: arv, Client: &http.Client{}}
 	kc.SetServiceRoots(map[string]string{"xxxx": ks.URL},
 		map[string]string{"xxxx": ks.URL},
 		map[string]string{})
diff --git a/services/keep-balance/integration_test.go b/services/keep-balance/integration_test.go
index 0793889..148b783 100644
--- a/services/keep-balance/integration_test.go
+++ b/services/keep-balance/integration_test.go
@@ -36,7 +36,7 @@ func (s *integrationSuite) SetUpSuite(c *check.C) {
 	arv.ApiToken = arvadostest.DataManagerToken
 	c.Assert(err, check.IsNil)
 	s.keepClient = &keepclient.KeepClient{
-		Arvados: &arv,
+		Arvados: arv,
 		Client:  &http.Client{},
 	}
 	c.Assert(s.keepClient.DiscoverKeepServers(), check.IsNil)
diff --git a/services/keep-web/server_test.go b/services/keep-web/server_test.go
index bddd6db..6441364 100644
--- a/services/keep-web/server_test.go
+++ b/services/keep-web/server_test.go
@@ -106,7 +106,7 @@ func (s *IntegrationSuite) test100BlockFile(c *check.C, blocksize int) {
 	arv, err := arvadosclient.MakeArvadosClient()
 	c.Assert(err, check.Equals, nil)
 	arv.ApiToken = arvadostest.ActiveToken
-	kc, err := keepclient.MakeKeepClient(&arv)
+	kc, err := keepclient.MakeKeepClient(arv)
 	c.Assert(err, check.Equals, nil)
 	loc, _, err := kc.PutB(testdata[:])
 	c.Assert(err, check.Equals, nil)
@@ -297,7 +297,7 @@ func (s *IntegrationSuite) SetUpSuite(c *check.C) {
 	arv, err := arvadosclient.MakeArvadosClient()
 	c.Assert(err, check.Equals, nil)
 	arv.ApiToken = arvadostest.ActiveToken
-	kc, err := keepclient.MakeKeepClient(&arv)
+	kc, err := keepclient.MakeKeepClient(arv)
 	c.Assert(err, check.Equals, nil)
 	kc.PutB([]byte("Hello world\n"))
 	kc.PutB([]byte("foo"))
diff --git a/services/keepproxy/keepproxy_test.go b/services/keepproxy/keepproxy_test.go
index e6c1298..6a349da 100644
--- a/services/keepproxy/keepproxy_test.go
+++ b/services/keepproxy/keepproxy_test.go
@@ -101,7 +101,7 @@ func runProxy(c *C, args []string, bogusClientToken bool) *keepclient.KeepClient
 	if bogusClientToken {
 		arv.ApiToken = "bogus-token"
 	}
-	kc := keepclient.New(&arv)
+	kc := keepclient.New(arv)
 	sr := map[string]string{
 		TestProxyUUID: "http://" + listener.Addr().String(),
 	}
@@ -504,7 +504,7 @@ func (s *ServerRequiredSuite) TestAskGetKeepProxyConnectionError(c *C) {
 	c.Assert(err, Equals, nil)
 
 	// keepclient with no such keep server
-	kc := keepclient.New(&arv)
+	kc := keepclient.New(arv)
 	locals := map[string]string{
 		TestProxyUUID: "http://localhost:12345",
 	}
diff --git a/services/keepstore/pull_worker_integration_test.go b/services/keepstore/pull_worker_integration_test.go
index e0613a2..77b4c75 100644
--- a/services/keepstore/pull_worker_integration_test.go
+++ b/services/keepstore/pull_worker_integration_test.go
@@ -37,7 +37,7 @@ func SetupPullWorkerIntegrationTest(t *testing.T, testData PullWorkIntegrationTe
 
 	// keep client
 	keepClient = &keepclient.KeepClient{
-		Arvados:       &arv,
+		Arvados:       arv,
 		Want_replicas: 1,
 		Client:        &http.Client{},
 	}
diff --git a/services/keepstore/pull_worker_test.go b/services/keepstore/pull_worker_test.go
index 5076b85..4d85d5f 100644
--- a/services/keepstore/pull_worker_test.go
+++ b/services/keepstore/pull_worker_test.go
@@ -39,7 +39,7 @@ func (s *PullWorkerTestSuite) SetUpTest(c *C) {
 func RunTestPullWorker(c *C) {
 	arv, err := arvadosclient.MakeArvadosClient()
 	c.Assert(err, Equals, nil)
-	keepClient, err := keepclient.MakeKeepClient(&arv)
+	keepClient, err := keepclient.MakeKeepClient(arv)
 	c.Assert(err, Equals, nil)
 
 	pullq = NewWorkQueue()

commit 35ff079469606b58e2ec17631045d7f7ec0e3b53
Author: Tom Clegg <tom at curoverse.com>
Date:   Mon Sep 26 23:18:24 2016 -0400

    9848: Copy the output and log collections (if any) when finalizing a container request.
    
    When reusing a container, ensure log and output are readable.

diff --git a/services/api/app/models/container.rb b/services/api/app/models/container.rb
index c1c3eae..a60ea42 100644
--- a/services/api/app/models/container.rb
+++ b/services/api/app/models/container.rb
@@ -5,6 +5,7 @@ class Container < ArvadosModel
   include KindAndEtag
   include CommonApiTemplate
   include WhitelistUpdate
+  extend CurrentApiClient
 
   serialize :environment, Hash
   serialize :mounts, Hash
@@ -87,10 +88,34 @@ class Container < ArvadosModel
       where('mounts = ?', self.deep_sort_hash(attrs[:mounts]).to_yaml).
       where('runtime_constraints = ?', self.deep_sort_hash(attrs[:runtime_constraints]).to_yaml)
 
-    # Check for Completed candidates that only had consistent outputs.
+    # Check for Completed candidates that had consistent outputs.
     completed = candidates.where(state: Complete).where(exit_code: 0)
-    if completed.select("output").group('output').limit(2).length == 1
-      return completed.order('finished_at asc').limit(1).first
+    outputs = completed.select('output').group('output').limit(2)
+    if outputs.count.count != 1
+      Rails.logger.debug("Found #{outputs.count.length} different outputs")
+    elsif Collection.
+        readable_by(current_user).
+        where(portable_data_hash: outputs.first.output).
+        count < 1
+      Rails.logger.info("Found reusable container(s) " +
+                        "but output #{outputs.first} is not readable " +
+                        "by user #{current_user.uuid}")
+    else
+      # Return the oldest eligible container whose log is still
+      # present and readable by current_user.
+      readable_pdh = Collection.
+        readable_by(current_user).
+        select('portable_data_hash')
+      completed = completed.
+        where("log in (#{readable_pdh.to_sql})").
+        order('finished_at asc').
+        limit(1)
+      if completed.first
+        return completed.first
+      else
+        Rails.logger.info("Found reusable container(s) but none with a log " +
+                          "readable by user #{current_user.uuid}")
+      end
     end
 
     # Check for Running candidates and return the most likely to finish sooner.
@@ -284,13 +309,13 @@ class Container < ArvadosModel
       act_as_system_user do
         # Notify container requests associated with this container
         ContainerRequest.where(container_uuid: uuid,
-                               :state => ContainerRequest::Committed).each do |cr|
+                               state: ContainerRequest::Committed).each do |cr|
           cr.container_completed!
         end
 
         # Try to cancel any outstanding container requests made by this container.
         ContainerRequest.where(requesting_container_uuid: uuid,
-                               :state => ContainerRequest::Committed).each do |cr|
+                               state: ContainerRequest::Committed).each do |cr|
           cr.priority = 0
           cr.save
         end
diff --git a/services/api/app/models/container_request.rb b/services/api/app/models/container_request.rb
index 872b3c5..1fe8365 100644
--- a/services/api/app/models/container_request.rb
+++ b/services/api/app/models/container_request.rb
@@ -64,10 +64,24 @@ class ContainerRequest < ArvadosModel
     %w(modified_by_client_uuid container_uuid requesting_container_uuid)
   end
 
+  # Finalize the container request after the container has
+  # finished/cancelled.
   def container_completed!
-    # may implement retry logic here in the future.
-    self.state = ContainerRequest::Final
-    self.save!
+    update_attributes!(state: ContainerRequest::Final)
+    c = Container.find_by_uuid(container_uuid)
+    ['output', 'log'].each do |out_type|
+      pdh = c.send(out_type)
+      next if pdh.nil?
+      manifest = Collection.where(portable_data_hash: pdh).first.manifest_text
+      Collection.create!(owner_uuid: owner_uuid,
+                         manifest_text: manifest,
+                         portable_data_hash: pdh,
+                         name: "Container #{out_type} for request #{uuid}",
+                         properties: {
+                           'type' => out_type,
+                           'container_request' => uuid,
+                         })
+    end
   end
 
   protected
diff --git a/services/api/test/fixtures/containers.yml b/services/api/test/fixtures/containers.yml
index 87098dc..29266d3 100644
--- a/services/api/test/fixtures/containers.yml
+++ b/services/api/test/fixtures/containers.yml
@@ -74,7 +74,7 @@ completed:
   container_image: test
   cwd: test
   log: ea10d51bcf88862dbcc36eb292017dfd+45
-  output: zzzzz-4zz18-znfnqtbbv4spc3w
+  output: 1f4b0bc7583c2a7f9102c395f4ffc5e3+45
   output_path: test
   command: ["echo", "hello"]
   runtime_constraints:
@@ -93,7 +93,7 @@ completed_older:
   finished_at: 2016-01-14 11:12:13.111111111 Z
   container_image: test
   cwd: test
-  output: test
+  output: 1f4b0bc7583c2a7f9102c395f4ffc5e3+45
   output_path: test
   command: ["echo", "hello"]
   runtime_constraints:
@@ -110,7 +110,7 @@ requester:
   updated_at: 2016-01-11 11:11:11.111111111 Z
   container_image: test
   cwd: test
-  output: test
+  output: 1f4b0bc7583c2a7f9102c395f4ffc5e3+45
   output_path: test
   command: ["echo", "hello"]
   runtime_constraints:
@@ -127,7 +127,7 @@ requester_container:
   updated_at: 2016-01-11 11:11:11.111111111 Z
   container_image: test
   cwd: test
-  output: test
+  output: 1f4b0bc7583c2a7f9102c395f4ffc5e3+45
   output_path: test
   command: ["echo", "hello"]
   runtime_constraints:
@@ -145,7 +145,7 @@ failed_container:
   updated_at: 2016-01-11 11:11:11.111111111 Z
   container_image: test
   cwd: test
-  output: test
+  output: 1f4b0bc7583c2a7f9102c395f4ffc5e3+45
   output_path: test
   command: ["echo", "hello"]
   runtime_constraints:
diff --git a/services/api/test/unit/container_request_test.rb b/services/api/test/unit/container_request_test.rb
index e44084e..372d94a 100644
--- a/services/api/test/unit/container_request_test.rb
+++ b/services/api/test/unit/container_request_test.rb
@@ -215,7 +215,10 @@ class ContainerRequestTest < ActiveSupport::TestCase
 
   test "Request is finalized when its container is completed" do
     set_user_from_auth :active
-    cr = create_minimal_req!(priority: 1, state: "Committed")
+    project = groups(:private)
+    cr = create_minimal_req!(owner_uuid: project.uuid,
+                             priority: 1,
+                             state: "Committed")
 
     c = act_as_system_user do
       c = Container.find_by_uuid(cr.container_uuid)
@@ -228,11 +231,19 @@ class ContainerRequestTest < ActiveSupport::TestCase
     assert_equal "Committed", cr.state
 
     act_as_system_user do
-      c.update_attributes!(state: Container::Complete)
+      c.update_attributes!(state: Container::Complete,
+                           output: '1f4b0bc7583c2a7f9102c395f4ffc5e3+45',
+                           log: 'fa7aeb5140e2848d39b416daeef4ffc5+45')
     end
 
     cr.reload
     assert_equal "Final", cr.state
+    ['output', 'log'].each do |out_type|
+      pdh = Container.find_by_uuid(cr.container_uuid).send(out_type)
+      assert_equal(1, Collection.where(portable_data_hash: pdh,
+                                       owner_uuid: project.uuid).count,
+                   "Container #{out_type} should be copied to #{project.uuid}")
+    end
   end
 
   test "Container makes container request, then is cancelled" do
diff --git a/services/api/test/unit/container_test.rb b/services/api/test/unit/container_test.rb
index 85741bb..8894ed9 100644
--- a/services/api/test/unit/container_test.rb
+++ b/services/api/test/unit/container_test.rb
@@ -122,7 +122,7 @@ class ContainerTest < ActiveSupport::TestCase
       state: Container::Complete,
       exit_code: 0,
       log: 'ea10d51bcf88862dbcc36eb292017dfd+45',
-      output: 'zzzzz-4zz18-znfnqtbbv4spc3w'
+      output: '1f4b0bc7583c2a7f9102c395f4ffc5e3+45'
     }
 
     c_older, _ = minimal_new(common_attrs)
@@ -148,7 +148,7 @@ class ContainerTest < ActiveSupport::TestCase
     completed_attrs = {
       state: Container::Complete,
       exit_code: 0,
-      log: 'test',
+      log: 'ea10d51bcf88862dbcc36eb292017dfd+45',
     }
 
     c_output1, _ = minimal_new(common_attrs)
@@ -157,11 +157,11 @@ class ContainerTest < ActiveSupport::TestCase
     set_user_from_auth :dispatch1
     c_output1.update_attributes!({state: Container::Locked})
     c_output1.update_attributes!({state: Container::Running})
-    c_output1.update_attributes!(completed_attrs.merge({output: 'output 1'}))
+    c_output1.update_attributes!(completed_attrs.merge({output: '1f4b0bc7583c2a7f9102c395f4ffc5e3+45'}))
 
     c_output2.update_attributes!({state: Container::Locked})
     c_output2.update_attributes!({state: Container::Running})
-    c_output2.update_attributes!(completed_attrs.merge({output: 'output 2'}))
+    c_output2.update_attributes!(completed_attrs.merge({output: 'fa7aeb5140e2848d39b416daeef4ffc5+45'}))
 
     reused = Container.find_reusable(common_attrs)
     assert_nil reused
@@ -239,8 +239,8 @@ class ContainerTest < ActiveSupport::TestCase
     c_failed.update_attributes!({state: Container::Running})
     c_failed.update_attributes!({state: Container::Complete,
                                  exit_code: 42,
-                                 log: "test",
-                                 output: "test"})
+                                 log: 'ea10d51bcf88862dbcc36eb292017dfd+45',
+                                 output: 'ea10d51bcf88862dbcc36eb292017dfd+45'})
     c_running.update_attributes!({state: Container::Locked})
     c_running.update_attributes!({state: Container::Running,
                                   progress: 0.15})
@@ -259,14 +259,14 @@ class ContainerTest < ActiveSupport::TestCase
     c_completed.update_attributes!({state: Container::Running})
     c_completed.update_attributes!({state: Container::Complete,
                                     exit_code: 0,
-                                    log: "ea10d51bcf88862dbcc36eb292017dfd+45",
-                                    output: "zzzzz-4zz18-znfnqtbbv4spc3w"})
+                                    log: 'ea10d51bcf88862dbcc36eb292017dfd+45',
+                                    output: '1f4b0bc7583c2a7f9102c395f4ffc5e3+45'})
     c_running.update_attributes!({state: Container::Locked})
     c_running.update_attributes!({state: Container::Running,
                                   progress: 0.15})
     reused = Container.find_reusable(common_attrs)
     assert_not_nil reused
-    assert_equal reused.uuid, c_completed.uuid
+    assert_equal c_completed.uuid, reused.uuid
   end
 
   test "find_reusable method should select running over locked container" do

commit 7bc11e7efb6e742f412dc24ec9394f3c9f098de4
Author: Tom Clegg <tom at curoverse.com>
Date:   Mon Sep 26 23:23:01 2016 -0400

    9848: group some assertions into bigger test cases.

diff --git a/services/api/test/unit/repository_test.rb b/services/api/test/unit/repository_test.rb
index 288e118..3fb0cce 100644
--- a/services/api/test/unit/repository_test.rb
+++ b/services/api/test/unit/repository_test.rb
@@ -35,8 +35,8 @@ class RepositoryTest < ActiveSupport::TestCase
 
   {active: "active/", admin: "admin/", system_user: ""}.
       each_pair do |user_sym, name_prefix|
-    %w(a aa a0 aA Aa AA A0).each do |name|
-      test "'#{name_prefix}#{name}' is a valid name for #{user_sym} repo" do
+    test "valid names for #{user_sym} repo" do
+      %w(a aa a0 aA Aa AA A0).each do |name|
         repo = new_repo(user_sym, name: name_prefix + name)
         assert(repo.valid?)
       end
@@ -51,8 +51,8 @@ class RepositoryTest < ActiveSupport::TestCase
       refute(repo.valid?)
     end
 
-    "\\.-_/!@#$%^&*()[]{}".each_char do |bad_char|
-      test "name containing #{bad_char.inspect} is invalid for #{user_sym}" do
+    test "name containing bad char is invalid for #{user_sym}" do
+      "\\.-_/!@#$%^&*()[]{}".each_char do |bad_char|
         repo = new_repo(user_sym, name: "#{name_prefix}bad#{bad_char}reponame")
         refute(repo.valid?)
       end

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list