Skip to content

✨ Added filter function to controller reconciler #8016

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions bootstrap/kubeadm/controllers/alias.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/controller"

kubeadmbootstrapcontrollers "sigs.k8s.io/cluster-api/bootstrap/kubeadm/internal/controllers"
"sigs.k8s.io/cluster-api/util/predicates"
)

// Following types provides access to reconcilers implemented in internal/controllers, thus
Expand All @@ -42,6 +43,9 @@ type KubeadmConfigReconciler struct {
// WatchFilterValue is the label value used to filter events prior to reconciliation.
WatchFilterValue string

// Filter checks if a KubeadmConfig object should be reconciled.
Filter predicates.Filter
Copy link
Member

@fabriziopandini fabriziopandini Jan 30, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/hold

My personal opinion is that we should not add a new knob to filter what we reconcile which is orthogonal to WatchFilterValue and its possible improvements being discussed in #7775.
IMO those efforts should converge and come up with a common definition of what a filter is, that could translate into e.g. a watch filter struct or other solutions.

Also, worth to notice that such type of change in order to actually work should be accepted by provider implementers as well, and this requires a discussion during office hours or a small proposal to rally on async.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am ok with moving discussion to #7775.


// TokenTTL is the amount of time a bootstrap token (and therefore a KubeadmConfig) will be valid.
TokenTTL time.Duration
}
Expand All @@ -51,6 +55,7 @@ func (r *KubeadmConfigReconciler) SetupWithManager(ctx context.Context, mgr ctrl
return (&kubeadmbootstrapcontrollers.KubeadmConfigReconciler{
Client: r.Client,
WatchFilterValue: r.WatchFilterValue,
Filter: r.Filter,
TokenTTL: r.TokenTTL,
}).SetupWithManager(ctx, mgr, options)
}
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@ type KubeadmConfigReconciler struct {
// WatchFilterValue is the label value used to filter events prior to reconciliation.
WatchFilterValue string

Filter predicates.Filter

// TokenTTL is the amount of time a bootstrap token (and therefore a KubeadmConfig) will be valid.
TokenTTL time.Duration

Expand Down Expand Up @@ -160,6 +162,9 @@ func (r *KubeadmConfigReconciler) Reconcile(ctx context.Context, req ctrl.Reques
log.Error(err, "Failed to get config")
return ctrl.Result{}, err
}
if r.Filter != nil && !r.Filter.Filter(config) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Filters should be applied before actually entering in the reconcile loop (e.g. in while setting up controllers)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But then how can we filter out resources that are triggered by another kind of resource?

return ctrl.Result{}, nil
}

// AddOwners adds the owners of KubeadmConfig as k/v pairs to the logger.
// Specifically, it will add KubeadmControlPlane, MachineSet and MachineDeployment.
Expand Down
29 changes: 29 additions & 0 deletions controllers/alias.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import (
machinedeploymenttopologycontroller "sigs.k8s.io/cluster-api/internal/controllers/topology/machinedeployment"
machinesettopologycontroller "sigs.k8s.io/cluster-api/internal/controllers/topology/machineset"
runtimeclient "sigs.k8s.io/cluster-api/internal/runtime/client"
"sigs.k8s.io/cluster-api/util/predicates"
)

// Following types provides access to reconcilers implemented in internal/controllers, thus
Expand All @@ -46,13 +47,17 @@ type ClusterReconciler struct {

// WatchFilterValue is the label value used to filter events prior to reconciliation.
WatchFilterValue string

// Filter checks if a Cluster object should be reconciled.
Filter predicates.Filter
}

func (r *ClusterReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
return (&clustercontroller.Reconciler{
Client: r.Client,
APIReader: r.APIReader,
WatchFilterValue: r.WatchFilterValue,
Filter: r.Filter,
}).SetupWithManager(ctx, mgr, options)
}

Expand All @@ -64,6 +69,8 @@ type MachineReconciler struct {

// WatchFilterValue is the label value used to filter events prior to reconciliation.
WatchFilterValue string
// Filter checks if a Machine object should be reconciled.
Filter predicates.Filter
}

