Add 3 tier revision engine

Signed-off-by: Per G. da Silva <pegoncal@redhat.com>

Author: Per G. da Silva

api/v1/clusterobjectset_types.go

@@ -518,7 +518,7 @@ type ClusterObjectSetStatus struct {
 	// different content. Each entry covers all fully-resolved object
 	// manifests within a phase, making it source-agnostic.
 	//
-	// +kubebuilder:validation:XValidation:rule="self == oldSelf || oldSelf.size() == 0",message="observedPhases is immutable"
+	// +kubebuilder:validation:XValidation:rule="oldSelf.size() == 0 || (self.size() == oldSelf.size() && oldSelf.all(o, self.exists(n, n.name == o.name)))",message="observedPhases: phases cannot be added or removed once set"
 	// +kubebuilder:validation:MaxItems=20
 	// +listType=map
 	// +listMapKey=name
@@ -527,22 +527,31 @@ type ClusterObjectSetStatus struct {
 }
 
 // ObservedPhase records the observed content digest of a resolved phase.
+// +kubebuilder:validation:XValidation:rule="!has(oldSelf.completedAt) || (has(self.completedAt) && self.completedAt == oldSelf.completedAt)",message="completedAt is immutable once set"
 type ObservedPhase struct {
 	// name is the phase name matching a phase in spec.phases.
 	//
 	// +required
 	// +kubebuilder:validation:MinLength=1
 	// +kubebuilder:validation:MaxLength=63
 	// +kubebuilder:validation:XValidation:rule=`!format.dns1123Label().validate(self).hasValue()`,message="the value must consist of only lowercase alphanumeric characters and hyphens, and must start and end with an alphanumeric character."
+	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="name is immutable"
 	Name string `json:"name"`
 
+	// completedAt is the timestamp when this phase first became Complete.
+	// Set once and never cleared. Zero value means the phase has never been
+	// Complete.
+	// +optional
+	CompletedAt metav1.Time `json:"completedAt,omitzero"`
+
 	// digest is the digest of the phase's resolved object content
 	// at first successful resolution, in the format "<algorithm>:<hex>".
 	//
 	// +required
 	// +kubebuilder:validation:MinLength=1
 	// +kubebuilder:validation:MaxLength=256
 	// +kubebuilder:validation:XValidation:rule=`self.matches('^[a-z0-9]+:[a-f0-9]+$')`,message="digest must be in the format '<algorithm>:<hex>'"
+	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="digest is immutable"
 	Digest string `json:"digest"`
 }
 

api/v1/zz_generated.deepcopy.go

@@ -62,6 +62,7 @@ import (
 
 	ocv1 "github.com/operator-framework/operator-controller/api/v1"
 	clusterobjctrl "github.com/operator-framework/operator-controller/internal/object-controller/controllers"
+	"github.com/operator-framework/operator-controller/internal/object-controller/revision"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/action"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/applier"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/catalogmetadata/cache"
@@ -673,7 +674,7 @@ func (c *boxcutterReconcilerConfigurator) Configure(ceReconciler *controllers.Cl
 	// Wrap the discovery client with caching to reduce memory usage from repeated OpenAPI schema fetches
 	discoveryClient := memory.NewMemCacheClient(baseDiscoveryClient)
 
-	revisionEngineFactory, err := clusterobjctrl.NewDefaultRevisionEngineFactory(
+	revisionEngineFactory, err := revision.NewDefaultRevisionEngineFactory(
 		c.mgr.GetScheme(),
 		c.trackingCache,
 		discoveryClient,

applyconfigurations/api/v1/observedphase.go

@@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:
   annotations:
-    controller-gen.kubebuilder.io/version: v0.20.1
+    controller-gen.kubebuilder.io/version: v0.21.0
     olm.operatorframework.io/generator: experimental
   name: clusterobjectsets.olm.operatorframework.io
 spec:
@@ -632,6 +632,13 @@ spec:
                   description: ObservedPhase records the observed content digest of
                     a resolved phase.
                   properties:
+                    completedAt:
+                      description: |-
+                        completedAt is the timestamp when this phase first became Complete.
+                        Set once and never cleared. Zero value means the phase has never been
+                        Complete.
+                      format: date-time
+                      type: string
                     digest:
                       description: |-
                         digest is the digest of the phase's resolved object content
@@ -642,6 +649,8 @@ spec:
                       x-kubernetes-validations:
                       - message: digest must be in the format '<algorithm>:<hex>'
                         rule: self.matches('^[a-z0-9]+:[a-f0-9]+$')
+                      - message: digest is immutable
+                        rule: self == oldSelf
                     name:
                       description: name is the phase name matching a phase in spec.phases.
                       maxLength: 63
@@ -652,18 +661,26 @@ spec:
                           characters and hyphens, and must start and end with an alphanumeric
                           character.
                         rule: '!format.dns1123Label().validate(self).hasValue()'
+                      - message: name is immutable
+                        rule: self == oldSelf
                   required:
                   - digest
                   - name
                   type: object
+                  x-kubernetes-validations:
+                  - message: completedAt is immutable once set
+                    rule: '!has(oldSelf.completedAt) || (has(self.completedAt) &&
+                      self.completedAt == oldSelf.completedAt)'
                 maxItems: 20
                 type: array
                 x-kubernetes-list-map-keys:
                 - name
                 x-kubernetes-list-type: map
                 x-kubernetes-validations:
-                - message: observedPhases is immutable
-                  rule: self == oldSelf || oldSelf.size() == 0
+                - message: 'observedPhases: phases cannot be added or removed once
+                    set'
+                  rule: oldSelf.size() == 0 || (self.size() == oldSelf.size() && oldSelf.all(o,
+                    self.exists(n, n.name == o.name)))
             type: object
         type: object
     served: true

applyconfigurations/internal/internal.go

@@ -42,6 +42,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/source"
 
 	ocv1 "github.com/operator-framework/operator-controller/api/v1"
+	"github.com/operator-framework/operator-controller/internal/object-controller/revision"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/labels"
 )
 
@@ -53,7 +54,7 @@ const (
 // as part of the boxcutter integration.
 type ClusterObjectSetReconciler struct {
 	Client                client.Client
-	RevisionEngineFactory RevisionEngineFactory
+	RevisionEngineFactory revision.EngineFactory
 	TrackingCache         trackingCache
 	Clock                 clock.Clock
 }
@@ -150,13 +151,13 @@ func (c *ClusterObjectSetReconciler) reconcile(ctx context.Context, cos *ocv1.Cl
 		return ctrl.Result{}, nil
 	}
 
-	revisionEngine, err := c.RevisionEngineFactory.CreateRevisionEngine(ctx, cos)
+	revisionEngine, err := c.RevisionEngineFactory.New(ctx, cos)
 	if err != nil {
 		setRetryingConditions(l, cos, err.Error(), isDeadlineExceeded)
 		return ctrl.Result{}, fmt.Errorf("failed to create revision engine: %v", err)
 	}
 
-	revision := boxcutter.NewRevisionWithOwner(
+	bcRevision := boxcutter.NewRevisionWithOwner(
 		cos.Name,
 		cos.Spec.Revision,
 		phases,
@@ -169,20 +170,20 @@ func (c *ClusterObjectSetReconciler) reconcile(ctx context.Context, cos *ocv1.Cl
 			markAsAvailableUnknown(cos, ocv1.ClusterObjectSetReasonReconciling, err.Error())
 			return ctrl.Result{}, fmt.Errorf("error stopping informers: %v", err)
 		}
-		return c.archive(ctx, revisionEngine, cos, revision)
+		return c.archive(ctx, revisionEngine, cos, bcRevision)
 	}
 
 	if err := c.ensureFinalizer(ctx, cos, clusterObjectSetTeardownFinalizer); err != nil {
 		return ctrl.Result{}, fmt.Errorf("error ensuring teardown finalizer: %v", err)
 	}
 
-	if err := c.establishWatch(ctx, cos, revision); err != nil {
+	if err := c.establishWatch(ctx, cos, bcRevision); err != nil {
 		werr := fmt.Errorf("establish watch: %v", err)
 		setRetryingConditions(l, cos, werr.Error(), isDeadlineExceeded)
 		return ctrl.Result{}, werr
 	}
 
-	rres, err := revisionEngine.Reconcile(ctx, revision, opts...)
+	rres, err := revisionEngine.Reconcile(ctx, bcRevision, opts...)
 	if err != nil {
 		if rres != nil {
 			// Log detailed reconcile reports only in debug mode (V(1)) to reduce verbosity.
@@ -200,6 +201,14 @@ func (c *ClusterObjectSetReconciler) reconcile(ctx context.Context, cos *ocv1.Cl
 		return ctrl.Result{RequeueAfter: 10 * time.Second}, nil
 	}
 
+	// Set phase completedAt
+	now := metav1.NewTime(time.Now())
+	for i, pres := range rres.GetPhases() {
+		if pres.IsComplete() && cos.Status.ObservedPhases[i].CompletedAt.IsZero() {
+			cos.Status.ObservedPhases[i].CompletedAt = now
+		}
+	}
+
 	for i, pres := range rres.GetPhases() {
 		if verr := pres.GetValidationError(); verr != nil {
 			l.Error(fmt.Errorf("%w", verr), "phase preflight validation failed, retrying after 10s", "phase", i)
@@ -308,7 +317,7 @@ func (c *ClusterObjectSetReconciler) delete(ctx context.Context, cos *ocv1.Clust
 	return ctrl.Result{}, nil
 }
 
-func (c *ClusterObjectSetReconciler) archive(ctx context.Context, revisionEngine RevisionEngine, cos *ocv1.ClusterObjectSet, revision boxcutter.RevisionBuilder) (ctrl.Result, error) {
+func (c *ClusterObjectSetReconciler) archive(ctx context.Context, revisionEngine revision.Engine, cos *ocv1.ClusterObjectSet, revision boxcutter.RevisionBuilder) (ctrl.Result, error) {
 	l := log.FromContext(ctx)
 	tdres, err := revisionEngine.Teardown(ctx, revision)
 	if err != nil {

cmd/operator-controller/main.go

@@ -29,6 +29,7 @@ import (
 
 	ocv1 "github.com/operator-framework/operator-controller/api/v1"
 	"github.com/operator-framework/operator-controller/internal/object-controller/controllers"
+	"github.com/operator-framework/operator-controller/internal/object-controller/revision"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/labels"
 	mockcontrollers "github.com/operator-framework/operator-controller/internal/testutil/mock/controllers"
 	mockmachinery "github.com/operator-framework/operator-controller/internal/testutil/mock/machinery"
@@ -1269,8 +1270,8 @@ func newMockTrackingCache(ctrl *gomock.Controller, cl client.Client, freeFn func
 
 // newNoopMockRevisionEngine creates a MockRevisionEngine with no expectations set.
 // Useful for tests where the engine is never called (e.g., error paths that fail before reaching the engine).
-func newNoopMockRevisionEngine(ctrl *gomock.Controller) *mockcontrollers.MockRevisionEngine {
-	return mockcontrollers.NewMockRevisionEngine(ctrl)
+func newNoopMockRevisionEngine(ctrl *gomock.Controller) *mockcontrollers.MockEngine {
+	return mockcontrollers.NewMockEngine(ctrl)
 }
 
 // newMockRevisionEngineWithReconcile creates a MockRevisionEngine with a Reconcile expectation.
@@ -1279,8 +1280,8 @@ func newMockRevisionEngineWithReconcile(
 	ctrl *gomock.Controller,
 	reconcileFn func(context.Context, machinerytypes.Revision, ...machinerytypes.RevisionReconcileOption) (machinery.RevisionResult, error),
 	teardownFn func(context.Context, machinerytypes.Revision, ...machinerytypes.RevisionTeardownOption) (machinery.RevisionTeardownResult, error),
-) *mockcontrollers.MockRevisionEngine {
-	m := mockcontrollers.NewMockRevisionEngine(ctrl)
+) *mockcontrollers.MockEngine {
+	m := mockcontrollers.NewMockEngine(ctrl)
 	m.EXPECT().Reconcile(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn(reconcileFn).AnyTimes()
 	if teardownFn != nil {
 		m.EXPECT().Teardown(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn(teardownFn).AnyTimes()
@@ -1292,12 +1293,12 @@ func newMockRevisionEngineWithReconcile(
 // that returns the given engine and error.
 func newMockRevisionEngineFactoryWithEngine(
 	ctrl *gomock.Controller,
-	engine controllers.RevisionEngine,
+	engine revision.Engine,
 	createErr error,
-) *mockcontrollers.MockRevisionEngineFactory {
-	m := mockcontrollers.NewMockRevisionEngineFactory(ctrl)
-	m.EXPECT().CreateRevisionEngine(gomock.Any(), gomock.Any()).DoAndReturn(
-		func(ctx context.Context, rev *ocv1.ClusterObjectSet) (controllers.RevisionEngine, error) {
+) *mockcontrollers.MockEngineFactory {
+	m := mockcontrollers.NewMockEngineFactory(ctrl)
+	m.EXPECT().New(gomock.Any(), gomock.Any()).DoAndReturn(
+		func(ctx context.Context, rev *ocv1.ClusterObjectSet) (revision.Engine, error) {
 			if createErr != nil {
 				return nil, createErr
 			}