CM-1114: Add health probes and richer status conditions

api/operator/v1alpha1/conditions.go

@@ -25,6 +25,13 @@ const (
 	//   - Failed
 	//   - Ready: operand successfully deployed and ready
 	Ready string = "Ready"
+
+	// Progressing indicates whether the operator is actively reconciling
+	// toward the desired state.
+	//   Status:
+	//   - True: reconciliation is in progress
+	//   - False: reconciliation is idle (succeeded or permanently failed)
+	Progressing string = "Progressing"
 )
 
 const (
@@ -33,6 +40,14 @@ const (
 	ReasonReady string = "Ready"
 
 	ReasonInProgress string = "Progressing"
+
+	ReasonReconciling string = "Reconciling"
+
+	ReasonWaitingForDependencies string = "WaitingForDependencies"
+
+	ReasonValidationFailed string = "ValidationFailed"
+
+	ReasonMultipleInstancesFound string = "MultipleInstancesFound"
 )
 
 func (c *ConditionalStatus) GetCondition(t string) *metav1.Condition {

api/operator/v1alpha1/conditions_test.go

@@ -37,6 +37,29 @@ func TestGetCondition(t *testing.T) {
 				Reason: ReasonReady,
 			},
 		},
+		{
+			name: "progressing condition present",
+			conditionalStatus: ConditionalStatus{
+				Conditions: []metav1.Condition{
+					{
+						Type:   Ready,
+						Status: metav1.ConditionTrue,
+						Reason: ReasonReady,
+					},
+					{
+						Type:   Progressing,
+						Status: metav1.ConditionFalse,
+						Reason: ReasonReady,
+					},
+				},
+			},
+			condition: Progressing,
+			expectedCondition: &metav1.Condition{
+				Type:   Progressing,
+				Status: metav1.ConditionFalse,
+				Reason: ReasonReady,
+			},
+		},
 		{
 			name: "requested condition not present",
 			conditionalStatus: ConditionalStatus{

bundle/manifests/cert-manager-operator.clusterserviceversion.yaml

@@ -805,11 +805,29 @@ spec:
                 - name: UNSUPPORTED_ADDON_FEATURES
                 image: openshift.io/cert-manager-operator:latest
                 imagePullPolicy: IfNotPresent
+                livenessProbe:
+                  failureThreshold: 3
+                  httpGet:
+                    path: /healthz
+                    port: https
+                    scheme: HTTPS
+                  initialDelaySeconds: 15
+                  periodSeconds: 10
+                  timeoutSeconds: 5
                 name: cert-manager-operator
                 ports:
                 - containerPort: 8443
                   name: https
                   protocol: TCP
+                readinessProbe:
+                  failureThreshold: 3
+                  httpGet:
+                    path: /readyz
+                    port: https
+                    scheme: HTTPS
+                  initialDelaySeconds: 5
+                  periodSeconds: 10
+                  timeoutSeconds: 5
                 resources:
                   requests:
                     cpu: 10m

config/manager/manager.yaml

@@ -102,6 +102,24 @@ spec:
           image: controller:latest
           imagePullPolicy: IfNotPresent
           name: cert-manager-operator
+          livenessProbe:
+            httpGet:
+              path: /healthz
+              port: https
+              scheme: HTTPS
+            initialDelaySeconds: 15
+            periodSeconds: 10
+            timeoutSeconds: 5
+            failureThreshold: 3
+          readinessProbe:
+            httpGet:
+              path: /readyz
+              port: https
+              scheme: HTTPS
+            initialDelaySeconds: 5
+            periodSeconds: 10
+            timeoutSeconds: 5
+            failureThreshold: 3
           securityContext:
             allowPrivilegeEscalation: false
             capabilities:

pkg/controller/common/errors.go

@@ -23,9 +23,10 @@ const (
 
 // ReconcileError represents an error that occurred during reconciliation.
 type ReconcileError struct {
-	Reason  ErrorReason `json:"reason,omitempty"`
-	Message string      `json:"message,omitempty"`
-	Err     error       `json:"error,omitempty"`
+	Reason          ErrorReason `json:"reason,omitempty"`
+	ConditionReason string      `json:"conditionReason,omitempty"`
+	Message         string      `json:"message,omitempty"`
+	Err             error       `json:"error,omitempty"`
 }
 
 var _ error = &ReconcileError{}
@@ -117,6 +118,16 @@ func IsMultipleInstanceError(err error) bool {
 	return false
 }
 
+// WithConditionReason sets the condition reason that will appear in status
+// conditions when this error is processed by HandleReconcileResult.
+func (e *ReconcileError) WithConditionReason(reason string) *ReconcileError {
+	if e == nil {
+		return nil
+	}
+	e.ConditionReason = reason
+	return e
+}
+
 // Error implements the error interface.
 func (e *ReconcileError) Error() string {
 	return fmt.Sprintf("%s: %s", e.Message, e.Err)
@@ -126,3 +137,13 @@ func (e *ReconcileError) Error() string {
 func (e *ReconcileError) Unwrap() error {
 	return e.Err
 }
+
+// GetConditionReason extracts the ConditionReason from a ReconcileError in the
+// error chain. Returns empty string if not set or not a ReconcileError.
+func GetConditionReason(err error) string {
+	rerr := &ReconcileError{}
+	if errors.As(err, &rerr) {
+		return rerr.ConditionReason
+	}
+	return ""
+}

pkg/controller/common/errors_test.go

@@ -284,6 +284,89 @@ func TestIsRetryRequiredError(t *testing.T) {
 	}
 }
 
+func TestWithConditionReason(t *testing.T) {
+	tests := []struct {
+		name       string
+		err        *ReconcileError
+		reason     string
+		wantNil    bool
+		wantReason string
+	}{
+		{
+			name:       "sets condition reason on irrecoverable error",
+			err:        NewIrrecoverableError(fmt.Errorf("x"), "msg"),
+			reason:     "ValidationFailed",
+			wantReason: "ValidationFailed",
+		},
+		{
+			name:       "sets condition reason on retry error",
+			err:        NewRetryRequiredError(fmt.Errorf("x"), "msg"),
+			reason:     "ResourceApplyFailed",
+			wantReason: "ResourceApplyFailed",
+		},
+		{
+			name:    "nil error returns nil",
+			err:     nil,
+			reason:  "ValidationFailed",
+			wantNil: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := tt.err.WithConditionReason(tt.reason)
+			if tt.wantNil {
+				if got != nil {
+					t.Errorf("expected nil, got %v", got)
+				}
+				return
+			}
+			if got == nil {
+				t.Fatal("expected non-nil")
+			}
+			if got.ConditionReason != tt.wantReason {
+				t.Errorf("ConditionReason = %q, want %q", got.ConditionReason, tt.wantReason)
+			}
+		})
+	}
+}
+
+func TestGetConditionReason(t *testing.T) {
+	tests := []struct {
+		name       string
+		err        error
+		wantReason string
+	}{
+		{
+			name:       "extracts reason from ReconcileError",
+			err:        NewIrrecoverableError(fmt.Errorf("x"), "msg").WithConditionReason("ValidationFailed"),
+			wantReason: "ValidationFailed",
+		},
+		{
+			name:       "returns empty for ReconcileError without ConditionReason",
+			err:        NewIrrecoverableError(fmt.Errorf("x"), "msg"),
+			wantReason: "",
+		},
+		{
+			name:       "returns empty for plain error",
+			err:        fmt.Errorf("plain"),
+			wantReason: "",
+		},
+		{
+			name:       "returns empty for nil",
+			err:        nil,
+			wantReason: "",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := GetConditionReason(tt.err)
+			if got != tt.wantReason {
+				t.Errorf("GetConditionReason() = %q, want %q", got, tt.wantReason)
+			}
+		})
+	}
+}
+
 func TestIsMultipleInstanceError(t *testing.T) {
 	tests := []struct {
 		name string

pkg/controller/common/reconcile_result.go

@@ -11,6 +11,15 @@ import (
 	v1alpha1 "github.com/openshift/cert-manager-operator/api/operator/v1alpha1"
 )
 
+// resolveConditionReason extracts a specific condition reason from the error
+// chain if one was set via WithConditionReason, otherwise returns the default.
+func resolveConditionReason(err error, defaultReason string) string {
+	if reason := GetConditionReason(err); reason != "" {
+		return reason
+	}
+	return defaultReason
+}
+
 // HandleReconcileResult processes the result of a reconciliation attempt and
 // updates status conditions accordingly.
 func HandleReconcileResult(
@@ -24,52 +33,53 @@ func HandleReconcileResult(
 
 	if reconcileErr != nil {
 		if IsIrrecoverableError(reconcileErr) {
-			// Permanent failure - don't retry
-			// Set Degraded=True, Ready=False
-			// Set both conditions atomically before updating status
-			degradedChanged := status.SetCondition(v1alpha1.Degraded, metav1.ConditionTrue, v1alpha1.ReasonFailed, fmt.Sprintf("reconciliation failed with irrecoverable error not retrying: %v", reconcileErr))
-			readyChanged := status.SetCondition(v1alpha1.Ready, metav1.ConditionFalse, v1alpha1.ReasonFailed, "")
+			reason := resolveConditionReason(reconcileErr, v1alpha1.ReasonFailed)
+			degradedChanged := status.SetCondition(v1alpha1.Degraded, metav1.ConditionTrue, reason, fmt.Sprintf("reconciliation failed with irrecoverable error not retrying: %v", reconcileErr))
+			readyChanged := status.SetCondition(v1alpha1.Ready, metav1.ConditionFalse, reason, "")
+			progressingChanged := status.SetCondition(v1alpha1.Progressing, metav1.ConditionFalse, reason, "")
 
-			if degradedChanged || readyChanged {
+			if degradedChanged || readyChanged || progressingChanged {
 				log.V(2).Info("updating conditions on irrecoverable error",
 					"degradedChanged", degradedChanged,
 					"readyChanged", readyChanged,
+					"progressingChanged", progressingChanged,
+					"reason", reason,
 					"error", reconcileErr)
 				errUpdate = updateConditionFn(nil)
 			}
 			return ctrl.Result{}, errUpdate
 		}
 
-		// Temporary failure - retry after delay
-		// Set Degraded=False, Ready=False with "in progress" message
-		// Set both conditions atomically before updating status
+		readyReason := resolveConditionReason(reconcileErr, v1alpha1.ReasonInProgress)
+		progressingReason := resolveConditionReason(reconcileErr, v1alpha1.ReasonReconciling)
 		degradedChanged := status.SetCondition(v1alpha1.Degraded, metav1.ConditionFalse, v1alpha1.ReasonReady, "")
-		readyChanged := status.SetCondition(v1alpha1.Ready, metav1.ConditionFalse, v1alpha1.ReasonInProgress, fmt.Sprintf("reconciliation failed, retrying: %v", reconcileErr))
+		readyChanged := status.SetCondition(v1alpha1.Ready, metav1.ConditionFalse, readyReason, fmt.Sprintf("reconciliation failed, retrying: %v", reconcileErr))
+		progressingChanged := status.SetCondition(v1alpha1.Progressing, metav1.ConditionTrue, progressingReason, fmt.Sprintf("reconciliation in progress: %v", reconcileErr))
 
-		if degradedChanged || readyChanged {
+		if degradedChanged || readyChanged || progressingChanged {
 			log.V(2).Info("updating conditions on recoverable error",
 				"degradedChanged", degradedChanged,
 				"readyChanged", readyChanged,
+				"progressingChanged", progressingChanged,
+				"reason", readyReason,
 				"error", reconcileErr)
 			errUpdate = updateConditionFn(reconcileErr)
 		}
-		// For recoverable errors, either requeue manually or return error, not both.
-		// If status update failed, return the update error; otherwise requeue.
 		if errUpdate != nil {
 			return ctrl.Result{}, errUpdate
 		}
 		return ctrl.Result{RequeueAfter: requeueDuration}, nil
 	}
 
-	// Success - update status
-	// Set both conditions atomically before updating status on success
 	degradedChanged := status.SetCondition(v1alpha1.Degraded, metav1.ConditionFalse, v1alpha1.ReasonReady, "")
 	readyChanged := status.SetCondition(v1alpha1.Ready, metav1.ConditionTrue, v1alpha1.ReasonReady, "reconciliation successful")
+	progressingChanged := status.SetCondition(v1alpha1.Progressing, metav1.ConditionFalse, v1alpha1.ReasonReady, "")
 
-	if degradedChanged || readyChanged {
+	if degradedChanged || readyChanged || progressingChanged {
 		log.V(2).Info("updating conditions on successful reconciliation",
 			"degradedChanged", degradedChanged,
-			"readyChanged", readyChanged)
+			"readyChanged", readyChanged,
+			"progressingChanged", progressingChanged)
 		errUpdate = updateConditionFn(nil)
 	}
 	return ctrl.Result{}, errUpdate