Skip to content

Commit 01760b0

Browse files
author
Per Goncalves da Silva
committed
Teardown revision on archival
Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>
1 parent e17412a commit 01760b0

File tree

3 files changed

+112
-18
lines changed

3 files changed

+112
-18
lines changed

‎internal/operator-controller/controllers/clusterextensionrevision_controller.go‎

Lines changed: 34 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -140,11 +140,19 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev
140140
return ctrl.Result{}, fmt.Errorf("converting to boxcutter revision: %v", err)
141141
}
142142

143+
revisionEngine, err := c.RevisionEngineFactory.CreateRevisionEngine(ctx, rev)
144+
if err != nil {
145+
setRetryingConditions(rev, err.Error())
146+
return ctrl.Result{}, fmt.Errorf("failed to create revision engine: %v", err)
147+
}
148+
149+
//
150+
// Teardown
151+
//
143152
if !rev.DeletionTimestamp.IsZero() || rev.Spec.LifecycleState == ocv1.ClusterExtensionRevisionLifecycleStateArchived {
144-
return c.teardown(ctx, rev)
153+
return c.teardown(ctx, revisionEngine, revision, rev)
145154
}
146155

147-
revVersion := rev.GetAnnotations()[labels.BundleVersionKey]
148156
//
149157
// Reconcile
150158
//
@@ -158,12 +166,6 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev
158166
return ctrl.Result{}, werr
159167
}
160168

161-
revisionEngine, err := c.RevisionEngineFactory.CreateRevisionEngine(ctx, rev)
162-
if err != nil {
163-
setRetryingConditions(rev, err.Error())
164-
return ctrl.Result{}, fmt.Errorf("failed to create revision engine: %v", err)
165-
}
166-
167169
rres, err := revisionEngine.Reconcile(ctx, *revision, opts...)
168170
if err != nil {
169171
if rres != nil {
@@ -203,6 +205,7 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev
203205
}
204206
}
205207

208+
revVersion := rev.GetAnnotations()[labels.BundleVersionKey]
206209
if !rres.InTransition() {
207210
markAsProgressing(rev, ocv1.ReasonSucceeded, fmt.Sprintf("Revision %s has rolled out.", revVersion))
208211
} else {
@@ -275,18 +278,32 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev
275278
return ctrl.Result{}, nil
276279
}
277280

