Skip to content

Commit 511d4be

Browse files
author
Per Goncalves da Silva
committed
Surface revision retrying condition to ClusterExtension
Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>
1 parent 9be3edf commit 511d4be

File tree

3 files changed

+24
-62
lines changed

3 files changed

+24
-62
lines changed

‎internal/operator-controller/applier/boxcutter.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -363,6 +363,9 @@ func (bc *Boxcutter) apply(ctx context.Context, contentFS fs.FS, ext *ocv1.Clust
363363
if progressingCondition == nil && availableCondition == nil && succeededCondition == nil {
364364
return false, "New revision created", nil
365365
} else if progressingCondition != nil && progressingCondition.Status == metav1.ConditionTrue {
366+
if progressingCondition.Reason == ocv1.ClusterExtensionRevisionReasonRetrying {
367+
return false, "", errors.New(progressingCondition.Message)
368+
}
366369
return false, progressingCondition.Message, nil
367370
} else if availableCondition != nil && availableCondition.Status != metav1.ConditionTrue {
368371
return false, "", errors.New(availableCondition.Message)

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev
212212
l.Error(fmt.Errorf("object collision detected"), "object collision, retrying after 10s", "phase", i, "collisions", collidingObjs)
213213
// collisions are probably stickier than phase roll out probe failures - so we'd probably want to set
214214
// Progressing to false here due to the collision
215-
markAsNotProgressing(rev, ocv1.ClusterExtensionRevisionReasonRetrying, fmt.Sprintf("revision object collisions in phase %d\n%s", i, strings.Join(collidingObjs, "\n\n")))
215+
markAsProgressing(rev, ocv1.ClusterExtensionRevisionReasonRetrying, fmt.Sprintf("revision object collisions in phase %d\n%s", i, strings.Join(collidingObjs, "\n\n")))
216216

217217
// NOTE(pedjak): not sure if we want to retry here - collisions are probably not transient?
218218
return ctrl.Result{RequeueAfter: 10 * time.Second}, nil

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

Lines changed: 20 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,9 @@ import (
3333
"github.com/operator-framework/operator-controller/internal/operator-controller/labels"
3434
)
3535

36-
func Test_ClusterExtensionRevisionReconciler_Reconcile_RevisionReconciliation(t *testing.T) {
37-
const (
38-
clusterExtensionRevisionName = "test-ext-1"
39-
)
36+
const clusterExtensionRevisionName = "test-ext-1"
4037

38+
func Test_ClusterExtensionRevisionReconciler_Reconcile_RevisionReconciliation(t *testing.T) {
4139
testScheme := newScheme(t)
4240

4341
for _, tc := range []struct {
@@ -52,7 +50,7 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_RevisionReconciliation(t
5250
revisionResult: mockRevisionResult{},
5351
existingObjs: func() []client.Object {
5452
ext := newTestClusterExtension()
55-
rev1 := newTestClusterExtensionRevision(clusterExtensionRevisionName)
53+
rev1 := newTestClusterExtensionRevision()
5654
require.NoError(t, controllerutil.SetControllerReference(ext, rev1, testScheme))
5755
return []client.Object{ext, rev1}
5856
},
@@ -70,7 +68,7 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_RevisionReconciliation(t
7068
revisionResult: mockRevisionResult{},
7169
existingObjs: func() []client.Object {
7270
ext := newTestClusterExtension()
73-
rev1 := newTestClusterExtensionRevision(clusterExtensionRevisionName)
71+
rev1 := newTestClusterExtensionRevision()
7472
require.NoError(t, controllerutil.SetControllerReference(ext, rev1, testScheme))
7573
return []client.Object{ext, rev1}
7674
},
@@ -161,7 +159,7 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_RevisionReconciliation(t
161159
},
162160
existingObjs: func() []client.Object {
163161
ext := newTestClusterExtension()
164-
rev1 := newTestClusterExtensionRevision(clusterExtensionRevisionName)
162+
rev1 := newTestClusterExtensionRevision()
165163
require.NoError(t, controllerutil.SetControllerReference(ext, rev1, testScheme))
166164
return []client.Object{ext, rev1}
167165
},
@@ -252,7 +250,7 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_RevisionReconciliation(t
252250
},
253251
existingObjs: func() []client.Object {
254252
ext := newTestClusterExtension()
255-
rev1 := newTestClusterExtensionRevision(clusterExtensionRevisionName)
253+
rev1 := newTestClusterExtensionRevision()
256254
require.NoError(t, controllerutil.SetControllerReference(ext, rev1, testScheme))
257255
return []client.Object{ext, rev1}
258256
},
@@ -276,7 +274,7 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_RevisionReconciliation(t
276274

277275
existingObjs: func() []client.Object {
278276
ext := newTestClusterExtension()
279-
rev1 := newTestClusterExtensionRevision(clusterExtensionRevisionName)
277+
rev1 := newTestClusterExtensionRevision()
280278
require.NoError(t, controllerutil.SetControllerReference(ext, rev1, testScheme))
281279
return []client.Object{ext, rev1}
282280
},
@@ -301,7 +299,7 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_RevisionReconciliation(t
301299
},
302300
existingObjs: func() []client.Object {
303301
ext := newTestClusterExtension()
304-
rev1 := newTestClusterExtensionRevision(clusterExtensionRevisionName)
302+
rev1 := newTestClusterExtensionRevision()
305303
require.NoError(t, controllerutil.SetControllerReference(ext, rev1, testScheme))
306304
return []client.Object{ext, rev1}
307305
},
@@ -326,7 +324,7 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_RevisionReconciliation(t
326324
},
327325
existingObjs: func() []client.Object {
328326
ext := newTestClusterExtension()
329-
rev1 := newTestClusterExtensionRevision(clusterExtensionRevisionName)
327+
rev1 := newTestClusterExtensionRevision()
330328
require.NoError(t, controllerutil.SetControllerReference(ext, rev1, testScheme))
331329
meta.SetStatusCondition(&rev1.Status.Conditions, metav1.Condition{
332330
Type: ocv1.TypeProgressing,
@@ -358,7 +356,7 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_RevisionReconciliation(t
358356
},
359357
existingObjs: func() []client.Object {
360358
ext := newTestClusterExtension()
361-
rev1 := newTestClusterExtensionRevision(clusterExtensionRevisionName)
359+
rev1 := newTestClusterExtensionRevision()
362360
require.NoError(t, controllerutil.SetControllerReference(ext, rev1, testScheme))
363361
return []client.Object{ext, rev1}
364362
},
@@ -383,45 +381,6 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_RevisionReconciliation(t
383381
require.Equal(t, int64(1), cond.ObservedGeneration)
384382
},
385383
},
386-
{
387-
name: "archive previous revisions on successful rollout",
388-
revisionResult: mockRevisionResult{
389-
isComplete: true,
390-
},
391-
existingObjs: func() []client.Object {
392-
ext := newTestClusterExtension()
393-
prevRev1 := newTestClusterExtensionRevision("prev-rev-1")
394-
require.NoError(t, controllerutil.SetControllerReference(ext, prevRev1, testScheme))
395-
prevRev2 := newTestClusterExtensionRevision("prev-rev-2")
396-
require.NoError(t, controllerutil.SetControllerReference(ext, prevRev2, testScheme))
397-
rev1 := newTestClusterExtensionRevision("test-ext-1")
398-
rev1.Spec.Previous = []ocv1.ClusterExtensionRevisionPrevious{
399-
{
400-
Name: "prev-rev-1",
401-
UID: "prev-rev-1",
402-
}, {
403-
Name: "prev-rev-2",
404-
UID: "prev-rev-2",
405-
},
406-
}
407-
require.NoError(t, controllerutil.SetControllerReference(ext, rev1, testScheme))
408-
return []client.Object{ext, prevRev1, prevRev2, rev1}
409-
},
410-
validate: func(t *testing.T, c client.Client) {
411-
rev := &ocv1.ClusterExtensionRevision{}
412-
err := c.Get(t.Context(), client.ObjectKey{
413-
Name: "prev-rev-1",
414-
}, rev)
415-
require.NoError(t, err)
416-
require.Equal(t, ocv1.ClusterExtensionRevisionLifecycleStateArchived, rev.Spec.LifecycleState)
417-
418-
err = c.Get(t.Context(), client.ObjectKey{
419-
Name: "prev-rev-2",
420-
}, rev)
421-
require.NoError(t, err)
422-
require.Equal(t, ocv1.ClusterExtensionRevisionLifecycleStateArchived, rev.Spec.LifecycleState)
423-
},
424-
},
425384
} {
426385
t.Run(tc.name, func(t *testing.T) {
427386
// create extension and cluster extension
@@ -541,7 +500,7 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_ValidationError_Retries(t
541500
} {
542501
t.Run(tc.name, func(t *testing.T) {
543502
ext := newTestClusterExtension()
544-
rev1 := newTestClusterExtensionRevision(clusterExtensionRevisionName)
503+
rev1 := newTestClusterExtensionRevision()
545504
require.NoError(t, controllerutil.SetControllerReference(ext, rev1, testScheme))
546505

547506
// create extension and cluster extension
@@ -595,7 +554,7 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_Deletion(t *testing.T) {
595554
name: "teardown finalizer is removed",
596555
revisionResult: mockRevisionResult{},
597556
existingObjs: func() []client.Object {
598-
rev1 := newTestClusterExtensionRevision(clusterExtensionRevisionName)
557+
rev1 := newTestClusterExtensionRevision()
599558
rev1.Finalizers = []string{
600559
"olm.operatorframework.io/teardown",
601560
}
@@ -618,7 +577,7 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_Deletion(t *testing.T) {
618577
revisionResult: mockRevisionResult{},
619578
existingObjs: func() []client.Object {
620579
ext := newTestClusterExtension()
621-
rev1 := newTestClusterExtensionRevision(clusterExtensionRevisionName)
580+
rev1 := newTestClusterExtensionRevision()
622581
rev1.Finalizers = []string{
623582
"olm.operatorframework.io/teardown",
624583
}
@@ -648,7 +607,7 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_Deletion(t *testing.T) {
648607
revisionResult: mockRevisionResult{},
649608
existingObjs: func() []client.Object {
650609
ext := newTestClusterExtension()
651-
rev1 := newTestClusterExtensionRevision(clusterExtensionRevisionName)
610+
rev1 := newTestClusterExtensionRevision()
652611
rev1.Finalizers = []string{
653612
"olm.operatorframework.io/teardown",
654613
}
@@ -677,7 +636,7 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_Deletion(t *testing.T) {
677636
revisionResult: mockRevisionResult{},
678637
existingObjs: func() []client.Object {
679638
ext := newTestClusterExtension()
680-
rev1 := newTestClusterExtensionRevision(clusterExtensionRevisionName)
639+
rev1 := newTestClusterExtensionRevision()
681640
rev1.Finalizers = []string{
682641
"olm.operatorframework.io/teardown",
683642
}
@@ -718,7 +677,7 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_Deletion(t *testing.T) {
718677
revisionResult: mockRevisionResult{},
719678
existingObjs: func() []client.Object {
720679
ext := newTestClusterExtension()
721-
rev1 := newTestClusterExtensionRevision(clusterExtensionRevisionName)
680+
rev1 := newTestClusterExtensionRevision()
722681
rev1.Finalizers = []string{
723682
"olm.operatorframework.io/teardown",
724683
}
@@ -761,7 +720,7 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_Deletion(t *testing.T) {
761720
revisionResult: mockRevisionResult{},
762721
existingObjs: func() []client.Object {
763722
ext := newTestClusterExtension()
764-
rev1 := newTestClusterExtensionRevision(clusterExtensionRevisionName)
723+
rev1 := newTestClusterExtensionRevision()
765724
rev1.Finalizers = []string{
766725
"olm.operatorframework.io/teardown",
767726
}
@@ -845,11 +804,11 @@ func newTestClusterExtension() *ocv1.ClusterExtension {
845804
}
846805
}
847806

848-
func newTestClusterExtensionRevision(name string) *ocv1.ClusterExtensionRevision {
807+
func newTestClusterExtensionRevision() *ocv1.ClusterExtensionRevision {
849808
return &ocv1.ClusterExtensionRevision{
850809
ObjectMeta: metav1.ObjectMeta{
851-
Name: name,
852-
UID: types.UID(name),
810+
Name: clusterExtensionRevisionName,
811+
UID: types.UID(clusterExtensionRevisionName),
853812
Generation: int64(1),
854813
Annotations: map[string]string{
855814
labels.PackageNameKey: "some-package",

0 commit comments

Comments
 (0)