Merge pull request #1955 from shiftstack/issue1954

🐛 Fix a race when updating status before reconcile completes
kubernetes-sigs · Mar 18, 2024 · 67fa7b7 · 67fa7b7
2 parents c1fc60d + 6f8ac4f
commit 67fa7b7
Show file tree

Hide file tree

Showing 2 changed files with 2 additions and 39 deletions.
diff --git a/controllers/openstackcluster_controller.go b/controllers/openstackcluster_controller.go
@@ -20,7 +20,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"reflect"
 	"time"
 
 	"github.com/gophercloud/gophercloud/openstack/networking/v2/networks"
@@ -43,9 +42,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
-	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
-	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1"
@@ -823,22 +820,7 @@ func (r *OpenStackClusterReconciler) SetupWithManager(ctx context.Context, mgr c
 
 	return ctrl.NewControllerManagedBy(mgr).
 		WithOptions(options).
-		For(&infrav1.OpenStackCluster{},
-			builder.WithPredicates(
-				predicate.Funcs{
-					// Avoid reconciling if the event triggering the reconciliation is related to incremental status updates
-					UpdateFunc: func(e event.UpdateEvent) bool {
-						oldCluster := e.ObjectOld.(*infrav1.OpenStackCluster).DeepCopy()
-						newCluster := e.ObjectNew.(*infrav1.OpenStackCluster).DeepCopy()
-						oldCluster.Status = infrav1.OpenStackClusterStatus{}
-						newCluster.Status = infrav1.OpenStackClusterStatus{}
-						oldCluster.ObjectMeta.ResourceVersion = ""
-						newCluster.ObjectMeta.ResourceVersion = ""
-						return !reflect.DeepEqual(oldCluster, newCluster)
-					},
-				},
-			),
-		).
+		For(&infrav1.OpenStackCluster{}).
 		Watches(
 			&clusterv1.Cluster{},
 			handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, o client.Object) []reconcile.Request {

diff --git a/controllers/openstackmachine_controller.go b/controllers/openstackmachine_controller.go
@@ -21,7 +21,6 @@ import (
 	"encoding/base64"
 	"errors"
 	"fmt"
-	"reflect"
 	"time"
 
 	"github.com/go-logr/logr"
@@ -45,9 +44,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
-	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
-	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1"
@@ -212,23 +209,7 @@ func patchMachine(ctx context.Context, patchHelper *patch.Helper, openStackMachi
 func (r *OpenStackMachineReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
 	return ctrl.NewControllerManagedBy(mgr).
 		WithOptions(options).
-		For(
-			&infrav1.OpenStackMachine{},
-			builder.WithPredicates(
-				predicate.Funcs{
-					// Avoid reconciling if the event triggering the reconciliation is related to incremental status updates
-					UpdateFunc: func(e event.UpdateEvent) bool {
-						oldMachine := e.ObjectOld.(*infrav1.OpenStackMachine).DeepCopy()
-						newMachine := e.ObjectNew.(*infrav1.OpenStackMachine).DeepCopy()
-						oldMachine.Status = infrav1.OpenStackMachineStatus{}
-						newMachine.Status = infrav1.OpenStackMachineStatus{}
-						oldMachine.ObjectMeta.ResourceVersion = ""
-						newMachine.ObjectMeta.ResourceVersion = ""
-						return !reflect.DeepEqual(oldMachine, newMachine)
-					},
-				},
-			),
-		).
+		For(&infrav1.OpenStackMachine{}).
 		Watches(
 			&clusterv1.Machine{},
 			handler.EnqueueRequestsFromMapFunc(util.MachineToInfrastructureMapFunc(infrav1.GroupVersion.WithKind("OpenStackMachine"))),