🤖 Sync from open-cluster-management-io/config-policy-controller: #227

controllers/operatorpolicy_controller.go

@@ -632,13 +632,15 @@ func (r *OperatorPolicyReconciler) mustnothaveOpGroup(
  return nil, changed, nil
  }
 
- foundOpGroupName := ""
+ var foundOpGroupName string
+ var foundOpGroup *unstructured.Unstructured
 
  for _, opGroup := range foundOpGroups {
  emptyNameMatch := desiredOpGroup.Name == "" && opGroup.GetGenerateName() == desiredOpGroup.GenerateName
 
  if opGroup.GetName() == desiredOpGroup.Name || emptyNameMatch {
  foundOpGroupName = opGroup.GetName()
+ foundOpGroup = opGroup.DeepCopy()
 
  break
  }
@@ -684,6 +686,11 @@ func (r *OperatorPolicyReconciler) mustnothaveOpGroup(
  }
  }
 
+ if foundOpGroup.GetDeletionTimestamp() != nil {
+ // No "early" condition because that would cause the status to flap
+ return nil, updateStatus(policy, deletingCond("OperatorGroup"), deletingObj(desiredOpGroup)), nil
+ }
+
  earlyConds := []metav1.Condition{}
 
  if changed {
@@ -872,6 +879,11 @@ func (r *OperatorPolicyReconciler) mustnothaveSubscription(
  return foundSub, nil, changed, nil
  }
 
+ if foundSub.GetDeletionTimestamp() != nil {
+ // No "early" condition because that would cause the status to flap
+ return foundSub, nil, updateStatus(policy, deletingCond("Subscription"), deletingObj(foundSub)), nil
+ }
+
  earlyConds := []metav1.Condition{}
 
  if changed {
@@ -1174,21 +1186,40 @@ func (r *OperatorPolicyReconciler) mustnothaveInstallPlan(
  earlyConds = append(earlyConds, calculateComplianceCondition(policy))
  }
 
- deletedInstallPlans := make([]policyv1.RelatedObject, 0, len(ownedInstallPlans))
+ anyAlreadyDeleting := false
 
  for i := range ownedInstallPlans {
+ if ownedInstallPlans[i].GetDeletionTimestamp() != nil {
+ anyAlreadyDeleting = true
+ relatedInstallPlans[i] = deletingObj(&ownedInstallPlans[i])
+
+ continue
+ }
+
  err := r.Delete(ctx, &ownedInstallPlans[i])
  if err != nil {
- changed := updateStatus(policy, foundNotWantedCond("InstallPlan"), deletedInstallPlans...)
+ changed := updateStatus(policy, foundNotWantedCond("InstallPlan"), relatedInstallPlans...)
+
+ if anyAlreadyDeleting {
+ // reset the "early" conditions to avoid flapping.
+ earlyConds = []metav1.Condition{}
+ }
 
  return earlyConds, changed, fmt.Errorf("error deleting the InstallPlan: %w", err)
  }
 
  ownedInstallPlans[i].SetGroupVersionKind(installPlanGVK) // Delete stripped this information
- deletedInstallPlans = append(deletedInstallPlans, deletedObj(&ownedInstallPlans[i]))
+ relatedInstallPlans[i] = deletedObj(&ownedInstallPlans[i])
+ }
+
+ if anyAlreadyDeleting {
+ // reset the "early" conditions to avoid flapping.
+ earlyConds = []metav1.Condition{}
+
+ return earlyConds, updateStatus(policy, deletingCond("InstallPlan"), relatedInstallPlans...), nil
  }
 
- updateStatus(policy, deletedCond("InstallPlan"), deletedInstallPlans...)
+ updateStatus(policy, deletedCond("InstallPlan"), relatedInstallPlans...)
 
  return earlyConds, true, nil
 }
@@ -1285,21 +1316,40 @@ func (r *OperatorPolicyReconciler) mustnothaveCSV(
  earlyConds = append(earlyConds, calculateComplianceCondition(policy))
  }
 
- deletedCSVs := make([]policyv1.RelatedObject, 0, len(csvList))
+ anyAlreadyDeleting := false
 
  for i := range csvList {
+ if csvList[i].GetDeletionTimestamp() != nil {
+ anyAlreadyDeleting = true
+ relatedCSVs[i] = deletingObj(&csvList[i])
+
+ continue
+ }
+
  err := r.Delete(ctx, &csvList[i])
  if err != nil {
- changed := updateStatus(policy, foundNotWantedCond("ClusterServiceVersion"), deletedCSVs...)
+ changed := updateStatus(policy, foundNotWantedCond("ClusterServiceVersion"), relatedCSVs...)
+
+ if anyAlreadyDeleting {
+ // reset the "early" conditions to avoid flapping
+ earlyConds = []metav1.Condition{}
+ }
 
  return earlyConds, changed, fmt.Errorf("error deleting ClusterServiceVersion: %w", err)
  }
 
  csvList[i].SetGroupVersionKind(clusterServiceVersionGVK)
- deletedCSVs = append(deletedCSVs, deletedObj(&csvList[i]))
+ relatedCSVs[i] = deletedObj(&csvList[i])
+ }
+
+ if anyAlreadyDeleting {
+ // reset the "early" conditions to avoid flapping
+ earlyConds = []metav1.Condition{}
+
+ return earlyConds, updateStatus(policy, deletingCond("ClusterServiceVersion"), relatedCSVs...), nil
  }
 
- updateStatus(policy, deletedCond("ClusterServiceVersion"), deletedCSVs...)
+ updateStatus(policy, deletedCond("ClusterServiceVersion"), relatedCSVs...)
 
  return earlyConds, true, nil
 }
