⚠ split ClusterObjectSet Ready ValidationFailure reason

api/v1/clusterobjectset_types.go

@@ -29,14 +29,15 @@ const (
 	ClusterObjectSetTypeReady = "Ready"
 
 	// Condition Reasons
-	ClusterObjectSetReasonArchived          = "Archived"
-	ClusterObjectSetReasonArchiving         = "Archiving"
-	ClusterObjectSetReasonObjectsNotReady   = "ObjectsNotReady"
-	ClusterObjectSetReasonSuperseded        = "Superseded"
-	ClusterObjectSetReasonObjectCollision   = "ObjectCollision"
-	ClusterObjectSetReasonRetryableError    = "RetryableError"
-	ClusterObjectSetReasonValidationFailure = "ValidationFailure"
-	ClusterObjectSetReasonAllObjectsReady   = "AllObjectsReady"
+	ClusterObjectSetReasonArchived             = "Archived"
+	ClusterObjectSetReasonArchiving            = "Archiving"
+	ClusterObjectSetReasonObjectsNotReady      = "ObjectsNotReady"
+	ClusterObjectSetReasonSuperseded           = "Superseded"
+	ClusterObjectSetReasonObjectCollision      = "ObjectCollision"
+	ClusterObjectSetReasonRetryableError       = "RetryableError"
+	ClusterObjectSetReasonPreconditionsNotMet  = "PreconditionsNotMet"
+	ClusterObjectSetReasonInvalidSpecification = "InvalidSpecification"
+	ClusterObjectSetReasonAllObjectsReady      = "AllObjectsReady"
 )
 
 // ClusterObjectSetSpec defines the desired state of ClusterObjectSet.
