test(engine): cleanup hook tests (#25673)

Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
This commit is contained in:
Alexandre Gaudreault 2025-12-18 09:07:19 -05:00 committed by GitHub
parent 6f0de8b858
commit 9a777c63fa
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 106 additions and 84 deletions

View file

@ -59,11 +59,12 @@ Hooks can be deleted in an automatic fashion using the annotation: argocd.argopr
argocd.argoproj.io/hook: PostSync
argocd.argoproj.io/hook-delete-policy: HookSucceeded
Hook deletion policies are governed by sync success and failure. A successful sync operation requires all hooks to complete successfully. A sync will fail if _any_ hooks fail.
The following policies define when the hook will be deleted.
- 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
- HookSucceeded - the hook resource is deleted if the sync succeeds
- HookFailed - the hook resource is deleted if the sync fails.
- BeforeHookCreation - the hook resource is deleted if it exist at the start of the sync.
# Sync Waves

View file

@ -0,0 +1,83 @@
package sync
import (
"fmt"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"github.com/argoproj/gitops-engine/pkg/health"
synccommon "github.com/argoproj/gitops-engine/pkg/sync/common"
"github.com/argoproj/gitops-engine/pkg/sync/hook"
"github.com/argoproj/gitops-engine/pkg/utils/kube"
testingutils "github.com/argoproj/gitops-engine/pkg/utils/testing"
)
type resourceNameHealthOverride map[string]health.HealthStatusCode
func (r resourceNameHealthOverride) GetResourceHealth(obj *unstructured.Unstructured) (*health.HealthStatus, error) {
if status, ok := r[obj.GetName()]; ok {
return &health.HealthStatus{Status: status, Message: "test"}, nil
}
return nil, nil
}
func getResourceResult(resources []synccommon.ResourceSyncResult, resourceKey kube.ResourceKey) *synccommon.ResourceSyncResult {
for _, res := range resources {
if res.ResourceKey == resourceKey {
return &res
}
}
return nil
}
func newHook(hookType synccommon.HookType, deletePolicy synccommon.HookDeletePolicy) *unstructured.Unstructured {
obj := testingutils.NewPod()
obj.SetNamespace(testingutils.FakeArgoCDNamespace)
testingutils.Annotate(obj, synccommon.AnnotationKeyHook, string(hookType))
testingutils.Annotate(obj, synccommon.AnnotationKeyHookDeletePolicy, string(deletePolicy))
obj.SetFinalizers([]string{hook.HookFinalizer})
return obj
}
func withReplaceAnnotation(un *unstructured.Unstructured) *unstructured.Unstructured {
un.SetAnnotations(map[string]string{synccommon.AnnotationSyncOptions: synccommon.SyncOptionReplace})
return un
}
func withServerSideApplyAnnotation(un *unstructured.Unstructured) *unstructured.Unstructured {
un.SetAnnotations(map[string]string{synccommon.AnnotationSyncOptions: synccommon.SyncOptionServerSideApply})
return un
}
func withDisableServerSideApplyAnnotation(un *unstructured.Unstructured) *unstructured.Unstructured {
un.SetAnnotations(map[string]string{synccommon.AnnotationSyncOptions: synccommon.SyncOptionDisableServerSideApply})
return un
}
func withReplaceAndServerSideApplyAnnotations(un *unstructured.Unstructured) *unstructured.Unstructured {
un.SetAnnotations(map[string]string{synccommon.AnnotationSyncOptions: "Replace=true,ServerSideApply=true"})
return un
}
func withForceAnnotation(un *unstructured.Unstructured) *unstructured.Unstructured {
un.SetAnnotations(map[string]string{synccommon.AnnotationSyncOptions: synccommon.SyncOptionForce})
return un
}
func withForceAndReplaceAnnotations(un *unstructured.Unstructured) *unstructured.Unstructured {
un.SetAnnotations(map[string]string{synccommon.AnnotationSyncOptions: "Force=true,Replace=true"})
return un
}
func createNamespaceTask(namespace string) (*syncTask, error) {
nsSpec := &corev1.Namespace{TypeMeta: metav1.TypeMeta{APIVersion: "v1", Kind: kube.NamespaceKind}, ObjectMeta: metav1.ObjectMeta{Name: namespace}}
unstructuredObj, err := kube.ToUnstructured(nsSpec)
task := &syncTask{phase: synccommon.SyncPhasePreSync, targetObj: unstructuredObj}
if err != nil {
return task, fmt.Errorf("failed to convert namespace spec to unstructured: %w", err)
}
return task, nil
}

View file

@ -4,7 +4,6 @@ import (
"context"
"encoding/json"
"errors"
"fmt"
"net/http"
"net/http/httptest"
"reflect"
@ -833,11 +832,6 @@ func TestSyncOptionValidate(t *testing.T) {
}
}
func withReplaceAnnotation(un *unstructured.Unstructured) *unstructured.Unstructured {
un.SetAnnotations(map[string]string{synccommon.AnnotationSyncOptions: synccommon.SyncOptionReplace})
return un
}
func TestSync_Replace(t *testing.T) {
testCases := []struct {
name string
@ -872,21 +866,6 @@ func TestSync_Replace(t *testing.T) {
}
}
func withServerSideApplyAnnotation(un *unstructured.Unstructured) *unstructured.Unstructured {
un.SetAnnotations(map[string]string{synccommon.AnnotationSyncOptions: synccommon.SyncOptionServerSideApply})
return un
}
func withDisableServerSideApplyAnnotation(un *unstructured.Unstructured) *unstructured.Unstructured {
un.SetAnnotations(map[string]string{synccommon.AnnotationSyncOptions: synccommon.SyncOptionDisableServerSideApply})
return un
}
func withReplaceAndServerSideApplyAnnotations(un *unstructured.Unstructured) *unstructured.Unstructured {
un.SetAnnotations(map[string]string{synccommon.AnnotationSyncOptions: "Replace=true,ServerSideApply=true"})
return un
}
func TestSync_HookWithReplaceAndBeforeHookCreation_AlreadyDeleted(t *testing.T) {
// This test a race condition when Delete is called on an already deleted object
// LiveObj is set, but then the resource is deleted asynchronously in kubernetes
@ -992,16 +971,6 @@ func TestSyncContext_ServerSideApplyWithDryRun(t *testing.T) {
}
}
func withForceAnnotation(un *unstructured.Unstructured) *unstructured.Unstructured {
un.SetAnnotations(map[string]string{synccommon.AnnotationSyncOptions: synccommon.SyncOptionForce})
return un
}
func withForceAndReplaceAnnotations(un *unstructured.Unstructured) *unstructured.Unstructured {
un.SetAnnotations(map[string]string{synccommon.AnnotationSyncOptions: "Force=true,Replace=true"})
return un
}
func TestSync_Force(t *testing.T) {
testCases := []struct {
name string
@ -1299,24 +1268,13 @@ func TestNamespaceAutoCreationForNonExistingNs(t *testing.T) {
})
}
func createNamespaceTask(namespace string) (*syncTask, error) {
nsSpec := &corev1.Namespace{TypeMeta: metav1.TypeMeta{APIVersion: "v1", Kind: kube.NamespaceKind}, ObjectMeta: metav1.ObjectMeta{Name: namespace}}
unstructuredObj, err := kube.ToUnstructured(nsSpec)
task := &syncTask{phase: synccommon.SyncPhasePreSync, targetObj: unstructuredObj}
if err != nil {
return task, fmt.Errorf("failed to convert namespace spec to unstructured: %w", err)
}
return task, nil
}
func TestSyncFailureHookWithSuccessfulSync(t *testing.T) {
syncCtx := newTestSyncCtx(nil)
syncCtx.resources = groupResources(ReconciliationResult{
Live: []*unstructured.Unstructured{nil},
Target: []*unstructured.Unstructured{testingutils.NewPod()},
})
syncCtx.hooks = []*unstructured.Unstructured{newHook(synccommon.HookTypeSyncFail)}
syncCtx.hooks = []*unstructured.Unstructured{newHook(synccommon.HookTypeSyncFail, synccommon.HookDeletePolicyBeforeHookCreation)}
syncCtx.Sync()
phase, _, resources := syncCtx.GetState()
@ -1332,7 +1290,7 @@ func TestSyncFailureHookWithFailedSync(t *testing.T) {
Live: []*unstructured.Unstructured{nil},
Target: []*unstructured.Unstructured{pod},
})
syncCtx.hooks = []*unstructured.Unstructured{newHook(synccommon.HookTypeSyncFail)}
syncCtx.hooks = []*unstructured.Unstructured{newHook(synccommon.HookTypeSyncFail, synccommon.HookDeletePolicyBeforeHookCreation)}
mockKubectl := &kubetest.MockKubectlCmd{
Commands: map[string]kubetest.KubectlOutput{pod.GetName(): {Err: errors.New("")}},
}
@ -1384,19 +1342,12 @@ func TestBeforeHookCreation(t *testing.T) {
}
func TestSync_ExistingHooksWithFinalizer(t *testing.T) {
newHook := func(name string, hookType synccommon.HookType, deletePolicy synccommon.HookDeletePolicy) *unstructured.Unstructured {
obj := testingutils.NewPod()
obj.SetName(name)
obj.SetNamespace(testingutils.FakeArgoCDNamespace)
testingutils.Annotate(obj, synccommon.AnnotationKeyHook, string(hookType))
testingutils.Annotate(obj, synccommon.AnnotationKeyHookDeletePolicy, string(deletePolicy))
obj.SetFinalizers([]string{hook.HookFinalizer})
return obj
}
hook1 := newHook("existing-hook-1", synccommon.HookTypePreSync, synccommon.HookDeletePolicyBeforeHookCreation)
hook2 := newHook("existing-hook-2", synccommon.HookTypePreSync, synccommon.HookDeletePolicyHookFailed)
hook3 := newHook("existing-hook-3", synccommon.HookTypePreSync, synccommon.HookDeletePolicyHookSucceeded)
hook1 := newHook(synccommon.HookTypePreSync, synccommon.HookDeletePolicyBeforeHookCreation)
hook1.SetName("existing-hook-1")
hook2 := newHook(synccommon.HookTypePreSync, synccommon.HookDeletePolicyHookFailed)
hook2.SetName("existing-hook-2")
hook3 := newHook(synccommon.HookTypePreSync, synccommon.HookDeletePolicyHookSucceeded)
hook3.SetName("existing-hook-3")
syncCtx := newTestSyncCtx(nil)
fakeDynamicClient := fake.NewSimpleDynamicClient(runtime.NewScheme(), hook1, hook2, hook3)
@ -1436,9 +1387,9 @@ func TestRunSyncFailHooksFailed(t *testing.T) {
syncCtx := newTestSyncCtx(nil)
pod := testingutils.NewPod()
successfulSyncFailHook := newHook(synccommon.HookTypeSyncFail)
successfulSyncFailHook := newHook(synccommon.HookTypeSyncFail, synccommon.HookDeletePolicyBeforeHookCreation)
successfulSyncFailHook.SetName("successful-sync-fail-hook")
failedSyncFailHook := newHook(synccommon.HookTypeSyncFail)
failedSyncFailHook := newHook(synccommon.HookTypeSyncFail, synccommon.HookDeletePolicyBeforeHookCreation)
failedSyncFailHook.SetName("failed-sync-fail-hook")
syncCtx.resources = groupResources(ReconciliationResult{
Live: []*unstructured.Unstructured{nil},
@ -1479,24 +1430,15 @@ func TestRunSyncFailHooksFailed(t *testing.T) {
assert.Equal(t, synccommon.ResultCodeSynced, resources[2].Status)
}
type resourceNameHealthOverride map[string]health.HealthStatusCode
func (r resourceNameHealthOverride) GetResourceHealth(obj *unstructured.Unstructured) (*health.HealthStatus, error) {
if status, ok := r[obj.GetName()]; ok {
return &health.HealthStatus{Status: status, Message: "test"}, nil
}
return nil, nil
}
func TestRunSync_HooksNotDeletedIfPhaseNotCompleted(t *testing.T) {
hook1 := newHook(synccommon.HookTypePreSync)
hook1 := newHook(synccommon.HookTypePreSync, synccommon.HookDeletePolicyBeforeHookCreation)
hook1.SetName("completed-hook")
hook1.SetNamespace(testingutils.FakeArgoCDNamespace)
_ = testingutils.Annotate(hook1, synccommon.AnnotationKeyHookDeletePolicy, string(synccommon.HookDeletePolicyHookSucceeded))
completedHook := hook1.DeepCopy()
completedHook.SetFinalizers(append(completedHook.GetFinalizers(), hook.HookFinalizer))
hook2 := newHook(synccommon.HookTypePreSync)
hook2 := newHook(synccommon.HookTypePreSync, synccommon.HookDeletePolicyBeforeHookCreation)
hook2.SetNamespace(testingutils.FakeArgoCDNamespace)
hook2.SetName("in-progress-hook")
_ = testingutils.Annotate(hook2, synccommon.AnnotationKeyHookDeletePolicy, string(synccommon.HookDeletePolicyHookSucceeded))
@ -1549,14 +1491,14 @@ func TestRunSync_HooksNotDeletedIfPhaseNotCompleted(t *testing.T) {
}
func TestRunSync_HooksDeletedAfterPhaseCompleted(t *testing.T) {
hook1 := newHook(synccommon.HookTypePreSync)
hook1 := newHook(synccommon.HookTypePreSync, synccommon.HookDeletePolicyBeforeHookCreation)
hook1.SetName("completed-hook1")
hook1.SetNamespace(testingutils.FakeArgoCDNamespace)
_ = testingutils.Annotate(hook1, synccommon.AnnotationKeyHookDeletePolicy, string(synccommon.HookDeletePolicyHookSucceeded))
completedHook1 := hook1.DeepCopy()
completedHook1.SetFinalizers(append(completedHook1.GetFinalizers(), hook.HookFinalizer))
hook2 := newHook(synccommon.HookTypePreSync)
hook2 := newHook(synccommon.HookTypePreSync, synccommon.HookDeletePolicyBeforeHookCreation)
hook2.SetNamespace(testingutils.FakeArgoCDNamespace)
hook2.SetName("completed-hook2")
_ = testingutils.Annotate(hook2, synccommon.AnnotationKeyHookDeletePolicy, string(synccommon.HookDeletePolicyHookSucceeded))
@ -1606,14 +1548,14 @@ func TestRunSync_HooksDeletedAfterPhaseCompleted(t *testing.T) {
}
func TestRunSync_HooksDeletedAfterPhaseCompletedFailed(t *testing.T) {
hook1 := newHook(synccommon.HookTypeSync)
hook1 := newHook(synccommon.HookTypeSync, synccommon.HookDeletePolicyBeforeHookCreation)
hook1.SetName("completed-hook1")
hook1.SetNamespace(testingutils.FakeArgoCDNamespace)
_ = testingutils.Annotate(hook1, synccommon.AnnotationKeyHookDeletePolicy, string(synccommon.HookDeletePolicyHookFailed))
completedHook1 := hook1.DeepCopy()
completedHook1.SetFinalizers(append(completedHook1.GetFinalizers(), hook.HookFinalizer))
hook2 := newHook(synccommon.HookTypeSync)
hook2 := newHook(synccommon.HookTypeSync, synccommon.HookDeletePolicyBeforeHookCreation)
hook2.SetNamespace(testingutils.FakeArgoCDNamespace)
hook2.SetName("completed-hook2")
_ = testingutils.Annotate(hook2, synccommon.AnnotationKeyHookDeletePolicy, string(synccommon.HookDeletePolicyHookFailed))

View file

@ -10,10 +10,6 @@ import (
testingutils "github.com/argoproj/gitops-engine/pkg/utils/testing"
)
func newHook(hookType common.HookType) *unstructured.Unstructured {
return testingutils.Annotate(testingutils.NewPod(), "argocd.argoproj.io/hook", string(hookType))
}
func Test_syncTask_hookType(t *testing.T) {
type fields struct {
phase common.SyncPhase
@ -25,9 +21,9 @@ func Test_syncTask_hookType(t *testing.T) {
want common.HookType
}{
{"Empty", fields{common.SyncPhaseSync, testingutils.NewPod()}, ""},
{"PreSyncHook", fields{common.SyncPhasePreSync, newHook(common.HookTypePreSync)}, common.HookTypePreSync},
{"SyncHook", fields{common.SyncPhaseSync, newHook(common.HookTypeSync)}, common.HookTypeSync},
{"PostSyncHook", fields{common.SyncPhasePostSync, newHook(common.HookTypePostSync)}, common.HookTypePostSync},
{"PreSyncHook", fields{common.SyncPhasePreSync, newHook(common.HookTypePreSync, common.HookDeletePolicyBeforeHookCreation)}, common.HookTypePreSync},
{"SyncHook", fields{common.SyncPhaseSync, newHook(common.HookTypeSync, common.HookDeletePolicyBeforeHookCreation)}, common.HookTypeSync},
{"PostSyncHook", fields{common.SyncPhasePostSync, newHook(common.HookTypePostSync, common.HookDeletePolicyBeforeHookCreation)}, common.HookTypePostSync},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {