From 4af183878c217b22d1c78b5a761e8fcdfb090ce8 Mon Sep 17 00:00:00 2001 From: Jaydip Gabani Date: Wed, 15 Jan 2025 00:51:01 +0000 Subject: [PATCH] chore: removing finalizers and controllerswitch, fixes: #3084 Signed-off-by: Jaydip Gabani --- config/rbac/role.yaml | 9 ---- main.go | 34 +++++--------- .../gatekeeper-manager-role-clusterrole.yaml | 9 ---- manifest_staging/deploy/gatekeeper.yaml | 9 ---- pkg/controller/config/config_controller.go | 25 ++--------- .../config/config_controller_test.go | 11 ++--- .../configstatus/configstatus_controller.go | 2 - .../constraint/constraint_controller.go | 19 +------- .../constraintstatus_controller.go | 23 +++------- .../constrainttemplate_controller.go | 44 +++++-------------- .../constrainttemplate_controller_test.go | 7 +-- .../constrainttemplatestatus_controller.go | 28 +++--------- ...onstrainttemplatestatus_controller_test.go | 10 ++--- pkg/controller/controller.go | 25 +++++------ .../expansion/expansion_controller.go | 2 - .../expansionstatus_controller.go | 2 - .../externaldata/externaldata_controller.go | 3 -- .../externaldata_controller_test.go | 3 -- .../mutators/instances/mutator_controllers.go | 3 -- .../mutatorstatus/mutatorstatus_controller.go | 29 +++--------- .../pubsub/pubsub_config_controller.go | 3 -- pkg/controller/syncset/syncset_controller.go | 4 -- pkg/readiness/pruner/pruner_test.go | 21 ++++----- pkg/readiness/ready_tracker_test.go | 23 +++++----- pkg/watch/controller_switch.go | 44 ------------------- 25 files changed, 86 insertions(+), 306 deletions(-) delete mode 100644 pkg/watch/controller_switch.go diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index a6d56048491..cbb26ea7615 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -168,15 +168,6 @@ rules: - patch - update - watch -- apiGroups: - - templates.gatekeeper.sh - resources: - - constrainttemplates/finalizers - verbs: - - delete - - get - - patch - - update - apiGroups: - templates.gatekeeper.sh resources: diff --git a/main.go b/main.go index add56d97c49..00964c9acdc 100644 --- a/main.go +++ b/main.go @@ -284,10 +284,6 @@ func innerMain() int { close(setupFinished) } - // ControllerSwitch will be used to disable controllers during our teardown process, - // avoiding conflicts in finalizer cleanup. - sw := watch.NewSwitch() - // Setup tracker and register readiness probe. tracker, err := readiness.SetupTracker(mgr, mutation.Enabled(), *externaldata.ExternalDataEnabled, *expansion.ExpansionEnabled) if err != nil { @@ -316,7 +312,7 @@ func innerMain() int { setupErr := make(chan error) ctx := ctrl.SetupSignalHandler() go func() { - setupErr <- setupControllers(ctx, mgr, sw, tracker, setupFinished) + setupErr <- setupControllers(ctx, mgr, tracker, setupFinished) }() setupLog.Info("starting manager") @@ -348,12 +344,7 @@ blockingLoop: break blockingLoop } } - - // Manager stops controllers asynchronously. - // Instead, we use ControllerSwitch to synchronously prevent them from doing more work. - // This can be removed when finalizer and status teardown is removed. setupLog.Info("disabling controllers...") - sw.Stop() if hadError { return 1 @@ -361,7 +352,7 @@ blockingLoop: return 0 } -func setupControllers(ctx context.Context, mgr ctrl.Manager, sw *watch.ControllerSwitch, tracker *readiness.Tracker, setupFinished chan struct{}) error { +func setupControllers(ctx context.Context, mgr ctrl.Manager, tracker *readiness.Tracker, setupFinished chan struct{}) error { // Block until the setup (certificate generation) finishes. <-setupFinished @@ -508,17 +499,16 @@ func setupControllers(ctx context.Context, mgr ctrl.Manager, sw *watch.Controlle } opts := controller.Dependencies{ - CFClient: client, - WatchManger: wm, - SyncEventsCh: events, - CacheMgr: cm, - ControllerSwitch: sw, - Tracker: tracker, - ProcessExcluder: processExcluder, - MutationSystem: mutationSystem, - ExpansionSystem: expansionSystem, - ProviderCache: providerCache, - PubsubSystem: pubsubSystem, + CFClient: client, + WatchManger: wm, + SyncEventsCh: events, + CacheMgr: cm, + Tracker: tracker, + ProcessExcluder: processExcluder, + MutationSystem: mutationSystem, + ExpansionSystem: expansionSystem, + ProviderCache: providerCache, + PubsubSystem: pubsubSystem, } if err := controller.AddToManager(mgr, &opts); err != nil { diff --git a/manifest_staging/charts/gatekeeper/templates/gatekeeper-manager-role-clusterrole.yaml b/manifest_staging/charts/gatekeeper/templates/gatekeeper-manager-role-clusterrole.yaml index 591d36dc566..8221431f9b4 100644 --- a/manifest_staging/charts/gatekeeper/templates/gatekeeper-manager-role-clusterrole.yaml +++ b/manifest_staging/charts/gatekeeper/templates/gatekeeper-manager-role-clusterrole.yaml @@ -177,15 +177,6 @@ rules: - patch - update - watch -- apiGroups: - - templates.gatekeeper.sh - resources: - - constrainttemplates/finalizers - verbs: - - delete - - get - - patch - - update - apiGroups: - templates.gatekeeper.sh resources: diff --git a/manifest_staging/deploy/gatekeeper.yaml b/manifest_staging/deploy/gatekeeper.yaml index 0e2da7d3598..15dce06ba85 100644 --- a/manifest_staging/deploy/gatekeeper.yaml +++ b/manifest_staging/deploy/gatekeeper.yaml @@ -4967,15 +4967,6 @@ rules: - patch - update - watch -- apiGroups: - - templates.gatekeeper.sh - resources: - - constrainttemplates/finalizers - verbs: - - delete - - get - - patch - - update - apiGroups: - templates.gatekeeper.sh resources: diff --git a/pkg/controller/config/config_controller.go b/pkg/controller/config/config_controller.go index b3f4ec96e43..63cf6928f44 100644 --- a/pkg/controller/config/config_controller.go +++ b/pkg/controller/config/config_controller.go @@ -28,7 +28,6 @@ import ( "github.com/open-policy-agent/gatekeeper/v3/pkg/keys" "github.com/open-policy-agent/gatekeeper/v3/pkg/readiness" "github.com/open-policy-agent/gatekeeper/v3/pkg/util" - "github.com/open-policy-agent/gatekeeper/v3/pkg/watch" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" @@ -53,9 +52,8 @@ var ( ) type Adder struct { - ControllerSwitch *watch.ControllerSwitch - Tracker *readiness.Tracker - CacheManager *cm.CacheManager + Tracker *readiness.Tracker + CacheManager *cm.CacheManager // GetPod returns an instance of the currently running Gatekeeper pod GetPod func(context.Context) (*corev1.Pod, error) } @@ -63,7 +61,7 @@ type Adder struct { // Add creates a new ConfigController and adds it to the Manager with default RBAC. The Manager will set fields on the Controller // and Start it when the Manager is Started. func (a *Adder) Add(mgr manager.Manager) error { - r, err := newReconciler(mgr, a.CacheManager, a.ControllerSwitch, a.Tracker, a.GetPod) + r, err := newReconciler(mgr, a.CacheManager, a.Tracker, a.GetPod) if err != nil { return err } @@ -71,10 +69,6 @@ func (a *Adder) Add(mgr manager.Manager) error { return add(mgr, r) } -func (a *Adder) InjectControllerSwitch(cs *watch.ControllerSwitch) { - a.ControllerSwitch = cs -} - func (a *Adder) InjectTracker(t *readiness.Tracker) { a.Tracker = t } @@ -88,7 +82,7 @@ func (a *Adder) InjectGetPod(getPod func(ctx context.Context) (*corev1.Pod, erro } // newReconciler returns a new reconcile.Reconciler. -func newReconciler(mgr manager.Manager, cm *cm.CacheManager, cs *watch.ControllerSwitch, tracker *readiness.Tracker, getPod func(context.Context) (*corev1.Pod, error)) (*ReconcileConfig, error) { +func newReconciler(mgr manager.Manager, cm *cm.CacheManager, tracker *readiness.Tracker, getPod func(context.Context) (*corev1.Pod, error)) (*ReconcileConfig, error) { if cm == nil { return nil, fmt.Errorf("cacheManager must be non-nil") } @@ -98,7 +92,6 @@ func newReconciler(mgr manager.Manager, cm *cm.CacheManager, cs *watch.Controlle writer: mgr.GetClient(), statusClient: mgr.GetClient(), scheme: mgr.GetScheme(), - cs: cs, cacheManager: cm, tracker: tracker, getPod: getPod, @@ -138,7 +131,6 @@ type ReconcileConfig struct { scheme *runtime.Scheme cacheManager *cm.CacheManager - cs *watch.ControllerSwitch tracker *readiness.Tracker @@ -155,15 +147,6 @@ type ReconcileConfig struct { // and what is in the Config.Spec // Automatically generate RBAC rules to allow the Controller to read all things (for sync). func (r *ReconcileConfig) Reconcile(ctx context.Context, request reconcile.Request) (reconcile.Result, error) { - // Short-circuit if shutting down. - if r.cs != nil { - running := r.cs.Enter() - defer r.cs.Exit() - if !running { - return reconcile.Result{}, nil - } - } - // Fetch the Config instance if request.NamespacedName != keys.Config { log.Info("Ignoring unsupported config name", "namespace", request.NamespacedName.Namespace, "name", request.NamespacedName.Name) diff --git a/pkg/controller/config/config_controller_test.go b/pkg/controller/config/config_controller_test.go index 817e34e10b0..1697d93ee19 100644 --- a/pkg/controller/config/config_controller_test.go +++ b/pkg/controller/config/config_controller_test.go @@ -125,7 +125,6 @@ func TestReconcile(t *testing.T) { dataClient := &fakes.FakeCfClient{} - cs := watch.NewSwitch() tracker, err := readiness.SetupTracker(mgr, false, false, false) if err != nil { t.Fatal(err) @@ -158,7 +157,7 @@ func TestReconcile(t *testing.T) { fakes.WithName("no-pod"), ) - rec, err := newReconciler(mgr, cacheManager, cs, tracker, func(context.Context) (*v1.Pod, error) { return pod, nil }) + rec, err := newReconciler(mgr, cacheManager, tracker, func(context.Context) (*v1.Pod, error) { return pod, nil }) require.NoError(t, err) // Wrap the Controller Reconcile function so it writes each request to a map when it is finished reconciling. @@ -290,8 +289,6 @@ func TestReconcile(t *testing.T) { // fooPod should be namespace excluded, hence not added to the cache require.False(t, dataClient.Contains(map[fakes.CfDataKey]interface{}{{Gvk: fooPod.GroupVersionKind(), Key: "default"}: struct{}{}})) - - cs.Stop() } // tests that expectations for sync only resource gets canceled when it gets deleted. @@ -424,7 +421,6 @@ func setupController(ctx context.Context, mgr manager.Manager, wm *watch.Manager } } - cs := watch.NewSwitch() processExcluder := process.Get() syncMetricsCache := syncutil.NewMetricsCache() reg, err := wm.NewRegistrar( @@ -453,7 +449,7 @@ func setupController(ctx context.Context, mgr manager.Manager, wm *watch.Manager fakes.WithName("no-pod"), ) - rec, err := newReconciler(mgr, cacheManager, cs, tracker, func(context.Context) (*v1.Pod, error) { return pod, nil }) + rec, err := newReconciler(mgr, cacheManager, tracker, func(context.Context) (*v1.Pod, error) { return pod, nil }) if err != nil { return nil, fmt.Errorf("creating reconciler: %w", err) } @@ -600,7 +596,6 @@ func TestConfig_Retries(t *testing.T) { c := testclient.NewRetryClient(mgr.GetClient()) dataClient := &fakes.FakeCfClient{} - cs := watch.NewSwitch() tracker, err := readiness.SetupTracker(mgr, false, false, false) if err != nil { t.Fatal(err) @@ -632,7 +627,7 @@ func TestConfig_Retries(t *testing.T) { fakes.WithName("no-pod"), ) - rec, _ := newReconciler(mgr, cacheManager, cs, tracker, func(context.Context) (*v1.Pod, error) { return pod, nil }) + rec, _ := newReconciler(mgr, cacheManager, tracker, func(context.Context) (*v1.Pod, error) { return pod, nil }) err = add(mgr, rec) if err != nil { t.Fatal(err) diff --git a/pkg/controller/configstatus/configstatus_controller.go b/pkg/controller/configstatus/configstatus_controller.go index b0bae57e02a..c63c0b69fc5 100644 --- a/pkg/controller/configstatus/configstatus_controller.go +++ b/pkg/controller/configstatus/configstatus_controller.go @@ -46,8 +46,6 @@ type Adder struct { WatchManager *watch.Manager } -func (a *Adder) InjectControllerSwitch(_ *watch.ControllerSwitch) {} - func (a *Adder) InjectTracker(_ *readiness.Tracker) {} // Add creates a new config Status Controller and adds it to the Manager. The Manager will set fields on the Controller diff --git a/pkg/controller/constraint/constraint_controller.go b/pkg/controller/constraint/constraint_controller.go index 55fb3c3740f..6e77443da52 100644 --- a/pkg/controller/constraint/constraint_controller.go +++ b/pkg/controller/constraint/constraint_controller.go @@ -87,7 +87,6 @@ type Adder struct { CFClient *constraintclient.Client ConstraintsCache *ConstraintsCache WatchManager *watch.Manager - ControllerSwitch *watch.ControllerSwitch Events <-chan event.GenericEvent Tracker *readiness.Tracker GetPod func(context.Context) (*corev1.Pod, error) @@ -107,10 +106,6 @@ func (a *Adder) InjectWatchManager(w *watch.Manager) { a.WatchManager = w } -func (a *Adder) InjectControllerSwitch(cs *watch.ControllerSwitch) { - a.ControllerSwitch = cs -} - func (a *Adder) InjectTracker(t *readiness.Tracker) { a.Tracker = t } @@ -127,7 +122,7 @@ func (a *Adder) Add(mgr manager.Manager) error { return err } - r := newReconciler(mgr, a.CFClient, a.ControllerSwitch, reporter, a.ConstraintsCache, a.Tracker) + r := newReconciler(mgr, a.CFClient, reporter, a.ConstraintsCache, a.Tracker) if a.GetPod != nil { r.getPod = a.GetPod } @@ -151,7 +146,6 @@ type tags struct { func newReconciler( mgr manager.Manager, cfClient *constraintclient.Client, - cs *watch.ControllerSwitch, reporter StatsReporter, constraintsCache *ConstraintsCache, tracker *readiness.Tracker, @@ -162,7 +156,6 @@ func newReconciler( statusClient: mgr.GetClient(), reader: mgr.GetCache(), - cs: cs, scheme: mgr.GetScheme(), cfClient: cfClient, log: log, @@ -209,7 +202,6 @@ type ReconcileConstraint struct { writer client.Writer statusClient client.StatusClient - cs *watch.ControllerSwitch scheme *runtime.Scheme cfClient *constraintclient.Client log logr.Logger @@ -229,15 +221,6 @@ type ReconcileConstraint struct { // Reconcile reads that state of the cluster for a constraint object and makes changes based on the state read // and what is in the constraint.Spec. func (r *ReconcileConstraint) Reconcile(ctx context.Context, request reconcile.Request) (reconcile.Result, error) { - // Short-circuit if shutting down. - if r.cs != nil { - running := r.cs.Enter() - defer r.cs.Exit() - if !running { - return reconcile.Result{}, nil - } - } - gvk, unpackedRequest, err := util.UnpackRequest(request) if err != nil { // Unrecoverable, do not retry. diff --git a/pkg/controller/constraintstatus/constraintstatus_controller.go b/pkg/controller/constraintstatus/constraintstatus_controller.go index 7f6714c85f2..ca9d915656d 100644 --- a/pkg/controller/constraintstatus/constraintstatus_controller.go +++ b/pkg/controller/constraintstatus/constraintstatus_controller.go @@ -45,11 +45,10 @@ import ( var log = logf.Log.WithName("controller").WithValues(logging.Process, "constraint_status_controller") type Adder struct { - CFClient *constraintclient.Client - WatchManager *watch.Manager - ControllerSwitch *watch.ControllerSwitch - Events <-chan event.GenericEvent - IfWatching func(schema.GroupVersionKind, func() error) (bool, error) + CFClient *constraintclient.Client + WatchManager *watch.Manager + Events <-chan event.GenericEvent + IfWatching func(schema.GroupVersionKind, func() error) (bool, error) } // Add creates a new Constraint Status Controller and adds it to the Manager. The Manager will set fields on the Controller @@ -58,7 +57,7 @@ func (a *Adder) Add(mgr manager.Manager) error { if !operations.IsAssigned(operations.Status) { return nil } - r := newReconciler(mgr, a.ControllerSwitch) + r := newReconciler(mgr) if a.IfWatching != nil { r.ifWatching = a.IfWatching } @@ -68,7 +67,6 @@ func (a *Adder) Add(mgr manager.Manager) error { // newReconciler returns a new reconcile.Reconciler. func newReconciler( mgr manager.Manager, - cs *watch.ControllerSwitch, ) *ReconcileConstraintStatus { return &ReconcileConstraintStatus{ // Separate reader and writer because manager's default client bypasses the cache for unstructured resources. @@ -76,7 +74,6 @@ func newReconciler( statusClient: mgr.GetClient(), reader: mgr.GetCache(), - cs: cs, scheme: mgr.GetScheme(), log: log, ifWatching: func(_ schema.GroupVersionKind, fn func() error) (bool, error) { return true, fn() }, @@ -145,7 +142,6 @@ type ReconcileConstraintStatus struct { writer client.Writer statusClient client.StatusClient - cs *watch.ControllerSwitch scheme *runtime.Scheme log logr.Logger ifWatching func(schema.GroupVersionKind, func() error) (bool, error) @@ -157,15 +153,6 @@ type ReconcileConstraintStatus struct { // Reconcile reads that state of the cluster for a constraint object and makes changes based on the state read // and what is in the constraint.Spec. func (r *ReconcileConstraintStatus) Reconcile(ctx context.Context, request reconcile.Request) (reconcile.Result, error) { - // Short-circuit if shutting down. - if r.cs != nil { - running := r.cs.Enter() - defer r.cs.Exit() - if !running { - return reconcile.Result{}, nil - } - } - gvk, unpackedRequest, err := util.UnpackRequest(request) if err != nil { // Unrecoverable, do not retry. diff --git a/pkg/controller/constrainttemplate/constrainttemplate_controller.go b/pkg/controller/constrainttemplate/constrainttemplate_controller.go index 03ff303361d..2871e135dd9 100644 --- a/pkg/controller/constrainttemplate/constrainttemplate_controller.go +++ b/pkg/controller/constrainttemplate/constrainttemplate_controller.go @@ -77,11 +77,10 @@ var gvkConstraintTemplate = schema.GroupVersionKind{ } type Adder struct { - CFClient *constraintclient.Client - WatchManager *watch.Manager - ControllerSwitch *watch.ControllerSwitch - Tracker *readiness.Tracker - GetPod func(context.Context) (*corev1.Pod, error) + CFClient *constraintclient.Client + WatchManager *watch.Manager + Tracker *readiness.Tracker + GetPod func(context.Context) (*corev1.Pod, error) } // Add creates a new ConstraintTemplate Controller and adds it to the Manager with default RBAC. The Manager will set fields on the Controller @@ -92,7 +91,7 @@ func (a *Adder) Add(mgr manager.Manager) error { } // events will be used to receive events from dynamic watches registered events := make(chan event.GenericEvent, 1024) - r, err := newReconciler(mgr, a.CFClient, a.WatchManager, a.ControllerSwitch, a.Tracker, events, events, a.GetPod) + r, err := newReconciler(mgr, a.CFClient, a.WatchManager, a.Tracker, events, events, a.GetPod) if err != nil { return err } @@ -107,10 +106,6 @@ func (a *Adder) InjectWatchManager(wm *watch.Manager) { a.WatchManager = wm } -func (a *Adder) InjectControllerSwitch(cs *watch.ControllerSwitch) { - a.ControllerSwitch = cs -} - func (a *Adder) InjectTracker(t *readiness.Tracker) { a.Tracker = t } @@ -123,7 +118,7 @@ func (a *Adder) InjectGetPod(getPod func(context.Context) (*corev1.Pod, error)) // cstrEvents is the channel from which constraint controller will receive the events // regEvents is the channel registered by Registrar to put the events in // cstrEvents and regEvents point to same event channel except for testing. -func newReconciler(mgr manager.Manager, cfClient *constraintclient.Client, wm *watch.Manager, cs *watch.ControllerSwitch, tracker *readiness.Tracker, cstrEvents <-chan event.GenericEvent, regEvents chan<- event.GenericEvent, getPod func(context.Context) (*corev1.Pod, error)) (*ReconcileConstraintTemplate, error) { +func newReconciler(mgr manager.Manager, cfClient *constraintclient.Client, wm *watch.Manager, tracker *readiness.Tracker, cstrEvents <-chan event.GenericEvent, regEvents chan<- event.GenericEvent, getPod func(context.Context) (*corev1.Pod, error)) (*ReconcileConstraintTemplate, error) { // constraintsCache contains total number of constraints and shared mutex and vap label constraintsCache := constraint.NewConstraintsCache() @@ -142,7 +137,6 @@ func newReconciler(mgr manager.Manager, cfClient *constraintclient.Client, wm *w CFClient: cfClient, ConstraintsCache: constraintsCache, WatchManager: wm, - ControllerSwitch: cs, Events: cstrEvents, Tracker: tracker, GetPod: getPod, @@ -158,20 +152,18 @@ func newReconciler(mgr manager.Manager, cfClient *constraintclient.Client, wm *w // via the registrar below. statusEvents := make(chan event.GenericEvent, 1024) csAdder := constraintstatus.Adder{ - CFClient: cfClient, - WatchManager: wm, - ControllerSwitch: cs, - Events: statusEvents, - IfWatching: statusW.IfWatching, + CFClient: cfClient, + WatchManager: wm, + Events: statusEvents, + IfWatching: statusW.IfWatching, } if err := csAdder.Add(mgr); err != nil { return nil, err } ctsAdder := constrainttemplatestatus.Adder{ - CfClient: cfClient, - WatchManager: wm, - ControllerSwitch: cs, + CfClient: cfClient, + WatchManager: wm, } if err := ctsAdder.Add(mgr); err != nil { return nil, err @@ -185,7 +177,6 @@ func newReconciler(mgr manager.Manager, cfClient *constraintclient.Client, wm *w cfClient: cfClient, watcher: w, statusWatcher: statusW, - cs: cs, metrics: r, tracker: tracker, getPod: getPod, @@ -245,7 +236,6 @@ type ReconcileConstraintTemplate struct { watcher *watch.Registrar statusWatcher *watch.Registrar cfClient *constraintclient.Client - cs *watch.ControllerSwitch metrics *reporter tracker *readiness.Tracker getPod func(context.Context) (*corev1.Pod, error) @@ -255,8 +245,6 @@ type ReconcileConstraintTemplate struct { // +kubebuilder:rbac:groups=admissionregistration.k8s.io,resources=validatingadmissionpolicies;validatingadmissionpolicybindings,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=apiextensions.k8s.io,resources=customresourcedefinitions,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=templates.gatekeeper.sh,resources=constrainttemplates,verbs=get;list;watch;create;update;patch;delete -// TODO(acpana): remove in 3.16 as per https://github.com/open-policy-agent/gatekeeper/issues/3084 -// +kubebuilder:rbac:groups=templates.gatekeeper.sh,resources=constrainttemplates/finalizers,verbs=get;update;patch;delete // +kubebuilder:rbac:groups=templates.gatekeeper.sh,resources=constrainttemplates/status,verbs=get;update;patch // +kubebuilder:rbac:groups=externaldata.gatekeeper.sh,resources=providers,verbs=get;list;watch;create;update;patch;delete @@ -264,14 +252,6 @@ type ReconcileConstraintTemplate struct { // and what is in the ConstraintTemplate.Spec. func (r *ReconcileConstraintTemplate) Reconcile(ctx context.Context, request reconcile.Request) (reconcile.Result, error) { logger := logger.WithValues("template_name", request.Name) - // Short-circuit if shutting down. - if r.cs != nil { - running := r.cs.Enter() - defer r.cs.Exit() - if !running { - return reconcile.Result{}, nil - } - } defer r.metrics.registry.report(ctx, r.metrics) diff --git a/pkg/controller/constrainttemplate/constrainttemplate_controller_test.go b/pkg/controller/constrainttemplate/constrainttemplate_controller_test.go index c0606e4b689..5ab10810734 100644 --- a/pkg/controller/constrainttemplate/constrainttemplate_controller_test.go +++ b/pkg/controller/constrainttemplate/constrainttemplate_controller_test.go @@ -38,7 +38,6 @@ import ( "github.com/open-policy-agent/gatekeeper/v3/pkg/readiness" "github.com/open-policy-agent/gatekeeper/v3/pkg/target" "github.com/open-policy-agent/gatekeeper/v3/pkg/util" - "github.com/open-policy-agent/gatekeeper/v3/pkg/watch" testclient "github.com/open-policy-agent/gatekeeper/v3/test/clients" "github.com/open-policy-agent/gatekeeper/v3/test/testutils" "golang.org/x/net/context" @@ -246,7 +245,6 @@ func TestReconcile(t *testing.T) { testutils.Setenv(t, "POD_NAME", "no-pod") - cs := watch.NewSwitch() tracker, err := readiness.SetupTracker(mgr, false, false, false) if err != nil { t.Fatal(err) @@ -259,7 +257,7 @@ func TestReconcile(t *testing.T) { // events will be used to receive events from dynamic watches registered events := make(chan event.GenericEvent, 1024) - rec, err := newReconciler(mgr, cfClient, wm, cs, tracker, events, events, func(context.Context) (*corev1.Pod, error) { return pod, nil }) + rec, err := newReconciler(mgr, cfClient, wm, tracker, events, events, func(context.Context) (*corev1.Pod, error) { return pod, nil }) if err != nil { t.Fatal(err) } @@ -1498,7 +1496,6 @@ violation[{"msg": "denied!"}] { testutils.Setenv(t, "POD_NAME", "no-pod") - cs := watch.NewSwitch() pod := fakes.Pod( fakes.WithNamespace("gatekeeper-system"), fakes.WithName("no-pod"), @@ -1506,7 +1503,7 @@ violation[{"msg": "denied!"}] { // events will be used to receive events from dynamic watches registered events := make(chan event.GenericEvent, 1024) - rec, err := newReconciler(mgr, cfClient, wm, cs, tracker, events, nil, func(context.Context) (*corev1.Pod, error) { return pod, nil }) + rec, err := newReconciler(mgr, cfClient, wm, tracker, events, nil, func(context.Context) (*corev1.Pod, error) { return pod, nil }) if err != nil { t.Fatal(err) } diff --git a/pkg/controller/constrainttemplatestatus/constrainttemplatestatus_controller.go b/pkg/controller/constrainttemplatestatus/constrainttemplatestatus_controller.go index 6947a2c3ad8..5b1b75b547c 100644 --- a/pkg/controller/constrainttemplatestatus/constrainttemplatestatus_controller.go +++ b/pkg/controller/constrainttemplatestatus/constrainttemplatestatus_controller.go @@ -45,9 +45,8 @@ import ( var log = logf.Log.WithName("controller").WithValues(logging.Process, "constraint_template_status_controller") type Adder struct { - CfClient *constraintclient.Client - WatchManager *watch.Manager - ControllerSwitch *watch.ControllerSwitch + CfClient *constraintclient.Client + WatchManager *watch.Manager } // Add creates a new Constraint Status Controller and adds it to the Manager. The Manager will set fields on the Controller @@ -56,24 +55,21 @@ func (a *Adder) Add(mgr manager.Manager) error { if !operations.IsAssigned(operations.Status) { return nil } - r := newReconciler(mgr, a.ControllerSwitch) + r := newReconciler(mgr) return add(mgr, r) } // newReconciler returns a new reconcile.Reconciler. func newReconciler( mgr manager.Manager, - cs *watch.ControllerSwitch, ) reconcile.Reconciler { return &ReconcileConstraintStatus{ // Separate reader and writer because manager's default client bypasses the cache for unstructured resources. writer: mgr.GetClient(), statusClient: mgr.GetClient(), reader: mgr.GetCache(), - - cs: cs, - scheme: mgr.GetScheme(), - log: log, + scheme: mgr.GetScheme(), + log: log, } } @@ -132,10 +128,8 @@ type ReconcileConstraintStatus struct { reader client.Reader writer client.Writer statusClient client.StatusClient - - cs *watch.ControllerSwitch - scheme *runtime.Scheme - log logr.Logger + scheme *runtime.Scheme + log logr.Logger } // +kubebuilder:rbac:groups=constraints.gatekeeper.sh,resources=*,verbs=get;list;watch;create;update;patch;delete @@ -144,14 +138,6 @@ type ReconcileConstraintStatus struct { // Reconcile reads that state of the cluster for a constraint object and makes changes based on the state read // and what is in the constraint.Spec. func (r *ReconcileConstraintStatus) Reconcile(ctx context.Context, request reconcile.Request) (reconcile.Result, error) { - // Short-circuit if shutting down. - if r.cs != nil { - running := r.cs.Enter() - defer r.cs.Exit() - if !running { - return reconcile.Result{}, nil - } - } template := &unstructured.Unstructured{} gv := constrainttemplatev1beta1.SchemeGroupVersion template.SetGroupVersionKind(gv.WithKind("ConstraintTemplate")) diff --git a/pkg/controller/constrainttemplatestatus/constrainttemplatestatus_controller_test.go b/pkg/controller/constrainttemplatestatus/constrainttemplatestatus_controller_test.go index 4a43271cbd1..0663cc79dee 100644 --- a/pkg/controller/constrainttemplatestatus/constrainttemplatestatus_controller_test.go +++ b/pkg/controller/constrainttemplatestatus/constrainttemplatestatus_controller_test.go @@ -122,7 +122,6 @@ violation[{"msg": "denied!"}] { testutils.Setenv(t, "POD_NAME", "no-pod") - cs := watch.NewSwitch() tracker, err := readiness.SetupTracker(mgr, false, false, false) if err != nil { t.Fatal(err) @@ -133,11 +132,10 @@ violation[{"msg": "denied!"}] { ) adder := constrainttemplate.Adder{ - CFClient: cfClient, - WatchManager: wm, - ControllerSwitch: cs, - Tracker: tracker, - GetPod: func(context.Context) (*corev1.Pod, error) { return pod, nil }, + CFClient: cfClient, + WatchManager: wm, + Tracker: tracker, + GetPod: func(context.Context) (*corev1.Pod, error) { return pod, nil }, } err = adder.Add(mgr) if err != nil { diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index d129dc03e02..97b6044a523 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -47,7 +47,6 @@ import ( var debugUseFakePod = flag.Bool("debug-use-fake-pod", false, "Use a fake pod name so the Gatekeeper executable can be run outside of Kubernetes") type Injector interface { - InjectControllerSwitch(*watch.ControllerSwitch) InjectTracker(tracker *readiness.Tracker) Add(mgr manager.Manager) error @@ -94,18 +93,17 @@ var AddToManagerFuncs []func(manager.Manager) error // Dependencies are dependencies that can be injected into controllers. type Dependencies struct { - CFClient *constraintclient.Client - WatchManger *watch.Manager - ControllerSwitch *watch.ControllerSwitch - Tracker *readiness.Tracker - GetPod func(context.Context) (*corev1.Pod, error) - ProcessExcluder *process.Excluder - MutationSystem *mutation.System - ExpansionSystem *expansion.System - ProviderCache *externaldata.ProviderCache - PubsubSystem *pubsub.System - SyncEventsCh chan event.GenericEvent - CacheMgr *cm.CacheManager + CFClient *constraintclient.Client + WatchManger *watch.Manager + Tracker *readiness.Tracker + GetPod func(context.Context) (*corev1.Pod, error) + ProcessExcluder *process.Excluder + MutationSystem *mutation.System + ExpansionSystem *expansion.System + ProviderCache *externaldata.ProviderCache + PubsubSystem *pubsub.System + SyncEventsCh chan event.GenericEvent + CacheMgr *cm.CacheManager } type defaultPodGetter struct { @@ -194,7 +192,6 @@ func AddToManager(m manager.Manager, deps *Dependencies) error { } for _, a := range Injectors { - a.InjectControllerSwitch(deps.ControllerSwitch) a.InjectTracker(deps.Tracker) if a2, ok := a.(DataClientInjector); ok { diff --git a/pkg/controller/expansion/expansion_controller.go b/pkg/controller/expansion/expansion_controller.go index 3539508adf0..7e73aaf3b5a 100644 --- a/pkg/controller/expansion/expansion_controller.go +++ b/pkg/controller/expansion/expansion_controller.go @@ -50,8 +50,6 @@ func (a *Adder) Add(mgr manager.Manager) error { return add(mgr, r) } -func (a *Adder) InjectControllerSwitch(_ *watch.ControllerSwitch) {} - func (a *Adder) InjectTracker(tracker *readiness.Tracker) { a.Tracker = tracker } diff --git a/pkg/controller/expansionstatus/expansionstatus_controller.go b/pkg/controller/expansionstatus/expansionstatus_controller.go index e512860f856..baf24537c8f 100644 --- a/pkg/controller/expansionstatus/expansionstatus_controller.go +++ b/pkg/controller/expansionstatus/expansionstatus_controller.go @@ -49,8 +49,6 @@ type Adder struct { WatchManager *watch.Manager } -func (a *Adder) InjectControllerSwitch(_ *watch.ControllerSwitch) {} - func (a *Adder) InjectTracker(_ *readiness.Tracker) {} // Add creates a new Constraint Status Controller and adds it to the Manager. The Manager will set fields on the Controller diff --git a/pkg/controller/externaldata/externaldata_controller.go b/pkg/controller/externaldata/externaldata_controller.go index 2f650b34abb..79822a24405 100644 --- a/pkg/controller/externaldata/externaldata_controller.go +++ b/pkg/controller/externaldata/externaldata_controller.go @@ -10,7 +10,6 @@ import ( "github.com/open-policy-agent/gatekeeper/v3/pkg/externaldata" "github.com/open-policy-agent/gatekeeper/v3/pkg/logging" "github.com/open-policy-agent/gatekeeper/v3/pkg/readiness" - "github.com/open-policy-agent/gatekeeper/v3/pkg/watch" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -45,8 +44,6 @@ func (a *Adder) InjectCFClient(c *constraintclient.Client) { a.CFClient = c } -func (a *Adder) InjectControllerSwitch(_ *watch.ControllerSwitch) {} - func (a *Adder) InjectTracker(t *readiness.Tracker) { a.Tracker = t } diff --git a/pkg/controller/externaldata/externaldata_controller_test.go b/pkg/controller/externaldata/externaldata_controller_test.go index b695db24dc0..caa17fba714 100644 --- a/pkg/controller/externaldata/externaldata_controller_test.go +++ b/pkg/controller/externaldata/externaldata_controller_test.go @@ -17,7 +17,6 @@ import ( "github.com/open-policy-agent/gatekeeper/v3/pkg/readiness" "github.com/open-policy-agent/gatekeeper/v3/pkg/target" "github.com/open-policy-agent/gatekeeper/v3/pkg/util" - "github.com/open-policy-agent/gatekeeper/v3/pkg/watch" testclient "github.com/open-policy-agent/gatekeeper/v3/test/clients" "github.com/open-policy-agent/gatekeeper/v3/test/testutils" "github.com/prometheus/client_golang/prometheus" @@ -93,7 +92,6 @@ func TestReconcile(t *testing.T) { t.Fatalf("unable to set up constraint framework client: %s", err) } - cs := watch.NewSwitch() tracker, err := readiness.SetupTracker(mgr, false, true, false) if err != nil { t.Fatal(err) @@ -182,5 +180,4 @@ func TestReconcile(t *testing.T) { }) testMgrStopped() - cs.Stop() } diff --git a/pkg/controller/mutators/instances/mutator_controllers.go b/pkg/controller/mutators/instances/mutator_controllers.go index da74eb9d1a5..13f78a1c1bc 100644 --- a/pkg/controller/mutators/instances/mutator_controllers.go +++ b/pkg/controller/mutators/instances/mutator_controllers.go @@ -11,7 +11,6 @@ import ( "github.com/open-policy-agent/gatekeeper/v3/pkg/mutation/mutators" "github.com/open-policy-agent/gatekeeper/v3/pkg/mutation/types" "github.com/open-policy-agent/gatekeeper/v3/pkg/readiness" - "github.com/open-policy-agent/gatekeeper/v3/pkg/watch" corev1 "k8s.io/api/core/v1" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/event" @@ -124,8 +123,6 @@ func (a *Adder) Add(mgr manager.Manager) error { return assignMetadata.Add(mgr) } -func (a *Adder) InjectControllerSwitch(_ *watch.ControllerSwitch) {} - func (a *Adder) InjectTracker(t *readiness.Tracker) { a.Tracker = t } diff --git a/pkg/controller/mutatorstatus/mutatorstatus_controller.go b/pkg/controller/mutatorstatus/mutatorstatus_controller.go index bc63c784fe7..e9dc77d4e74 100644 --- a/pkg/controller/mutatorstatus/mutatorstatus_controller.go +++ b/pkg/controller/mutatorstatus/mutatorstatus_controller.go @@ -46,12 +46,9 @@ import ( var log = logf.Log.WithName("controller").WithValues(logging.Process, "mutator_status_controller") type Adder struct { - WatchManager *watch.Manager - ControllerSwitch *watch.ControllerSwitch + WatchManager *watch.Manager } -func (a *Adder) InjectControllerSwitch(_ *watch.ControllerSwitch) {} - func (a *Adder) InjectTracker(_ *readiness.Tracker) {} // Add creates a new Mutator Status Controller and adds it to the Manager. The Manager will set fields on the Controller @@ -60,24 +57,21 @@ func (a *Adder) Add(mgr manager.Manager) error { if !operations.IsAssigned(operations.MutationStatus) { return nil } - r := newReconciler(mgr, a.ControllerSwitch) + r := newReconciler(mgr) return add(mgr, r) } // newReconciler returns a new reconcile.Reconciler. func newReconciler( mgr manager.Manager, - cs *watch.ControllerSwitch, ) reconcile.Reconciler { return &ReconcileMutatorStatus{ // Separate reader and writer because manager's default client bypasses the cache for unstructured resources. writer: mgr.GetClient(), statusClient: mgr.GetClient(), reader: mgr.GetCache(), - - cs: cs, - scheme: mgr.GetScheme(), - log: log, + scheme: mgr.GetScheme(), + log: log, } } @@ -217,10 +211,8 @@ type ReconcileMutatorStatus struct { reader client.Reader writer client.Writer statusClient client.StatusClient - - cs *watch.ControllerSwitch - scheme *runtime.Scheme - log logr.Logger + scheme *runtime.Scheme + log logr.Logger } // +kubebuilder:rbac:groups=mutations.gatekeeper.sh,resources=*,verbs=get;list;watch;create;update;patch;delete @@ -229,15 +221,6 @@ type ReconcileMutatorStatus struct { // Reconcile reads that state of the cluster for a mutator object and makes changes based on the state read // and what is in the mutator.Spec. func (r *ReconcileMutatorStatus) Reconcile(ctx context.Context, request reconcile.Request) (reconcile.Result, error) { - // Short-circuit if shutting down. - if r.cs != nil { - running := r.cs.Enter() - defer r.cs.Exit() - if !running { - return reconcile.Result{}, nil - } - } - gvk, unpackedRequest, err := util.UnpackRequest(request) if err != nil { // Unrecoverable, do not retry. diff --git a/pkg/controller/pubsub/pubsub_config_controller.go b/pkg/controller/pubsub/pubsub_config_controller.go index c34fa1b97b8..d95a2e3687d 100644 --- a/pkg/controller/pubsub/pubsub_config_controller.go +++ b/pkg/controller/pubsub/pubsub_config_controller.go @@ -10,7 +10,6 @@ import ( "github.com/open-policy-agent/gatekeeper/v3/pkg/pubsub" "github.com/open-policy-agent/gatekeeper/v3/pkg/readiness" "github.com/open-policy-agent/gatekeeper/v3/pkg/util" - "github.com/open-policy-agent/gatekeeper/v3/pkg/watch" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" @@ -42,8 +41,6 @@ func (a *Adder) Add(mgr manager.Manager) error { return add(mgr, r) } -func (a *Adder) InjectControllerSwitch(_ *watch.ControllerSwitch) {} - func (a *Adder) InjectTracker(_ *readiness.Tracker) {} func (a *Adder) InjectPubsubSystem(pubsubSystem *pubsub.System) { diff --git a/pkg/controller/syncset/syncset_controller.go b/pkg/controller/syncset/syncset_controller.go index 597824985de..b104b8b1d96 100644 --- a/pkg/controller/syncset/syncset_controller.go +++ b/pkg/controller/syncset/syncset_controller.go @@ -10,7 +10,6 @@ import ( "github.com/open-policy-agent/gatekeeper/v3/pkg/logging" "github.com/open-policy-agent/gatekeeper/v3/pkg/operations" "github.com/open-policy-agent/gatekeeper/v3/pkg/readiness" - "github.com/open-policy-agent/gatekeeper/v3/pkg/watch" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" @@ -56,9 +55,6 @@ func (a *Adder) InjectCacheManager(o *cm.CacheManager) { a.CacheManager = o } -func (a *Adder) InjectControllerSwitch(_ *watch.ControllerSwitch) { -} - func (a *Adder) InjectTracker(t *readiness.Tracker) { a.Tracker = t } diff --git a/pkg/readiness/pruner/pruner_test.go b/pkg/readiness/pruner/pruner_test.go index eaa8daba831..36991bb2f49 100644 --- a/pkg/readiness/pruner/pruner_test.go +++ b/pkg/readiness/pruner/pruner_test.go @@ -15,7 +15,6 @@ import ( "github.com/open-policy-agent/gatekeeper/v3/pkg/mutation" "github.com/open-policy-agent/gatekeeper/v3/pkg/readiness" "github.com/open-policy-agent/gatekeeper/v3/pkg/syncutil" - "github.com/open-policy-agent/gatekeeper/v3/pkg/watch" testclient "github.com/open-policy-agent/gatekeeper/v3/test/clients" "github.com/open-policy-agent/gatekeeper/v3/test/testutils" "github.com/stretchr/testify/require" @@ -81,20 +80,18 @@ func setupTest(_ context.Context, t *testing.T, readyTrackerClient readiness.Lis cm, err := cachemanager.NewCacheManager(config) require.NoError(t, err, "creating cachemanager") - sw := watch.NewSwitch() mutationSystem := mutation.NewSystem(mutation.SystemOpts{}) frameworksexternaldata.NewCache() opts := controller.Dependencies{ - CFClient: testutils.SetupDataClient(t), - WatchManger: wm, - ControllerSwitch: sw, - Tracker: tracker, - ProcessExcluder: process.Get(), - MutationSystem: mutationSystem, - ExpansionSystem: expansion.NewSystem(mutationSystem), - ProviderCache: frameworksexternaldata.NewCache(), - CacheMgr: cm, - SyncEventsCh: events, + CFClient: testutils.SetupDataClient(t), + WatchManger: wm, + Tracker: tracker, + ProcessExcluder: process.Get(), + MutationSystem: mutationSystem, + ExpansionSystem: expansion.NewSystem(mutationSystem), + ProviderCache: frameworksexternaldata.NewCache(), + CacheMgr: cm, + SyncEventsCh: events, } require.NoError(t, controller.AddToManager(mgr, &opts), "registering controllers") diff --git a/pkg/readiness/ready_tracker_test.go b/pkg/readiness/ready_tracker_test.go index 147e5f184f1..8dc893ad0c1 100644 --- a/pkg/readiness/ready_tracker_test.go +++ b/pkg/readiness/ready_tracker_test.go @@ -128,8 +128,6 @@ func setupController( return fmt.Errorf("setting up tracker: %w", err) } - sw := watch.NewSwitch() - pod := fakes.Pod( fakes.WithNamespace("gatekeeper-system"), fakes.WithName("no-pod"), @@ -159,17 +157,16 @@ func setupController( // Setup all Controllers opts := controller.Dependencies{ - CFClient: cfClient, - WatchManger: wm, - ControllerSwitch: sw, - Tracker: tracker, - GetPod: func(_ context.Context) (*corev1.Pod, error) { return pod, nil }, - ProcessExcluder: processExcluder, - MutationSystem: mutationSystem, - ExpansionSystem: expansionSystem, - ProviderCache: providerCache, - CacheMgr: cacheManager, - SyncEventsCh: events, + CFClient: cfClient, + WatchManger: wm, + Tracker: tracker, + GetPod: func(_ context.Context) (*corev1.Pod, error) { return pod, nil }, + ProcessExcluder: processExcluder, + MutationSystem: mutationSystem, + ExpansionSystem: expansionSystem, + ProviderCache: providerCache, + CacheMgr: cacheManager, + SyncEventsCh: events, } if err := controller.AddToManager(mgr, &opts); err != nil { return fmt.Errorf("registering controllers: %w", err) diff --git a/pkg/watch/controller_switch.go b/pkg/watch/controller_switch.go deleted file mode 100644 index e6f27db4d8b..00000000000 --- a/pkg/watch/controller_switch.go +++ /dev/null @@ -1,44 +0,0 @@ -/* - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package watch - -import ( - "sync" -) - -type ControllerSwitch struct { - running bool - runningLock sync.RWMutex -} - -func NewSwitch() *ControllerSwitch { - return &ControllerSwitch{running: true} -} - -func (c *ControllerSwitch) Stop() { - c.runningLock.Lock() - defer c.runningLock.Unlock() - c.running = false -} - -func (c *ControllerSwitch) Enter() bool { - c.runningLock.RLock() - return c.running -} - -func (c *ControllerSwitch) Exit() { - c.runningLock.RUnlock() -}