Skip to content

Conversation

@cx-shmuel-felman
Copy link

@cx-shmuel-felman cx-shmuel-felman commented Jan 13, 2026

Summary

This PR removes the default deletion policy for resources with hook annotations. Resources with hook annotations should not be automatically deleted when only the hook phase annotation is specified.

Motivation

Previously, resources with hook annotations would be automatically deleted due to two different mechanisms:

  1. Resources with sync hook annotations (PreSync, PostSync, Sync, SyncFail) had an automatic BeforeHookCreation deletion policy applied in the gitops-engine
  2. Resources with delete hook annotations (PreDelete, PostDelete) were automatically deleted during the cleanup phase in the controller

This was unexpected behavior because:

  • The hook annotation primarily controls when resources are synced (timing/phase), not their lifecycle
  • Automatic deletion should be explicit, not default
  • It caused confusion and loss of state for resources with hook annotations
  • Users expected these resources to persist unless explicitly configured otherwise

Changes

1. gitops-engine/pkg/sync/hook/delete_policy.go

Removed the automatic assignment of BeforeHookCreation deletion policy when no policy is specified:

Before:

if len(policies) == 0 {
    policies = append(policies, common.HookDeletePolicyBeforeHookCreation)
}

After:

// No default deletion policy - hook resources should only be deleted when explicitly configured
return policies

2. controller/hook.go

Removed the auto-deletion logic in the cleanupHooks function for resources with PreDelete/PostDelete hook annotations:

Before:

if len(deletePolicies) == 0 {
    // If no delete policy is specified, always delete hooks during cleanup phase
    shouldDelete = true
}

After:

// Only delete hook resources if they have an explicit deletion policy that matches the current state
for _, policy := range deletePolicies {
    if (policy == common.HookDeletePolicyHookFailed && aggregatedHealth == health.HealthStatusDegraded) ||
        (policy == common.HookDeletePolicyHookSucceeded && aggregatedHealth == health.HealthStatusHealthy) {
        shouldDelete = true
        break
    }
}

3. Updated Tests and Documentation

  • Updated unit tests in gitops-engine/pkg/sync/hook/delete_policy_test.go
  • Updated documentation in gitops-engine/pkg/sync/doc.go to clarify that resources with hook annotations are NOT deleted by default

Behavior Changes

Before this PR:

apiVersion: batch/v1
kind: Job
metadata:
  annotations:
    argocd.argoproj.io/hook: PreSync

☠️ This Job resource would be automatically deleted and recreated on every sync

After this PR:

apiVersion: batch/v1
kind: Job
metadata:
  annotations:
    argocd.argoproj.io/hook: PreSync

✅ This Job resource will persist across syncs

To get the old auto-delete behavior:

apiVersion: batch/v1
kind: Job
metadata:
  annotations:
    argocd.argoproj.io/hook: PreSync
    argocd.argoproj.io/hook-delete-policy: BeforeHookCreation  # Explicit deletion policy

Testing

  • ✅ All unit tests in gitops-engine/pkg/sync/hook pass
  • ✅ All unit tests in gitops-engine/pkg/sync pass
  • ✅ All controller tests pass
  • ✅ No linter errors

Migration Guide

For users who relied on the old default behavior and want resources with hook annotations to be deleted before recreation, add an explicit deletion policy:

apiVersion: batch/v1
kind: Job
metadata:
  annotations:
    argocd.argoproj.io/hook: PreSync
    argocd.argoproj.io/hook-delete-policy: BeforeHookCreation  # Add this

Available deletion policies:

  • BeforeHookCreation - Delete the resource before creating it (old default behavior)
  • HookSucceeded - Delete the resource after it succeeds
  • HookFailed - Delete the resource after it fails

Impact

This is a behavior change that makes ArgoCD more intuitive:

  • Resources with hook annotations are treated like any other resource - they persist by default
  • Deletion must be explicitly configured via the hook-delete-policy annotation
  • Aligns with user expectations
  • Reduces surprise and confusion

Related Issues

Fixes #25956

Checklist

  • Code changes made
  • Tests updated
  • Documentation updated
  • All tests passing
  • No linter errors
  • Commits signed-off (DCO)

@cx-shmuel-felman cx-shmuel-felman requested a review from a team as a code owner January 13, 2026 10:26
@bunnyshell
Copy link

bunnyshell bot commented Jan 13, 2026

✅ Preview Environment deployed on Bunnyshell

Component Endpoints
argocd https://argocd-aaehlm.bunnyenv.com/
argocd-ttyd https://argocd-web-cli-aaehlm.bunnyenv.com/

See: Environment Details | Pipeline Logs

Available commands (reply to this comment):

  • 🔴 /bns:stop to stop the environment
  • 🚀 /bns:deploy to redeploy the environment
  • /bns:delete to remove the environment

@cx-shmuel-felman cx-shmuel-felman force-pushed the fix/hook-default-deletion-policy branch from 082561b to 3e2d142 Compare January 13, 2026 10:29
@cx-shmuel-felman cx-shmuel-felman marked this pull request as draft January 13, 2026 16:52
@cx-shmuel-felman cx-shmuel-felman marked this pull request as ready for review January 13, 2026 17:06
@cx-shmuel-felman cx-shmuel-felman force-pushed the fix/hook-default-deletion-policy branch from 64363c7 to ba9ffbd Compare January 13, 2026 17:19
Resources with hook annotations should not be automatically deleted when
only the hook phase annotation is specified. This change makes resource
deletion opt-in rather than opt-out for all hook types.

Previously, resources with hook annotations would be automatically deleted due to:
1. Resources with sync hook annotations (PreSync, PostSync, Sync, SyncFail)
   had automatic BeforeHookCreation deletion policy applied
2. Resources with delete hook annotations (PreDelete, PostDelete) were
   auto-deleted during cleanup phase when no deletion policy was specified

This was unexpected behavior because:
- The hook annotation primarily controls WHEN resources are synced
- Automatic resource deletion should be explicit, not default
- It caused confusion and loss of state for resources with hook annotations
- Users expected these resources to persist unless explicitly configured

Changes made:

1. gitops-engine/pkg/sync/hook/delete_policy.go:
   - Removed automatic assignment of BeforeHookCreation policy
   - Returns empty deletion policies by default

2. controller/hook.go:
   - Removed auto-deletion logic in cleanupHooks function
   - Resources with PreDelete/PostDelete hooks only delete with explicit policy

3. Updated tests and documentation to reflect new behavior

With this change:
- All resources with hook annotations persist by default
- Users must explicitly add hook-delete-policy annotation to enable
  automatic deletion (HookSucceeded, HookFailed, BeforeHookCreation)
- This aligns with user expectations and makes behavior more intuitive

Fixes argoproj#25956

Signed-off-by: Shmuel Felman <[email protected]>
@cx-shmuel-felman cx-shmuel-felman force-pushed the fix/hook-default-deletion-policy branch from ba9ffbd to db0e63e Compare January 13, 2026 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Hook resources should not be deleted by default when only hook phase annotation is specified

1 participant