Merge pull request #1294 from theobarberbany/fixup-condition-reasons

OCPBUGS-42529: Updates paused condition messages

Author: openshift-merge-bot[bot]

pkg/controller/machine/controller.go

@@ -187,7 +187,7 @@ func (r *ReconcileMachine) Reconcile(ctx context.Context, request reconcile.Requ
 			conditions.Set(m, conditions.TrueConditionWithReason(
 				PausedCondition,
 				PausedConditionReason,
-				"The AuthoritativeAPI is set to %s", m.Status.AuthoritativeAPI,
+				"The AuthoritativeAPI is set to %s", string(m.Status.AuthoritativeAPI),
 			))
 			if patchErr := r.updateStatus(ctx, m, ptr.Deref(m.Status.Phase, ""), nil, originalConditions); patchErr != nil {
 				klog.Errorf("%v: error patching status: %v", machineName, patchErr)
@@ -197,12 +197,19 @@ func (r *ReconcileMachine) Reconcile(ctx context.Context, request reconcile.Requ
 			return reconcile.Result{}, nil
 		}
 
+		var pausedFalseReason string
+		if m.Status.AuthoritativeAPI != "" {
+			pausedFalseReason = fmt.Sprintf("The AuthoritativeAPI is set to %s", string(m.Status.AuthoritativeAPI))
+		} else {
+			pausedFalseReason = "The AuthoritativeAPI is not set"
+		}
+
 		// Set the paused condition to false, continue reconciliation
 		conditions.Set(m, conditions.FalseCondition(
 			PausedCondition,
 			NotPausedConditionReason,
 			machinev1.ConditionSeverityInfo,
-			"The AuthoritativeAPI is set to %s", m.Status.AuthoritativeAPI,
+			pausedFalseReason,
 		))
 		if patchErr := r.updateStatus(ctx, m, ptr.Deref(m.Status.Phase, ""), nil, originalConditions); patchErr != nil {
 			klog.Errorf("%v: error patching status: %v", machineName, patchErr)

pkg/controller/machineset/controller.go

@@ -27,6 +27,7 @@ import (
 
 	openshiftfeatures "github.com/openshift/api/features"
 	machinev1 "github.com/openshift/api/machine/v1beta1"
+	"github.com/openshift/machine-api-operator/pkg/controller/machine"
 	"github.com/openshift/machine-api-operator/pkg/util"
 	"github.com/openshift/machine-api-operator/pkg/util/conditions"
 	corev1 "k8s.io/api/core/v1"
@@ -47,14 +48,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/source"
 )
 
-const (
-	PausedCondition machinev1.ConditionType = "Paused"
-
-	PausedConditionReason = "AuthoritativeAPI is not set to MachineAPI"
-
-	NotPausedConditionReason = "AuthoritativeAPI is set to MachineAPI"
-)
-
 var (
 	controllerKind = machinev1.SchemeGroupVersion.WithKind("MachineSet")
 
@@ -190,9 +183,9 @@ func (r *ReconcileMachineSet) Reconcile(ctx context.Context, request reconcile.R
 		if machineSet.Status.AuthoritativeAPI != "" &&
 			machineSet.Status.AuthoritativeAPI != machinev1.MachineAuthorityMachineAPI {
 			conditions.Set(machineSetCopy, conditions.TrueConditionWithReason(
-				PausedCondition,
-				PausedConditionReason,
-				"The AuthoritativeAPI is set to %s", machineSet.Status.AuthoritativeAPI,
+				machine.PausedCondition,
+				machine.PausedConditionReason,
+				"The AuthoritativeAPI is set to %s", string(machineSet.Status.AuthoritativeAPI),
 			))
 
 			_, err := updateMachineSetStatus(r.Client, machineSet, machineSetCopy.Status)
@@ -204,10 +197,19 @@ func (r *ReconcileMachineSet) Reconcile(ctx context.Context, request reconcile.R
 			return reconcile.Result{}, nil
 		}
 
+		var pausedFalseReason string
+		if machineSet.Status.AuthoritativeAPI != "" {
+			pausedFalseReason = fmt.Sprintf("The AuthoritativeAPI is set to %s", string(machineSet.Status.AuthoritativeAPI))
+		} else {
+			pausedFalseReason = "The AuthoritativeAPI is not set"
+		}
+
+		// Set the paused condition to false, continue reconciliation
 		conditions.Set(machineSetCopy, conditions.FalseCondition(
-			PausedCondition,
-			NotPausedConditionReason,
-			"The AuthoritativeAPI is set to %s", string(machineSet.Status.AuthoritativeAPI),
+			machine.PausedCondition,
+			machine.NotPausedConditionReason,
+			machinev1.ConditionSeverityInfo,
+			pausedFalseReason,
 		))
 		_, err := updateMachineSetStatus(r.Client, machineSet, machineSetCopy.Status)
 		if err != nil {

pkg/controller/machineset/machineset_controller_test.go

@@ -25,6 +25,7 @@ import (
 
 	machinev1 "github.com/openshift/api/machine/v1beta1"
 	machinev1resourcebuilder "github.com/openshift/cluster-api-actuator-pkg/testutils/resourcebuilder/machine/v1beta1"
+	"github.com/openshift/machine-api-operator/pkg/controller/machine"
 
 	"github.com/openshift/cluster-control-plane-machine-set-operator/test/e2e/framework"
 	testutils "github.com/openshift/machine-api-operator/pkg/util/testing"
@@ -167,7 +168,7 @@ var _ = Describe("MachineSet Reconciler", func() {
 
 		By("Verifying the paused condition is approproately set to true")
 		Eventually(k.Object(instance), timeout).Should(HaveField("Status.Conditions", ContainElement(SatisfyAll(
-			HaveField("Type", Equal(PausedCondition)),
+			HaveField("Type", Equal(machine.PausedCondition)),
 			HaveField("Status", Equal(corev1.ConditionTrue)),
 		))))
 
@@ -190,7 +191,7 @@ var _ = Describe("MachineSet Reconciler", func() {
 					}
 
 					return g.Expect(localInstance.Status.Conditions).Should(ContainElement(SatisfyAll(
-						HaveField("Type", Equal(PausedCondition)),
+						HaveField("Type", Equal(machine.PausedCondition)),
 						HaveField("Status", Equal(corev1.ConditionTrue)),
 					)))
 
@@ -222,7 +223,7 @@ var _ = Describe("MachineSet Reconciler", func() {
 
 		By("Verifying the paused condition is approproately set to false")
 		Eventually(k.Object(instance), timeout).Should(HaveField("Status.Conditions", ContainElement(SatisfyAll(
-			HaveField("Type", Equal(PausedCondition)),
+			HaveField("Type", Equal(machine.PausedCondition)),
 			HaveField("Status", Equal(corev1.ConditionFalse)),
 		))))
 
@@ -233,7 +234,7 @@ var _ = Describe("MachineSet Reconciler", func() {
 
 		By("Verifying the paused condition is still approproately set to false")
 		Eventually(k.Object(instance), timeout).Should(HaveField("Status.Conditions", ContainElement(SatisfyAll(
-			HaveField("Type", Equal(PausedCondition)),
+			HaveField("Type", Equal(machine.PausedCondition)),
 			HaveField("Status", Equal(corev1.ConditionFalse)),
 		))))
 	})