From 890cf3138ea2c9def0a510198601fe2f775f37db Mon Sep 17 00:00:00 2001 From: akutz Date: Mon, 27 Oct 2025 18:14:08 -0500 Subject: [PATCH] Do not reconcile VMs if only status updated This patch updates the VM controller to filter any UPDATE events unless the VM's generation has changed. This ensures that all events related purely to status updates do not cause additional reconciles. When there is an occasion to need to reconcile a VM due to what would have been a status change, it is possible to use the VM's channel-based source to enqueue an event. --- .../virtualmachine_controller.go | 21 ++++++++++++++++++- .../volume/volume_controller.go | 15 +++++++++++++ .../volume/volume_controller_suite_test.go | 3 ++- .../volume/volume_controller_unit_test.go | 2 ++ .../volumebatch/volumebatch_controller.go | 15 +++++++++++++ .../volumebatch_controller_suite_test.go | 3 ++- .../volumebatch_controller_unit_test.go | 2 ++ 7 files changed, 58 insertions(+), 3 deletions(-) diff --git a/controllers/virtualmachine/virtualmachine/virtualmachine_controller.go b/controllers/virtualmachine/virtualmachine/virtualmachine_controller.go index cb65059b9..095bb19dc 100644 --- a/controllers/virtualmachine/virtualmachine/virtualmachine_controller.go +++ b/controllers/virtualmachine/virtualmachine/virtualmachine_controller.go @@ -20,6 +20,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" "sigs.k8s.io/controller-runtime/pkg/source" @@ -99,6 +100,11 @@ func AddToManager(ctx *pkgctx.ControllerManagerContext, mgr manager.Manager) err GetPriority: kubeutil.GetVirtualMachineReconcilePriority, }) + // Filter UPDATE events that do not have updated metadata or spec. + // Please note, this does not apply to GENERIC events -- the async watcher + // and volume controller will still result in the VM being reconciled. + builder = builder.WithEventFilter(predicate.GenerationChangedPredicate{}) + builder = builder.Watches(&vmopv1.VirtualMachineClass{}, handler.EnqueueRequestsFromMapFunc(classToVMMapperFn(ctx, r.Client))) @@ -114,11 +120,24 @@ func AddToManager(ctx *pkgctx.ControllerManagerContext, mgr manager.Manager) err builder = builder.WatchesRawSource(source.Channel( cource.FromContextWithBuffer(ctx, "VirtualMachine", 100), &kubeutil.EnqueueRequestForObject{ - Logger: ctrl.Log.WithName("asyncvmqueue"), + Logger: ctrl.Log.WithName("vmqueue.async"), GetPriority: kubeutil.GetVirtualMachineReconcilePriority, })) } + builder = builder.WatchesRawSource(source.Channel( + cource.FromContextWithBuffer(ctx, "VirtualMachineVolumes", 100), + &kubeutil.EnqueueRequestForObject{ + Logger: ctrl.Log.WithName("vmqueue.volumes"), + GetPriority: func( + _ context.Context, + _ kubeutil.EventType, + _, _ client.Object, _ int) int { + + return 50 + }, + })) + if pkgcfg.FromContext(ctx).Features.FastDeploy { builder = builder.Watches( &vmopv1.VirtualMachineImageCache{}, diff --git a/controllers/virtualmachine/volume/volume_controller.go b/controllers/virtualmachine/volume/volume_controller.go index 31e00951a..ac9ace73e 100644 --- a/controllers/virtualmachine/volume/volume_controller.go +++ b/controllers/virtualmachine/volume/volume_controller.go @@ -16,6 +16,7 @@ import ( "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" storagev1 "k8s.io/api/storage/v1" + apiequality "k8s.io/apimachinery/pkg/api/equality" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" apierrorsutil "k8s.io/apimachinery/pkg/util/errors" @@ -26,6 +27,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/manager" "sigs.k8s.io/controller-runtime/pkg/reconcile" @@ -44,6 +46,7 @@ import ( "github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/constants" "github.com/vmware-tanzu/vm-operator/pkg/record" pkgutil "github.com/vmware-tanzu/vm-operator/pkg/util" + "github.com/vmware-tanzu/vm-operator/pkg/util/kube/cource" vmopv1util "github.com/vmware-tanzu/vm-operator/pkg/util/vmopv1" ) @@ -216,12 +219,15 @@ type Reconciler struct { // controller can block for a long time, consuming all of the workers. func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (_ ctrl.Result, reterr error) { ctx = pkgcfg.JoinContext(ctx, r.Context) + ctx = cource.JoinContext(ctx, r.Context) vm := &vmopv1.VirtualMachine{} if err := r.Get(ctx, request.NamespacedName, vm); err != nil { return ctrl.Result{}, client.IgnoreNotFound(err) } + origVM := vm.DeepCopy() + volCtx := &pkgctx.VolumeContext{ Context: ctx, Logger: pkglog.FromContextOrDefault(ctx), @@ -251,6 +257,15 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (_ ctr reterr = err } volCtx.Logger.Error(err, "patch failed") + } else if !apiequality.Semantic.DeepEqual( + vm.Status.Volumes, + origVM.Status.Volumes) { + + // Notify the VM channel that the VM's status.volumes was updated. + chanSource := cource.FromContextWithBuffer(ctx, "VirtualMachineVolumes", 100) + chanSource <- event.GenericEvent{ + Object: vm, + } } }() diff --git a/controllers/virtualmachine/volume/volume_controller_suite_test.go b/controllers/virtualmachine/volume/volume_controller_suite_test.go index 4e8912bbe..88da9e945 100644 --- a/controllers/virtualmachine/volume/volume_controller_suite_test.go +++ b/controllers/virtualmachine/volume/volume_controller_suite_test.go @@ -15,13 +15,14 @@ import ( pkgcfg "github.com/vmware-tanzu/vm-operator/pkg/config" pkgctx "github.com/vmware-tanzu/vm-operator/pkg/context" providerfake "github.com/vmware-tanzu/vm-operator/pkg/providers/fake" + "github.com/vmware-tanzu/vm-operator/pkg/util/kube/cource" "github.com/vmware-tanzu/vm-operator/test/builder" ) var intgFakeVMProvider = providerfake.NewVMProvider() var suite = builder.NewTestSuiteForControllerWithContext( - pkgcfg.NewContextWithDefaultConfig(), + cource.WithContext(pkgcfg.NewContextWithDefaultConfig()), volume.AddToManager, func(ctx *pkgctx.ControllerManagerContext, _ ctrlmgr.Manager) error { ctx.VMProvider = intgFakeVMProvider diff --git a/controllers/virtualmachine/volume/volume_controller_unit_test.go b/controllers/virtualmachine/volume/volume_controller_unit_test.go index bf2b0db0e..ac2373cfa 100644 --- a/controllers/virtualmachine/volume/volume_controller_unit_test.go +++ b/controllers/virtualmachine/volume/volume_controller_unit_test.go @@ -35,6 +35,7 @@ import ( providerfake "github.com/vmware-tanzu/vm-operator/pkg/providers/fake" "github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/constants" "github.com/vmware-tanzu/vm-operator/pkg/util" + "github.com/vmware-tanzu/vm-operator/pkg/util/kube/cource" "github.com/vmware-tanzu/vm-operator/pkg/util/ptr" vmopv1util "github.com/vmware-tanzu/vm-operator/pkg/util/vmopv1" "github.com/vmware-tanzu/vm-operator/test/builder" @@ -151,6 +152,7 @@ func unitTestsReconcile() { JustBeforeEach(func() { ctx = suite.NewUnitTestContextForController() + ctx.Context = cource.WithContext(ctx.Context) // Replace the fake client with our own that has the expected index. ctx.Client = fake.NewClientBuilder(). diff --git a/controllers/virtualmachine/volumebatch/volumebatch_controller.go b/controllers/virtualmachine/volumebatch/volumebatch_controller.go index b0f43e40c..7acf9870e 100644 --- a/controllers/virtualmachine/volumebatch/volumebatch_controller.go +++ b/controllers/virtualmachine/volumebatch/volumebatch_controller.go @@ -14,6 +14,7 @@ import ( "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" storagev1 "k8s.io/api/storage/v1" + apiequality "k8s.io/apimachinery/pkg/api/equality" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" storagehelpers "k8s.io/component-helpers/storage/volume" @@ -21,6 +22,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/manager" "sigs.k8s.io/controller-runtime/pkg/reconcile" @@ -28,6 +30,7 @@ import ( cnsv1alpha1 "github.com/vmware-tanzu/vm-operator/external/vsphere-csi-driver/api/v1alpha1" pkgerr "github.com/vmware-tanzu/vm-operator/pkg/errors" + "github.com/vmware-tanzu/vm-operator/pkg/util/kube/cource" "github.com/vmware-tanzu/vm-operator/pkg/util/ptr" vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha5" @@ -144,12 +147,15 @@ type Reconciler struct { // Reconcile reconciles a VirtualMachine object and processes the volumes for batch attachment. func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (_ ctrl.Result, reterr error) { ctx = pkgcfg.JoinContext(ctx, r.Context) + ctx = cource.JoinContext(ctx, r.Context) vm := &vmopv1.VirtualMachine{} if err := r.Get(ctx, request.NamespacedName, vm); err != nil { return ctrl.Result{}, client.IgnoreNotFound(err) } + origVM := vm.DeepCopy() + volCtx := &pkgctx.VolumeContext{ Context: ctx, Logger: pkglog.FromContextOrDefault(ctx), @@ -179,6 +185,15 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (_ ctr reterr = err } volCtx.Logger.Error(err, "patch failed") + } else if !apiequality.Semantic.DeepEqual( + vm.Status.Volumes, + origVM.Status.Volumes) { + + // Notify the VM channel that the VM's status.volumes was updated. + chanSource := cource.FromContextWithBuffer(ctx, "VirtualMachineVolumes", 100) + chanSource <- event.GenericEvent{ + Object: vm, + } } }() diff --git a/controllers/virtualmachine/volumebatch/volumebatch_controller_suite_test.go b/controllers/virtualmachine/volumebatch/volumebatch_controller_suite_test.go index a7fdf33dc..588c520c3 100644 --- a/controllers/virtualmachine/volumebatch/volumebatch_controller_suite_test.go +++ b/controllers/virtualmachine/volumebatch/volumebatch_controller_suite_test.go @@ -15,13 +15,14 @@ import ( pkgcfg "github.com/vmware-tanzu/vm-operator/pkg/config" pkgctx "github.com/vmware-tanzu/vm-operator/pkg/context" providerfake "github.com/vmware-tanzu/vm-operator/pkg/providers/fake" + "github.com/vmware-tanzu/vm-operator/pkg/util/kube/cource" "github.com/vmware-tanzu/vm-operator/test/builder" ) var intgFakeVMProvider = providerfake.NewVMProvider() var suite = builder.NewTestSuiteForControllerWithContext( - pkgcfg.NewContextWithDefaultConfig(), + cource.WithContext(pkgcfg.NewContextWithDefaultConfig()), volumebatch.AddToManager, func(ctx *pkgctx.ControllerManagerContext, _ ctrlmgr.Manager) error { ctx.VMProvider = intgFakeVMProvider diff --git a/controllers/virtualmachine/volumebatch/volumebatch_controller_unit_test.go b/controllers/virtualmachine/volumebatch/volumebatch_controller_unit_test.go index 6587b30ae..64b8c5cab 100644 --- a/controllers/virtualmachine/volumebatch/volumebatch_controller_unit_test.go +++ b/controllers/virtualmachine/volumebatch/volumebatch_controller_unit_test.go @@ -35,6 +35,7 @@ import ( providerfake "github.com/vmware-tanzu/vm-operator/pkg/providers/fake" "github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/constants" "github.com/vmware-tanzu/vm-operator/pkg/util" + "github.com/vmware-tanzu/vm-operator/pkg/util/kube/cource" "github.com/vmware-tanzu/vm-operator/pkg/util/ptr" ) @@ -153,6 +154,7 @@ func unitTestsReconcile() { JustBeforeEach(func() { ctx = suite.NewUnitTestContextForController() + ctx.Context = cource.WithContext(ctx.Context) // Replace the fake client with our own that has the expected index. ctx.Client = fake.NewClientBuilder().