feat: PreDelete hooks support (Issue #13975) (#22288)

Signed-off-by: Dan Garfield <dan@codefresh.io> Signed-off-by: Pedro Ribeiro <pedro.ribeiro@cross-join.com> Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: Andrii Korotkov <andrii.korotkov@verkada.com> Co-authored-by: Dan Garfield <dan@codefresh.io> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Andrii Korotkov <137232734+andrii-korotkov-verkada@users.noreply.github.com> Co-authored-by: Ishita Sequeira <46771830+ishitasequeira@users.noreply.github.com> Co-authored-by: pedro-ribeiro-rci <pedro.ribeiro@rci.rogers.ca> Co-authored-by: Pedro Ribeiro <pedro.ribeiro@cross-join.com> Co-authored-by: Blake Pettersson <blake.pettersson@gmail.com> Co-authored-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
2026-02-20 01:28:45 +01:00 · 2025-12-05 20:27:03 +00:00
parent 0c6fa288c2
commit 3bf3d8a212
12 changed files with 670 additions and 68 deletions
--- a/controller/appcontroller.go
+++ b/controller/appcontroller.go
@@ -1062,6 +1062,9 @@ func (ctrl *ApplicationController) processAppOperationQueueItem() (processNext b
 			})
 			message := fmt.Sprintf("Unable to delete application resources: %v", err.Error())
 			ctrl.logAppEvent(context.TODO(), app, argo.EventInfo{Reason: argo.EventReasonStatusRefreshed, Type: corev1.EventTypeWarning}, message)
+		} else {
+			// Clear DeletionError condition if deletion is progressing successfully
+			app.Status.SetConditions([]appv1.ApplicationCondition{}, map[appv1.ApplicationConditionType]bool{appv1.ApplicationConditionDeletionError: true})
 		}
 		ts.AddCheckpoint("finalize_application_deletion_ms")
 	}
