Clean up when deployment is being deleted

controllers/configurationpolicy_controller.go

@@ -18,6 +18,7 @@ import (
 	gocmp "github.com/google/go-cmp/cmp"
 	"github.com/prometheus/client_golang/prometheus"
 	templates "github.com/stolostron/go-template-utils/v3/pkg/templates"
+	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	extensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	extensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
@@ -51,6 +52,7 @@ const (
 	ControllerName                 string = "configuration-policy-controller"
 	CRDName                        string = "configurationpolicies.policy.open-cluster-management.io"
 	complianceStatusConditionLimit        = 10
+	pruneObjectFinalizer           string = "policy.open-cluster-management.io/delete-related-objects"
 )
 
 var log = ctrl.Log.WithName(ControllerName)
@@ -188,6 +190,26 @@ func (r *ConfigurationPolicyReconciler) PeriodicallyExecConfigPolicies(freq uint
 			skipLoop = true
 		}
 
+		needDeploymentFinalizer := false
+
+		for _, plc := range policiesList.Items {
+			if objHasFinalizer(&plc, pruneObjectFinalizer) {
+				needDeploymentFinalizer = true
+
+				break
+			}
+		}
+
+		if err := r.manageDeploymentFinalizer(needDeploymentFinalizer); err != nil {
+			if errors.Is(err, common.ErrNoNamespace) || errors.Is(err, common.ErrRunLocal) {
+				log.Info("Not managing the controller's deployment finalizer because it is running locally")
+			} else {
+				log.Error(err, "Failed to manage the controller's deployment finalizer, skipping loop")
+
+				skipLoop = true
+			}
+		}
+
 		// This is done every loop cycle since the channel needs to be variable in size to account for the number of
 		// policies changing.
 		policyQueue := make(chan *policyv1.ConfigurationPolicy, len(policiesList.Items))
@@ -565,6 +587,52 @@ func (r *ConfigurationPolicyReconciler) cleanUpChildObjects(plc policyv1.Configu
 	return deletionFailures
 }
 
+// cleanupImmediately returns true when the cluster is in a state where configurationpolicies should
+// be removed as soon as possible, ignoring the pruneObjectBehavior of the policies. This is the
+// case when the CRD or the controller's deployment are already being deleted.
+func (r *ConfigurationPolicyReconciler) cleanupImmediately() (bool, error) {
+	deployDeleting, deployErr := r.deploymentIsDeleting()
+	if deployErr == nil && deployDeleting {
+		return true, nil
+	}
+
+	defDeleting, defErr := r.definitionIsDeleting()
+	if defErr == nil && defDeleting {
+		return true, nil
+	}
+
+	if deployErr == nil && defErr == nil {
+		// if either was deleting, we would've already returned.
+		return false, nil
+	}
+
+	// At least one had an unexpected error, so the decision can't be made right now
+	//nolint:errorlint // we can't choose just one of the errors to "correctly" wrap
+	return false, fmt.Errorf("deploymentIsDeleting error: '%v', definitionIsDeleting error: '%v'",
+		deployErr, defErr)
+}
+
+func (r *ConfigurationPolicyReconciler) deploymentIsDeleting() (bool, error) {
+	key, keyErr := common.GetOperatorNamespacedName()
+	if keyErr != nil {
+		if errors.Is(keyErr, common.ErrNoNamespace) || errors.Is(keyErr, common.ErrRunLocal) {
+			// running locally
+			return false, nil
+		}
+
+		return false, keyErr
+	}
+
+	deployment := appsv1.Deployment{}
+
+	err := r.Get(context.TODO(), key, &deployment)
+	if err != nil {
+		return false, err
+	}
+
+	return deployment.DeletionTimestamp != nil, nil
+}
+
 func (r *ConfigurationPolicyReconciler) definitionIsDeleting() (bool, error) {
 	key := types.NamespacedName{Name: CRDName}
 	v1def := extensionsv1.CustomResourceDefinition{}
@@ -631,13 +699,31 @@ func (r *ConfigurationPolicyReconciler) handleObjectTemplates(plc policyv1.Confi
 		return
 	}
 
-	pruneObjectFinalizer := "policy.open-cluster-management.io/delete-related-objects"
-
 	// object handling for when configurationPolicy is deleted
 	if plc.Spec.PruneObjectBehavior == "DeleteIfCreated" || plc.Spec.PruneObjectBehavior == "DeleteAll" {
+		cleanupNow, err := r.cleanupImmediately()
+		if err != nil {
+			log.Error(err, "Error determining whether to cleanup immediately, requeueing policy")
+
+			return
+		}
+
+		if cleanupNow {
+			if objHasFinalizer(&plc, pruneObjectFinalizer) {
+				plc.SetFinalizers(removeObjFinalizer(&plc, pruneObjectFinalizer))
+
+				err := r.Update(context.TODO(), &plc)
+				if err != nil {
+					log.V(1).Error(err, "Error removing finalizer for configuration policy", plc)
+				}
+			}
+
+			return
+		}
+
 		// set finalizer if it hasn't been set
-		if !configPlcHasFinalizer(plc, pruneObjectFinalizer) {
-			plc.SetFinalizers(addConfigPlcFinalizer(plc, pruneObjectFinalizer))
+		if !objHasFinalizer(&plc, pruneObjectFinalizer) {
+			plc.SetFinalizers(addObjFinalizer(&plc, pruneObjectFinalizer))
 
 			err := r.Update(context.TODO(), &plc)
 			if err != nil {
@@ -647,32 +733,13 @@ func (r *ConfigurationPolicyReconciler) handleObjectTemplates(plc policyv1.Confi
 
 		// kick off object deletion if configurationPolicy has been deleted
 		if plc.ObjectMeta.DeletionTimestamp != nil {
-			// If the CRD is deleting, don't prune objects
-			crdDeleting, err := r.definitionIsDeleting()
-			if err != nil {
-				log.Error(err, "Error getting configurationpolicies CRD, requeueing policy")
-
-				return
-			} else if crdDeleting {
-				log.V(1).Info("The configuraionpolicy CRD is being deleted, ignoring and removing " +
-					"the delete-related-objects finalizer")
-
-				plc.SetFinalizers(removeConfigPlcFinalizer(plc, pruneObjectFinalizer))
-				err := r.Update(context.TODO(), &plc)
-				if err != nil {
-					log.V(1).Error(err, "Error unsetting finalizer for configuration policy", plc)
-				}
-
-				return
-			} // else: CRD is not being deleted
-
 			log.V(1).Info("Config policy has been deleted, handling child objects")
 
 			failures := r.cleanUpChildObjects(plc)
 
 			if len(failures) == 0 {
 				log.V(1).Info("Objects have been successfully cleaned up, removing finalizer")
-				plc.SetFinalizers(removeConfigPlcFinalizer(plc, pruneObjectFinalizer))
+				plc.SetFinalizers(removeObjFinalizer(&plc, pruneObjectFinalizer))
 
 				err := r.Update(context.TODO(), &plc)
 				if err != nil {
@@ -704,9 +771,9 @@ func (r *ConfigurationPolicyReconciler) handleObjectTemplates(plc policyv1.Confi
 
 			return
 		}
-	} else if configPlcHasFinalizer(plc, pruneObjectFinalizer) {
+	} else if objHasFinalizer(&plc, pruneObjectFinalizer) {
 		// if pruneObjectBehavior is none, no finalizer is needed
-		plc.SetFinalizers(removeConfigPlcFinalizer(plc, pruneObjectFinalizer))
+		plc.SetFinalizers(removeObjFinalizer(&plc, pruneObjectFinalizer))
 		err := r.Update(context.TODO(), &plc)
 		if err != nil {
 			log.V(1).Error(err, "Error unsetting finalizer for configuration policy", plc)
@@ -2672,3 +2739,31 @@ func convertPolicyStatusToString(plc *policyv1.ConfigurationPolicy) (results str
 
 	return result
 }
+
+func (r *ConfigurationPolicyReconciler) manageDeploymentFinalizer(shouldBeSet bool) error {
+	key, err := common.GetOperatorNamespacedName()
+	if err != nil {
+		return err
+	}
+
+	deployment := appsv1.Deployment{}
+	if err := r.Client.Get(context.TODO(), key, &deployment); err != nil {
+		return err
+	}
+
+	if objHasFinalizer(&deployment, pruneObjectFinalizer) {
+		if shouldBeSet {
+			return nil
+		}
+
+		deployment.SetFinalizers(removeObjFinalizer(&deployment, pruneObjectFinalizer))
+	} else {
+		if !shouldBeSet {
+			return nil
+		}
+
+		deployment.SetFinalizers(addObjFinalizer(&deployment, pruneObjectFinalizer))
+	}
+
+	return r.Update(context.TODO(), &deployment)
+}

controllers/configurationpolicy_utils.go

@@ -10,6 +10,7 @@ import (
 	"strings"
 
 	apiRes "k8s.io/apimachinery/pkg/api/resource"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/util/json"
@@ -530,8 +531,8 @@ func sortAndJoinKeys(m map[string]bool, sep string) string {
 	return strings.Join(keys, sep)
 }
 
-func configPlcHasFinalizer(plc policyv1.ConfigurationPolicy, finalizer string) bool {
-	for _, existingFinalizer := range plc.GetFinalizers() {
+func objHasFinalizer(obj metav1.Object, finalizer string) bool {
+	for _, existingFinalizer := range obj.GetFinalizers() {
 		if existingFinalizer == finalizer {
 			return true
 		}
@@ -540,18 +541,19 @@ func configPlcHasFinalizer(plc policyv1.ConfigurationPolicy, finalizer string) b
 	return false
 }
 
-func addConfigPlcFinalizer(plc policyv1.ConfigurationPolicy, finalizer string) []string {
-	if configPlcHasFinalizer(plc, finalizer) {
-		return plc.GetFinalizers()
+func addObjFinalizer(obj metav1.Object, finalizer string) []string {
+	if objHasFinalizer(obj, finalizer) {
+		return obj.GetFinalizers()
 	}
 
-	return append(plc.GetFinalizers(), finalizer)
+	return append(obj.GetFinalizers(), finalizer)
 }
 
-func removeConfigPlcFinalizer(plc policyv1.ConfigurationPolicy, finalizer string) []string {
+// nolint: unparam
+func removeObjFinalizer(obj metav1.Object, finalizer string) []string {
 	result := []string{}
 
-	for _, existingFinalizer := range plc.GetFinalizers() {
+	for _, existingFinalizer := range obj.GetFinalizers() {
 		if existingFinalizer != finalizer {
 			result = append(result, existingFinalizer)
 		}

main.go

@@ -15,9 +15,8 @@ import (
 	"github.com/go-logr/zapr"
 	"github.com/spf13/pflag"
 	"github.com/stolostron/go-log-utils/zaputil"
+	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
-
-	// to ensure that exec-entrypoint and run can make use of them.
 	extensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	extensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
 	"k8s.io/apimachinery/pkg/fields"
@@ -26,6 +25,8 @@ import (
 	"k8s.io/client-go/dynamic"
 	"k8s.io/client-go/kubernetes"
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
+
+	// Import all k8s client auth plugins to ensure that exec-entrypoint and run can use them
 	_ "k8s.io/client-go/plugin/pkg/client/auth"
 	"k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/clientcmd"
@@ -147,14 +148,6 @@ func main() {
 
 	printVersion()
 
-	namespace, err := common.GetWatchNamespace()
-	if err != nil {
-		log.Error(err, "Failed to get watch namespace")
-		os.Exit(1)
-	}
-
-	log.V(2).Info("Configured the watch namespace", "namespace", namespace)
-
 	// Get a config to talk to the apiserver
 	cfg, err := config.GetConfig()
 	if err != nil {
@@ -163,29 +156,65 @@ func main() {
 	}
 
 	// Set a field selector so that a watch on CRDs will be limited to just the configuration policy CRD.
-	newCacheFunc := cache.BuilderWithOptions(
-		cache.Options{
-			SelectorsByObject: cache.SelectorsByObject{
-				&extensionsv1.CustomResourceDefinition{}: {
-					Field: fields.SelectorFromSet(fields.Set{"metadata.name": controllers.CRDName}),
-				},
-				&extensionsv1beta1.CustomResourceDefinition{}: {
-					Field: fields.SelectorFromSet(fields.Set{"metadata.name": controllers.CRDName}),
-				},
-			},
+	cacheSelectors := cache.SelectorsByObject{
+		&extensionsv1.CustomResourceDefinition{}: {
+			Field: fields.SelectorFromSet(fields.Set{"metadata.name": controllers.CRDName}),
 		},
-	)
+		&extensionsv1beta1.CustomResourceDefinition{}: {
+			Field: fields.SelectorFromSet(fields.Set{"metadata.name": controllers.CRDName}),
+		},
+	}
+
+	ctrlKey, err := common.GetOperatorNamespacedName()
+	if err != nil {
+		if errors.Is(err, common.ErrNoNamespace) || errors.Is(err, common.ErrRunLocal) {
+			log.Info("Running locally, skipping restrictions on the Deployment cache")
+		} else {
+			log.Error(err, "Failed to identify the controller's deployment")
+			os.Exit(1)
+		}
+	} else {
+		cacheSelectors[&appsv1.Deployment{}] = cache.ObjectSelector{
+			Field: fields.SelectorFromSet(fields.Set{
+				"metadata.namespace": ctrlKey.Namespace,
+				"metadata.name":      ctrlKey.Name,
+			}),
+		}
+	}
+
+	watchNamespace, err := common.GetWatchNamespace()
+	if err != nil {
+		log.Error(err, "Failed to get watch namespace")
+		os.Exit(1)
+	}
+
+	if strings.Contains(watchNamespace, ",") {
+		err = fmt.Errorf("multiple watched namespaces are not allowed for this controller")
+		log.Error(err, "Failed to get watch namespace")
+		os.Exit(1)
+	}
+
+	log.V(2).Info("Configured the watch namespace", "watchNamespace", watchNamespace)
+
+	if watchNamespace != "" {
+		cacheSelectors[&policyv1.ConfigurationPolicy{}] = cache.ObjectSelector{
+			Field: fields.SelectorFromSet(fields.Set{
+				"metadata.namespace": watchNamespace,
+			}),
+		}
+	} else {
+		log.Info("Skipping restrictions on the ConfigurationPolicy cache because watchNamespace is empty")
+	}
 
 	// Set default manager options
 	options := manager.Options{
-		Namespace:              namespace,
 		MetricsBindAddress:     metricsAddr,
 		Scheme:                 scheme,
 		Port:                   9443,
 		HealthProbeBindAddress: probeAddr,
 		LeaderElection:         enableLeaderElection,
 		LeaderElectionID:       "config-policy-controller.open-cluster-management.io",
-		NewCache:               newCacheFunc,
+		NewCache:               cache.BuilderWithOptions(cache.Options{SelectorsByObject: cacheSelectors}),
 		// Disable the cache for Secrets to avoid a watch getting created when the `policy-encryption-key`
 		// Secret is retrieved. Special cache handling is done by the controller.
 		ClientDisableCacheFor: []client.Object{&corev1.Secret{}},
@@ -217,11 +246,6 @@ func main() {
 		),
 	}
 
-	if strings.Contains(namespace, ",") {
-		options.Namespace = ""
-		options.NewCache = cache.MultiNamespacedCacheBuilder(strings.Split(namespace, ","))
-	}
-
 	if legacyLeaderElection {
 		// If legacyLeaderElection is enabled, then that means the lease API is not available.
 		// In this case, use the legacy leader election method of a ConfigMap.