Skip to content

🐛 OPRUN-4783: Surface deployment owner conflicts on ClusterExtensions #3094

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
fao89 wants to merge 1 commit into
base: main
Choose a base branch
from
+236 −5
Open

🐛 OPRUN-4783: Surface deployment owner conflicts on ClusterExtensions #3094

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions docs/draft/concepts/clusterobjectsets.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,14 @@ exceeded its progress deadline. Availability still depends only on owned active
revisions; a previous ready revision can keep the COD `Available=True` during the
conflict.

For a ClusterExtension, `OwnerConflict` maps to `Progressing=True` with reason
`Retrying`, preserving the deployment's message about the conflicting revision
and its controller. A first install reports `Installed=False` with reason
`Failed`. During an upgrade, the previously installed bundle and its `Installed=True`
condition are preserved until the new rollout completes, with `observedGeneration`
updated to acknowledge the current spec. Reconciliation resumes automatically when
the ownership conflict clears.

`RevisionObjectCollision` and `RevisionInvalidSpecification` are *hard stalls*:
they will not resolve on their own, so they report `Progressing=False` and take
precedence over `ProgressDeadlineExceeded` (the actionable cause is more useful
Expand Down
16 changes: 11 additions & 5 deletions internal/operator-controller/controllers/clusterobjectdeployment_status.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,10 @@ func progressingConditionFromClusterObjectDeployment(codProgressing *metav1.Cond
prog.Reason = ocv1.ReasonSucceeded
prog.Message = "Desired state reached"
case ocv1.ClusterObjectDeploymentReasonRevisionObjectCollision,
ocv1.ClusterObjectDeploymentReasonRevisionPreconditionsNotMet:
// The head revision is impeded by a foreign-owned object or an unmet precondition. These clear
// on their own once the obstruction is resolved, so the extension keeps retrying.
ocv1.ClusterObjectDeploymentReasonRevisionPreconditionsNotMet,
ocv1.ClusterObjectDeploymentReasonOwnerConflict:
// A foreign controller owns a revision or one of its objects, or a precondition is unmet.
// Reconciliation resumes automatically once the obstruction is resolved, so keep retrying.
prog.Reason = ocv1.ReasonRetrying
case ocv1.ClusterObjectDeploymentReasonProgressDeadlineExceeded:
prog.Status = metav1.ConditionFalse
Expand Down Expand Up @@ -113,9 +114,13 @@ func setInstalledFromClusterObjectDeployment(ext *ocv1.ClusterExtension, cod *oc
if ext.Status.Install == nil {
// Nothing has ever been installed (first install in progress or failing).
setInstalledStatusConditionFalse(ext, installFailureReasonFromClusterObjectDeployment(cod), "No bundle installed")
return
}
// A bundle from a prior completion stays installed while a new rollout is in flight.
// Acknowledge the current spec without changing its installed bundle or condition details.
if installed := apimeta.FindStatusCondition(ext.Status.Conditions, ocv1.TypeInstalled); installed != nil {
installed.ObservedGeneration = ext.GetGeneration()
}
// Otherwise a bundle is already installed (from a prior completion) and a new rollout is in flight;
// leave Installed=True and .status.install unchanged until the rollout completes.
}

// clusterObjectDeploymentRolloutComplete reports whether the deployment's current-generation rollout
Expand Down Expand Up @@ -153,6 +158,7 @@ func installFailureReasonFromClusterObjectDeployment(cod *ocv1.ClusterObjectDepl
switch prog.Reason {
case ocv1.ClusterObjectDeploymentReasonRevisionObjectCollision,
ocv1.ClusterObjectDeploymentReasonRevisionPreconditionsNotMet,
ocv1.ClusterObjectDeploymentReasonOwnerConflict,
ocv1.ClusterObjectDeploymentReasonRevisionInvalidSpecification,
ocv1.ClusterObjectDeploymentReasonInvalidSpecification,
ocv1.ClusterObjectDeploymentReasonProgressDeadlineExceeded:
Expand Down
90 changes: 90 additions & 0 deletions internal/operator-controller/controllers/clusterobjectdeployment_status_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,11 @@ func TestProgressingConditionFromClusterObjectDeployment(t *testing.T) {
cod: &metav1.Condition{Status: metav1.ConditionFalse, Reason: ocv1.ClusterObjectDeploymentReasonRevisionObjectCollision, Message: "owned by another controller"},
want: metav1.Condition{Status: metav1.ConditionTrue, Reason: ocv1.ReasonRetrying, Message: "owned by another controller"},
},
{
name: "owner conflict (deployment False) maps to Retrying (extension True)",
cod: &metav1.Condition{Status: metav1.ConditionFalse, Reason: ocv1.ClusterObjectDeploymentReasonOwnerConflict, Message: "revision is controlled by another controller"},
want: metav1.Condition{Status: metav1.ConditionTrue, Reason: ocv1.ReasonRetrying, Message: "revision is controlled by another controller"},
},
{
name: "preconditions not met maps to Retrying",
cod: &metav1.Condition{Status: metav1.ConditionTrue, Reason: ocv1.ClusterObjectDeploymentReasonRevisionPreconditionsNotMet, Message: "waiting on secret"},
Expand Down Expand Up @@ -274,3 +279,88 @@ func TestReflectClusterObjectDeploymentStatus(t *testing.T) {
require.Equal(t, metav1.ConditionFalse, apimeta.FindStatusCondition(ext.Status.Conditions, ocv1.TypeInstalled).Status)
})
}

func TestReflectClusterObjectDeploymentStatusOwnerConflict(t *testing.T) {
scheme := runtime.NewScheme()
require.NoError(t, ocv1.AddToScheme(scheme))

for _, tc := range []struct {
name string
installed bool
}{
{name: "first install"},
{name: "upgrade", installed: true},
} {
t.Run(tc.name, func(t *testing.T) {
ext := &ocv1.ClusterExtension{ObjectMeta: metav1.ObjectMeta{Name: "test-ext", Generation: 3}}
if tc.installed {
ext.Status.Install = &ocv1.ClusterExtensionInstallStatus{
Bundle: ocv1.BundleMetadata{Name: "test.1.2.0", Version: "1.2.0"},
}
setInstalledStatusConditionSuccess(ext, "Installed bundle test.1.2.0 successfully")
ext.Generation++
}
previous := ext.DeepCopy()
cod := codWithProgressing(ext.Name, 2, metav1.ConditionFalse, ocv1.ClusterObjectDeploymentReasonOwnerConflict,
"revision is controlled by another controller")
cod.Spec.Template.Metadata.Annotations = map[string]string{
labels.BundleNameKey: "test.1.3.0", labels.BundleVersionKey: "1.3.0",
}
cl := fake.NewClientBuilder().WithScheme(scheme).WithStatusSubresource(cod).WithObjects(cod).Build()
reflectStatus := ReflectClusterObjectDeploymentStatus(cl)

res, err := reflectStatus(t.Context(), &reconcileState{}, ext)
require.NoError(t, err)
require.Nil(t, res)
progressing := apimeta.FindStatusCondition(ext.Status.Conditions, ocv1.TypeProgressing)
require.NotNil(t, progressing)
require.Equal(t, metav1.ConditionTrue, progressing.Status)
require.Equal(t, ocv1.ReasonRetrying, progressing.Reason)
require.Equal(t, "revision is controlled by another controller", progressing.Message)
require.Equal(t, ext.Generation, progressing.ObservedGeneration)
require.Equal(t, previous.Status.Install, ext.Status.Install, "a conflicting rollout must not promote the new bundle")

installed := apimeta.FindStatusCondition(ext.Status.Conditions, ocv1.TypeInstalled)
require.NotNil(t, installed)
if tc.installed {
expected := *apimeta.FindStatusCondition(previous.Status.Conditions, ocv1.TypeInstalled)
expected.ObservedGeneration = ext.Generation
require.Equal(t, &expected, installed)
} else {
require.Equal(t, metav1.ConditionFalse, installed.Status)
require.Equal(t, ocv1.ReasonFailed, installed.Reason)
}
require.Equal(t, ext.Generation, installed.ObservedGeneration)

apimeta.SetStatusCondition(&cod.Status.Conditions, metav1.Condition{
Type: ocv1.ClusterObjectDeploymentTypeProgressing, Status: metav1.ConditionTrue,
Reason: ocv1.ClusterObjectDeploymentReasonNewClusterObjectSetProgressing, Message: "latest revision is progressing",
ObservedGeneration: cod.Generation,
})
require.NoError(t, cl.Status().Update(t.Context(), cod))
_, err = reflectStatus(t.Context(), &reconcileState{}, ext)
require.NoError(t, err)
require.Equal(t, ocv1.ReasonRollingOut, apimeta.FindStatusCondition(ext.Status.Conditions, ocv1.TypeProgressing).Reason)
require.Equal(t, previous.Status.Install, ext.Status.Install)
if !tc.installed {
require.Equal(t, ocv1.ReasonAbsent, apimeta.FindStatusCondition(ext.Status.Conditions, ocv1.TypeInstalled).Reason)
}

apimeta.SetStatusCondition(&cod.Status.Conditions, metav1.Condition{
Type: ocv1.ClusterObjectDeploymentTypeProgressing, Status: metav1.ConditionFalse,
Reason: ocv1.ClusterObjectDeploymentReasonNewClusterObjectSetProgressed, Message: "latest revision has progressed",
ObservedGeneration: cod.Generation,
})
require.NoError(t, cl.Status().Update(t.Context(), cod))
_, err = reflectStatus(t.Context(), &reconcileState{}, ext)
require.NoError(t, err)
require.Equal(t, ocv1.ReasonSucceeded, apimeta.FindStatusCondition(ext.Status.Conditions, ocv1.TypeProgressing).Reason)
require.Equal(t, &ocv1.ClusterExtensionInstallStatus{
Bundle: ocv1.BundleMetadata{Name: "test.1.3.0", Version: "1.3.0"},
}, ext.Status.Install)
installed = apimeta.FindStatusCondition(ext.Status.Conditions, ocv1.TypeInstalled)
require.Equal(t, metav1.ConditionTrue, installed.Status)
require.Equal(t, ocv1.ReasonSucceeded, installed.Reason)
})
}
}
61 changes: 61 additions & 0 deletions test/e2e/features/install.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,67 @@ Feature: Install ClusterExtension
[{"type":"olm.test-property","value":"some-value"}]
"""

@BoxcutterRuntime
Scenario: First install reports Retrying and a failed install when a foreign-owned revision blocks the deployment
Given a catalog "test" with packages:
| package | version | channel | replaces | contents |
| test | 1.2.0 | beta | | ConfigMap |
And ClusterObjectSet with Namespace owner "${TEST_NAMESPACE}" is applied
"""
apiVersion: olm.operatorframework.io/v1
kind: ClusterObjectSet
metadata:
name: ${NAME}-2
spec:
group: ${NAME}
revision: 2
lifecycleState: Active
collisionProtection: Prevent
phases:
- name: resources
objects:
- object:
apiVersion: v1
kind: ConfigMap
metadata:
name: ${NAME}-foreign
namespace: ${TEST_NAMESPACE}
data:
k: foreign
"""
And ClusterObjectSet "${NAME}-2" reports Ready as True with Reason AllObjectsReady
When ClusterExtension is applied
"""
apiVersion: olm.operatorframework.io/v1
kind: ClusterExtension
metadata:
name: ${NAME}
spec:
namespace: ${TEST_NAMESPACE}
source:
sourceType: Catalog
catalog:
packageName: ${PACKAGE:test}
version: 1.2.0
selector:
matchLabels:
"olm.operatorframework.io/metadata.name": ${CATALOG:test}
"""
Then ClusterObjectDeployment "${NAME}" reports Progressing as False with Reason OwnerConflict and Message includes:
"""
ClusterObjectSet "${NAME}-2" (revision 2) is controlled by v1 Namespace "${TEST_NAMESPACE}"
"""
And ClusterExtension reports Progressing as True with Reason Retrying and Message includes:
"""
ClusterObjectSet "${NAME}-2" (revision 2) is controlled by v1 Namespace "${TEST_NAMESPACE}"
"""
And ClusterExtension reports Installed as False with Reason Failed
When ClusterObjectSet "${NAME}-2" lifecycle is set to "Archived"
Then ClusterExtension is rolled out
And ClusterExtension is available
And bundle "${PACKAGE:test}.1.2.0" is installed in version "1.2.0"
And ClusterObjectSet "${NAME}-3" reports Ready as True with Reason AllObjectsReady

@BoxcutterRuntime
Scenario: ClusterExtension installs via a ClusterObjectDeployment that owns the revision
Given a catalog "test" with packages:
Expand Down
66 changes: 66 additions & 0 deletions test/e2e/features/update.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,72 @@ Feature: Update ClusterExtension
And ClusterExtension is available
And bundle "${PACKAGE:test}.1.0.1" is installed in version "1.0.1"

@BoxcutterRuntime
Scenario: Update preserves the installed bundle when a foreign-owned revision blocks the deployment
Given ClusterExtension is applied
"""
apiVersion: olm.operatorframework.io/v1
kind: ClusterExtension
metadata:
name: ${NAME}
spec:
namespace: ${TEST_NAMESPACE}
source:
sourceType: Catalog
catalog:
packageName: ${PACKAGE:test}
selector:
matchLabels:
"olm.operatorframework.io/metadata.name": ${CATALOG:test}
version: 1.0.0
"""
And ClusterExtension is rolled out
And ClusterExtension is available
And bundle "${PACKAGE:test}.1.0.0" is installed in version "1.0.0"
# The missing namespace keeps the foreign revision from completing and
# archiving the healthy installed revision.
And ClusterObjectSet with Namespace owner "${TEST_NAMESPACE}" is applied
"""
apiVersion: olm.operatorframework.io/v1
kind: ClusterObjectSet
metadata:
name: ${NAME}-2
spec:
group: ${NAME}
revision: 2
lifecycleState: Active
collisionProtection: Prevent
phases:
- name: resources
objects:
- object:
apiVersion: v1
kind: ConfigMap
metadata:
name: ${NAME}-foreign
namespace: ${TEST_NAMESPACE}-missing
data:
k: foreign
"""
And ClusterObjectSet "${NAME}-2" reports Ready as False with Reason PreconditionsNotMet
When ClusterExtension version is updated to "1.0.1"
Then ClusterObjectDeployment "${NAME}" reports Progressing as False with Reason OwnerConflict and Message includes:
"""
ClusterObjectSet "${NAME}-2" (revision 2) is controlled by v1 Namespace "${TEST_NAMESPACE}"
"""
And ClusterExtension reports Progressing as True with Reason Retrying and Message includes:
"""
ClusterObjectSet "${NAME}-2" (revision 2) is controlled by v1 Namespace "${TEST_NAMESPACE}"
"""
And ClusterExtension reports Installed as True with Reason Succeeded
And bundle "${PACKAGE:test}.1.0.0" is installed in version "1.0.0"
And ClusterExtension is available
When ClusterObjectSet "${NAME}-2" lifecycle is set to "Archived"
Then ClusterExtension is rolled out
And ClusterExtension is available
And bundle "${PACKAGE:test}.1.0.1" is installed in version "1.0.1"
And ClusterObjectSet "${NAME}-3" reports Ready as True with Reason AllObjectsReady

Scenario: Cannot update extension to non successor version
Given ClusterExtension is applied
"""
Expand Down
Loading