From 0127421546c793687cc45fe3970c1072e0e37da0 Mon Sep 17 00:00:00 2001 From: udnay Date: Tue, 15 Oct 2024 11:27:36 -0400 Subject: [PATCH 1/5] Adding a check and default huge value for deprecated `kv.snapshot_recovery.max_rate` cluster setting. --- pkg/clustersql/settings.go | 10 ++++++---- pkg/clustersql/settings_test.go | 18 ++++++++++++++++++ 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/pkg/clustersql/settings.go b/pkg/clustersql/settings.go index 06f8cc131..f56dcaca5 100644 --- a/pkg/clustersql/settings.go +++ b/pkg/clustersql/settings.go @@ -78,14 +78,16 @@ func RangeMoveDuration(ctx context.Context, db *sql.DB, zones ...Zone) (time.Dur return 0, errors.Wrap(err, "failed to get kv.snapshot_rebalance.max_rate") } - recoveryRate, err := GetClusterSetting(ctx, db, "kv.snapshot_recovery.max_rate") + rebalanceBytes, err := humanize.ParseBytes(rebalanceRate) if err != nil { - return 0, errors.Wrap(err, "failed to get kv.snapshot_recovery.max_rate") + return 0, errors.Wrap(err, "failed to parse kv.snapshot_rebalance.max_rate as uint64") } - rebalanceBytes, err := humanize.ParseBytes(rebalanceRate) + recoveryRate, err := GetClusterSetting(ctx, db, "kv.snapshot_recovery.max_rate") if err != nil { - return 0, errors.Wrap(err, "failed to parse kv.snapshot_rebalance.max_rate as uint64") + // This setting has been removed in 24.1, so if an error is returned set it to a default + // huge number + recoveryRate = "10TB" } recoveryBytes, err := humanize.ParseBytes(recoveryRate) diff --git a/pkg/clustersql/settings_test.go b/pkg/clustersql/settings_test.go index 2498f1ac2..5acfa9b72 100644 --- a/pkg/clustersql/settings_test.go +++ b/pkg/clustersql/settings_test.go @@ -248,4 +248,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) + }) } From 824e2c7c2276d54ae069f81ab32e0b9519081113 Mon Sep 17 00:00:00 2001 From: udnay Date: Tue, 15 Oct 2024 11:37:07 -0400 Subject: [PATCH 2/5] Update cockroach versions for e2e tests --- e2e/e2e.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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" From 91253eb73032ece97fe203d5c670b9b539c5737a Mon Sep 17 00:00:00 2001 From: udnay Date: Wed, 16 Oct 2024 09:34:49 -0400 Subject: [PATCH 3/5] regen files --- config/manager/patches/image.yaml | 2 ++ .../bases/cockroach-operator.clusterserviceversion.yaml | 2 ++ config/manifests/patches/deployment_patch.yaml | 2 ++ crdb-versions.yaml | 3 +++ install/operator.yaml | 2 ++ 5 files changed, 11 insertions(+) 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/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 From cfb5f41966b672d6bfe4dfec053d685b647f591c Mon Sep 17 00:00:00 2001 From: udnay Date: Wed, 16 Oct 2024 10:02:16 -0400 Subject: [PATCH 4/5] Fixup ordering --- pkg/clustersql/settings.go | 10 +++++----- pkg/clustersql/settings_test.go | 1 - 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/pkg/clustersql/settings.go b/pkg/clustersql/settings.go index f56dcaca5..84b5a06d0 100644 --- a/pkg/clustersql/settings.go +++ b/pkg/clustersql/settings.go @@ -78,11 +78,6 @@ func RangeMoveDuration(ctx context.Context, db *sql.DB, zones ...Zone) (time.Dur return 0, errors.Wrap(err, "failed to get kv.snapshot_rebalance.max_rate") } - rebalanceBytes, err := humanize.ParseBytes(rebalanceRate) - if err != nil { - return 0, errors.Wrap(err, "failed to parse kv.snapshot_rebalance.max_rate as uint64") - } - recoveryRate, err := GetClusterSetting(ctx, db, "kv.snapshot_recovery.max_rate") if err != nil { // This setting has been removed in 24.1, so if an error is returned set it to a default @@ -90,6 +85,11 @@ func RangeMoveDuration(ctx context.Context, db *sql.DB, zones ...Zone) (time.Dur recoveryRate = "10TB" } + rebalanceBytes, err := humanize.ParseBytes(rebalanceRate) + if err != nil { + return 0, errors.Wrap(err, "failed to parse kv.snapshot_rebalance.max_rate as uint64") + } + recoveryBytes, err := humanize.ParseBytes(recoveryRate) if err != nil { return 0, errors.Wrap(err, "failed to parse kv.snapshot_recovery.max_rate as uint64") diff --git a/pkg/clustersql/settings_test.go b/pkg/clustersql/settings_test.go index 5acfa9b72..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 { From 7481ce7c2fc4f74caf6388e52a6fb14b4d2c6b9a Mon Sep 17 00:00:00 2001 From: udnay Date: Wed, 16 Oct 2024 10:31:16 -0400 Subject: [PATCH 5/5] update version checker tests --- e2e/versionchecker/versionchecker_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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")