Skip to content

Commit

Permalink
Clean up when deployment is being deleted
Browse files Browse the repository at this point in the history 
Previously, when the controller is removed, it could leave a mess behind
in the form of ConfigurationPolicy objects with finalizers on them. That
would also result in the CRD being stuck.

Now, the controller will put a finalizer on its own deployment when at
least one ConfigurationPolicy needs a finalizer. Also, while the
deployment is being deleted, the finalizers on ConfigurationPolicies
will be removed immediately, in the same way they would be when the CRD
was being deleted.

Note: in this implementation, related objects will *never* be pruned
when the controller is removed.

Refs:
 - https://issues.redhat.com/browse/ACM-2923

Signed-off-by: Justin Kulikauskas <jkulikau@redhat.com>
  • Loading branch information
@JustinKuli
JustinKuli committed Jan 19, 2023
1 parent fbd1c67 commit c8f8abb
Show file tree
Hide file tree
Showing 3 changed files with 135 additions and 27 deletions.
106 changes: 87 additions & 19 deletions controllers/configurationpolicy_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -188,6 +190,26 @@ func (r *ConfigurationPolicyReconciler) PeriodicallyExecConfigPolicies(freq uint
skipLoop = true
}

needDeploymentFinalizer := false

for _, plc := range policiesList.Items {
if plc.Spec.PruneObjectBehavior == "DeleteAll" || plc.Spec.PruneObjectBehavior == "DeleteIfCreated" {
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))
Expand Down Expand Up @@ -565,18 +587,38 @@ func (r *ConfigurationPolicyReconciler) cleanUpChildObjects(plc policyv1.Configu
return deletionFailures
}

func (r *ConfigurationPolicyReconciler) definitionIsDeleting() (bool, error) {
key := types.NamespacedName{Name: CRDName}
// 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) {
runLocal := false

opKey, opErr := common.GetOperatorNamespacedName()
if opErr != nil && (errors.Is(opErr, common.ErrNoNamespace) || errors.Is(opErr, common.ErrRunLocal)) {
runLocal = true
opErr = nil
}

if !runLocal && opErr == nil {
deployment := appsv1.Deployment{}

opErr = r.Get(context.TODO(), opKey, &deployment)
if opErr == nil && deployment.DeletionTimestamp != nil {
return true, nil
}
}

crdKey := types.NamespacedName{Name: CRDName}
v1def := extensionsv1.CustomResourceDefinition{}

v1err := r.Get(context.TODO(), key, &v1def)
v1err := r.Get(context.TODO(), crdKey, &v1def)
if v1err == nil {
return (v1def.ObjectMeta.DeletionTimestamp != nil), nil
}

v1beta1def := extensionsv1beta1.CustomResourceDefinition{}

v1beta1err := r.Get(context.TODO(), key, &v1beta1def)
v1beta1err := r.Get(context.TODO(), crdKey, &v1beta1def)
if v1beta1err == nil {
return (v1beta1def.DeletionTimestamp != nil), nil
}
Expand All @@ -587,8 +629,8 @@ func (r *ConfigurationPolicyReconciler) definitionIsDeleting() (bool, error) {
return true, nil
}

// Both had unexpected errors, return them and retry later
return false, fmt.Errorf("v1: %v, v1beta1: %v", v1err, v1beta1err) //nolint:errorlint
// Unexpected errors prevent a decision from being made, return them and retry later
return false, fmt.Errorf("op: %v, v1: %v, v1beta1: %v", opErr, v1err, v1beta1err) //nolint:errorlint
}

// handleObjectTemplates iterates through all policy templates in a given policy and processes them
Expand Down Expand Up @@ -631,13 +673,11 @@ 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" {
// 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 {
Expand All @@ -648,31 +688,31 @@ 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()
cleanupNow, err := r.cleanupImmediately()
if err != nil {
log.Error(err, "Error getting configurationpolicies CRD, requeueing policy")
log.Error(err, "Error determining whether to cleanup immediately, requeueing policy")

return
} else if crdDeleting {
log.V(1).Info("The configuraionpolicy CRD is being deleted, ignoring and removing " +
} else if cleanupNow {
log.V(1).Info("Cleaning up immediately by ignoring and removing " +
"the delete-related-objects finalizer")

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

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 {
Expand Down Expand Up @@ -704,9 +744,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)
Expand Down Expand Up @@ -2672,3 +2712,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.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)
}
18 changes: 10 additions & 8 deletions controllers/configurationpolicy_utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
Expand All @@ -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)
}
Expand Down
38 changes: 38 additions & 0 deletions pkg/common/operator-sdk-port.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import (
"fmt"
"os"
"strings"

"k8s.io/apimachinery/pkg/types"
)

type RunModeType string
Expand All @@ -22,6 +24,10 @@ const (
// which specifies the Namespace to watch.
// An empty value means the operator is running with cluster scope.
watchNamespaceEnvVar = "WATCH_NAMESPACE"

// OperatorNameEnvVar is the constant for env variable OPERATOR_NAME
// which is the name of the current operator
OperatorNameEnvVar = "OPERATOR_NAME"
)

// ErrNoNamespace indicates that a namespace could not be found for the current
Expand Down Expand Up @@ -63,3 +69,35 @@ func GetOperatorNamespace() (string, error) {

return strings.TrimSpace(string(nsBytes)), nil
}

// GetOperatorName returns the operator name
func GetOperatorName() (string, error) {
operatorName, found := os.LookupEnv(OperatorNameEnvVar)
if !found {
return "", fmt.Errorf("%s must be set", OperatorNameEnvVar)
}

if len(operatorName) == 0 {
return "", fmt.Errorf("%s must not be empty", OperatorNameEnvVar)
}

return operatorName, nil
}

// GetOperatorNamespacedName returns the name and namespace of the operator.
func GetOperatorNamespacedName() (types.NamespacedName, error) {
key := types.NamespacedName{}
var err error

key.Namespace, err = GetOperatorNamespace()
if err != nil {
return key, err
}

key.Name, err = GetOperatorName()
if err != nil {
return key, err
}

return key, nil
}

0 comments on commit c8f8abb

Please sign in to comment.