Adding a check and default huge value for deprecated cluster setting (#1053)

* Adding a check and default huge value for deprecated `kv.snapshot_recovery.max_rate` cluster setting.

* Update cockroach versions for e2e tests


* update version checker tests

Author: Yandu Oppacher

config/manager/patches/image.yaml

@@ -284,6 +284,8 @@ spec:
               value: cockroachdb/cockroach:v23.1.26
             - name: RELATED_IMAGE_COCKROACH_v23_1_27
               value: cockroachdb/cockroach:v23.1.27
+            - name: RELATED_IMAGE_COCKROACH_v23_1_28
+              value: cockroachdb/cockroach:v23.1.28
             - name: RELATED_IMAGE_COCKROACH_v23_2_0
               value: cockroachdb/cockroach:v23.2.0
             - name: RELATED_IMAGE_COCKROACH_v23_2_1

config/manifests/bases/cockroach-operator.clusterserviceversion.yaml

@@ -457,6 +457,8 @@ spec:
     name: RELATED_IMAGE_COCKROACH_v23_1_26
   - image: registry.connect.redhat.com/cockroachdb/cockroach@sha256:19f36f53f7da67755eb86da77098c5eeb84b381f9c18aa29ba573793dc498564
     name: RELATED_IMAGE_COCKROACH_v23_1_27
+  - image: registry.connect.redhat.com/cockroachdb/cockroach@sha256:8737b67b91983817e3e4ff3f47f0b070bd36162d309b60993529426c6979cb24
+    name: RELATED_IMAGE_COCKROACH_v23_1_28
   - image: registry.connect.redhat.com/cockroachdb/cockroach@sha256:56109e57ee0379cf48644bcf8226a9238e01139cacc5499002c99f973f121911
     name: RELATED_IMAGE_COCKROACH_v23_2_0
   - image: registry.connect.redhat.com/cockroachdb/cockroach@sha256:4e5f7df1dc1e1db398c36d590431e7e5782897b209972d8e9e4671971c10d1b6

config/manifests/patches/deployment_patch.yaml

@@ -293,6 +293,8 @@ spec:
               value: registry.connect.redhat.com/cockroachdb/cockroach@sha256:d467383af41aa80c26172964c34152abeba45121599804e502984655b72179f0
             - name: RELATED_IMAGE_COCKROACH_v23_1_27
               value: registry.connect.redhat.com/cockroachdb/cockroach@sha256:19f36f53f7da67755eb86da77098c5eeb84b381f9c18aa29ba573793dc498564
+            - name: RELATED_IMAGE_COCKROACH_v23_1_28
+              value: registry.connect.redhat.com/cockroachdb/cockroach@sha256:8737b67b91983817e3e4ff3f47f0b070bd36162d309b60993529426c6979cb24
             - name: RELATED_IMAGE_COCKROACH_v23_2_0
               value: registry.connect.redhat.com/cockroachdb/cockroach@sha256:56109e57ee0379cf48644bcf8226a9238e01139cacc5499002c99f973f121911
             - name: RELATED_IMAGE_COCKROACH_v23_2_1

crdb-versions.yaml

@@ -409,6 +409,9 @@ CrdbVersions:
 - image: cockroachdb/cockroach:v23.1.27
   redhatImage: registry.connect.redhat.com/cockroachdb/cockroach@sha256:19f36f53f7da67755eb86da77098c5eeb84b381f9c18aa29ba573793dc498564
   tag: v23.1.27
+- image: cockroachdb/cockroach:v23.1.28
+  redhatImage: registry.connect.redhat.com/cockroachdb/cockroach@sha256:8737b67b91983817e3e4ff3f47f0b070bd36162d309b60993529426c6979cb24
+  tag: v23.1.28
 - image: cockroachdb/cockroach:v23.2.0
   redhatImage: registry.connect.redhat.com/cockroachdb/cockroach@sha256:56109e57ee0379cf48644bcf8226a9238e01139cacc5499002c99f973f121911
   tag: v23.2.0

e2e/e2e.go

@@ -28,9 +28,9 @@ var (
 
 // Some common values used in e2e test suites.
 const (
-	MinorVersion1      = "cockroachdb/cockroach:v20.2.8"
-	MinorVersion2      = "cockroachdb/cockroach:v20.2.9"
-	MajorVersion       = "cockroachdb/cockroach:v21.1.0"
+	MinorVersion1      = "cockroachdb/cockroach:v24.1.0"
+	MinorVersion2      = "cockroachdb/cockroach:v24.1.2"
+	MajorVersion       = "cockroachdb/cockroach:v24.2.2"
 	NonExistentVersion = "cockroachdb/cockroach-non-existent:v21.1.999"
 	SkipFeatureVersion = "cockroachdb/cockroach:v20.1.0"
 	InvalidImage       = "nginx:latest"

e2e/versionchecker/versionchecker_test.go

@@ -93,7 +93,7 @@ func TestLoggingAPIValidCheck(t *testing.T) {
 	testutil.RequireLoggingConfigMap(t, sb, "logging-configmap", string(logJson))
 
 	builder := testutil.NewBuilder("crdb").Namespaced(sb.Namespace).WithNodeCount(3).WithTLS().
-		WithImage("cockroachdb/cockroach:v21.1.0").
+		WithImage("cockroachdb/cockroach:v24.2.2").
 		WithPVDataStore("32Mi").
 		WithClusterLogging("logging-configmap")
 

install/operator.yaml

@@ -642,6 +642,8 @@ spec:
           value: cockroachdb/cockroach:v23.1.26
         - name: RELATED_IMAGE_COCKROACH_v23_1_27
           value: cockroachdb/cockroach:v23.1.27
+        - name: RELATED_IMAGE_COCKROACH_v23_1_28
+          value: cockroachdb/cockroach:v23.1.28
         - name: RELATED_IMAGE_COCKROACH_v23_2_0
           value: cockroachdb/cockroach:v23.2.0
         - name: RELATED_IMAGE_COCKROACH_v23_2_1

pkg/clustersql/settings.go

@@ -80,7 +80,9 @@ func RangeMoveDuration(ctx context.Context, db *sql.DB, zones ...Zone) (time.Dur
 
 	recoveryRate, err := GetClusterSetting(ctx, db, "kv.snapshot_recovery.max_rate")
 	if err != nil {
-		return 0, errors.Wrap(err, "failed to get kv.snapshot_recovery.max_rate")
+		// This setting has been removed in 24.1, so if an error is returned set it to a default
+		// huge number
+		recoveryRate = "10TB"
 	}
 
 	rebalanceBytes, err := humanize.ParseBytes(rebalanceRate)

pkg/clustersql/settings_test.go

@@ -199,7 +199,6 @@ func TestRangeMoveDuration(t *testing.T) {
 		// make it easier to stub previous values when validating them in a loop.
 		tests := []string{
 			"kv.snapshot_rebalance.max_rate",
-			"kv.snapshot_recovery.max_rate",
 		}
 
 		for i, tt := range tests {
@@ -248,4 +247,22 @@ func TestRangeMoveDuration(t *testing.T) {
 			require.Contains(t, err.Error(), fmt.Sprintf("failed to parse %s as uint64", tt.badKey))
 		}
 	})
+
+	t.Run("returns value when recovery rate setting not found", func(t *testing.T) {
+		mock.
+			ExpectQuery("SHOW CLUSTER SETTING " + "kv.snapshot_rebalance.max_rate").
+			WillReturnRows(sqlmock.NewRows([]string{"name"}).AddRow("1MB"))
+
+		// Recovery rate setting not found, so returns an error
+		mock.
+			ExpectQuery("SHOW CLUSTER SETTING " + "kv.snapshot_recovery.max_rate").
+			WillReturnError(errors.New("boom"))
+
+		d, err := RangeMoveDuration(ctx, db,
+			Zone{Target: "zone-1", Config: ZoneConfig{RangeMaxBytes: 2500000}},
+			Zone{Target: "zone-2", Config: ZoneConfig{RangeMaxBytes: 3750000}},
+		)
+		require.NoError(t, err)
+		require.Equal(t, time.Duration(3)*time.Second, d)
+	})
 }