Merge pull request #1873 from kube-logging/feat/add-finalizer-cleanup

feat: add option to cleanup stuck finalizers

Author: pepov

Makefile

@@ -144,14 +144,16 @@ list: ## List all make targets
 manager: codegen fmt vet ## Build manager binary
 	go build -o bin/manager main.go
 
+HELM_MANIFEST_OVERRIDE='s@manager-role@{{ template "logging-operator.fullname" . }}\n  annotations:\n  {{- if .Values.rbac.retainOnDelete }}\n    "helm.sh/resource-policy": keep\n  {{- end }}@'
+
 .PHONY: manifests
 manifests: ${CONTROLLER_GEN} ## Generate manifests e.g. CRD, RBAC etc.
 	cd pkg/sdk && $(CONTROLLER_GEN) $(CRD_OPTIONS)  webhook paths="./..." output:crd:artifacts:config=../../config/crd/bases output:webhook:artifacts:config=../../config/webhook
 	$(CONTROLLER_GEN) $(CRD_OPTIONS) rbac:roleName=manager-role paths="./controllers/..." output:rbac:artifacts:config=./config/rbac
 	cp config/crd/bases/* charts/logging-operator/crds/
 	for f in config/crd/bases/*.yaml; do sed '/controller-gen.kubebuilder.io\/version/ r hack/crds.annotations.snippet.txt' $${f} > charts/logging-operator/charts/logging-operator-crds/templates/$${f##*/}; done
 	echo "{{- if .Values.rbac.enabled }}" > ./charts/logging-operator/templates/clusterrole.yaml
-	cat config/rbac/role.yaml | sed -e 's@manager-role@{{ template "logging-operator.fullname" . }}@' | sed -e '/creationTimestamp/d' | cat >> ./charts/logging-operator/templates/clusterrole.yaml
+	cat config/rbac/role.yaml | sed -e $(HELM_MANIFEST_OVERRIDE) | cat >> ./charts/logging-operator/templates/clusterrole.yaml
 	echo "{{- end }}" >> ./charts/logging-operator/templates/clusterrole.yaml
 
 .PHONY: run

charts/logging-operator/README.md

@@ -53,6 +53,7 @@ Use `createCustomResource=false` with Helm v3 to avoid trying to create CRDs fro
 | http.port | int | `8080` | HTTP listen port number. |
 | http.service | object | `{"annotations":{},"clusterIP":"None","labels":{},"type":"ClusterIP"}` | Service definition for query http service. |
 | rbac.enabled | bool | `true` | Create rbac service account and roles. |
+| rbac.retainOnDelete | bool | `false` | Keep the operators RBAC resources after the operator is deleted to allow removing pending finalizers. |
 | monitoring.serviceMonitor.enabled | bool | `false` | Create a Prometheus Operator ServiceMonitor object. |
 | monitoring.serviceMonitor.additionalLabels | object | `{}` |  |
 | monitoring.serviceMonitor.metricRelabelings | list | `[]` |  |

charts/logging-operator/templates/clusterrole.yaml

@@ -4,6 +4,10 @@ apiVersion: rbac.authorization.k8s.io/v1
 kind: ClusterRole
 metadata:
   name: {{ template "logging-operator.fullname" . }}
+  annotations:
+  {{- if .Values.rbac.retainOnDelete }}
+    "helm.sh/resource-policy": keep
+  {{- end }}
 rules:
 - apiGroups:
   - ""

charts/logging-operator/templates/clusterrolebinding.yaml

@@ -4,6 +4,10 @@ apiVersion: rbac.authorization.k8s.io/v1
 kind: ClusterRoleBinding
 metadata:
   name: {{ template "logging-operator.fullname" . }}
+  annotations:
+  {{- if .Values.rbac.retainOnDelete }}
+    "helm.sh/resource-policy": keep
+  {{- end }}
   labels:
 {{ include "logging-operator.labels" . | indent 4 }}
 subjects:
@@ -14,5 +18,4 @@ roleRef:
   apiGroup: rbac.authorization.k8s.io
   kind: ClusterRole
   name: {{ template "logging-operator.fullname" . }}
-
-  {{- end }}
+{{- end }}

charts/logging-operator/templates/serviceaccount.yaml

@@ -7,8 +7,11 @@ metadata:
   namespace: {{ include "logging-operator.namespace" . }}
   labels:
 {{ include "logging-operator.labels" . | indent 4 }}
-{{- with .Values.serviceAccount.annotations }}
   annotations:
+  {{- if .Values.rbac.retainOnDelete }}
+    "helm.sh/resource-policy": keep
+  {{- end }}
+  {{- with .Values.serviceAccount.annotations }}
 {{ toYaml . | indent 4 }}
-{{- end }}
+  {{- end }}
 {{- end }}

charts/logging-operator/values.yaml

@@ -60,6 +60,8 @@ http:
 rbac:
   # -- Create rbac service account and roles.
   enabled: true
