diff --git a/internal/operator-controller/controllers/clusterextensionrevision_controller.go b/internal/operator-controller/controllers/clusterextensionrevision_controller.go index 2c8525b2b1..138bcd8eba 100644 --- a/internal/operator-controller/controllers/clusterextensionrevision_controller.go +++ b/internal/operator-controller/controllers/clusterextensionrevision_controller.go @@ -141,7 +141,7 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev } if !rev.DeletionTimestamp.IsZero() || rev.Spec.LifecycleState == ocv1.ClusterExtensionRevisionLifecycleStateArchived { - return c.teardown(ctx, rev, revision) + return c.teardown(ctx, rev) } revVersion := rev.GetAnnotations()[labels.BundleVersionKey] @@ -275,33 +275,7 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev return ctrl.Result{}, nil } -func (c *ClusterExtensionRevisionReconciler) teardown(ctx context.Context, rev *ocv1.ClusterExtensionRevision, revision *boxcutter.Revision) (ctrl.Result, error) { - l := log.FromContext(ctx) - - revisionEngine, err := c.RevisionEngineFactory.CreateRevisionEngine(ctx, rev) - if err != nil { - markAsAvailableUnknown(rev, ocv1.ClusterExtensionRevisionReasonReconciling, err.Error()) - return ctrl.Result{}, fmt.Errorf("failed to create revision engine for teardown: %v", err) - } - - tres, err := revisionEngine.Teardown(ctx, *revision) - if err != nil { - if tres != nil { - l.V(1).Info("teardown failure report", "report", tres.String()) - } - markAsAvailableUnknown(rev, ocv1.ClusterExtensionRevisionReasonReconciling, err.Error()) - return ctrl.Result{}, fmt.Errorf("revision teardown: %v", err) - } - - // Log detailed teardown reports only in debug mode (V(1)) to reduce verbosity. - l.V(1).Info("teardown report", "report", tres.String()) - if !tres.IsComplete() { - // TODO: If it is not complete, it seems like it would be good to update - // the status in some way to tell the user that the teardown is still - // in progress. - return ctrl.Result{}, nil - } - +func (c *ClusterExtensionRevisionReconciler) teardown(ctx context.Context, rev *ocv1.ClusterExtensionRevision) (ctrl.Result, error) { if err := c.TrackingCache.Free(ctx, rev); err != nil { markAsAvailableUnknown(rev, ocv1.ClusterExtensionRevisionReasonReconciling, err.Error()) return ctrl.Result{}, fmt.Errorf("error stopping informers: %v", err) diff --git a/internal/operator-controller/controllers/clusterextensionrevision_controller_test.go b/internal/operator-controller/controllers/clusterextensionrevision_controller_test.go index a4192e7845..f456976f43 100644 --- a/internal/operator-controller/controllers/clusterextensionrevision_controller_test.go +++ b/internal/operator-controller/controllers/clusterextensionrevision_controller_test.go @@ -697,40 +697,6 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_Deletion(t *testing.T) { require.True(t, apierrors.IsNotFound(err)) }, }, - { - name: "set Available:Unknown:Reconciling and surface tear down errors when deleted", - revisionResult: mockRevisionResult{}, - existingObjs: func() []client.Object { - ext := newTestClusterExtension() - rev1 := newTestClusterExtensionRevision(t, clusterExtensionRevisionName, ext, testScheme) - rev1.Finalizers = []string{ - "olm.operatorframework.io/teardown", - } - rev1.DeletionTimestamp = &metav1.Time{Time: time.Now()} - return []client.Object{rev1, ext} - }, - revisionEngineTeardownFn: func(t *testing.T) func(ctx context.Context, rev machinerytypes.Revision, opts ...machinerytypes.RevisionTeardownOption) (machinery.RevisionTeardownResult, error) { - return func(ctx context.Context, rev machinerytypes.Revision, opts ...machinerytypes.RevisionTeardownOption) (machinery.RevisionTeardownResult, error) { - return nil, fmt.Errorf("some teardown error") - } - }, - expectedErr: "some teardown error", - validate: func(t *testing.T, c client.Client) { - t.Log("cluster revision is not deleted and still contains finalizer") - rev := &ocv1.ClusterExtensionRevision{} - err := c.Get(t.Context(), client.ObjectKey{ - Name: clusterExtensionRevisionName, - }, rev) - require.NoError(t, err) - require.NotContains(t, "olm.operatorframework.io/teardown", rev.Finalizers) - cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterExtensionRevisionTypeAvailable) - require.NotNil(t, cond) - require.Equal(t, metav1.ConditionUnknown, cond.Status) - require.Equal(t, ocv1.ClusterExtensionRevisionReasonReconciling, cond.Reason) - require.Equal(t, "some teardown error", cond.Message) - require.Equal(t, int64(1), cond.ObservedGeneration) - }, - }, { name: "set Available:Unknown:Reconciling and surface tracking cache cleanup errors when deleted", revisionResult: mockRevisionResult{}, @@ -851,34 +817,6 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_Deletion(t *testing.T) { require.NotContains(t, rev.Finalizers, "olm.operatorframework.io/teardown") }, }, - { - name: "surfaces revision teardown error when in archived state", - revisionResult: mockRevisionResult{}, - existingObjs: func() []client.Object { - ext := newTestClusterExtension() - rev1 := newTestClusterExtensionRevision(t, clusterExtensionRevisionName, ext, testScheme) - rev1.Finalizers = []string{ - "olm.operatorframework.io/teardown", - } - rev1.Spec.LifecycleState = ocv1.ClusterExtensionRevisionLifecycleStateArchived - return []client.Object{rev1, ext} - }, - revisionEngineTeardownFn: func(t *testing.T) func(ctx context.Context, rev machinerytypes.Revision, opts ...machinerytypes.RevisionTeardownOption) (machinery.RevisionTeardownResult, error) { - return func(ctx context.Context, rev machinerytypes.Revision, opts ...machinerytypes.RevisionTeardownOption) (machinery.RevisionTeardownResult, error) { - return nil, fmt.Errorf("some teardown error") - } - }, - expectedErr: "some teardown error", - validate: func(t *testing.T, c client.Client) { - t.Log("cluster revision is not deleted and still contains finalizer") - rev := &ocv1.ClusterExtensionRevision{} - err := c.Get(t.Context(), client.ObjectKey{ - Name: clusterExtensionRevisionName, - }, rev) - require.NoError(t, err) - require.NotContains(t, "olm.operatorframework.io/teardown", rev.Finalizers) - }, - }, } { t.Run(tc.name, func(t *testing.T) { // create extension and cluster extension @@ -1439,34 +1377,4 @@ func Test_ClusterExtensionRevisionReconciler_getScopedClient_Errors(t *testing.T require.Contains(t, err.Error(), "failed to create revision engine") require.Contains(t, err.Error(), "token getter failed") }) - - t.Run("factory fails during teardown", func(t *testing.T) { - ext := newTestClusterExtension() - rev := newTestClusterExtensionRevision(t, "test-rev", ext, testScheme) - rev.DeletionTimestamp = &metav1.Time{Time: time.Now()} - rev.Finalizers = []string{"olm.operatorframework.io/teardown"} - - testClient := fake.NewClientBuilder(). - WithScheme(testScheme). - WithObjects(ext, rev). - Build() - - failingFactory := &mockRevisionEngineFactory{ - createErr: errors.New("serviceaccount not found"), - } - - reconciler := &controllers.ClusterExtensionRevisionReconciler{ - Client: testClient, - RevisionEngineFactory: failingFactory, - TrackingCache: &mockTrackingCache{client: testClient}, - } - - _, err := reconciler.Reconcile(t.Context(), ctrl.Request{ - NamespacedName: types.NamespacedName{Name: "test-rev"}, - }) - - require.Error(t, err) - require.Contains(t, err.Error(), "failed to create revision engine for teardown") - require.Contains(t, err.Error(), "serviceaccount not found") - }) }