[ARVADOS] updated: 1.3.0-1607-g76ee36c49

Git user git at public.curoverse.com
Mon Sep 16 05:39:57 UTC 2019


Summary of changes:
 doc/install/install-keepstore.html.textile.liquid  |   2 +-
 lib/config/config.default.yml                      |  18 +-
 lib/config/deprecated_keepstore.go                 | 298 +++++++++++----------
 lib/config/deprecated_keepstore_test.go            |   2 +-
 lib/config/export.go                               |   2 +-
 lib/config/generated_config.go                     |  18 +-
 sdk/go/arvados/config.go                           |   2 +-
 .../arvados/v1/keep_services_controller.rb         |  19 +-
 services/api/app/models/keep_service.rb            |  19 +-
 .../arvados/v1/keep_services_controller_test.rb    |   3 +-
 services/keepstore/command.go                      |  10 +-
 services/keepstore/handler_test.go                 |   4 +-
 12 files changed, 199 insertions(+), 198 deletions(-)

       via  76ee36c49088274ab4e8465c0f4b20878d66c706 (commit)
       via  6653f96c23ff461bc4cadf5184a95e1c9142f7e6 (commit)
       via  11597243529c762114fc34907f772c331d6f6ef5 (commit)
       via  e5437560d2f30350370a1c96397716ac56a7398d (commit)
       via  d99713e58b0b9eb284fc3ea7f4008fa80bc5bcc5 (commit)
       via  a0ce99d4067878a7a32a9a2db44eb4bc0929d879 (commit)
      from  08d2f48c3cf3eac9d71261172677c54f03669fe5 (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 76ee36c49088274ab4e8465c0f4b20878d66c706
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Mon Sep 16 01:12:33 2019 -0400

    13647: Remove superfluous conditionals.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/lib/config/deprecated_keepstore.go b/lib/config/deprecated_keepstore.go
index 67ead7bce..43ca30a00 100644
--- a/lib/config/deprecated_keepstore.go
+++ b/lib/config/deprecated_keepstore.go
@@ -129,10 +129,10 @@ func (ldr *Loader) loadOldKeepstoreConfig(cfg *arvados.Config) error {
 		cluster.SystemLogs.LogLevel = "info"
 	}
 
-	if v := oc.TLSCertificateFile; v != nil && "file://"+*v != cluster.TLS.Certificate {
+	if v := oc.TLSCertificateFile; v != nil {
 		cluster.TLS.Certificate = "file://" + *v
 	}
-	if v := oc.TLSKeyFile; v != nil && "file://"+*v != cluster.TLS.Key {
+	if v := oc.TLSKeyFile; v != nil {
 		cluster.TLS.Key = "file://" + *v
 	}
 	if v := oc.Listen; v != nil {
@@ -157,16 +157,16 @@ func (ldr *Loader) loadOldKeepstoreConfig(cfg *arvados.Config) error {
 		}
 	}
 
-	if v := oc.LogFormat; v != nil && *v != cluster.SystemLogs.Format {
+	if v := oc.LogFormat; v != nil {
 		cluster.SystemLogs.Format = *v
 	}
-	if v := oc.MaxBuffers; v != nil && *v != cluster.API.MaxKeepBlobBuffers {
+	if v := oc.MaxBuffers; v != nil {
 		cluster.API.MaxKeepBlobBuffers = *v
 	}
-	if v := oc.MaxRequests; v != nil && *v != cluster.API.MaxConcurrentRequests {
+	if v := oc.MaxRequests; v != nil {
 		cluster.API.MaxConcurrentRequests = *v
 	}
-	if v := oc.BlobSignatureTTL; v != nil && *v != cluster.Collections.BlobSigningTTL {
+	if v := oc.BlobSignatureTTL; v != nil {
 		cluster.Collections.BlobSigningTTL = *v
 	}
 	if v := oc.BlobSigningKeyFile; v != nil {
@@ -178,7 +178,7 @@ func (ldr *Loader) loadOldKeepstoreConfig(cfg *arvados.Config) error {
 			cluster.Collections.BlobSigningKey = key
 		}
 	}
-	if v := oc.RequireSignatures; v != nil && *v != cluster.Collections.BlobSigning {
+	if v := oc.RequireSignatures; v != nil {
 		cluster.Collections.BlobSigning = *v
 	}
 	if v := oc.SystemAuthTokenFile; v != nil {
@@ -195,22 +195,22 @@ func (ldr *Loader) loadOldKeepstoreConfig(cfg *arvados.Config) error {
 			cluster.SystemRootToken = key
 		}
 	}
-	if v := oc.EnableDelete; v != nil && *v != cluster.Collections.BlobTrash {
+	if v := oc.EnableDelete; v != nil {
 		cluster.Collections.BlobTrash = *v
 	}
-	if v := oc.TrashLifetime; v != nil && *v != cluster.Collections.BlobTrashLifetime {
+	if v := oc.TrashLifetime; v != nil {
 		cluster.Collections.BlobTrashLifetime = *v
 	}
-	if v := oc.TrashCheckInterval; v != nil && *v != cluster.Collections.BlobTrashCheckInterval {
+	if v := oc.TrashCheckInterval; v != nil {
 		cluster.Collections.BlobTrashCheckInterval = *v
 	}
-	if v := oc.TrashWorkers; v != nil && *v != cluster.Collections.BlobReplicateConcurrency {
+	if v := oc.TrashWorkers; v != nil {
 		cluster.Collections.BlobTrashConcurrency = *v
 	}
-	if v := oc.EmptyTrashWorkers; v != nil && *v != cluster.Collections.BlobReplicateConcurrency {
+	if v := oc.EmptyTrashWorkers; v != nil {
 		cluster.Collections.BlobDeleteConcurrency = *v
 	}
-	if v := oc.PullWorkers; v != nil && *v != cluster.Collections.BlobReplicateConcurrency {
+	if v := oc.PullWorkers; v != nil {
 		cluster.Collections.BlobReplicateConcurrency = *v
 	}
 	if oc.Volumes == nil || len(*oc.Volumes) == 0 {

commit 6653f96c23ff461bc4cadf5184a95e1c9142f7e6
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Sun Sep 15 20:07:23 2019 -0400

    13647: Move ...Controller.from_config_or_db to KeepService.all.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/services/api/app/controllers/arvados/v1/keep_services_controller.rb b/services/api/app/controllers/arvados/v1/keep_services_controller.rb
index 865f9c942..c6e889435 100644
--- a/services/api/app/controllers/arvados/v1/keep_services_controller.rb
+++ b/services/api/app/controllers/arvados/v1/keep_services_controller.rb
@@ -10,29 +10,16 @@ class Arvados::V1::KeepServicesController < ApplicationController
 
   def find_objects_for_index
     # all users can list all keep services
-    @objects = from_config_or_db
+    @objects = KeepService.all
     super
   end
 
   def accessible
     if request.headers['X-External-Client'] == '1'
-      @objects = from_config_or_db.where('service_type=?', 'proxy')
+      @objects = KeepService.where('service_type=?', 'proxy')
     else
-      @objects = from_config_or_db.where('service_type<>?', 'proxy')
+      @objects = KeepService.where('service_type<>?', 'proxy')
     end
     render_list
   end
-
-  private
-
-  # return the set of keep services from the database (if this is an
-  # older installation or test system where entries have been added
-  # manually) or, preferably, the cluster config file.
-  def from_config_or_db
-    if KeepService.all.count == 0
-      KeepService.from_config
-    else
-      KeepService.all
-    end
-  end
 end
diff --git a/services/api/app/models/keep_service.rb b/services/api/app/models/keep_service.rb
index f9dd01837..777f6bfb2 100644
--- a/services/api/app/models/keep_service.rb
+++ b/services/api/app/models/keep_service.rb
@@ -20,6 +20,21 @@ class KeepService < ArvadosModel
   api_accessible :superuser, :extend => :user do |t|
   end
 
+  # return the set of keep services from the database (if this is an
+  # older installation or test system where entries have been added
+  # manually) or, preferably, the cluster config file.
+  def self.all *args
+    if super.count == 0
+      from_config
+    else
+      super
+    end
+  end
+
+  def self.where *args
+    all.where *args
+  end
+
   protected
 
   def permission_to_create
@@ -45,10 +60,10 @@ class KeepService < ArvadosModel
     end
     if values.length == 0
       # return empty set as AR relation
-      return where('1=0')
+      return unscoped.where('1=0')
     else
       sql = "(values #{values.join(", ")}) as keep_services (id, uuid, service_host, service_port, service_ssl_flag, service_type, read_only, created_at, modified_at, owner_uuid, modified_by_user_uuid, modified_by_client_uuid)"
-      return KeepService.from(sql)
+      return unscoped.from(sql)
     end
   end
 
diff --git a/services/api/test/functional/arvados/v1/keep_services_controller_test.rb b/services/api/test/functional/arvados/v1/keep_services_controller_test.rb
index 33c8aad3e..f41a1d679 100644
--- a/services/api/test/functional/arvados/v1/keep_services_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/keep_services_controller_test.rb
@@ -37,7 +37,7 @@ class Arvados::V1::KeepServicesControllerTest < ActionController::TestCase
   end
 
   test "report configured servers if db is empty" do
-    KeepService.all.delete_all
+    KeepService.unscoped.all.delete_all
     expect_rvz = {}
     n = 0
     Rails.configuration.Services.Keepstore.InternalURLs.each do |k,v|
@@ -57,6 +57,7 @@ class Arvados::V1::KeepServicesControllerTest < ActionController::TestCase
     get :index,
       params: {:format => :json},
       headers: auth(:active)
+    assert_response :success
     json_response['items'].each do |svc|
       url = "#{svc['service_ssl_flag'] ? 'https' : 'http'}://#{svc['service_host']}:#{svc['service_port']}"
       assert_equal true, expect_rvz.has_key?(url), "#{url} does not match any configured service: expecting #{expect_rvz}"

commit 11597243529c762114fc34907f772c331d6f6ef5
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Fri Sep 13 16:28:01 2019 -0400

    13647: Remove obsolete reference to out-of-sync configs.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/lib/config/config.default.yml b/lib/config/config.default.yml
index 523327d8c..b26987185 100644
--- a/lib/config/config.default.yml
+++ b/lib/config/config.default.yml
@@ -370,10 +370,6 @@ Clusters:
       # still has permission) the client can retrieve the collection again
       # to get fresh signatures.
       #
-      # This must be exactly equal to the -blob-signature-ttl flag used by
-      # keepstore servers.  Otherwise, reading data blocks and saving
-      # collections will fail with HTTP 403 permission errors.
-      #
       # Modifying BlobSigningTTL invalidates existing signatures; see
       # BlobSigningKey note above.
       #
diff --git a/lib/config/generated_config.go b/lib/config/generated_config.go
index 51fd38554..bb64fcb8e 100644
--- a/lib/config/generated_config.go
+++ b/lib/config/generated_config.go
@@ -376,10 +376,6 @@ Clusters:
       # still has permission) the client can retrieve the collection again
       # to get fresh signatures.
       #
-      # This must be exactly equal to the -blob-signature-ttl flag used by
-      # keepstore servers.  Otherwise, reading data blocks and saving
-      # collections will fail with HTTP 403 permission errors.
-      #
       # Modifying BlobSigningTTL invalidates existing signatures; see
       # BlobSigningKey note above.
       #

commit e5437560d2f30350370a1c96397716ac56a7398d
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Fri Sep 13 16:26:51 2019 -0400

    13647: Fix BlobSigning config comment.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/lib/config/config.default.yml b/lib/config/config.default.yml
index a083094df..523327d8c 100644
--- a/lib/config/config.default.yml
+++ b/lib/config/config.default.yml
@@ -313,15 +313,9 @@ Clusters:
       MaxRequestLogParamsSize: 2000
 
     Collections:
-      # Allow clients to create collections by providing a manifest with
-      # unsigned data blob locators. IMPORTANT: This effectively disables
-      # access controls for data stored in Keep: a client who knows a hash
-      # can write a manifest that references the hash, pass it to
-      # collections.create (which will create a permission link), use
-      # collections.get to obtain a signature for that data locator, and
-      # use that signed locator to retrieve the data from Keep. Therefore,
-      # do not turn this on if your users expect to keep data private from
-      # one another!
+
+      # Enable access controls for data stored in Keep. This should
+      # always be set to true on a production cluster.
       BlobSigning: true
 
       # BlobSigningKey is a string of alphanumeric characters used to
diff --git a/lib/config/generated_config.go b/lib/config/generated_config.go
index 117f189d9..51fd38554 100644
--- a/lib/config/generated_config.go
+++ b/lib/config/generated_config.go
@@ -319,15 +319,9 @@ Clusters:
       MaxRequestLogParamsSize: 2000
 
     Collections:
-      # Allow clients to create collections by providing a manifest with
-      # unsigned data blob locators. IMPORTANT: This effectively disables
-      # access controls for data stored in Keep: a client who knows a hash
-      # can write a manifest that references the hash, pass it to
-      # collections.create (which will create a permission link), use
-      # collections.get to obtain a signature for that data locator, and
-      # use that signed locator to retrieve the data from Keep. Therefore,
-      # do not turn this on if your users expect to keep data private from
-      # one another!
+
+      # Enable access controls for data stored in Keep. This should
+      # always be set to true on a production cluster.
       BlobSigning: true
 
       # BlobSigningKey is a string of alphanumeric characters used to

commit d99713e58b0b9eb284fc3ea7f4008fa80bc5bcc5
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Fri Sep 13 15:50:48 2019 -0400

    13647: Rename MaxKeepBlockBuffers -> MaxKeepBlobBuffers
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/doc/install/install-keepstore.html.textile.liquid b/doc/install/install-keepstore.html.textile.liquid
index 92fd91eb0..5d0e2df80 100644
--- a/doc/install/install-keepstore.html.textile.liquid
+++ b/doc/install/install-keepstore.html.textile.liquid
@@ -66,7 +66,7 @@ Add or update the following sections of @/etc/arvados/config.yml@ as needed. Ref
         InternalURLs:
           "http://<span class="userinput">keep0.uuid_prefix.your.domain</span>:25107/": {}
     API:
-      MaxKeepBlockBuffers: 128
+      MaxKeepBlobBuffers: 128
 </code></pre>
 </notextile>
 
diff --git a/lib/config/config.default.yml b/lib/config/config.default.yml
index c1d79ff27..a083094df 100644
--- a/lib/config/config.default.yml
+++ b/lib/config/config.default.yml
@@ -184,7 +184,7 @@ Clusters:
 
       # Maximum number of 64MiB memory buffers per keepstore server
       # process, or 0 for no limit.
-      MaxKeepBlockBuffers: 128
+      MaxKeepBlobBuffers: 128
 
       # API methods to disable. Disabled methods are not listed in the
       # discovery document, and respond 404 to all requests.
diff --git a/lib/config/deprecated_keepstore.go b/lib/config/deprecated_keepstore.go
index dfd44a0ef..67ead7bce 100644
--- a/lib/config/deprecated_keepstore.go
+++ b/lib/config/deprecated_keepstore.go
@@ -160,8 +160,8 @@ func (ldr *Loader) loadOldKeepstoreConfig(cfg *arvados.Config) error {
 	if v := oc.LogFormat; v != nil && *v != cluster.SystemLogs.Format {
 		cluster.SystemLogs.Format = *v
 	}
-	if v := oc.MaxBuffers; v != nil && *v != cluster.API.MaxKeepBlockBuffers {
-		cluster.API.MaxKeepBlockBuffers = *v
+	if v := oc.MaxBuffers; v != nil && *v != cluster.API.MaxKeepBlobBuffers {
+		cluster.API.MaxKeepBlobBuffers = *v
 	}
 	if v := oc.MaxRequests; v != nil && *v != cluster.API.MaxConcurrentRequests {
 		cluster.API.MaxConcurrentRequests = *v
diff --git a/lib/config/deprecated_keepstore_test.go b/lib/config/deprecated_keepstore_test.go
index 4891ceb8a..0948e2e2f 100644
--- a/lib/config/deprecated_keepstore_test.go
+++ b/lib/config/deprecated_keepstore_test.go
@@ -121,7 +121,7 @@ Clusters:
       Format: text
       LogLevel: debug
     API:
-      MaxKeepBlockBuffers: 1234
+      MaxKeepBlobBuffers: 1234
       MaxConcurrentRequests: 2345
     Collections:
       BlobSigningTTL: 123m
diff --git a/lib/config/export.go b/lib/config/export.go
index f545abc14..8df561c00 100644
--- a/lib/config/export.go
+++ b/lib/config/export.go
@@ -66,7 +66,7 @@ var whitelist = map[string]bool{
 	"API.MaxConcurrentRequests":                    false,
 	"API.MaxIndexDatabaseRead":                     false,
 	"API.MaxItemsPerResponse":                      true,
-	"API.MaxKeepBlockBuffers":                      false,
+	"API.MaxKeepBlobBuffers":                       false,
 	"API.MaxRequestAmplification":                  false,
 	"API.MaxRequestSize":                           true,
 	"API.RailsSessionSecretToken":                  false,
diff --git a/lib/config/generated_config.go b/lib/config/generated_config.go
index ecc2e8ea0..117f189d9 100644
--- a/lib/config/generated_config.go
+++ b/lib/config/generated_config.go
@@ -190,7 +190,7 @@ Clusters:
 
       # Maximum number of 64MiB memory buffers per keepstore server
       # process, or 0 for no limit.
-      MaxKeepBlockBuffers: 128
+      MaxKeepBlobBuffers: 128
 
       # API methods to disable. Disabled methods are not listed in the
       # discovery document, and respond 404 to all requests.
diff --git a/sdk/go/arvados/config.go b/sdk/go/arvados/config.go
index a058f6181..4a7221f05 100644
--- a/sdk/go/arvados/config.go
+++ b/sdk/go/arvados/config.go
@@ -82,7 +82,7 @@ type Cluster struct {
 		MaxIndexDatabaseRead           int
 		MaxItemsPerResponse            int
 		MaxConcurrentRequests          int
-		MaxKeepBlockBuffers            int
+		MaxKeepBlobBuffers             int
 		MaxRequestAmplification        int
 		MaxRequestSize                 int
 		RailsSessionSecretToken        string
diff --git a/services/keepstore/command.go b/services/keepstore/command.go
index df279687d..51031d360 100644
--- a/services/keepstore/command.go
+++ b/services/keepstore/command.go
@@ -48,7 +48,7 @@ func runCommand(prog string, args []string, stdin io.Reader, stdout, stderr io.W
 func convertKeepstoreFlagsToServiceFlags(args []string, lgr logrus.FieldLogger) ([]string, bool) {
 	flags := flag.NewFlagSet("", flag.ContinueOnError)
 	flags.String("listen", "", "Services.Keepstore.InternalURLs")
-	flags.Int("max-buffers", 0, "API.MaxKeepBlockBuffers")
+	flags.Int("max-buffers", 0, "API.MaxKeepBlobBuffers")
 	flags.Int("max-requests", 0, "API.MaxConcurrentRequests")
 	flags.Bool("never-delete", false, "Collections.BlobTrash")
 	flags.Bool("enforce-permissions", false, "Collections.BlobSigning")
@@ -148,14 +148,14 @@ func newHandlerOrErrorHandler(ctx context.Context, cluster *arvados.Cluster, tok
 func (h *handler) setup(ctx context.Context, cluster *arvados.Cluster, token string, reg *prometheus.Registry, serviceURL arvados.URL) error {
 	h.Cluster = cluster
 	h.Logger = ctxlog.FromContext(ctx)
-	if h.Cluster.API.MaxKeepBlockBuffers <= 0 {
+	if h.Cluster.API.MaxKeepBlobBuffers <= 0 {
 		return fmt.Errorf("MaxBuffers must be greater than zero")
 	}
-	bufs = newBufferPool(h.Logger, h.Cluster.API.MaxKeepBlockBuffers, BlockSize)
+	bufs = newBufferPool(h.Logger, h.Cluster.API.MaxKeepBlobBuffers, BlockSize)
 
 	if h.Cluster.API.MaxConcurrentRequests < 1 {
-		h.Cluster.API.MaxConcurrentRequests = h.Cluster.API.MaxKeepBlockBuffers * 2
-		h.Logger.Warnf("MaxRequests <1 or not specified; defaulting to MaxKeepBlockBuffers * 2 == %d", h.Cluster.API.MaxConcurrentRequests)
+		h.Cluster.API.MaxConcurrentRequests = h.Cluster.API.MaxKeepBlobBuffers * 2
+		h.Logger.Warnf("MaxRequests <1 or not specified; defaulting to MaxKeepBlobBuffers * 2 == %d", h.Cluster.API.MaxConcurrentRequests)
 	}
 
 	if h.Cluster.Collections.BlobSigningKey != "" {
diff --git a/services/keepstore/handler_test.go b/services/keepstore/handler_test.go
index 251ad0a1d..299460297 100644
--- a/services/keepstore/handler_test.go
+++ b/services/keepstore/handler_test.go
@@ -947,7 +947,7 @@ func (s *HandlerSuite) TestPutHandlerNoBufferleak(c *check.C) {
 
 	ok := make(chan bool)
 	go func() {
-		for i := 0; i < s.cluster.API.MaxKeepBlockBuffers+1; i++ {
+		for i := 0; i < s.cluster.API.MaxKeepBlobBuffers+1; i++ {
 			// Unauthenticated request, no server key
 			// => OK (unsigned response)
 			unsignedLocator := "/" + TestHash
@@ -1039,7 +1039,7 @@ func (s *HandlerSuite) TestGetHandlerNoBufferLeak(c *check.C) {
 
 	ok := make(chan bool)
 	go func() {
-		for i := 0; i < s.cluster.API.MaxKeepBlockBuffers+1; i++ {
+		for i := 0; i < s.cluster.API.MaxKeepBlobBuffers+1; i++ {
 			// Unauthenticated request, unsigned locator
 			// => OK
 			unsignedLocator := "/" + TestHash

commit a0ce99d4067878a7a32a9a2db44eb4bc0929d879
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Fri Sep 13 15:44:39 2019 -0400

    13647: Split up long func.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/lib/config/deprecated_keepstore.go b/lib/config/deprecated_keepstore.go
index 0d1a3c4e6..dfd44a0ef 100644
--- a/lib/config/deprecated_keepstore.go
+++ b/lib/config/deprecated_keepstore.go
@@ -213,151 +213,169 @@ func (ldr *Loader) loadOldKeepstoreConfig(cfg *arvados.Config) error {
 	if v := oc.PullWorkers; v != nil && *v != cluster.Collections.BlobReplicateConcurrency {
 		cluster.Collections.BlobReplicateConcurrency = *v
 	}
-	if v := oc.Volumes; v == nil {
+	if oc.Volumes == nil || len(*oc.Volumes) == 0 {
 		ldr.Logger.Warn("no volumes in legacy config; discovering local directory volumes")
 		err := ldr.discoverLocalVolumes(cluster, oc.DiscoverVolumesFromMountsFile, myURL)
 		if err != nil {
 			return fmt.Errorf("error discovering local directory volumes: %s", err)
 		}
 	} else {
-		for i, oldvol := range *v {
-			var accessViaHosts map[arvados.URL]arvados.VolumeAccess
-			oldUUID, found := ldr.alreadyMigrated(oldvol, cluster.Volumes, myURL)
-			if found {
-				accessViaHosts = cluster.Volumes[oldUUID].AccessViaHosts
-				writers := false
-				for _, va := range accessViaHosts {
-					if !va.ReadOnly {
-						writers = true
-					}
-				}
-				if writers || len(accessViaHosts) == 0 {
-					ldr.Logger.Infof("ignoring volume #%d's parameters in legacy keepstore config: using matching entry in cluster config instead", i)
-					if len(accessViaHosts) > 0 {
-						cluster.Volumes[oldUUID].AccessViaHosts[myURL] = arvados.VolumeAccess{ReadOnly: oldvol.ReadOnly}
-					}
-					continue
+		err := ldr.migrateOldKeepstoreVolumes(cluster, oc, myURL)
+		if err != nil {
+			return err
+		}
+	}
+
+	cfg.Clusters[cluster.ClusterID] = *cluster
+	return nil
+}
+
+// Merge Volumes section of old keepstore config into cluster config.
+func (ldr *Loader) migrateOldKeepstoreVolumes(cluster *arvados.Cluster, oc oldKeepstoreConfig, myURL arvados.URL) error {
+	for i, oldvol := range *oc.Volumes {
+		var accessViaHosts map[arvados.URL]arvados.VolumeAccess
+		oldUUID, found := ldr.alreadyMigrated(oldvol, cluster.Volumes, myURL)
+		if found {
+			accessViaHosts = cluster.Volumes[oldUUID].AccessViaHosts
+			writers := false
+			for _, va := range accessViaHosts {
+				if !va.ReadOnly {
+					writers = true
 				}
 			}
-			var newvol arvados.Volume
-			if found {
+			if writers || len(accessViaHosts) == 0 {
 				ldr.Logger.Infof("ignoring volume #%d's parameters in legacy keepstore config: using matching entry in cluster config instead", i)
-				newvol = cluster.Volumes[oldUUID]
-				// Remove the old entry. It will be
-				// added back below, possibly with a
-				// new UUID.
-				delete(cluster.Volumes, oldUUID)
-			} else {
-				var params interface{}
-				switch oldvol.Type {
-				case "S3":
-					accesskeydata, err := ioutil.ReadFile(oldvol.AccessKeyFile)
-					if err != nil && oldvol.AccessKeyFile != "" {
-						return fmt.Errorf("error reading AccessKeyFile: %s", err)
-					}
-					secretkeydata, err := ioutil.ReadFile(oldvol.SecretKeyFile)
-					if err != nil && oldvol.SecretKeyFile != "" {
-						return fmt.Errorf("error reading SecretKeyFile: %s", err)
-					}
-					newvol = arvados.Volume{
-						Driver:         "S3",
-						ReadOnly:       oldvol.ReadOnly,
-						Replication:    oldvol.S3Replication,
-						StorageClasses: array2boolmap(oldvol.StorageClasses),
-					}
-					params = arvados.S3VolumeDriverParameters{
-						AccessKey:          string(bytes.TrimSpace(accesskeydata)),
-						SecretKey:          string(bytes.TrimSpace(secretkeydata)),
-						Endpoint:           oldvol.Endpoint,
-						Region:             oldvol.Region,
-						Bucket:             oldvol.Bucket,
-						LocationConstraint: oldvol.LocationConstraint,
-						IndexPageSize:      oldvol.IndexPageSize,
-						ConnectTimeout:     oldvol.ConnectTimeout,
-						ReadTimeout:        oldvol.ReadTimeout,
-						RaceWindow:         oldvol.RaceWindow,
-						UnsafeDelete:       oldvol.UnsafeDelete,
-					}
-				case "Azure":
-					keydata, err := ioutil.ReadFile(oldvol.StorageAccountKeyFile)
-					if err != nil && oldvol.StorageAccountKeyFile != "" {
-						return fmt.Errorf("error reading StorageAccountKeyFile: %s", err)
-					}
-					newvol = arvados.Volume{
-						Driver:         "Azure",
-						ReadOnly:       oldvol.ReadOnly,
-						Replication:    oldvol.AzureReplication,
-						StorageClasses: array2boolmap(oldvol.StorageClasses),
-					}
-					params = arvados.AzureVolumeDriverParameters{
-						StorageAccountName:   oldvol.StorageAccountName,
-						StorageAccountKey:    string(bytes.TrimSpace(keydata)),
-						StorageBaseURL:       oldvol.StorageBaseURL,
-						ContainerName:        oldvol.ContainerName,
-						RequestTimeout:       oldvol.RequestTimeout,
-						ListBlobsRetryDelay:  oldvol.ListBlobsRetryDelay,
-						ListBlobsMaxAttempts: oldvol.ListBlobsMaxAttempts,
-					}
-				case "Directory":
-					newvol = arvados.Volume{
-						Driver:         "Directory",
-						ReadOnly:       oldvol.ReadOnly,
-						Replication:    oldvol.DirectoryReplication,
-						StorageClasses: array2boolmap(oldvol.StorageClasses),
-					}
-					params = arvados.DirectoryVolumeDriverParameters{
-						Root:      oldvol.Root,
-						Serialize: oldvol.Serialize,
-					}
-				default:
-					return fmt.Errorf("unsupported volume type %q", oldvol.Type)
-				}
-				dp, err := json.Marshal(params)
-				if err != nil {
-					return err
-				}
-				newvol.DriverParameters = json.RawMessage(dp)
-				if newvol.Replication < 1 {
-					newvol.Replication = 1
+				if len(accessViaHosts) > 0 {
+					cluster.Volumes[oldUUID].AccessViaHosts[myURL] = arvados.VolumeAccess{ReadOnly: oldvol.ReadOnly}
 				}
+				continue
 			}
-			if accessViaHosts == nil {
-				accessViaHosts = make(map[arvados.URL]arvados.VolumeAccess, 1)
+		}
+		var newvol arvados.Volume
+		if found {
+			ldr.Logger.Infof("ignoring volume #%d's parameters in legacy keepstore config: using matching entry in cluster config instead", i)
+			newvol = cluster.Volumes[oldUUID]
+			// Remove the old entry. It will be added back
+			// below, possibly with a new UUID.
+			delete(cluster.Volumes, oldUUID)
+		} else {
+			v, err := ldr.translateOldKeepstoreVolume(oldvol)
+			if err != nil {
+				return err
 			}
-			accessViaHosts[myURL] = arvados.VolumeAccess{ReadOnly: oldvol.ReadOnly}
-			newvol.AccessViaHosts = accessViaHosts
-
-			volUUID := oldUUID
-			if oldvol.ReadOnly {
-			} else if oc.Listen == nil {
-				ldr.Logger.Warn("cannot find optimal volume UUID because Listen address is not given in legacy keepstore config")
-			} else if uuid, _, err := findKeepServicesItem(cluster, *oc.Listen); err != nil {
-				ldr.Logger.WithError(err).Warn("cannot find optimal volume UUID: failed to find a matching keep_service listing for this legacy keepstore config")
-			} else if len(uuid) != 27 {
-				ldr.Logger.WithField("UUID", uuid).Warn("cannot find optimal volume UUID: keep_service UUID does not have expected format")
+			newvol = v
+		}
+		if accessViaHosts == nil {
+			accessViaHosts = make(map[arvados.URL]arvados.VolumeAccess, 1)
+		}
+		accessViaHosts[myURL] = arvados.VolumeAccess{ReadOnly: oldvol.ReadOnly}
+		newvol.AccessViaHosts = accessViaHosts
+
+		volUUID := oldUUID
+		if oldvol.ReadOnly {
+		} else if oc.Listen == nil {
+			ldr.Logger.Warn("cannot find optimal volume UUID because Listen address is not given in legacy keepstore config")
+		} else if uuid, _, err := findKeepServicesItem(cluster, *oc.Listen); err != nil {
+			ldr.Logger.WithError(err).Warn("cannot find optimal volume UUID: failed to find a matching keep_service listing for this legacy keepstore config")
+		} else if len(uuid) != 27 {
+			ldr.Logger.WithField("UUID", uuid).Warn("cannot find optimal volume UUID: keep_service UUID does not have expected format")
+		} else {
+			rendezvousUUID := cluster.ClusterID + "-nyw5e-" + uuid[12:]
+			if _, ok := cluster.Volumes[rendezvousUUID]; ok {
+				ldr.Logger.Warn("suggesting a random volume UUID because the volume ID matching our keep_service UUID is already in use")
 			} else {
-				rendezvousUUID := cluster.ClusterID + "-nyw5e-" + uuid[12:]
-				if _, ok := cluster.Volumes[rendezvousUUID]; ok {
-					ldr.Logger.Warn("suggesting a random volume UUID because the volume ID matching our keep_service UUID is already in use")
-				} else {
-					volUUID = rendezvousUUID
-				}
-				si := cluster.Services.Keepstore.InternalURLs[myURL]
-				si.Rendezvous = uuid[12:]
-				cluster.Services.Keepstore.InternalURLs[myURL] = si
-			}
-			if volUUID == "" {
-				volUUID = newUUID(cluster.ClusterID, "nyw5e")
-				ldr.Logger.WithField("UUID", volUUID).Infof("suggesting a random volume UUID for volume #%d in legacy config", i)
+				volUUID = rendezvousUUID
 			}
-			cluster.Volumes[volUUID] = newvol
+			si := cluster.Services.Keepstore.InternalURLs[myURL]
+			si.Rendezvous = uuid[12:]
+			cluster.Services.Keepstore.InternalURLs[myURL] = si
 		}
+		if volUUID == "" {
+			volUUID = newUUID(cluster.ClusterID, "nyw5e")
+			ldr.Logger.WithField("UUID", volUUID).Infof("suggesting a random volume UUID for volume #%d in legacy config", i)
+		}
+		cluster.Volumes[volUUID] = newvol
 	}
-
-	cfg.Clusters[cluster.ClusterID] = *cluster
 	return nil
 }
 
+func (ldr *Loader) translateOldKeepstoreVolume(oldvol oldKeepstoreVolume) (arvados.Volume, error) {
+	var newvol arvados.Volume
+	var params interface{}
+	switch oldvol.Type {
+	case "S3":
+		accesskeydata, err := ioutil.ReadFile(oldvol.AccessKeyFile)
+		if err != nil && oldvol.AccessKeyFile != "" {
+			return newvol, fmt.Errorf("error reading AccessKeyFile: %s", err)
+		}
+		secretkeydata, err := ioutil.ReadFile(oldvol.SecretKeyFile)
+		if err != nil && oldvol.SecretKeyFile != "" {
+			return newvol, fmt.Errorf("error reading SecretKeyFile: %s", err)
+		}
+		newvol = arvados.Volume{
+			Driver:         "S3",
+			ReadOnly:       oldvol.ReadOnly,
+			Replication:    oldvol.S3Replication,
+			StorageClasses: array2boolmap(oldvol.StorageClasses),
+		}
+		params = arvados.S3VolumeDriverParameters{
+			AccessKey:          string(bytes.TrimSpace(accesskeydata)),
+			SecretKey:          string(bytes.TrimSpace(secretkeydata)),
+			Endpoint:           oldvol.Endpoint,
+			Region:             oldvol.Region,
+			Bucket:             oldvol.Bucket,
+			LocationConstraint: oldvol.LocationConstraint,
+			IndexPageSize:      oldvol.IndexPageSize,
+			ConnectTimeout:     oldvol.ConnectTimeout,
+			ReadTimeout:        oldvol.ReadTimeout,
+			RaceWindow:         oldvol.RaceWindow,
+			UnsafeDelete:       oldvol.UnsafeDelete,
+		}
+	case "Azure":
+		keydata, err := ioutil.ReadFile(oldvol.StorageAccountKeyFile)
+		if err != nil && oldvol.StorageAccountKeyFile != "" {
+			return newvol, fmt.Errorf("error reading StorageAccountKeyFile: %s", err)
+		}
+		newvol = arvados.Volume{
+			Driver:         "Azure",
+			ReadOnly:       oldvol.ReadOnly,
+			Replication:    oldvol.AzureReplication,
+			StorageClasses: array2boolmap(oldvol.StorageClasses),
+		}
+		params = arvados.AzureVolumeDriverParameters{
+			StorageAccountName:   oldvol.StorageAccountName,
+			StorageAccountKey:    string(bytes.TrimSpace(keydata)),
+			StorageBaseURL:       oldvol.StorageBaseURL,
+			ContainerName:        oldvol.ContainerName,
+			RequestTimeout:       oldvol.RequestTimeout,
+			ListBlobsRetryDelay:  oldvol.ListBlobsRetryDelay,
+			ListBlobsMaxAttempts: oldvol.ListBlobsMaxAttempts,
+		}
+	case "Directory":
+		newvol = arvados.Volume{
+			Driver:         "Directory",
+			ReadOnly:       oldvol.ReadOnly,
+			Replication:    oldvol.DirectoryReplication,
+			StorageClasses: array2boolmap(oldvol.StorageClasses),
+		}
+		params = arvados.DirectoryVolumeDriverParameters{
+			Root:      oldvol.Root,
+			Serialize: oldvol.Serialize,
+		}
+	default:
+		return newvol, fmt.Errorf("unsupported volume type %q", oldvol.Type)
+	}
+	dp, err := json.Marshal(params)
+	if err != nil {
+		return newvol, err
+	}
+	newvol.DriverParameters = json.RawMessage(dp)
+	if newvol.Replication < 1 {
+		newvol.Replication = 1
+	}
+	return newvol, nil
+}
+
 func (ldr *Loader) alreadyMigrated(oldvol oldKeepstoreVolume, newvols map[string]arvados.Volume, myURL arvados.URL) (string, bool) {
 	for uuid, newvol := range newvols {
 		if oldvol.Type != newvol.Driver {

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list