func (r *MachineReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
Expand All @@ -72,6 +79,7 @@ func (r *MachineReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manag
APIReader: r.APIReader,
Tracker: r.Tracker,
WatchFilterValue: r.WatchFilterValue,
Filter: r.Filter,
}).SetupWithManager(ctx, mgr, options)
}

Expand All @@ -83,6 +91,8 @@ type MachineSetReconciler struct {

// WatchFilterValue is the label value used to filter events prior to reconciliation.
WatchFilterValue string
// Filter checks if a MachineSet object should be reconciled.
Filter predicates.Filter
}

func (r *MachineSetReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
Expand All @@ -91,6 +101,7 @@ func (r *MachineSetReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Ma
APIReader: r.APIReader,
Tracker: r.Tracker,
WatchFilterValue: r.WatchFilterValue,
Filter: r.Filter,
}).SetupWithManager(ctx, mgr, options)
}

Expand All @@ -101,13 +112,16 @@ type MachineDeploymentReconciler struct {

// WatchFilterValue is the label value used to filter events prior to reconciliation.
WatchFilterValue string
// Filter checks if a MachineDeployment object should be reconciled.
Filter predicates.Filter
}

func (r *MachineDeploymentReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
return (&machinedeploymentcontroller.Reconciler{
Client: r.Client,
APIReader: r.APIReader,
WatchFilterValue: r.WatchFilterValue,
Filter: r.Filter,
}).SetupWithManager(ctx, mgr, options)
}

Expand All @@ -118,13 +132,16 @@ type MachineHealthCheckReconciler struct {

// WatchFilterValue is the label value used to filter events prior to reconciliation.
WatchFilterValue string
// Filter checks if a MachineHealthCheck object should be reconciled.
Filter predicates.Filter
}

func (r *MachineHealthCheckReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
return (&machinehealthcheckcontroller.Reconciler{
Client: r.Client,
Tracker: r.Tracker,
WatchFilterValue: r.WatchFilterValue,
Filter: r.Filter,
}).SetupWithManager(ctx, mgr, options)
}

Expand All @@ -139,6 +156,8 @@ type ClusterTopologyReconciler struct {

// WatchFilterValue is the label value used to filter events prior to reconciliation.
WatchFilterValue string
// Filter checks if a ClusterTopology object should be reconciled.
Filter predicates.Filter

// UnstructuredCachingClient provides a client that forces caching of unstructured objects,
// thus allowing to optimize reads for templates or provider specific objects in a managed topology.
Expand All @@ -152,6 +171,7 @@ func (r *ClusterTopologyReconciler) SetupWithManager(ctx context.Context, mgr ct
RuntimeClient: r.RuntimeClient,
UnstructuredCachingClient: r.UnstructuredCachingClient,
WatchFilterValue: r.WatchFilterValue,
Filter: r.Filter,
}).SetupWithManager(ctx, mgr, options)
}

Expand All @@ -165,13 +185,16 @@ type MachineDeploymentTopologyReconciler struct {
// race conditions caused by an outdated cache.
APIReader client.Reader
WatchFilterValue string
// Filter checks if a MachineDeploymentTopology object should be reconciled.
Filter predicates.Filter
}

func (r *MachineDeploymentTopologyReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
return (&machinedeploymenttopologycontroller.Reconciler{
Client: r.Client,
APIReader: r.APIReader,
WatchFilterValue: r.WatchFilterValue,
Filter: r.Filter,
}).SetupWithManager(ctx, mgr, options)
}

Expand All @@ -185,13 +208,16 @@ type MachineSetTopologyReconciler struct {
// race conditions caused by an outdated cache.
APIReader client.Reader
WatchFilterValue string
// Filter checks if a MachineSetTopology object should be reconciled.
Filter predicates.Filter
}