@@ -1203,17 +1206,21 @@ func (ctrl *ApplicationController) finalizeApplicationDeletion(app *appv1.Applic
 	if err != nil {
 		return err
 	}
+
+	// Get destination cluster
 	destCluster, err := argo.GetDestinationCluster(context.Background(), app.Spec.Destination, ctrl.db)
 	if err != nil {
 		logCtx.WithError(err).Warn("Unable to get destination cluster")
 		app.UnSetCascadedDeletion()
 		app.UnSetPostDeleteFinalizerAll()
+		app.UnSetPreDeleteFinalizerAll()
 		if err := ctrl.updateFinalizers(app); err != nil {
 			return err
 		}
 		logCtx.Infof("Resource entries removed from undefined cluster")
 		return nil
 	}
+
 	clusterRESTConfig, err := destCluster.RESTConfig()
 	if err != nil {
 		return err
@@ -1225,9 +1232,30 @@ func (ctrl *ApplicationController) finalizeApplicationDeletion(app *appv1.Applic
 		return fmt.Errorf("cannot apply impersonation: %w", err)
 	}

+	// Handle PreDelete hooks - run them before any deletion occurs
+	if app.HasPreDeleteFinalizer() {
+		objsMap, err := ctrl.getPermittedAppLiveObjects(destCluster, app, proj, projectClusters)
+		if err != nil {
+			return fmt.Errorf("error getting permitted app live objects: %w", err)
+		}
+
+		done, err := ctrl.executePreDeleteHooks(app, proj, objsMap, config, logCtx)
+		if err != nil {
+			return fmt.Errorf("error executing pre-delete hooks: %w", err)
+		}
+		if !done {
+			// PreDelete hooks are still running - wait for them to complete
+			return nil
+		}
+		// PreDelete hooks are done - remove the finalizer so we can continue with deletion
+		app.UnSetPreDeleteFinalizer()
+		if err := ctrl.updateFinalizers(app); err != nil {
+			return fmt.Errorf("error updating pre-delete finalizers: %w", err)
+		}
+	}
+
 	if app.CascadedDeletion() {
 		deletionApproved := app.IsDeletionConfirmed(app.DeletionTimestamp.Time)
-
 		logCtx.Infof("Deleting resources")
 		// ApplicationDestination points to a valid cluster, so we may clean up the live objects
 		objs := make([]*unstructured.Unstructured, 0)
@@ -1304,6 +1332,23 @@ func (ctrl *ApplicationController) finalizeApplicationDeletion(app *appv1.Applic
 		return ctrl.updateFinalizers(app)
 	}

+	if app.HasPreDeleteFinalizer("cleanup") {
+		objsMap, err := ctrl.getPermittedAppLiveObjects(destCluster, app, proj, projectClusters)
+		if err != nil {
+			return fmt.Errorf("error getting permitted app live objects for pre-delete cleanup: %w", err)
+		}
+
+		done, err := ctrl.cleanupPreDeleteHooks(objsMap, config, logCtx)
+		if err != nil {
+			return fmt.Errorf("error cleaning up pre-delete hooks: %w", err)
+		}
+		if !done {
+			return nil
+		}
+		app.UnSetPreDeleteFinalizer("cleanup")
+		return ctrl.updateFinalizers(app)
+	}
+
 	if app.HasPostDeleteFinalizer("cleanup") {
 		objsMap, err := ctrl.getPermittedAppLiveObjects(destCluster, app, proj, projectClusters)
 		if err != nil {
@@ -1321,7 +1366,7 @@ func (ctrl *ApplicationController) finalizeApplicationDeletion(app *appv1.Applic
 		return ctrl.updateFinalizers(app)
 	}

-	if !app.CascadedDeletion() && !app.HasPostDeleteFinalizer() {
+	if !app.CascadedDeletion() && !app.HasPostDeleteFinalizer() && !app.HasPreDeleteFinalizer() {
 		if err := ctrl.cache.SetAppManagedResources(app.Name, nil); err != nil {
 			return err
 		}
@@ -1835,10 +1880,25 @@ func (ctrl *ApplicationController) processAppRefreshQueueItem() (processNext boo
 	app.Status.SourceTypes = compareResult.appSourceTypes
 	app.Status.ControllerNamespace = ctrl.namespace
 	ts.AddCheckpoint("app_status_update_ms")
-	patchDuration = ctrl.persistAppStatus(origApp, &app.Status)
-	// This is a partly a duplicate of patch_ms, but more descriptive and allows to have measurement for the next step.
-	ts.AddCheckpoint("persist_app_status_ms")
-	if (compareResult.hasPostDeleteHooks != app.HasPostDeleteFinalizer() || compareResult.hasPostDeleteHooks != app.HasPostDeleteFinalizer("cleanup")) &&
+	// Update finalizers BEFORE persisting status to avoid race condition where app shows "Synced"
+	// but doesn't have finalizers yet, which would allow deletion without running pre-delete hooks
+	if (compareResult.hasPreDeleteHooks != app.HasPreDeleteFinalizer() ||
+		compareResult.hasPreDeleteHooks != app.HasPreDeleteFinalizer("cleanup")) &&
+		app.GetDeletionTimestamp() == nil {
+		if compareResult.hasPreDeleteHooks {
+			app.SetPreDeleteFinalizer()
+			app.SetPreDeleteFinalizer("cleanup")
+		} else {
+			app.UnSetPreDeleteFinalizer()
+			app.UnSetPreDeleteFinalizer("cleanup")
+		}
+
+		if err := ctrl.updateFinalizers(app); err != nil {
+			logCtx.Errorf("Failed to update pre-delete finalizers: %v", err)
+		}
+	}
+	if (compareResult.hasPostDeleteHooks != app.HasPostDeleteFinalizer() ||
+		compareResult.hasPostDeleteHooks != app.HasPostDeleteFinalizer("cleanup")) &&
 		app.GetDeletionTimestamp() == nil {
 		if compareResult.hasPostDeleteHooks {
 			app.SetPostDeleteFinalizer()
@@ -1849,10 +1909,13 @@ func (ctrl *ApplicationController) processAppRefreshQueueItem() (processNext boo
 		}

 		if err := ctrl.updateFinalizers(app); err != nil {
-			logCtx.WithError(err).Error("Failed to update finalizers")
+			logCtx.WithError(err).Error("Failed to update post-delete finalizers")
 		}
 	}
 	ts.AddCheckpoint("process_finalizers_ms")
+	patchDuration = ctrl.persistAppStatus(origApp, &app.Status)
+	// This is a partly a duplicate of patch_ms, but more descriptive and allows to have measurement for the next step.
+	ts.AddCheckpoint("persist_app_status_ms")
 	return processNext
 }

--- a/controller/appcontroller_test.go
+++ b/controller/appcontroller_test.go
@@ -407,6 +407,37 @@ metadata:
 data:
 `

+var fakePreDeleteHook = `
+{
+  "apiVersion": "v1",
+  "kind": "Pod",
+  "metadata": {
+    "name": "pre-delete-hook",
+    "namespace": "default",
+    "labels": {
+      "app.kubernetes.io/instance": "my-app"
+    },
+    "annotations": {
+      "argocd.argoproj.io/hook": "PreDelete"
+    }
+  },
+  "spec": {
+    "containers": [
+      {
+        "name": "pre-delete-hook",
+        "image": "busybox",
+        "restartPolicy": "Never",
+        "command": [
+          "/bin/sh",
+          "-c",
+          "sleep 5 && echo hello from the pre-delete-hook pod"
+        ]
+      }
+    ]
+  }
+}
+`
+
 var fakePostDeleteHook = `
 {
  "apiVersion": "batch/v1",
@@ -557,6 +588,15 @@ func newFakeCM() map[string]any {
 	return cm
 }

+func newFakePreDeleteHook() map[string]any {
+	var cm map[string]any
+	err := yaml.Unmarshal([]byte(fakePreDeleteHook), &cm)
+	if err != nil {
+		panic(err)
+	}
+	return cm
+}
+
 func newFakePostDeleteHook() map[string]any {
 	var hook map[string]any
 	err := yaml.Unmarshal([]byte(fakePostDeleteHook), &hook)
@@ -1114,6 +1154,40 @@ func TestFinalizeAppDeletion(t *testing.T) {
 		testShouldDelete(app3)
 	})

+	t.Run("PreDelete_HookIsCreated", func(t *testing.T) {
+		app := newFakeApp()
+		app.SetPreDeleteFinalizer()
+		app.Spec.Destination.Namespace = test.FakeArgoCDNamespace
+		ctrl := newFakeController(context.Background(), &fakeData{
+			manifestResponses: []*apiclient.ManifestResponse{{
+				Manifests: []string{fakePreDeleteHook},
+			}},
+			apps:            []runtime.Object{app, &defaultProj},
+			managedLiveObjs: map[kube.ResourceKey]*unstructured.Unstructured{},
+		}, nil)
+
+		patched := false
+		fakeAppCs := ctrl.applicationClientset.(*appclientset.Clientset)
+		defaultReactor := fakeAppCs.ReactionChain[0]
+		fakeAppCs.ReactionChain = nil
+		fakeAppCs.AddReactor("get", "*", func(action kubetesting.Action) (handled bool, ret runtime.Object, err error) {
+			return defaultReactor.React(action)
+		})
+		fakeAppCs.AddReactor("patch", "*", func(_ kubetesting.Action) (handled bool, ret runtime.Object, err error) {
+			patched = true
+			return true, &v1alpha1.Application{}, nil
+		})
+		err := ctrl.finalizeApplicationDeletion(app, func(_ string) ([]*v1alpha1.Cluster, error) {
+			return []*v1alpha1.Cluster{}, nil
+		})
+		require.NoError(t, err)
+		// finalizer is not deleted
+		assert.False(t, patched)
+		// pre-delete hook is created
+		require.Len(t, ctrl.kubectl.(*MockKubectl).CreatedResources, 1)
+		require.Equal(t, "pre-delete-hook", ctrl.kubectl.(*MockKubectl).CreatedResources[0].GetName())
+	})
+
 	t.Run("PostDelete_HookIsCreated", func(t *testing.T) {
 		app := newFakeApp()
 		app.SetPostDeleteFinalizer()
@@ -1148,6 +1222,41 @@ func TestFinalizeAppDeletion(t *testing.T) {
 		require.Equal(t, "post-delete-hook", ctrl.kubectl.(*MockKubectl).CreatedResources[0].GetName())
 	})

+	t.Run("PreDelete_HookIsExecuted", func(t *testing.T) {
+		app := newFakeApp()
+		app.SetPreDeleteFinalizer()
+		app.Spec.Destination.Namespace = test.FakeArgoCDNamespace
+		liveHook := &unstructured.Unstructured{Object: newFakePreDeleteHook()}
+		require.NoError(t, unstructured.SetNestedField(liveHook.Object, "Succeeded", "status", "phase"))
+		ctrl := newFakeController(context.Background(), &fakeData{
+			manifestResponses: []*apiclient.ManifestResponse{{
+				Manifests: []string{fakePreDeleteHook},
+			}},
+			apps: []runtime.Object{app, &defaultProj},
+			managedLiveObjs: map[kube.ResourceKey]*unstructured.Unstructured{
+				kube.GetResourceKey(liveHook): liveHook,
+			},
+		}, nil)
+
+		patched := false
+		fakeAppCs := ctrl.applicationClientset.(*appclientset.Clientset)
+		defaultReactor := fakeAppCs.ReactionChain[0]
+		fakeAppCs.ReactionChain = nil
+		fakeAppCs.AddReactor("get", "*", func(action kubetesting.Action) (handled bool, ret runtime.Object, err error) {
+			return defaultReactor.React(action)
+		})
+		fakeAppCs.AddReactor("patch", "*", func(_ kubetesting.Action) (handled bool, ret runtime.Object, err error) {
+			patched = true
+			return true, &v1alpha1.Application{}, nil
+		})
+		err := ctrl.finalizeApplicationDeletion(app, func(_ string) ([]*v1alpha1.Cluster, error) {
+			return []*v1alpha1.Cluster{}, nil
+		})
+		require.NoError(t, err)
+		// finalizer is removed
+		assert.True(t, patched)
+	})
+
 	t.Run("PostDelete_HookIsExecuted", func(t *testing.T) {
 		app := newFakeApp()
 		app.SetPostDeleteFinalizer()
--- a/controller/hook.go
+++ b/controller/hook.go
@@ -2,6 +2,8 @@ package controller

 import (
 	"context"
+	"fmt"
+	"strings"

 	"github.com/argoproj/gitops-engine/pkg/health"
 	"github.com/argoproj/gitops-engine/pkg/sync/common"
@@ -14,26 +16,33 @@ import (

 	"github.com/argoproj/argo-cd/v3/util/lua"

-	"github.com/argoproj/argo-cd/v3/pkg/apis/application/v1alpha1"
+	appv1 "github.com/argoproj/argo-cd/v3/pkg/apis/application/v1alpha1"
 )

-var (
-	postDeleteHook  = "PostDelete"
-	postDeleteHooks = map[string]string{
-		"argocd.argoproj.io/hook": postDeleteHook,
+type HookType string
+
+const (
+	PreDeleteHookType  HookType = "PreDelete"
+	PostDeleteHookType HookType = "PostDelete"
+)
+
+var hookTypeAnnotations = map[HookType]map[string]string{
+	PreDeleteHookType: {
+		"argocd.argoproj.io/hook": string(PreDeleteHookType),
+		"helm.sh/hook":            "pre-delete",
+	},
+	PostDeleteHookType: {
+		"argocd.argoproj.io/hook": string(PostDeleteHookType),
 		"helm.sh/hook":            "post-delete",
-	}
-)
-
-func isHook(obj *unstructured.Unstructured) bool {
-	return hook.IsHook(obj) || isPostDeleteHook(obj)
+	},
 }

-func isPostDeleteHook(obj *unstructured.Unstructured) bool {
+func isHookOfType(obj *unstructured.Unstructured, hookType HookType) bool {
 	if obj == nil || obj.GetAnnotations() == nil {
 		return false
 	}
-	for k, v := range postDeleteHooks {
+
+	for k, v := range hookTypeAnnotations[hookType] {
 		if val, ok := obj.GetAnnotations()[k]; ok && val == v {
 			return true
 		}
@@ -41,11 +50,34 @@ func isPostDeleteHook(obj *unstructured.Unstructured) bool {
 	return false
 }

-func (ctrl *ApplicationController) executePostDeleteHooks(app *v1alpha1.Application, proj *v1alpha1.AppProject, liveObjs map[kube.ResourceKey]*unstructured.Unstructured, config *rest.Config, logCtx *log.Entry) (bool, error) {
+func isHook(obj *unstructured.Unstructured) bool {
+	if hook.IsHook(obj) {
+		return true
+	}
+
+	for hookType := range hookTypeAnnotations {
+		if isHookOfType(obj, hookType) {
+			return true
+		}
+	}
+	return false
+}
+
+func isPreDeleteHook(obj *unstructured.Unstructured) bool {
+	return isHookOfType(obj, PreDeleteHookType)
+}
+
+func isPostDeleteHook(obj *unstructured.Unstructured) bool {
+	return isHookOfType(obj, PostDeleteHookType)
+}
+
+// executeHooks is a generic function to execute hooks of a specified type
+func (ctrl *ApplicationController) executeHooks(hookType HookType, app *appv1.Application, proj *appv1.AppProject, liveObjs map[kube.ResourceKey]*unstructured.Unstructured, config *rest.Config, logCtx *log.Entry) (bool, error) {
 	appLabelKey, err := ctrl.settingsMgr.GetAppInstanceLabelKey()
 	if err != nil {
 		return false, err
 	}
+
 	var revisions []string
 	for _, src := range app.Spec.GetSources() {
 		revisions = append(revisions, src.TargetRevision)
@@ -55,44 +87,62 @@ func (ctrl *ApplicationController) executePostDeleteHooks(app *v1alpha1.Applicat
 	if err != nil {
 		return false, err
 	}
+
+	// Find existing hooks of the specified type
 	runningHooks := map[kube.ResourceKey]*unstructured.Unstructured{}
 	for key, obj := range liveObjs {
-		if isPostDeleteHook(obj) {
+		if isHookOfType(obj, hookType) {
 			runningHooks[key] = obj
 		}
 	}

+	// Find expected hooks that need to be created
 	expectedHook := map[kube.ResourceKey]*unstructured.Unstructured{}
 	for _, obj := range targets {
 		if obj.GetNamespace() == "" {
 			obj.SetNamespace(app.Spec.Destination.Namespace)
 		}
-		if !isPostDeleteHook(obj) {
+		if !isHookOfType(obj, hookType) {
 			continue
 		}
 		if runningHook := runningHooks[kube.GetResourceKey(obj)]; runningHook == nil {
 			expectedHook[kube.GetResourceKey(obj)] = obj
 		}
 	}
+
+	// Create hooks that don't exist yet
 	createdCnt := 0
 	for _, obj := range expectedHook {
+		// Add app instance label so the hook can be tracked and cleaned up
+		labels := obj.GetLabels()
+		if labels == nil {
+			labels = make(map[string]string)
+		}
+		labels[appLabelKey] = app.InstanceName(ctrl.namespace)
+		obj.SetLabels(labels)
+
 		_, err = ctrl.kubectl.CreateResource(context.Background(), config, obj.GroupVersionKind(), obj.GetName(), obj.GetNamespace(), obj, metav1.CreateOptions{})
 		if err != nil {
 			return false, err
 		}
 		createdCnt++
 	}
+
 	if createdCnt > 0 {
-		logCtx.Infof("Created %d post-delete hooks", createdCnt)
+		logCtx.Infof("Created %d %s hooks", createdCnt, hookType)
 		return false, nil
 	}
+
+	// Check health of running hooks
 	resourceOverrides, err := ctrl.settingsMgr.GetResourceOverrides()
 	if err != nil {
 		return false, err
 	}
 	healthOverrides := lua.ResourceHealthOverrides(resourceOverrides)

-	progressingHooksCnt := 0
+	progressingHooksCount := 0
+	var failedHooks []string
+	var failedHookObjects []*unstructured.Unstructured
 	for _, obj := range runningHooks {
 		hookHealth, err := health.GetResourceHealth(obj, healthOverrides)
 		if err != nil {
@@ -110,19 +160,37 @@ func (ctrl *ApplicationController) executePostDeleteHooks(app *v1alpha1.Applicat
 				Status: health.HealthStatusHealthy,
 			}
 		}
-		if hookHealth.Status == health.HealthStatusProgressing {
-			progressingHooksCnt++
+		switch hookHealth.Status {
+		case health.HealthStatusProgressing:
+			progressingHooksCount++
+		case health.HealthStatusDegraded:
+			failedHooks = append(failedHooks, fmt.Sprintf("%s/%s", obj.GetNamespace(), obj.GetName()))
+			failedHookObjects = append(failedHookObjects, obj)
 		}
 	}
-	if progressingHooksCnt > 0 {
-		logCtx.Infof("Waiting for %d post-delete hooks to complete", progressingHooksCnt)
+
+	if len(failedHooks) > 0 {
+		// Delete failed hooks to allow retry with potentially fixed hook definitions
+		logCtx.Infof("Deleting %d failed %s hook(s) to allow retry", len(failedHookObjects), hookType)
+		for _, obj := range failedHookObjects {
+			err = ctrl.kubectl.DeleteResource(context.Background(), config, obj.GroupVersionKind(), obj.GetName(), obj.GetNamespace(), metav1.DeleteOptions{})
+			if err != nil {
+				logCtx.WithError(err).Warnf("Failed to delete failed hook %s/%s", obj.GetNamespace(), obj.GetName())
+			}
+		}
+		return false, fmt.Errorf("%s hook(s) failed: %s", hookType, strings.Join(failedHooks, ", "))
+	}
+
+	if progressingHooksCount > 0 {
+		logCtx.Infof("Waiting for %d %s hooks to complete", progressingHooksCount, hookType)
 		return false, nil
 	}

 	return true, nil
 }

-func (ctrl *ApplicationController) cleanupPostDeleteHooks(liveObjs map[kube.ResourceKey]*unstructured.Unstructured, config *rest.Config, logCtx *log.Entry) (bool, error) {
+// cleanupHooks is a generic function to clean up hooks of a specified type
+func (ctrl *ApplicationController) cleanupHooks(hookType HookType, liveObjs map[kube.ResourceKey]*unstructured.Unstructured, config *rest.Config, logCtx *log.Entry) (bool, error) {
 	resourceOverrides, err := ctrl.settingsMgr.GetResourceOverrides()
 	if err != nil {
 		return false, err
@@ -132,8 +200,10 @@ func (ctrl *ApplicationController) cleanupPostDeleteHooks(liveObjs map[kube.Reso
 	pendingDeletionCount := 0
 	aggregatedHealth := health.HealthStatusHealthy
 	var hooks []*unstructured.Unstructured
+
+	// Collect hooks and determine overall health
 	for _, obj := range liveObjs {
-		if !isPostDeleteHook(obj) {
+		if !isHookOfType(obj, hookType) {
 			continue
 		}
 		hookHealth, err := health.GetResourceHealth(obj, healthOverrides)
@@ -151,25 +221,60 @@ func (ctrl *ApplicationController) cleanupPostDeleteHooks(liveObjs map[kube.Reso
 		hooks = append(hooks, obj)
 	}

+	// Process hooks for deletion
 	for _, obj := range hooks {
-		for _, policy := range hook.DeletePolicies(obj) {
-			if (policy != common.HookDeletePolicyHookFailed || aggregatedHealth != health.HealthStatusDegraded) && (policy != common.HookDeletePolicyHookSucceeded || aggregatedHealth != health.HealthStatusHealthy) {
-				continue
+		deletePolicies := hook.DeletePolicies(obj)
+		shouldDelete := false
+
+		if len(deletePolicies) == 0 {
+			// If no delete policy is specified, always delete hooks during cleanup phase
+			shouldDelete = true
+		} else {
+			// Check if any delete policy matches the current hook state
+			for _, policy := range deletePolicies {
+				if (policy == common.HookDeletePolicyHookFailed && aggregatedHealth == health.HealthStatusDegraded) ||
+					(policy == common.HookDeletePolicyHookSucceeded && aggregatedHealth == health.HealthStatusHealthy) {
+					shouldDelete = true
+					break
+				}
 			}
+		}
+
+		if shouldDelete {
 			pendingDeletionCount++
 			if obj.GetDeletionTimestamp() != nil {
 				continue
 			}
-			logCtx.Infof("Deleting post-delete hook %s/%s", obj.GetNamespace(), obj.GetName())
+			logCtx.Infof("Deleting %s hook %s/%s", hookType, obj.GetNamespace(), obj.GetName())
 			err = ctrl.kubectl.DeleteResource(context.Background(), config, obj.GroupVersionKind(), obj.GetName(), obj.GetNamespace(), metav1.DeleteOptions{})
 			if err != nil {
 				return false, err
 			}
 		}
 	}
+
 	if pendingDeletionCount > 0 {
-		logCtx.Infof("Waiting for %d post-delete hooks to be deleted", pendingDeletionCount)
+		logCtx.Infof("Waiting for %d %s hooks to be deleted", pendingDeletionCount, hookType)
 		return false, nil
 	}
+
 	return true, nil
 }
+
+// Execute and cleanup hooks for pre-delete and post-delete operations
+
+func (ctrl *ApplicationController) executePreDeleteHooks(app *appv1.Application, proj *appv1.AppProject, liveObjs map[kube.ResourceKey]*unstructured.Unstructured, config *rest.Config, logCtx *log.Entry) (bool, error) {
+	return ctrl.executeHooks(PreDeleteHookType, app, proj, liveObjs, config, logCtx)
+}
+
+func (ctrl *ApplicationController) cleanupPreDeleteHooks(liveObjs map[kube.ResourceKey]*unstructured.Unstructured, config *rest.Config, logCtx *log.Entry) (bool, error) {
+	return ctrl.cleanupHooks(PreDeleteHookType, liveObjs, config, logCtx)
+}
+
+func (ctrl *ApplicationController) executePostDeleteHooks(app *appv1.Application, proj *appv1.AppProject, liveObjs map[kube.ResourceKey]*unstructured.Unstructured, config *rest.Config, logCtx *log.Entry) (bool, error) {
+	return ctrl.executeHooks(PostDeleteHookType, app, proj, liveObjs, config, logCtx)
+}
+
+func (ctrl *ApplicationController) cleanupPostDeleteHooks(liveObjs map[kube.ResourceKey]*unstructured.Unstructured, config *rest.Config, logCtx *log.Entry) (bool, error) {
+	return ctrl.cleanupHooks(PostDeleteHookType, liveObjs, config, logCtx)
+}
--- a/controller/hook_test.go
+++ b/controller/hook_test.go
@@ -0,0 +1,173 @@
+package controller
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+)
+
+func TestIsHookOfType(t *testing.T) {
+	tests := []struct {
+		name     string
+		hookType HookType
+		annot    map[string]string
+		expected bool
+	}{
+		{
+			name:     "ArgoCD PreDelete hook",
+			hookType: PreDeleteHookType,
+			annot:    map[string]string{"argocd.argoproj.io/hook": "PreDelete"},
+			expected: true,
+		},
+		{
+			name:     "Helm PreDelete hook",
+			hookType: PreDeleteHookType,
+			annot:    map[string]string{"helm.sh/hook": "pre-delete"},
+			expected: true,
+		},
+		{
+			name:     "ArgoCD PostDelete hook",
+			hookType: PostDeleteHookType,
+			annot:    map[string]string{"argocd.argoproj.io/hook": "PostDelete"},
+			expected: true,
+		},
+		{
+			name:     "Helm PostDelete hook",
+			hookType: PostDeleteHookType,
+			annot:    map[string]string{"helm.sh/hook": "post-delete"},
+			expected: true,
+		},
+		{
+			name:     "Not a hook",
+			hookType: PreDeleteHookType,
+			annot:    map[string]string{"some-other": "annotation"},
+			expected: false,
+		},
+		{
+			name:     "Wrong hook type",
+			hookType: PreDeleteHookType,
+			annot:    map[string]string{"argocd.argoproj.io/hook": "PostDelete"},
+			expected: false,
+		},
+		{
+			name:     "Nil annotations",
+			hookType: PreDeleteHookType,
+			annot:    nil,
+			expected: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			obj := &unstructured.Unstructured{}
+			obj.SetAnnotations(tt.annot)
+			result := isHookOfType(obj, tt.hookType)
+			assert.Equal(t, tt.expected, result)
+		})
+	}
+}
+
+func TestIsHook(t *testing.T) {
+	tests := []struct {
+		name     string
+		annot    map[string]string
+		expected bool
+	}{
+		{
+			name:     "ArgoCD PreDelete hook",
+			annot:    map[string]string{"argocd.argoproj.io/hook": "PreDelete"},
+			expected: true,
+		},
+		{
+			name:     "ArgoCD PostDelete hook",
+			annot:    map[string]string{"argocd.argoproj.io/hook": "PostDelete"},
+			expected: true,
+		},
+		{
+			name:     "ArgoCD PreSync hook",
+			annot:    map[string]string{"argocd.argoproj.io/hook": "PreSync"},
+			expected: true,
+		},
+		{
+			name:     "Not a hook",
+			annot:    map[string]string{"some-other": "annotation"},
+			expected: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			obj := &unstructured.Unstructured{}
+			obj.SetAnnotations(tt.annot)
+			result := isHook(obj)
+			assert.Equal(t, tt.expected, result)
+		})
+	}
+}
+
+func TestIsPreDeleteHook(t *testing.T) {
+	tests := []struct {
+		name     string
+		annot    map[string]string
+		expected bool
+	}{
+		{
+			name:     "ArgoCD PreDelete hook",
+			annot:    map[string]string{"argocd.argoproj.io/hook": "PreDelete"},
+			expected: true,
+		},
+		{
+			name:     "Helm PreDelete hook",
+			annot:    map[string]string{"helm.sh/hook": "pre-delete"},
+			expected: true,
+		},
+		{
+			name:     "ArgoCD PostDelete hook",
+			annot:    map[string]string{"argocd.argoproj.io/hook": "PostDelete"},
+			expected: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			obj := &unstructured.Unstructured{}
+			obj.SetAnnotations(tt.annot)
+			result := isPreDeleteHook(obj)
+			assert.Equal(t, tt.expected, result)
+		})
+	}
+}
+
+func TestIsPostDeleteHook(t *testing.T) {
+	tests := []struct {
+		name     string
+		annot    map[string]string
+		expected bool
+	}{
+		{
+			name:     "ArgoCD PostDelete hook",
+			annot:    map[string]string{"argocd.argoproj.io/hook": "PostDelete"},
+			expected: true,
+		},
+		{
+			name:     "Helm PostDelete hook",
+			annot:    map[string]string{"helm.sh/hook": "post-delete"},
+			expected: true,
+		},
+		{
+			name:     "ArgoCD PreDelete hook",
+			annot:    map[string]string{"argocd.argoproj.io/hook": "PreDelete"},
+			expected: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			obj := &unstructured.Unstructured{}
+			obj.SetAnnotations(tt.annot)
+			result := isPostDeleteHook(obj)
+			assert.Equal(t, tt.expected, result)
+		})
+	}
+}
--- a/controller/state.go
+++ b/controller/state.go
@@ -95,6 +95,7 @@ type comparisonResult struct {
 	timings            map[string]time.Duration
 	diffResultList     *diff.DiffResultList
 	hasPostDeleteHooks bool
+	hasPreDeleteHooks  bool
 	// revisionsMayHaveChanges indicates if there are any possibilities that the revisions contain changes
 	revisionsMayHaveChanges bool
 }
@@ -765,14 +766,26 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *v1
 			}
 		}
 	}
+	hasPreDeleteHooks := false
 	hasPostDeleteHooks := false
+	// Filter out PreDelete and PostDelete hooks from targetObjs since they should not be synced
+	// as regular resources. They are only executed during deletion.
+	var targetObjsForSync []*unstructured.Unstructured
 	for _, obj := range targetObjs {
+		if isPreDeleteHook(obj) {
+			hasPreDeleteHooks = true
+			// Skip PreDelete hooks - they are not synced, only executed during deletion
+			continue
+		}
 		if isPostDeleteHook(obj) {
 			hasPostDeleteHooks = true
+			// Skip PostDelete hooks - they are not synced, only executed after deletion
+			continue
 		}
+		targetObjsForSync = append(targetObjsForSync, obj)
 	}

-	reconciliation := sync.Reconcile(targetObjs, liveObjByKey, app.Spec.Destination.Namespace, infoProvider)
+	reconciliation := sync.Reconcile(targetObjsForSync, liveObjByKey, app.Spec.Destination.Namespace, infoProvider)
 	ts.AddCheckpoint("live_ms")

 	compareOptions, err := m.settingsMgr.GetResourceCompareOptions()
@@ -989,6 +1002,7 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *v1
 		diffConfig:              diffConfig,
 		diffResultList:          diffResults,
 		hasPostDeleteHooks:      hasPostDeleteHooks,
+		hasPreDeleteHooks:       hasPreDeleteHooks,
 		revisionsMayHaveChanges: revisionsMayHaveChanges,
 	}

--- a/controller/sync.go
+++ b/controller/sync.go
@@ -330,6 +330,7 @@ func (m *appStateManager) SyncAppState(app *v1alpha1.Application, project *v1alp
 		sync.WithResourcesFilter(func(key kube.ResourceKey, target *unstructured.Unstructured, live *unstructured.Unstructured) bool {
 			return (len(syncOp.Resources) == 0 ||
 				isPostDeleteHook(target) ||
+				isPreDeleteHook(target) ||
 				argo.ContainsSyncResource(key.Name, key.Namespace, schema.GroupVersionKind{Kind: key.Kind, Group: key.Group}, syncOp.Resources)) &&
 				m.isSelfReferencedObj(live, target, app.GetName(), v1alpha1.TrackingMethod(trackingMethod), installationID)
 		}),
--- a/docs/user-guide/helm.md
+++ b/docs/user-guide/helm.md
@@ -273,7 +273,7 @@ Argo CD supports many (most?) Helm hooks by mapping the Helm annotations onto Ar
 | Helm Annotation                 | Notes                                                                                         |
 | ------------------------------- |-----------------------------------------------------------------------------------------------|
 | `helm.sh/hook: crd-install`     | Supported as equivalent to normal Argo CD CRD handling.                                |
-| `helm.sh/hook: pre-delete`      | Not supported. In Helm stable there are 3 cases used to clean up CRDs and 3 to clean-up jobs. |
+| `helm.sh/hook: pre-delete`      | Supported as equivalent to `argocd.argoproj.io/hook: PreDelete`                               |
 | `helm.sh/hook: pre-rollback`    | Not supported. Never used in Helm stable.                                                     |
 | `helm.sh/hook: pre-install`     | Supported as equivalent to `argocd.argoproj.io/hook: PreSync`.                                |
 | `helm.sh/hook: pre-upgrade`     | Supported as equivalent to `argocd.argoproj.io/hook: PreSync`.                                |
--- a/docs/user-guide/sync-waves.md
+++ b/docs/user-guide/sync-waves.md
@@ -4,14 +4,15 @@ Sync phases and hooks define when resources are applied such as before or after

 Argo CD has the following hook types:

-| Hook | Description |
-|------|-------------|
-| `PreSync` | Executes prior to the application of the manifests. |
-| `Sync`  | Executes after all `PreSync` hooks completed and were successful, at the same time as the application of the manifests. |
-| `Skip` | Indicates to Argo CD to skip the application of the manifest. |
-| `PostSync` | Executes after all `Sync` hooks completed and were successful, a successful application, and all resources in a `Healthy` state. |
-| `SyncFail` | Executes when the sync operation fails. |
-| `PostDelete` | Executes after all Application resources are deleted. _Available starting in v2.10._ |
+| Hook         | Description                                                                                                                                                                |
+|--------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
+| `PreSync`    | Executes prior to the application of the manifests.                                                                                                                        |
+| `Sync`       | Executes after all `PreSync` hooks completed and were successful, at the same time as the application of the manifests.                                                    |
+| `Skip`       | Indicates to Argo CD to skip the application of the manifest.                                                                                                              |
+| `PostSync`   | Executes after all `Sync` hooks completed and were successful, a successful application, and all resources in a `Healthy` state.                                           |
+| `SyncFail`   | Executes when the sync operation fails.                                                                                                                                    |
+| `PreDelete`  | Executes before Application resources are deleted. Only runs when the entire Application is being deleted, not during normal sync operations (even with pruning enabled. ) |
+| `PostDelete` | Executes after all Application resources are deleted. _Available starting in v2.10._                                                                                       |

 Adding the argocd.argoproj.io/hook annotation to a resource will assign it to a specific phase. During a Sync operation, Argo CD will apply the resource during the appropriate phase of the deployment. Hooks can be any type of Kubernetes resource kind, but tend to be Pod, Job or Argo Workflows. Multiple hooks can be specified as a comma separated list.

@@ -41,14 +42,58 @@ Argo CD offers several methods to clean up hooks and decide how much history wil
 In the most basic case you can use the argocd.argoproj.io/hook-delete-policy to decide when a hook will be deleted.
 This can take the following values:

-| Policy | Description |
-|--------|-------------|
-| `HookSucceeded` | The hook resource is deleted after the hook succeeded (e.g. Job/Workflow completed successfully). |
-| `HookFailed` | The hook resource is deleted after the hook failed. |
+| Policy               | Description                                                                                                                     |
+|----------------------|---------------------------------------------------------------------------------------------------------------------------------|
+| `HookSucceeded`      | The hook resource is deleted after the hook succeeded (e.g. Job/Workflow completed successfully).                               |
+| `HookFailed`         | The hook resource is deleted after the hook failed.                                                                             |
 | `BeforeHookCreation` | Any existing hook resource is deleted before the new one is created (since v1.3). It is meant to be used with `/metadata/name`. |

 Note that if no deletion policy is specified, Argo CD will automatically assume `BeforeHookCreation` rules.

+## PreDelete and PostDelete Hooks
+
+### PreDelete Hooks
+
+PreDelete hooks execute before an Application and its resources are deleted. They run only during Application deletion (e.g., `kubectl delete application` or `argocd app delete`), not during normal sync operations, even when pruning is enabled.
+
+**Behavior:**
+
+1. When an Application is deleted, Argo CD checks for PreDelete hooks defined in the Application's manifests
+2. If PreDelete hooks exist, they are created and executed before any Application resources are deleted
+3. Argo CD waits for all PreDelete hooks to reach a Healthy state before proceeding with deletion
+4. Once all PreDelete hooks complete successfully, they are cleaned up and the Application resources are deleted
+
+**Failure Handling:**
+
+If a PreDelete hook fails (e.g., a Job fails or a Pod crashes), the Application deletion is blocked:
+
+- The Application will remain in a deleting state with a `DeletionError` condition
+- Application resources will not be deleted until the hook succeeds
+- The user can fix the failing hook by updating its manifest in the git repository
+- After fixing the hook, Argo CD will automatically retry the deletion on the next reconciliation
+- Alternatively, the user can manually delete the failing hook resource to allow deletion to proceed
+
+
+### PostDelete Hooks
+
+PostDelete hooks execute after all Application resources have been deleted. They are useful for cleanup operations, notifications, or removing external resources.
+
+**Behavior:**
+
+1. Application resources are deleted first
+2. Once all Application resources are fully removed, PostDelete hooks are created and executed
+3. Argo CD waits for all PostDelete hooks to reach a Healthy state
+4. Once all PostDelete hooks complete successfully, they are cleaned up and the Application is fully removed
+
+**Failure Handling:**
+
+If a PostDelete hook fails:
+
+- The Application is already deleted from the cluster, but the Application CR remains with a `DeletionError` condition
+- The user can fix the failing hook by updating its manifest in the git repository
+- After fixing the hook, Argo CD will automatically retry on the next reconciliation
+- Alternatively, the user can manually delete the failing hook resource to complete the Application deletion
+
 ## How sync waves work?

 Argo CD also offers an alternative method of changing the sync order of resources. These are sync waves. They are defined by the argocd.argoproj.io/sync-wave annotation. The value is an integer that defines the ordering (ArgoCD will start deploying from the lowest number and finish with the highest number).
@@ -176,20 +221,20 @@ spec:

 Upgrading ingress-nginx controller (managed by helm) with ArgoCD 2.x sometimes fails to work resulting in:

-.|.
-|-
-OPERATION|Sync
-PHASE|Running
-MESSAGE|waiting for completion of hook batch/Job/ingress-nginx-admission-create
+| .         | .                                                                       |
+|-----------|-------------------------------------------------------------------------|
+| OPERATION | Sync                                                                    |
+| PHASE     | Running                                                                 |
+| MESSAGE   | waiting for completion of hook batch/Job/ingress-nginx-admission-create |

-.|.
-|-
-KIND     |batch/v1/Job
-NAMESPACE|ingress-nginx
-NAME     |ingress-nginx-admission-create
-STATUS   |Running
-HOOK     |PreSync
-MESSAGE  |Pending deletion
+| .         | .                              |
+|-----------|--------------------------------|
+| KIND      | batch/v1/Job                   |
+| NAMESPACE | ingress-nginx                  |
+| NAME      | ingress-nginx-admission-create |
+| STATUS    | Running                        |
+| HOOK      | PreSync                        |
+| MESSAGE   | Pending deletion               |

 To work around this, a helm user can add:

--- a/pkg/apis/application/v1alpha1/application_defaults.go
+++ b/pkg/apis/application/v1alpha1/application_defaults.go
@@ -9,6 +9,9 @@ const (
 	// ResourcesFinalizerName is the finalizer value which we inject to finalize deletion of an application
 	ResourcesFinalizerName string = "resources-finalizer.argocd.argoproj.io"

+	// PreDeleteFinalizerName is the finalizer that controls pre-delete hooks execution
+	PreDeleteFinalizerName string = "pre-delete-finalizer.argocd.argoproj.io"
+
 	// PostDeleteFinalizerName is the finalizer that controls post-delete hooks execution
 	PostDeleteFinalizerName string = "post-delete-finalizer.argocd.argoproj.io"

--- a/pkg/apis/application/v1alpha1/types.go
+++ b/pkg/apis/application/v1alpha1/types.go
@@ -3350,6 +3350,26 @@ func (app *Application) IsHydrateRequested() bool {
 	return false
 }

+func (app *Application) HasPreDeleteFinalizer(stage ...string) bool {
+	return getFinalizerIndex(app.ObjectMeta, strings.Join(append([]string{PreDeleteFinalizerName}, stage...), "/")) > -1
+}
+
+func (app *Application) SetPreDeleteFinalizer(stage ...string) {
+	setFinalizer(&app.ObjectMeta, strings.Join(append([]string{PreDeleteFinalizerName}, stage...), "/"), true)
+}
+
+func (app *Application) UnSetPreDeleteFinalizer(stage ...string) {
+	setFinalizer(&app.ObjectMeta, strings.Join(append([]string{PreDeleteFinalizerName}, stage...), "/"), false)
+}
+
+func (app *Application) UnSetPreDeleteFinalizerAll() {
+	for _, finalizer := range app.Finalizers {
+		if strings.HasPrefix(finalizer, PreDeleteFinalizerName) {
+			setFinalizer(&app.ObjectMeta, finalizer, false)
+		}
+	}
+}
+
 func (app *Application) HasPostDeleteFinalizer(stage ...string) bool {
 	return getFinalizerIndex(app.ObjectMeta, strings.Join(append([]string{PostDeleteFinalizerName}, stage...), "/")) > -1
 }
--- a/test/e2e/hook_test.go
+++ b/test/e2e/hook_test.go
@@ -52,6 +52,53 @@ func testHookSuccessful(t *testing.T, hookType HookType) {
 		Expect(ResourceResultIs(ResourceResult{Version: "v1", Kind: "Pod", Namespace: DeploymentNamespace(), Images: []string{"quay.io/argoprojlabs/argocd-e2e-container:0.1"}, Name: "hook", Message: "pod/hook created", HookType: hookType, HookPhase: OperationSucceeded, SyncPhase: SyncPhase(hookType)}))
 }

+func TestPreDeleteHook(t *testing.T) {
+	Given(t).
+		Path("pre-delete-hook").
+		When().
+		CreateApp().
+		Sync().
+		Then().
+		Expect(SyncStatusIs(SyncStatusCodeSynced)).
+		And(func(_ *Application) {
+			_, err := KubeClientset.CoreV1().ConfigMaps(DeploymentNamespace()).Get(
+				t.Context(), "guestbook-ui", metav1.GetOptions{},
+			)
+			require.NoError(t, err)
+		}).
+		When().
+		Delete(true).
+		Then().
+		Expect(DoesNotExist()).
+		Expect(NotPod(func(p corev1.Pod) bool {
+			return p.Name == "hook"
+		}))
+}
+
+func TestPreDeleteHookFailureAndRetry(t *testing.T) {
+	Given(t).
+		Path("pre-delete-hook").
+		When().
+		// Patch hook to make it fail
+		PatchFile("hook.yaml", `[{"op": "replace", "path": "/spec/containers/0/command/0", "value": "false"}]`).
+		CreateApp().
+		Sync().
+		Then().
+		Expect(SyncStatusIs(SyncStatusCodeSynced)).
+		When().
+		Delete(false). // Non-blocking delete
+		Then().
+		// App should still exist because pre-delete hook failed
+		Expect(Condition(ApplicationConditionDeletionError, "")).
+		When().
+		// Fix the hook by patching it to succeed
+		PatchFile("hook.yaml", `[{"op": "replace", "path": "/spec/containers/0/command", "value": ["sleep", "3"]}]`).
+		Refresh(RefreshTypeNormal).
+		Then().
+		// After fixing the hook, deletion should eventually succeed
+		Expect(DoesNotExist())
+}
+
 func TestPostDeleteHook(t *testing.T) {
 	Given(t).
 		Path("post-delete-hook").
@@ -61,12 +108,9 @@ func TestPostDeleteHook(t *testing.T) {
 		Delete(true).
 		Then().
 		Expect(DoesNotExist()).
-		AndAction(func() {
-			hooks, err := KubeClientset.CoreV1().Pods(DeploymentNamespace()).List(t.Context(), metav1.ListOptions{})
-			require.NoError(t, err)
-			assert.Len(t, hooks.Items, 1)
-			assert.Equal(t, "hook", hooks.Items[0].Name)
-		})
+		Expect(Pod(func(p corev1.Pod) bool {
+			return p.Name == "hook"
+		}))
 }

 // make sure that hooks do not appear in "argocd app diff"
--- a/test/e2e/testdata/pre-delete-hook/hook.yaml
+++ b/test/e2e/testdata/pre-delete-hook/hook.yaml
@@ -0,0 +1,25 @@
+apiVersion: v1
+kind: Pod
+metadata:
+  annotations:
+    argocd.argoproj.io/hook: PreDelete
+  labels:
+    argocd.argoproj.io/hook-type: PreDelete
+  name: hook
+spec:
+  containers:
+    - command:
+        - "sleep"
+        - "3"
+      image: quay.io/argoprojlabs/argocd-e2e-container:0.1
+      imagePullPolicy: IfNotPresent
+      name: main
+  restartPolicy: Never
+---
+# Regular resource to be managed and deleted
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: guestbook-ui
+data:
+  app: "guestbook"