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 <ebiagiotti@veritasgenetics.com>

diff --git a/lib/config/config.default.yml b/lib/config/config.default.yml
index 4338c18ed1494894ad7ef92a2e78f84aa922a230..c62a100cedb6c894bf842ecf8d8ef8f194e475af 100644 (file)
--- 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 ba2c79acf197a07f0cd520864d690e8c21bb511a..593a9bdcbcaf51b19a40b91a0fd083a3def2f3ba 100644 (file)
--- 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 8b80d6275be2156bf8ae61e1a31523cbcef90458..ff1bb9434a42c8babc3cedef9165e7ad3d16d949 100644 (file)
--- 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 3806bbd8ad6680743fb4eda0b87dbf5ec86ace58..cd5b6bba14f8c3c70d9d5cd6a0ec035b26b2b106 100644 (file)
--- 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 82e02cce8176f14f5426c96a6baf877e3ef303d7..4379ae535e85f580f63d0e6f040084996cb5edf9 100644 (file)
--- 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."))
        }