Handle deletion prevention correctly

If an object with the deletion prevention annotation is removed from the inventory, the config.k8s.io/owning-inventory annotation should be removed from the object, and the object should be removed from the inventory.
kubernetes-sigs · Oct 12, 2021 · 8d24d1a · 8d24d1a
1 parent 223d4d5
commit 8d24d1a
Show file tree

Hide file tree

Showing 18 changed files with 381 additions and 46 deletions.
diff --git a/examples/alphaTestExamples/pruneAndDelete.md b/examples/alphaTestExamples/pruneAndDelete.md
@@ -34,10 +34,14 @@ function expectedOutputLine() {
 }
 ```
 
-In this example we will just use two ConfigMap resources for simplicity, but
-of course any type of resource can be used. On one of our ConfigMaps, we add the
-**cli-utils.sigs.k8s.io/on-remove** annotation with the value of **keep**. This 
-annotation tells the kapply tool that this resource should not be deleted, even
+In this example we will just use three ConfigMap resources for simplicity, but
+of course any type of resource can be used. 
+
+- the first ConfigMap resource does not have any annotations;
+- the second ConfigMap resource has the **cli-utils.sigs.k8s.io/on-remove** annotation with the value of **keep**; 
+- the third ConfigMap resource has the **client.lifecycle.config.k8s.io/deletion** annotation with the value of **detach**.
+
+These two annotations tell the kapply tool that a resource should not be deleted, even
 if it would otherwise be pruned or deleted with the destroy command.
 
 <!-- @createFirstCM @testE2EAgainstLatestRelease-->
@@ -53,7 +57,7 @@ data:
 EOF
 ```
 
-This ConfigMap includes the lifecycle directive annotation
+This ConfigMap includes the **cli-utils.sigs.k8s.io/on-remove** annotation
 
 <!-- @createSecondCM @testE2EAgainstLatestRelease-->
 ```
@@ -70,6 +74,24 @@ data:
 EOF
 ```
 
+
+This ConfigMap includes the **client.lifecycle.config.k8s.io/deletion** annotation
+
+<!-- @createSecondCM @testE2EAgainstLatestRelease-->
+```
+cat <<EOF >$BASE/configMap3.yaml
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: thirdmap
+  annotations:
+    client.lifecycle.config.k8s.io/deletion: detach
+data:
+  artist: Husker Du
+  album: New Day Rising
+EOF
+```
+
 ## Run end-to-end tests
 
 The following requires installation of [kind].
@@ -90,14 +112,14 @@ expectedOutputLine "namespace: default is used for inventory object"
 
 ```
 
-Apply both resources to the cluster.
+Apply the three resources to the cluster.
 <!-- @runApply @testE2EAgainstLatestRelease -->
 ```
 kapply apply $BASE --reconcile-timeout=1m > $OUTPUT/status
 ```
 
 Use the preview command to show what will happen if we run destroy. This should
-show that the second ConfigMap will not be deleted even when using the destroy 
+show that secondmap and thirdmap will not be deleted even when using the destroy 
 command.
 <!-- @runDestroyPreview @testE2EAgainstLatestRelease -->
 ```
@@ -106,10 +128,12 @@ kapply preview --destroy $BASE > $OUTPUT/status
 expectedOutputLine "configmap/firstmap deleted (preview)"
 
 expectedOutputLine "configmap/secondmap delete skipped (preview)"
+
+expectedOutputLine "configmap/thirdmap delete skipped (preview)"
 ```
 
-We run the destroy command and see that the resource without the annotation
-has been deleted, while the resource with the annotation is still in the 
+We run the destroy command and see that the resource without the annotations (firstmap)
+has been deleted, while the resources with the annotations (secondmap and thirdmap)  are still in the 
 cluster.
 <!-- @runDestroy @testE2EAgainstLatestRelease -->
 ```
@@ -119,45 +143,58 @@ expectedOutputLine "configmap/firstmap deleted"
 
 expectedOutputLine "configmap/secondmap delete skipped"
 
-expectedOutputLine "1 resource(s) deleted, 1 skipped"
+expectedOutputLine "configmap/thirdmap delete skipped"
+
+expectedOutputLine "1 resource(s) deleted, 2 skipped"
 expectedNotFound "resource(s) pruned"
 
 kubectl get cm --no-headers | awk '{print $1}' > $OUTPUT/status
 expectedOutputLine "secondmap"
-```
 
+kubectl get cm --no-headers | awk '{print $1}' > $OUTPUT/status
+expectedOutputLine "thirdmap"
+```
 
 Apply the resources back to the cluster so we can demonstrate the lifecycle
 directive with pruning.
 <!-- @runApplyAgain @testE2EAgainstLatestRelease -->
 ```
-kapply apply $BASE --reconcile-timeout=1m > $OUTPUT/status
+kapply apply $BASE --inventory-policy=adopt --reconcile-timeout=1m > $OUTPUT/status
 ```
 
-Delete the manifest for the second configmap
+Delete the manifest for secondmap and thirdmap
 <!-- @runDeleteManifest @testE2EAgainstLatestRelease -->
 ```
 rm $BASE/configMap2.yaml
+
+rm $BASE/configMap3.yaml
 ```
 
-Run preview to see that while secondmap would normally be pruned, it 
+Run preview to see that while secondmap and thirdmap would normally be pruned, they
 will instead be skipped due to the lifecycle directive.
 <!-- @runPreviewForPrune @testE2EAgainstLatestRelease -->
 ```
 kapply preview $BASE > $OUTPUT/status
 
 expectedOutputLine "configmap/secondmap prune skipped (preview)"
+
+expectedOutputLine "configmap/thirdmap prune skipped (preview)"
 ```
 
