Fix tests, add new test. Add webhook to validate the allowed values f…

…or allow-upgrade and add RequiresUpdate Condition to the Datacenter.Status

apis/cassandra/v1beta1/cassandradatacenter_types.go

@@ -65,6 +65,9 @@ const (
 	// cluster has gone through scale up operation.
 	NoAutomatedCleanupAnnotation = "cassandra.datastax.com/no-cleanup"
 
+	// UpdateAllowedAnnotation marks the Datacenter to allow upgrades to StatefulSets even if CassandraDatacenter object was not modified. Allowed values are "once" and "always"
+	UpdateAllowedAnnotation = "cassandra.datastax.com/allow-upgrade"
+
 	CassNodeState = "cassandra.datastax.com/node-state"
 
 	ProgressUpdating ProgressState = "Updating"
@@ -379,6 +382,7 @@ const (
 	DatacenterRollingRestart DatacenterConditionType = "RollingRestart"
 	DatacenterValid          DatacenterConditionType = "Valid"
 	DatacenterDecommission   DatacenterConditionType = "Decommission"
+	DatacenterRequiresUpdate DatacenterConditionType = "RequiresUpdate"
 
 	// DatacenterHealthy indicates if QUORUM can be reached from all deployed nodes.
 	// If this check fails, certain operations such as scaling up will not proceed.

apis/cassandra/v1beta1/cassandradatacenter_webhook.go

@@ -25,6 +25,7 @@ import (
 
 	"github.com/k8ssandra/cass-operator/pkg/images"
 
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	ctrl "sigs.k8s.io/controller-runtime"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
@@ -139,6 +140,12 @@ func ValidateSingleDatacenter(dc CassandraDatacenter) error {
 		return err
 	}
 
+	if metav1.HasAnnotation(dc.ObjectMeta, UpdateAllowedAnnotation) {
+		if dc.Annotations[UpdateAllowedAnnotation] != "once" && dc.Annotations[UpdateAllowedAnnotation] != "always" {
+			return attemptedTo("use %s annotation with value other than 'once' or 'always'", UpdateAllowedAnnotation)
+		}
+	}
+
 	return ValidateFQLConfig(dc)
 }
 

apis/cassandra/v1beta1/webhook_test.go

@@ -318,6 +318,38 @@ func Test_ValidateSingleDatacenter(t *testing.T) {
 			},
 			errString: "configure DatacenterService with reserved annotations and/or labels (prefixes cassandra.datastax.com and/or k8ssandra.io)",
 		},
+		{
+			name: "Allow upgrade should not accept invalid values",
+			dc: &CassandraDatacenter{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "exampleDC",
+					Annotations: map[string]string{
+						"cassandra.datastax.com/allow-upgrade": "invalid",
+					},
+				},
+				Spec: CassandraDatacenterSpec{
+					ServerType:    "dse",
+					ServerVersion: "6.8.42",
+				},
+			},
+			errString: "use cassandra.datastax.com/allow-upgrade annotation with value other than 'once' or 'always'",
+		},
+		{
+			name: "Allow upgrade should accept once value",
+			dc: &CassandraDatacenter{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "exampleDC",
+					Annotations: map[string]string{
+						"cassandra.datastax.com/allow-upgrade": "once",
+					},
+				},
+				Spec: CassandraDatacenterSpec{
+					ServerType:    "dse",
+					ServerVersion: "6.8.42",
+				},
+			},
+			errString: "",
+		},
 	}
 
 	for _, tt := range tests {

pkg/reconciliation/constructor.go

@@ -10,6 +10,7 @@ import (
 	"github.com/k8ssandra/cass-operator/pkg/oplabels"
 	"github.com/k8ssandra/cass-operator/pkg/utils"
 
+	corev1 "k8s.io/api/core/v1"
 	policyv1 "k8s.io/api/policy/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/intstr"
@@ -62,17 +63,18 @@ func setOperatorProgressStatus(rc *ReconciliationContext, newState api.ProgressS
 		if rc.Datacenter.Status.DatacenterName == nil {
 			rc.Datacenter.Status.DatacenterName = &rc.Datacenter.Spec.DatacenterName
 		}
+		rc.setCondition(api.NewDatacenterCondition(api.DatacenterRequiresUpdate, corev1.ConditionFalse))
 	}
 	if err := rc.Client.Status().Patch(rc.Ctx, rc.Datacenter, patch); err != nil {
 		rc.ReqLogger.Error(err, "error updating the Cassandra Operator Progress state")
 		return err
 	}
 
 	// The allow-upgrade=once annotation is temporary and should be removed after first successful reconcile
-	if metav1.HasAnnotation(rc.Datacenter.ObjectMeta, "cassandra.datastax.com/allow-upgrade") && rc.Datacenter.Annotations["cassandra.datastax.com/allow-upgrade"] == "once" {
+	if metav1.HasAnnotation(rc.Datacenter.ObjectMeta, api.UpdateAllowedAnnotation) && rc.Datacenter.Annotations[api.UpdateAllowedAnnotation] == "once" {
 		// remove the annotation
 		patch = client.MergeFrom(rc.Datacenter.DeepCopy())
-		delete(rc.Datacenter.ObjectMeta.Annotations, "cassandra.datastax.com/allow-upgrade")
+		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

pkg/reconciliation/reconcile_racks.go

@@ -166,8 +166,9 @@ func (rc *ReconciliationContext) CheckRackCreation() result.ReconcileResult {
 	return result.Continue()
 }
 
-func (rc *ReconciliationContext) AllowdUpdate() bool {
-	return rc.Datacenter.GenerationChanged() || metav1.HasAnnotation(rc.Datacenter.ObjectMeta, "cassandra.datastax.com/allow-upgrade")
+func (rc *ReconciliationContext) UpdateAllowed() bool {
+	// HasAnnotation might require also checking if it's "once / always".. or then we need to validate those allowed values in the webhook
+	return rc.Datacenter.GenerationChanged() || metav1.HasAnnotation(rc.Datacenter.ObjectMeta, api.UpdateAllowedAnnotation)
 }
 
 func (rc *ReconciliationContext) CheckRackPodTemplate() result.ReconcileResult {
@@ -203,15 +204,20 @@ func (rc *ReconciliationContext) CheckRackPodTemplate() result.ReconcileResult {
 			return result.Error(err)
 		}
 
-		if !utils.ResourcesHaveSameHash(statefulSet, desiredSts) {
+		if !utils.ResourcesHaveSameHash(statefulSet, desiredSts) && !rc.UpdateAllowed() {
 			logger.
-				WithValues("rackName", rackName, "allowUpdate", rc.AllowdUpdate()).
-				Info("statefulset needs an update")
-
-			// TODO If AllowedUpdate is false, but we notice there's a difference, add a new Condition to the Status
+				WithValues("rackName", rackName).
+				Info("update is blocked, but statefulset needs an update. Marking datacenter as requiring update.")
+			dcPatch := client.MergeFrom(dc.DeepCopy())
+			rc.setCondition(api.NewDatacenterCondition(api.DatacenterRequiresUpdate, corev1.ConditionTrue))
+			if err := rc.Client.Status().Patch(rc.Ctx, dc, dcPatch); err != nil {
+				logger.Error(err, "error patching datacenter status for updating")
+				return result.Error(err)
+			}
+			return result.Continue()
 		}
 
-		if !utils.ResourcesHaveSameHash(statefulSet, desiredSts) && rc.AllowdUpdate() {
+		if !utils.ResourcesHaveSameHash(statefulSet, desiredSts) && rc.UpdateAllowed() {
 			logger.
 				WithValues("rackName", rackName).
 				Info("statefulset needs an update")

pkg/reconciliation/reconcile_racks_test.go

@@ -24,6 +24,7 @@ import (
 	"github.com/k8ssandra/cass-operator/pkg/utils"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -349,6 +350,34 @@ func TestCheckRackPodTemplate_CanaryUpgrade(t *testing.T) {
 	assert.True(t, result.Completed())
 }
 
+func TestCheckRackPodTemplate_GenerationCheck(t *testing.T) {
+	assert := assert.New(t)
+	rc, _, cleanpMockSrc := setupTest()
+	defer cleanpMockSrc()
+
+	require.NoError(t, rc.CalculateRackInformation())
+
+	res := rc.CheckRackCreation()
+	assert.False(res.Completed(), "CheckRackCreation did not complete as expected")
+
+	// Update the generation manually and now verify we won't do updates to StatefulSets if the generation hasn't changed
+	rc.Datacenter.Status.ObservedGeneration = rc.Datacenter.Generation
+	rc.Datacenter.Spec.ServerVersion = "6.8.44"
+
+	res = rc.CheckRackPodTemplate()
+	assert.Equal(result.Continue(), res)
+	cond, found := rc.Datacenter.GetCondition(api.DatacenterRequiresUpdate)
+	assert.True(found)
+	assert.Equal(corev1.ConditionTrue, cond.Status)
+
+	// Add annotation
+	metav1.SetMetaDataAnnotation(&rc.Datacenter.ObjectMeta, api.UpdateAllowedAnnotation, "always")
+	rc.Datacenter.Spec.ServerVersion = "6.8.44" // This needs to be reapplied, since we call Patch in the CheckRackPodTemplate()
+
+	res = rc.CheckRackPodTemplate()
+	assert.True(res.Completed())
+}
+
 func TestReconcilePods(t *testing.T) {
 	t.Skip()
 	rc, _, cleanupMockScr := setupTest()

pkg/reconciliation/testing.go

@@ -77,8 +77,9 @@ func CreateMockReconciliationContext(
 	// Instance a cassandraDatacenter
 	cassandraDatacenter := &api.CassandraDatacenter{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      name,
-			Namespace: namespace,
+			Name:       name,
+			Namespace:  namespace,
+			Generation: 1,
 		},
 		Spec: api.CassandraDatacenterSpec{
 			Size:          size,