Change default labels to use Kubernetes resource name (#698)

* Change default labels to use Kubernetes resource datacenter name and not datacenter override name. Create new Status field for MetadataVersion and if we keep running old ones (or old CRD), we simply fetch also with the older possible name

* Try to fix the flaky envtest

* Some test changes

* Update some tests to get Github to run correct tests

* Test for datacenterPods() functionality

* Move autoupdate-spec annotation removal

* Fix DatacenterName description

* Regenerate CRD

apis/cassandra/v1beta1/cassandradatacenter_types.go

@@ -261,8 +261,7 @@ type CassandraDatacenterSpec struct {
 	// CDC allows configuration of the change data capture agent which can run within the Management API container. Use it to send data to Pulsar.
 	CDC *CDCConfiguration `json:"cdc,omitempty"`
 
-	// DatacenterName allows to override the name of the Cassandra datacenter. Kubernetes objects will be named after a sanitized version of it if set, and if not metadata.name. In Cassandra the DC name will be overridden by this value.
-	// It may generate some confusion as objects created for the DC will have a different name than the CasandraDatacenter object itself.
+	// DatacenterName allows to override the name of the Cassandra datacenter. In Cassandra the DC name will be overridden by this value.
 	// This setting can create conflicts if multiple DCs coexist in the same namespace if metadata.name for a DC with no override is set to the same value as the override name of another DC.
 	// Use cautiously.
 	// +optional
@@ -488,6 +487,9 @@ type CassandraDatacenterStatus struct {
 	// This field is used to perform validation checks preventing a user from changing the override
 	// +optional
 	DatacenterName *string `json:"datacenterName,omitempty"`
+
+	// +optional
+	MetadataVersion int64 `json:"metadataVersion,omitempty"`
 }
 
 // CassandraDatacenter is the Schema for the cassandradatacenters API
@@ -599,7 +601,7 @@ func (dc *CassandraDatacenter) SetCondition(condition DatacenterCondition) {
 // GetDatacenterLabels ...
 func (dc *CassandraDatacenter) GetDatacenterLabels() map[string]string {
 	labels := dc.GetClusterLabels()
-	labels[DatacenterLabel] = CleanLabelValue(dc.DatacenterName())
+	labels[DatacenterLabel] = CleanLabelValue(dc.Name)
 	return labels
 }
 
@@ -664,19 +666,19 @@ func (dc *CassandraDatacenter) GetSeedServiceName() string {
 }
 
 func (dc *CassandraDatacenter) GetAdditionalSeedsServiceName() string {
-	return CleanupForKubernetes(dc.Spec.ClusterName) + "-" + dc.SanitizedName() + "-additional-seed-service"
+	return CleanupForKubernetes(dc.Spec.ClusterName) + "-" + dc.LabelResourceName() + "-additional-seed-service"
 }
 
 func (dc *CassandraDatacenter) GetAllPodsServiceName() string {
-	return CleanupForKubernetes(dc.Spec.ClusterName) + "-" + dc.SanitizedName() + "-all-pods-service"
+	return CleanupForKubernetes(dc.Spec.ClusterName) + "-" + dc.LabelResourceName() + "-all-pods-service"
 }
 
 func (dc *CassandraDatacenter) GetDatacenterServiceName() string {
-	return CleanupForKubernetes(dc.Spec.ClusterName) + "-" + dc.SanitizedName() + "-service"
+	return CleanupForKubernetes(dc.Spec.ClusterName) + "-" + dc.LabelResourceName() + "-service"
 }
 
 func (dc *CassandraDatacenter) GetNodePortServiceName() string {
-	return CleanupForKubernetes(dc.Spec.ClusterName) + "-" + dc.SanitizedName() + "-node-port-service"
+	return CleanupForKubernetes(dc.Spec.ClusterName) + "-" + dc.LabelResourceName() + "-node-port-service"
 }
 
 func (dc *CassandraDatacenter) ShouldGenerateSuperuserSecret() bool {
@@ -973,9 +975,17 @@ func SplitRacks(nodeCount, rackCount int) []int {
 	return topology
 }
 
-// SanitizedName returns a sanitized version of the name returned by DatacenterName()
-func (dc *CassandraDatacenter) SanitizedName() string {
-	return CleanupForKubernetes(dc.DatacenterName())
+func (dc *CassandraDatacenter) DatacenterNameStatus() bool {
+	return dc.Status.DatacenterName != nil
+}
+
+// LabelResourceName returns a sanitized version of the name returned by DatacenterName()
+func (dc *CassandraDatacenter) LabelResourceName() string {
+	// If existing cluster, return dc.DatacenterName() else return dc.Name
+	if dc.DatacenterNameStatus() {
+		return CleanupForKubernetes(*dc.Status.DatacenterName)
+	}
+	return CleanupForKubernetes(dc.Name)
 }
 
 // DatacenterName returns the Cassandra DC name override if it exists,

apis/cassandra/v1beta1/cassandradatacenter_types_test.go

@@ -4,6 +4,7 @@ import (
 	"encoding/json"
 	"testing"
 
+	"github.com/Jeffail/gabs/v2"
 	"github.com/stretchr/testify/assert"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/utils/ptr"
@@ -190,3 +191,50 @@ func TestUseClientImageEnforce(t *testing.T) {
 		assert.True(dc.UseClientImage())
 	}
 }
+
+func TestDatacenterNoOverrideConfig(t *testing.T) {
+	assert := assert.New(t)
+	dc := CassandraDatacenter{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "dc1",
+		},
+		Spec: CassandraDatacenterSpec{
+			ClusterName: "cluster1",
+		},
+	}
+
+	config, err := dc.GetConfigAsJSON(dc.Spec.Config)
+	assert.NoError(err)
+
+	container, err := gabs.ParseJSON([]byte(config))
+	assert.NoError(err)
+
+	dataCenterInfo := container.ChildrenMap()["datacenter-info"]
+	assert.NotEmpty(dataCenterInfo)
+	assert.Equal(dc.Name, dataCenterInfo.ChildrenMap()["name"].Data().(string))
+	assert.Equal(dc.DatacenterName(), dc.Name)
+}
+
+func TestDatacenterOverrideInConfig(t *testing.T) {
+	assert := assert.New(t)
+	dc := CassandraDatacenter{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "dc1",
+		},
+		Spec: CassandraDatacenterSpec{
+			ClusterName:    "cluster1",
+			DatacenterName: "Home_Dc",
+		},
+	}
+
+	config, err := dc.GetConfigAsJSON(dc.Spec.Config)
+	assert.NoError(err)
+
+	container, err := gabs.ParseJSON([]byte(config))
+	assert.NoError(err)
+
+	dataCenterInfo := container.ChildrenMap()["datacenter-info"]
+	assert.NotEmpty(dataCenterInfo)
+	assert.Equal(dc.Spec.DatacenterName, dataCenterInfo.ChildrenMap()["name"].Data().(string))
+	assert.Equal(dc.DatacenterName(), dc.Spec.DatacenterName)
+}

config/crd/bases/cassandra.datastax.com_cassandradatacenters.yaml

@@ -300,8 +300,7 @@ spec:
                 type: string
               datacenterName:
                 description: |-
-                  DatacenterName allows to override the name of the Cassandra datacenter. Kubernetes objects will be named after a sanitized version of it if set, and if not metadata.name. In Cassandra the DC name will be overridden by this value.
-                  It may generate some confusion as objects created for the DC will have a different name than the CasandraDatacenter object itself.
+                  DatacenterName allows to override the name of the Cassandra datacenter. In Cassandra the DC name will be overridden by this value.
                   This setting can create conflicts if multiple DCs coexist in the same namespace if metadata.name for a DC with no override is set to the same value as the override name of another DC.
                   Use cautiously.
                 type: string
@@ -11221,6 +11220,9 @@ spec:
                   with the management API
                 format: date-time
                 type: string
+              metadataVersion:
+                format: int64
+                type: integer
               nodeReplacements:
                 items:
                   type: string

config/manager/image_config.yaml

@@ -18,7 +18,7 @@ defaults:
   # Note, postfix is ignored if repository is not set
   cassandra:
     repository: "k8ssandra/cass-management-api"
-    suffix: "-ubi8"
+    suffix: "-ubi"
   dse:
     repository: "datastax/dse-mgmtapi-6_8"
     suffix: "-ubi8"

internal/controllers/cassandra/cassandradatacenter_controller_test.go

@@ -155,8 +155,9 @@ var _ = Describe("CassandraDatacenter tests", func() {
 				refreshDatacenter(ctx, &dc)
 
 				By("Updating the size to 3")
+				patch := client.MergeFrom(dc.DeepCopy())
 				dc.Spec.Size = 3
-				Expect(k8sClient.Update(ctx, &dc)).To(Succeed())
+				Expect(k8sClient.Patch(ctx, &dc, patch)).To(Succeed())
 
 				waitForDatacenterCondition(ctx, dcName, cassdcapi.DatacenterScalingUp, corev1.ConditionTrue).Should(Succeed())
 				waitForDatacenterProgress(ctx, dcName, cassdcapi.ProgressUpdating).Should(Succeed())

internal/controllers/control/cassandratask_controller.go

@@ -202,7 +202,7 @@ func (r *CassandraTaskReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 		return ctrl.Result{}, errors.Wrapf(err, "unable to fetch target CassandraDatacenter: %s", cassTask.Spec.Datacenter)
 	}
 
-	logger = log.FromContext(ctx, "datacenterName", dc.SanitizedName(), "clusterName", dc.Spec.ClusterName)
+	logger = log.FromContext(ctx, "datacenterName", dc.LabelResourceName(), "clusterName", dc.Spec.ClusterName)
 	log.IntoContext(ctx, logger)
 
 	// If we're active, we can proceed - otherwise verify if we're allowed to run

pkg/images/images_test.go

@@ -94,7 +94,7 @@ func TestDefaultImageConfigParsing(t *testing.T) {
 
 	path, err = GetCassandraImage("cassandra", "4.1.4")
 	assert.NoError(err)
-	assert.Equal("k8ssandra/cass-management-api:4.1.4-ubi8", path)
+	assert.Equal("k8ssandra/cass-management-api:4.1.4-ubi", path)
 }
 
 func TestImageConfigParsing(t *testing.T) {

pkg/reconciliation/construct_podtemplatespec.go

@@ -379,7 +379,7 @@ func addVolumes(dc *api.CassandraDatacenter, baseTemplate *corev1.PodTemplateSpe
 			Name: "encryption-cred-storage",
 			VolumeSource: corev1.VolumeSource{
 				Secret: &corev1.SecretVolumeSource{
-					SecretName: fmt.Sprintf("%s-keystore", dc.SanitizedName()),
+					SecretName: fmt.Sprintf("%s-keystore", dc.LabelResourceName()),
 				},
 			},
 		}
@@ -614,7 +614,7 @@ func getConfigDataEnVars(dc *api.CassandraDatacenter) ([]corev1.EnvVar, error) {
 			return envVars, nil
 		}
 
-		return nil, fmt.Errorf("datacenter %s is missing %s annotation", dc.SanitizedName(), api.ConfigHashAnnotation)
+		return nil, fmt.Errorf("datacenter %s is missing %s annotation", dc.LabelResourceName(), api.ConfigHashAnnotation)
 	}
 
 	configData, err := dc.GetConfigAsJSON(dc.Spec.Config)

pkg/reconciliation/construct_podtemplatespec_test.go

@@ -1548,7 +1548,7 @@ func Test_makeImage(t *testing.T) {
 				serverType:    "cassandra",
 				serverVersion: "3.11.10",
 			},
-			want:      "localhost:5000/k8ssandra/cass-management-api:3.11.10-ubi8",
+			want:      "localhost:5000/k8ssandra/cass-management-api:3.11.10-ubi",
 			errString: "",
 		},
 		{

pkg/reconciliation/construct_statefulset.go

@@ -26,7 +26,7 @@ func newNamespacedNameForStatefulSet(
 	dc *api.CassandraDatacenter,
 	rackName string) types.NamespacedName {
 
-	name := api.CleanupForKubernetes(dc.Spec.ClusterName) + "-" + dc.SanitizedName() + "-" + api.CleanupSubdomain(rackName) + "-sts"
+	name := api.CleanupForKubernetes(dc.Spec.ClusterName) + "-" + dc.LabelResourceName() + "-" + api.CleanupSubdomain(rackName) + "-sts"
 	ns := dc.Namespace
 
 	return types.NamespacedName{

pkg/reconciliation/construct_statefulset_test.go

@@ -641,7 +641,7 @@ func Test_newStatefulSetForCassandraDatacenter_dcNameOverride(t *testing.T) {
 		oplabels.NameLabel:      oplabels.NameLabelValue,
 		oplabels.CreatedByLabel: oplabels.CreatedByLabelValue,
 		oplabels.VersionLabel:   "4.0.1",
-		api.DatacenterLabel:     "MySuperDC",
+		api.DatacenterLabel:     "dc1",
 		api.ClusterLabel:        "piclem",
 		api.RackLabel:           dc.Spec.Racks[0].Name,
 	}
@@ -652,7 +652,7 @@ func Test_newStatefulSetForCassandraDatacenter_dcNameOverride(t *testing.T) {
 		oplabels.NameLabel:      oplabels.NameLabelValue,
 		oplabels.CreatedByLabel: oplabels.CreatedByLabelValue,
 		oplabels.VersionLabel:   "4.0.1",
-		api.DatacenterLabel:     "MySuperDC",
+		api.DatacenterLabel:     "dc1",
 		api.ClusterLabel:        "piclem",
 		api.RackLabel:           dc.Spec.Racks[0].Name,
 		api.CassNodeState:       stateReadyToStart,

pkg/reconciliation/constructor.go

@@ -31,7 +31,7 @@ func newPodDisruptionBudgetForDatacenter(dc *api.CassandraDatacenter) *policyv1.
 
 	pdb := &policyv1.PodDisruptionBudget{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:        dc.SanitizedName() + "-pdb",
+			Name:        dc.LabelResourceName() + "-pdb",
 			Namespace:   dc.Namespace,
 			Labels:      labels,
 			Annotations: anns,
@@ -62,8 +62,11 @@ func setOperatorProgressStatus(rc *ReconciliationContext, newState api.ProgressS
 	rc.Datacenter.Status.CassandraOperatorProgress = newState
 
 	if newState == api.ProgressReady {
+		if rc.Datacenter.Status.MetadataVersion < 1 {
+			rc.Datacenter.Status.MetadataVersion = 1
+		}
 		if rc.Datacenter.Status.DatacenterName == nil {
-			rc.Datacenter.Status.DatacenterName = &rc.Datacenter.Spec.DatacenterName
+			rc.Datacenter.Status.DatacenterName = &rc.Datacenter.Name
 		}
 	}
 	if err := rc.Client.Status().Patch(rc.Ctx, rc.Datacenter, patch); err != nil {
@@ -73,17 +76,6 @@ func setOperatorProgressStatus(rc *ReconciliationContext, newState api.ProgressS
 
 	monitoring.UpdateOperatorDatacenterProgressStatusMetric(rc.Datacenter, newState)
 
-	// The allow-upgrade=once annotation is temporary and should be removed after first successful reconcile
-	if metav1.HasAnnotation(rc.Datacenter.ObjectMeta, api.UpdateAllowedAnnotation) && rc.Datacenter.Annotations[api.UpdateAllowedAnnotation] == string(api.AllowUpdateOnce) {
-		// remove the annotation
-		patch = client.MergeFrom(rc.Datacenter.DeepCopy())
-		delete(rc.Datacenter.ObjectMeta.Annotations, api.UpdateAllowedAnnotation)
-		if err := rc.Client.Patch(rc.Ctx, rc.Datacenter, patch); err != nil {
-			rc.ReqLogger.Error(err, "error removing the allow-upgrade=once annotation")
-			return err
-		}
-	}
-
 	return nil
 }
 
@@ -101,5 +93,16 @@ func setDatacenterStatus(rc *ReconciliationContext) error {
 		return err
 	}
 
+	// The allow-upgrade=once annotation is temporary and should be removed after first successful reconcile
+	if metav1.HasAnnotation(rc.Datacenter.ObjectMeta, api.UpdateAllowedAnnotation) && rc.Datacenter.Annotations[api.UpdateAllowedAnnotation] == string(api.AllowUpdateOnce) {
+		// remove the annotation
+		patch := client.MergeFrom(rc.Datacenter.DeepCopy())
+		delete(rc.Datacenter.ObjectMeta.Annotations, api.UpdateAllowedAnnotation)
+		if err := rc.Client.Patch(rc.Ctx, rc.Datacenter, patch); err != nil {
+			rc.ReqLogger.Error(err, "error removing the allow-upgrade=once annotation")
+			return err
+		}
+	}
+
 	return nil
 }

pkg/reconciliation/context.go

@@ -92,7 +92,7 @@ func CreateReconciliationContext(
 	}
 
 	rc.ReqLogger = rc.ReqLogger.
-		WithValues("datacenterName", dc.SanitizedName()).
+		WithValues("datacenterName", dc.LabelResourceName()).
 		WithValues("clusterName", dc.Spec.ClusterName)
 
 	log.IntoContext(ctx, rc.ReqLogger)
@@ -146,8 +146,8 @@ func (rc *ReconciliationContext) validateDatacenterNameConflicts() []error {
 		errs = append(errs, fmt.Errorf("failed to list CassandraDatacenters in namespace %s: %w", dc.Namespace, err))
 	} else {
 		for _, existingDc := range cassandraDatacenters.Items {
-			if existingDc.SanitizedName() == dc.SanitizedName() && existingDc.Name != dc.Name {
-				errs = append(errs, fmt.Errorf("datacenter name/override %s/%s is already in use by CassandraDatacenter %s/%s", dc.Name, dc.SanitizedName(), existingDc.Name, existingDc.SanitizedName()))
+			if existingDc.LabelResourceName() == dc.LabelResourceName() && existingDc.Name != dc.Name {
+				errs = append(errs, fmt.Errorf("datacenter name/override %s/%s is already in use by CassandraDatacenter %s/%s", dc.Name, dc.LabelResourceName(), existingDc.Name, existingDc.LabelResourceName()))
 			}
 		}
 	}
@@ -164,7 +164,7 @@ func (rc *ReconciliationContext) validateDatacenterNameOverride() []error {
 		return errs
 	} else {
 		if *dc.Status.DatacenterName != dc.Spec.DatacenterName {
-			errs = append(errs, fmt.Errorf("datacenter %s name override '%s' cannot be changed after creation to '%s'.", dc.Name, dc.Spec.DatacenterName, *dc.Status.DatacenterName))
+			errs = append(errs, fmt.Errorf("datacenter %s name override '%s' cannot be changed after creation to '%s'", dc.Name, dc.Spec.DatacenterName, *dc.Status.DatacenterName))
 		}
 	}
 

pkg/reconciliation/handler_reconcile_test.go

@@ -19,14 +19,15 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/client-go/tools/record"
+	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 )
 
 func TestReconcile(t *testing.T) {
 	var (
-		name            = "cluster-example-cluster"
+		name            = "dc1-example"
 		namespace       = "default"
 		size      int32 = 2
 	)
@@ -74,6 +75,7 @@ func TestReconcile(t *testing.T) {
 		Client:   fakeClient,
 		Scheme:   s,
 		Recorder: record.NewFakeRecorder(100),
+		Log:      ctrl.Log.WithName("controllers").WithName("CassandraDatacenter"),
 	}
 
 	request := reconcile.Request{
@@ -88,8 +90,8 @@ func TestReconcile(t *testing.T) {
 		t.Fatalf("Reconciliation Failure: (%v)", err)
 	}
 
-	if result != (reconcile.Result{Requeue: true, RequeueAfter: 2 * time.Second}) {
-		t.Error("Reconcile did not return a correct result.")
+	if result != (reconcile.Result{Requeue: true, RequeueAfter: 10 * time.Second}) {
+		t.Errorf("Reconcile did not return a correct result. (%v)", result)
 	}
 }
 

pkg/reconciliation/handler_test.go

@@ -220,7 +220,11 @@ func TestConflictingDcNameOverride(t *testing.T) {
 				Spec: api.CassandraDatacenterSpec{
 					ClusterName:    "cluster1",
 					DatacenterName: "CassandraDatacenter_example",
-				}}}
+				},
+				Status: api.CassandraDatacenterStatus{
+					DatacenterName: ptr.To[string]("CassandraDatacenter_example"),
+				},
+			}}
 		})
 
 	errs := rc.validateDatacenterNameConflicts()

pkg/reconciliation/reconcile_configsecret.go

@@ -131,7 +131,7 @@ func getConfigFromConfigSecret(dc *api.CassandraDatacenter, secret *corev1.Secre
 
 // getDatacenterConfigSecretName The format is clusterName-dcName-config
 func getDatacenterConfigSecretName(dc *api.CassandraDatacenter) string {
-	return api.CleanupForKubernetes(dc.Spec.ClusterName) + "-" + dc.SanitizedName() + "-config"
+	return api.CleanupForKubernetes(dc.Spec.ClusterName) + "-" + dc.LabelResourceName() + "-config"
 }
 
 // getDatacenterConfigSecret Fetches the secret from the api server or creates a new secret