Skip pruning when the CRD is being deleted

In some upgrade cases, the ConfigurationPolicy CRD might be temporarily
removed. This usually triggers all of the ConfigurationPolicies to be
deleted, which would trigger the pruning behavior. But deleting those
resources could cause problems, even though they might immediately be
recreated when the upgrade finishes. Even outside of upgrades, deleting
the CRD "by accident" should probably not trigger the prune behavior.

Now, before pruning anything, the controller checks if the CRD is being
deleted.

BUG: `DeleteIfCreated` will not work perfectly through an upgrade: if
the object was created by the policy *before* the CRD was removed, it
won't be pruned by the policy after everything is re-created. But
missing a deletion seems like a better behavior than deleting extra
things.

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

Signed-off-by: Justin Kulikauskas <jkulikau@redhat.com>
(cherry picked from commit d426227)

controllers/configurationpolicy_controller.go

@@ -19,12 +19,15 @@ import (
  "github.com/prometheus/client_golang/prometheus"
  templates "github.com/stolostron/go-template-utils/v3/pkg/templates"
  corev1 "k8s.io/api/core/v1"
+ extensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
+ extensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
  k8serrors "k8s.io/apimachinery/pkg/api/errors"
  meta "k8s.io/apimachinery/pkg/api/meta"
  metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
  "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
  "k8s.io/apimachinery/pkg/runtime"
  "k8s.io/apimachinery/pkg/runtime/schema"
+ "k8s.io/apimachinery/pkg/types"
  apimachineryerrors "k8s.io/apimachinery/pkg/util/errors"
  "k8s.io/apimachinery/pkg/util/json"
  "k8s.io/client-go/dynamic"
@@ -557,6 +560,32 @@ func (r *ConfigurationPolicyReconciler) cleanUpChildObjects(plc policyv1.Configu
  return deletionFailures
 }
 
+func (r *ConfigurationPolicyReconciler) definitionIsDeleting() (bool, error) {
+ key := types.NamespacedName{Name: "configurationpolicies.policy.open-cluster-management.io"}
+ v1def := extensionsv1.CustomResourceDefinition{}
+
+ v1err := r.Get(context.TODO(), key, &v1def)
+ if v1err == nil {
+ return (v1def.ObjectMeta.DeletionTimestamp != nil), nil
+ }
+
+ v1beta1def := extensionsv1beta1.CustomResourceDefinition{}
+
+ v1beta1err := r.Get(context.TODO(), key, &v1beta1def)
+ if v1beta1err == nil {
+ return (v1beta1def.DeletionTimestamp != nil), nil
+ }
+
+ // It might not be possible to get a not-found on the CRD while reconciling the CR...
+ // But in that case, it seems reasonable to still consider it "deleting"
+ if k8serrors.IsNotFound(v1err) || k8serrors.IsNotFound(v1beta1err) {
+ return true, nil
+ }
+
+ // Both had unexpected errors, return them and retry later
+ return false, fmt.Errorf("v1: %v, v1beta1: %v", v1err, v1beta1err) //nolint:errorlint
+}
+
 // handleObjectTemplates iterates through all policy templates in a given policy and processes them
 func (r *ConfigurationPolicyReconciler) handleObjectTemplates(plc policyv1.ConfigurationPolicy) {
  log := log.WithValues("policy", plc.GetName())
@@ -613,6 +642,25 @@ 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)

main.go

@@ -16,12 +16,14 @@ import (
  "github.com/spf13/pflag"
  "github.com/stolostron/go-log-utils/zaputil"
  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"
  k8sruntime "k8s.io/apimachinery/pkg/runtime"
  utilruntime "k8s.io/apimachinery/pkg/util/runtime"
  "k8s.io/client-go/kubernetes"
  clientgoscheme "k8s.io/client-go/kubernetes/scheme"
-
- // to ensure that exec-entrypoint and run can make use of them.
  _ "k8s.io/client-go/plugin/pkg/client/auth"
  "k8s.io/client-go/rest"
  "k8s.io/client-go/tools/clientcmd"
@@ -56,6 +58,8 @@ func init() {
  utilruntime.Must(clientgoscheme.AddToScheme(scheme))
  //+kubebuilder:scaffold:scheme
  utilruntime.Must(policyv1.AddToScheme(scheme))
+ utilruntime.Must(extensionsv1.AddToScheme(scheme))
+ utilruntime.Must(extensionsv1beta1.AddToScheme(scheme))
 }
 
 func main() {

test/e2e/case20_delete_objects_test.go

@@ -24,6 +24,8 @@ const (
  case20ConfigPolicyNameInform string = "policy-pod-inform"
  case20ConfigPolicyNameFinalizer string = "policy-pod-create-withfinalizer"
  case20ConfigPolicyNameChange string = "policy-pod-change-remediation"
+ case20ConfigPolicyNameMHPDA string = "policy-pod-mhpda"
+ case20PodMHPDAName string = "nginx-pod-e2e20-mhpda"
  case20PodYaml string = "../resources/case20_delete_objects/case20_pod.yaml"
  case20PolicyYamlCreate string = "../resources/case20_delete_objects/case20_create_pod.yaml"
  case20PolicyYamlEdit string = "../resources/case20_delete_objects/case20_edit_pod.yaml"
@@ -32,6 +34,10 @@ const (
  case20PolicyYamlFinalizer string = "../resources/case20_delete_objects/case20_createpod_finalizer.yaml"
  case20PolicyYamlChangeInform string = "../resources/case20_delete_objects/case20_change_inform.yaml"
  case20PolicyYamlChangeEnforce string = "../resources/case20_delete_objects/case20_change_enforce.yaml"
+ case20PolicyYamlMHPDA string = "../resources/case20_delete_objects/case20_musthave_pod_deleteall.yaml"
+
+ // For the CRD deletion test
+ case20ConfigPolicyCRDPath string = "../../deploy/crds/policy.open-cluster-management.io_configurationpolicies.yaml"
 )
 
 var _ = Describe("Test status fields being set for object deletion", func() {
@@ -532,3 +538,69 @@ var _ = Describe("Test objects that should be deleted are actually being deleted
  })
  })
 })
+
+var _ = Describe("Test objects are not deleted when the CRD is removed", Ordered, func() {
+ AfterAll(func() {
+ deleteConfigPolicies([]string{case20ConfigPolicyNameMHPDA})
+ utils.Kubectl("apply", "-f", case20ConfigPolicyCRDPath)
+ })
+
+ It("creates the policy to manage a pod", func() {
+ By("Creating " + case20ConfigPolicyNameMHPDA + " on managed")
+ utils.Kubectl("apply", "-f", case20PolicyYamlMHPDA, "-n", testNamespace)
+ plc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy,
+ case20ConfigPolicyNameMHPDA, testNamespace, true, defaultTimeoutSeconds)
+ Expect(plc).NotTo(BeNil())
+ Eventually(func() interface{} {
+ managedPlc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy,
+ case20ConfigPolicyNameMHPDA, testNamespace, true, defaultTimeoutSeconds)
+
+ return utils.GetComplianceState(managedPlc)
+ }, defaultTimeoutSeconds, 1).Should(Equal("Compliant"))
+ })
+
+ It("deletes the ConfigurationPolicy CRD and compares the pod UID before and after", func() {
+ By("Getting the pod UID")
+ oldPodUID := ""
+ Eventually(func() interface{} {
+ pod := utils.GetWithTimeout(clientManagedDynamic, gvrPod,
+ case20PodMHPDAName, "default", true, defaultTimeoutSeconds)
+
+ oldPodUID = string(pod.GetUID())
+
+ return pod
+ }, defaultTimeoutSeconds, 1).Should(Not(BeNil()))
+
+ By("Deleting the ConfigurationPolicy CRD")
+ utils.Kubectl("delete", "-f", case20ConfigPolicyCRDPath)
+
+ By("Checking that the ConfigurationPolicy is gone")
+ namespace := clientManagedDynamic.Resource(gvrConfigPolicy).Namespace(testNamespace)
+ _, err := namespace.Get(context.TODO(), case20ConfigPolicyNameMHPDA, metav1.GetOptions{})
+ Expect(err).NotTo(BeNil())
+ Expect(err.Error()).To(ContainSubstring("the server could not find the requested resource"))
+
+ By("Recreating the CRD")
+ utils.Kubectl("apply", "-f", case20ConfigPolicyCRDPath)
+
+ By("Recreating the ConfigurationPolicy")
+ utils.Kubectl("apply", "-f", case20PolicyYamlMHPDA, "-n", testNamespace)
+ plc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy,
+ case20ConfigPolicyNameMHPDA, testNamespace, true, defaultTimeoutSeconds)
+ Expect(plc).NotTo(BeNil())
+ Eventually(func() interface{} {
+ managedPlc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy,
+ case20ConfigPolicyNameMHPDA, testNamespace, true, defaultTimeoutSeconds)
+
+ return utils.GetComplianceState(managedPlc)
+ }, defaultTimeoutSeconds, 1).Should(Equal("Compliant"))
+
+ By("Checking the pod UID")
+ Eventually(func() interface{} {
+ pod := utils.GetWithTimeout(clientManagedDynamic, gvrPod,
+ case20PodMHPDAName, "default", true, defaultTimeoutSeconds)
+
+ return string(pod.GetUID())
+ }, defaultTimeoutSeconds, 1).Should(Equal(oldPodUID))
+ })
+})

test/resources/case20_delete_objects/case20_musthave_pod_deleteall.yaml

@@ -0,0 +1,23 @@
+apiVersion: policy.open-cluster-management.io/v1
+kind: ConfigurationPolicy
+metadata:
+ name: policy-pod-mhpda
+spec:
+ remediationAction: enforce
+ namespaceSelector:
+ exclude: ["kube-*"]
+ include: ["default"]
+ pruneObjectBehavior: DeleteAll
+ object-templates:
+ - complianceType: musthave
+ objectDefinition:
+ apiVersion: v1
+ kind: Pod
+ metadata:
+ name: nginx-pod-e2e20-mhpda
+ spec:
+ containers:
+ - image: nginx:1.7.9
+ name: nginx
+ ports:
+ - containerPort: 80