Skip to content
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

chore: removing finalizers and controllerswitch #3779

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
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
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
Loading