Skip to content

Commit 6f8ac4f

Browse files
committed
Fix a race when updating status before reconcile completes
We were ignoring any update which only touched the status. This code has been there an extremely long time, and was originally copied from CAPA. Unfortunately this introduces a race for recently added code which sets port status and exits the reconcile early for transactional safety.
1 parent c1fc60d commit 6f8ac4f

File tree

2 files changed

+2
-39
lines changed

2 files changed

+2
-39
lines changed

controllers/openstackcluster_controller.go

Lines changed: 1 addition & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import (
2020
"context"
2121
"errors"
2222
"fmt"
23-
"reflect"
2423
"time"
2524

2625
"github.com/gophercloud/gophercloud/openstack/networking/v2/networks"
@@ -43,9 +42,7 @@ import (
4342
"sigs.k8s.io/controller-runtime/pkg/client"
4443
"sigs.k8s.io/controller-runtime/pkg/controller"
4544
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
46-
"sigs.k8s.io/controller-runtime/pkg/event"
4745
"sigs.k8s.io/controller-runtime/pkg/handler"
48-
"sigs.k8s.io/controller-runtime/pkg/predicate"
4946
"sigs.k8s.io/controller-runtime/pkg/reconcile"
5047

5148
infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1"
@@ -823,22 +820,7 @@ func (r *OpenStackClusterReconciler) SetupWithManager(ctx context.Context, mgr c
823820

824821
return ctrl.NewControllerManagedBy(mgr).
825822
WithOptions(options).
826-
For(&infrav1.OpenStackCluster{},
827-
builder.WithPredicates(
828-
predicate.Funcs{
829-
// Avoid reconciling if the event triggering the reconciliation is related to incremental status updates
830-
UpdateFunc: func(e event.UpdateEvent) bool {
831-
oldCluster := e.ObjectOld.(*infrav1.OpenStackCluster).DeepCopy()
832-
newCluster := e.ObjectNew.(*infrav1.OpenStackCluster).DeepCopy()
833-
oldCluster.Status = infrav1.OpenStackClusterStatus{}
834-
newCluster.Status = infrav1.OpenStackClusterStatus{}
835-
oldCluster.ObjectMeta.ResourceVersion = ""
836-
newCluster.ObjectMeta.ResourceVersion = ""
837-
return !reflect.DeepEqual(oldCluster, newCluster)
838-
},
839-
},
840-
),
841-
).
823+
For(&infrav1.OpenStackCluster{}).
842824
Watches(
843825
&clusterv1.Cluster{},
844826
handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, o client.Object) []reconcile.Request {

controllers/openstackmachine_controller.go

Lines changed: 1 addition & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ import (
2121
"encoding/base64"
2222
"errors"
2323
"fmt"
24-
"reflect"
2524
"time"
2625

2726
"github.com/go-logr/logr"
@@ -45,9 +44,7 @@ import (
4544
"sigs.k8s.io/controller-runtime/pkg/client"
4645
"sigs.k8s.io/controller-runtime/pkg/controller"
4746
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
48-
"sigs.k8s.io/controller-runtime/pkg/event"
4947
"sigs.k8s.io/controller-runtime/pkg/handler"
50-
"sigs.k8s.io/controller-runtime/pkg/predicate"
5148
"sigs.k8s.io/controller-runtime/pkg/reconcile"
5249

5350
infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1"
@@ -212,23 +209,7 @@ func patchMachine(ctx context.Context, patchHelper *patch.Helper, openStackMachi
212209
func (r *OpenStackMachineReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
213210
return ctrl.NewControllerManagedBy(mgr).
214211
WithOptions(options).
215-
For(
216-
&infrav1.OpenStackMachine{},
217-
builder.WithPredicates(
218-
predicate.Funcs{
219-
// Avoid reconciling if the event triggering the reconciliation is related to incremental status updates
220-
UpdateFunc: func(e event.UpdateEvent) bool {
221-
oldMachine := e.ObjectOld.(*infrav1.OpenStackMachine).DeepCopy()
222-
newMachine := e.ObjectNew.(*infrav1.OpenStackMachine).DeepCopy()
223-
oldMachine.Status = infrav1.OpenStackMachineStatus{}
224-
newMachine.Status = infrav1.OpenStackMachineStatus{}
225-
oldMachine.ObjectMeta.ResourceVersion = ""
226-
newMachine.ObjectMeta.ResourceVersion = ""
227-
return !reflect.DeepEqual(oldMachine, newMachine)
228-
},
229-
},
230-
),
231-
).
212+
For(&infrav1.OpenStackMachine{}).
232213
Watches(
233214
&clusterv1.Machine{},
234215
handler.EnqueueRequestsFromMapFunc(util.MachineToInfrastructureMapFunc(infrav1.GroupVersion.WithKind("OpenStackMachine"))),

0 commit comments

Comments
 (0)