fix: support 3.x to 4.x upgrades when num_tokens are not set in k8ssa…

…ndraCluster (#1118) * fix: skip validation of num_tokens when upgrading from 3.x to 4.x Signed-off-by: Karol Szwaj <[email protected]> * add defaultNumTokens to k8ssadracluster Signed-off-by: Karol Szwaj <[email protected]> * support 3.x to 4.x upgrade when default num_tokens are set Signed-off-by: Karol Szwaj <[email protected]> * adjust webhook Signed-off-by: Karol Szwaj <[email protected]> * adjust webhook test Signed-off-by: Karol Szwaj <[email protected]> * adjust webhook test Signed-off-by: Karol Szwaj <[email protected]> * update test Signed-off-by: Karol Szwaj <[email protected]> * update datacenter test Signed-off-by: Karol Szwaj <[email protected]> * use float Signed-off-by: Karol Szwaj <[email protected]> --------- Signed-off-by: Karol Szwaj <[email protected]>
k8ssandra · Dec 20, 2023 · 5542252 · 5542252
1 parent c5045e8
commit 5542252
Show file tree

Hide file tree

Showing 7 changed files with 168 additions and 22 deletions.
diff --git a/apis/k8ssandra/v1alpha1/k8ssandracluster_types.go b/apis/k8ssandra/v1alpha1/k8ssandracluster_types.go
@@ -17,6 +17,7 @@ limitations under the License.
 package v1alpha1
 
 import (
+	"github.com/Masterminds/semver/v3"
 	cassdcapi "github.com/k8ssandra/cass-operator/apis/cassandra/v1beta1"
 	medusaapi "github.com/k8ssandra/k8ssandra-operator/apis/medusa/v1alpha1"
 	reaperapi "github.com/k8ssandra/k8ssandra-operator/apis/reaper/v1alpha1"
@@ -524,3 +525,10 @@ const (
 	ServerDistributionCassandra = ServerDistribution("cassandra")
 	ServerDistributionDse       = ServerDistribution("dse")
 )
+
+func (kc *K8ssandraCluster) DefaultNumTokens(serverVersion *semver.Version) float64 {
+	if kc.Spec.Cassandra.ServerType == ServerDistributionCassandra && serverVersion.Major() == 3 {
+		return float64(256)
+	}
+	return float64(16)
+}
diff --git a/apis/k8ssandra/v1alpha1/k8ssandracluster_webhook.go b/apis/k8ssandra/v1alpha1/k8ssandracluster_webhook.go
@@ -19,6 +19,7 @@ package v1alpha1
 import (
 	"fmt"
 
+	"github.com/Masterminds/semver/v3"
 	"github.com/k8ssandra/k8ssandra-operator/pkg/clientcache"
 	"github.com/pkg/errors"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -114,18 +115,24 @@ func (r *K8ssandraCluster) ValidateUpdate(old runtime.Object) error {
 
 	oldCassConfig := oldCluster.Spec.Cassandra.DatacenterOptions.CassandraConfig
 	newCassConfig := r.Spec.Cassandra.DatacenterOptions.CassandraConfig
-
-	var oldNumTokens, newNumTokens interface{}
-
-	if oldCassConfig != nil {
-		oldNumTokens = oldCassConfig.CassandraYaml["num_tokens"]
-	}
-	if newCassConfig != nil {
-		newNumTokens = newCassConfig.CassandraYaml["num_tokens"]
-	}
-
-	if oldNumTokens != newNumTokens {
-		return ErrNumTokens
+	if oldCassConfig != nil && newCassConfig != nil {
+		oldNumTokens, oldNumTokensExists := oldCassConfig.CassandraYaml["num_tokens"]
+		newNumTokens, newNumTokensExists := newCassConfig.CassandraYaml["num_tokens"]
+
+		if !oldNumTokensExists {
+			cassVersion, err := semver.NewVersion(oldCluster.Spec.Cassandra.ServerVersion)
+			if err != nil {
+				return err
+			}
+			defaultNumTokens := oldCluster.DefaultNumTokens(cassVersion)
+			if newNumTokensExists && newNumTokens.(float64) != defaultNumTokens {
+				return ErrNumTokens
+			}
+		} else {
+			if oldNumTokens != newNumTokens {
+				return ErrNumTokens
+			}
+		}
 	}
 
 	// Verify that the cluster name override was not changed

diff --git a/apis/k8ssandra/v1alpha1/k8ssandracluster_webhook_test.go b/apis/k8ssandra/v1alpha1/k8ssandracluster_webhook_test.go
@@ -143,6 +143,7 @@ func TestWebhook(t *testing.T) {
 	t.Run("ReaperKeyspaceValidation", testReaperKeyspaceValidation)
 	t.Run("StorageConfigValidation", testStorageConfigValidation)
 	t.Run("NumTokensValidation", testNumTokens)
+	t.Run("NumTokensValidationInUpdate", testNumTokensInUpdate)
 }
 
 func testContextValidation(t *testing.T) {
@@ -159,6 +160,24 @@ func testContextValidation(t *testing.T) {
 	required.Error(err)
 }
 
+func testNumTokensInUpdate(t *testing.T) {
+	require := require.New(t)
+	createNamespace(require, "numtokensupdate-namespace")
+	cluster := createMinimalClusterObj("numtokens-test-update", "numtokensupdate-namespace")
+	cluster.Spec.Cassandra.ServerVersion = "3.11.10"
+	cluster.Spec.Cassandra.DatacenterOptions.CassandraConfig = &CassandraConfig{}
+	err := k8sClient.Create(ctx, cluster)
+	require.NoError(err)
+
+	// Now update to 4.1.3
+	cluster.Spec.Cassandra.DatacenterOptions.CassandraConfig.CassandraYaml = unstructured.Unstructured{"num_tokens": 256}
+	cluster.Spec.Cassandra.ServerVersion = "4.1.3"
+
+	// This should be acceptable change, since 3.11.10 defaulted to 256 and so it is the same value
+	err = k8sClient.Update(ctx, cluster)
+	require.NoError(err)
+}
+
 func testReaperKeyspaceValidation(t *testing.T) {
 	required := require.New(t)
 	createNamespace(required, "update-namespace")
@@ -316,7 +335,6 @@ func testNumTokens(t *testing.T) {
 
 	errorOnValidate = (*newCluster).ValidateUpdate(oldCluster)
 	required.Error(errorOnValidate, "expected error when changing the value of num tokens while also changing other field values")
-
 }
 
 func createNamespace(require *require.Assertions, namespace string) {

diff --git a/controllers/k8ssandra/datacenters.go b/controllers/k8ssandra/datacenters.go
@@ -7,6 +7,7 @@ import (
 	"strconv"
 	"strings"
 
+	"github.com/Masterminds/semver/v3"
 	"github.com/go-logr/logr"
 	cassdcapi "github.com/k8ssandra/cass-operator/apis/cassandra/v1beta1"
 	cassctlapi "github.com/k8ssandra/cass-operator/apis/control/v1alpha1"
@@ -158,7 +159,14 @@ func (r *K8ssandraClusterReconciler) reconcileDatacenters(ctx context.Context, k
 					dcLogger.Error(err, "Stopped cannot be set to true until the CassandraDatacenter is fully rebuilt")
 				}
 
-				if err := cassandra.ValidateConfig(desiredDc, actualDc); err != nil {
+				// Set the proper default during upgrade from 3.x to 4.x
+				if kc.Spec.Cassandra.ServerType == api.ServerDistributionCassandra && semver.MustParse(actualDc.Spec.ServerVersion).Major() == 3 && semver.MustParse(desiredDc.Spec.ServerVersion).Major() > 3 {
+					if desiredDc, err = cassandra.SetNewDefaultNumTokens(kc, desiredDc, actualDc); err != nil {
+						return result.Error(fmt.Errorf("couldn't set the proper default during upgrade: %v", err)), actualDcs
+					}
+				}
+
+				if err = cassandra.ValidateConfig(desiredDc, actualDc); err != nil {
 					return result.Error(fmt.Errorf("invalid Cassandra config: %v", err)), actualDcs
 				}
 

diff --git a/controllers/k8ssandra/k8ssandracluster_controller.go b/controllers/k8ssandra/k8ssandracluster_controller.go
@@ -18,6 +18,7 @@ package k8ssandra
 
 import (
 	"context"
+
 	v1 "k8s.io/api/core/v1"
 
 	"github.com/go-logr/logr"

diff --git a/pkg/cassandra/datacenter.go b/pkg/cassandra/datacenter.go
@@ -1,6 +1,7 @@
 package cassandra
 
 import (
+	"encoding/json"
 	"fmt"
 
 	"github.com/Masterminds/semver/v3"
@@ -517,6 +518,38 @@ func ValidateConfig(desiredDc, actualDc *cassdcapi.CassandraDatacenter) error {
 	return nil
 }
 
+func SetNewDefaultNumTokens(kc *api.K8ssandraCluster, desiredDc, actualDc *cassdcapi.CassandraDatacenter) (*cassdcapi.CassandraDatacenter, error) {
+	desiredConfig, err := utils.UnmarshalToMap(desiredDc.Spec.Config)
+	if err != nil {
+		return nil, err
+	}
+	actualConfig, err := utils.UnmarshalToMap(actualDc.Spec.Config)
+	if err != nil {
+		return nil, err
+	}
+
+	actualCassYaml := actualConfig["cassandra-yaml"].(map[string]interface{})
+	desiredCassYaml := desiredConfig["cassandra-yaml"].(map[string]interface{})
+
+	var numTokensExists bool
+	cassConfig := kc.Spec.Cassandra.CassandraConfig
+	if cassConfig != nil {
+		_, numTokensExists = cassConfig.CassandraYaml["num_tokens"]
+	}
+
+	if !numTokensExists {
+		desiredCassYaml["num_tokens"] = actualCassYaml["num_tokens"]
+		desiredConfig["cassandra-yaml"] = desiredCassYaml
+		newConfig, err := json.Marshal(desiredConfig)
+		if err != nil {
+			return nil, err
+		}
+		desiredDc.Spec.Config = newConfig
+	}
+
+	return desiredDc, nil
+}
+
 func AddOrUpdateVolume(dcConfig *DatacenterConfig, volume *corev1.Volume, volumeIndex int, found bool) {
 	AddOrUpdateVolumeToSpec(&dcConfig.PodTemplateSpec, volume, volumeIndex, found)
 }

diff --git a/pkg/cassandra/datacenter_test.go b/pkg/cassandra/datacenter_test.go
@@ -3,23 +3,21 @@ package cassandra
 import (
 	"testing"
 
+	"github.com/Masterminds/semver/v3"
+	cassdcapi "github.com/k8ssandra/cass-operator/apis/cassandra/v1beta1"
 	"github.com/k8ssandra/cass-operator/pkg/reconciliation"
-
+	api "github.com/k8ssandra/k8ssandra-operator/apis/k8ssandra/v1alpha1"
+	"github.com/k8ssandra/k8ssandra-operator/apis/telemetry/v1alpha1"
 	"github.com/k8ssandra/k8ssandra-operator/pkg/meta"
 	"github.com/k8ssandra/k8ssandra-operator/pkg/unstructured"
-	"k8s.io/utils/pointer"
-
+	"github.com/k8ssandra/k8ssandra-operator/pkg/utils"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-
-	"github.com/Masterminds/semver/v3"
-	cassdcapi "github.com/k8ssandra/cass-operator/apis/cassandra/v1beta1"
-	api "github.com/k8ssandra/k8ssandra-operator/apis/k8ssandra/v1alpha1"
-	"github.com/k8ssandra/k8ssandra-operator/apis/telemetry/v1alpha1"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/utils/pointer"
 )
 
 func TestCoalesce(t *testing.T) {
@@ -1482,3 +1480,76 @@ func GetDatacenterConfig() DatacenterConfig {
 		McacEnabled: true,
 	}
 }
+
+func TestSetNewDefaultNumTokens(t *testing.T) {
+	testCases := []struct {
+		name                string
+		kc                  *api.K8ssandraCluster
+		actualDcConfig      *unstructured.Unstructured
+		desiredDcConfig     *unstructured.Unstructured
+		expectedDcNumTokens float64
+	}{
+		{
+			name: "num_tokens exists in CassandraConfig",
+			kc: &api.K8ssandraCluster{
+				Spec: api.K8ssandraClusterSpec{
+					Cassandra: &api.CassandraClusterTemplate{
+						DatacenterOptions: api.DatacenterOptions{
+							CassandraConfig: &api.CassandraConfig{
+								CassandraYaml: unstructured.Unstructured{"num_tokens": 33},
+							},
+						},
+					},
+				},
+			},
+			actualDcConfig:      &unstructured.Unstructured{"num_tokens": 24},
+			desiredDcConfig:     &unstructured.Unstructured{"num_tokens": 33},
+			expectedDcNumTokens: 33,
+		},
+		{
+			name: "num_tokens does not exist, set to value from actualDc",
+			kc: &api.K8ssandraCluster{
+				Spec: api.K8ssandraClusterSpec{
+					Cassandra: &api.CassandraClusterTemplate{
+						DatacenterOptions: api.DatacenterOptions{
+							CassandraConfig: &api.CassandraConfig{
+								CassandraYaml: unstructured.Unstructured{"other": "setting"},
+							},
+						},
+					},
+				},
+			},
+			actualDcConfig:      &unstructured.Unstructured{"num_tokens": 256},
+			desiredDcConfig:     &unstructured.Unstructured{"num_tokens": 16},
+			expectedDcNumTokens: 256,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			actualDcConfig := GetDatacenterConfig()
+
+			actualDcConfig.CassandraConfig.CassandraYaml = *tc.actualDcConfig
+			actualDc, err := NewDatacenter(
+				types.NamespacedName{Name: "testdc", Namespace: "test-namespace"},
+				&actualDcConfig,
+			)
+			assert.NoError(t, err)
+			desiredDcConfig := GetDatacenterConfig()
+			desiredDcConfig.CassandraConfig.CassandraYaml = *tc.desiredDcConfig
+			desiredDc, err := NewDatacenter(
+				types.NamespacedName{Name: "testdc", Namespace: "test-namespace"},
+				&desiredDcConfig,
+			)
+			assert.NoError(t, err)
+
+			got, err := SetNewDefaultNumTokens(tc.kc, desiredDc, actualDc)
+			assert.NoError(t, err)
+
+			config, err := utils.UnmarshalToMap(got.Spec.Config)
+			assert.NoError(t, err)
+			cassYaml := config["cassandra-yaml"].(map[string]interface{})
+			assert.Equal(t, tc.expectedDcNumTokens, cassYaml["num_tokens"])
+		})
+	}
+}