[ARVADOS] updated: 6613ec1e9c705fb5b950611fd160d4a2babed251

Git user git at public.curoverse.com
Thu Sep 29 09:36:16 EDT 2016


Summary of changes:
 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               | 35 ++++++++++--
 services/api/app/models/container_request.rb       | 20 +++++--
 services/api/test/fixtures/containers.yml          | 10 ++--
 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 +-
 30 files changed, 187 insertions(+), 118 deletions(-)

       via  6613ec1e9c705fb5b950611fd160d4a2babed251 (commit)
       via  17b7538373558196516676c9c72839d308966f86 (commit)
       via  7314917d65573b0e9d55f7b6522463c470356fba (commit)
       via  a642669b7faa1b04350c451bf2e85692e730abba (commit)
       via  e9587a4ba9e13719473ae222b10291ce58fb5560 (commit)
      from  9cb3552d964d162d6eacff40435650440bc6f692 (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 6613ec1e9c705fb5b950611fd160d4a2babed251
Merge: 9cb3552 17b7538
Author: Tom Clegg <tom at curoverse.com>
Date:   Wed Sep 28 16:20:00 2016 -0400

    Merge branch '9848-copy-container-output' refs #9848


commit 17b7538373558196516676c9c72839d308966f86
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 7314917d65573b0e9d55f7b6522463c470356fba
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 39ec15d..e7ccf25 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 a642669b7faa1b04350c451bf2e85692e730abba
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 e9587a4ba9e13719473ae222b10291ce58fb5560
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