278-
func (c *ClusterExtensionRevisionReconciler) teardown(ctx context.Context, rev *ocv1.ClusterExtensionRevision) (ctrl.Result, error) {
279-
if err := c.TrackingCache.Free(ctx, rev); err != nil {
280-
markAsAvailableUnknown(rev, ocv1.ClusterExtensionRevisionReasonReconciling, err.Error())
281+
func (c *ClusterExtensionRevisionReconciler) teardown(
282+
ctx context.Context,
283+
revisionEngine RevisionEngine,
284+
revision *boxcutter.Revision,
285+
cer *ocv1.ClusterExtensionRevision,
286+
) (ctrl.Result, error) {
287+
if err := c.TrackingCache.Free(ctx, cer); err != nil {
288+
markAsAvailableUnknown(cer, ocv1.ClusterExtensionRevisionReasonReconciling, err.Error())
281289
return ctrl.Result{}, fmt.Errorf("error stopping informers: %v", err)
282290
}
283-
284-
// Ensure conditions are set before removing the finalizer when archiving
285-
if rev.Spec.LifecycleState == ocv1.ClusterExtensionRevisionLifecycleStateArchived && markAsArchived(rev) {
286-
return ctrl.Result{}, nil
291+
if cer.Spec.LifecycleState == ocv1.ClusterExtensionRevisionLifecycleStateArchived {
292+
tdres, err := revisionEngine.Teardown(ctx, *revision)
293+
if err != nil {
294+
setRetryingConditions(cer, fmt.Sprintf("error archiving: %v", err))
295+
return ctrl.Result{}, fmt.Errorf("error tearing down revision: %v", err)
296+
}
297+
if tdres != nil && !tdres.IsComplete() {
298+
setRetryingConditions(cer, "tearing down revision")
299+
return ctrl.Result{RequeueAfter: 5 * time.Second}, nil
300+
}
301+
// Ensure conditions are set before removing the finalizer when archiving
302+
if markAsArchived(cer) {
303+
return ctrl.Result{}, nil
304+
}
287305
}
288-
289-
if err := c.removeFinalizer(ctx, rev, clusterExtensionRevisionTeardownFinalizer); err != nil {
306+
if err := c.removeFinalizer(ctx, cer, clusterExtensionRevisionTeardownFinalizer); err != nil {
290307
return ctrl.Result{}, fmt.Errorf("error removing teardown finalizer: %v", err)
291308
}
292309
return ctrl.Result{}, nil

‎internal/operator-controller/controllers/clusterextensionrevision_controller_test.go‎

Lines changed: 72 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -644,6 +644,7 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_Deletion(t *testing.T) {
644644
validate func(*testing.T, client.Client)
645645
trackingCacheFreeFn func(context.Context, client.Object) error
646646
expectedErr string
647+
expectedResult ctrl.Result
647648
}{
648649
{
649650
name: "teardown finalizer is removed",
@@ -775,6 +776,76 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_Deletion(t *testing.T) {
775776
require.Equal(t, int64(1), cond.ObservedGeneration)
776777
},
777778
},
779+
{
780+
name: "set Progressing:True:Retrying and requeue when archived revision teardown is incomplete",
781+
revisionResult: mockRevisionResult{},
782+
existingObjs: func() []client.Object {
783+
ext := newTestClusterExtension()
784+
rev1 := newTestClusterExtensionRevision(t, clusterExtensionRevisionName, ext, testScheme)
785+
rev1.Finalizers = []string{
786+
"olm.operatorframework.io/teardown",
787+
}
788+
rev1.Spec.LifecycleState = ocv1.ClusterExtensionRevisionLifecycleStateArchived
789+
return []client.Object{rev1, ext}
790+
},
791+
revisionEngineTeardownFn: func(t *testing.T) func(ctx context.Context, rev machinerytypes.Revision, opts ...machinerytypes.RevisionTeardownOption) (machinery.RevisionTeardownResult, error) {
792+
return func(ctx context.Context, rev machinerytypes.Revision, opts ...machinerytypes.RevisionTeardownOption) (machinery.RevisionTeardownResult, error) {
793+
return &mockRevisionTeardownResult{
794+
isComplete: false,
795+
}, nil
796+
}
797+
},
798+
expectedResult: ctrl.Result{RequeueAfter: 5 * time.Second},
799+
validate: func(t *testing.T, c client.Client) {
800+
rev := &ocv1.ClusterExtensionRevision{}
801+
err := c.Get(t.Context(), client.ObjectKey{
802+
Name: clusterExtensionRevisionName,
803+
}, rev)
804+
require.NoError(t, err)
805+
cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterExtensionRevisionTypeProgressing)
806+
require.NotNil(t, cond)
807+
require.Equal(t, metav1.ConditionTrue, cond.Status)
808+
require.Equal(t, ocv1.ClusterExtensionRevisionReasonRetrying, cond.Reason)
809+
require.Equal(t, "tearing down revision", cond.Message)
810+
811+
// Finalizer should still be present
812+
require.Contains(t, rev.Finalizers, "olm.operatorframework.io/teardown")
813+
},
814+
},
815+
{
816+
name: "return error and set retrying conditions when archived revision teardown fails",
817+
revisionResult: mockRevisionResult{},
818+
existingObjs: func() []client.Object {
819+
ext := newTestClusterExtension()
820+
rev1 := newTestClusterExtensionRevision(t, clusterExtensionRevisionName, ext, testScheme)
821+
rev1.Finalizers = []string{
822+
"olm.operatorframework.io/teardown",
823+
}
824+
rev1.Spec.LifecycleState = ocv1.ClusterExtensionRevisionLifecycleStateArchived
825+
return []client.Object{rev1, ext}
826+
},
827+
revisionEngineTeardownFn: func(t *testing.T) func(ctx context.Context, rev machinerytypes.Revision, opts ...machinerytypes.RevisionTeardownOption) (machinery.RevisionTeardownResult, error) {
828+
return func(ctx context.Context, rev machinerytypes.Revision, opts ...machinerytypes.RevisionTeardownOption) (machinery.RevisionTeardownResult, error) {
829+
return nil, fmt.Errorf("teardown failed: connection refused")
830+
}
831+
},
832+
expectedErr: "error tearing down revision",
833+
validate: func(t *testing.T, c client.Client) {
834+
rev := &ocv1.ClusterExtensionRevision{}
835+
err := c.Get(t.Context(), client.ObjectKey{
836+
Name: clusterExtensionRevisionName,
837+
}, rev)
838+
require.NoError(t, err)
839+
cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterExtensionRevisionTypeProgressing)
840+
require.NotNil(t, cond)
841+
require.Equal(t, metav1.ConditionTrue, cond.Status)
842+
require.Equal(t, ocv1.ClusterExtensionRevisionReasonRetrying, cond.Reason)
843+
require.Contains(t, cond.Message, "teardown failed: connection refused")
844+
845+
// Finalizer should still be present
846+
require.Contains(t, rev.Finalizers, "olm.operatorframework.io/teardown")
847+
},
848+
},
778849
{
779850
name: "revision is torn down when in archived state and finalizer is removed",
780851
revisionResult: mockRevisionResult{},
@@ -847,7 +918,7 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_Deletion(t *testing.T) {
847918
})
848919

849920
// reconcile cluster extension revision
850-
require.Equal(t, ctrl.Result{}, result)
921+
require.Equal(t, tc.expectedResult, result)
851922
if tc.expectedErr != "" {
852923
require.Contains(t, err.Error(), tc.expectedErr)
853924
} else {

‎testdata/images/bundles/test-operator/v1.0.0/manifests/dummy.configmap.yaml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
apiVersion: v1
2+
kind: ConfigMap
3+
metadata:
4+
name: dummy-configmap
5+
data:
6+
why: "this config map does not exist in higher versions of the bundle - it is useful to test whether resources removed between versions are removed from the cluster as well"

0 commit comments

Comments
 (0)