From 2056a0c685e839516c81544b0f4a627076ac260d Mon Sep 17 00:00:00 2001 From: Daniel Franz Date: Mon, 26 Oct 2026 12:47:41 +0900 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20add=20ObservedPhase.objectDetails:?= =?UTF-8?q?=20per-object=20failure=20detail?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds objectDetails []ObjectStatus to ObservedPhase, listing incomplete objects with their failure messages during Reconciling, Pending, and TearingDown phases. ObjectStatus carries group/version/kind/namespace/ name and a slice of human-readable messages from probe failures, ownership collisions, and modification diffs. Signed-off-by: Daniel Franz Co-Authored-By: Claude Sonnet 4.6 (1M context) --- api/v1/clusterobjectset_types.go | 73 ++- api/v1/zz_generated.deepcopy.go | 27 + applyconfigurations/api/v1/objectstatus.go | 103 ++++ applyconfigurations/api/v1/observedphase.go | 24 +- applyconfigurations/internal/internal.go | 30 ++ applyconfigurations/utils.go | 2 + docs/api-reference/olmv1-api-reference.md | 2 + ...peratorframework.io_clusterobjectsets.yaml | 71 ++- internal/object-controller/status/status.go | 168 ++++++- .../object-controller/status/status_test.go | 470 +++++++++++++++++- manifests/experimental-e2e.yaml | 71 ++- manifests/experimental.yaml | 71 ++- 12 files changed, 1099 insertions(+), 13 deletions(-) create mode 100644 applyconfigurations/api/v1/objectstatus.go diff --git a/api/v1/clusterobjectset_types.go b/api/v1/clusterobjectset_types.go index cb8b2814c8..c23a5328b5 100644 --- a/api/v1/clusterobjectset_types.go +++ b/api/v1/clusterobjectset_types.go @@ -578,7 +578,7 @@ type ObservedPhase struct { Name string `json:"name"` // status is the current state of this phase in the rollout. Must be one of - // InvalidSpecification, Pending, Reconciling, + // InvalidSpecification, PreconditionsNotMet, Pending, Reconciling, // Stabilizing, Ready, Unknown, Superseded, Archiving, or Archived. // +required Status PhaseStatus `json:"status"` @@ -618,6 +618,19 @@ type ObservedPhase struct { // +kubebuilder:validation:Type=string // +kubebuilder:validation:Format=date-time LastTransitionTime metav1.Time `json:"lastTransitionTime,omitzero"` + + // objectDetails lists objects in this phase that are not yet + // complete. For Reconciling, Pending, and Stabilizing phases, this + // includes probe failures, collisions, creation/update errors, and + // any other condition preventing completion. For Archiving phases, + // this lists objects still awaiting deletion. Each entry identifies + // the object and carries failure messages. Empty when status is + // Ready, Superseded, Archived, or Unknown. The list may contain at + // most 50 entries, matching the maximum number of objects per phase. + // +kubebuilder:validation:MaxItems=50 + // +listType=atomic + // +optional + ObjectDetails []ObjectStatus `json:"objectDetails,omitempty"` } // ObjectCounts reports the number of objects in a ClusterObjectSet or phase by state. @@ -652,16 +665,70 @@ type ObjectCounts struct { Ready int64 `json:"ready"` } +// ObjectStatus identifies a managed object and its failure messages. +type ObjectStatus struct { + // group is the API group of the object (empty string for core + // resources). At most 253 characters (DNS subdomain max). + // +kubebuilder:validation:MaxLength=253 + // +optional + Group string `json:"group,omitempty"` + + // version is the API version of the object. Must be between 1 and + // 63 characters. + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=63 + // +required + Version string `json:"version"` + + // kind is the kind of the object. Must be between 1 and 63 + // characters. + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=63 + // +required + Kind string `json:"kind"` + + // namespace is the namespace of the object. Empty for cluster-scoped + // resources. At most 253 characters (DNS subdomain max). + // +kubebuilder:validation:MaxLength=253 + // +optional + Namespace string `json:"namespace,omitempty"` + + // name is the name of the object. Must be between 1 and 253 + // characters (DNS subdomain max). + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=253 + // +required + Name string `json:"name"` + + // messages lists the failure reasons for this object. The maximum + // of 18 entries accounts for up to 16 assertion probe failures, + // one collision message, and one message to note truncation if necessary. + // Each message is at most 1024 characters; longer messages are truncated + // by the controller. + // +kubebuilder:validation:MaxItems=18 + // +kubebuilder:validation:items:MaxLength=1024 + // +listType=atomic + // +optional + Messages []string `json:"messages,omitempty"` +} + // PhaseStatus describes the current state of a phase in the rollout. // -// +kubebuilder:validation:Enum=InvalidSpecification;Pending;Reconciling;Stabilizing;Ready;Unknown;Superseded;Archiving;Archived +// +kubebuilder:validation:Enum=InvalidSpecification;PreconditionsNotMet;Pending;Reconciling;Stabilizing;Ready;Unknown;Superseded;Archiving;Archived type PhaseStatus string const ( // PhaseStatusInvalidSpecification indicates the phase failed preflight validation - // in a permanent way (e.g. cross-phase duplication). + // in a permanent way that requires a spec change to resolve (e.g. a schema + // violation, cross-phase object duplication, or an invalid phase name). PhaseStatusInvalidSpecification PhaseStatus = "InvalidSpecification" + // PhaseStatusPreconditionsNotMet indicates the phase failed a recoverable preflight + // dry-run check — for example, RBAC permissions or a dependency CRD that has not + // yet propagated to the cluster. The controller retries automatically; no spec + // change is required. + PhaseStatusPreconditionsNotMet PhaseStatus = "PreconditionsNotMet" + // PhaseStatusPending indicates the phase is not being actively // reconciled, but the controller has checked and some objects do not // match the desired state. diff --git a/api/v1/zz_generated.deepcopy.go b/api/v1/zz_generated.deepcopy.go index fbd793cbde..67436066fc 100644 --- a/api/v1/zz_generated.deepcopy.go +++ b/api/v1/zz_generated.deepcopy.go @@ -863,12 +863,39 @@ func (in *ObjectSourceRef) DeepCopy() *ObjectSourceRef { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ObjectStatus) DeepCopyInto(out *ObjectStatus) { + *out = *in + if in.Messages != nil { + in, out := &in.Messages, &out.Messages + *out = make([]string, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ObjectStatus. +func (in *ObjectStatus) DeepCopy() *ObjectStatus { + if in == nil { + return nil + } + out := new(ObjectStatus) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ObservedPhase) DeepCopyInto(out *ObservedPhase) { *out = *in in.CompletedAt.DeepCopyInto(&out.CompletedAt) out.ObjectCounts = in.ObjectCounts in.LastTransitionTime.DeepCopyInto(&out.LastTransitionTime) + if in.ObjectDetails != nil { + in, out := &in.ObjectDetails, &out.ObjectDetails + *out = make([]ObjectStatus, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ObservedPhase. diff --git a/applyconfigurations/api/v1/objectstatus.go b/applyconfigurations/api/v1/objectstatus.go new file mode 100644 index 0000000000..f0714db81c --- /dev/null +++ b/applyconfigurations/api/v1/objectstatus.go @@ -0,0 +1,103 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Code generated by controller-gen-v0.21. DO NOT EDIT. + +package v1 + +// ObjectStatusApplyConfiguration represents a declarative configuration of the ObjectStatus type for use +// with apply. +// +// ObjectStatus identifies a managed object and its failure messages. +type ObjectStatusApplyConfiguration struct { + // group is the API group of the object (empty string for core + // resources). At most 253 characters (DNS subdomain max). + Group *string `json:"group,omitempty"` + // version is the API version of the object. Must be between 1 and + // 63 characters. + Version *string `json:"version,omitempty"` + // kind is the kind of the object. Must be between 1 and 63 + // characters. + Kind *string `json:"kind,omitempty"` + // namespace is the namespace of the object. Empty for cluster-scoped + // resources. At most 253 characters (DNS subdomain max). + Namespace *string `json:"namespace,omitempty"` + // name is the name of the object. Must be between 1 and 253 + // characters (DNS subdomain max). + Name *string `json:"name,omitempty"` + // messages lists the failure reasons for this object. The maximum + // of 18 entries accounts for up to 16 assertion probe failures, + // one collision message, and one message to note truncation if necessary. + // Each message is at most 1024 characters; longer messages are truncated + // by the controller. + Messages []string `json:"messages,omitempty"` +} + +// ObjectStatusApplyConfiguration constructs a declarative configuration of the ObjectStatus type for use with +// apply. +func ObjectStatus() *ObjectStatusApplyConfiguration { + return &ObjectStatusApplyConfiguration{} +} + +// WithGroup sets the Group field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the Group field is set to the value of the last call. +func (b *ObjectStatusApplyConfiguration) WithGroup(value string) *ObjectStatusApplyConfiguration { + b.Group = &value + return b +} + +// WithVersion sets the Version field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the Version field is set to the value of the last call. +func (b *ObjectStatusApplyConfiguration) WithVersion(value string) *ObjectStatusApplyConfiguration { + b.Version = &value + return b +} + +// WithKind sets the Kind field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the Kind field is set to the value of the last call. +func (b *ObjectStatusApplyConfiguration) WithKind(value string) *ObjectStatusApplyConfiguration { + b.Kind = &value + return b +} + +// WithNamespace sets the Namespace field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the Namespace field is set to the value of the last call. +func (b *ObjectStatusApplyConfiguration) WithNamespace(value string) *ObjectStatusApplyConfiguration { + b.Namespace = &value + return b +} + +// WithName sets the Name field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the Name field is set to the value of the last call. +func (b *ObjectStatusApplyConfiguration) WithName(value string) *ObjectStatusApplyConfiguration { + b.Name = &value + return b +} + +// WithMessages adds the given value to the Messages field in the declarative configuration +// and returns the receiver, so that objects can be build by chaining "With" function invocations. +// If called multiple times, values provided by each call will be appended to the Messages field. +func (b *ObjectStatusApplyConfiguration) WithMessages(values ...string) *ObjectStatusApplyConfiguration { + for i := range values { + b.Messages = append(b.Messages, values[i]) + } + return b +} diff --git a/applyconfigurations/api/v1/observedphase.go b/applyconfigurations/api/v1/observedphase.go index 208b8605a6..30decd9600 100644 --- a/applyconfigurations/api/v1/observedphase.go +++ b/applyconfigurations/api/v1/observedphase.go @@ -31,7 +31,7 @@ type ObservedPhaseApplyConfiguration struct { // name is the phase name matching a phase in spec.phases. Name *string `json:"name,omitempty"` // status is the current state of this phase in the rollout. Must be one of - // InvalidSpecification, Pending, Reconciling, + // InvalidSpecification, PreconditionsNotMet, Pending, Reconciling, // Stabilizing, Ready, Unknown, Superseded, Archiving, or Archived. Status *apiv1.PhaseStatus `json:"status,omitempty"` // completedAt is the timestamp when this phase first became Ready. @@ -50,6 +50,15 @@ type ObservedPhaseApplyConfiguration struct { ObjectCounts *ObjectCountsApplyConfiguration `json:"objectCounts,omitempty"` // lastTransitionTime is the last time the phase transitioned from one status to another. LastTransitionTime *metav1.Time `json:"lastTransitionTime,omitempty"` + // objectDetails lists objects in this phase that are not yet + // complete. For Reconciling, Pending, and Stabilizing phases, this + // includes probe failures, collisions, creation/update errors, and + // any other condition preventing completion. For Archiving phases, + // this lists objects still awaiting deletion. Each entry identifies + // the object and carries failure messages. Empty when status is + // Ready, Superseded, Archived, or Unknown. The list may contain at + // most 50 entries, matching the maximum number of objects per phase. + ObjectDetails []ObjectStatusApplyConfiguration `json:"objectDetails,omitempty"` } // ObservedPhaseApplyConfiguration constructs a declarative configuration of the ObservedPhase type for use with @@ -113,3 +122,16 @@ func (b *ObservedPhaseApplyConfiguration) WithLastTransitionTime(value metav1.Ti b.LastTransitionTime = &value return b } + +// WithObjectDetails adds the given value to the ObjectDetails field in the declarative configuration +// and returns the receiver, so that objects can be build by chaining "With" function invocations. +// If called multiple times, values provided by each call will be appended to the ObjectDetails field. +func (b *ObservedPhaseApplyConfiguration) WithObjectDetails(values ...*ObjectStatusApplyConfiguration) *ObservedPhaseApplyConfiguration { + for i := range values { + if values[i] == nil { + panic("nil value passed to WithObjectDetails") + } + b.ObjectDetails = append(b.ObjectDetails, *values[i]) + } + return b +} diff --git a/applyconfigurations/internal/internal.go b/applyconfigurations/internal/internal.go index e14ea7f04c..a71f087ad8 100644 --- a/applyconfigurations/internal/internal.go +++ b/applyconfigurations/internal/internal.go @@ -506,6 +506,30 @@ var schemaYAML = typed.YAMLObject(`types: - name: namespace type: scalar: string +- name: com.github.operator-framework.operator-controller.api.v1.ObjectStatus + map: + fields: + - name: group + type: + scalar: string + - name: kind + type: + scalar: string + - name: messages + type: + list: + elementType: + scalar: string + elementRelationship: atomic + - name: name + type: + scalar: string + - name: namespace + type: + scalar: string + - name: version + type: + scalar: string - name: com.github.operator-framework.operator-controller.api.v1.ObservedPhase map: fields: @@ -527,6 +551,12 @@ var schemaYAML = typed.YAMLObject(`types: - name: objectCounts type: namedType: com.github.operator-framework.operator-controller.api.v1.ObjectCounts + - name: objectDetails + type: + list: + elementType: + namedType: com.github.operator-framework.operator-controller.api.v1.ObjectStatus + elementRelationship: atomic - name: status type: namedType: com.github.operator-framework.operator-controller.api.v1.PhaseStatus diff --git a/applyconfigurations/utils.go b/applyconfigurations/utils.go index 7f3fc9f1d4..3e199d3ad4 100644 --- a/applyconfigurations/utils.go +++ b/applyconfigurations/utils.go @@ -98,6 +98,8 @@ func ForKind(kind schema.GroupVersionKind) interface{} { return &apiv1.ObjectSelectorApplyConfiguration{} case v1.SchemeGroupVersion.WithKind("ObjectSourceRef"): return &apiv1.ObjectSourceRefApplyConfiguration{} + case v1.SchemeGroupVersion.WithKind("ObjectStatus"): + return &apiv1.ObjectStatusApplyConfiguration{} case v1.SchemeGroupVersion.WithKind("ObservedPhase"): return &apiv1.ObservedPhaseApplyConfiguration{} case v1.SchemeGroupVersion.WithKind("PreflightConfig"): diff --git a/docs/api-reference/olmv1-api-reference.md b/docs/api-reference/olmv1-api-reference.md index adeba895b0..2c33623f02 100644 --- a/docs/api-reference/olmv1-api-reference.md +++ b/docs/api-reference/olmv1-api-reference.md @@ -690,6 +690,8 @@ _Appears in:_ + + #### PreflightConfig diff --git a/helm/olmv1/base/object-controller/crd/experimental/olm.operatorframework.io_clusterobjectsets.yaml b/helm/olmv1/base/object-controller/crd/experimental/olm.operatorframework.io_clusterobjectsets.yaml index 7cf88d20dd..8661270922 100644 --- a/helm/olmv1/base/object-controller/crd/experimental/olm.operatorframework.io_clusterobjectsets.yaml +++ b/helm/olmv1/base/object-controller/crd/experimental/olm.operatorframework.io_clusterobjectsets.yaml @@ -816,13 +816,82 @@ spec: - synced - total type: object + objectDetails: + description: |- + objectDetails lists objects in this phase that are not yet + complete. For Reconciling, Pending, and Stabilizing phases, this + includes probe failures, collisions, creation/update errors, and + any other condition preventing completion. For Archiving phases, + this lists objects still awaiting deletion. Each entry identifies + the object and carries failure messages. Empty when status is + Ready, Superseded, Archived, or Unknown. The list may contain at + most 50 entries, matching the maximum number of objects per phase. + items: + description: ObjectStatus identifies a managed object and + its failure messages. + properties: + group: + description: |- + group is the API group of the object (empty string for core + resources). At most 253 characters (DNS subdomain max). + maxLength: 253 + type: string + kind: + description: |- + kind is the kind of the object. Must be between 1 and 63 + characters. + maxLength: 63 + minLength: 1 + type: string + messages: + description: |- + messages lists the failure reasons for this object. The maximum + of 18 entries accounts for up to 16 assertion probe failures, + one collision message, and one message to note truncation if necessary. + Each message is at most 1024 characters; longer messages are truncated + by the controller. + items: + maxLength: 1024 + type: string + maxItems: 18 + type: array + x-kubernetes-list-type: atomic + name: + description: |- + name is the name of the object. Must be between 1 and 253 + characters (DNS subdomain max). + maxLength: 253 + minLength: 1 + type: string + namespace: + description: |- + namespace is the namespace of the object. Empty for cluster-scoped + resources. At most 253 characters (DNS subdomain max). + maxLength: 253 + type: string + version: + description: |- + version is the API version of the object. Must be between 1 and + 63 characters. + maxLength: 63 + minLength: 1 + type: string + required: + - kind + - name + - version + type: object + maxItems: 50 + type: array + x-kubernetes-list-type: atomic status: description: |- status is the current state of this phase in the rollout. Must be one of - InvalidSpecification, Pending, Reconciling, + InvalidSpecification, PreconditionsNotMet, Pending, Reconciling, Stabilizing, Ready, Unknown, Superseded, Archiving, or Archived. enum: - InvalidSpecification + - PreconditionsNotMet - Pending - Reconciling - Stabilizing diff --git a/internal/object-controller/status/status.go b/internal/object-controller/status/status.go index a490f634eb..9f29711c0a 100644 --- a/internal/object-controller/status/status.go +++ b/internal/object-controller/status/status.go @@ -1,10 +1,18 @@ package status import ( + "fmt" + "maps" "slices" + "strings" + apierrors "k8s.io/apimachinery/pkg/api/errors" + apimeta "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "pkg.package-operator.run/boxcutter/machinery" + "pkg.package-operator.run/boxcutter/machinery/types" + "pkg.package-operator.run/boxcutter/validation" + "sigs.k8s.io/structured-merge-diff/v6/fieldpath" ocv1 "github.com/operator-framework/operator-controller/api/v1" ) @@ -62,6 +70,9 @@ func UpdateObservedPhasesForReconcile(objSet *ocv1.ClusterObjectSet, rres machin if !precedingPhasesComplete { observedPhase.Status = ocv1.PhaseStatusPending observedPhase.Message = "waiting for earlier phases to complete" + } else if isRecoverablePhaseValidation(pres.GetValidationError()) { + observedPhase.Status = ocv1.PhaseStatusPreconditionsNotMet + observedPhase.Message = truncateMessage(pres.GetValidationError().Error()) } else { observedPhase.Status = ocv1.PhaseStatusInvalidSpecification observedPhase.Message = truncateMessage(pres.GetValidationError().Error()) @@ -109,7 +120,7 @@ func UpdateObservedPhasesForReconcile(objSet *ocv1.ClusterObjectSet, rres machin var ( // TODO: consider reframing as one or more objects have transitioned (rather than all - since there can be differences b/w the revisions) - isSuperseded = (HasSupersededObject(pres) && pres.IsComplete()) || pres.HasProgressed() + isSuperseded = (HasSupersededObject(pres) && pres.IsComplete()) || (len(pres.GetObjects()) > 0 && pres.HasProgressed()) isReady = pres.IsComplete() && !isSuperseded isSynced = observedPhase.ObjectCounts.Synced == observedPhase.ObjectCounts.Total isReconciling = !isSynced && !isPaused @@ -123,7 +134,6 @@ func UpdateObservedPhasesForReconcile(objSet *ocv1.ClusterObjectSet, rres machin observedPhase.Status = ocv1.PhaseStatusSuperseded observedPhase.Message = "all objects have progressed to a new revision" case isSynced: - // TODO: collect assertion failures and add to status observedPhase.Status = ocv1.PhaseStatusStabilizing observedPhase.Message = "all objects are synced; waiting for them to become ready" case isReconciling: @@ -136,6 +146,15 @@ func UpdateObservedPhasesForReconcile(objSet *ocv1.ClusterObjectSet, rres machin observedPhase.Status = ocv1.PhaseStatusPending observedPhase.Message = "waiting for previous phases to be ready" } + + if !isReady && !isSuperseded { + for _, obj := range pres.GetObjects() { + if obj.IsComplete() || obj.Action() == machinery.ActionProgressed { + continue + } + observedPhase.ObjectDetails = append(observedPhase.ObjectDetails, objectStatus(obj)) + } + } updateObservedPhaseStatus(objSet, observedPhase, now) } objSet.Status.ObjectCounts = sumObjectCounts(objSet.Status.ObservedPhases) @@ -171,6 +190,16 @@ func UpdateObservedPhasesForTeardown(objSet *ocv1.ClusterObjectSet, tres machine observedPhase.Status = ocv1.PhaseStatusPending observedPhase.Message = "waiting for later phases to complete teardown" } + for _, ref := range pres.Waiting() { + observedPhase.ObjectDetails = append(observedPhase.ObjectDetails, ocv1.ObjectStatus{ + Group: ref.Group, + Version: ref.Version, + Kind: ref.Kind, + Namespace: ref.Namespace, + Name: ref.Name, + Messages: []string{"awaiting deletion"}, + }) + } updateObservedPhaseStatus(objSet, observedPhase, now) } objSet.Status.ObjectCounts = sumObjectCounts(objSet.Status.ObservedPhases) @@ -226,6 +255,141 @@ func HasSupersededObject(pres machinery.PhaseResult) bool { return false } +// objectStatus returns details describing why obj is not yet complete. +func objectStatus(obj machinery.ObjectResult) ocv1.ObjectStatus { + ref := types.ToObjectRef(obj.Object()) + objStatus := ocv1.ObjectStatus{ + Group: ref.Group, + Version: ref.Version, + Kind: ref.Kind, + Namespace: ref.Namespace, + Name: ref.Name, + } + + if obj.IsPaused() && obj.Action() != machinery.ActionIdle { + objStatus.Messages = pausedObjectMessages(obj) + return objStatus + } + + var msgs []string + + if obj.Action() == machinery.ActionCollision { + msgs = append(msgs, "object ownership collision") + } + + probes := obj.ProbeResults() + for _, probeType := range slices.Sorted(maps.Keys(probes)) { + result := probes[probeType] + if result.Status == types.ProbeStatusTrue { + continue + } + for _, m := range result.Messages { + msgs = append(msgs, truncateMessage(fmt.Sprintf("%s: %s", probeType, m))) + } + if len(result.Messages) == 0 { + msgs = append(msgs, truncateMessage(fmt.Sprintf("%s: %s", probeType, result.Status))) + } + } + + if len(msgs) == 0 { + msgs = append(msgs, "not yet complete") + } else if len(msgs) > 18 { + msgs = msgs[:17] + msgs = append(msgs, "additional objectDetails messages were removed for size constraints") + } + + objStatus.Messages = msgs + return objStatus +} + +// pausedObjectMessages returns messages describing why a paused object is not +// idle, including field-level diff details when available. +func pausedObjectMessages(obj machinery.ObjectResult) []string { + var summary string + switch obj.Action() { + case machinery.ActionCreated: + return []string{"object does not exist"} + case machinery.ActionRecovered: + summary = "object was modified by another actor" + case machinery.ActionCollision: + summary = "object ownership collision" + default: + summary = "object content has changed" + } + + if details := compareDetails(obj); len(details) > 0 { + summary += "\n" + strings.Join(details, "\n") + } + return []string{truncateMessage(summary)} +} + +// compareResulter is implemented by ObjectResult types that expose field-level +// comparison results. +type compareResulter interface { + CompareResult() machinery.CompareResult +} + +// compareDetails extracts human-readable field diff lines (added, modified, +// removed) from obj's CompareResult, if obj implements compareResulter. +func compareDetails(obj machinery.ObjectResult) []string { + cr, ok := obj.(compareResulter) + if !ok { + return nil + } + comp := cr.CompareResult().Comparison + if comp == nil { + return nil + } + var details []string + if comp.Added != nil && !comp.Added.Empty() { + comp.Added.Leaves().Iterate(func(p fieldpath.Path) { + details = append(details, fmt.Sprintf(" - added: %s", p)) + }) + } + if comp.Modified != nil && !comp.Modified.Empty() { + comp.Modified.Leaves().Iterate(func(p fieldpath.Path) { + details = append(details, fmt.Sprintf(" - modified: %s", p)) + }) + } + if comp.Removed != nil && !comp.Removed.Empty() { + comp.Removed.Leaves().Iterate(func(p fieldpath.Path) { + details = append(details, fmt.Sprintf(" - removed: %s", p)) + }) + } + return details +} + +// isRecoverablePhaseValidation reports whether a PhaseValidationError represents a +// recoverable precondition failure — specifically, a dry-run rejection caused by RBAC +// not yet propagated (Forbidden/Unauthorized) or a dependency CRD not yet registered +// (NoMatch). The controller retries automatically; no spec change is required. +// Returns false for permanent failures (invalid phase name, schema violations, +// duplicate objects) or any mixture of recoverable and permanent errors. +func isRecoverablePhaseValidation(verr *validation.PhaseValidationError) bool { + if verr == nil || verr.PhaseError != nil || len(verr.Objects) == 0 { + return false + } + for _, obj := range verr.Objects { + for _, e := range obj.Errors { + if !isRecoverableObjectError(e) { + return false + } + } + } + return true +} + +// isRecoverableObjectError reports whether an object-level error is a recoverable +// dry-run rejection: Forbidden or Unauthorized (RBAC not yet propagated) or a +// NoMatch error (dependency CRD not yet registered). errors.As traverses the unwrap +// chain, so this works whether or not the error is further wrapped in a +// validation.DryRunValidationError. +func isRecoverableObjectError(err error) bool { + return apierrors.IsForbidden(err) || + apierrors.IsUnauthorized(err) || + apimeta.IsNoMatchError(err) +} + const maxMessageLength = 1024 // truncateMessage truncates s to at most maxMessageLength runes to fit within the CRD limit. diff --git a/internal/object-controller/status/status_test.go b/internal/object-controller/status/status_test.go index 457b4ee4bc..ba4b0439c4 100644 --- a/internal/object-controller/status/status_test.go +++ b/internal/object-controller/status/status_test.go @@ -1,6 +1,7 @@ package status import ( + "fmt" "strings" "testing" "time" @@ -8,11 +9,15 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" "pkg.package-operator.run/boxcutter/machinery" boxcuttertypes "pkg.package-operator.run/boxcutter/machinery/types" "pkg.package-operator.run/boxcutter/validation" + "sigs.k8s.io/structured-merge-diff/v6/fieldpath" + "sigs.k8s.io/structured-merge-diff/v6/typed" ocv1 "github.com/operator-framework/operator-controller/api/v1" ) @@ -54,6 +59,65 @@ func TestUpdateObservedPhasesForReconcile(t *testing.T) { } UpdateObservedPhasesForReconcile(&cos, result, now) assert.Equal(t, ocv1.PhaseStatusInvalidSpecification, cos.Status.ObservedPhases[0].Status) + assert.Equal(t, int64(1), cos.Status.ObservedPhases[0].ObjectCounts.Total) + }) + + t.Run("forbidden dry-run response reports PreconditionsNotMet not InvalidSpecification", func(t *testing.T) { + // A Forbidden response means RBAC has not yet propagated; the controller retries + // automatically so this must not be reported as a permanent InvalidSpecification. + cos := cosWithPhaseObjects(struct { + name string + objects int + }{"p1", 1}) + forbiddenErr := apierrors.NewForbidden( + schema.GroupResource{Group: "apps", Resource: "deployments"}, + "my-deploy", + fmt.Errorf("not allowed"), + ) + ve := &validation.PhaseValidationError{ + PhaseName: "p1", + Objects: []validation.ObjectValidationError{ + { + ObjectRef: boxcuttertypes.ToObjectRef(obj), + Errors: []error{forbiddenErr}, + }, + }, + } + result := &fakeRevisionResult{ + phases: []machinery.PhaseResult{ + &fakePhaseResult{name: "p1", complete: false, validationError: ve}, + }, + } + UpdateObservedPhasesForReconcile(&cos, result, now) + assert.Equal(t, ocv1.PhaseStatusPreconditionsNotMet, cos.Status.ObservedPhases[0].Status) + }) + + t.Run("permanent dry-run response (Invalid) reports InvalidSpecification not PreconditionsNotMet", func(t *testing.T) { + cos := cosWithPhaseObjects(struct { + name string + objects int + }{"p1", 1}) + invalidErr := apierrors.NewInvalid( + schema.GroupKind{Group: "apps", Kind: "Deployment"}, + "my-deploy", + nil, + ) + ve := &validation.PhaseValidationError{ + PhaseName: "p1", + Objects: []validation.ObjectValidationError{ + { + ObjectRef: boxcuttertypes.ToObjectRef(obj), + Errors: []error{invalidErr}, + }, + }, + } + result := &fakeRevisionResult{ + phases: []machinery.PhaseResult{ + &fakePhaseResult{name: "p1", complete: false, validationError: ve}, + }, + } + UpdateObservedPhasesForReconcile(&cos, result, now) + assert.Equal(t, ocv1.PhaseStatusInvalidSpecification, cos.Status.ObservedPhases[0].Status) }) t.Run("phase validation error when preceding phases are incomplete reports Pending", func(t *testing.T) { @@ -80,6 +144,7 @@ func TestUpdateObservedPhasesForReconcile(t *testing.T) { } UpdateObservedPhasesForReconcile(&cos, result, now) assert.Equal(t, ocv1.PhaseStatusPending, cos.Status.ObservedPhases[1].Status) + assert.Equal(t, int64(1), cos.Status.ObservedPhases[1].ObjectCounts.Total) }) t.Run("per-phase progressed marks phase as Superseded", func(t *testing.T) { @@ -95,8 +160,15 @@ func TestUpdateObservedPhasesForReconcile(t *testing.T) { ) result := &fakeRevisionResult{ phases: []machinery.PhaseResult{ - &fakePhaseResult{name: "p1", complete: false, progressed: true}, - &fakePhaseResult{name: "p2", complete: false, progressed: true}, + &fakePhaseResult{name: "p1", complete: false, progressed: true, objects: []machinery.ObjectResult{ + &fakeObjectResult{obj: obj, complete: false, action: machinery.ActionProgressed}, + &fakeObjectResult{obj: obj, complete: false, action: machinery.ActionProgressed}, + }}, + &fakePhaseResult{name: "p2", complete: false, progressed: true, objects: []machinery.ObjectResult{ + &fakeObjectResult{obj: obj, complete: false, action: machinery.ActionProgressed}, + &fakeObjectResult{obj: obj, complete: false, action: machinery.ActionProgressed}, + &fakeObjectResult{obj: obj, complete: false, action: machinery.ActionProgressed}, + }}, }, } UpdateObservedPhasesForReconcile(&cos, result, now) @@ -107,6 +179,22 @@ func TestUpdateObservedPhasesForReconcile(t *testing.T) { assert.Equal(t, int64(3), cos.Status.ObservedPhases[1].ObjectCounts.Total) }) + t.Run("empty phase with HasProgressed vacuously true does not report Superseded", func(t *testing.T) { + // boxcutter's HasProgressed() returns true vacuously for a 0-object phase (0 == 0). + // The len > 0 guard must prevent this from being reported as Superseded. + cos := cosWithPhaseObjects(struct { + name string + objects int + }{"p1", 0}) + result := &fakeRevisionResult{ + phases: []machinery.PhaseResult{ + &fakePhaseResult{name: "p1", complete: true, progressed: true}, + }, + } + UpdateObservedPhasesForReconcile(&cos, result, now) + assert.Equal(t, ocv1.PhaseStatusReady, cos.Status.ObservedPhases[0].Status) + }) + t.Run("complete phase with any ActionProgressed object reports Superseded not Ready", func(t *testing.T) { // HasProgressed()==false but the phase is complete and one object has been claimed // by a newer revision. The phase must show Superseded, not Ready, so the caller @@ -204,7 +292,7 @@ func TestUpdateObservedPhasesForReconcile(t *testing.T) { assert.Equal(t, ocv1.ObjectCounts{Total: 3, Present: 3, Synced: 3}, cos.Status.ObservedPhases[0].ObjectCounts) }) - t.Run("paused write actions do not increment Synced, keeping phase Reconciling", func(t *testing.T) { + t.Run("all-paused phase reports Pending with write actions not counted as Synced", func(t *testing.T) { // ActionUpdated and ActionRecovered on paused objects do not count as Synced; // paused ActionCreated counts as neither Present nor Synced. cos := cosWithPhaseObjects(struct { @@ -255,6 +343,100 @@ func TestUpdateObservedPhasesForReconcile(t *testing.T) { require.Len(t, cos.Status.ObservedPhases, 2) assert.Equal(t, ocv1.PhaseStatusPending, cos.Status.ObservedPhases[1].Status) assert.Equal(t, ocv1.ObjectCounts{Total: 3, Present: 2, Synced: 1, Ready: 1}, cos.Status.ObservedPhases[1].ObjectCounts) + assert.Len(t, cos.Status.ObservedPhases[1].ObjectDetails, 2) + }) + + t.Run("Ready phase has no objectDetails", func(t *testing.T) { + cos := cosWithPhaseObjects(struct { + name string + objects int + }{"p1", 2}) + result := &fakeRevisionResult{ + phases: []machinery.PhaseResult{ + &fakePhaseResult{name: "p1", complete: true, objects: []machinery.ObjectResult{ + &fakeObjectResult{obj: obj, complete: true, action: machinery.ActionIdle}, + &fakeObjectResult{obj: obj, complete: true, action: machinery.ActionIdle}, + }}, + }, + } + UpdateObservedPhasesForReconcile(&cos, result, now) + assert.Equal(t, ocv1.PhaseStatusReady, cos.Status.ObservedPhases[0].Status) + assert.Empty(t, cos.Status.ObservedPhases[0].ObjectDetails) + }) + + t.Run("Superseded phase has no objectDetails", func(t *testing.T) { + cos := cosWithPhaseObjects(struct { + name string + objects int + }{"p1", 2}) + result := &fakeRevisionResult{ + phases: []machinery.PhaseResult{ + &fakePhaseResult{name: "p1", complete: false, progressed: true, objects: []machinery.ObjectResult{ + &fakeObjectResult{obj: obj, complete: false, action: machinery.ActionProgressed}, + &fakeObjectResult{obj: obj, complete: false, action: machinery.ActionProgressed}, + }}, + }, + } + UpdateObservedPhasesForReconcile(&cos, result, now) + assert.Equal(t, ocv1.PhaseStatusSuperseded, cos.Status.ObservedPhases[0].Status) + assert.Empty(t, cos.Status.ObservedPhases[0].ObjectDetails) + }) + + t.Run("Stabilizing phase populates objectDetails", func(t *testing.T) { + cos := cosWithPhaseObjects(struct { + name string + objects int + }{"p1", 2}) + result := &fakeRevisionResult{ + phases: []machinery.PhaseResult{ + &fakePhaseResult{name: "p1", complete: false, objects: []machinery.ObjectResult{ + &fakeObjectResult{obj: obj, complete: true, action: machinery.ActionIdle}, + &fakeObjectResult{obj: obj, complete: false, action: machinery.ActionIdle}, + }}, + }, + } + UpdateObservedPhasesForReconcile(&cos, result, now) + assert.Equal(t, ocv1.PhaseStatusStabilizing, cos.Status.ObservedPhases[0].Status) + assert.Len(t, cos.Status.ObservedPhases[0].ObjectDetails, 1) + }) + + t.Run("Reconciling phase populates objectDetails", func(t *testing.T) { + // ActionProgressed counts as Present but not Synced, so isSynced=false and isPaused=false → Reconciling. + cos := cosWithPhaseObjects(struct { + name string + objects int + }{"p1", 2}) + result := &fakeRevisionResult{ + phases: []machinery.PhaseResult{ + &fakePhaseResult{name: "p1", complete: false, objects: []machinery.ObjectResult{ + &fakeObjectResult{obj: obj, complete: false, action: machinery.ActionIdle}, + &fakeObjectResult{obj: obj, complete: false, action: machinery.ActionProgressed}, + }}, + }, + } + UpdateObservedPhasesForReconcile(&cos, result, now) + assert.Equal(t, ocv1.PhaseStatusReconciling, cos.Status.ObservedPhases[0].Status) + assert.Len(t, cos.Status.ObservedPhases[0].ObjectDetails, 1) + }) + + t.Run("ActionProgressed objects are excluded from objectDetails", func(t *testing.T) { + // ActionProgressed means the object has been claimed by a newer revision; + // it is not a failure condition and must not appear in objectDetails. + cos := cosWithPhaseObjects(struct { + name string + objects int + }{"p1", 2}) + result := &fakeRevisionResult{ + phases: []machinery.PhaseResult{ + &fakePhaseResult{name: "p1", complete: false, objects: []machinery.ObjectResult{ + &fakeObjectResult{obj: obj, complete: false, action: machinery.ActionProgressed}, + &fakeObjectResult{obj: obj, complete: false, action: machinery.ActionCreated}, + }}, + }, + } + UpdateObservedPhasesForReconcile(&cos, result, now) + require.Len(t, cos.Status.ObservedPhases[0].ObjectDetails, 1) + assert.Equal(t, "my-cm", cos.Status.ObservedPhases[0].ObjectDetails[0].Name) }) t.Run("aggregate ObjectCounts sums across phases", func(t *testing.T) { @@ -305,6 +487,35 @@ func TestUpdateObservedPhasesForTeardown(t *testing.T) { assert.Equal(t, ocv1.PhaseStatusArchiving, phase.Status) assert.Equal(t, "removing all phase objects owned by this object set", phase.Message) assert.Equal(t, ocv1.ObjectCounts{Total: 3, Present: 2}, phase.ObjectCounts) + assert.Len(t, phase.ObjectDetails, 2) + }) + + t.Run("objectDetails contain awaiting deletion with correct object identity", func(t *testing.T) { + waitingObj := &unstructured.Unstructured{} + waitingObj.SetGroupVersionKind(corev1.SchemeGroupVersion.WithKind("ConfigMap")) + waitingObj.SetName("my-cm") + waitingObj.SetNamespace("default") + ref := boxcuttertypes.ToObjectRef(waitingObj) + + cos := cosWithPhaseObjects(struct { + name string + objects int + }{"p1", 1}) + result := &fakeTeardownResult{ + active: "p1", + phases: []machinery.PhaseTeardownResult{ + &fakePhaseTeardownResult{name: "p1", waiting: []boxcuttertypes.ObjectRef{ref}}, + }, + } + UpdateObservedPhasesForTeardown(&cos, result, now) + require.Len(t, cos.Status.ObservedPhases[0].ObjectDetails, 1) + detail := cos.Status.ObservedPhases[0].ObjectDetails[0] + assert.Equal(t, corev1.SchemeGroupVersion.Group, detail.Group) + assert.Equal(t, corev1.SchemeGroupVersion.Version, detail.Version) + assert.Equal(t, "ConfigMap", detail.Kind) + assert.Equal(t, "default", detail.Namespace) + assert.Equal(t, "my-cm", detail.Name) + assert.Equal(t, []string{"awaiting deletion"}, detail.Messages) }) t.Run("gone phase gets Archived status with message", func(t *testing.T) { @@ -323,6 +534,7 @@ func TestUpdateObservedPhasesForTeardown(t *testing.T) { phase := cos.Status.ObservedPhases[0] assert.Equal(t, ocv1.PhaseStatusArchived, phase.Status) assert.Equal(t, ocv1.ObjectCounts{Total: 5}, phase.ObjectCounts) + assert.Empty(t, phase.ObjectDetails) }) t.Run("multi-phase teardown: status, messages, and aggregate counts", func(t *testing.T) { @@ -358,15 +570,18 @@ func TestUpdateObservedPhasesForTeardown(t *testing.T) { assert.Equal(t, ocv1.PhaseStatusPending, phases[0].Status) assert.Equal(t, "waiting for later phases to complete teardown", phases[0].Message) assert.Equal(t, ocv1.ObjectCounts{Total: 2, Present: 2}, phases[0].ObjectCounts) + assert.Len(t, phases[0].ObjectDetails, 2) // p2 — active teardown assert.Equal(t, ocv1.PhaseStatusArchiving, phases[1].Status) assert.Equal(t, "removing all phase objects owned by this object set", phases[1].Message) assert.Equal(t, ocv1.ObjectCounts{Total: 3, Present: 2}, phases[1].ObjectCounts) + assert.Len(t, phases[1].ObjectDetails, 2) - // p3 — complete (in GonePhaseNames) + // p3 — complete (in GonePhaseNames), no waiting objects assert.Equal(t, ocv1.PhaseStatusArchived, phases[2].Status) assert.Equal(t, ocv1.ObjectCounts{Total: 4}, phases[2].ObjectCounts) + assert.Empty(t, phases[2].ObjectDetails) // aggregate assert.Equal(t, ocv1.ObjectCounts{Total: 9, Present: 4}, cos.Status.ObjectCounts) @@ -531,6 +746,246 @@ func TestTruncateMessage(t *testing.T) { }) } +func TestObjectStatus(t *testing.T) { + obj := &unstructured.Unstructured{} + obj.SetGroupVersionKind(corev1.SchemeGroupVersion.WithKind("ConfigMap")) + obj.SetName("my-cm") + obj.SetNamespace("default") + + t.Run("populates object identity from GVK and object ref", func(t *testing.T) { + result := objectStatus(&fakeObjectResult{obj: obj, probes: boxcuttertypes.ProbeResultContainer{}}) + assert.Equal(t, corev1.SchemeGroupVersion.Group, result.Group) + assert.Equal(t, corev1.SchemeGroupVersion.Version, result.Version) + assert.Equal(t, "ConfigMap", result.Kind) + assert.Equal(t, "default", result.Namespace) + assert.Equal(t, "my-cm", result.Name) + }) + + t.Run("collision returns ownership collision message", func(t *testing.T) { + result := objectStatus(&fakeCollisionResult{ + fakeObjectResult: fakeObjectResult{obj: obj, probes: boxcuttertypes.ProbeResultContainer{}}, + }) + assert.Equal(t, []string{"object ownership collision"}, result.Messages) + }) + + t.Run("failing probe with message", func(t *testing.T) { + result := objectStatus(&fakeObjectResult{ + obj: obj, + probes: boxcuttertypes.ProbeResultContainer{ + "readiness": {Status: boxcuttertypes.ProbeStatusFalse, Messages: []string{"not ready"}}, + }, + }) + assert.Equal(t, []string{"readiness: not ready"}, result.Messages) + }) + + t.Run("failing probe without message shows status", func(t *testing.T) { + result := objectStatus(&fakeObjectResult{ + obj: obj, + probes: boxcuttertypes.ProbeResultContainer{ + "readiness": {Status: boxcuttertypes.ProbeStatusFalse}, + }, + }) + assert.Equal(t, []string{"readiness: False"}, result.Messages) + }) + + t.Run("passing probe is omitted and falls back to not yet complete", func(t *testing.T) { + result := objectStatus(&fakeObjectResult{ + obj: obj, + probes: boxcuttertypes.ProbeResultContainer{ + "readiness": {Status: boxcuttertypes.ProbeStatusTrue}, + }, + }) + assert.Equal(t, []string{"not yet complete"}, result.Messages) + }) + + t.Run("paused non-idle delegates to pausedObjectMessages", func(t *testing.T) { + result := objectStatus(&fakeObjectResult{ + obj: obj, + paused: true, + action: machinery.ActionCreated, + probes: boxcuttertypes.ProbeResultContainer{}, + }) + assert.Equal(t, []string{"object does not exist"}, result.Messages) + }) + + t.Run("no issues returns not yet complete", func(t *testing.T) { + result := objectStatus(&fakeObjectResult{ + obj: obj, + action: machinery.ActionIdle, + probes: boxcuttertypes.ProbeResultContainer{}, + }) + assert.Equal(t, []string{"not yet complete"}, result.Messages) + }) + + t.Run("more than 18 messages are cut to 17 plus a truncation notice", func(t *testing.T) { + probes := boxcuttertypes.ProbeResultContainer{} + for i := range 20 { + probes[fmt.Sprintf("probe-%02d", i)] = boxcuttertypes.ProbeResult{ + Status: boxcuttertypes.ProbeStatusFalse, + Messages: []string{"failed"}, + } + } + result := objectStatus(&fakeObjectResult{obj: obj, probes: probes}) + require.Len(t, result.Messages, 18) + assert.Equal(t, "additional objectDetails messages were removed for size constraints", result.Messages[17]) + }) +} + +func TestPausedObjectMessages(t *testing.T) { + obj := &unstructured.Unstructured{} + obj.SetGroupVersionKind(corev1.SchemeGroupVersion.WithKind("ConfigMap")) + obj.SetName("my-cm") + + t.Run("ActionCreated returns object does not exist", func(t *testing.T) { + msgs := pausedObjectMessages(&fakeObjectResult{obj: obj, paused: true, action: machinery.ActionCreated}) + assert.Equal(t, []string{"object does not exist"}, msgs) + }) + + t.Run("ActionCollision without compare result returns ownership collision", func(t *testing.T) { + msgs := pausedObjectMessages(&fakeObjectResult{obj: obj, paused: true, action: machinery.ActionCollision}) + assert.Equal(t, []string{"object ownership collision"}, msgs) + }) + + t.Run("ActionCollision with compare result includes diff details", func(t *testing.T) { + msgs := pausedObjectMessages(&fakeCompareObjectResult{ + fakeObjectResult: fakeObjectResult{obj: obj, paused: true, action: machinery.ActionCollision}, + compareResult: machinery.CompareResult{ + Comparison: &typed.Comparison{ + Modified: fieldpath.NewSet(fieldpath.MakePathOrDie("metadata", "annotations")), + }, + }, + }) + require.Len(t, msgs, 1) + assert.Contains(t, msgs[0], "object ownership collision") + assert.Contains(t, msgs[0], " - modified: .metadata.annotations") + }) + + t.Run("ActionRecovered without compare result", func(t *testing.T) { + msgs := pausedObjectMessages(&fakeObjectResult{obj: obj, paused: true, action: machinery.ActionRecovered}) + assert.Equal(t, []string{"object was modified by another actor"}, msgs) + }) + + t.Run("ActionRecovered with compare result", func(t *testing.T) { + msgs := pausedObjectMessages(&fakeCompareObjectResult{ + fakeObjectResult: fakeObjectResult{obj: obj, paused: true, action: machinery.ActionRecovered}, + compareResult: machinery.CompareResult{ + Comparison: &typed.Comparison{ + Modified: fieldpath.NewSet(fieldpath.MakePathOrDie("data", "key")), + }, + }, + }) + require.Len(t, msgs, 1) + assert.Contains(t, msgs[0], "object was modified by another actor") + assert.Contains(t, msgs[0], " - modified: .data.key") + }) + + t.Run("default action without compare result", func(t *testing.T) { + msgs := pausedObjectMessages(&fakeObjectResult{obj: obj, paused: true, action: machinery.ActionUpdated}) + assert.Equal(t, []string{"object content has changed"}, msgs) + }) + + t.Run("default action with compare result", func(t *testing.T) { + msgs := pausedObjectMessages(&fakeCompareObjectResult{ + fakeObjectResult: fakeObjectResult{obj: obj, paused: true, action: machinery.ActionUpdated}, + compareResult: machinery.CompareResult{ + Comparison: &typed.Comparison{ + Added: fieldpath.NewSet(fieldpath.MakePathOrDie("data", "new-key")), + }, + }, + }) + require.Len(t, msgs, 1) + assert.Contains(t, msgs[0], "object content has changed") + assert.Contains(t, msgs[0], " - added: .data.new-key") + }) + + t.Run("long summary is truncated to maxMessageLength", func(t *testing.T) { + modified := fieldpath.NewSet() + for i := range 200 { + modified.Insert(fieldpath.MakePathOrDie("data", strings.Repeat("k", i+1))) + } + msgs := pausedObjectMessages(&fakeCompareObjectResult{ + fakeObjectResult: fakeObjectResult{obj: obj, paused: true, action: machinery.ActionRecovered}, + compareResult: machinery.CompareResult{ + Comparison: &typed.Comparison{Modified: modified}, + }, + }) + require.Len(t, msgs, 1) + assert.LessOrEqual(t, len([]rune(msgs[0])), maxMessageLength) + }) +} + +func TestCompareDetails(t *testing.T) { + obj := &unstructured.Unstructured{} + obj.SetGroupVersionKind(corev1.SchemeGroupVersion.WithKind("ConfigMap")) + obj.SetName("my-cm") + + t.Run("object without compareResulter returns nil", func(t *testing.T) { + result := compareDetails(&fakeObjectResult{obj: obj}) + assert.Nil(t, result) + }) + + t.Run("nil Comparison returns nil", func(t *testing.T) { + result := compareDetails(&fakeCompareObjectResult{ + fakeObjectResult: fakeObjectResult{obj: obj}, + compareResult: machinery.CompareResult{Comparison: nil}, + }) + assert.Nil(t, result) + }) + + t.Run("added fields only", func(t *testing.T) { + result := compareDetails(&fakeCompareObjectResult{ + fakeObjectResult: fakeObjectResult{obj: obj}, + compareResult: machinery.CompareResult{ + Comparison: &typed.Comparison{ + Added: fieldpath.NewSet(fieldpath.MakePathOrDie("data", "foo")), + }, + }, + }) + assert.Equal(t, []string{" - added: .data.foo"}, result) + }) + + t.Run("modified fields only", func(t *testing.T) { + result := compareDetails(&fakeCompareObjectResult{ + fakeObjectResult: fakeObjectResult{obj: obj}, + compareResult: machinery.CompareResult{ + Comparison: &typed.Comparison{ + Modified: fieldpath.NewSet(fieldpath.MakePathOrDie("data", "bar")), + }, + }, + }) + assert.Equal(t, []string{" - modified: .data.bar"}, result) + }) + + t.Run("removed fields only", func(t *testing.T) { + result := compareDetails(&fakeCompareObjectResult{ + fakeObjectResult: fakeObjectResult{obj: obj}, + compareResult: machinery.CompareResult{ + Comparison: &typed.Comparison{ + Removed: fieldpath.NewSet(fieldpath.MakePathOrDie("data", "old")), + }, + }, + }) + assert.Equal(t, []string{" - removed: .data.old"}, result) + }) + + t.Run("all three field types", func(t *testing.T) { + result := compareDetails(&fakeCompareObjectResult{ + fakeObjectResult: fakeObjectResult{obj: obj}, + compareResult: machinery.CompareResult{ + Comparison: &typed.Comparison{ + Added: fieldpath.NewSet(fieldpath.MakePathOrDie("data", "new")), + Modified: fieldpath.NewSet(fieldpath.MakePathOrDie("data", "changed")), + Removed: fieldpath.NewSet(fieldpath.MakePathOrDie("data", "gone")), + }, + }, + }) + require.Len(t, result, 3) + assert.Equal(t, " - added: .data.new", result[0]) + assert.Equal(t, " - modified: .data.changed", result[1]) + assert.Equal(t, " - removed: .data.gone", result[2]) + }) +} + // Test fakes type fakeRevisionResult struct { @@ -588,6 +1043,13 @@ type fakeCollisionResult struct { func (f *fakeCollisionResult) Action() machinery.Action { return machinery.ActionCollision } +type fakeCompareObjectResult struct { + fakeObjectResult + compareResult machinery.CompareResult +} + +func (f *fakeCompareObjectResult) CompareResult() machinery.CompareResult { return f.compareResult } + type fakeTeardownResult struct { complete bool active string diff --git a/manifests/experimental-e2e.yaml b/manifests/experimental-e2e.yaml index 5d9e9ac1a0..941c81db1b 100644 --- a/manifests/experimental-e2e.yaml +++ b/manifests/experimental-e2e.yaml @@ -2774,13 +2774,82 @@ spec: - synced - total type: object + objectDetails: + description: |- + objectDetails lists objects in this phase that are not yet + complete. For Reconciling, Pending, and Stabilizing phases, this + includes probe failures, collisions, creation/update errors, and + any other condition preventing completion. For Archiving phases, + this lists objects still awaiting deletion. Each entry identifies + the object and carries failure messages. Empty when status is + Ready, Superseded, Archived, or Unknown. The list may contain at + most 50 entries, matching the maximum number of objects per phase. + items: + description: ObjectStatus identifies a managed object and + its failure messages. + properties: + group: + description: |- + group is the API group of the object (empty string for core + resources). At most 253 characters (DNS subdomain max). + maxLength: 253 + type: string + kind: + description: |- + kind is the kind of the object. Must be between 1 and 63 + characters. + maxLength: 63 + minLength: 1 + type: string + messages: + description: |- + messages lists the failure reasons for this object. The maximum + of 18 entries accounts for up to 16 assertion probe failures, + one collision message, and one message to note truncation if necessary. + Each message is at most 1024 characters; longer messages are truncated + by the controller. + items: + maxLength: 1024 + type: string + maxItems: 18 + type: array + x-kubernetes-list-type: atomic + name: + description: |- + name is the name of the object. Must be between 1 and 253 + characters (DNS subdomain max). + maxLength: 253 + minLength: 1 + type: string + namespace: + description: |- + namespace is the namespace of the object. Empty for cluster-scoped + resources. At most 253 characters (DNS subdomain max). + maxLength: 253 + type: string + version: + description: |- + version is the API version of the object. Must be between 1 and + 63 characters. + maxLength: 63 + minLength: 1 + type: string + required: + - kind + - name + - version + type: object + maxItems: 50 + type: array + x-kubernetes-list-type: atomic status: description: |- status is the current state of this phase in the rollout. Must be one of - InvalidSpecification, Pending, Reconciling, + InvalidSpecification, PreconditionsNotMet, Pending, Reconciling, Stabilizing, Ready, Unknown, Superseded, Archiving, or Archived. enum: - InvalidSpecification + - PreconditionsNotMet - Pending - Reconciling - Stabilizing diff --git a/manifests/experimental.yaml b/manifests/experimental.yaml index 10afe2d5c5..1f56f63d51 100644 --- a/manifests/experimental.yaml +++ b/manifests/experimental.yaml @@ -2735,13 +2735,82 @@ spec: - synced - total type: object + objectDetails: + description: |- + objectDetails lists objects in this phase that are not yet + complete. For Reconciling, Pending, and Stabilizing phases, this + includes probe failures, collisions, creation/update errors, and + any other condition preventing completion. For Archiving phases, + this lists objects still awaiting deletion. Each entry identifies + the object and carries failure messages. Empty when status is + Ready, Superseded, Archived, or Unknown. The list may contain at + most 50 entries, matching the maximum number of objects per phase. + items: + description: ObjectStatus identifies a managed object and + its failure messages. + properties: + group: + description: |- + group is the API group of the object (empty string for core + resources). At most 253 characters (DNS subdomain max). + maxLength: 253 + type: string + kind: + description: |- + kind is the kind of the object. Must be between 1 and 63 + characters. + maxLength: 63 + minLength: 1 + type: string + messages: + description: |- + messages lists the failure reasons for this object. The maximum + of 18 entries accounts for up to 16 assertion probe failures, + one collision message, and one message to note truncation if necessary. + Each message is at most 1024 characters; longer messages are truncated + by the controller. + items: + maxLength: 1024 + type: string + maxItems: 18 + type: array + x-kubernetes-list-type: atomic + name: + description: |- + name is the name of the object. Must be between 1 and 253 + characters (DNS subdomain max). + maxLength: 253 + minLength: 1 + type: string + namespace: + description: |- + namespace is the namespace of the object. Empty for cluster-scoped + resources. At most 253 characters (DNS subdomain max). + maxLength: 253 + type: string + version: + description: |- + version is the API version of the object. Must be between 1 and + 63 characters. + maxLength: 63 + minLength: 1 + type: string + required: + - kind + - name + - version + type: object + maxItems: 50 + type: array + x-kubernetes-list-type: atomic status: description: |- status is the current state of this phase in the rollout. Must be one of - InvalidSpecification, Pending, Reconciling, + InvalidSpecification, PreconditionsNotMet, Pending, Reconciling, Stabilizing, Ready, Unknown, Superseded, Archiving, or Archived. enum: - InvalidSpecification + - PreconditionsNotMet - Pending - Reconciling - Stabilizing