fix(appset): add debug logs around deleting ownerReferences and add w…

…arning docs about policy behavior (argoproj#18006) * chore(appset): add logs for debug Signed-off-by: mikutas <23391543+mikutas@users.noreply.github.com> * fix(appset): remove finalizer regardless with deleteAllowed Signed-off-by: mikutas <23391543+mikutas@users.noreply.github.com> * docs: update about appset policy Signed-off-by: mikutas <23391543+mikutas@users.noreply.github.com> * fix: wrong log message Signed-off-by: mikutas <23391543+mikutas@users.noreply.github.com> * fix: log messages Signed-off-by: mikutas <23391543+mikutas@users.noreply.github.com> * fix: log message Co-authored-by: Ishita Sequeira <46771830+ishitasequeira@users.noreply.github.com> Signed-off-by: Takumi Sue <23391543+mikutas@users.noreply.github.com> * docs: add explanation Signed-off-by: mikutas <23391543+mikutas@users.noreply.github.com> --------- Signed-off-by: mikutas <23391543+mikutas@users.noreply.github.com> Signed-off-by: Takumi Sue <23391543+mikutas@users.noreply.github.com> Co-authored-by: Ishita Sequeira <46771830+ishitasequeira@users.noreply.github.com>
AmadeusITGroup · Jun 16, 2024 · 6d0a69f · 6d0a69f
1 parent 66a38c2
commit 6d0a69f
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 9 deletions.
diff --git a/applicationset/controllers/applicationset_controller.go b/applicationset/controllers/applicationset_controller.go
@@ -111,15 +111,19 @@ func (r *ApplicationSetReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 
 	// Do not attempt to further reconcile the ApplicationSet if it is being deleted.
 	if applicationSetInfo.ObjectMeta.DeletionTimestamp != nil {
+		appsetName := applicationSetInfo.ObjectMeta.Name
+		logCtx.Debugf("DeletionTimestamp is set on %s", appsetName)
 		deleteAllowed := utils.DefaultPolicy(applicationSetInfo.Spec.SyncPolicy, r.Policy, r.EnablePolicyOverride).AllowDelete()
 		if !deleteAllowed {
+			logCtx.Debugf("ApplicationSet policy does not allow to delete")
 			if err := r.removeOwnerReferencesOnDeleteAppSet(ctx, applicationSetInfo); err != nil {
 				return ctrl.Result{}, err
 			}
-			controllerutil.RemoveFinalizer(&applicationSetInfo, argov1alpha1.ResourcesFinalizerName)
-			if err := r.Update(ctx, &applicationSetInfo); err != nil {
-				return ctrl.Result{}, err
-			}
+			logCtx.Debugf("ownerReferences referring %s is deleted from generated applications", appsetName)
+		}
+		controllerutil.RemoveFinalizer(&applicationSetInfo, argov1alpha1.ResourcesFinalizerName)
+		if err := r.Update(ctx, &applicationSetInfo); err != nil {
+			return ctrl.Result{}, err
 		}
 		return ctrl.Result{}, nil
 	}

diff --git a/docs/operator-manual/applicationset/Controlling-Resource-Modification.md b/docs/operator-manual/applicationset/Controlling-Resource-Modification.md
@@ -32,16 +32,19 @@ spec:
 
 ```
 
-- Policy `create-only`: Prevents ApplicationSet controller from modifying or deleting Applications. Prevents Application controller from deleting Applications according to [ownerReferences](https://kubernetes.io/docs/concepts/overview/working-with-objects/owners-dependents/).
-- Policy `create-update`: Prevents ApplicationSet controller from deleting Applications. Update is allowed. Prevents Application controller from deleting Applications according to [ownerReferences](https://kubernetes.io/docs/concepts/overview/working-with-objects/owners-dependents/).
+- Policy `create-only`: Prevents ApplicationSet controller from modifying or deleting Applications. **WARNING**: It doesn't prevent Application controller from deleting Applications according to [ownerReferences](https://kubernetes.io/docs/concepts/overview/working-with-objects/owners-dependents/) when deleting ApplicationSet.
+- Policy `create-update`: Prevents ApplicationSet controller from deleting Applications. Update is allowed. **WARNING**: It doesn't prevent Application controller from deleting Applications according to [ownerReferences](https://kubernetes.io/docs/concepts/overview/working-with-objects/owners-dependents/) when deleting ApplicationSet.
 - Policy `create-delete`: Prevents ApplicationSet controller from modifying Applications. Delete is allowed.
 - Policy `sync`: Update and Delete are allowed.
 
 If the controller parameter `--policy` is set, it takes precedence on the field `applicationsSync`. It is possible to allow per ApplicationSet sync policy by setting variable `ARGOCD_APPLICATIONSET_CONTROLLER_ENABLE_POLICY_OVERRIDE` to argocd-cmd-params-cm `applicationsetcontroller.enable.policy.override` or directly with controller parameter `--enable-policy-override` (default to `false`).
 
-### Controller parameter
+### Policy - `create-only`: Prevent ApplicationSet controller from modifying and deleting Applications
+
+To allow the ApplicationSet controller to *create* `Application` resources, but prevent any further modification, such as *deletion*, or modification of Application fields, add this parameter in the ApplicationSet controller:
+
+**WARNING**: "*deletion*" indicates the case as the result of comparing generated Application between before and after, there are Applications which no longer exist. It doesn't indicate the case Applications are deleted according to ownerReferences to ApplicationSet. See [How to prevent Application controller from deleting Applications when deleting ApplicationSet](#how-to-prevent-application-controller-from-deleting-applications-when-deleting-applicationset)
 
-To allow the ApplicationSet controller to *create* `Application` resources, but prevent any further modification, such as deletion, or modification of Application fields, add this parameter in the ApplicationSet controller:
 ```
 --policy create-only
 ```
@@ -57,9 +60,12 @@ spec:
     applicationsSync: create-only
 ```
 
-## Policy - `create-update`: Prevent ApplicationSet controller from deleting Applications
+### Policy - `create-update`: Prevent ApplicationSet controller from deleting Applications
 
 To allow the ApplicationSet controller to create or modify `Application` resources, but prevent Applications from being deleted, add the following parameter to the ApplicationSet controller `Deployment`:
+
+**WARNING**: "*deletion*" indicates the case as the result of comparing generated Application between before and after, there are Applications which no longer exist. It doesn't indicate the case Applications are deleted according to ownerReferences to ApplicationSet. See [How to prevent Application controller from deleting Applications when deleting ApplicationSet](#how-to-prevent-application-controller-from-deleting-applications-when-deleting-applicationset)
+
 ```
 --policy create-update
 ```
@@ -77,6 +83,22 @@ spec:
     applicationsSync: create-update
 ```
 
+### How to prevent Application controller from deleting Applications when deleting ApplicationSet
+
+By default, `create-only` and `create-update` policy isn't effective against preventing deletion of Applications when deleting ApplicationSet.
+You must set the finalizer to ApplicationSet to prevent deletion in such case, and use background cascading deletion.
+If you use foreground cascading deletion, there's no guarantee to preserve applications.
+
+```yaml
+apiVersion: argoproj.io/v1alpha1
+kind: ApplicationSet
+metadata:
+  finalizers:
+  - resources-finalizer.argocd.argoproj.io
+spec:
+  # (...)
+```
+
 ## Ignore certain changes to Applications
 
 The ApplicationSet spec includes an `ignoreApplicationDifferences` field, which allows you to specify which fields of