Skip to content

Commit

Permalink
Add scheme as struct member for tracker
Browse files Browse the repository at this point in the history
  • Loading branch information
Karthik-K-N committed Sep 18, 2024
1 parent 5085785 commit 2afb5cd
Show file tree
Hide file tree
Showing 10 changed files with 18 additions and 15 deletions.
5 changes: 3 additions & 2 deletions controllers/external/tracker.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,11 @@ type ObjectTracker struct {

Controller controller.Controller
Cache cache.Cache
Scheme *runtime.Scheme
}

// Watch uses the controller to issue a Watch only if the object hasn't been seen before.
func (o *ObjectTracker) Watch(scheme *runtime.Scheme, log logr.Logger, obj client.Object, handler handler.EventHandler, p ...predicate.Predicate) error {
func (o *ObjectTracker) Watch(log logr.Logger, obj client.Object, handler handler.EventHandler, p ...predicate.Predicate) error {
// Consider this a no-op if the controller isn't present.
if o.Controller == nil {
return nil
Expand All @@ -59,7 +60,7 @@ func (o *ObjectTracker) Watch(scheme *runtime.Scheme, log logr.Logger, obj clien
o.Cache,
obj.DeepCopyObject().(client.Object),
handler,
append(p, predicates.ResourceNotPaused(scheme, log))...,
append(p, predicates.ResourceNotPaused(o.Scheme, log))...,
))
if err != nil {
o.m.Delete(key)
Expand Down
11 changes: 4 additions & 7 deletions controllers/external/tracker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
. "github.com/onsi/gomega"
"github.com/pkg/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"sigs.k8s.io/controller-runtime/pkg/controller"
"sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/source"
Expand Down Expand Up @@ -67,12 +66,11 @@ func TestRetryWatch(t *testing.T) {
ctrl := newWatchCountController(true)
tracker := ObjectTracker{Controller: ctrl}

scheme := runtime.NewScheme()
err := tracker.Watch(scheme, logger, &clusterv1.Cluster{}, nil)
err := tracker.Watch(logger, &clusterv1.Cluster{}, nil)
g.Expect(err).To(HaveOccurred())
g.Expect(ctrl.count).Should(Equal(1))
// Calling Watch on same Object kind that failed earlier should be retryable.
err = tracker.Watch(scheme, logger, &clusterv1.Cluster{}, nil)
err = tracker.Watch(logger, &clusterv1.Cluster{}, nil)
g.Expect(err).To(HaveOccurred())
g.Expect(ctrl.count).Should(Equal(2))
}
Expand All @@ -88,12 +86,11 @@ func TestWatchMultipleTimes(t *testing.T) {
APIVersion: clusterv1.GroupVersion.Version,
},
}
scheme := runtime.NewScheme()
err := tracker.Watch(scheme, logger, obj, nil)
err := tracker.Watch(logger, obj, nil)
g.Expect(err).ToNot(HaveOccurred())
g.Expect(ctrl.count).Should(Equal(1))
// Calling Watch on same Object kind should not register watch again.
err = tracker.Watch(scheme, logger, obj, nil)
err = tracker.Watch(logger, obj, nil)
g.Expect(err).ToNot(HaveOccurred())
g.Expect(ctrl.count).Should(Equal(1))
}
1 change: 1 addition & 0 deletions exp/internal/controllers/machinepool_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ func (r *MachinePoolReconciler) SetupWithManager(ctx context.Context, mgr ctrl.M
r.externalTracker = external.ObjectTracker{
Controller: c,
Cache: mgr.GetCache(),
Scheme: mgr.GetScheme(),
}
r.ssaCache = ssa.NewCache()

Expand Down
4 changes: 2 additions & 2 deletions exp/internal/controllers/machinepool_controller_phases.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ func (r *MachinePoolReconciler) reconcileExternal(ctx context.Context, cluster *
}

// Ensure we add a watch to the external object, if there isn't one already.
if err := r.externalTracker.Watch(r.Client.Scheme(), log, obj, handler.EnqueueRequestForOwner(r.Client.Scheme(), r.Client.RESTMapper(), &expv1.MachinePool{})); err != nil {
if err := r.externalTracker.Watch(log, obj, handler.EnqueueRequestForOwner(r.Client.Scheme(), r.Client.RESTMapper(), &expv1.MachinePool{})); err != nil {
return external.ReconcileOutput{}, err
}

Expand Down Expand Up @@ -379,7 +379,7 @@ func (r *MachinePoolReconciler) reconcileMachines(ctx context.Context, s *scope,
sampleInfraMachine.SetKind(infraMachineKind)

// Add watcher for infraMachine, if there isn't one already.
if err := r.externalTracker.Watch(r.Client.Scheme(), log, sampleInfraMachine, handler.EnqueueRequestsFromMapFunc(r.infraMachineToMachinePoolMapper)); err != nil {
if err := r.externalTracker.Watch(log, sampleInfraMachine, handler.EnqueueRequestsFromMapFunc(r.infraMachineToMachinePoolMapper)); err != nil {
return err
}

Expand Down
1 change: 1 addition & 0 deletions internal/controllers/cluster/cluster_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opt
r.externalTracker = external.ObjectTracker{
Controller: c,
Cache: mgr.GetCache(),
Scheme: mgr.GetScheme(),
}
return nil
}
Expand Down
2 changes: 1 addition & 1 deletion internal/controllers/cluster/cluster_controller_phases.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func (r *Reconciler) reconcileExternal(ctx context.Context, cluster *clusterv1.C
}

// Ensure we add a watcher to the external object.
if err := r.externalTracker.Watch(r.Client.Scheme(), log, obj, handler.EnqueueRequestForOwner(r.Client.Scheme(), r.Client.RESTMapper(), &clusterv1.Cluster{})); err != nil {
if err := r.externalTracker.Watch(log, obj, handler.EnqueueRequestForOwner(r.Client.Scheme(), r.Client.RESTMapper(), &clusterv1.Cluster{})); err != nil {
return external.ReconcileOutput{}, err
}

Expand Down
1 change: 1 addition & 0 deletions internal/controllers/machine/machine_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opt
r.externalTracker = external.ObjectTracker{
Controller: c,
Cache: mgr.GetCache(),
Scheme: mgr.GetScheme(),
}
r.ssaCache = ssa.NewCache()
r.drainCache = drain.NewCache()
Expand Down
2 changes: 1 addition & 1 deletion internal/controllers/machine/machine_controller_phases.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ func (r *Reconciler) ensureExternalOwnershipAndWatch(ctx context.Context, cluste
}

// Ensure we add a watch to the external object, if there isn't one already.
if err := r.externalTracker.Watch(r.Client.Scheme(), log, obj, handler.EnqueueRequestForOwner(r.Client.Scheme(), r.Client.RESTMapper(), &clusterv1.Machine{})); err != nil {
if err := r.externalTracker.Watch(log, obj, handler.EnqueueRequestForOwner(r.Client.Scheme(), r.Client.RESTMapper(), &clusterv1.Machine{})); err != nil {
return external.ReconcileOutput{}, err
}

Expand Down
5 changes: 3 additions & 2 deletions internal/controllers/topology/cluster/cluster_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opt
r.externalTracker = external.ObjectTracker{
Controller: c,
Cache: mgr.GetCache(),
Scheme: mgr.GetScheme(),
}
r.desiredStateGenerator = desiredstate.NewGenerator(r.Client, r.Tracker, r.RuntimeClient)
r.recorder = mgr.GetEventRecorderFor("topology/cluster-controller")
Expand Down Expand Up @@ -295,15 +296,15 @@ func (r *Reconciler) reconcile(ctx context.Context, s *scope.Scope) (ctrl.Result
// setupDynamicWatches create watches for InfrastructureCluster and ControlPlane CRs when they exist.
func (r *Reconciler) setupDynamicWatches(ctx context.Context, s *scope.Scope) error {
if s.Current.InfrastructureCluster != nil {
if err := r.externalTracker.Watch(r.Client.Scheme(), ctrl.LoggerFrom(ctx), s.Current.InfrastructureCluster,
if err := r.externalTracker.Watch(ctrl.LoggerFrom(ctx), s.Current.InfrastructureCluster,
handler.EnqueueRequestForOwner(r.Client.Scheme(), r.Client.RESTMapper(), &clusterv1.Cluster{}),
// Only trigger Cluster reconciliation if the InfrastructureCluster is topology owned.
predicates.ResourceIsTopologyOwned(ctrl.LoggerFrom(ctx))); err != nil {
return errors.Wrap(err, "error watching Infrastructure CR")
}
}
if s.Current.ControlPlane.Object != nil {
if err := r.externalTracker.Watch(r.Client.Scheme(), ctrl.LoggerFrom(ctx), s.Current.ControlPlane.Object,
if err := r.externalTracker.Watch(ctrl.LoggerFrom(ctx), s.Current.ControlPlane.Object,
handler.EnqueueRequestForOwner(r.Client.Scheme(), r.Client.RESTMapper(), &clusterv1.Cluster{}),
// Only trigger Cluster reconciliation if the ControlPlane is topology owned.
predicates.ResourceIsTopologyOwned(ctrl.LoggerFrom(ctx))); err != nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,7 @@ func (r *DockerMachinePoolReconciler) SetupWithManager(ctx context.Context, mgr
r.externalTracker = external.ObjectTracker{
Controller: c,
Cache: mgr.GetCache(),
Scheme: mgr.GetScheme(),
}
r.ssaCache = ssa.NewCache()

Expand Down

0 comments on commit 2afb5cd

Please sign in to comment.