From bc0c4d50b4653b3300d48e8c099bd260122699ae Mon Sep 17 00:00:00 2001 From: Daniel Franz Date: Thu, 28 Aug 2025 16:08:23 +0900 Subject: [PATCH] Const Cleanup Captures conditions and reasons used by ClusterExtensionRevision into consts. Signed-off-by: Daniel Franz --- api/v1/clusterextension_types_test.go | 2 + api/v1/clusterextensionrevision_types.go | 19 +++++- .../operator-controller/applier/boxcutter.go | 7 +-- .../clusterextension_controller.go | 3 +- .../clusterextensionrevision_controller.go | 58 +++++++++---------- ...lusterextensionrevision_controller_test.go | 26 ++++----- 6 files changed, 66 insertions(+), 49 deletions(-) diff --git a/api/v1/clusterextension_types_test.go b/api/v1/clusterextension_types_test.go index 7bc9a33934..feb9b4e224 100644 --- a/api/v1/clusterextension_types_test.go +++ b/api/v1/clusterextension_types_test.go @@ -14,6 +14,8 @@ import ( "github.com/operator-framework/operator-controller/internal/operator-controller/conditionsets" ) +// TODO Expand these tests to cover Types/Reasons/etc. from other APIs as well + func TestClusterExtensionTypeRegistration(t *testing.T) { types, err := parseConstants("Type") if err != nil { diff --git a/api/v1/clusterextensionrevision_types.go b/api/v1/clusterextensionrevision_types.go index 66382730cb..69802de34f 100644 --- a/api/v1/clusterextensionrevision_types.go +++ b/api/v1/clusterextensionrevision_types.go @@ -22,7 +22,24 @@ import ( "k8s.io/apimachinery/pkg/types" ) -const ClusterExtensionRevisionKind = "ClusterExtensionRevision" +const ( + ClusterExtensionRevisionKind = "ClusterExtensionRevision" + + // Condition Types + ClusterExtensionRevisionTypeAvailable = "Available" + ClusterExtensionRevisionTypeSucceeded = "Succeeded" + + // Condition Reasons + ClusterExtensionRevisionReasonAvailable = "Available" + ClusterExtensionRevisionReasonReconcileFailure = "ReconcileFailure" + ClusterExtensionRevisionReasonRevisionValidationFailure = "RevisionValidationFailure" + ClusterExtensionRevisionReasonPhaseValidationError = "PhaseValidationError" + ClusterExtensionRevisionReasonObjectCollisions = "ObjectCollisions" + ClusterExtensionRevisionReasonRolloutSuccess = "RolloutSuccess" + ClusterExtensionRevisionReasonProbeFailure = "ProbeFailure" + ClusterExtensionRevisionReasonIncomplete = "Incomplete" + ClusterExtensionRevisionReasonProgressing = "Progressing" +) // ClusterExtensionRevisionSpec defines the desired state of ClusterExtensionRevision. type ClusterExtensionRevisionSpec struct { diff --git a/internal/operator-controller/applier/boxcutter.go b/internal/operator-controller/applier/boxcutter.go index 2331051598..116e565f8d 100644 --- a/internal/operator-controller/applier/boxcutter.go +++ b/internal/operator-controller/applier/boxcutter.go @@ -199,10 +199,9 @@ func (bc *Boxcutter) apply(ctx context.Context, contentFS fs.FS, ext *ocv1.Clust // TODO: Delete archived previous revisions over a certain revision limit - // TODO: Define constants for the ClusterExtensionRevision condition types. - progressingCondition := meta.FindStatusCondition(currentRevision.Status.Conditions, "Progressing") - availableCondition := meta.FindStatusCondition(currentRevision.Status.Conditions, "Available") - succeededCondition := meta.FindStatusCondition(currentRevision.Status.Conditions, "Succeeded") + progressingCondition := meta.FindStatusCondition(currentRevision.Status.Conditions, ocv1.TypeProgressing) + availableCondition := meta.FindStatusCondition(currentRevision.Status.Conditions, ocv1.ClusterExtensionRevisionTypeAvailable) + succeededCondition := meta.FindStatusCondition(currentRevision.Status.Conditions, ocv1.ClusterExtensionRevisionTypeSucceeded) if progressingCondition == nil && availableCondition == nil && succeededCondition == nil { return false, "New revision created", nil diff --git a/internal/operator-controller/controllers/clusterextension_controller.go b/internal/operator-controller/controllers/clusterextension_controller.go index b8e8c234f9..2520368426 100644 --- a/internal/operator-controller/controllers/clusterextension_controller.go +++ b/internal/operator-controller/controllers/clusterextension_controller.go @@ -557,8 +557,7 @@ func (d *BoxcutterRevisionStatesGetter) GetRevisionStates(ctx context.Context, e }, } - // TODO: we should make constants for the ClusterExtensionRevision condition types. - if installedCondition := apimeta.FindStatusCondition(rev.Status.Conditions, "Succeeded"); installedCondition == nil || installedCondition.Status != metav1.ConditionTrue { + if installedCondition := apimeta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterExtensionRevisionTypeSucceeded); installedCondition == nil || installedCondition.Status != metav1.ConditionTrue { rs.RollingOut = append(rs.RollingOut, rm) } else { rs.Installed = rm diff --git a/internal/operator-controller/controllers/clusterextensionrevision_controller.go b/internal/operator-controller/controllers/clusterextensionrevision_controller.go index 027da83549..ed04c5b4fc 100644 --- a/internal/operator-controller/controllers/clusterextensionrevision_controller.go +++ b/internal/operator-controller/controllers/clusterextensionrevision_controller.go @@ -92,9 +92,9 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev tres, err := c.RevisionEngine.Teardown(ctx, *revision) if err != nil { meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{ - Type: "Available", + Type: ocv1.ClusterExtensionRevisionTypeAvailable, Status: metav1.ConditionFalse, - Reason: "ReconcileFailure", + Reason: ocv1.ClusterExtensionRevisionReasonReconcileFailure, Message: err.Error(), ObservedGeneration: rev.Generation, }) @@ -108,9 +108,9 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev if err := c.TrackingCache.Free(ctx, rev); err != nil { meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{ - Type: "Available", + Type: ocv1.ClusterExtensionRevisionTypeAvailable, Status: metav1.ConditionFalse, - Reason: "ReconcileFailure", + Reason: ocv1.ClusterExtensionRevisionReasonReconcileFailure, Message: err.Error(), ObservedGeneration: rev.Generation, }) @@ -124,9 +124,9 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev // if err := c.ensureFinalizer(ctx, rev, clusterExtensionRevisionTeardownFinalizer); err != nil { meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{ - Type: "Available", + Type: ocv1.ClusterExtensionRevisionTypeAvailable, Status: metav1.ConditionFalse, - Reason: "ReconcileFailure", + Reason: ocv1.ClusterExtensionRevisionReasonReconcileFailure, Message: err.Error(), ObservedGeneration: rev.Generation, }) @@ -134,9 +134,9 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev } if err := c.establishWatch(ctx, rev, revision); err != nil { meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{ - Type: "Available", + Type: ocv1.ClusterExtensionRevisionTypeAvailable, Status: metav1.ConditionFalse, - Reason: "ReconcileFailure", + Reason: ocv1.ClusterExtensionRevisionReasonReconcileFailure, Message: err.Error(), ObservedGeneration: rev.Generation, }) @@ -145,9 +145,9 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev rres, err := c.RevisionEngine.Reconcile(ctx, *revision, opts...) if err != nil { meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{ - Type: "Available", + Type: ocv1.ClusterExtensionRevisionTypeAvailable, Status: metav1.ConditionFalse, - Reason: "ReconcileFailure", + Reason: ocv1.ClusterExtensionRevisionReasonReconcileFailure, Message: err.Error(), ObservedGeneration: rev.Generation, }) @@ -160,9 +160,9 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev if verr := rres.GetValidationError(); verr != nil { l.Info("preflight error, retrying after 10s", "err", verr.String()) meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{ - Type: "Available", + Type: ocv1.ClusterExtensionRevisionTypeAvailable, Status: metav1.ConditionFalse, - Reason: "RevisionValidationFailure", + Reason: ocv1.ClusterExtensionRevisionReasonRevisionValidationFailure, Message: fmt.Sprintf("revision validation error: %s", verr), ObservedGeneration: rev.Generation, }) @@ -172,9 +172,9 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev if verr := pres.GetValidationError(); verr != nil { l.Info("preflight error, retrying after 10s", "err", verr.String()) meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{ - Type: "Available", + Type: ocv1.ClusterExtensionRevisionTypeAvailable, Status: metav1.ConditionFalse, - Reason: "PhaseValidationError", + Reason: ocv1.ClusterExtensionRevisionReasonPhaseValidationError, Message: fmt.Sprintf("phase %d validation error: %s", i, verr), ObservedGeneration: rev.Generation, }) @@ -189,9 +189,9 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev if len(collidingObjs) > 0 { l.Info("object collision error, retrying after 10s", "collisions", collidingObjs) meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{ - Type: "Available", + Type: ocv1.ClusterExtensionRevisionTypeAvailable, Status: metav1.ConditionFalse, - Reason: "ObjectCollisions", + Reason: ocv1.ClusterExtensionRevisionReasonObjectCollisions, Message: fmt.Sprintf("revision object collisions in phase %d\n%s", i, strings.Join(collidingObjs, "\n\n")), ObservedGeneration: rev.Generation, }) @@ -211,17 +211,17 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev // Report status. meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{ - Type: "Available", + Type: ocv1.ClusterExtensionRevisionTypeAvailable, Status: metav1.ConditionTrue, - Reason: "Available", + Reason: ocv1.ClusterExtensionRevisionReasonAvailable, Message: "Object is available and passes all probes.", ObservedGeneration: rev.Generation, }) - if !meta.IsStatusConditionTrue(rev.Status.Conditions, "Succeeded") { + if !meta.IsStatusConditionTrue(rev.Status.Conditions, ocv1.ClusterExtensionRevisionTypeSucceeded) { meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{ - Type: "Succeeded", + Type: ocv1.ClusterExtensionRevisionTypeSucceeded, Status: metav1.ConditionTrue, - Reason: "RolloutSuccess", + Reason: ocv1.ClusterExtensionRevisionReasonRolloutSuccess, Message: "Revision succeeded rolling out.", ObservedGeneration: rev.Generation, }) @@ -250,17 +250,17 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev } if len(probeFailureMsgs) > 0 { meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{ - Type: "Available", + Type: ocv1.ClusterExtensionRevisionTypeAvailable, Status: metav1.ConditionFalse, - Reason: "ProbeFailure", + Reason: ocv1.ClusterExtensionRevisionReasonProbeFailure, Message: strings.Join(probeFailureMsgs, "\n"), ObservedGeneration: rev.Generation, }) } else { meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{ - Type: "Available", + Type: ocv1.ClusterExtensionRevisionTypeAvailable, Status: metav1.ConditionFalse, - Reason: "Incomplete", + Reason: ocv1.ClusterExtensionRevisionReasonIncomplete, Message: "Revision has not been rolled out completely.", ObservedGeneration: rev.Generation, }) @@ -268,14 +268,14 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev } if rres.InTransistion() { meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{ - Type: "Progressing", + Type: ocv1.TypeProgressing, Status: metav1.ConditionTrue, - Reason: "Progressing", + Reason: ocv1.ClusterExtensionRevisionReasonProgressing, Message: "Rollout in progress.", ObservedGeneration: rev.Generation, }) } else { - meta.RemoveStatusCondition(&rev.Status.Conditions, "Progressing") + meta.RemoveStatusCondition(&rev.Status.Conditions, ocv1.TypeProgressing) } return ctrl.Result{}, c.Client.Status().Update(ctx, rev) @@ -412,7 +412,7 @@ func toBoxcutterRevision(rev *ocv1.ClusterExtensionRevision) (*boxcutter.Revisio return false, []string{".status.observedGeneration outdated"} } for _, cond := range depl.Status.Conditions { - if cond.Type == "Available" && + if cond.Type == ocv1.ClusterExtensionRevisionTypeAvailable && cond.Status == corev1.ConditionTrue && depl.Status.UpdatedReplicas == *depl.Spec.Replicas { return true, nil diff --git a/internal/operator-controller/controllers/clusterextensionrevision_controller_test.go b/internal/operator-controller/controllers/clusterextensionrevision_controller_test.go index 0275b7a126..694bd4d4af 100644 --- a/internal/operator-controller/controllers/clusterextensionrevision_controller_test.go +++ b/internal/operator-controller/controllers/clusterextensionrevision_controller_test.go @@ -77,10 +77,10 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_RevisionProgression(t *te Name: clusterExtensionRevisionName, }, rev) require.NoError(t, err) - cond := meta.FindStatusCondition(rev.Status.Conditions, "Available") + cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterExtensionRevisionTypeAvailable) require.NotNil(t, cond) require.Equal(t, metav1.ConditionFalse, cond.Status) - require.Equal(t, "Incomplete", cond.Reason) + require.Equal(t, ocv1.ClusterExtensionRevisionReasonIncomplete, cond.Reason) require.Equal(t, "Revision has not been rolled out completely.", cond.Message) require.Equal(t, int64(1), cond.ObservedGeneration) }, @@ -166,10 +166,10 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_RevisionProgression(t *te Name: clusterExtensionRevisionName, }, rev) require.NoError(t, err) - cond := meta.FindStatusCondition(rev.Status.Conditions, "Available") + cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterExtensionRevisionTypeAvailable) require.NotNil(t, cond) require.Equal(t, metav1.ConditionFalse, cond.Status) - require.Equal(t, "ProbeFailure", cond.Reason) + require.Equal(t, ocv1.ClusterExtensionRevisionReasonProbeFailure, cond.Reason) require.Equal(t, "Object Service.v1 my-namespace/my-service: something bad happened and something worse happened\nObject ConfigMap.v1 my-namespace/my-configmap: we have a problem", cond.Message) require.Equal(t, int64(1), cond.ObservedGeneration) }, @@ -191,10 +191,10 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_RevisionProgression(t *te Name: clusterExtensionRevisionName, }, rev) require.NoError(t, err) - cond := meta.FindStatusCondition(rev.Status.Conditions, "Progressing") + cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.TypeProgressing) require.NotNil(t, cond) require.Equal(t, metav1.ConditionTrue, cond.Status) - require.Equal(t, "Progressing", cond.Reason) + require.Equal(t, ocv1.ClusterExtensionRevisionReasonProgressing, cond.Reason) require.Equal(t, "Rollout in progress.", cond.Message) require.Equal(t, int64(1), cond.ObservedGeneration) }, @@ -209,9 +209,9 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_RevisionProgression(t *te rev1 := newTestClusterExtensionRevision(clusterExtensionRevisionName) require.NoError(t, controllerutil.SetControllerReference(ext, rev1, testScheme)) meta.SetStatusCondition(&rev1.Status.Conditions, metav1.Condition{ - Type: "Progressing", + Type: ocv1.TypeProgressing, Status: metav1.ConditionTrue, - Reason: "Progressing", + Reason: ocv1.ClusterExtensionRevisionReasonProgressing, Message: "some message", ObservedGeneration: 1, }) @@ -223,7 +223,7 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_RevisionProgression(t *te Name: clusterExtensionRevisionName, }, rev) require.NoError(t, err) - cond := meta.FindStatusCondition(rev.Status.Conditions, "Progressing") + cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.TypeProgressing) require.Nil(t, cond) }, }, @@ -244,17 +244,17 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_RevisionProgression(t *te Name: clusterExtensionRevisionName, }, rev) require.NoError(t, err) - cond := meta.FindStatusCondition(rev.Status.Conditions, "Available") + cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterExtensionRevisionTypeAvailable) require.NotNil(t, cond) require.Equal(t, metav1.ConditionTrue, cond.Status) - require.Equal(t, "Available", cond.Reason) + require.Equal(t, ocv1.ClusterExtensionRevisionReasonAvailable, cond.Reason) require.Equal(t, "Object is available and passes all probes.", cond.Message) require.Equal(t, int64(1), cond.ObservedGeneration) - cond = meta.FindStatusCondition(rev.Status.Conditions, "Succeeded") + cond = meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterExtensionRevisionTypeSucceeded) require.NotNil(t, cond) require.Equal(t, metav1.ConditionTrue, cond.Status) - require.Equal(t, "RolloutSuccess", cond.Reason) + require.Equal(t, ocv1.ClusterExtensionRevisionReasonRolloutSuccess, cond.Reason) require.Equal(t, "Revision succeeded rolling out.", cond.Message) require.Equal(t, int64(1), cond.ObservedGeneration) },