From bddf5e108dadcd7bbd1109eb987739263163e07f Mon Sep 17 00:00:00 2001 From: Yandu Oppacher Date: Thu, 17 Oct 2024 21:47:02 -0400 Subject: [PATCH] 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 --- config/manager/patches/image.yaml | 2 ++ ...kroach-operator.clusterserviceversion.yaml | 2 ++ .../manifests/patches/deployment_patch.yaml | 2 ++ crdb-versions.yaml | 3 +++ e2e/e2e.go | 6 +++--- e2e/versionchecker/versionchecker_test.go | 2 +- install/operator.yaml | 2 ++ pkg/clustersql/settings.go | 4 +++- pkg/clustersql/settings_test.go | 19 ++++++++++++++++++- 9 files changed, 36 insertions(+), 6 deletions(-) diff --git a/config/manager/patches/image.yaml b/config/manager/patches/image.yaml index ee03ceb3c..5b5d7e6d2 100644 --- a/config/manager/patches/image.yaml +++ b/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 diff --git a/config/manifests/bases/cockroach-operator.clusterserviceversion.yaml b/config/manifests/bases/cockroach-operator.clusterserviceversion.yaml index e8850ebef..15992f272 100644 --- a/config/manifests/bases/cockroach-operator.clusterserviceversion.yaml +++ b/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 diff --git a/config/manifests/patches/deployment_patch.yaml b/config/manifests/patches/deployment_patch.yaml index 2513dd66b..429a678f0 100644 --- a/config/manifests/patches/deployment_patch.yaml +++ b/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 diff --git a/crdb-versions.yaml b/crdb-versions.yaml index 7d5081f6b..b286d0f25 100644 --- a/crdb-versions.yaml +++ b/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 diff --git a/e2e/e2e.go b/e2e/e2e.go index 0af82ec1d..d27cdf14c 100644 --- a/e2e/e2e.go +++ b/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" diff --git a/e2e/versionchecker/versionchecker_test.go b/e2e/versionchecker/versionchecker_test.go index 6c9b644cc..cd11306d2 100644 --- a/e2e/versionchecker/versionchecker_test.go +++ b/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") diff --git a/install/operator.yaml b/install/operator.yaml index 48d435566..7e07c2698 100644 --- a/install/operator.yaml +++ b/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 diff --git a/pkg/clustersql/settings.go b/pkg/clustersql/settings.go index 06f8cc131..84b5a06d0 100644 --- a/pkg/clustersql/settings.go +++ b/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) diff --git a/pkg/clustersql/settings_test.go b/pkg/clustersql/settings_test.go index 2498f1ac2..c9366f228 100644 --- a/pkg/clustersql/settings_test.go +++ b/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) + }) }