fixup! Use Patch instead of Update for finalizer operations

Signed-off-by: Todd Short <[email protected]>

Author: tmshort

internal/catalogd/controllers/core/clustercatalog_controller.go

@@ -48,8 +48,7 @@ const (
 	fbcDeletionFinalizer         = "olm.operatorframework.io/delete-server-cache"
 	// CatalogSources are polled if PollInterval is mentioned, in intervals of wait.Jitter(pollDuration, maxFactor)
 	// wait.Jitter returns a time.Duration between pollDuration and pollDuration + maxFactor * pollDuration.
-	requeueJitterMaxFactor   = 0.01
-	lastAppliedConfiguration = "kubectl.kubernetes.io/last-applied-configuration"
+	requeueJitterMaxFactor = 0.01
 )
 
 // ClusterCatalogReconciler reconciles a Catalog object
@@ -106,7 +105,7 @@ 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)
-	unexpectedFieldsChanged := checkForUnexpectedFieldChange(existingCatsrc, *reconciledCatsrc)
+	unexpectedFieldsChanged := checkForUnexpectedFieldChange(&existingCatsrc, reconciledCatsrc)
 
 	if unexpectedFieldsChanged {
 		panic("spec or metadata changed by reconciler")
@@ -407,20 +406,15 @@ 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{}
-	a.ManagedFields, b.ManagedFields = nil, nil
-	a.ResourceVersion, b.ResourceVersion = "", ""
-	// Remove kubectl's last-applied-configuration annotation which may be added by the API server
-	if a.Annotations != nil {
-		delete(a.Annotations, lastAppliedConfiguration)
+// Compare resources - Annotations/Labels/Spec
+func checkForUnexpectedFieldChange(a, b *ocv1.ClusterCatalog) bool {
+	if !equality.Semantic.DeepEqual(a.ObjectMeta.Annotations, b.ObjectMeta.Annotations) {
+		return true
 	}
-	if b.Annotations != nil {
-		delete(b.Annotations, lastAppliedConfiguration)
+	if !equality.Semantic.DeepEqual(a.ObjectMeta.Labels, b.ObjectMeta.Labels) {
+		return true
 	}
-	return !equality.Semantic.DeepEqual(a, b)
+	return !equality.Semantic.DeepEqual(a.Spec, b.Spec)
 }
 
 func (r *ClusterCatalogReconciler) deleteStoredCatalog(catalogName string) {

internal/operator-controller/controllers/clusterextension_controller.go

@@ -117,7 +117,7 @@ func (r *ClusterExtensionReconciler) Reconcile(ctx context.Context, req ctrl.Req
 	updateStatus := !equality.Semantic.DeepEqual(existingExt.Status, reconciledExt.Status)
 
 	// If any unexpected fields have changed, panic before updating the resource
-	unexpectedFieldsChanged := checkForUnexpectedClusterExtensionFieldChange(*existingExt, *reconciledExt)
+	unexpectedFieldsChanged := checkForUnexpectedClusterExtensionFieldChange(existingExt, reconciledExt)
 	if unexpectedFieldsChanged {
 		panic("spec or metadata changed by reconciler")
 	}
@@ -149,20 +149,15 @@ func ensureAllConditionsWithReason(ext *ocv1.ClusterExtension, reason v1alpha1.C
 	}
 }
 
-// Compare resources - ignoring status, metadata.finalizers, metadata.resourceVersion, and metadata.managedFields
-func checkForUnexpectedClusterExtensionFieldChange(a, b ocv1.ClusterExtension) bool {
-	a.Status, b.Status = ocv1.ClusterExtensionStatus{}, ocv1.ClusterExtensionStatus{}
-	a.Finalizers, b.Finalizers = []string{}, []string{}
-	a.ResourceVersion, b.ResourceVersion = "", ""
-	a.ManagedFields, b.ManagedFields = nil, nil
-	// Remove kubectl's last-applied-configuration annotation which may be added by the API server
-	if a.Annotations != nil {
-		delete(a.Annotations, "kubectl.kubernetes.io/last-applied-configuration")
+// Compare resources - Annotations/Labels/Spec
+func checkForUnexpectedClusterExtensionFieldChange(a, b *ocv1.ClusterExtension) bool {
+	if !equality.Semantic.DeepEqual(a.ObjectMeta.Annotations, b.ObjectMeta.Annotations) {
+		return true
 	}
-	if b.Annotations != nil {
-		delete(b.Annotations, "kubectl.kubernetes.io/last-applied-configuration")
+	if !equality.Semantic.DeepEqual(a.ObjectMeta.Labels, b.ObjectMeta.Labels) {
+		return true
 	}
-	return !equality.Semantic.DeepEqual(a, b)
+	return !equality.Semantic.DeepEqual(a.Spec, b.Spec)
 }
 
 // Helper function to do the actual reconcile

internal/operator-controller/controllers/clusterextensionrevision_controller.go

@@ -82,7 +82,7 @@ func (c *ClusterExtensionRevisionReconciler) Reconcile(ctx context.Context, req
 	// Do checks before any Update()s, as Update() may modify the resource structure!
 	updateStatus := !equality.Semantic.DeepEqual(existingRev.Status, reconciledRev.Status)
 
-	unexpectedFieldsChanged := checkForUnexpectedClusterExtensionRevisionFieldChange(*existingRev, *reconciledRev)
+	unexpectedFieldsChanged := checkForUnexpectedClusterExtensionRevisionFieldChange(existingRev, reconciledRev)
 	if unexpectedFieldsChanged {
 		panic("spec or metadata changed by reconciler")
 	}
@@ -100,21 +100,13 @@ func (c *ClusterExtensionRevisionReconciler) Reconcile(ctx context.Context, req
 	return res, reconcileErr
 }
 
-// Compare resources - ignoring status & metadata.finalizers
-func checkForUnexpectedClusterExtensionRevisionFieldChange(a, b ocv1.ClusterExtensionRevision) bool {
-	a.Status, b.Status = ocv1.ClusterExtensionRevisionStatus{}, ocv1.ClusterExtensionRevisionStatus{}
-
-	// when finalizers are updated during reconcile, we expect finalizers, managedFields, and resourceVersion
-	// to be updated, so we ignore changes in these fields.
-	a.Finalizers, b.Finalizers = []string{}, []string{}
-	a.ManagedFields, b.ManagedFields = nil, nil
-	a.ResourceVersion, b.ResourceVersion = "", ""
-	// Remove kubectl's last-applied-configuration annotation which may be added by the API server
-	if a.Annotations != nil {
-		delete(a.Annotations, "kubectl.kubernetes.io/last-applied-configuration")
-	}
-	if b.Annotations != nil {
-		delete(b.Annotations, "kubectl.kubernetes.io/last-applied-configuration")
+// Compare resources - Annotations/Labels/Spec
+func checkForUnexpectedClusterExtensionRevisionFieldChange(a, b *ocv1.ClusterExtensionRevision) bool {
+	if !equality.Semantic.DeepEqual(a.ObjectMeta.Annotations, b.ObjectMeta.Annotations) {
+		return true
+	}
+	if !equality.Semantic.DeepEqual(a.ObjectMeta.Labels, b.ObjectMeta.Labels) {
+		return true
 	}
 	return !equality.Semantic.DeepEqual(a.Spec, b.Spec)
 }