@@ -1420,21 +1470,40 @@ func (r *OperatorPolicyReconciler) handleCRDs(
  earlyConds = append(earlyConds, calculateComplianceCondition(policy))
  }
 
- deletedCRDs := make([]policyv1.RelatedObject, 0, len(crdList))
+ anyAlreadyDeleting := false
 
  for i := range crdList {
+ if crdList[i].GetDeletionTimestamp() != nil {
+ anyAlreadyDeleting = true
+ relatedCRDs[i] = deletingObj(&crdList[i])
+
+ continue
+ }
+
  err := r.Delete(ctx, &crdList[i])
  if err != nil {
- changed := updateStatus(policy, foundNotWantedCond("CustomResourceDefinition"), deletedCRDs...)
+ changed := updateStatus(policy, foundNotWantedCond("CustomResourceDefinition"), relatedCRDs...)
+
+ if anyAlreadyDeleting {
+ // reset the "early" conditions to avoid flapping.
+ earlyConds = []metav1.Condition{}
+ }
 
  return earlyConds, changed, fmt.Errorf("error deleting the CRD: %w", err)
  }
 
  crdList[i].SetGroupVersionKind(customResourceDefinitionGVK)
- deletedCRDs = append(deletedCRDs, deletedObj(&crdList[i]))
+ relatedCRDs[i] = deletedObj(&crdList[i])
+ }
+
+ if anyAlreadyDeleting {
+ // reset the "early" conditions to avoid flapping.
+ earlyConds = []metav1.Condition{}
+
+ return earlyConds, updateStatus(policy, deletingCond("CustomResourceDefinition"), relatedCRDs...), nil
  }
 
- updateStatus(policy, deletedCond("CustomResourceDefinition"), deletedCRDs...)
+ updateStatus(policy, deletedCond("CustomResourceDefinition"), relatedCRDs...)
 
  return earlyConds, true, nil
 }

controllers/operatorpolicy_status.go

@@ -459,6 +459,17 @@ func deletedCond(kind string) metav1.Condition {
  }
 }
 
+// deletingCond returns a NonCompliant condition, with a Reason like '____Deleting',
+// and a Message like 'the ____ has a deletion timestamp'
+func deletingCond(kind string) metav1.Condition {
+ return metav1.Condition{
+ Type: condType(kind),
+ Status: metav1.ConditionFalse,
+ Reason: kind + "Deleting",
+ Message: "the " + kind + " has a deletion timestamp",
+ }
+}
+
 // keptCond returns a Compliant condition, with a Reason like '____Kept',
 // and a Message like 'the policy specifies to keep the ____'
 func keptCond(kind string) metav1.Condition {
@@ -839,6 +850,14 @@ func deletedObj(obj client.Object) policyv1.RelatedObject {
  }
 }
 
