Refactor statusConditions updates, Fixes k8ssandra#669

burmanm · Jul 12, 2024 · 2a18a26 · 2a18a26
1 parent 2a9cbd7
commit 2a18a26
Show file tree

Hide file tree

Showing 4 changed files with 79 additions and 179 deletions.
diff --git a/pkg/reconciliation/constructor.go b/pkg/reconciliation/constructor.go
@@ -85,11 +85,16 @@ func setOperatorProgressStatus(rc *ReconciliationContext, newState api.ProgressS
 }
 
 func setDatacenterStatus(rc *ReconciliationContext) error {
-	patch := client.MergeFrom(rc.Datacenter.DeepCopy())
-	rc.Datacenter.Status.ObservedGeneration = rc.Datacenter.Generation
-	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")
+	if rc.Datacenter.Status.ObservedGeneration != rc.Datacenter.Generation {
+		patch := client.MergeFrom(rc.Datacenter.DeepCopy())
+		rc.Datacenter.Status.ObservedGeneration = rc.Datacenter.Generation
+		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
+		}
+	}
+
+	if err := rc.setConditionStatus(api.DatacenterRequiresUpdate, corev1.ConditionFalse); err != nil {
 		return err
 	}
 

diff --git a/pkg/reconciliation/decommission_node.go b/pkg/reconciliation/decommission_node.go
@@ -16,6 +16,7 @@ import (
 	"github.com/k8ssandra/cass-operator/pkg/events"
 	"github.com/k8ssandra/cass-operator/pkg/httphelper"
 	"github.com/k8ssandra/cass-operator/pkg/monitoring"
+	"github.com/pkg/errors"
 	"k8s.io/apimachinery/pkg/types"
 )
 
@@ -82,19 +83,8 @@ func (rc *ReconciliationContext) DecommissionNodes(epData httphelper.CassMetadat
 		if maxReplicas > desiredNodeCount {
 			logger.V(1).Info("reconcile_racks::DecommissionNodes::scaleDownRack", "Rack", rackInfo.RackName, "maxReplicas", maxReplicas, "desiredNodeCount", desiredNodeCount)
 
-			dcPatch := client.MergeFrom(dc.DeepCopy())
-			updated := false
-
-			updated = rc.setCondition(
-				api.NewDatacenterCondition(
-					api.DatacenterScalingDown, corev1.ConditionTrue)) || updated
-
-			if updated {
-				err := rc.Client.Status().Patch(rc.Ctx, dc, dcPatch)
-				if err != nil {
-					logger.Error(err, "error patching datacenter status for scaling down rack started")
-					return result.Error(err)
-				}
+			if err := rc.setConditionStatus(api.DatacenterScalingDown, corev1.ConditionTrue); err != nil {
+				return result.Error(err)
 			}
 
 			rc.ReqLogger.Info(
@@ -219,21 +209,8 @@ func (rc *ReconciliationContext) CheckDecommissioningNodes(epData httphelper.Cas
 		}
 	}
 
-	dcPatch := client.MergeFrom(rc.Datacenter.DeepCopy())
-	updated := false
-
-	updated = rc.setCondition(
-		api.NewDatacenterCondition(
-			api.DatacenterScalingDown, corev1.ConditionFalse)) || updated
-
-	if updated {
-		err := rc.Client.Status().Patch(rc.Ctx, rc.Datacenter, dcPatch)
-		if err != nil {
-			rc.ReqLogger.Error(err, "error patching datacenter status for scaling down finished")
-			return result.Error(err)
-		}
-		// Requeue after updating to ensure we verify previous steps with the new size
-		return result.RequeueSoon(0)
+	if err := rc.setConditionStatus(api.DatacenterScalingDown, corev1.ConditionFalse); err != nil {
+		return result.Error(err)
 	}
 
 	return result.Continue()
@@ -423,20 +400,12 @@ func (rc *ReconciliationContext) EnsurePodsCanAbsorbDecommData(decommPod *corev1
 			)
 			rc.ReqLogger.Error(fmt.Errorf(msg), msg)
 
-			dcPatch := client.MergeFrom(rc.Datacenter.DeepCopy())
-			updated := rc.setCondition(
+			if err := rc.setCondition(
 				api.NewDatacenterConditionWithReason(api.DatacenterValid,
 					corev1.ConditionFalse, "notEnoughSpaceToScaleDown", msg,
 				),
-			)
-
-			if updated {
-				patchErr := rc.Client.Status().Patch(rc.Ctx, rc.Datacenter, dcPatch)
-				if patchErr != nil {
-					msg := "error patching condition Valid for failed scale down."
-					rc.ReqLogger.Error(patchErr, msg)
-					return patchErr
-				}
+			); err != nil {
+				return errors.Wrap(err, msg)
 			}
 
 			return fmt.Errorf(msg)

diff --git a/pkg/reconciliation/reconcile_datacenter.go b/pkg/reconciliation/reconcile_datacenter.go
@@ -62,13 +62,8 @@ func (rc *ReconciliationContext) ProcessDeletion() result.ReconcileResult {
 			}
 
 			if len(dcs) > 1 {
-				dcPatch := client.MergeFrom(rc.Datacenter.DeepCopy())
-				if updated := rc.setCondition(api.NewDatacenterCondition(api.DatacenterDecommission, corev1.ConditionTrue)); updated {
-					err := rc.Client.Status().Patch(rc.Ctx, rc.Datacenter, dcPatch)
-					if err != nil {
-						rc.ReqLogger.Error(err, "error patching datacenter status for decommissiong started")
-						return result.Error(err)
-					}
+				if err := rc.setConditionStatus(api.DatacenterDecommission, corev1.ConditionTrue); err != nil {
+					return result.Error(err)
 				}
 
 				rc.ReqLogger.V(1).Info("Decommissioning the datacenter to 0 nodes first before deletion")