From c8bfd534fc6e33b0b37f9fed1ee6232159edb631 Mon Sep 17 00:00:00 2001 From: Eric Biagiotti Date: Mon, 30 Sep 2019 12:08:15 -0400 Subject: [PATCH] 14714: Updates KeepServiceTypes legacy mapping and error messages - Config loading will now fail if the KeepServiceList config option is provided with any value other than "disk", which is equal to the new behavior. - Changes default BalanceCollectionBatch config to zero. Arvados-DCO-1.1-Signed-off-by: Eric Biagiotti --- lib/config/config.default.yml | 8 ++++---- lib/config/deprecated.go | 15 +++++++++++---- lib/config/deprecated_test.go | 12 ++++++++++++ lib/config/generated_config.go | 8 ++++---- services/keep-balance/main.go | 2 +- 5 files changed, 32 insertions(+), 13 deletions(-) diff --git a/lib/config/config.default.yml b/lib/config/config.default.yml index 4338c18ed1..c62a100ced 100644 --- a/lib/config/config.default.yml +++ b/lib/config/config.default.yml @@ -382,9 +382,9 @@ Clusters: # The default is 2 weeks. BlobSigningTTL: 336h - # When running keep-balance, this is the destination filename for the - # list of lost block hashes if there are any, one per line. Updated atomically during - # each successful run. + # When running keep-balance, this is the destination filename for + # the list of lost block hashes if there are any, one per line. + # Updated automically during each successful run. BlobMissingReport: "" # keep-balance operates periodically, i.e.: do a @@ -403,7 +403,7 @@ Clusters: # API transaction. If this is zero, page size is # determined by the API server's own page size limits (see # API.MaxItemsPerResponse and API.MaxIndexDatabaseRead). - BalanceCollectionBatch: 100000 + BalanceCollectionBatch: 0 # The size of keep-balance's internal queue of # collections. Higher values use more memory and improve throughput diff --git a/lib/config/deprecated.go b/lib/config/deprecated.go index ba2c79acf1..593a9bdcbc 100644 --- a/lib/config/deprecated.go +++ b/lib/config/deprecated.go @@ -531,12 +531,19 @@ func (ldr *Loader) loadOldKeepBalanceConfig(cfg *arvados.Config) error { cluster.API.KeepServiceRequestTimeout = *oc.RequestTimeout } - msg := "To balance specfic keep services, please update to the cluster config." - if oc.KeepServiceTypes != nil && len(*oc.KeepServiceTypes) > 0 { - ldr.Logger.Warnf("The KeepServiceType configuration option is not longer supported and is being ignored. %s", msg) + msg := "The %s configuration option is no longer supported. Please remove it from your configuration file. Keep-balance will operate on all configured volumes." + + // If the keep service type provided is "disk" silently ignore it, since + // this is what ends up being done anyway. + if oc.KeepServiceTypes != nil { + numTypes := len(*oc.KeepServiceTypes) + if numTypes != 0 && !(numTypes == 1 && (*oc.KeepServiceTypes)[0] == "disk") { + return fmt.Errorf(msg, "KeepServiceType") + } } + if oc.KeepServiceList != nil { - return fmt.Errorf("The KeepServiceList configuration option is no longer supported. Please remove it from your configuration file. %s", msg) + return fmt.Errorf(msg, "KeepServiceList") } cfg.Clusters[cluster.ClusterID] = *cluster diff --git a/lib/config/deprecated_test.go b/lib/config/deprecated_test.go index 8b80d6275b..ff1bb9434a 100644 --- a/lib/config/deprecated_test.go +++ b/lib/config/deprecated_test.go @@ -236,6 +236,18 @@ func (s *LoadSuite) TestLegacyKeepBalanceConfig(c *check.C) { _, err = testLoadLegacyConfig(content, f, c) c.Check(err, check.IsNil) + content = []byte(fmtKeepBalanceConfig(`"KeepServiceTypes":[],`)) + _, err = testLoadLegacyConfig(content, f, c) + c.Check(err, check.IsNil) + + content = []byte(fmtKeepBalanceConfig(`"KeepServiceTypes":["proxy"],`)) + _, err = testLoadLegacyConfig(content, f, c) + c.Check(err, check.NotNil) + + content = []byte(fmtKeepBalanceConfig(`"KeepServiceTypes":["disk", "proxy"],`)) + _, err = testLoadLegacyConfig(content, f, c) + c.Check(err, check.NotNil) + content = []byte(fmtKeepBalanceConfig(`"KeepServiceList":{},`)) _, err = testLoadLegacyConfig(content, f, c) c.Check(err, check.NotNil) diff --git a/lib/config/generated_config.go b/lib/config/generated_config.go index 3806bbd8ad..cd5b6bba14 100644 --- a/lib/config/generated_config.go +++ b/lib/config/generated_config.go @@ -388,9 +388,9 @@ Clusters: # The default is 2 weeks. BlobSigningTTL: 336h - # When running keep-balance, this is the destination filename for the - # list of lost block hashes if there are any, one per line. Updated atomically during - # each successful run. + # When running keep-balance, this is the destination filename for + # the list of lost block hashes if there are any, one per line. + # Updated automically during each successful run. BlobMissingReport: "" # keep-balance operates periodically, i.e.: do a @@ -409,7 +409,7 @@ Clusters: # API transaction. If this is zero, page size is # determined by the API server's own page size limits (see # API.MaxItemsPerResponse and API.MaxIndexDatabaseRead). - BalanceCollectionBatch: 100000 + BalanceCollectionBatch: 0 # The size of keep-balance's internal queue of # collections. Higher values use more memory and improve throughput diff --git a/services/keep-balance/main.go b/services/keep-balance/main.go index 82e02cce81..4379ae535e 100644 --- a/services/keep-balance/main.go +++ b/services/keep-balance/main.go @@ -29,7 +29,7 @@ var ( func newHandler(ctx context.Context, cluster *arvados.Cluster, token string, registry *prometheus.Registry) service.Handler { if !options.Once && cluster.Collections.BalancePeriod == arvados.Duration(0) { - return service.ErrorHandler(ctx, cluster, fmt.Errorf("You must either run keep-balance with the -once flag, or set Collections.BalancePeriod in the config. "+ + return service.ErrorHandler(ctx, cluster, fmt.Errorf("cannot start service: Collections.BalancePeriod is zero (if you want to run once and then exit, use the -once flag)")) "If using the legacy keep-balance.yml config, RunPeriod is the equivalant of Collections.BalancePeriod.")) } -- 2.30.2