func (r *MachineSetTopologyReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
return (&machinesettopologycontroller.Reconciler{
Client: r.Client,
APIReader: r.APIReader,
WatchFilterValue: r.WatchFilterValue,
Filter: r.Filter,
}).SetupWithManager(ctx, mgr, options)
}

Expand All @@ -202,6 +228,8 @@ type ClusterClassReconciler struct {

// WatchFilterValue is the label value used to filter events prior to reconciliation.
WatchFilterValue string
// Filter checks if a ClusterClass object should be reconciled.
Filter predicates.Filter

// UnstructuredCachingClient provides a client that forces caching of unstructured objects,
// thus allowing to optimize reads for templates or provider specific objects.
Expand All @@ -214,5 +242,6 @@ func (r *ClusterClassReconciler) SetupWithManager(ctx context.Context, mgr ctrl.
APIReader: r.APIReader,
UnstructuredCachingClient: r.UnstructuredCachingClient,
WatchFilterValue: r.WatchFilterValue,
Filter: r.Filter,
}).SetupWithManager(ctx, mgr, options)
}
4 changes: 4 additions & 0 deletions controlplane/kubeadm/controllers/alias.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (

"sigs.k8s.io/cluster-api/controllers/remote"
kubeadmcontrolplanecontrollers "sigs.k8s.io/cluster-api/controlplane/kubeadm/internal/controllers"
"sigs.k8s.io/cluster-api/util/predicates"
)

// KubeadmControlPlaneReconciler reconciles a KubeadmControlPlane object.
Expand All @@ -39,6 +40,8 @@ type KubeadmControlPlaneReconciler struct {

// WatchFilterValue is the label value used to filter events prior to reconciliation.
WatchFilterValue string
// Filter checks if a KubeadmControlPlane object should be reconciled.
Filter predicates.Filter
}

// SetupWithManager sets up the reconciler with the Manager.
Expand All @@ -50,5 +53,6 @@ func (r *KubeadmControlPlaneReconciler) SetupWithManager(ctx context.Context, mg
EtcdDialTimeout: r.EtcdDialTimeout,
EtcdCallTimeout: r.EtcdCallTimeout,
WatchFilterValue: r.WatchFilterValue,
Filter: r.Filter,
}).SetupWithManager(ctx, mgr, options)
}
6 changes: 6 additions & 0 deletions controlplane/kubeadm/internal/controllers/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ type KubeadmControlPlaneReconciler struct {
// WatchFilterValue is the label value used to filter events prior to reconciliation.
WatchFilterValue string

Filter predicates.Filter

managementCluster internal.ManagementCluster
managementClusterUncached internal.ManagementCluster
}
Expand Down Expand Up @@ -136,6 +138,10 @@ func (r *KubeadmControlPlaneReconciler) Reconcile(ctx context.Context, req ctrl.
return ctrl.Result{Requeue: true}, nil
}

if r.Filter != nil && !r.Filter.Filter(kcp) {
return ctrl.Result{}, nil
}