+func deletingObj(obj client.Object) policyv1.RelatedObject {
+ return policyv1.RelatedObject{
+ Object: policyv1.ObjectResourceFromObj(obj),
+ Compliant: string(policyv1.NonCompliant),
+ Reason: "The object is being deleted but has not been removed yet",
+ }
+}
+
 // matchedObj returns a Compliant RelatedObject with reason = 'Resource found as expected'
 func matchedObj(obj client.Object) policyv1.RelatedObject {
  return policyv1.RelatedObject{

test/e2e/case38_install_operator_test.go

@@ -1902,15 +1902,185 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() {
  utils.GetWithTimeout(clientManagedDynamic, gvrCRD, "quayregistries.quay.redhat.com",
  "", true, eventuallyTimeout)
  })
- It("Should remove things when enforced while set to Delete everything", func(ctx SpecContext) {
- // Change the removal behaviors from Keep to Delete
+ It("Should report a special status when the resources are stuck", func(ctx SpecContext) {
+ By("Adding a finalizer to each of the resources")
+ pol, err := clientManagedDynamic.Resource(gvrOperatorPolicy).
+ Namespace(opPolTestNS).Get(ctx, opPolName, metav1.GetOptions{})
+ Expect(err).NotTo(HaveOccurred())
+ Expect(pol).NotTo(BeNil())
+
+ relatedObjects, found, err := unstructured.NestedSlice(pol.Object, "status", "relatedObjects")
+ Expect(err).NotTo(HaveOccurred())
+ Expect(found).To(BeTrue())
+
+ var opGroupName, installPlanName, csvName string
+ crdNames := make([]string, 0)
+
+ for _, relatedObject := range relatedObjects {
+ relatedObj, ok := relatedObject.(map[string]interface{})
+ Expect(ok).To(BeTrue())
+
+ objKind, found, err := unstructured.NestedString(relatedObj, "object", "kind")
+ Expect(err).NotTo(HaveOccurred())
+ Expect(found).To(BeTrue())
+
+ objName, found, err := unstructured.NestedString(relatedObj, "object", "metadata", "name")
+ Expect(err).NotTo(HaveOccurred())
+ Expect(found).To(BeTrue())
+
+ switch objKind {
+ case "OperatorGroup":
+ opGroupName = objName
+ case "Subscription":
+ // just do the finalizer; we already know the subscription name
+ case "ClusterServiceVersion":
+ csvName = objName
+ case "InstallPlan":
+ installPlanName = objName
+ case "CustomResourceDefinition":
+ crdNames = append(crdNames, objName)
+ default:
+ // skip adding / removing the finalizer for other types
+ continue
+ }
+
+ utils.Kubectl("patch", objKind, objName, "-n", opPolTestNS, "--type=json", "-p",
+ `[{"op": "add", "path": "/metadata/finalizers", "value": ["donutdelete"]}]`)
+ DeferCleanup(func() {
+ By("removing the finalizer from " + objKind + " " + objName)
+ utils.Kubectl("patch", objKind, objName, "-n", opPolTestNS, "--type=json", "-p",
+ `[{"op": "remove", "path": "/metadata/finalizers"}]`)
+ })
+ }
+
+ By("Setting the removal behaviors to Delete")
  utils.Kubectl("patch", "operatorpolicy", opPolName, "-n", opPolTestNS, "--type=json", "-p",
  `[{"op": "replace", "path": "/spec/removalBehavior/operatorGroups", "value": "Delete"},`+
  `{"op": "replace", "path": "/spec/removalBehavior/subscriptions", "value": "Delete"},`+
  `{"op": "replace", "path": "/spec/removalBehavior/clusterServiceVersions", "value": "Delete"},`+
  `{"op": "replace", "path": "/spec/removalBehavior/installPlans", "value": "Delete"},`+
  `{"op": "replace", "path": "/spec/removalBehavior/customResourceDefinitions", "value": "Delete"}]`)
 
+ check(
+ opPolName,
+ false,
+ []policyv1.RelatedObject{{
+ Object: policyv1.ObjectResource{
+ Kind: "OperatorGroup",
+ APIVersion: "operators.coreos.com/v1",
+ Metadata: policyv1.ObjectMetadata{
+ Namespace: opPolTestNS,
+ Name: opGroupName,
+ },
+ },
+ Compliant: "NonCompliant",
+ Reason: "The object is being deleted but has not been removed yet",
+ }},
+ metav1.Condition{
+ Type: "OperatorGroupCompliant",
+ Status: metav1.ConditionFalse,
+ Reason: "OperatorGroupDeleting",
+ Message: "the OperatorGroup has a deletion timestamp",
+ },
+ `the OperatorGroup was deleted`,
+ )
+ check(
+ opPolName,
+ false,
+ []policyv1.RelatedObject{{
+ Object: policyv1.ObjectResource{
+ Kind: "Subscription",
+ APIVersion: "operators.coreos.com/v1alpha1",
+ Metadata: policyv1.ObjectMetadata{
+ Namespace: opPolTestNS,
+ Name: "project-quay",
+ },
+ },
+ Compliant: "NonCompliant",
+ Reason: "The object is being deleted but has not been removed yet",
+ }},
+ metav1.Condition{
+ Type: "SubscriptionCompliant",
+ Status: metav1.ConditionFalse,
+ Reason: "SubscriptionDeleting",
+ Message: "the Subscription has a deletion timestamp",
+ },
+ `the Subscription was deleted`,
+ )
+ check(
+ opPolName,
+ false,
+ []policyv1.RelatedObject{{
+ Object: policyv1.ObjectResource{
+ Kind: "InstallPlan",
+ APIVersion: "operators.coreos.com/v1alpha1",
+ Metadata: policyv1.ObjectMetadata{
+ Namespace: opPolTestNS,
+ Name: installPlanName,
+ },
+ },
+ Compliant: "NonCompliant",
+ Reason: "The object is being deleted but has not been removed yet",
+ }},
+ metav1.Condition{
+ Type: "InstallPlanCompliant",
+ Status: metav1.ConditionFalse,
+ Reason: "InstallPlanDeleting",
+ Message: "the InstallPlan has a deletion timestamp",
+ },
+ `the InstallPlan was deleted`,
+ )
+ check(
+ opPolName,
+ false,
+ []policyv1.RelatedObject{{
+ Object: policyv1.ObjectResource{
+ Kind: "ClusterServiceVersion",
+ APIVersion: "operators.coreos.com/v1alpha1",
+ Metadata: policyv1.ObjectMetadata{
+ Namespace: opPolTestNS,
+ Name: csvName,
+ },
+ },
+ Compliant: "NonCompliant",
+ Reason: "The object is being deleted but has not been removed yet",
+ }},
+ metav1.Condition{
+ Type: "ClusterServiceVersionCompliant",
+ Status: metav1.ConditionFalse,
+ Reason: "ClusterServiceVersionDeleting",
+ Message: "the ClusterServiceVersion has a deletion timestamp",
+ },
+ `the ClusterServiceVersion was deleted`,
+ )
+ desiredCRDObjects := make([]policyv1.RelatedObject, 0)
+ for _, name := range crdNames {
+ desiredCRDObjects = append(desiredCRDObjects, policyv1.RelatedObject{
+ Object: policyv1.ObjectResource{
+ Kind: "CustomResourceDefinition",
+ APIVersion: "apiextensions.k8s.io/v1",
+ Metadata: policyv1.ObjectMetadata{
+ Name: name,
+ },
+ },
+ Compliant: "NonCompliant",
+ Reason: "The object is being deleted but has not been removed yet",
+ })
+ }
+ check(
+ opPolName,
+ false,
+ desiredCRDObjects,
+ metav1.Condition{
+ Type: "CustomResourceDefinitionCompliant",
+ Status: metav1.ConditionFalse,
+ Reason: "CustomResourceDefinitionDeleting",
+ Message: "the CustomResourceDefinition has a deletion timestamp",
+ },
+ `the CustomResourceDefinition was deleted`,
+ )
+ })
+ It("Should report things as gone after the finalizers are removed", func() {
  By("Checking that certain (named) resources are not there, indicating the removal was completed")
  utils.GetWithTimeout(clientManagedDynamic, gvrClusterServiceVersion, "quay-operator.v3.8.13",
  opPolTestNS, false, eventuallyTimeout)