@@ -504,7 +505,8 @@ type ClusterObjectSetStatus struct {
 	//   - False with reason ObjectsNotReady: one or more objects are still being applied or have not yet passed their readiness probes during rollout.
 	//   - False with reason Superseded: one or more objects have been adopted by a newer revision of the object set.
 	//   - False with reason ObjectCollision: one or more objects are controlled by another owner, or are unowned while collision protection prevents adoption, so this revision cannot take ownership of them.
-	//   - False with reason ValidationFailure: the revision failed a pre-apply check (an invalid revision/phase object or probe, preflight validation, a mutable referenced Secret, or changed referenced content) and requires manual intervention.
+	//   - False with reason PreconditionsNotMet: a precondition for applying the revision is not yet satisfied (a referenced Secret is mutable or has invalid or changed content, or a preflight dry-run was rejected, e.g. an immutable field, a conflicting object, or RBAC/dependency CRDs not yet propagated); the revision keeps retrying and recovers once the precondition is met.
+	//   - False with reason InvalidSpecification: the revision spec is invalid (a malformed revision/phase object or progression probe) and requires a spec change to proceed.
 	//   - False with reason ProgressDeadlineExceeded: the revision did not roll out within spec.progressDeadlineMinutes.
 	//   - False with reason RetryableError: the revision encountered a transient error that may resolve on a subsequent reconciliation.
 	//   - False with reason Archiving: the revision is being archived and its owned objects are still being torn down.

docs/draft/concepts/clusterobjectsets.md

@@ -220,7 +220,8 @@ Indicates whether all objects in the revision have been successfully rolled out
 | False | `ObjectsNotReady` | One or more objects are still being applied or have not yet passed their readiness probes during rollout |
 | False | `Superseded` | One or more objects have been adopted by a newer revision of the object set |
 | False | `ObjectCollision` | One or more objects are controlled by another owner, or are unowned and collision protection prevents adoption |
-| False | `ValidationFailure` | A pre-apply check failed (an invalid revision/phase object or probe, preflight validation, a mutable referenced Secret, or changed referenced content); requires manual intervention |
+| False | `PreconditionsNotMet` | A precondition for applying the revision is not yet satisfied (a referenced Secret is mutable or has invalid or changed content, or a preflight dry-run was rejected, e.g. an immutable field, a conflicting object, or RBAC/dependency CRDs not yet propagated); the revision keeps retrying and recovers once the precondition is met |
+| False | `InvalidSpecification` | The revision spec is invalid (a malformed revision/phase object or progression probe); requires a spec change |
 | Unknown | `RetryableError` | Error prevented probe observation |
 | False | `Archiving` | The revision is being archived; owned objects are still being torn down |
 | False | `Archived` | Objects torn down after archival |

docs/draft/concepts/large-bundle-support.md

@@ -150,7 +150,7 @@ Recommended conventions:
 2. **Immutability**: Secrets must set `immutable: true`. The reconciler verifies
    that all referenced Secrets have `immutable: true` set before proceeding.
    Mutable referenced Secrets are rejected and the ClusterObjectSet reports
-   `Ready=False, Reason=ValidationFailure`. Additionally, the reconciler records
+   `Ready=False, Reason=PreconditionsNotMet`. Additionally, the reconciler records
    content hashes of the resolved phases on first successful reconciliation
    and reports the same condition if the content changes (e.g., if a Secret is
    deleted and recreated with the same name but different data).
@@ -397,7 +397,7 @@ referenced Secrets have `immutable: true` set. After successfully building
 the phases (resolving all refs), the reconciler computes a per-phase content
 digest and compares it against the digests recorded in `.status.observedPhases`
 (if present). If any phase's content has changed, the ClusterObjectSet reports
-`Ready=False, Reason=ValidationFailure`. On first successful build, phase
+`Ready=False, Reason=PreconditionsNotMet`. On first successful build, phase
 content digests are persisted to status for future comparisons.
 
 When processing a COS phase:
@@ -419,8 +419,8 @@ The reconciler distinguishes two failure modes:
   transient error. The ClusterObjectSet reports `Ready=False, Reason=RetryableError`
   and the controller retries with backoff.
 - **Invalid referenced content** — a missing key, a malformed manifest, or content
-  that has changed since first rollout — is a validation failure. The ClusterObjectSet
-  reports `Ready=False, Reason=ValidationFailure`. Because referenced Secrets are not
+  that has changed since first rollout — is an unmet precondition. The ClusterObjectSet
+  reports `Ready=False, Reason=PreconditionsNotMet`. Because referenced Secrets are not
   watched and the `ref` is immutable, the reconciler retries this case periodically so
   the revision recovers automatically once the referenced Secret is replaced with
   corrected content. Referenced Secrets are immutable, so correcting the content means

helm/olmv1/base/object-controller/crd/experimental/olm.operatorframework.io_clusterobjectsets.yaml

@@ -562,7 +562,8 @@ spec:
                     - False with reason ObjectsNotReady: one or more objects are still being applied or have not yet passed their readiness probes during rollout.
                     - False with reason Superseded: one or more objects have been adopted by a newer revision of the object set.
                     - False with reason ObjectCollision: one or more objects are controlled by another owner, or are unowned while collision protection prevents adoption, so this revision cannot take ownership of them.
-                    - False with reason ValidationFailure: the revision failed a pre-apply check (an invalid revision/phase object or probe, preflight validation, a mutable referenced Secret, or changed referenced content) and requires manual intervention.
+                    - False with reason PreconditionsNotMet: a precondition for applying the revision is not yet satisfied (a referenced Secret is mutable or has invalid or changed content, or a preflight dry-run was rejected, e.g. an immutable field, a conflicting object, or RBAC/dependency CRDs not yet propagated); the revision keeps retrying and recovers once the precondition is met.
+                    - False with reason InvalidSpecification: the revision spec is invalid (a malformed revision/phase object or progression probe) and requires a spec change to proceed.
                     - False with reason ProgressDeadlineExceeded: the revision did not roll out within spec.progressDeadlineMinutes.
                     - False with reason RetryableError: the revision encountered a transient error that may resolve on a subsequent reconciliation.
                     - False with reason Archiving: the revision is being archived and its owned objects are still being torn down.

internal/object-controller/controllers/clusterobjectset_controller.go

@@ -49,7 +49,7 @@ const (
 	clusterObjectSetTeardownFinalizer = "olm.operatorframework.io/teardown"
 
 	// validationRetryInterval is how often the reconciler re-checks a revision that is
-	// Ready=False/ValidationFailure. These failures stem from inputs that are not watched
+	// Ready=False/PreconditionsNotMet. These failures stem from inputs that are not watched
 	// — a mutable or changed referenced Secret, an invalid referenced manifest, or a
 	// preflight dry-run rejected until RBAC or a dependency CRD propagates — and the ref
 	// spec is immutable, so a periodic retry lets the revision recover (once the Secret is
@@ -145,9 +145,9 @@ func (c *ClusterObjectSetReconciler) reconcile(ctx context.Context, cos *ocv1.Cl
 		if errors.As(err, &mutErr) {
 			// A confirmed mutable Secret requires user action (replacing the immutable Secret),
 			// which is not watched, so keep retrying so the revision can recover.
-			// ValidationFailure takes precedence over ProgressDeadlineExceeded: it is more actionable.
+			// PreconditionsNotMet takes precedence over ProgressDeadlineExceeded: it is more actionable.
 			l.Error(err, "referenced Secret is mutable, marking revision not ready")
-			markAsNotReady(cos, ocv1.ClusterObjectSetReasonValidationFailure, err.Error())
+			markAsNotReady(cos, ocv1.ClusterObjectSetReasonPreconditionsNotMet, err.Error())
 			return ctrl.Result{RequeueAfter: validationRetryInterval}, nil
 		}
 		// A Secret read error is transient; surface it as retryable and requeue with backoff.
@@ -164,15 +164,15 @@ func (c *ClusterObjectSetReconciler) reconcile(ctx context.Context, cos *ocv1.Cl
 			// is not watched and the ref is immutable, so keep retrying so the revision can
 			// recover once the Secret is replaced with valid content.
 			l.Error(err, "invalid referenced content, marking revision not ready")
-			markAsNotReady(cos, ocv1.ClusterObjectSetReasonValidationFailure, err.Error())
+			markAsNotReady(cos, ocv1.ClusterObjectSetReasonPreconditionsNotMet, fmt.Sprintf("Referenced content is invalid: %v. Fix the Secret content.", err))
 			return ctrl.Result{RequeueAfter: validationRetryInterval}, nil
 		}
 		var cfgErr *configValidationError
 		if errors.As(err, &cfgErr) {
 			// Invalid revision spec; a spec edit (or a new revision) re-triggers reconciliation,
 			// so no periodic retry is needed.
 			l.Error(err, "invalid configuration, marking revision not ready")
-			markAsNotReady(cos, ocv1.ClusterObjectSetReasonValidationFailure, err.Error())
+			markAsNotReady(cos, ocv1.ClusterObjectSetReasonInvalidSpecification, fmt.Sprintf("The object set spec is invalid: %v.", err))
 			return ctrl.Result{}, nil
 		}
 		setRetryableErrorConditions(cos, err.Error(), isDeadlineExceeded)
@@ -186,7 +186,7 @@ func (c *ClusterObjectSetReconciler) reconcile(ctx context.Context, cos *ocv1.Cl
 		// Secrets are not watched, so keep retrying so the revision can recover once the
 		// original content is restored.
 		l.Error(err, "resolved phases content changed, marking revision not ready")
-		markAsNotReady(cos, ocv1.ClusterObjectSetReasonValidationFailure, err.Error())
+		markAsNotReady(cos, ocv1.ClusterObjectSetReasonPreconditionsNotMet, fmt.Sprintf("Referenced Secret content no longer matches the original rollout: %v. Restore the original content.", err))
 		return ctrl.Result{RequeueAfter: validationRetryInterval}, nil
 	}
 
@@ -240,7 +240,7 @@ func (c *ClusterObjectSetReconciler) reconcile(ctx context.Context, cos *ocv1.Cl
 	// or a dependency CRD still propagating), and those inputs are not watched.
 	if verr := rres.GetValidationError(); verr != nil {
 		l.Error(fmt.Errorf("%w", verr), "preflight validation failed, retrying after 10s")
-		markAsNotReady(cos, ocv1.ClusterObjectSetReasonValidationFailure, fmt.Sprintf("revision validation error: %s", verr))
+		markAsNotReady(cos, ocv1.ClusterObjectSetReasonPreconditionsNotMet, fmt.Sprintf("A pre-apply check rejected the revision: %v.", verr))
 		return ctrl.Result{RequeueAfter: validationRetryInterval}, nil
 	}
 
@@ -263,7 +263,7 @@ func (c *ClusterObjectSetReconciler) reconcile(ctx context.Context, cos *ocv1.Cl
 				continue
 			}
 			l.Error(fmt.Errorf("%w", verr), "phase preflight validation failed, retrying after 10s", "phase", pres.GetName())
-			markAsNotReady(cos, ocv1.ClusterObjectSetReasonValidationFailure, fmt.Sprintf("phase %q validation error: %s", pres.GetName(), verr))
+			markAsNotReady(cos, ocv1.ClusterObjectSetReasonPreconditionsNotMet, fmt.Sprintf("A pre-apply check rejected phase %q: %v.", pres.GetName(), verr))
 			return ctrl.Result{RequeueAfter: validationRetryInterval}, nil
 		}
 
@@ -564,14 +564,14 @@ func (c *ClusterObjectSetReconciler) listOtherActiveRevisions(
 
 // configValidationError marks an error caused by invalid user-provided configuration in the
 // revision spec (a malformed revision/phase object or a bad progression probe). The spec is
-// either immutable or watched, so the reconciler reports it as Ready=False/ValidationFailure
+// either immutable or watched, so the reconciler reports it as Ready=False/InvalidSpecification
 // without a periodic retry: a spec edit re-triggers reconciliation.
 type configValidationError struct{ error }
 
 // referencedContentError marks a validation failure caused by the content of a referenced
 // Secret (a missing key or a malformed manifest). Referenced Secrets are not watched and the
 // phase/ref spec is immutable, so the failure can only clear when the Secret is replaced.
-// The reconciler reports it as Ready=False/ValidationFailure but keeps retrying periodically
+// The reconciler reports it as Ready=False/PreconditionsNotMet but keeps retrying periodically
 // so the revision can recover without a controller restart.
 type referencedContentError struct{ error }
 
@@ -614,7 +614,7 @@ func (c *ClusterObjectSetReconciler) buildBoxcutterPhases(ctx context.Context, c
 				}
 				obj = resolved
 			default:
-				return nil, nil, nil, &configValidationError{fmt.Errorf("object in phase %q has neither object nor ref", specPhase.Name)}
+				return nil, nil, nil, &configValidationError{fmt.Errorf("an object in phase %q has neither an inline object nor a Secret reference", specPhase.Name)}
 			}
 
 			objs = append(objs, obj)
@@ -724,7 +724,7 @@ func buildProgressionProbes(progressionProbes []ocv1.ProgressionProbe) (probing.
 				fieldValueProbe := probing.FieldValueProbe(probe.FieldValue)
 				assertions = append(assertions, &fieldValueProbe)
 			default:
-				return nil, &configValidationError{fmt.Errorf("unknown progressionProbe assertion probe type: %s", probe.Type)}
+				return nil, &configValidationError{fmt.Errorf("unknown progression probe assertion type %q", probe.Type)}
 			}
 		}
 
@@ -740,14 +740,14 @@ func buildProgressionProbes(progressionProbes []ocv1.ProgressionProbe) (probing.
 		case ocv1.SelectorTypeLabel:
 			selector, err := metav1.LabelSelectorAsSelector(&progressionProbe.Selector.Label)
 			if err != nil {
-				return nil, &configValidationError{fmt.Errorf("invalid label selector in progressionProbe (%v): %w", progressionProbe.Selector.Label, err)}
+				return nil, &configValidationError{fmt.Errorf("a progression probe has an invalid label selector (%v): %w", progressionProbe.Selector.Label, err)}
 			}
 			selectorProbe = &probing.LabelSelector{
 				Selector: selector,
 				Prober:   assertions,
 			}
 		default:
-			return nil, &configValidationError{fmt.Errorf("unknown progressionProbe selector type: %s", progressionProbe.Selector.Type)}
+			return nil, &configValidationError{fmt.Errorf("unknown progression probe selector type %q", progressionProbe.Selector.Type)}
 		}
 		userProbes = append(userProbes, &probing.ObservedGenerationProbe{
 			Prober: selectorProbe,
@@ -868,9 +868,9 @@ func verifyObservedPhases(stored, current []ocv1.ObservedPhase) error {
 }
 
 // mutableSecretError is returned by verifyReferencedSecretsImmutable when one or
-// more referenced Secrets are confirmed to be mutable. It distinguishes a
-// (non-retryable) validation failure from a transient Secret read error, so the
-// caller can reserve ValidationFailure for the former and retry the latter.
+// more referenced Secrets are confirmed to be mutable. It distinguishes a confirmed
+// precondition failure (reported as PreconditionsNotMet and retried on a timer) from
+// a transient Secret read error (surfaced as a plain error for backoff requeue).
 type mutableSecretError struct {
 	msg string
 }
@@ -929,7 +929,7 @@ func (c *ClusterObjectSetReconciler) verifyReferencedSecretsImmutable(ctx contex
 
 	if len(mutableSecrets) > 0 {
 		return &mutableSecretError{msg: fmt.Sprintf(
-			"the following secrets are not immutable (referenced secrets must have immutable set to true): %s",
+			"Referenced Secret(s) must be immutable but are not: %s. Recreate them with immutable set to true.",
 			strings.Join(mutableSecrets, ", "))}
 	}
 

internal/object-controller/controllers/clusterobjectset_controller_internal_test.go

@@ -468,7 +468,7 @@ func TestVerifyReferencedSecretsImmutable(t *testing.T) {
 
 		err := reconciler.verifyReferencedSecretsImmutable(t.Context(), cos)
 		require.Error(t, err)
-		assert.Contains(t, err.Error(), "not immutable")
+		assert.Contains(t, err.Error(), "must be immutable but are not")
 	})
 
 	t.Run("skips phases with inline objects only", func(t *testing.T) {