diff --git a/api/v1/clusterobjectdeployment_types.go b/api/v1/clusterobjectdeployment_types.go
index 0e2cea864d..f653b9e767 100644
--- a/api/v1/clusterobjectdeployment_types.go
+++ b/api/v1/clusterobjectdeployment_types.go
@@ -36,6 +36,7 @@ const (
ClusterObjectDeploymentReasonRevisionPreconditionsNotMet = "RevisionPreconditionsNotMet"
ClusterObjectDeploymentReasonRevisionObjectCollision = "RevisionObjectCollision"
ClusterObjectDeploymentReasonRevisionInvalidSpecification = "RevisionInvalidSpecification"
+ ClusterObjectDeploymentReasonOwnerConflict = "OwnerConflict"
// Available condition reasons.
ClusterObjectDeploymentReasonAvailable = "Available"
@@ -152,6 +153,10 @@ type ClusterObjectDeploymentStatus struct {
// Progressing is False with reason InvalidSpecification when spec.template.metadata
// contains labels or annotations that are not valid for the resulting
// ClusterObjectSet; no new revision is created until the metadata is corrected.
+ // Progressing is False with reason OwnerConflict when the highest active,
+ // non-deleting revision in the deployment's group is controlled by another
+ // controller. No new revision is created while the conflict persists, even if
+ // an owned revision has completed or exceeded its progress deadline.
//
// While a rollout is incomplete, Progressing also reflects why the latest (head
// active) ClusterObjectSet revision is not yet Ready. It is True with reason
@@ -165,10 +170,12 @@ type ClusterObjectDeploymentStatus struct {
// named ClusterObjectSet revision for the specific cause.
//
// Available is True with reason Available when at least one active (non-archived)
- // ClusterObjectSet revision is Ready, so the deployment remains available across
+ // owned ClusterObjectSet revision is Ready, so the deployment remains available across
// rollouts while a previous revision is still Ready. It is False with reason
// Unavailable when active revisions exist but none are Ready yet, or with reason
- // NoActiveRevision when no active revision exists.
+ // NoActiveRevision when no owned active revision exists. Revisions without a
+ // controller owner, or controlled by a same-named deployment with an older UID,
+ // are adopted before status is calculated. Foreign-owned revisions are excluded.
// +optional
// +listType=map
// +listMapKey=type
diff --git a/applyconfigurations/api/v1/clusterobjectdeploymentstatus.go b/applyconfigurations/api/v1/clusterobjectdeploymentstatus.go
index 092520dcac..9f5aa8f732 100644
--- a/applyconfigurations/api/v1/clusterobjectdeploymentstatus.go
+++ b/applyconfigurations/api/v1/clusterobjectdeploymentstatus.go
@@ -37,6 +37,10 @@ type ClusterObjectDeploymentStatusApplyConfiguration struct {
// Progressing is False with reason InvalidSpecification when spec.template.metadata
// contains labels or annotations that are not valid for the resulting
// ClusterObjectSet; no new revision is created until the metadata is corrected.
+ // Progressing is False with reason OwnerConflict when the highest active,
+ // non-deleting revision in the deployment's group is controlled by another
+ // controller. No new revision is created while the conflict persists, even if
+ // an owned revision has completed or exceeded its progress deadline.
//
// While a rollout is incomplete, Progressing also reflects why the latest (head
// active) ClusterObjectSet revision is not yet Ready. It is True with reason
@@ -50,10 +54,12 @@ type ClusterObjectDeploymentStatusApplyConfiguration struct {
// named ClusterObjectSet revision for the specific cause.
//
// Available is True with reason Available when at least one active (non-archived)
- // ClusterObjectSet revision is Ready, so the deployment remains available across
+ // owned ClusterObjectSet revision is Ready, so the deployment remains available across
// rollouts while a previous revision is still Ready. It is False with reason
// Unavailable when active revisions exist but none are Ready yet, or with reason
- // NoActiveRevision when no active revision exists.
+ // NoActiveRevision when no owned active revision exists. Revisions without a
+ // controller owner, or controlled by a same-named deployment with an older UID,
+ // are adopted before status is calculated. Foreign-owned revisions are excluded.
Conditions []metav1.ConditionApplyConfiguration `json:"conditions,omitempty"`
}
diff --git a/docs/api-reference/olmv1-api-reference.md b/docs/api-reference/olmv1-api-reference.md
index 0ce3a4ffdc..04859e560d 100644
--- a/docs/api-reference/olmv1-api-reference.md
+++ b/docs/api-reference/olmv1-api-reference.md
@@ -458,7 +458,7 @@ _Appears in:_
| Field | Description | Default | Validation |
| --- | --- | --- | --- |
-| `conditions` _[Condition](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.31/#condition-v1-meta) array_ | conditions describes rollout progress and availability for the observed generation.
Progressing is True with reason NewClusterObjectSetProgressing while the desired
rollout is incomplete, or False with reason NewClusterObjectSetProgressed after
completion. Completion remains recorded even if the completed revision later
becomes unhealthy. Progressing is False with reason ProgressDeadlineExceeded when
the latest revision does not roll out within spec.template.spec.progressDeadlineMinutes.
Progressing is False with reason InvalidSpecification when spec.template.metadata
contains labels or annotations that are not valid for the resulting
ClusterObjectSet; no new revision is created until the metadata is corrected.
While a rollout is incomplete, Progressing also reflects why the latest (head
active) ClusterObjectSet revision is not yet Ready. It is True with reason
RevisionPreconditionsNotMet when the revision's preconditions are not yet met
(for example RBAC or a dependency CRD that has not propagated); the controller
keeps retrying. It is False with reason RevisionObjectCollision when the
revision's objects are owned by another controller or already exist, which the
user must resolve. It is False with reason RevisionInvalidSpecification when the
revision's own objects are invalid; this is distinct from InvalidSpecification
above, which refers to the deployment's spec.template.metadata. Inspect the
named ClusterObjectSet revision for the specific cause.
Available is True with reason Available when at least one active (non-archived)
ClusterObjectSet revision is Ready, so the deployment remains available across
rollouts while a previous revision is still Ready. It is False with reason
Unavailable when active revisions exist but none are Ready yet, or with reason
NoActiveRevision when no active revision exists. | | Optional: \{\}
|
+| `conditions` _[Condition](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.31/#condition-v1-meta) array_ | conditions describes rollout progress and availability for the observed generation.
Progressing is True with reason NewClusterObjectSetProgressing while the desired
rollout is incomplete, or False with reason NewClusterObjectSetProgressed after
completion. Completion remains recorded even if the completed revision later
becomes unhealthy. Progressing is False with reason ProgressDeadlineExceeded when
the latest revision does not roll out within spec.template.spec.progressDeadlineMinutes.
Progressing is False with reason InvalidSpecification when spec.template.metadata
contains labels or annotations that are not valid for the resulting
ClusterObjectSet; no new revision is created until the metadata is corrected.
Progressing is False with reason OwnerConflict when the highest active,
non-deleting revision in the deployment's group is controlled by another
controller. No new revision is created while the conflict persists, even if
an owned revision has completed or exceeded its progress deadline.
While a rollout is incomplete, Progressing also reflects why the latest (head
active) ClusterObjectSet revision is not yet Ready. It is True with reason
RevisionPreconditionsNotMet when the revision's preconditions are not yet met
(for example RBAC or a dependency CRD that has not propagated); the controller
keeps retrying. It is False with reason RevisionObjectCollision when the
revision's objects are owned by another controller or already exist, which the
user must resolve. It is False with reason RevisionInvalidSpecification when the
revision's own objects are invalid; this is distinct from InvalidSpecification
above, which refers to the deployment's spec.template.metadata. Inspect the
named ClusterObjectSet revision for the specific cause.
Available is True with reason Available when at least one active (non-archived)
owned ClusterObjectSet revision is Ready, so the deployment remains available across
rollouts while a previous revision is still Ready. It is False with reason
Unavailable when active revisions exist but none are Ready yet, or with reason
NoActiveRevision when no owned active revision exists. Revisions without a
controller owner, or controlled by a same-named deployment with an older UID,
are adopted before status is calculated. Foreign-owned revisions are excluded. | | Optional: \{\}
|
#### ClusterObjectDeploymentTemplate
diff --git a/helm/olmv1/base/object-controller/crd/experimental/olm.operatorframework.io_clusterobjectdeployments.yaml b/helm/olmv1/base/object-controller/crd/experimental/olm.operatorframework.io_clusterobjectdeployments.yaml
index 48294b7a35..41cfe95195 100644
--- a/helm/olmv1/base/object-controller/crd/experimental/olm.operatorframework.io_clusterobjectdeployments.yaml
+++ b/helm/olmv1/base/object-controller/crd/experimental/olm.operatorframework.io_clusterobjectdeployments.yaml
@@ -550,6 +550,10 @@ spec:
Progressing is False with reason InvalidSpecification when spec.template.metadata
contains labels or annotations that are not valid for the resulting
ClusterObjectSet; no new revision is created until the metadata is corrected.
+ Progressing is False with reason OwnerConflict when the highest active,
+ non-deleting revision in the deployment's group is controlled by another
+ controller. No new revision is created while the conflict persists, even if
+ an owned revision has completed or exceeded its progress deadline.
While a rollout is incomplete, Progressing also reflects why the latest (head
active) ClusterObjectSet revision is not yet Ready. It is True with reason
@@ -563,10 +567,12 @@ spec:
named ClusterObjectSet revision for the specific cause.
Available is True with reason Available when at least one active (non-archived)
- ClusterObjectSet revision is Ready, so the deployment remains available across
+ owned ClusterObjectSet revision is Ready, so the deployment remains available across
rollouts while a previous revision is still Ready. It is False with reason
Unavailable when active revisions exist but none are Ready yet, or with reason
- NoActiveRevision when no active revision exists.
+ NoActiveRevision when no owned active revision exists. Revisions without a
+ controller owner, or controlled by a same-named deployment with an older UID,
+ are adopted before status is calculated. Foreign-owned revisions are excluded.
items:
description: Condition contains details for one aspect of the current
state of this API Resource.
diff --git a/internal/object-controller/controllers/clusterobjectdeployment_controller.go b/internal/object-controller/controllers/clusterobjectdeployment_controller.go
index ad3caf37b6..4f1b0078ec 100644
--- a/internal/object-controller/controllers/clusterobjectdeployment_controller.go
+++ b/internal/object-controller/controllers/clusterobjectdeployment_controller.go
@@ -19,6 +19,7 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
metav1validation "k8s.io/apimachinery/pkg/apis/meta/v1/validation"
"k8s.io/apimachinery/pkg/runtime"
+ "k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/client-go/util/workqueue"
@@ -96,26 +97,92 @@ func (c *ClusterObjectDeploymentReconciler) reconcile(ctx context.Context, cod *
return ctrl.Result{}, err
}
+ owned, adoptionErr := c.adoptOrphans(ctx, cod, revisions)
+
// Garbage collection is best-effort housekeeping and must not block rollout or
// status reconciliation: a persistently failing prune (e.g. a delete denied by
// an admission policy) would otherwise indefinitely stall deployment of an
// updated template and leave status stale even with a healthy active revision.
// Preserve the error, continue reconciling, and return it joined with the
// rollout outcome so the failure is still surfaced and retried.
- gcErr := c.garbageCollectArchivedRevisions(ctx, cod, revisions)
+ gcErr := c.garbageCollectArchivedRevisions(ctx, cod, owned)
+ setAvailable(cod, owned)
+
+ // Group membership determines revision ordering, while controller ownership
+ // determines management authority. Never create a competing revision above a
+ // foreign-controlled leader, even if an older owned revision has completed.
+ if latest := latestActiveRevision(revisions); latest != nil && hasForeignController(cod, latest) {
+ owner := metav1.GetControllerOf(latest)
+ setProgressing(cod, metav1.ConditionFalse, ocv1.ClusterObjectDeploymentReasonOwnerConflict,
+ fmt.Sprintf("ClusterObjectSet %q (revision %d) is controlled by %s %s %q (UID %q); no higher revision will be created while this owner conflict persists",
+ latest.Name, latest.Spec.Revision, owner.APIVersion, owner.Kind, owner.Name, owner.UID))
+ return ctrl.Result{}, errors.Join(adoptionErr, gcErr)
+ }
+ if adoptionErr != nil {
+ setProgressing(cod, metav1.ConditionTrue, ocv1.ClusterObjectDeploymentReasonNewClusterObjectSetProgressing, "waiting for revisions to be adopted")
+ return ctrl.Result{}, errors.Join(adoptionErr, gcErr)
+ }
- result, rolloutErr := c.reconcileRollout(ctx, cod, revisions)
+ result, rolloutErr := c.reconcileRollout(ctx, cod, owned, revisions)
return result, errors.Join(gcErr, rolloutErr)
}
+// adoptOrphans returns the deployment's owned revisions, adopting non-deleting
+// revisions with no controller or a reference to a previous UID of this COD.
+// Successful adoptions also update the group slice before conflict detection.
+func (c *ClusterObjectDeploymentReconciler) adoptOrphans(ctx context.Context, cod *ocv1.ClusterObjectDeployment, revisions []ocv1.ClusterObjectSet) ([]ocv1.ClusterObjectSet, error) {
+ owned := make([]ocv1.ClusterObjectSet, 0, len(revisions))
+ var errs []error
+ for i := range revisions {
+ rev := &revisions[i]
+ if hasForeignController(cod, rev) {
+ continue
+ }
+ if metav1.IsControlledBy(rev, cod) {
+ owned = append(owned, *rev)
+ continue
+ }
+ if !rev.DeletionTimestamp.IsZero() {
+ continue
+ }
+
+ adopted := rev.DeepCopy()
+ // SetControllerReference replaces a same-group/kind/name reference,
+ // including an older UID, and preserves unrelated owner references.
+ if err := controllerutil.SetControllerReference(cod, adopted, c.Scheme); err != nil {
+ errs = append(errs, fmt.Errorf("setting controller reference on revision %q: %w", rev.Name, err))
+ continue
+ }
+ if err := c.Client.Patch(ctx, adopted, client.MergeFromWithOptions(rev, client.MergeFromWithOptimisticLock{})); err != nil {
+ errs = append(errs, fmt.Errorf("adopting revision %q: %w", rev.Name, err))
+ continue
+ }
+ *rev = *adopted
+ owned = append(owned, *rev)
+ }
+ return owned, errors.Join(errs...)
+}
+
+// hasForeignController distinguishes another controller from an adoptable orphan
+// or a reference to an older UID of a deployment with the same group, kind, and
+// name. API versions within the same group identify the same controller kind.
+func hasForeignController(cod *ocv1.ClusterObjectDeployment, cos *ocv1.ClusterObjectSet) bool {
+ owner := metav1.GetControllerOf(cos)
+ if owner == nil {
+ return false
+ }
+ gv, err := schema.ParseGroupVersion(owner.APIVersion)
+ return err != nil || gv.Group != ocv1.GroupVersion.Group ||
+ owner.Kind != ocv1.ClusterObjectDeploymentKind || owner.Name != cod.Name
+}
+
// reconcileRollout creates a new ClusterObjectSet revision when the template has
// changed (or no active revision exists) and reflects the latest revision's
-// rollout progress in the deployment's Available and Progressing conditions.
-func (c *ClusterObjectDeploymentReconciler) reconcileRollout(ctx context.Context, cod *ocv1.ClusterObjectDeployment, revisions []ocv1.ClusterObjectSet) (ctrl.Result, error) {
+// rollout progress in the deployment's Progressing condition. Rollout decisions
+// use owned revisions; revision allocation uses the entire group.
+func (c *ClusterObjectDeploymentReconciler) reconcileRollout(ctx context.Context, cod *ocv1.ClusterObjectDeployment, owned, group []ocv1.ClusterObjectSet) (ctrl.Result, error) {
hash := templateHash(cod.Spec.Template)
- setAvailable(cod, revisions)
-
// The template metadata is propagated verbatim onto each revision's
// ClusterObjectSet. Invalid labels or annotations would make that revision
// rejected by the API server and stall the rollout with the real cause buried
@@ -125,10 +192,10 @@ func (c *ClusterObjectDeploymentReconciler) reconcileRollout(ctx context.Context
return ctrl.Result{}, nil
}
- latest := latestActiveRevision(revisions)
+ latest := latestActiveRevision(owned)
if latest == nil || latest.Labels[labels.TemplateHashKey] != hash {
var err error
- latest, err = c.createRevision(ctx, cod, revisions, hash)
+ latest, err = c.createRevision(ctx, cod, group, hash)
if err != nil {
setProgressing(cod, metav1.ConditionTrue, ocv1.ClusterObjectDeploymentReasonNewClusterObjectSetProgressing, "waiting for new revision to be created")
return ctrl.Result{}, err
@@ -224,18 +291,8 @@ func (c *ClusterObjectDeploymentReconciler) listRevisions(ctx context.Context, g
// revisions must be sorted ascending by revision number (oldest first), as
// returned by listRevisions.
//
-// NOTE: The candidate revisions come from listRevisions, which selects by
-// spec.group (equal to the deployment name) rather than by this deployment's
-// controller owner reference. This is deliberate and consistent with the rest of
-// OLMv1: sibling membership of a revision family is defined by spec.group, not by
-// ownership (see createRevision's naming/grouping and the ClusterObjectSet
-// controller's own sibling archival, both keyed on spec.group). A consequence is
-// that a ClusterObjectSet sharing this group but created by a different owner
-// would be counted toward the limit and could be deleted here. That is tolerated
-// because the deployment name is the group and is treated as an exclusive
-// namespace for its revisions — nothing else should create ClusterObjectSets in
-// it. If siblinghood is ever redefined in terms of ownership, this function and
-// listRevisions must be updated together to filter by owner reference and UID.
+// The caller supplies only owned (including successfully adopted) revisions.
+// Foreign-owned group members neither count toward the limit nor get pruned.
func (c *ClusterObjectDeploymentReconciler) garbageCollectArchivedRevisions(ctx context.Context, cod *ocv1.ClusterObjectDeployment, revisions []ocv1.ClusterObjectSet) error {
limit := defaultRevisionHistoryLimit
if cod.Spec.RevisionHistoryLimit != nil {
@@ -258,7 +315,9 @@ func (c *ClusterObjectDeploymentReconciler) garbageCollectArchivedRevisions(ctx
// the others on every reconcile.
var errs []error
for i := 0; i < len(archived)-limit; i++ {
- if err := c.Client.Delete(ctx, archived[i]); err != nil && !apierrors.IsNotFound(err) {
+ if err := c.Client.Delete(ctx, archived[i], client.Preconditions{
+ UID: &archived[i].UID, ResourceVersion: &archived[i].ResourceVersion,
+ }); err != nil && !apierrors.IsNotFound(err) {
errs = append(errs, fmt.Errorf("deleting archived revision %q: %w", archived[i].Name, err))
}
}
@@ -277,8 +336,6 @@ func archivedTeardownComplete(rev *ocv1.ClusterObjectSet) bool {
// createRevision builds and creates the next ClusterObjectSet revision owned by
// the deployment. Any create error, including AlreadyExists, is returned.
-//
-// TODO: revisit AlreadyExists handling under the adoption use-case.
func (c *ClusterObjectDeploymentReconciler) createRevision(ctx context.Context, cod *ocv1.ClusterObjectDeployment, revisions []ocv1.ClusterObjectSet, hash string) (*ocv1.ClusterObjectSet, error) {
revision, err := nextRevisionNumber(revisions)
if err != nil {
@@ -328,6 +385,12 @@ func newCODDeadlineAwareRateLimiter(delegate workqueue.TypedRateLimiter[ctrl.Req
if err := reader.List(ctx, &revisions, client.MatchingFields{clusterObjectSetGroupIndex: cod.Name}); err != nil {
return 0, false, err
}
+ if latest := latestActiveRevision(revisions.Items); latest != nil && hasForeignController(cod, latest) {
+ return 0, false, nil
+ }
+ revisions.Items = slices.DeleteFunc(revisions.Items, func(rev ocv1.ClusterObjectSet) bool {
+ return hasForeignController(cod, &rev) || !metav1.IsControlledBy(&rev, cod)
+ })
latest := latestActiveRevision(revisions.Items)
if latest == nil || latest.Labels[labels.TemplateHashKey] != templateHash(cod.Spec.Template) {
// Creating the desired revision is outside the MVP deadline.
@@ -400,15 +463,8 @@ func (c *ClusterObjectDeploymentReconciler) SetupWithManager(mgr ctrl.Manager) e
predicate.ResourceVersionChangedPredicate{},
),
).
- Owns(
- &ocv1.ClusterObjectSet{},
- builder.WithPredicates(
- predicate.ResourceVersionChangedPredicate{},
- ),
- ).
- // A ClusterObjectSet's spec.group matches the name of the ClusterObjectDeployment
- // that owns (or should adopt) it. Owns() only enqueues already-owned revisions, so
- // this watch additionally surfaces not-yet-adopted (orphan) ClusterObjectSets.
+ // Observe every group member, including foreign-owned revisions, so
+ // ownership conflicts and their resolution always trigger reconciliation.
Watches(
&ocv1.ClusterObjectSet{},
handler.EnqueueRequestsFromMapFunc(mapClusterObjectSetToClusterObjectDeployment),
diff --git a/internal/object-controller/controllers/clusterobjectdeployment_controller_test.go b/internal/object-controller/controllers/clusterobjectdeployment_controller_test.go
index 7f798e9239..3945008bcb 100644
--- a/internal/object-controller/controllers/clusterobjectdeployment_controller_test.go
+++ b/internal/object-controller/controllers/clusterobjectdeployment_controller_test.go
@@ -5,6 +5,7 @@ package controllers_test
import (
"context"
"encoding/json"
+ "errors"
"fmt"
"math"
"strings"
@@ -34,7 +35,7 @@ const clusterObjectDeploymentName = "test-cod-1"
func codFixture() *ocv1.ClusterObjectDeployment {
return &ocv1.ClusterObjectDeployment{
- ObjectMeta: metav1.ObjectMeta{Name: clusterObjectDeploymentName},
+ ObjectMeta: metav1.ObjectMeta{Name: clusterObjectDeploymentName, UID: "test-cod-uid"},
Spec: ocv1.ClusterObjectDeploymentSpec{
Template: ocv1.ClusterObjectDeploymentTemplate{
Spec: ocv1.ClusterObjectDeploymentTemplateSpec{
@@ -823,6 +824,7 @@ func Test_ClusterObjectDeploymentReconciler_RevisionBounds(t *testing.T) {
changed bool
wantErr bool
wantCreates int
+ foreign bool
}{
{
name: "allocates final revision after archived foreign-owned history",
@@ -831,6 +833,7 @@ func Test_ClusterObjectDeploymentReconciler_RevisionBounds(t *testing.T) {
changed: false,
wantErr: false,
wantCreates: 1,
+ foreign: true,
},
{
name: "changed template at maximum creates nothing",
@@ -840,6 +843,13 @@ func Test_ClusterObjectDeploymentReconciler_RevisionBounds(t *testing.T) {
wantErr: true,
wantCreates: 0,
},
+ {
+ name: "foreign active maximum reports owner conflict instead of revision exhaustion",
+ latest: ocv1.ClusterObjectSetMaxRevision,
+ lifecycleState: ocv1.ClusterObjectSetLifecycleStateActive,
+ changed: true,
+ foreign: true,
+ },
{
name: "stored maximum int64 cannot overflow",
latest: math.MaxInt64,
@@ -859,6 +869,11 @@ func Test_ClusterObjectDeploymentReconciler_RevisionBounds(t *testing.T) {
} {
t.Run(tc.name, func(t *testing.T) {
cod := codFixture()
+ owner := cod.DeepCopy()
+ if tc.foreign {
+ owner.Name = "other-deployment"
+ owner.UID = "other-uid"
+ }
latest := &ocv1.ClusterObjectSet{
ObjectMeta: metav1.ObjectMeta{
Name: fmt.Sprintf("%s-%d", cod.Name, tc.latest),
@@ -866,12 +881,7 @@ func Test_ClusterObjectDeploymentReconciler_RevisionBounds(t *testing.T) {
labels.TemplateHashKey: hashutil.DeepHashObject(cod.Spec.Template),
},
OwnerReferences: []metav1.OwnerReference{
- *metav1.NewControllerRef(&ocv1.ClusterObjectDeployment{
- ObjectMeta: metav1.ObjectMeta{
- Name: "other-deployment",
- UID: "other-uid",
- },
- }, ocv1.GroupVersion.WithKind(ocv1.ClusterObjectDeploymentKind)),
+ *metav1.NewControllerRef(owner, ocv1.GroupVersion.WithKind(ocv1.ClusterObjectDeploymentKind)),
},
},
Spec: ocv1.ClusterObjectSetSpec{
@@ -902,6 +912,9 @@ func Test_ClusterObjectDeploymentReconciler_RevisionBounds(t *testing.T) {
require.NoError(t, err)
}
require.Equal(t, tc.wantCreates, creates)
+ if tc.foreign && tc.lifecycleState == ocv1.ClusterObjectSetLifecycleStateActive {
+ require.Equal(t, ocv1.ClusterObjectDeploymentReasonOwnerConflict, progressing(t, c).Reason)
+ }
retained, ok := getCOS(t, c, latest.Name)
require.True(t, ok)
require.Equal(t, latest.Spec, retained.Spec)
@@ -913,3 +926,418 @@ func Test_ClusterObjectDeploymentReconciler_RevisionBounds(t *testing.T) {
})
}
}
+
+// deploymentRevision is an owned revision that matches the current template.
+func deploymentRevision(cod *ocv1.ClusterObjectDeployment, revision int64, lifecycle ocv1.ClusterObjectSetLifecycleState) *ocv1.ClusterObjectSet {
+ cos := revisionCOS(revision, lifecycle)
+ cos.OwnerReferences = []metav1.OwnerReference{*metav1.NewControllerRef(cod, ocv1.GroupVersion.WithKind(ocv1.ClusterObjectDeploymentKind))}
+ cos.Labels = map[string]string{labels.TemplateHashKey: hashutil.DeepHashObject(cod.Spec.Template)}
+ cos.Spec.Phases = cod.Spec.Template.DeepCopy().Spec.Phases
+ return cos
+}
+
+func foreignControllerReference() metav1.OwnerReference {
+ return metav1.OwnerReference{
+ APIVersion: ocv1.GroupVersion.String(), Kind: "ClusterExtension",
+ Name: "another-controller", UID: "foreign-uid", Controller: ptr.To(true),
+ }
+}
+
+func Test_ClusterObjectDeploymentReconciler_AdoptsGroupRevisions(t *testing.T) {
+ for _, tc := range []struct {
+ name string
+ owners []metav1.OwnerReference
+ }{
+ {name: "orphan"},
+ {
+ name: "preserves non-controller owner",
+ owners: []metav1.OwnerReference{{
+ APIVersion: ocv1.GroupVersion.String(), Kind: "ClusterExtension",
+ Name: "observer", UID: "observer-uid", Controller: ptr.To(false),
+ }},
+ },
+ {
+ name: "same deployment with an older UID",
+ owners: []metav1.OwnerReference{{
+ APIVersion: ocv1.GroupVersion.String(), Kind: ocv1.ClusterObjectDeploymentKind,
+ Name: clusterObjectDeploymentName, UID: "old-cod-uid", Controller: ptr.To(true),
+ }},
+ },
+ {
+ name: "same deployment through an older API version",
+ owners: []metav1.OwnerReference{{
+ APIVersion: ocv1.GroupVersion.Group + "/v1alpha1", Kind: ocv1.ClusterObjectDeploymentKind,
+ Name: clusterObjectDeploymentName, UID: "old-cod-uid", Controller: ptr.To(true),
+ }},
+ },
+ } {
+ t.Run(tc.name, func(t *testing.T) {
+ cod := codFixture()
+ cos := deploymentRevision(cod, 7, ocv1.ClusterObjectSetLifecycleStateActive)
+ cos.OwnerReferences = tc.owners
+ cos.Annotations = map[string]string{"preserve": "annotation"}
+ cos.Status.CompletedAt = metav1.Now()
+ setCOSReady(cos, metav1.ConditionTrue, ocv1.ClusterObjectSetReasonAllObjectsReady)
+ c := newCODClient(t, cod, cos)
+ before, found := getCOS(t, c, cos.Name)
+ require.True(t, found)
+
+ _, err := reconcileCOD(t, c)
+ require.NoError(t, err)
+ adopted, found := getCOS(t, c, cos.Name)
+ require.True(t, found)
+ require.True(t, metav1.IsControlledBy(adopted, cod))
+ require.Equal(t, cos.Spec, adopted.Spec)
+ require.Equal(t, cos.Labels, adopted.Labels)
+ require.Equal(t, cos.Annotations, adopted.Annotations)
+ require.Equal(t, before.Status, adopted.Status)
+ for _, owner := range tc.owners {
+ if owner.Controller != nil && !*owner.Controller {
+ require.Contains(t, adopted.OwnerReferences, owner)
+ }
+ }
+ require.Equal(t, metav1.ConditionTrue, available(t, c).Status)
+ require.Equal(t, ocv1.ClusterObjectDeploymentReasonNewClusterObjectSetProgressed, progressing(t, c).Reason)
+ var revisions ocv1.ClusterObjectSetList
+ require.NoError(t, c.List(t.Context(), &revisions))
+ require.Len(t, revisions.Items, 1, "adoption must not create a matching revision")
+ })
+ }
+}
+
+func Test_ClusterObjectDeploymentReconciler_OwnerConflict(t *testing.T) {
+ for _, tc := range []struct {
+ name string
+ owned bool
+ ready bool
+ completed bool
+ changed bool
+ owner *metav1.OwnerReference
+ ownedReason string
+ wantAvailable string
+ }{
+ {name: "no owned active revision", wantAvailable: ocv1.ClusterObjectDeploymentReasonNoActiveRevision},
+ {name: "incomplete owned revision", owned: true, wantAvailable: ocv1.ClusterObjectDeploymentReasonUnavailable},
+ {name: "completed owned revision", owned: true, ready: true, completed: true, wantAvailable: ocv1.ClusterObjectDeploymentReasonAvailable},
+ {name: "expired owned deadline", owned: true, ready: true, wantAvailable: ocv1.ClusterObjectDeploymentReasonAvailable},
+ {name: "changed template", owned: true, ready: true, completed: true, changed: true, wantAvailable: ocv1.ClusterObjectDeploymentReasonAvailable},
+ {name: "owned revision preconditions are unmet", owned: true, ownedReason: ocv1.ClusterObjectSetReasonPreconditionsNotMet, wantAvailable: ocv1.ClusterObjectDeploymentReasonUnavailable},
+ {name: "owned revision has an object collision", owned: true, ownedReason: ocv1.ClusterObjectSetReasonObjectCollision, wantAvailable: ocv1.ClusterObjectDeploymentReasonUnavailable},
+ {name: "owned revision has an invalid specification", owned: true, ownedReason: ocv1.ClusterObjectSetReasonInvalidSpecification, wantAvailable: ocv1.ClusterObjectDeploymentReasonUnavailable},
+ {
+ name: "same deployment name with a different controller kind",
+ owner: &metav1.OwnerReference{
+ APIVersion: ocv1.GroupVersion.String(), Kind: "ClusterExtension",
+ Name: clusterObjectDeploymentName, UID: "foreign-uid", Controller: ptr.To(true),
+ },
+ wantAvailable: ocv1.ClusterObjectDeploymentReasonNoActiveRevision,
+ },
+ {
+ name: "same deployment name and kind from a different API group",
+ owner: &metav1.OwnerReference{
+ APIVersion: "other.example.com/v1", Kind: ocv1.ClusterObjectDeploymentKind,
+ Name: clusterObjectDeploymentName, UID: "foreign-uid", Controller: ptr.To(true),
+ },
+ wantAvailable: ocv1.ClusterObjectDeploymentReasonNoActiveRevision,
+ },
+ } {
+ t.Run(tc.name, func(t *testing.T) {
+ cod := codFixture()
+ cod.Generation = 3
+ foreign := deploymentRevision(cod, 3, ocv1.ClusterObjectSetLifecycleStateActive)
+ foreign.OwnerReferences = []metav1.OwnerReference{foreignControllerReference()}
+ if tc.owner != nil {
+ foreign.OwnerReferences = []metav1.OwnerReference{*tc.owner}
+ }
+ foreign.Spec.Phases[0].Objects[0].Object.SetName("foreign-cm")
+ setCOSReady(foreign, metav1.ConditionTrue, ocv1.ClusterObjectSetReasonAllObjectsReady)
+ objects := []client.Object{cod, foreign}
+ if tc.owned {
+ owned := deploymentRevision(cod, 1, ocv1.ClusterObjectSetLifecycleStateActive)
+ owned.CreationTimestamp = metav1.NewTime(time.Now().Add(-time.Hour))
+ setCOSReady(owned, metav1.ConditionFalse, ocv1.ClusterObjectSetReasonObjectsNotReady)
+ if tc.ownedReason != "" {
+ setCOSReady(owned, metav1.ConditionFalse, tc.ownedReason)
+ }
+ if tc.ready {
+ setCOSReady(owned, metav1.ConditionTrue, ocv1.ClusterObjectSetReasonAllObjectsReady)
+ }
+ if tc.completed {
+ owned.Status.CompletedAt = metav1.Now()
+ }
+ objects = append(objects, owned)
+ }
+ if tc.changed {
+ cod.Spec.Template.Spec.CollisionProtection = ocv1.CollisionProtectionNone
+ }
+ creates := 0
+ c := interceptor.NewClient(newCODClient(t, objects...), interceptor.Funcs{
+ Create: func(ctx context.Context, cl client.WithWatch, obj client.Object, opts ...client.CreateOption) error {
+ creates++
+ return cl.Create(ctx, obj, opts...)
+ },
+ })
+ before, found := getCOS(t, c, foreign.Name)
+ require.True(t, found)
+ for range 2 {
+ result, err := reconcileCOD(t, c)
+ require.NoError(t, err)
+ require.Zero(t, result.RequeueAfter)
+ condition := progressing(t, c)
+ require.Equal(t, metav1.ConditionFalse, condition.Status)
+ require.Equal(t, ocv1.ClusterObjectDeploymentReasonOwnerConflict, condition.Reason)
+ require.Equal(t, cod.Generation, condition.ObservedGeneration)
+ owner := metav1.GetControllerOf(foreign)
+ for _, fragment := range []string{foreign.Name, "revision 3", owner.APIVersion, owner.Kind, owner.Name, string(owner.UID), "no higher revision"} {
+ require.Contains(t, condition.Message, fragment)
+ }
+ require.Equal(t, tc.wantAvailable, available(t, c).Reason)
+ }
+ require.Zero(t, creates)
+ retained, found := getCOS(t, c, foreign.Name)
+ require.True(t, found)
+ require.Equal(t, foreign.OwnerReferences, retained.OwnerReferences)
+ require.Equal(t, foreign.Spec, retained.Spec)
+ require.Equal(t, before.Status, retained.Status)
+ })
+ }
+}
+
+func Test_ClusterObjectDeploymentReconciler_OwnerConflictRecovery(t *testing.T) {
+ for _, resolution := range []string{"archived", "orphaned", "old deployment UID", "deleted", "deleting"} {
+ t.Run(resolution, func(t *testing.T) {
+ cod := codFixture()
+ owned := deploymentRevision(cod, 1, ocv1.ClusterObjectSetLifecycleStateActive)
+ setCOSReady(owned, metav1.ConditionTrue, ocv1.ClusterObjectSetReasonAllObjectsReady)
+ foreign := deploymentRevision(cod, 3, ocv1.ClusterObjectSetLifecycleStateActive)
+ foreign.OwnerReferences = []metav1.OwnerReference{foreignControllerReference()}
+ if resolution == "deleting" {
+ foreign.Finalizers = []string{"test/hold"}
+ }
+ c := newCODClient(t, cod, owned, foreign)
+ _, err := reconcileCOD(t, c)
+ require.NoError(t, err)
+ require.Equal(t, ocv1.ClusterObjectDeploymentReasonOwnerConflict, progressing(t, c).Reason)
+
+ require.NoError(t, c.Get(t.Context(), client.ObjectKeyFromObject(cod), cod))
+ cod.Spec.Template.Spec.CollisionProtection = ocv1.CollisionProtectionNone
+ require.NoError(t, c.Update(t.Context(), cod))
+ _, err = reconcileCOD(t, c)
+ require.NoError(t, err)
+ _, found := getCOS(t, c, cod.Name+"-4")
+ require.False(t, found, "template changes must remain blocked during the conflict")
+
+ expectedRevision := int64(4)
+ switch resolution {
+ case "archived":
+ foreign.Spec.LifecycleState = ocv1.ClusterObjectSetLifecycleStateArchived
+ require.NoError(t, c.Update(t.Context(), foreign))
+ case "orphaned":
+ foreign.OwnerReferences = nil
+ require.NoError(t, c.Update(t.Context(), foreign))
+ case "old deployment UID":
+ stale := cod.DeepCopy()
+ stale.UID = "old-cod-uid"
+ foreign.OwnerReferences = []metav1.OwnerReference{*metav1.NewControllerRef(stale, ocv1.GroupVersion.WithKind(ocv1.ClusterObjectDeploymentKind))}
+ require.NoError(t, c.Update(t.Context(), foreign))
+ case "deleted":
+ require.NoError(t, c.Delete(t.Context(), foreign))
+ expectedRevision = 2
+ case "deleting":
+ require.NoError(t, c.Delete(t.Context(), foreign))
+ }
+ _, err = reconcileCOD(t, c)
+ require.NoError(t, err)
+ created, found := getCOS(t, c, fmt.Sprintf("%s-%d", cod.Name, expectedRevision))
+ require.True(t, found)
+ require.True(t, metav1.IsControlledBy(created, cod))
+ require.Equal(t, metav1.ConditionTrue, progressing(t, c).Status)
+ require.Equal(t, ocv1.ClusterObjectDeploymentReasonNewClusterObjectSetProgressing, progressing(t, c).Reason)
+ require.Equal(t, metav1.ConditionTrue, available(t, c).Status)
+ if resolution == "orphaned" || resolution == "old deployment UID" {
+ adopted, found := getCOS(t, c, foreign.Name)
+ require.True(t, found)
+ require.True(t, metav1.IsControlledBy(adopted, cod))
+ }
+ })
+ }
+}
+
+func Test_ClusterObjectDeploymentReconciler_ForeignHistoryExcluded(t *testing.T) {
+ for _, changed := range []bool{false, true} {
+ t.Run(fmt.Sprintf("template changed %t", changed), func(t *testing.T) {
+ cod := codFixture()
+ cod.Spec.RevisionHistoryLimit = ptr.To(int32(1))
+ archived := deploymentRevision(cod, 2, ocv1.ClusterObjectSetLifecycleStateArchived)
+ active := deploymentRevision(cod, 4, ocv1.ClusterObjectSetLifecycleStateActive)
+ foreignLower := deploymentRevision(cod, 1, ocv1.ClusterObjectSetLifecycleStateActive)
+ foreignArchived := deploymentRevision(cod, 8, ocv1.ClusterObjectSetLifecycleStateArchived)
+ foreignDeleting := deploymentRevision(cod, 9, ocv1.ClusterObjectSetLifecycleStateActive)
+ foreignDeleting.DeletionTimestamp = ptr.To(metav1.Now())
+ foreignDeleting.Finalizers = []string{"test/hold"}
+ foreign := []*ocv1.ClusterObjectSet{foreignLower, foreignArchived, foreignDeleting}
+ for _, cos := range foreign {
+ cos.OwnerReferences = []metav1.OwnerReference{foreignControllerReference()}
+ }
+ setCOSReady(foreignLower, metav1.ConditionTrue, ocv1.ClusterObjectSetReasonAllObjectsReady)
+ if changed {
+ cod.Spec.Template.Spec.CollisionProtection = ocv1.CollisionProtectionNone
+ }
+ c := newCODClient(t, cod, archived, active, foreignLower, foreignArchived, foreignDeleting)
+
+ _, err := reconcileCOD(t, c)
+ require.NoError(t, err)
+ require.Equal(t, ocv1.ClusterObjectDeploymentReasonUnavailable, available(t, c).Reason, "a ready foreign revision must not make the deployment available")
+ require.Equal(t, ocv1.ClusterObjectDeploymentReasonNewClusterObjectSetProgressing, progressing(t, c).Reason)
+ _, found := getCOS(t, c, archived.Name)
+ require.True(t, found, "foreign history must not count toward the retention limit")
+ for _, cos := range foreign {
+ retained, found := getCOS(t, c, cos.Name)
+ require.True(t, found)
+ require.Equal(t, cos.Spec, retained.Spec)
+ require.Equal(t, cos.OwnerReferences, retained.OwnerReferences)
+ }
+ created, found := getCOS(t, c, cod.Name+"-10")
+ require.Equal(t, changed, found, "allocation must include foreign archived and deleting revisions")
+ if changed {
+ require.Equal(t, int64(10), created.Spec.Revision)
+ }
+ })
+ }
+}
+
+func Test_ClusterObjectDeploymentReconciler_AdoptionFailure(t *testing.T) {
+ for _, conflict := range []bool{false, true} {
+ t.Run(fmt.Sprintf("owner conflict %t", conflict), func(t *testing.T) {
+ cod := codFixture()
+ owned := deploymentRevision(cod, 1, ocv1.ClusterObjectSetLifecycleStateActive)
+ setCOSReady(owned, metav1.ConditionTrue, ocv1.ClusterObjectSetReasonAllObjectsReady)
+ orphan := deploymentRevision(cod, 3, ocv1.ClusterObjectSetLifecycleStateActive)
+ orphan.OwnerReferences = nil
+ orphan.Labels = nil // Successful adoption will require a new matching revision.
+ foreign := deploymentRevision(cod, 4, ocv1.ClusterObjectSetLifecycleStateArchived)
+ foreign.OwnerReferences = []metav1.OwnerReference{foreignControllerReference()}
+ if conflict {
+ foreign.Spec.LifecycleState = ocv1.ClusterObjectSetLifecycleStateActive
+ }
+ adoptionErr := errors.New("adoption denied")
+ deny := true
+ creates := 0
+ c := interceptor.NewClient(newCODClient(t, cod, owned, orphan, foreign), interceptor.Funcs{
+ Patch: func(ctx context.Context, cl client.WithWatch, obj client.Object, patch client.Patch, opts ...client.PatchOption) error {
+ if deny && obj.GetName() == orphan.Name {
+ return adoptionErr
+ }
+ return cl.Patch(ctx, obj, patch, opts...)
+ },
+ Create: func(ctx context.Context, cl client.WithWatch, obj client.Object, opts ...client.CreateOption) error {
+ creates++
+ return cl.Create(ctx, obj, opts...)
+ },
+ })
+ _, err := reconcileCOD(t, c)
+ require.ErrorIs(t, err, adoptionErr)
+ require.Zero(t, creates)
+ require.Equal(t, metav1.ConditionTrue, available(t, c).Status)
+ unadopted, found := getCOS(t, c, orphan.Name)
+ require.True(t, found)
+ require.Nil(t, metav1.GetControllerOf(unadopted))
+ if conflict {
+ require.Equal(t, ocv1.ClusterObjectDeploymentReasonOwnerConflict, progressing(t, c).Reason)
+ } else {
+ require.Equal(t, ocv1.ClusterObjectDeploymentReasonNewClusterObjectSetProgressing, progressing(t, c).Reason)
+ require.Contains(t, progressing(t, c).Message, "adopted")
+ }
+
+ deny = false
+ _, err = reconcileCOD(t, c)
+ require.NoError(t, err)
+ adopted, found := getCOS(t, c, orphan.Name)
+ require.True(t, found)
+ require.True(t, metav1.IsControlledBy(adopted, cod))
+ if conflict {
+ require.Zero(t, creates)
+ require.Equal(t, ocv1.ClusterObjectDeploymentReasonOwnerConflict, progressing(t, c).Reason)
+ } else {
+ require.Equal(t, 1, creates)
+ _, found := getCOS(t, c, cod.Name+"-5")
+ require.True(t, found)
+ }
+ })
+ }
+}
+
+func Test_ClusterObjectDeploymentReconciler_ConflictContinuesOwnedPruning(t *testing.T) {
+ cod := codFixture()
+ cod.Spec.RevisionHistoryLimit = ptr.To(int32(0))
+ archived := deploymentRevision(cod, 1, ocv1.ClusterObjectSetLifecycleStateArchived)
+ foreign := deploymentRevision(cod, 2, ocv1.ClusterObjectSetLifecycleStateActive)
+ foreign.OwnerReferences = []metav1.OwnerReference{foreignControllerReference()}
+ c := newCODClient(t, cod, archived, foreign)
+
+ _, err := reconcileCOD(t, c)
+ require.NoError(t, err)
+ _, found := getCOS(t, c, archived.Name)
+ require.False(t, found, "owned history cleanup continues during a conflict")
+ retained, found := getCOS(t, c, foreign.Name)
+ require.True(t, found)
+ require.Equal(t, foreign.Spec, retained.Spec)
+ require.Equal(t, foreign.OwnerReferences, retained.OwnerReferences)
+ require.Equal(t, ocv1.ClusterObjectDeploymentReasonOwnerConflict, progressing(t, c).Reason)
+}
+
+func Test_ClusterObjectDeploymentReconciler_ConcurrentOwnerClaim(t *testing.T) {
+ for _, operation := range []string{"adoption", "pruning"} {
+ t.Run(operation, func(t *testing.T) {
+ cod := codFixture()
+ cod.Spec.RevisionHistoryLimit = ptr.To(int32(0))
+ cos := deploymentRevision(cod, 1, ocv1.ClusterObjectSetLifecycleStateActive)
+ if operation == "adoption" {
+ cos.OwnerReferences = nil
+ } else {
+ cos.Spec.LifecycleState = ocv1.ClusterObjectSetLifecycleStateArchived
+ setCOSReady(cos, metav1.ConditionFalse, ocv1.ClusterObjectSetReasonArchived)
+ }
+ claimed := false
+ claim := func(ctx context.Context, cl client.WithWatch) {
+ t.Helper()
+ if claimed {
+ return
+ }
+ current := &ocv1.ClusterObjectSet{}
+ require.NoError(t, cl.Get(ctx, client.ObjectKeyFromObject(cos), current))
+ current.OwnerReferences = []metav1.OwnerReference{foreignControllerReference()}
+ require.NoError(t, cl.Update(ctx, current))
+ claimed = true
+ }
+ c := interceptor.NewClient(newCODClient(t, cod, cos), interceptor.Funcs{
+ Patch: func(ctx context.Context, cl client.WithWatch, obj client.Object, patch client.Patch, opts ...client.PatchOption) error {
+ if operation == "adoption" && obj.GetName() == cos.Name {
+ claim(ctx, cl)
+ }
+ return cl.Patch(ctx, obj, patch, opts...)
+ },
+ Delete: func(ctx context.Context, cl client.WithWatch, obj client.Object, opts ...client.DeleteOption) error {
+ if operation == "pruning" && obj.GetName() == cos.Name {
+ claim(ctx, cl)
+ }
+ return cl.Delete(ctx, obj, opts...)
+ },
+ })
+
+ _, err := reconcileCOD(t, c)
+ require.True(t, apierrors.IsConflict(err), "a concurrent ownership change must reject the stale mutation: %v", err)
+ require.True(t, claimed)
+ retained, found := getCOS(t, c, cos.Name)
+ require.True(t, found)
+ require.Equal(t, foreignControllerReference(), *metav1.GetControllerOf(retained))
+ if operation == "adoption" {
+ _, found := getCOS(t, c, cod.Name+"-2")
+ require.False(t, found, "failed adoption must not create a competing revision")
+ _, err = reconcileCOD(t, c)
+ require.NoError(t, err)
+ require.Equal(t, ocv1.ClusterObjectDeploymentReasonOwnerConflict, progressing(t, c).Reason)
+ }
+ })
+ }
+}
diff --git a/internal/object-controller/controllers/clusterobjectdeployment_ownership_test.go b/internal/object-controller/controllers/clusterobjectdeployment_ownership_test.go
new file mode 100644
index 0000000000..e25c3223c4
--- /dev/null
+++ b/internal/object-controller/controllers/clusterobjectdeployment_ownership_test.go
@@ -0,0 +1,184 @@
+//go:build !standard
+
+package controllers_test
+
+import (
+ "context"
+ "fmt"
+ "testing"
+ "time"
+
+ "github.com/go-logr/logr"
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+ apierrors "k8s.io/apimachinery/pkg/api/errors"
+ apimeta "k8s.io/apimachinery/pkg/api/meta"
+ metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+ "k8s.io/utils/ptr"
+ ctrl "sigs.k8s.io/controller-runtime"
+ "sigs.k8s.io/controller-runtime/pkg/client"
+ ctrlconfig "sigs.k8s.io/controller-runtime/pkg/config"
+ metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
+
+ ocv1 "github.com/operator-framework/operator-controller/api/v1"
+ "github.com/operator-framework/operator-controller/internal/object-controller/controllers"
+)
+
+// TestClusterObjectDeploymentGroupEvents runs the COD controller against envtest
+// to exercise its real group watch. COS status is supplied as input to isolate
+// orchestration from the COS controller's independent revision lifecycle. Owned
+// and foreign revisions describe disjoint objects so ownership handover cannot
+// account for changes in availability.
+func TestClusterObjectDeploymentGroupEvents(t *testing.T) {
+ cl := newCODManagerClient(t)
+ for _, resolution := range []string{"archival", "orphaning", "deletion"} {
+ t.Run(resolution, func(t *testing.T) {
+ cod := codFixture()
+ cod.Name = "cod-group-events-" + resolution
+ cod.UID = ""
+ cod.Spec.Template.Spec.Phases[0].Objects[0].Object.SetName("owned-" + resolution)
+ require.NoError(t, cl.Create(t.Context(), cod))
+ t.Cleanup(func() {
+ ctx := context.Background()
+ require.NoError(t, client.IgnoreNotFound(cl.Delete(ctx, cod)))
+ var revisions ocv1.ClusterObjectSetList
+ require.NoError(t, cl.List(ctx, &revisions))
+ for i := range revisions.Items {
+ if revisions.Items[i].Spec.Group == cod.Name {
+ require.NoError(t, client.IgnoreNotFound(cl.Delete(ctx, &revisions.Items[i])))
+ }
+ }
+ })
+
+ owned := &ocv1.ClusterObjectSet{}
+ require.EventuallyWithT(t, func(collect *assert.CollectT) {
+ assert.NoError(collect, cl.Get(t.Context(), client.ObjectKey{Name: cod.Name + "-1"}, owned))
+ }, 10*time.Second, 10*time.Millisecond)
+ owned.Status.CompletedAt = metav1.Now()
+ owned.Status.ObjectCounts = ocv1.ObjectCounts{Total: 1, Present: 1, Synced: 1, Ready: 1}
+ setCOSReady(owned, metav1.ConditionTrue, ocv1.ClusterObjectSetReasonAllObjectsReady)
+ require.NoError(t, cl.Status().Update(t.Context(), owned))
+ waitForCODCondition(t, cl, cod.Name, ocv1.ClusterObjectDeploymentTypeAvailable,
+ metav1.ConditionTrue, ocv1.ClusterObjectDeploymentReasonAvailable, 0)
+
+ foreign := owned.DeepCopy()
+ foreign.ObjectMeta = metav1.ObjectMeta{
+ Name: cod.Name + "-3", Labels: owned.Labels,
+ OwnerReferences: []metav1.OwnerReference{foreignControllerReference()},
+ }
+ foreign.Spec.Revision = 3
+ foreign.Spec.Phases[0].Objects[0].Object.SetName("foreign-" + resolution)
+ foreign.Status = ocv1.ClusterObjectSetStatus{}
+ require.NoError(t, cl.Create(t.Context(), foreign))
+ waitForCODCondition(t, cl, cod.Name, ocv1.ClusterObjectDeploymentTypeProgressing,
+ metav1.ConditionFalse, ocv1.ClusterObjectDeploymentReasonOwnerConflict, 0)
+
+ // A ready foreign revision cannot supply the COD's availability.
+ setCOSReady(foreign, metav1.ConditionTrue, ocv1.ClusterObjectSetReasonAllObjectsReady)
+ foreign.Status.ObjectCounts = ocv1.ObjectCounts{Total: 1, Present: 1, Synced: 1, Ready: 1}
+ require.NoError(t, cl.Status().Update(t.Context(), foreign))
+ markCOSReady(t, cl, owned.Name, false)
+ waitForCODCondition(t, cl, cod.Name, ocv1.ClusterObjectDeploymentTypeAvailable,
+ metav1.ConditionFalse, ocv1.ClusterObjectDeploymentReasonUnavailable, 0)
+ markCOSReady(t, cl, owned.Name, true)
+ waitForCODCondition(t, cl, cod.Name, ocv1.ClusterObjectDeploymentTypeAvailable,
+ metav1.ConditionTrue, ocv1.ClusterObjectDeploymentReasonAvailable, 0)
+
+ require.NoError(t, cl.Get(t.Context(), client.ObjectKeyFromObject(cod), cod))
+ cod.Spec.Template.Spec.CollisionProtection = ocv1.CollisionProtectionNone
+ require.NoError(t, cl.Update(t.Context(), cod))
+ waitForCODCondition(t, cl, cod.Name, ocv1.ClusterObjectDeploymentTypeProgressing,
+ metav1.ConditionFalse, ocv1.ClusterObjectDeploymentReasonOwnerConflict, cod.Generation)
+ require.True(t, apierrors.IsNotFound(cl.Get(t.Context(), client.ObjectKey{Name: cod.Name + "-4"}, &ocv1.ClusterObjectSet{})))
+ require.NoError(t, cl.Get(t.Context(), client.ObjectKeyFromObject(owned), owned))
+ require.Equal(t, ocv1.ObjectCounts{Total: 1, Present: 1, Synced: 1, Ready: 1}, owned.Status.ObjectCounts)
+
+ // Only the foreign COS changes here. Its group event must release the
+ // conflict and reconcile the already-updated COD template.
+ require.NoError(t, cl.Get(t.Context(), client.ObjectKeyFromObject(foreign), foreign))
+ expectedRevision := int64(4)
+ switch resolution {
+ case "archival":
+ foreign.Spec.LifecycleState = ocv1.ClusterObjectSetLifecycleStateArchived
+ require.NoError(t, cl.Update(t.Context(), foreign))
+ case "orphaning":
+ foreign.OwnerReferences = nil
+ require.NoError(t, cl.Update(t.Context(), foreign))
+ case "deletion":
+ require.NoError(t, cl.Delete(t.Context(), foreign))
+ expectedRevision = 2
+ }
+ created := &ocv1.ClusterObjectSet{}
+ require.EventuallyWithT(t, func(collect *assert.CollectT) {
+ if assert.NoError(collect, cl.Get(t.Context(), client.ObjectKey{Name: fmt.Sprintf("%s-%d", cod.Name, expectedRevision)}, created)) {
+ assert.True(collect, metav1.IsControlledBy(created, cod))
+ }
+ }, 10*time.Second, 10*time.Millisecond)
+ waitForCODCondition(t, cl, cod.Name, ocv1.ClusterObjectDeploymentTypeProgressing,
+ metav1.ConditionTrue, ocv1.ClusterObjectDeploymentReasonNewClusterObjectSetProgressing, cod.Generation)
+ waitForCODCondition(t, cl, cod.Name, ocv1.ClusterObjectDeploymentTypeAvailable,
+ metav1.ConditionTrue, ocv1.ClusterObjectDeploymentReasonAvailable, cod.Generation)
+ if resolution != "deletion" {
+ require.NoError(t, cl.Get(t.Context(), client.ObjectKeyFromObject(foreign), foreign))
+ if resolution == "orphaning" {
+ require.True(t, metav1.IsControlledBy(foreign, cod))
+ } else {
+ require.Equal(t, foreignControllerReference(), *metav1.GetControllerOf(foreign))
+ }
+ }
+ })
+ }
+}
+
+func newCODManagerClient(t *testing.T) client.Client {
+ t.Helper()
+ scheme := newScheme(t)
+ cl, err := client.New(config, client.Options{Scheme: scheme})
+ require.NoError(t, err)
+ mgr, err := ctrl.NewManager(config, ctrl.Options{
+ Scheme: scheme, Logger: logr.Discard(),
+ Metrics: metricsserver.Options{BindAddress: "0"},
+ HealthProbeBindAddress: "0",
+ Controller: ctrlconfig.Controller{SkipNameValidation: ptr.To(true)},
+ })
+ require.NoError(t, err)
+ require.NoError(t, controllers.SetupIndexes(t.Context(), mgr.GetFieldIndexer()))
+ require.NoError(t, (&controllers.ClusterObjectDeploymentReconciler{
+ Client: mgr.GetClient(), Scheme: scheme,
+ }).SetupWithManager(mgr))
+ ctx, cancel := context.WithCancel(t.Context())
+ done := make(chan error, 1)
+ go func() { done <- mgr.Start(ctx) }()
+ t.Cleanup(func() {
+ cancel()
+ select {
+ case err := <-done:
+ require.NoError(t, err)
+ case <-time.After(10 * time.Second):
+ t.Fatal("COD manager did not stop")
+ }
+ })
+ syncCtx, syncCancel := context.WithTimeout(ctx, 10*time.Second)
+ defer syncCancel()
+ require.True(t, mgr.GetCache().WaitForCacheSync(syncCtx))
+ return cl
+}
+
+func waitForCODCondition(t *testing.T, cl client.Client, name, conditionType string, status metav1.ConditionStatus, reason string, generation int64) {
+ t.Helper()
+ require.EventuallyWithT(t, func(collect *assert.CollectT) {
+ cod := &ocv1.ClusterObjectDeployment{}
+ if !assert.NoError(collect, cl.Get(t.Context(), client.ObjectKey{Name: name}, cod)) {
+ return
+ }
+ condition := apimeta.FindStatusCondition(cod.Status.Conditions, conditionType)
+ if !assert.NotNil(collect, condition) {
+ return
+ }
+ assert.Equal(collect, status, condition.Status)
+ assert.Equal(collect, reason, condition.Reason)
+ if generation != 0 {
+ assert.Equal(collect, generation, condition.ObservedGeneration)
+ }
+ }, 10*time.Second, 10*time.Millisecond)
+}
diff --git a/internal/object-controller/controllers/progress_deadline_test.go b/internal/object-controller/controllers/progress_deadline_test.go
index 7cb061af27..99f3be6100 100644
--- a/internal/object-controller/controllers/progress_deadline_test.go
+++ b/internal/object-controller/controllers/progress_deadline_test.go
@@ -282,6 +282,19 @@ func TestCODDeadlineAwareRateLimiter(t *testing.T) {
{name: "keep shorter backoff", elapsed: time.Minute, backoff: 30 * time.Second, expectWait: 30 * time.Second},
{name: "expired revision wakes immediately", elapsed: 10 * time.Minute, backoff: 15 * time.Minute},
{name: "creation pending has no deadline", elapsed: time.Hour, backoff: 15 * time.Minute, omitCOS: true, expectWait: 15 * time.Minute},
+ {
+ name: "foreign revision has no deployment deadline", elapsed: time.Hour, backoff: 15 * time.Minute, expectWait: 15 * time.Minute,
+ mutate: func(_ *ocv1.ClusterObjectDeployment, cos *ocv1.ClusterObjectSet) {
+ cos.OwnerReferences[0].Name = "other-deployment"
+ cos.OwnerReferences[0].UID = "foreign-uid"
+ },
+ },
+ {
+ name: "unadopted revision has no deployment deadline", elapsed: time.Hour, backoff: 15 * time.Minute, expectWait: 15 * time.Minute,
+ mutate: func(_ *ocv1.ClusterObjectDeployment, cos *ocv1.ClusterObjectSet) {
+ cos.OwnerReferences = nil
+ },
+ },
{
name: "changed template waits for its own revision", elapsed: time.Hour, backoff: 15 * time.Minute, expectWait: 15 * time.Minute,
mutate: func(cod *ocv1.ClusterObjectDeployment, _ *ocv1.ClusterObjectSet) {
@@ -428,6 +441,23 @@ func TestCODDeadlineAwareRateLimiter_DeadlineRemoved(t *testing.T) {
revisions.Items[0].DeletionTimestamp = &metav1.Time{Time: creation}
},
},
+ {
+ name: "revision ownership changes",
+ mutateRevisions: func(revisions *ocv1.ClusterObjectSetList) {
+ revisions.Items[0].OwnerReferences[0].UID = "old-deployment-uid"
+ },
+ },
+ {
+ name: "foreign leading revision appears",
+ mutateRevisions: func(revisions *ocv1.ClusterObjectSetList) {
+ foreign := revisions.Items[0].DeepCopy()
+ foreign.Name += "-foreign"
+ foreign.Spec.Revision++
+ foreign.OwnerReferences[0].Name = "other-deployment"
+ foreign.OwnerReferences[0].UID = "foreign-uid"
+ revisions.Items = append(revisions.Items, *foreign)
+ },
+ },
} {
t.Run(tc.name, func(t *testing.T) {
cod, cos := deadlineRateLimiterObjects(creation)
@@ -473,11 +503,12 @@ func TestCODDeadlineAwareRateLimiter_DeadlineRemoved(t *testing.T) {
func deadlineRateLimiterObjects(creation time.Time) (*ocv1.ClusterObjectDeployment, *ocv1.ClusterObjectSet) {
cod := &ocv1.ClusterObjectDeployment{
- ObjectMeta: metav1.ObjectMeta{Name: "test-cod"},
+ ObjectMeta: metav1.ObjectMeta{Name: "test-cod", UID: "test-cod-uid"},
Spec: ocv1.ClusterObjectDeploymentSpec{Template: testTemplate()},
}
cod.Spec.Template.Spec.ProgressDeadlineMinutes = 10
cos := buildClusterObjectSet(cod, 1, templateHash(cod.Spec.Template))
+ cos.OwnerReferences = []metav1.OwnerReference{*metav1.NewControllerRef(cod, ocv1.GroupVersion.WithKind(ocv1.ClusterObjectDeploymentKind))}
cos.CreationTimestamp = metav1.NewTime(creation)
return cod, cos
}
diff --git a/manifests/experimental-e2e.yaml b/manifests/experimental-e2e.yaml
index d96a444aae..3e39ee628e 100644
--- a/manifests/experimental-e2e.yaml
+++ b/manifests/experimental-e2e.yaml
@@ -1879,6 +1879,10 @@ spec:
Progressing is False with reason InvalidSpecification when spec.template.metadata
contains labels or annotations that are not valid for the resulting
ClusterObjectSet; no new revision is created until the metadata is corrected.
+ Progressing is False with reason OwnerConflict when the highest active,
+ non-deleting revision in the deployment's group is controlled by another
+ controller. No new revision is created while the conflict persists, even if
+ an owned revision has completed or exceeded its progress deadline.
While a rollout is incomplete, Progressing also reflects why the latest (head
active) ClusterObjectSet revision is not yet Ready. It is True with reason
@@ -1892,10 +1896,12 @@ spec:
named ClusterObjectSet revision for the specific cause.
Available is True with reason Available when at least one active (non-archived)
- ClusterObjectSet revision is Ready, so the deployment remains available across
+ owned ClusterObjectSet revision is Ready, so the deployment remains available across
rollouts while a previous revision is still Ready. It is False with reason
Unavailable when active revisions exist but none are Ready yet, or with reason
- NoActiveRevision when no active revision exists.
+ NoActiveRevision when no owned active revision exists. Revisions without a
+ controller owner, or controlled by a same-named deployment with an older UID,
+ are adopted before status is calculated. Foreign-owned revisions are excluded.
items:
description: Condition contains details for one aspect of the current
state of this API Resource.
diff --git a/manifests/experimental.yaml b/manifests/experimental.yaml
index 0f13d68f83..0a05deddf8 100644
--- a/manifests/experimental.yaml
+++ b/manifests/experimental.yaml
@@ -1840,6 +1840,10 @@ spec:
Progressing is False with reason InvalidSpecification when spec.template.metadata
contains labels or annotations that are not valid for the resulting
ClusterObjectSet; no new revision is created until the metadata is corrected.
+ Progressing is False with reason OwnerConflict when the highest active,
+ non-deleting revision in the deployment's group is controlled by another
+ controller. No new revision is created while the conflict persists, even if
+ an owned revision has completed or exceeded its progress deadline.
While a rollout is incomplete, Progressing also reflects why the latest (head
active) ClusterObjectSet revision is not yet Ready. It is True with reason
@@ -1853,10 +1857,12 @@ spec:
named ClusterObjectSet revision for the specific cause.
Available is True with reason Available when at least one active (non-archived)
- ClusterObjectSet revision is Ready, so the deployment remains available across
+ owned ClusterObjectSet revision is Ready, so the deployment remains available across
rollouts while a previous revision is still Ready. It is False with reason
Unavailable when active revisions exist but none are Ready yet, or with reason
- NoActiveRevision when no active revision exists.
+ NoActiveRevision when no owned active revision exists. Revisions without a
+ controller owner, or controlled by a same-named deployment with an older UID,
+ are adopted before status is calculated. Foreign-owned revisions are excluded.
items:
description: Condition contains details for one aspect of the current
state of this API Resource.