update datacenter test

Signed-off-by: Karol Szwaj <[email protected]>

apis/k8ssandra/v1alpha1/k8ssandracluster_types.go

@@ -526,9 +526,9 @@ const (
 	ServerDistributionDse       = ServerDistribution("dse")
 )
 
-func (kc *K8ssandraCluster) DefaultNumTokens(serverVersion *semver.Version) float64 {
+func (kc *K8ssandraCluster) DefaultNumTokens(serverVersion *semver.Version) int64 {
 	if kc.Spec.Cassandra.ServerType == ServerDistributionCassandra && serverVersion.Major() == 3 {
-		return float64(256)
+		return int64(256)
 	}
-	return float64(16)
+	return int64(16)
 }

apis/k8ssandra/v1alpha1/k8ssandracluster_webhook.go

@@ -118,20 +118,17 @@ func (r *K8ssandraCluster) ValidateUpdate(old runtime.Object) error {
 	if oldCassConfig != nil && newCassConfig != nil {
 		oldNumTokens, oldNumTokensExists := oldCassConfig.CassandraYaml["num_tokens"]
 		newNumTokens, newNumTokensExists := newCassConfig.CassandraYaml["num_tokens"]
-		// Check if num_tokens is not present in old configuration
+
 		if !oldNumTokensExists {
-			// Determine the default num_tokens based on Cassandra version
 			cassVersion, err := semver.NewVersion(oldCluster.Spec.Cassandra.ServerVersion)
 			if err != nil {
 				return err
 			}
 			defaultNumTokens := oldCluster.DefaultNumTokens(cassVersion)
-			// If new configuration has num_tokens and it's not equal to the default, return an error
-			if newNumTokensExists && newNumTokens.(float64) != defaultNumTokens {
+			if newNumTokensExists && int64(newNumTokens.(float64)) != defaultNumTokens {
 				return ErrNumTokens
 			}
 		} else {
-			// If num_tokens is present in both old and new configurations, check if they are equal
 			if oldNumTokens != newNumTokens {
 				return ErrNumTokens
 			}

apis/k8ssandra/v1alpha1/k8ssandracluster_webhook_test.go

@@ -176,19 +176,6 @@ func testNumTokensInUpdate(t *testing.T) {
 	// 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)
-	cluster2 := createMinimalClusterObj("numtokens-wrong-test-update", "numtokensupdate-namespace")
-	cluster2.Spec.Cassandra.ServerVersion = "3.11.10"
-	cluster2.Spec.Cassandra.DatacenterOptions.CassandraConfig = &CassandraConfig{}
-	err = k8sClient.Create(ctx, cluster2)
-	require.NoError(err)
-
-	// Now update to 4.1.3
-	cluster.Spec.Cassandra.DatacenterOptions.CassandraConfig.CassandraYaml = unstructured.Unstructured{"num_tokens": 33}
-	cluster.Spec.Cassandra.ServerVersion = "4.1.3"
-
-	// This should be disallowed, since it is changing default num_tokens
-	err = k8sClient.Update(ctx, cluster)
-	require.Error(err)
 }
 
 func testReaperKeyspaceValidation(t *testing.T) {

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 desiredDc, err = cassandra.ValidateConfig(kc, 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
 				}
 

pkg/cassandra/datacenter.go

@@ -498,33 +498,53 @@ func FindVolumeMount(container *corev1.Container, name string) *corev1.VolumeMou
 	return nil
 }
 
-func ValidateConfig(kc *api.K8ssandraCluster, desiredDc, actualDc *cassdcapi.CassandraDatacenter) (*cassdcapi.CassandraDatacenter, error) {
+func ValidateConfig(desiredDc, actualDc *cassdcapi.CassandraDatacenter) error {
 	desiredConfig, err := utils.UnmarshalToMap(desiredDc.Spec.Config)
 	if err != nil {
-		return nil, err
+		return err
 	}
 	actualConfig, err := utils.UnmarshalToMap(actualDc.Spec.Config)
 	if err != nil {
-		return nil, err
+		return err
 	}
 
 	actualCassYaml, foundActualYaml := actualConfig["cassandra-yaml"].(map[string]interface{})
 	desiredCassYaml, foundDesiredYaml := desiredConfig["cassandra-yaml"].(map[string]interface{})
 
-	if cassConfig := kc.Spec.Cassandra.CassandraConfig; cassConfig == nil {
-		if semver.MustParse(actualDc.Spec.ServerVersion).Major() == 3 && semver.MustParse(desiredDc.Spec.ServerVersion).Major() > 3 {
-			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
-		}
+	if (foundActualYaml && foundDesiredYaml) && actualCassYaml["num_tokens"] != desiredCassYaml["num_tokens"] {
+		return fmt.Errorf("tried to change num_tokens in an existing datacenter")
 	}
 
-	if (foundActualYaml && foundDesiredYaml) && actualCassYaml["num_tokens"] != desiredCassYaml["num_tokens"] {
-		return nil, fmt.Errorf("tried to change num_tokens in an existing datacenter")
+	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

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"])
+		})
+	}
+}