From bad73626c4208fb95ac8e3d9503fc4482f936cb3 Mon Sep 17 00:00:00 2001 From: Tom Clegg Date: Thu, 5 Aug 2021 11:04:37 -0400 Subject: [PATCH] 17967: Use StorageClasses.*.Default instead of ["default"]. Arvados-DCO-1.1-Signed-off-by: Tom Clegg --- sdk/python/arvados/commands/put.py | 2 +- services/api/app/models/collection.rb | 4 ++-- services/api/app/models/container.rb | 2 +- services/api/app/models/container_request.rb | 2 +- services/api/config/arvados_config.rb | 12 ++++++++++++ services/keep-balance/balance.go | 4 ---- services/keep-balance/balance_run_test.go | 20 ++++++++++++-------- services/keep-balance/balance_test.go | 12 +++++++----- services/keepstore/handlers.go | 7 +++++++ 9 files changed, 43 insertions(+), 22 deletions(-) diff --git a/sdk/python/arvados/commands/put.py b/sdk/python/arvados/commands/put.py index ad04807712..d8e673bd34 100644 --- a/sdk/python/arvados/commands/put.py +++ b/sdk/python/arvados/commands/put.py @@ -913,7 +913,7 @@ class ArvPutUploadJob(object): self._local_collection = arvados.collection.Collection( self._state['manifest'], replication_desired=self.replication_desired, - storage_classes_desired=(self.storage_classes or ['default']), + storage_classes_desired=self.storage_classes, put_threads=self.put_threads, api_client=self._api_client, num_retries=self.num_retries) diff --git a/services/api/app/models/collection.rb b/services/api/app/models/collection.rb index 4e7b64cf53..5edca82a04 100644 --- a/services/api/app/models/collection.rb +++ b/services/api/app/models/collection.rb @@ -17,7 +17,7 @@ class Collection < ArvadosModel # Posgresql JSONB columns should NOT be declared as serialized, Rails 5 # already know how to properly treat them. attribute :properties, :jsonbHash, default: {} - attribute :storage_classes_desired, :jsonbArray, default: ["default"] + attribute :storage_classes_desired, :jsonbArray, default: Rails.configuration.DefaultStorageClasses attribute :storage_classes_confirmed, :jsonbArray, default: [] before_validation :default_empty_manifest @@ -630,7 +630,7 @@ class Collection < ArvadosModel # validation on empty desired storage classes return an error. def default_storage_classes if self.storage_classes_desired.nil? || self.storage_classes_desired.empty? - self.storage_classes_desired = ["default"] + self.storage_classes_desired = Rails.configuration.DefaultStorageClasses end self.storage_classes_confirmed ||= [] end diff --git a/services/api/app/models/container.rb b/services/api/app/models/container.rb index af058494b2..a880b65ac5 100644 --- a/services/api/app/models/container.rb +++ b/services/api/app/models/container.rb @@ -22,7 +22,7 @@ class Container < ArvadosModel attribute :secret_mounts, :jsonbHash, default: {} attribute :runtime_status, :jsonbHash, default: {} attribute :runtime_auth_scopes, :jsonbArray, default: [] - attribute :output_storage_classes, :jsonbArray, default: ["default"] + attribute :output_storage_classes, :jsonbArray, default: Rails.configuration.DefaultStorageClasses serialize :environment, Hash serialize :mounts, Hash diff --git a/services/api/app/models/container_request.rb b/services/api/app/models/container_request.rb index 1de71102c6..f603d4dd79 100644 --- a/services/api/app/models/container_request.rb +++ b/services/api/app/models/container_request.rb @@ -23,7 +23,7 @@ class ContainerRequest < ArvadosModel # already know how to properly treat them. attribute :properties, :jsonbHash, default: {} attribute :secret_mounts, :jsonbHash, default: {} - attribute :output_storage_classes, :jsonbArray, default: ["default"] + attribute :output_storage_classes, :jsonbArray, default: Rails.configuration.DefaultStorageClasses serialize :environment, Hash serialize :mounts, Hash diff --git a/services/api/config/arvados_config.rb b/services/api/config/arvados_config.rb index a6f1730e86..1b3c96a8ad 100644 --- a/services/api/config/arvados_config.rb +++ b/services/api/config/arvados_config.rb @@ -170,6 +170,7 @@ arvcfg.declare_config "RemoteClusters", Hash, :remote_hosts, ->(cfg, k, v) { ConfigLoader.set_cfg cfg, "RemoteClusters", h } arvcfg.declare_config "RemoteClusters.*.Proxy", Boolean, :remote_hosts_via_dns +arvcfg.declare_config "StorageClasses", Hash dbcfg = ConfigLoader.new @@ -237,6 +238,17 @@ if $arvados_config["Collections"]["DefaultTrashLifetime"] < 86400.seconds then raise "default_trash_lifetime is %d, must be at least 86400" % Rails.configuration.Collections.DefaultTrashLifetime end +default_storage_classes = [] +$arvados_config["StorageClasses"].each do |cls, cfg| + if cfg["Default"] + default_storage_classes << cls + end +end +if default_storage_classes.length == 0 + default_storage_classes = ["default"] +end +$arvados_config["DefaultStorageClasses"] = default_storage_classes.sort + # # Special case for test database where there's no database.yml, # because the Arvados config.yml doesn't have a concept of multiple diff --git a/services/keep-balance/balance.go b/services/keep-balance/balance.go index e69d941b1e..bb590e13b3 100644 --- a/services/keep-balance/balance.go +++ b/services/keep-balance/balance.go @@ -538,10 +538,6 @@ func (bal *Balancer) setupLookupTables() { // effectively read-only. mnt.ReadOnly = mnt.ReadOnly || srv.ReadOnly - if len(mnt.StorageClasses) == 0 { - bal.mountsByClass["default"][mnt] = true - continue - } for class := range mnt.StorageClasses { if mbc := bal.mountsByClass[class]; mbc == nil { bal.classes = append(bal.classes, class) diff --git a/services/keep-balance/balance_run_test.go b/services/keep-balance/balance_run_test.go index 18a8bdcf47..4e2c6803ca 100644 --- a/services/keep-balance/balance_run_test.go +++ b/services/keep-balance/balance_run_test.go @@ -87,20 +87,24 @@ var stubServices = []arvados.KeepService{ var stubMounts = map[string][]arvados.KeepMount{ "keep0.zzzzz.arvadosapi.com:25107": {{ - UUID: "zzzzz-ivpuk-000000000000000", - DeviceID: "keep0-vol0", + UUID: "zzzzz-ivpuk-000000000000000", + DeviceID: "keep0-vol0", + StorageClasses: map[string]bool{"default": true}, }}, "keep1.zzzzz.arvadosapi.com:25107": {{ - UUID: "zzzzz-ivpuk-100000000000000", - DeviceID: "keep1-vol0", + UUID: "zzzzz-ivpuk-100000000000000", + DeviceID: "keep1-vol0", + StorageClasses: map[string]bool{"default": true}, }}, "keep2.zzzzz.arvadosapi.com:25107": {{ - UUID: "zzzzz-ivpuk-200000000000000", - DeviceID: "keep2-vol0", + UUID: "zzzzz-ivpuk-200000000000000", + DeviceID: "keep2-vol0", + StorageClasses: map[string]bool{"default": true}, }}, "keep3.zzzzz.arvadosapi.com:25107": {{ - UUID: "zzzzz-ivpuk-300000000000000", - DeviceID: "keep3-vol0", + UUID: "zzzzz-ivpuk-300000000000000", + DeviceID: "keep3-vol0", + StorageClasses: map[string]bool{"default": true}, }}, } diff --git a/services/keep-balance/balance_test.go b/services/keep-balance/balance_test.go index 5bc66dbf3f..c529ac150e 100644 --- a/services/keep-balance/balance_test.go +++ b/services/keep-balance/balance_test.go @@ -85,7 +85,8 @@ func (bal *balancerSuite) SetUpTest(c *check.C) { } srv.mounts = []*KeepMount{{ KeepMount: arvados.KeepMount{ - UUID: fmt.Sprintf("zzzzz-mount-%015x", i), + UUID: fmt.Sprintf("zzzzz-mount-%015x", i), + StorageClasses: map[string]bool{"default": true}, }, KeepService: srv, }} @@ -166,10 +167,11 @@ func (bal *balancerSuite) testMultipleViews(c *check.C, readonly bool) { srv.mounts[0].KeepMount.DeviceID = fmt.Sprintf("writable-by-srv-%x", i) srv.mounts = append(srv.mounts, &KeepMount{ KeepMount: arvados.KeepMount{ - DeviceID: fmt.Sprintf("writable-by-srv-%x", (i+1)%len(bal.srvs)), - UUID: fmt.Sprintf("zzzzz-mount-%015x", i<<16), - ReadOnly: readonly, - Replication: 1, + DeviceID: fmt.Sprintf("writable-by-srv-%x", (i+1)%len(bal.srvs)), + UUID: fmt.Sprintf("zzzzz-mount-%015x", i<<16), + ReadOnly: readonly, + Replication: 1, + StorageClasses: map[string]bool{"default": true}, }, KeepService: srv, }) diff --git a/services/keepstore/handlers.go b/services/keepstore/handlers.go index a60d17d576..2b469a13eb 100644 --- a/services/keepstore/handlers.go +++ b/services/keepstore/handlers.go @@ -252,6 +252,13 @@ func (rtr *router) handlePUT(resp http.ResponseWriter, req *http.Request) { for i, sc := range wantStorageClasses { wantStorageClasses[i] = strings.TrimSpace(sc) } + } else { + // none specified -- use configured default + for class, cfg := range rtr.cluster.StorageClasses { + if cfg.Default { + wantStorageClasses = append(wantStorageClasses, class) + } + } } buf, err := getBufferWithContext(ctx, bufs, int(req.ContentLength)) -- 2.30.2