+  # -- Keep the operators RBAC resources after the operator is deleted to allow removing pending finalizers.
+  retainOnDelete: false
 
   # specify service account manually
   # serviceAccountName: custom

controllers/logging/logging_controller.go

@@ -51,10 +51,14 @@ import (
 	syslogngconfig "github.com/kube-logging/logging-operator/pkg/sdk/logging/model/syslogng/config"
 	loggingmodeltypes "github.com/kube-logging/logging-operator/pkg/sdk/logging/model/types"
 
-	"github.com/kube-logging/logging-operator/pkg/sdk/logging/api/v1beta1"
 	loggingv1beta1 "github.com/kube-logging/logging-operator/pkg/sdk/logging/api/v1beta1"
 )
 
+const (
+	SyslogNGConfigFinalizer = "syslogngconfig.logging.banzaicloud.io/finalizer"
+	FluentdConfigFinalizer  = "fluentdconfig.logging.banzaicloud.io/finalizer"
+)
+
 var fluentbitWarning sync.Once
 var promCrdWarning sync.Once
 
@@ -323,12 +327,10 @@ func (r *LoggingReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
 }
 
 func (r *LoggingReconciler) fluentdConfigFinalizer(ctx context.Context, logging *loggingv1beta1.Logging, externalFluentd *loggingv1beta1.FluentdConfig) (bool, error) {
-	fluentdConfigFinalizer := "fluentdconfig.logging.banzaicloud.io/finalizer"
-
 	if logging.DeletionTimestamp.IsZero() {
-		if externalFluentd != nil && !controllerutil.ContainsFinalizer(logging, fluentdConfigFinalizer) {
+		if externalFluentd != nil && !controllerutil.ContainsFinalizer(logging, FluentdConfigFinalizer) {
 			r.Log.Info("adding fluentdconfig finalizer")
-			controllerutil.AddFinalizer(logging, fluentdConfigFinalizer)
+			controllerutil.AddFinalizer(logging, FluentdConfigFinalizer)
 			if err := r.Update(ctx, logging); err != nil {
 				return true, err
 			}
@@ -339,9 +341,9 @@ func (r *LoggingReconciler) fluentdConfigFinalizer(ctx context.Context, logging
 		return false, errors.New(msg)
 	}
 
-	if controllerutil.ContainsFinalizer(logging, fluentdConfigFinalizer) && externalFluentd == nil {
+	if controllerutil.ContainsFinalizer(logging, FluentdConfigFinalizer) && externalFluentd == nil {
 		r.Log.Info("removing fluentdconfig finalizer")
-		controllerutil.RemoveFinalizer(logging, fluentdConfigFinalizer)
+		controllerutil.RemoveFinalizer(logging, FluentdConfigFinalizer)
 		if err := r.Update(ctx, logging); err != nil {
 			return true, err
 		}
@@ -351,12 +353,10 @@ func (r *LoggingReconciler) fluentdConfigFinalizer(ctx context.Context, logging
 }
 
 func (r *LoggingReconciler) syslogNGConfigFinalizer(ctx context.Context, logging *loggingv1beta1.Logging, externalSyslogNG *loggingv1beta1.SyslogNGConfig) (bool, error) {
-	syslogNGConfigFinalizer := "syslogngconfig.logging.banzaicloud.io/finalizer"
-
 	if logging.DeletionTimestamp.IsZero() {
-		if externalSyslogNG != nil && !controllerutil.ContainsFinalizer(logging, syslogNGConfigFinalizer) {
+		if externalSyslogNG != nil && !controllerutil.ContainsFinalizer(logging, SyslogNGConfigFinalizer) {
 			r.Log.Info("adding syslogngconfig finalizer")
-			controllerutil.AddFinalizer(logging, syslogNGConfigFinalizer)
+			controllerutil.AddFinalizer(logging, SyslogNGConfigFinalizer)
 			if err := r.Update(ctx, logging); err != nil {
 				return true, err
 			}
@@ -367,9 +367,9 @@ func (r *LoggingReconciler) syslogNGConfigFinalizer(ctx context.Context, logging
 		return false, errors.New(msg)
 	}
 
-	if controllerutil.ContainsFinalizer(logging, syslogNGConfigFinalizer) && externalSyslogNG == nil {
+	if controllerutil.ContainsFinalizer(logging, SyslogNGConfigFinalizer) && externalSyslogNG == nil {
 		r.Log.Info("removing syslogngconfig finalizer")
-		controllerutil.RemoveFinalizer(logging, syslogNGConfigFinalizer)
+		controllerutil.RemoveFinalizer(logging, SyslogNGConfigFinalizer)
 		if err := r.Update(ctx, logging); err != nil {
 			return true, err
 		}
@@ -378,7 +378,7 @@ func (r *LoggingReconciler) syslogNGConfigFinalizer(ctx context.Context, logging
 	return false, nil
 }
 
-func (r *LoggingReconciler) dynamicDefaults(ctx context.Context, log logr.Logger, syslogNGSpec *v1beta1.SyslogNGSpec) {
+func (r *LoggingReconciler) dynamicDefaults(ctx context.Context, log logr.Logger, syslogNGSpec *loggingv1beta1.SyslogNGSpec) {
 	nodes := corev1.NodeList{}
 	if err := r.Client.List(ctx, &nodes); err != nil {
 		log.Error(err, "listing nodes")

main.go

@@ -25,6 +25,7 @@ import (
 	"runtime/coverage"
 	"strings"
 	"syscall"
+	"time"
 
 	"emperror.dev/errors"
 	prometheusOperator "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1"
@@ -41,6 +42,7 @@ import (
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/cache"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
 	metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
@@ -80,6 +82,7 @@ func main() {
 	var enableprofile bool
 	var namespace string
 	var loggingRef string
+	var finalizerCleanup bool
 	var klogLevel int
 
 	flag.StringVar(&metricsAddr, "metrics-addr", ":8080", "The address the metric endpoint binds to.")
@@ -91,6 +94,7 @@ func main() {
 	flag.BoolVar(&enableprofile, "pprof", false, "Enable pprof")
 	flag.StringVar(&namespace, "watch-namespace", "", "Namespace to filter the list of watched objects")
 	flag.StringVar(&loggingRef, "watch-logging-name", "", "Logging resource name to optionally filter the list of watched objects based on which logging they belong to by checking the app.kubernetes.io/managed-by label")
+	flag.BoolVar(&finalizerCleanup, "finalizer-cleanup", false, "Remove finalizers from Logging resources during operator shutdown, useful for Helm uninstallation")
 	flag.Parse()
 
 	ctx := context.Background()
@@ -220,7 +224,7 @@ func main() {
 	// +kubebuilder:scaffold:builder
 	setupLog.Info("starting manager")
 
-	if err := mgr.Start(setupSignalHandler()); err != nil {
+	if err := mgr.Start(setupSignalHandler(mgr, finalizerCleanup)); err != nil {
 		setupLog.Error(err, "problem running manager")
 		os.Exit(1)
 	}
@@ -231,7 +235,7 @@ func main() {
 // SIGUSR1 handler for saving test coverage files
 var onlyOneSignalHandler = make(chan struct{})
 
-func setupSignalHandler() context.Context {
+func setupSignalHandler(mgr ctrl.Manager, finalizerCleanup bool) context.Context {
 	close(onlyOneSignalHandler) // panics when called twice
 
 	ctx, cancel := context.WithCancel(context.Background())
@@ -241,7 +245,16 @@ func setupSignalHandler() context.Context {
 	go func() {
 		<-c
 		cancel()
-		<-c
+
+		// Due to the way Helm handles uninstallation,
+		// the operator might be terminated before the finalizers are removed.
+		if finalizerCleanup {
+			cleanupCtx, cleanupCancel := context.WithTimeout(context.Background(), 60*time.Second)
+			defer cleanupCancel()
+
+			cleanupFinalizers(cleanupCtx, mgr.GetClient())
+		}
+
 		os.Exit(1) // second signal. Exit directly.
 	}()
 
@@ -267,6 +280,7 @@ func setupSignalHandler() context.Context {
 
 	return ctx
 }
+
 func detectContainerRuntime(ctx context.Context, c client.Reader) error {
 	var nodeList corev1.NodeList
 	if err := c.List(ctx, &nodeList, client.Limit(1)); err != nil {
@@ -326,3 +340,38 @@ func setupCustomCache(mgrOptions *ctrl.Options, namespace string, loggingRef str
 
 	return mgrOptions, nil
 }
+
+func cleanupFinalizers(ctx context.Context, client client.Client) {
+	log := ctrl.Log.WithName("finalizer-cleanup")
+	log.Info("Removing finalizers during operator shutdown")
+
+	// List all Logging resources
+	loggingList := &loggingv1beta1.LoggingList{}
+	if err := client.List(ctx, loggingList); err != nil {
+		log.Error(err, "Failed to list Logging resources")
+		return
+	}
+
+	finalizers := []string{
+		"fluentdconfig.logging.banzaicloud.io/finalizer",
+		"syslogngconfig.logging.banzaicloud.io/finalizer",
+	}
+	for _, logging := range loggingList.Items {
+		for _, finalizer := range finalizers {
+			if controllerutil.ContainsFinalizer(&logging, finalizer) {
+				log.Info(fmt.Sprintf("Removing finalizer: %s from: %s during operator shutdown",
+					finalizer,
+					logging.Name))
+
+				controllerutil.RemoveFinalizer(&logging, finalizer)
+				if err := client.Update(ctx, &logging); err != nil {
+					log.Error(err, fmt.Sprintf("Failed to remove finalizer: %s from: %s during operator shutdown",
+						finalizer,
+						logging.Name))
+					// continue trying to remove finalizers for other resources
+					continue
+				}
+			}
+		}
+	}
+}