CRE Previous Limit

Sets a limit to the number of previous ClusterExtensionRevisions we keep in the cluster, and trims the list of previous revisions when creating new revisions to stay at the limit. The limit has been set to 5 for now. Any revisions beyond this limit will be removed from the cluster.

Signed-off-by: Daniel Franz <dfranz@redhat.com>

Author: dtfranz

internal/operator-controller/applier/boxcutter.go

@@ -16,6 +16,7 @@ import (
 	"github.com/davecgh/go-spew/spew"
 	"helm.sh/helm/v3/pkg/release"
 	"helm.sh/helm/v3/pkg/storage/driver"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -35,7 +36,8 @@ import (
 )
 
 const (
-	RevisionHashAnnotation = "olm.operatorframework.io/hash"
+	RevisionHashAnnotation                = "olm.operatorframework.io/hash"
+	ClusterExtensionRevisionPreviousLimit = 5
 )
 
 type ClusterExtensionRevisionGenerator interface {
@@ -292,6 +294,10 @@ func (bc *Boxcutter) apply(ctx context.Context, contentFS fs.FS, ext *ocv1.Clust
 			})
 		}
 
+		if err = bc.garbageCollectPreviousRevisions(ctx, newRevision); err != nil {
+			return false, "", fmt.Errorf("garbage collecting old Revisions: %w", err)
+		}
+
 		if err := controllerutil.SetControllerReference(ext, newRevision, bc.Scheme); err != nil {
 			return false, "", fmt.Errorf("set ownerref: %w", err)
 		}
@@ -301,8 +307,6 @@ func (bc *Boxcutter) apply(ctx context.Context, contentFS fs.FS, ext *ocv1.Clust
 		currentRevision = newRevision
 	}
 
-	// TODO: Delete archived previous revisions over a certain revision limit
-
 	progressingCondition := meta.FindStatusCondition(currentRevision.Status.Conditions, ocv1.TypeProgressing)
 	availableCondition := meta.FindStatusCondition(currentRevision.Status.Conditions, ocv1.ClusterExtensionRevisionTypeAvailable)
 	succeededCondition := meta.FindStatusCondition(currentRevision.Status.Conditions, ocv1.ClusterExtensionRevisionTypeSucceeded)
@@ -319,6 +323,31 @@ func (bc *Boxcutter) apply(ctx context.Context, contentFS fs.FS, ext *ocv1.Clust
 	return true, "", nil
 }
 
+// garbageCollectPreviousRevisions trims the list of previous revisions down to ClusterExtensionRevisionPreviousLimit and deletes
+// the corresponding revisions from the cluster
+func (bc *Boxcutter) garbageCollectPreviousRevisions(ctx context.Context, revision *ocv1.ClusterExtensionRevision) error {
+	if revision == nil || len(revision.Spec.Previous) <= ClusterExtensionRevisionPreviousLimit {
+		return nil
+	}
+	for index, r := range revision.Spec.Previous {
+		if index < len(revision.Spec.Previous)-ClusterExtensionRevisionPreviousLimit {
+			// Delete oldest CREs from the cluster and list to reach ClusterExtensionRevisionPreviousLimit
+			if err := bc.Client.Delete(ctx, &ocv1.ClusterExtensionRevision{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: r.Name,
+				},
+			}); err != nil && !apierrors.IsNotFound(err) {
+				return fmt.Errorf("deleting previous Revision: %w", err)
+			}
+		} else {
+			break
+		}
+	}
+	// trim the list of previous revisions in place by preserving revisions starting from index len(revisions) - ClusterExtensionRevisionPreviousLimit
+	revision.Spec.Previous = revision.Spec.Previous[len(revision.Spec.Previous)-ClusterExtensionRevisionPreviousLimit:]
+	return nil
+}
+
 // getExistingRevisions returns the list of ClusterExtensionRevisions for a ClusterExtension with name extName in revision order (oldest to newest)
 func (bc *Boxcutter) getExistingRevisions(ctx context.Context, extName string) ([]ocv1.ClusterExtensionRevision, error) {
 	existingRevisionList := &ocv1.ClusterExtensionRevisionList{}

internal/operator-controller/applier/boxcutter_test.go

@@ -15,6 +15,7 @@ import (
 	"helm.sh/helm/v3/pkg/storage/driver"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -567,6 +568,83 @@ func TestBoxcutter_Apply(t *testing.T) {
 				assert.Empty(t, revList.Items)
 			},
 		},
+		{
+			name: "sixth revision",
+			mockBuilder: &mockBundleRevisionBuilder{
+				makeRevisionFunc: func(bundleFS fs.FS, ext *ocv1.ClusterExtension, objectLabels, revisionAnnotations map[string]string) (*ocv1.ClusterExtensionRevision, error) {
+					return &ocv1.ClusterExtensionRevision{
+						ObjectMeta: metav1.ObjectMeta{
+							Annotations: revisionAnnotations,
+							Labels: map[string]string{
+								controllers.ClusterExtensionRevisionOwnerLabel: ext.Name,
+							},
+						},
+						Spec: ocv1.ClusterExtensionRevisionSpec{},
+					}, nil
+				},
+			},
+			existingObjs: []client.Object{
+				&ocv1.ClusterExtensionRevision{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "rev-1",
+						Labels: map[string]string{
+							controllers.ClusterExtensionRevisionOwnerLabel: ext.Name,
+						},
+					},
+				},
+				&ocv1.ClusterExtensionRevision{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "rev-2",
+						Labels: map[string]string{
+							controllers.ClusterExtensionRevisionOwnerLabel: ext.Name,
+						},
+					},
+				},
+				&ocv1.ClusterExtensionRevision{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "rev-3",
+						Labels: map[string]string{
+							controllers.ClusterExtensionRevisionOwnerLabel: ext.Name,
+						},
+					},
+				},
+				&ocv1.ClusterExtensionRevision{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "rev-4",
+						Labels: map[string]string{
+							controllers.ClusterExtensionRevisionOwnerLabel: ext.Name,
+						},
+					},
+				},
+				&ocv1.ClusterExtensionRevision{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "rev-5",
+						Labels: map[string]string{
+							controllers.ClusterExtensionRevisionOwnerLabel: ext.Name,
+						},
+					},
+				},
+				&ocv1.ClusterExtensionRevision{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "rev-6",
+						Labels: map[string]string{
+							controllers.ClusterExtensionRevisionOwnerLabel: ext.Name,
+						},
+					},
+				},
+			},
+			validate: func(t *testing.T, c client.Client) {
+				rev1 := &ocv1.ClusterExtensionRevision{}
+				err := c.Get(t.Context(), client.ObjectKey{Name: "rev-1"}, rev1)
+				require.Error(t, err)
+				assert.True(t, apierrors.IsNotFound(err))
+
+				latest := &ocv1.ClusterExtensionRevision{}
+				err = c.Get(t.Context(), client.ObjectKey{Name: "test-ext-1"}, latest)
+				require.NoError(t, err)
+				assert.Len(t, latest.Spec.Previous, applier.ClusterExtensionRevisionPreviousLimit)
+			},
+		},
 	}
 
 	for _, tc := range testCases {