-Run apply and verify that secondmap is still in the cluster.
+Run apply and verify that secondmap and thirdmap are still in the cluster.
 <!-- @runApplyToPrune @testE2EAgainstLatestRelease -->
 ```
 kapply apply $BASE > $OUTPUT/status
 
 expectedOutputLine "configmap/secondmap prune skipped"
 
+expectedOutputLine "configmap/thirdmap prune skipped"
+
 kubectl get cm --no-headers | awk '{print $1}' > $OUTPUT/status
 expectedOutputLine "secondmap"
 
+kubectl get cm --no-headers | awk '{print $1}' > $OUTPUT/status
+expectedOutputLine "thirdmap"
+
 kind delete cluster;
 ```
diff --git a/pkg/apply/event/actiongroupeventtype_string.go b/pkg/apply/event/actiongroupeventtype_string.go
diff --git a/pkg/apply/event/applyeventoperation_string.go b/pkg/apply/event/applyeventoperation_string.go
diff --git a/pkg/apply/event/deleteeventoperation_string.go b/pkg/apply/event/deleteeventoperation_string.go
diff --git a/pkg/apply/event/pruneeventoperation_string.go b/pkg/apply/event/pruneeventoperation_string.go
diff --git a/pkg/apply/event/resourceaction_string.go b/pkg/apply/event/resourceaction_string.go
diff --git a/pkg/apply/event/type_string.go b/pkg/apply/event/type_string.go
diff --git a/pkg/apply/prune/prune.go b/pkg/apply/prune/prune.go
@@ -103,21 +103,42 @@ func (po *PruneOptions) Prune(pruneObjs []*unstructured.Unstructured,
 		var filtered bool
 		var reason string
 		var err error
-		for _, filter := range pruneFilters {
-			klog.V(6).Infof("prune filter %s: %s", filter.Name(), pruneID)
-			filtered, reason, err = filter.Filter(pruneObj)
+		for _, pruneFilter := range pruneFilters {
+			klog.V(6).Infof("prune filter %s: %s", pruneFilter.Name(), pruneID)
+			filtered, reason, err = pruneFilter.Filter(pruneObj)
 			if err != nil {
 				if klog.V(5).Enabled() {
-					klog.Errorf("error during %s, (%s): %s", filter.Name(), pruneID, err)
+					klog.Errorf("error during %s, (%s): %s", pruneFilter.Name(), pruneID, err)
 				}
 				taskContext.EventChannel() <- eventFactory.CreateFailedEvent(pruneID, err)
 				taskContext.CapturePruneFailure(pruneID)
 				break
 			}
 			if filtered {
-				klog.V(4).Infof("prune filtered (filter: %q, resource: %q, reason: %q)", filter.Name(), pruneID, reason)
+				klog.V(4).Infof("prune filtered (filter: %q, resource: %q, reason: %q)", pruneFilter.Name(), pruneID, reason)
+				// pruneFailure indicates whether `taskContext.CapturePruneFailure` should be called.
+				pruneFailure := true
+				if _, ok := pruneFilter.(filter.PreventRemoveFilter); ok {
+					if o.DryRunStrategy.ClientOrServerDryRun() {
+						pruneFailure = false
+					} else {
+						err := po.handleDeletePrevention(pruneObj)
+						if err != nil {
+							if klog.V(4).Enabled() {
+								klog.Errorf("Failed to remove the %q annotation from %s: %v", inventory.OwningInventoryKey, pruneID, err)
+							}
+							taskContext.EventChannel() <- eventFactory.CreateFailedEvent(pruneID, err)
+							taskContext.CapturePruneFailure(pruneID)
+							break
+						} else {
+							pruneFailure = false
+						}
+					}
+				}
 				taskContext.EventChannel() <- eventFactory.CreateSkippedEvent(pruneObj, reason)
-				taskContext.CapturePruneFailure(pruneID)
+				if pruneFailure {
+					taskContext.CapturePruneFailure(pruneID)
+				}
 				break
 			}
 		}
@@ -153,6 +174,26 @@ func (po *PruneOptions) Prune(pruneObjs []*unstructured.Unstructured,
 	return nil
 }
 
+// handleDeletePrevention removes the `config.k8s.io/owning-inventory` annotation from pruneObj.
+func (po *PruneOptions) handleDeletePrevention(pruneObj *unstructured.Unstructured) error {
+	pruneID := object.UnstructuredToObjMetaOrDie(pruneObj)
+	annotations := pruneObj.GetAnnotations()
+	if annotations != nil {
+		if _, ok := annotations[inventory.OwningInventoryKey]; ok {
+			klog.V(4).Infof("remove the %q annotation from the object %s", inventory.OwningInventoryKey, pruneID)
+			delete(annotations, inventory.OwningInventoryKey)
+			pruneObj.SetAnnotations(annotations)
+			namespacedClient, err := po.namespacedClient(pruneID)
+			if err != nil {
+				return err
+			}
+			_, err = namespacedClient.Update(context.TODO(), pruneObj, metav1.UpdateOptions{})
+			return err
+		}
+	}
+	return nil
+}
+
 // GetPruneObjs calculates the set of prune objects, and retrieves them
 // from the cluster. Set of prune objects equals the set of inventory
 // objects minus the set of currently applied objects. Returns an error