Revert "Merge pull request #572 from openshift-bot/synchronize-upstream"

This reverts commit 24cf078, reversing
changes made to 93bf7ab.

cmd/catalogd/main.go

@@ -59,7 +59,6 @@ import (
 	"github.com/operator-framework/operator-controller/internal/catalogd/storage"
 	"github.com/operator-framework/operator-controller/internal/catalogd/webhook"
 	sharedcontrollers "github.com/operator-framework/operator-controller/internal/shared/controllers"
-	cacheutil "github.com/operator-framework/operator-controller/internal/shared/util/cache"
 	fsutil "github.com/operator-framework/operator-controller/internal/shared/util/fs"
 	httputil "github.com/operator-framework/operator-controller/internal/shared/util/http"
 	imageutil "github.com/operator-framework/operator-controller/internal/shared/util/image"
@@ -255,8 +254,6 @@ func run(ctx context.Context) error {
 
 	cacheOptions := crcache.Options{
 		ByObject: map[client.Object]crcache.ByObject{},
-		// Memory optimization: strip managed fields and large annotations from cached objects
-		DefaultTransform: cacheutil.StripManagedFieldsAndAnnotations(),
 	}
 
 	saKey, err := sautil.GetServiceAccount()

cmd/operator-controller/main.go

@@ -78,7 +78,6 @@ import (
 	"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render/registryv1"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/scheme"
 	sharedcontrollers "github.com/operator-framework/operator-controller/internal/shared/controllers"
-	cacheutil "github.com/operator-framework/operator-controller/internal/shared/util/cache"
 	fsutil "github.com/operator-framework/operator-controller/internal/shared/util/fs"
 	httputil "github.com/operator-framework/operator-controller/internal/shared/util/http"
 	imageutil "github.com/operator-framework/operator-controller/internal/shared/util/image"
@@ -258,8 +257,6 @@ func run() error {
 			cfg.systemNamespace: {LabelSelector: k8slabels.Everything()},
 		},
 		DefaultLabelSelector: k8slabels.Nothing(),
-		// Memory optimization: strip managed fields and large annotations from cached objects
-		DefaultTransform: cacheutil.StripAnnotations(),
 	}
 
 	if features.OperatorControllerFeatureGate.Enabled(features.BoxcutterRuntime) {

commitchecker.yaml

@@ -1,4 +1,4 @@
-expectedMergeBase: 4e349e62c5314574f6194d64b1ff4508f2e9331f
+expectedMergeBase: 34394ce0a7067e1f1622dc2c381a9a12439b10d2
 upstreamBranch: main
 upstreamOrg: operator-framework
 upstreamRepo: operator-controller

internal/catalogd/controllers/core/clustercatalog_controller.go

@@ -41,7 +41,6 @@ import (
 	ocv1 "github.com/operator-framework/operator-controller/api/v1"
 	"github.com/operator-framework/operator-controller/internal/catalogd/storage"
 	imageutil "github.com/operator-framework/operator-controller/internal/shared/util/image"
-	k8sutil "github.com/operator-framework/operator-controller/internal/shared/util/k8s"
 )
 
 const (
@@ -108,16 +107,16 @@ func (r *ClusterCatalogReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 	// Do checks before any Update()s, as Update() may modify the resource structure!
 	updateStatus := !equality.Semantic.DeepEqual(existingCatsrc.Status, reconciledCatsrc.Status)
 	updateFinalizers := !equality.Semantic.DeepEqual(existingCatsrc.Finalizers, reconciledCatsrc.Finalizers)
-	unexpectedFieldsChanged := k8sutil.CheckForUnexpectedFieldChange(&existingCatsrc, reconciledCatsrc)
+	unexpectedFieldsChanged := checkForUnexpectedFieldChange(existingCatsrc, *reconciledCatsrc)
 
 	if unexpectedFieldsChanged {
 		panic("spec or metadata changed by reconciler")
 	}
 
 	// Save the finalizers off to the side. If we update the status, the reconciledCatsrc will be updated
 	// to contain the new state of the ClusterCatalog, which contains the status update, but (critically)
-	// does not contain the finalizers. After the status update, we will use the saved finalizers in the
-	// CreateOrPatch()
+	// does not contain the finalizers. After the status update, we need to re-add the finalizers to the
+	// reconciledCatsrc before updating the object.
 	finalizers := reconciledCatsrc.Finalizers
 
 	if updateStatus {
@@ -126,12 +125,10 @@ func (r *ClusterCatalogReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 		}
 	}
 
+	reconciledCatsrc.Finalizers = finalizers
+
 	if updateFinalizers {
-		// Use CreateOrPatch to update finalizers on the server
-		if _, err := controllerutil.CreateOrPatch(ctx, r.Client, reconciledCatsrc, func() error {
-			reconciledCatsrc.Finalizers = finalizers
-			return nil
-		}); err != nil {
+		if err := r.Update(ctx, reconciledCatsrc); err != nil {
 			reconcileErr = errors.Join(reconcileErr, fmt.Errorf("error updating finalizers: %v", err))
 		}
 	}
@@ -418,6 +415,13 @@ func (r *ClusterCatalogReconciler) needsPoll(lastSuccessfulPoll time.Time, catal
 	return nextPoll.Before(time.Now())
 }
 
+// Compare resources - ignoring status & metadata.finalizers
+func checkForUnexpectedFieldChange(a, b ocv1.ClusterCatalog) bool {
+	a.Status, b.Status = ocv1.ClusterCatalogStatus{}, ocv1.ClusterCatalogStatus{}
+	a.Finalizers, b.Finalizers = []string{}, []string{}
+	return !equality.Semantic.DeepEqual(a, b)
+}
+
 type finalizerFunc func(ctx context.Context, obj client.Object) (crfinalizer.Result, error)
 
 func (f finalizerFunc) Finalize(ctx context.Context, obj client.Object) (crfinalizer.Result, error) {

internal/operator-controller/applier/boxcutter.go

@@ -27,7 +27,6 @@ import (
 
 	ocv1 "github.com/operator-framework/operator-controller/api/v1"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/labels"
-	"github.com/operator-framework/operator-controller/internal/shared/util/cache"
 )
 
 const (
@@ -67,9 +66,6 @@ func (r *SimpleRevisionGenerator) GenerateRevisionFromHelmRelease(
 		maps.Copy(labels, objectLabels)
 		obj.SetLabels(labels)
 
-		// Memory optimization: strip large annotations
-		// Note: ApplyStripTransform never returns an error in practice
-		_ = cache.ApplyStripAnnotationsTransform(&obj)
 		sanitizedUnstructured(ctx, &obj)
 
 		objs = append(objs, ocv1.ClusterExtensionRevisionObject{
@@ -121,10 +117,6 @@ func (r *SimpleRevisionGenerator) GenerateRevision(
 		unstr := unstructured.Unstructured{Object: unstrObj}
 		unstr.SetGroupVersionKind(gvk)
 
-		// Memory optimization: strip large annotations
-		if err := cache.ApplyStripAnnotationsTransform(&unstr); err != nil {
-			return nil, err
-		}
 		sanitizedUnstructured(ctx, &unstr)
 
 		objs = append(objs, ocv1.ClusterExtensionRevisionObject{

internal/operator-controller/controllers/clusterextension_controller.go

@@ -35,7 +35,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/builder"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	crcontroller "sigs.k8s.io/controller-runtime/pkg/controller"
-	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/controller-runtime/pkg/event"
 	crhandler "sigs.k8s.io/controller-runtime/pkg/handler"
 	"sigs.k8s.io/controller-runtime/pkg/log"
@@ -49,7 +48,6 @@ import (
 	ocv1 "github.com/operator-framework/operator-controller/api/v1"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/conditionsets"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/labels"
-	k8sutil "github.com/operator-framework/operator-controller/internal/shared/util/k8s"
 )
 
 const (
@@ -137,28 +135,25 @@ func (r *ClusterExtensionReconciler) Reconcile(ctx context.Context, req ctrl.Req
 	updateFinalizers := !equality.Semantic.DeepEqual(existingExt.Finalizers, reconciledExt.Finalizers)
 
 	// If any unexpected fields have changed, panic before updating the resource
-	unexpectedFieldsChanged := k8sutil.CheckForUnexpectedFieldChange(existingExt, reconciledExt)
+	unexpectedFieldsChanged := checkForUnexpectedClusterExtensionFieldChange(*existingExt, *reconciledExt)
 	if unexpectedFieldsChanged {
 		panic("spec or metadata changed by reconciler")
 	}
 
 	// Save the finalizers off to the side. If we update the status, the reconciledExt will be updated
 	// to contain the new state of the ClusterExtension, which contains the status update, but (critically)
-	// does not contain the finalizers. After the status update, we will use the saved finalizers in the
-	// CreateOrPatch()
+	// does not contain the finalizers. After the status update, we need to re-add the finalizers to the
+	// reconciledExt before updating the object.
 	finalizers := reconciledExt.Finalizers
 	if updateStatus {
 		if err := r.Client.Status().Update(ctx, reconciledExt); err != nil {
 			reconcileErr = errors.Join(reconcileErr, fmt.Errorf("error updating status: %v", err))
 		}
 	}
+	reconciledExt.Finalizers = finalizers
 
 	if updateFinalizers {
-		// Use CreateOrPatch to update finalizers on the server
-		if _, err := controllerutil.CreateOrPatch(ctx, r.Client, reconciledExt, func() error {
-			reconciledExt.Finalizers = finalizers
-			return nil
-		}); err != nil {
+		if err := r.Update(ctx, reconciledExt); err != nil {
 			reconcileErr = errors.Join(reconcileErr, fmt.Errorf("error updating finalizers: %v", err))
 		}
 	}
@@ -184,6 +179,13 @@ func ensureAllConditionsWithReason(ext *ocv1.ClusterExtension, reason v1alpha1.C
 	}
 }
 
+// Compare resources - ignoring status & metadata.finalizers
+func checkForUnexpectedClusterExtensionFieldChange(a, b ocv1.ClusterExtension) bool {
+	a.Status, b.Status = ocv1.ClusterExtensionStatus{}, ocv1.ClusterExtensionStatus{}
+	a.Finalizers, b.Finalizers = []string{}, []string{}
+	return !equality.Semantic.DeepEqual(a, b)
+}
+
 // SetDeprecationStatus will set the appropriate deprecation statuses for a ClusterExtension
 // based on the provided bundle
 func SetDeprecationStatus(ext *ocv1.ClusterExtension, bundleName string, deprecation *declcfg.Deprecation) {