Skip to content

Commit

Permalink
chore: removing finalizers and controllerswitch (#3779)
Browse files Browse the repository at this point in the history
Signed-off-by: Jaydip Gabani <[email protected]>
  • Loading branch information
JaydipGabani authored Jan 23, 2025
1 parent d0905ae commit 165c1e4
Show file tree
Hide file tree
Showing 25 changed files with 86 additions and 306 deletions.
9 changes: 0 additions & 9 deletions config/rbac/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
34 changes: 12 additions & 22 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -348,20 +344,15 @@ 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
}
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

Expand Down Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
9 changes: 0 additions & 9 deletions manifest_staging/deploy/gatekeeper.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
25 changes: 4 additions & 21 deletions pkg/controller/config/config_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -53,28 +52,23 @@ 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)
}

// 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
}

return add(mgr, r)
}

func (a *Adder) InjectControllerSwitch(cs *watch.ControllerSwitch) {
a.ControllerSwitch = cs
}

func (a *Adder) InjectTracker(t *readiness.Tracker) {
a.Tracker = t
}
Expand All @@ -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")
}
Expand All @@ -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,
Expand Down Expand Up @@ -138,7 +131,6 @@ type ReconcileConfig struct {

scheme *runtime.Scheme
cacheManager *cm.CacheManager
cs *watch.ControllerSwitch

tracker *readiness.Tracker

Expand All @@ -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)
Expand Down
11 changes: 3 additions & 8 deletions pkg/controller/config/config_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 0 additions & 2 deletions pkg/controller/configstatus/configstatus_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
19 changes: 1 addition & 18 deletions pkg/controller/constraint/constraint_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
}
Expand All @@ -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
}
Expand All @@ -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,
Expand All @@ -162,7 +156,6 @@ func newReconciler(
statusClient: mgr.GetClient(),
reader: mgr.GetCache(),

cs: cs,
scheme: mgr.GetScheme(),
cfClient: cfClient,
log: log,
Expand Down Expand Up @@ -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
Expand All @@ -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.
Expand Down
Loading

0 comments on commit 165c1e4

Please sign in to comment.