Skip to content

Commit

Permalink
Bump up controller-runtime to v0.15.0 (#222)
Browse files Browse the repository at this point in the history
Major changes include:
- Setting up a Watch for a type has a different signature now; Builder
also has changes in how Watches are set up
- Handler methods for enqueuing reconciles are interfaces now
- Handler functions also take a context as an argument now
- Several other changes that hide types behind interfaces
- Webhook server is now an interface
- Cache and manager options related changes
- Polling functions use contexts now
- Client builder now exposes WithStatusSubresource to initialize
objects with status sub-resources.  Patching status of objects without
this resource will fail.
  • Loading branch information
aruneshpa authored Sep 21, 2023
1 parent c4e0f7f commit 1f638e5
Show file tree
Hide file tree
Showing 23 changed files with 268 additions and 302 deletions.
4 changes: 2 additions & 2 deletions controllers/infracluster/infracluster_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ func addCredSecretWatch(mgr manager.Manager, c controller.Controller, syncPeriod
return err
}

return c.Watch(source.NewKindWithCache(&corev1.Secret{}, nsCache), &handler.EnqueueRequestForObject{},
return c.Watch(source.Kind(nsCache, &corev1.Secret{}), &handler.EnqueueRequestForObject{},
predicate.Funcs{
CreateFunc: func(e event.CreateEvent) bool {
return e.Object.GetName() == VcCredsSecretName
Expand All @@ -110,7 +110,7 @@ func addWcpClusterCMWatch(mgr manager.Manager, c controller.Controller, syncPeri
return err
}

return c.Watch(source.NewKindWithCache(&corev1.ConfigMap{}, nsCache), &handler.EnqueueRequestForObject{},
return c.Watch(source.Kind(nsCache, &corev1.ConfigMap{}), &handler.EnqueueRequestForObject{},
predicate.Funcs{
CreateFunc: func(e event.CreateEvent) bool {
return e.Object.GetName() == WcpClusterConfigMapName
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ func AddToManager(ctx *context.ControllerManagerContext, mgr manager.Manager) er
}
// Add Watches to Namespaces so we can create TKG bindings when new namespaces are created.
err = c.Watch(
&source.Kind{Type: &corev1.Namespace{}},
source.Kind(mgr.GetCache(), &corev1.Namespace{}),
handler.EnqueueRequestsFromMapFunc(nsToProviderCMMapperFn(ctx)),
nsPrct)
if err != nil {
Expand All @@ -94,8 +94,8 @@ func AddToManager(ctx *context.ControllerManagerContext, mgr manager.Manager) er

// nsToProviderCMMapperFn returns a mapper function that can be used to queue reconcile request
// for the provider ConfigMap in response to an event on the Namespace.
func nsToProviderCMMapperFn(ctx *context.ControllerManagerContext) func(o client.Object) []reconcile.Request {
return func(o client.Object) []reconcile.Request {
func nsToProviderCMMapperFn(ctx *context.ControllerManagerContext) func(_ goctx.Context, o client.Object) []reconcile.Request {
return func(_ goctx.Context, o client.Object) []reconcile.Request {
logger := ctx.Logger.WithValues("namespaceName", o.GetName())

logger.V(4).Info("Reconciling provider ConfigMap due to a namespace creation")
Expand All @@ -114,7 +114,7 @@ func addConfigMapWatch(mgr manager.Manager, c controller.Controller, syncPeriod
return err
}

return c.Watch(source.NewKindWithCache(&corev1.ConfigMap{}, nsCache), &handler.EnqueueRequestForObject{},
return c.Watch(source.Kind(nsCache, &corev1.ConfigMap{}), &handler.EnqueueRequestForObject{},
predicate.Funcs{
CreateFunc: func(e event.CreateEvent) bool {
return e.Object.GetName() == config.ProviderConfigMapName
Expand Down
19 changes: 9 additions & 10 deletions controllers/virtualmachine/v1alpha1/virtualmachine_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import (
"sigs.k8s.io/controller-runtime/pkg/handler"
"sigs.k8s.io/controller-runtime/pkg/manager"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
"sigs.k8s.io/controller-runtime/pkg/source"

"github.com/go-logr/logr"
"github.com/pkg/errors"
Expand Down Expand Up @@ -100,15 +99,15 @@ func AddToManager(ctx *context.ControllerManagerContext, mgr manager.Manager) er
WithOptions(controller.Options{MaxConcurrentReconciles: ctx.MaxConcurrentReconciles})

if !lib.IsWCPVMImageRegistryEnabled() {
builder = builder.Watches(&source.Kind{Type: &vmopv1.ContentSourceBinding{}},
builder = builder.Watches(&vmopv1.ContentSourceBinding{},
handler.EnqueueRequestsFromMapFunc(csBindingToVMMapperFn(ctx, r.Client)))
}

if !lib.IsNamespacedVMClassFSSEnabled() {
builder = builder.Watches(&source.Kind{Type: &vmopv1.VirtualMachineClassBinding{}},
builder = builder.Watches(&vmopv1.VirtualMachineClassBinding{},
handler.EnqueueRequestsFromMapFunc(classBindingToVMMapperFn(ctx, r.Client)))
} else {
builder = builder.Watches(&source.Kind{Type: &vmopv1.VirtualMachineClass{}},
builder = builder.Watches(&vmopv1.VirtualMachineClass{},
handler.EnqueueRequestsFromMapFunc(classToVMMapperFn(ctx, r.Client)))
}

Expand All @@ -117,8 +116,8 @@ func AddToManager(ctx *context.ControllerManagerContext, mgr manager.Manager) er

// csBindingToVMMapperFn returns a mapper function that can be used to queue reconcile request
// for the VirtualMachines in response to an event on the ContentSourceBinding resource.
func csBindingToVMMapperFn(ctx *context.ControllerManagerContext, c client.Reader) func(o client.Object) []reconcile.Request {
return func(o client.Object) []reconcile.Request {
func csBindingToVMMapperFn(ctx *context.ControllerManagerContext, c client.Reader) func(_ goctx.Context, o client.Object) []reconcile.Request {
return func(_ goctx.Context, o client.Object) []reconcile.Request {
binding := o.(*vmopv1.ContentSourceBinding)
logger := ctx.Logger.WithValues("name", binding.Name, "namespace", binding.Namespace)

Expand Down Expand Up @@ -176,10 +175,10 @@ func csBindingToVMMapperFn(ctx *context.ControllerManagerContext, c client.Reade

// classBindingToVMMapperFn returns a mapper function that can be used to queue reconcile request
// for the VirtualMachines in response to an event on the VirtualMachineClassBinding resource.
func classBindingToVMMapperFn(ctx *context.ControllerManagerContext, c client.Client) func(o client.Object) []reconcile.Request {
func classBindingToVMMapperFn(ctx *context.ControllerManagerContext, c client.Client) func(_ goctx.Context, o client.Object) []reconcile.Request {
// For a given VirtualMachineClassBinding, return reconcile requests
// for those VirtualMachines with corresponding VirtualMachinesClasses referenced
return func(o client.Object) []reconcile.Request {
return func(_ goctx.Context, o client.Object) []reconcile.Request {
classBinding := o.(*vmopv1.VirtualMachineClassBinding)
logger := ctx.Logger.WithValues("name", classBinding.Name, "namespace", classBinding.Namespace)

Expand Down Expand Up @@ -241,10 +240,10 @@ func classBindingToVMMapperFn(ctx *context.ControllerManagerContext, c client.Cl
// classToVMMapperFn returns a mapper function that can be used to queue reconcile request
// for the VirtualMachines in response to an event on the VirtualMachineClass resource when
// WCP_Namespaced_VM_Class FSS is enabled.
func classToVMMapperFn(ctx *context.ControllerManagerContext, c client.Client) func(o client.Object) []reconcile.Request {
func classToVMMapperFn(ctx *context.ControllerManagerContext, c client.Client) func(_ goctx.Context, o client.Object) []reconcile.Request {
// For a given VirtualMachineClass, return reconcile requests
// for those VirtualMachines with corresponding VirtualMachinesClasses referenced
return func(o client.Object) []reconcile.Request {
return func(_ goctx.Context, o client.Object) []reconcile.Request {
class := o.(*vmopv1.VirtualMachineClass)
logger := ctx.Logger.WithValues("name", class.Name, "namespace", class.Namespace)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import (
"sigs.k8s.io/controller-runtime/pkg/handler"
"sigs.k8s.io/controller-runtime/pkg/manager"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
"sigs.k8s.io/controller-runtime/pkg/source"

"github.com/go-logr/logr"
"github.com/pkg/errors"
Expand Down Expand Up @@ -100,7 +99,7 @@ func AddToManager(ctx *context.ControllerManagerContext, mgr manager.Manager) er
}))).
WithOptions(controller.Options{MaxConcurrentReconciles: ctx.MaxConcurrentReconciles})

builder = builder.Watches(&source.Kind{Type: &vmopv1.VirtualMachineClass{}},
builder = builder.Watches(&vmopv1.VirtualMachineClass{},
handler.EnqueueRequestsFromMapFunc(classToVMMapperFn(ctx, r.Client)))

return builder.Complete(r)
Expand All @@ -109,10 +108,10 @@ func AddToManager(ctx *context.ControllerManagerContext, mgr manager.Manager) er
// classToVMMapperFn returns a mapper function that can be used to queue reconcile request
// for the VirtualMachines in response to an event on the VirtualMachineClass resource when
// WCP_Namespaced_VM_Class FSS is enabled.
func classToVMMapperFn(ctx *context.ControllerManagerContext, c client.Client) func(o client.Object) []reconcile.Request {
func classToVMMapperFn(ctx *context.ControllerManagerContext, c client.Client) func(_ goctx.Context, o client.Object) []reconcile.Request {
// For a given VirtualMachineClass, return reconcile requests
// for those VirtualMachines with corresponding VirtualMachinesClasses referenced
return func(o client.Object) []reconcile.Request {
return func(_ goctx.Context, o client.Object) []reconcile.Request {
class := o.(*vmopv1.VirtualMachineClass)
logger := ctx.Logger.WithValues("name", class.Name, "namespace", class.Namespace)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"sigs.k8s.io/controller-runtime/pkg/handler"
"sigs.k8s.io/controller-runtime/pkg/manager"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
"sigs.k8s.io/controller-runtime/pkg/source"

"github.com/go-logr/logr"
"github.com/pkg/errors"
Expand Down Expand Up @@ -84,7 +83,7 @@ func AddToManager(ctx *context.ControllerManagerContext, mgr manager.Manager) er
return ctrl.NewControllerManagedBy(mgr).
For(controlledType).
WithOptions(controller.Options{MaxConcurrentReconciles: ctx.MaxConcurrentReconciles}).
Watches(&source.Kind{Type: &vmopv1.VirtualMachineImage{}},
Watches(&vmopv1.VirtualMachineImage{},
handler.EnqueueRequestsFromMapFunc(vmiToVMPubMapperFn(ctx, r.Client))).
Complete(r)
}
Expand All @@ -93,10 +92,10 @@ func AddToManager(ctx *context.ControllerManagerContext, mgr manager.Manager) er
// for the VirtualMachinePublishRequests in response to an event on the VirtualMachineImage resource.
// Note: Only when WCP_VM_Image_Registry FSS is enabled, this controller will be added to the controller manager.
// In this case, the VirtualMachineImage is a namespace scoped resource.
func vmiToVMPubMapperFn(ctx *context.ControllerManagerContext, c client.Client) func(o client.Object) []reconcile.Request {
func vmiToVMPubMapperFn(ctx *context.ControllerManagerContext, c client.Client) func(_ goctx.Context, o client.Object) []reconcile.Request {
// For a given VirtualMachineImage, return reconcile requests
// for those VirtualMachinePublishRequests with corresponding VirtualMachinesImage as the target item.
return func(o client.Object) []reconcile.Request {
return func(_ goctx.Context, o client.Object) []reconcile.Request {
vmi := o.(*vmopv1.VirtualMachineImage)
logger := ctx.Logger.WithValues("name", vmi.Name, "namespace", vmi.Namespace)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,12 @@ func unitTestsReconcile() {
Expect(ctx.Client.Status().Update(ctx, obj)).To(Succeed())
})

It("Should not publish VM", func() {
// This is relying on incorrect behavior in controller-runtime's fake client
// which has been fixed by: https://github.com/kubernetes-sigs/controller-runtime/pull/2259
// where a Status would change the entire object and the next Update will
// fail due to conflict. Comment out the test while we figure out a way to
// mimic Update failure.
XIt("Should not publish VM", func() {
_, err := reconciler.ReconcileNormal(vmpubCtx)
Expect(err).To(HaveOccurred())

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"sigs.k8s.io/controller-runtime/pkg/handler"
"sigs.k8s.io/controller-runtime/pkg/manager"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
"sigs.k8s.io/controller-runtime/pkg/source"

"github.com/go-logr/logr"
"github.com/pkg/errors"
Expand Down Expand Up @@ -84,7 +83,7 @@ func AddToManager(ctx *context.ControllerManagerContext, mgr manager.Manager) er
return ctrl.NewControllerManagedBy(mgr).
For(controlledType).
WithOptions(controller.Options{MaxConcurrentReconciles: ctx.MaxConcurrentReconciles}).
Watches(&source.Kind{Type: &vmopv1.VirtualMachineImage{}},
Watches(&vmopv1.VirtualMachineImage{},
handler.EnqueueRequestsFromMapFunc(vmiToVMPubMapperFn(ctx, r.Client))).
Complete(r)
}
Expand All @@ -93,10 +92,10 @@ func AddToManager(ctx *context.ControllerManagerContext, mgr manager.Manager) er
// for the VirtualMachinePublishRequests in response to an event on the VirtualMachineImage resource.
// Note: Only when WCP_VM_Image_Registry FSS is enabled, this controller will be added to the controller manager.
// In this case, the VirtualMachineImage is a namespace scoped resource.
func vmiToVMPubMapperFn(ctx *context.ControllerManagerContext, c client.Client) func(o client.Object) []reconcile.Request {
func vmiToVMPubMapperFn(ctx *context.ControllerManagerContext, c client.Client) func(_ goctx.Context, o client.Object) []reconcile.Request {
// For a given VirtualMachineImage, return reconcile requests
// for those VirtualMachinePublishRequests with corresponding VirtualMachinesImage as the target item.
return func(o client.Object) []reconcile.Request {
return func(_ goctx.Context, o client.Object) []reconcile.Request {
vmi := o.(*vmopv1.VirtualMachineImage)
logger := ctx.Logger.WithValues("name", vmi.Name, "namespace", vmi.Namespace)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,12 @@ func unitTestsReconcile() {
obj.Annotations = map[string]string{"dummy": "dummy"}
Expect(ctx.Client.Status().Update(ctx, obj)).To(Succeed())
})

It("Should not publish VM", func() {
// This is relying on incorrect behavior in controller-runtime's fake client
// which has been fixed by: https://github.com/kubernetes-sigs/controller-runtime/pull/2259
// where a Status would change the entire object and the next Update will
// fail due to conflict. Comment out the test while we figure out a way to
// mimic Update failure.
XIt("Should not publish VM", func() {
_, err := reconciler.ReconcileNormal(vmpubCtx)
Expect(err).To(HaveOccurred())

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (
"sigs.k8s.io/controller-runtime/pkg/handler"
"sigs.k8s.io/controller-runtime/pkg/manager"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
"sigs.k8s.io/controller-runtime/pkg/source"

vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha1"
"github.com/vmware-tanzu/vm-operator/controllers/virtualmachineservice/v1alpha1/providers"
Expand Down Expand Up @@ -75,11 +74,11 @@ func AddToManager(ctx *context.ControllerManagerContext, mgr manager.Manager) er
return ctrl.NewControllerManagedBy(mgr).
For(controlledType).
WithOptions(controller.Options{MaxConcurrentReconciles: ctx.MaxConcurrentReconciles}).
Watches(&source.Kind{Type: &corev1.Service{}},
&handler.EnqueueRequestForOwner{OwnerType: &vmopv1.VirtualMachineService{}}).
Watches(&source.Kind{Type: &corev1.Endpoints{}},
&handler.EnqueueRequestForOwner{OwnerType: &vmopv1.VirtualMachineService{}}).
Watches(&source.Kind{Type: &vmopv1.VirtualMachine{}},
Watches(&corev1.Service{},
handler.EnqueueRequestForOwner(mgr.GetScheme(), mgr.GetRESTMapper(), &vmopv1.VirtualMachineService{})).
Watches(&corev1.Endpoints{},
handler.EnqueueRequestForOwner(mgr.GetScheme(), mgr.GetRESTMapper(), &vmopv1.VirtualMachineService{})).
Watches(&vmopv1.VirtualMachine{},
handler.EnqueueRequestsFromMapFunc(r.virtualMachineToVirtualMachineServiceMapper())).
Complete(r)
}
Expand Down Expand Up @@ -273,8 +272,8 @@ func (r *ReconcileVirtualMachineService) reconcileVMService(ctx *context.Virtual
// VirtualMachineServices that select a given VM via label selectors.
// TODO: The VM's labels could have been changed so this should also return VirtualMachineServices that the
// VM is currently an Endpoint for, because otherwise the VM won't be removed in a timely manner.
func (r *ReconcileVirtualMachineService) virtualMachineToVirtualMachineServiceMapper() func(o client.Object) []reconcile.Request {
return func(o client.Object) []reconcile.Request {
func (r *ReconcileVirtualMachineService) virtualMachineToVirtualMachineServiceMapper() func(_ goctx.Context, o client.Object) []reconcile.Request {
return func(_ goctx.Context, o client.Object) []reconcile.Request {
vm := o.(*vmopv1.VirtualMachine)

reconcileRequests, err := r.getVirtualMachineServicesSelectingVirtualMachine(goctx.Background(), vm)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (
"sigs.k8s.io/controller-runtime/pkg/handler"
"sigs.k8s.io/controller-runtime/pkg/manager"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
"sigs.k8s.io/controller-runtime/pkg/source"

vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha2"

Expand Down Expand Up @@ -76,11 +75,11 @@ func AddToManager(ctx *context.ControllerManagerContext, mgr manager.Manager) er
return ctrl.NewControllerManagedBy(mgr).
For(controlledType).
WithOptions(controller.Options{MaxConcurrentReconciles: ctx.MaxConcurrentReconciles}).
Watches(&source.Kind{Type: &corev1.Service{}},
&handler.EnqueueRequestForOwner{OwnerType: &vmopv1.VirtualMachineService{}}).
Watches(&source.Kind{Type: &corev1.Endpoints{}},
&handler.EnqueueRequestForOwner{OwnerType: &vmopv1.VirtualMachineService{}}).
Watches(&source.Kind{Type: &vmopv1.VirtualMachine{}},
Watches(&corev1.Service{},
handler.EnqueueRequestForOwner(mgr.GetScheme(), mgr.GetRESTMapper(), &vmopv1.VirtualMachineService{})).
Watches(&corev1.Endpoints{},
handler.EnqueueRequestForOwner(mgr.GetScheme(), mgr.GetRESTMapper(), &vmopv1.VirtualMachineService{})).
Watches(&vmopv1.VirtualMachine{},
handler.EnqueueRequestsFromMapFunc(r.virtualMachineToVirtualMachineServiceMapper())).
Complete(r)
}
Expand Down Expand Up @@ -274,8 +273,8 @@ func (r *ReconcileVirtualMachineService) reconcileVMService(ctx *context.Virtual
// VirtualMachineServices that select a given VM via label selectors.
// TODO: The VM's labels could have been changed so this should also return VirtualMachineServices that the
// VM is currently an Endpoint for, because otherwise the VM won't be removed in a timely manner.
func (r *ReconcileVirtualMachineService) virtualMachineToVirtualMachineServiceMapper() func(o client.Object) []reconcile.Request {
return func(o client.Object) []reconcile.Request {
func (r *ReconcileVirtualMachineService) virtualMachineToVirtualMachineServiceMapper() func(_ goctx.Context, o client.Object) []reconcile.Request {
return func(_ goctx.Context, o client.Object) []reconcile.Request {
vm := o.(*vmopv1.VirtualMachine)

reconcileRequests, err := r.getVirtualMachineServicesSelectingVirtualMachine(goctx.Background(), vm)
Expand Down
22 changes: 13 additions & 9 deletions controllers/volume/v1alpha1/volume_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,25 +66,29 @@ func AddToManager(ctx *context.ControllerManagerContext, mgr manager.Manager) er
}

// Watch for changes to VirtualMachine.
err = c.Watch(&source.Kind{Type: &vmopv1.VirtualMachine{}}, &handler.EnqueueRequestForObject{})
err = c.Watch(source.Kind(mgr.GetCache(), &vmopv1.VirtualMachine{}), &handler.EnqueueRequestForObject{})
if err != nil {
return err
}

// Watch for changes for CnsNodeVmAttachment, and enqueue VirtualMachine which is the owner of CnsNodeVmAttachment.
err = c.Watch(&source.Kind{Type: &cnsv1alpha1.CnsNodeVmAttachment{}}, &handler.EnqueueRequestForOwner{
OwnerType: &vmopv1.VirtualMachine{},
IsController: true,
})
err = c.Watch(source.Kind(mgr.GetCache(), &cnsv1alpha1.CnsNodeVmAttachment{}),
handler.EnqueueRequestForOwner(
mgr.GetScheme(),
mgr.GetRESTMapper(),
&vmopv1.VirtualMachine{},
handler.OnlyControllerOwner()))
if err != nil {
return err
}

// Watch for changes for PersistentVolumeClaim, and enqueue VirtualMachine which is the owner of PersistentVolumeClaim.
err = c.Watch(&source.Kind{Type: &corev1.PersistentVolumeClaim{}}, &handler.EnqueueRequestForOwner{
OwnerType: &vmopv1.VirtualMachine{},
IsController: true,
})
err = c.Watch(source.Kind(mgr.GetCache(), &corev1.PersistentVolumeClaim{}),
handler.EnqueueRequestForOwner(
mgr.GetScheme(),
mgr.GetRESTMapper(),
&vmopv1.VirtualMachine{},
handler.OnlyControllerOwner()))
if err != nil {
return err
}
Expand Down
Loading

0 comments on commit 1f638e5

Please sign in to comment.