fix: invoke cleanup process during shutdown (#5974) (#5981)

* invoke cleanup process during shutdown Signed-off-by: ShutingZhao <shuting@nirmata.com> * fix shutdown cleanup Signed-off-by: ShutingZhao <shuting@nirmata.com> * fix shutdown panic Signed-off-by: ShutingZhao <shuting@nirmata.com> * cleanup webhooks by the label Signed-off-by: ShutingZhao <shuting@nirmata.com> Signed-off-by: ShutingZhao <shuting@nirmata.com> Co-authored-by: Charles-Edouard Brétéché <charled.breteche@gmail.com> Signed-off-by: ShutingZhao <shuting@nirmata.com> Co-authored-by: shuting <shuting@nirmata.com> Co-authored-by: Charles-Edouard Brétéché <charled.breteche@gmail.com>
2024-12-14 11:57:48 +00:00 · 2023-01-12 05:30:04 +00:00 · 2023-01-12 05:30:04 +00:00 · d1c27ad50c
commit d1c27ad50c
parent 38c46bd308
9 changed files with 89 additions and 54 deletions
--- a/charts/kyverno/templates/clusterrole.yaml
+++ b/charts/kyverno/templates/clusterrole.yaml
@ -184,4 +184,5 @@ rules:
  - patch
  - update
  - watch
+  - deletecollection
 {{- end }}
--- a/cmd/internal/setup.go
+++ b/cmd/internal/setup.go
@ -10,8 +10,10 @@ import (
 func shutdown(logger logr.Logger, sdowns ...context.CancelFunc) context.CancelFunc {
 	return func() {
 		for i := range sdowns {
-			logger.Info("shutting down...")
-			defer sdowns[i]()
+			if sdowns[i] != nil {
+				logger.Info("shutting down...")
+				defer sdowns[i]()
+			}
 		}
 	}
 }
--- a/cmd/kyverno/main.go
+++ b/cmd/kyverno/main.go
@ -303,6 +303,7 @@ func createrLeaderControllers(
 		kubeKyvernoInformer.Core().V1().Secrets(),
 		kubeKyvernoInformer.Core().V1().ConfigMaps(),
 		kubeKyvernoInformer.Coordination().V1().Leases(),
+		kubeInformer.Rbac().V1().ClusterRoles(),
 		serverIP,
 		int32(webhookTimeout),
 		autoUpdateWebhooks,
@ -687,11 +688,7 @@ func main() {
 	}
 	// start webhooks server
 	server.Run(signalCtx.Done())
-	// wait for termination signal
-	<-signalCtx.Done()
 	wg.Wait()
-	// wait for server cleanup
-	<-server.Cleanup()
 	// say goodbye...
 	logger.V(2).Info("Kyverno shutdown successful")
 }
--- a/config/install.yaml
+++ b/config/install.yaml
@ -30643,6 +30643,7 @@ rules:
  - patch
  - update
  - watch
+  - deletecollection
 ---
 kind: ClusterRoleBinding
 apiVersion: rbac.authorization.k8s.io/v1
--- a/pkg/controllers/generic/webhook/controller.go
+++ b/pkg/controllers/generic/webhook/controller.go
@ -11,6 +11,7 @@ import (
 	"github.com/kyverno/kyverno/pkg/controllers"
 	"github.com/kyverno/kyverno/pkg/logging"
 	"github.com/kyverno/kyverno/pkg/tls"
+	"github.com/kyverno/kyverno/pkg/utils"
 	controllerutils "github.com/kyverno/kyverno/pkg/utils/controller"
 	admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
 	corev1 "k8s.io/api/core/v1"
@ -24,8 +25,7 @@ import (
 )

 const (
-	maxRetries     = 10
-	managedByLabel = "webhook.kyverno.io/managed-by"
+	maxRetries = 10
 )

 var (
@ -148,7 +148,7 @@ func objectMeta(name string, owner ...metav1.OwnerReference) metav1.ObjectMeta {
 	return metav1.ObjectMeta{
 		Name: name,
 		Labels: map[string]string{
-			managedByLabel: kyvernov1.ValueKyvernoApp,
+			utils.ManagedByLabel: kyvernov1.ValueKyvernoApp,
 		},
 		OwnerReferences: owner,
 	}
--- a/pkg/controllers/webhook/controller.go
+++ b/pkg/controllers/webhook/controller.go
@ -17,9 +17,11 @@ import (
 	"github.com/kyverno/kyverno/pkg/config"
 	"github.com/kyverno/kyverno/pkg/controllers"
 	"github.com/kyverno/kyverno/pkg/tls"
+	"github.com/kyverno/kyverno/pkg/utils"
 	controllerutils "github.com/kyverno/kyverno/pkg/utils/controller"
 	kubeutils "github.com/kyverno/kyverno/pkg/utils/kube"
 	runtimeutils "github.com/kyverno/kyverno/pkg/utils/runtime"
+	"github.com/kyverno/kyverno/pkg/utils/wildcard"
 	admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
 	coordinationv1 "k8s.io/api/coordination/v1"
 	corev1 "k8s.io/api/core/v1"
@ -31,9 +33,11 @@ import (
 	admissionregistrationv1informers "k8s.io/client-go/informers/admissionregistration/v1"
 	coordinationv1informers "k8s.io/client-go/informers/coordination/v1"
 	corev1informers "k8s.io/client-go/informers/core/v1"
+	rbacv1informers "k8s.io/client-go/informers/rbac/v1"
 	admissionregistrationv1listers "k8s.io/client-go/listers/admissionregistration/v1"
 	coordinationv1listers "k8s.io/client-go/listers/coordination/v1"
 	corev1listers "k8s.io/client-go/listers/core/v1"
+	rbacv1listers "k8s.io/client-go/listers/rbac/v1"
 	"k8s.io/client-go/tools/cache"
 	"k8s.io/client-go/util/workqueue"
 )
@ -46,7 +50,6 @@ const (
 	AnnotationLastRequestTime = "kyverno.io/last-request-time"
 	IdleDeadline              = tickerInterval * 10
 	maxRetries                = 10
-	managedByLabel            = "webhook.kyverno.io/managed-by"
 	tickerInterval            = 10 * time.Second
 )

@ -77,13 +80,14 @@ type controller struct {
 	kyvernoClient   versioned.Interface

 	// listers
-	mwcLister       admissionregistrationv1listers.MutatingWebhookConfigurationLister
-	vwcLister       admissionregistrationv1listers.ValidatingWebhookConfigurationLister
-	cpolLister      kyvernov1listers.ClusterPolicyLister
-	polLister       kyvernov1listers.PolicyLister
-	secretLister    corev1listers.SecretLister
-	configMapLister corev1listers.ConfigMapLister
-	leaseLister     coordinationv1listers.LeaseLister
+	mwcLister         admissionregistrationv1listers.MutatingWebhookConfigurationLister
+	vwcLister         admissionregistrationv1listers.ValidatingWebhookConfigurationLister
+	cpolLister        kyvernov1listers.ClusterPolicyLister
+	polLister         kyvernov1listers.PolicyLister
+	secretLister      corev1listers.SecretLister
+	configMapLister   corev1listers.ConfigMapLister
+	leaseLister       coordinationv1listers.LeaseLister
+	clusterroleLister rbacv1listers.ClusterRoleLister

 	// queue
 	queue workqueue.RateLimitingInterface
@ -113,6 +117,7 @@ func NewController(
 	secretInformer corev1informers.SecretInformer,
 	configMapInformer corev1informers.ConfigMapInformer,
 	leaseInformer coordinationv1informers.LeaseInformer,
+	clusterroleInformer rbacv1informers.ClusterRoleInformer,
 	server string,
 	defaultTimeout int32,
 	autoUpdateWebhooks bool,
@ -133,6 +138,7 @@ func NewController(
 		secretLister:       secretInformer.Lister(),
 		configMapLister:    configMapInformer.Lister(),
 		leaseLister:        leaseInformer.Lister(),
+		clusterroleLister:  clusterroleInformer.Lister(),
 		queue:              queue,
 		server:             server,
 		defaultTimeout:     defaultTimeout,
@ -510,7 +516,7 @@ func (c *controller) reconcile(ctx context.Context, logger logr.Logger, key, nam

 func (c *controller) buildVerifyMutatingWebhookConfiguration(caBundle []byte) (*admissionregistrationv1.MutatingWebhookConfiguration, error) {
 	return &admissionregistrationv1.MutatingWebhookConfiguration{
-			ObjectMeta: objectMeta(config.VerifyMutatingWebhookConfigurationName),
+			ObjectMeta: objectMeta(config.VerifyMutatingWebhookConfigurationName, c.buildOwner()...),
 			Webhooks: []admissionregistrationv1.MutatingWebhook{{
 				Name:         config.VerifyMutatingWebhookName,
 				ClientConfig: c.clientConfig(caBundle, config.VerifyMutatingWebhookServicePath),
@ -536,7 +542,7 @@ func (c *controller) buildVerifyMutatingWebhookConfiguration(caBundle []byte) (*

 func (c *controller) buildPolicyMutatingWebhookConfiguration(caBundle []byte) (*admissionregistrationv1.MutatingWebhookConfiguration, error) {
 	return &admissionregistrationv1.MutatingWebhookConfiguration{
-			ObjectMeta: objectMeta(config.PolicyMutatingWebhookConfigurationName),
+			ObjectMeta: objectMeta(config.PolicyMutatingWebhookConfigurationName, c.buildOwner()...),
 			Webhooks: []admissionregistrationv1.MutatingWebhook{{
 				Name:         config.PolicyMutatingWebhookName,
 				ClientConfig: c.clientConfig(caBundle, config.PolicyMutatingWebhookServicePath),
@ -558,7 +564,7 @@ func (c *controller) buildPolicyMutatingWebhookConfiguration(caBundle []byte) (*

 func (c *controller) buildPolicyValidatingWebhookConfiguration(caBundle []byte) (*admissionregistrationv1.ValidatingWebhookConfiguration, error) {
 	return &admissionregistrationv1.ValidatingWebhookConfiguration{
-			ObjectMeta: objectMeta(config.PolicyValidatingWebhookConfigurationName),
+			ObjectMeta: objectMeta(config.PolicyValidatingWebhookConfigurationName, c.buildOwner()...),
 			Webhooks: []admissionregistrationv1.ValidatingWebhook{{
 				Name:         config.PolicyValidatingWebhookName,
 				ClientConfig: c.clientConfig(caBundle, config.PolicyValidatingWebhookServicePath),
@ -579,7 +585,7 @@ func (c *controller) buildPolicyValidatingWebhookConfiguration(caBundle []byte)

 func (c *controller) buildDefaultResourceMutatingWebhookConfiguration(caBundle []byte) (*admissionregistrationv1.MutatingWebhookConfiguration, error) {
 	return &admissionregistrationv1.MutatingWebhookConfiguration{
-			ObjectMeta: objectMeta(config.MutatingWebhookConfigurationName),
+			ObjectMeta: objectMeta(config.MutatingWebhookConfigurationName, c.buildOwner()...),
 			Webhooks: []admissionregistrationv1.MutatingWebhook{{
 				Name:         config.MutatingWebhookName + "-ignore",
 				ClientConfig: c.clientConfig(caBundle, config.MutatingWebhookServicePath+"/ignore"),
@ -606,7 +612,7 @@ func (c *controller) buildDefaultResourceMutatingWebhookConfiguration(caBundle [

 func (c *controller) buildResourceMutatingWebhookConfiguration(caBundle []byte) (*admissionregistrationv1.MutatingWebhookConfiguration, error) {
 	result := admissionregistrationv1.MutatingWebhookConfiguration{
-		ObjectMeta: objectMeta(config.MutatingWebhookConfigurationName),
+		ObjectMeta: objectMeta(config.MutatingWebhookConfigurationName, c.buildOwner()...),
 		Webhooks:   []admissionregistrationv1.MutatingWebhook{},
 	}
 	if c.watchdogCheck() {
@ -685,7 +691,7 @@ func (c *controller) buildDefaultResourceValidatingWebhookConfiguration(caBundle
 		sideEffects = &noneOnDryRun
 	}
 	return &admissionregistrationv1.ValidatingWebhookConfiguration{
-			ObjectMeta: objectMeta(config.ValidatingWebhookConfigurationName),
+			ObjectMeta: objectMeta(config.ValidatingWebhookConfigurationName, c.buildOwner()...),
 			Webhooks: []admissionregistrationv1.ValidatingWebhook{{
 				Name:         config.ValidatingWebhookName + "-ignore",
 				ClientConfig: c.clientConfig(caBundle, config.ValidatingWebhookServicePath+"/ignore"),
@ -713,7 +719,7 @@ func (c *controller) buildDefaultResourceValidatingWebhookConfiguration(caBundle

 func (c *controller) buildResourceValidatingWebhookConfiguration(caBundle []byte) (*admissionregistrationv1.ValidatingWebhookConfiguration, error) {
 	result := admissionregistrationv1.ValidatingWebhookConfiguration{
-		ObjectMeta: objectMeta(config.ValidatingWebhookConfigurationName),
+		ObjectMeta: objectMeta(config.ValidatingWebhookConfigurationName, c.buildOwner()...),
 		Webhooks:   []admissionregistrationv1.ValidatingWebhook{},
 	}
 	if c.watchdogCheck() {
@ -866,3 +872,29 @@ func (c *controller) mergeWebhook(dst *webhook, policy kyvernov1.PolicyInterface
 		}
 	}
 }
+
+func (c *controller) buildOwner() []metav1.OwnerReference {
+	selector := labels.SelectorFromSet(labels.Set(map[string]string{
+		utils.KyvernoComponentLabel: "kyverno",
+	}))
+
+	clusterroles, err := c.clusterroleLister.List(selector)
+	if err != nil {
+		logger.Error(err, "failed to fetch kyverno clusterroles, won't set owners for webhook configurations")
+		return nil
+	}
+
+	for _, clusterrole := range clusterroles {
+		if wildcard.Match("*:webhook", clusterrole.GetName()) {
+			return []metav1.OwnerReference{
+				{
+					APIVersion: "rbac.authorization.k8s.io/v1",
+					Kind:       "ClusterRole",
+					Name:       clusterrole.GetName(),
+					UID:        clusterrole.GetUID(),
+				},
+			}
+		}
+	}
+	return nil
+}
--- a/pkg/controllers/webhook/utils.go
+++ b/pkg/controllers/webhook/utils.go
@ -4,6 +4,7 @@ import (
 	"strings"

 	kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1"
+	"github.com/kyverno/kyverno/pkg/utils"
 	"golang.org/x/exp/slices"
 	admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@ -101,7 +102,7 @@ func objectMeta(name string, owner ...metav1.OwnerReference) metav1.ObjectMeta {
 	return metav1.ObjectMeta{
 		Name: name,
 		Labels: map[string]string{
-			managedByLabel: kyvernov1.ValueKyvernoApp,
+			utils.ManagedByLabel: kyvernov1.ValueKyvernoApp,
 		},
 		OwnerReferences: owner,
 	}
--- a/pkg/utils/annotations.go
+++ b/pkg/utils/annotations.go
@ -11,9 +11,11 @@ import (
 )

 const (
-	PolicyAnnotation = "policies.kyverno.io/last-applied-patches"
-	policyAnnotation = "policies.kyverno.io~1last-applied-patches"
-	oldAnnotation    = "policies.kyverno.io~1patches"
+	PolicyAnnotation      = "policies.kyverno.io/last-applied-patches"
+	policyAnnotation      = "policies.kyverno.io~1last-applied-patches"
+	oldAnnotation         = "policies.kyverno.io~1patches"
+	ManagedByLabel        = "webhook.kyverno.io/managed-by"
+	KyvernoComponentLabel = "app.kubernetes.io/component"
 )

 type RulePatch struct {
--- a/pkg/webhooks/server.go
+++ b/pkg/webhooks/server.go
@ -12,6 +12,7 @@ import (
 	"github.com/kyverno/kyverno/pkg/logging"
 	"github.com/kyverno/kyverno/pkg/metrics"
 	"github.com/kyverno/kyverno/pkg/toggle"
+	"github.com/kyverno/kyverno/pkg/utils"
 	controllerutils "github.com/kyverno/kyverno/pkg/utils/controller"
 	runtimeutils "github.com/kyverno/kyverno/pkg/utils/runtime"
 	"github.com/kyverno/kyverno/pkg/webhooks/handlers"
@ -32,9 +33,7 @@ type Server interface {
 	// Run TLS server in separate thread and returns control immediately
 	Run(<-chan struct{})
 	// Stop TLS server and returns control after the server is shut down
-	Stop(context.Context)
-	// Cleanup returns the chanel used to wait for the server to clean up resources
-	Cleanup() <-chan struct{}
+	Stop()
 }

 type ExceptionHandlers interface {
@ -59,10 +58,9 @@ type ResourceHandlers interface {
 type server struct {
 	server      *http.Server
 	runtime     runtimeutils.Runtime
-	mwcClient   controllerutils.DeleteClient[*admissionregistrationv1.MutatingWebhookConfiguration]
-	vwcClient   controllerutils.DeleteClient[*admissionregistrationv1.ValidatingWebhookConfiguration]
+	mwcClient   controllerutils.DeleteCollectionClient[*admissionregistrationv1.MutatingWebhookConfiguration]
+	vwcClient   controllerutils.DeleteCollectionClient[*admissionregistrationv1.ValidatingWebhookConfiguration]
 	leaseClient controllerutils.DeleteClient[*coordinationv1.Lease]
-	cleanUp     chan struct{}
 }

 type TlsProvider func() ([]byte, []byte, error)
@ -76,8 +74,8 @@ func NewServer(
 	metricsConfig metrics.MetricsConfigManager,
 	debugModeOpts DebugModeOptions,
 	tlsProvider TlsProvider,
-	mwcClient controllerutils.DeleteClient[*admissionregistrationv1.MutatingWebhookConfiguration],
-	vwcClient controllerutils.DeleteClient[*admissionregistrationv1.ValidatingWebhookConfiguration],
+	mwcClient controllerutils.DeleteCollectionClient[*admissionregistrationv1.MutatingWebhookConfiguration],
+	vwcClient controllerutils.DeleteCollectionClient[*admissionregistrationv1.ValidatingWebhookConfiguration],
 	leaseClient controllerutils.DeleteClient[*coordinationv1.Lease],
 	runtime runtimeutils.Runtime,
 ) Server {
@ -181,7 +179,6 @@ func NewServer(
 		vwcClient:   vwcClient,
 		leaseClient: leaseClient,
 		runtime:     runtime,
-		cleanUp:     make(chan struct{}),
 	}
 }

@ -193,9 +190,15 @@ func (s *server) Run(stopCh <-chan struct{}) {
 		}
 	}()
 	logger.Info("starting service")
+
+	<-stopCh
+	s.Stop()
 }

-func (s *server) Stop(ctx context.Context) {
+func (s *server) Stop() {
+	ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
+	defer cancel()
+
 	s.cleanup(ctx)
 	err := s.server.Shutdown(ctx)
 	if err != nil {
@ -207,10 +210,6 @@ func (s *server) Stop(ctx context.Context) {
 	}
 }

-func (s *server) Cleanup() <-chan struct{} {
-	return s.cleanUp
-}
-
 func (s *server) cleanup(ctx context.Context) {
 	if s.runtime.IsGoingDown() {
 		deleteLease := func(name string) {
@ -218,25 +217,25 @@ func (s *server) cleanup(ctx context.Context) {
 				logger.Error(err, "failed to clean up lease", "name", name)
 			}
 		}
-		deleteVwc := func(name string) {
-			if err := s.vwcClient.Delete(ctx, name, metav1.DeleteOptions{}); err != nil && !apierrors.IsNotFound(err) {
-				logger.Error(err, "failed to clean up validating webhook configuration", "name", name)
+		deleteVwc := func() {
+			if err := s.vwcClient.DeleteCollection(ctx, metav1.DeleteOptions{}, metav1.ListOptions{
+				LabelSelector: utils.ManagedByLabel,
+			}); err != nil && !apierrors.IsNotFound(err) {
+				logger.Error(err, "failed to clean up validating webhook configuration", "label", utils.ManagedByLabel)
 			}
 		}
-		deleteMwc := func(name string) {
-			if err := s.mwcClient.Delete(ctx, name, metav1.DeleteOptions{}); err != nil && !apierrors.IsNotFound(err) {
-				logger.Error(err, "failed to clean up mutating webhook configuration", "name", name)
+		deleteMwc := func() {
+			if err := s.mwcClient.DeleteCollection(ctx, metav1.DeleteOptions{}, metav1.ListOptions{
+				LabelSelector: utils.ManagedByLabel,
+			}); err != nil && !apierrors.IsNotFound(err) {
+				logger.Error(err, "failed to clean up mutating webhook configuration", "label", utils.ManagedByLabel)
 			}
 		}
 		deleteLease("kyvernopre-lock")
 		deleteLease("kyverno-health")
-		deleteVwc(config.ValidatingWebhookConfigurationName)
-		deleteVwc(config.PolicyValidatingWebhookConfigurationName)
-		deleteMwc(config.MutatingWebhookConfigurationName)
-		deleteMwc(config.PolicyMutatingWebhookConfigurationName)
-		deleteMwc(config.VerifyMutatingWebhookConfigurationName)
+		deleteVwc()
+		deleteMwc()
 	}
-	close(s.cleanUp)
 }

 func registerWebhookHandlers(