From 6d0a69fe4c68a92a129cd70466c383d21aaea142 Mon Sep 17 00:00:00 2001 From: Takumi Sue <23391543+mikutas@users.noreply.github.com> Date: Mon, 29 Apr 2024 08:26:12 +0900 Subject: [PATCH] fix(appset): add debug logs around deleting ownerReferences and add warning docs about policy behavior (#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> --- .../controllers/applicationset_controller.go | 12 ++++--- .../Controlling-Resource-Modification.md | 32 ++++++++++++++++--- 2 files changed, 35 insertions(+), 9 deletions(-) diff --git a/applicationset/controllers/applicationset_controller.go b/applicationset/controllers/applicationset_controller.go index abd23746893e8..10e2ea35573af 100644 --- 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 index d72cee60ad401..ae65fa3462e5b 100644 --- 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