// Fetch the Cluster.
cluster, err := util.GetOwnerCluster(ctx, r.Client, kcp.ObjectMeta)
if err != nil {
Expand Down
5 changes: 5 additions & 0 deletions internal/controllers/cluster/cluster_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ type Reconciler struct {

// WatchFilterValue is the label value used to filter events prior to reconciliation.
WatchFilterValue string
Filter predicates.Filter

recorder record.EventRecorder
externalTracker external.ObjectTracker
Expand Down Expand Up @@ -113,6 +114,10 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Re
return ctrl.Result{}, err
}

if r.Filter != nil && !r.Filter.Filter(cluster) {
return ctrl.Result{}, nil
}

// Return early if the object or Cluster is paused.
if annotations.IsPaused(cluster, cluster) {
log.Info("Reconciliation is paused for this object")
Expand Down
5 changes: 4 additions & 1 deletion internal/controllers/clusterclass/clusterclass_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ type Reconciler struct {

// WatchFilterValue is the label value used to filter events prior to reconciliation.
WatchFilterValue string
Filter predicates.Filter

// UnstructuredCachingClient provides a client that forces caching of unstructured objects,
// thus allowing to optimize reads for templates or provider specific objects.
Expand Down Expand Up @@ -82,7 +83,9 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Re
// Error reading the object - requeue the request.
return ctrl.Result{}, err
}

if r.Filter != nil && !r.Filter.Filter(clusterClass) {
return ctrl.Result{}, nil
}
// Return early if the ClusterClass is paused.
if annotations.HasPaused(clusterClass) {
log.Info("Reconciliation is paused for this object")
Expand Down
5 changes: 4 additions & 1 deletion internal/controllers/machine/machine_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ type Reconciler struct {

// WatchFilterValue is the label value used to filter events prior to reconciliation.
WatchFilterValue string
Filter predicates.Filter

controller controller.Controller
recorder record.EventRecorder
Expand Down Expand Up @@ -155,7 +156,9 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Re
// Error reading the object - requeue the request.
return ctrl.Result{}, err
}

if r.Filter != nil && !r.Filter.Filter(m) {
return ctrl.Result{}, nil
}
// AddOwners adds the owners of Machine as k/v pairs to the logger.
// Specifically, it will add KubeadmControlPlane, MachineSet and MachineDeployment.
ctx, log, err := clog.AddOwners(ctx, r.Client, m)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ type Reconciler struct {

// WatchFilterValue is the label value used to filter events prior to reconciliation.
WatchFilterValue string
Filter predicates.Filter

recorder record.EventRecorder
}
Expand Down Expand Up @@ -118,7 +119,9 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Re
// Error reading the object - requeue the request.
return ctrl.Result{}, err
}

if r.Filter != nil && !r.Filter.Filter(deployment) {
return ctrl.Result{}, nil
}
log = log.WithValues("Cluster", klog.KRef(deployment.Namespace, deployment.Spec.ClusterName))
ctx = ctrl.LoggerInto(ctx, log)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ type Reconciler struct {

// WatchFilterValue is the label value used to filter events prior to reconciliation.
WatchFilterValue string
Filter predicates.Filter

controller controller.Controller
recorder record.EventRecorder
Expand Down Expand Up @@ -131,7 +132,9 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Re
log.Error(err, "Failed to fetch MachineHealthCheck")
return ctrl.Result{}, err
}

if r.Filter != nil && !r.Filter.Filter(m) {
return ctrl.Result{}, nil
}
log = log.WithValues("Cluster", klog.KRef(m.Namespace, m.Spec.ClusterName))
ctx = ctrl.LoggerInto(ctx, log)

Expand Down
5 changes: 4 additions & 1 deletion internal/controllers/machineset/machineset_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ type Reconciler struct {

// WatchFilterValue is the label value used to filter events prior to reconciliation.
WatchFilterValue string
Filter predicates.Filter

recorder record.EventRecorder
}
Expand Down Expand Up @@ -130,7 +131,9 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Re
// Error reading the object - requeue the request.
return ctrl.Result{}, err
}

if r.Filter != nil && !r.Filter.Filter(machineSet) {
return ctrl.Result{}, nil
}
// AddOwners adds the owners of MachineSet as k/v pairs to the logger.
// Specifically, it will add MachineDeployment.
ctx, log, err := clog.AddOwners(ctx, r.Client, machineSet)
Expand Down
4 changes: 4 additions & 0 deletions internal/controllers/topology/cluster/cluster_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ type Reconciler struct {

// WatchFilterValue is the label value used to filter events prior to reconciliation.
WatchFilterValue string
Filter predicates.Filter

// UnstructuredCachingClient provides a client that forces caching of unstructured objects,
// thus allowing to optimize reads for templates or provider specific objects in a managed topology.
Expand Down Expand Up @@ -144,6 +145,9 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Re
// Error reading the object - requeue the request.
return ctrl.Result{}, err
}
if r.Filter != nil && !r.Filter.Filter(cluster) {
return ctrl.Result{}, nil
}
cluster.APIVersion = clusterv1.GroupVersion.String()
cluster.Kind = "Cluster"

Expand Down
Loading