marcel/argo-cd
1
0
Fork 0
mirror of https://github.com/argoproj/argo-cd.git synced 2026-02-20 01:28:45 +01:00
Code Issues Packages Projects Releases Wiki Activity

fix(engine): always preserve sync status for hooks (#25692)

Browse Source 
Signed-off-by: reggie-k <regina.voloshin@codefresh.io>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: Afzal Ansari <afzal442@gmail.com>
Signed-off-by: Julie Vogelman <julie_vogelman@intuit.com>
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Co-authored-by: Regina Voloshin <regina.voloshin@codefresh.io>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: reggie-k <19544836+reggie-k@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Afzal Ansari <afzal442@gmail.com>
Co-authored-by: Blake Pettersson <blake.pettersson@gmail.com>
Co-authored-by: Julie Vogelman <julie_vogelman@intuit.com>
Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
This commit is contained in:
Alexandre Gaudreault
2025-12-19 12:05:05 -05:00
committed by GitHub
parent 0114636cdc
commit 1dc85e564b
5 changed files with 43 additions and 43 deletions
Download Patch File Download Diff File Expand all files Collapse all files

20
gitops-engine/pkg/sync/sync_context.go
View File

@@ -457,7 +457,7 @@ func (sc *syncContext) Sync() {
// No need to perform a dry-run on the namespace creation, because if it fails we stop anyway
sc.log.WithValues("task", nsCreateTask).Info("Creating namespace")
if sc.runTasks(nsSyncTasks, false) == failed {
sc.setOperationFailed(syncTasks{}, nsSyncTasks, "the namespace failed to apply")
sc.executeSyncFailPhase(syncTasks{}, nsSyncTasks, "the namespace failed to apply")
return
}
@@ -485,9 +485,9 @@ func (sc *syncContext) Sync() {
// update the hook's result
operationState, message, err := sc.getOperationPhase(task.liveObj)
if err != nil {
sc.setResourceResult(task, "", common.OperationError, fmt.Sprintf("failed to get resource health: %v", err))
sc.setResourceResult(task, task.syncStatus, common.OperationError, fmt.Sprintf("failed to get resource health: %v", err))
} else {
sc.setResourceResult(task, "", operationState, message)
sc.setResourceResult(task, task.syncStatus, operationState, message)
}
} else {
// this must be calculated on the live object
@@ -557,7 +557,7 @@ func (sc *syncContext) Sync() {
// if there are any completed but unsuccessful tasks, sync is a failure.
if tasks.Any(func(t *syncTask) bool { return t.completed() && !t.successful() }) {
sc.deleteHooks(hooksPendingDeletionFailed)
sc.setOperationFailed(syncFailTasks, syncFailedTasks, "one or more synchronization tasks completed unsuccessfully")
sc.executeSyncFailPhase(syncFailTasks, syncFailedTasks, "one or more synchronization tasks completed unsuccessfully")
return
}
@@ -610,7 +610,7 @@ func (sc *syncContext) Sync() {
case failed:
syncFailedTasks, _ := tasks.Split(func(t *syncTask) bool { return t.syncStatus == common.ResultCodeSyncFailed })
sc.deleteHooks(hooksPendingDeletionFailed)
sc.setOperationFailed(syncFailTasks, syncFailedTasks, "one or more objects failed to apply")
sc.executeSyncFailPhase(syncFailTasks, syncFailedTasks, "one or more objects failed to apply")
case successful:
if remainingTasks.Len() == 0 {
// delete all completed hooks which have appropriate delete policy
@@ -731,7 +731,7 @@ func (sc *syncContext) deleteHooks(hooksPendingDeletion syncTasks) {
for _, task := range hooksPendingDeletion {
err := sc.deleteResource(task)
if err != nil && !apierrors.IsNotFound(err) {
sc.setResourceResult(task, "", common.OperationError, fmt.Sprintf("failed to delete resource: %v", err))
sc.setResourceResult(task, task.syncStatus, common.OperationError, fmt.Sprintf("failed to delete resource: %v", err))
}
}
}
@@ -747,7 +747,7 @@ func (sc *syncContext) GetState() (common.OperationPhase, string, []common.Resou
return sc.phase, sc.message, resourceRes
}
func (sc *syncContext) setOperationFailed(syncFailTasks, syncFailedTasks syncTasks, message string) {
func (sc *syncContext) executeSyncFailPhase(syncFailTasks, syncFailedTasks syncTasks, message string) {
errorMessageFactory := func(tasks syncTasks, message string) string {
messages := tasks.Map(func(task *syncTask) string {
return task.message
@@ -1323,13 +1323,13 @@ func (sc *syncContext) Terminate() {
if phase == common.OperationRunning {
err := sc.deleteResource(task)
if err != nil && !apierrors.IsNotFound(err) {
sc.setResourceResult(task, "", common.OperationFailed, fmt.Sprintf("Failed to delete: %v", err))
sc.setResourceResult(task, task.syncStatus, common.OperationFailed, fmt.Sprintf("Failed to delete: %v", err))
terminateSuccessful = false
} else {
sc.setResourceResult(task, "", common.OperationSucceeded, "Deleted")
sc.setResourceResult(task, task.syncStatus, common.OperationSucceeded, "Deleted")
}
} else {
sc.setResourceResult(task, "", phase, msg)
sc.setResourceResult(task, task.syncStatus, phase, msg)
}
}
if terminateSuccessful {

50
gitops-engine/pkg/sync/sync_context_test.go
View File

@@ -1951,19 +1951,19 @@ func TestSyncContext_GetDeleteOptions_WithPrunePropagationPolicy(t *testing.T) {
assert.Equal(t, metav1.DeletePropagationBackground, *opts.PropagationPolicy)
}
func TestSetOperationFailed(t *testing.T) {
func TestExecuteSyncFailPhase(t *testing.T) {
sc := syncContext{}
sc.log = textlogger.NewLogger(textlogger.NewConfig()).WithValues("application", "fake-app")
tasks := make([]*syncTask, 0)
tasks = append(tasks, &syncTask{message: "namespace not found"})
sc.setOperationFailed(nil, tasks, "one or more objects failed to apply")
sc.executeSyncFailPhase(nil, tasks, "one or more objects failed to apply")
assert.Equal(t, "one or more objects failed to apply, reason: namespace not found", sc.message)
}
func TestSetOperationFailedDuplicatedMessages(t *testing.T) {
func TestExecuteSyncFailPhase_DuplicatedMessages(t *testing.T) {
sc := syncContext{}
sc.log = textlogger.NewLogger(textlogger.NewConfig()).WithValues("application", "fake-app")
@@ -1971,16 +1971,16 @@ func TestSetOperationFailedDuplicatedMessages(t *testing.T) {
tasks = append(tasks, &syncTask{message: "namespace not found"})
tasks = append(tasks, &syncTask{message: "namespace not found"})
sc.setOperationFailed(nil, tasks, "one or more objects failed to apply")
sc.executeSyncFailPhase(nil, tasks, "one or more objects failed to apply")
assert.Equal(t, "one or more objects failed to apply, reason: namespace not found", sc.message)
}
func TestSetOperationFailedNoTasks(t *testing.T) {
func TestExecuteSyncFailPhase_NoTasks(t *testing.T) {
sc := syncContext{}
sc.log = textlogger.NewLogger(textlogger.NewConfig()).WithValues("application", "fake-app")
sc.setOperationFailed(nil, nil, "one or more objects failed to apply")
sc.executeSyncFailPhase(nil, nil, "one or more objects failed to apply")
assert.Equal(t, "one or more objects failed to apply", sc.message)
}
@@ -2223,15 +2223,15 @@ func TestWaitForCleanUpBeforeNextWave(t *testing.T) {
var phase synccommon.OperationPhase
var msg string
var result []synccommon.ResourceSyncResult
var results []synccommon.ResourceSyncResult
// 1st sync should prune only pod3
syncCtx.Sync()
phase, _, result = syncCtx.GetState()
phase, _, results = syncCtx.GetState()
assert.Equal(t, synccommon.OperationRunning, phase)
assert.Len(t, result, 1)
assert.Equal(t, "pod-3", result[0].ResourceKey.Name)
assert.Equal(t, synccommon.ResultCodePruned, result[0].Status)
assert.Len(t, results, 1)
assert.Equal(t, "pod-3", results[0].ResourceKey.Name)
assert.Equal(t, synccommon.ResultCodePruned, results[0].Status)
// simulate successful delete of pod3
syncCtx.resources = groupResources(ReconciliationResult{
@@ -2241,11 +2241,11 @@ func TestWaitForCleanUpBeforeNextWave(t *testing.T) {
// next sync should prune only pod2
syncCtx.Sync()
phase, _, result = syncCtx.GetState()
phase, _, results = syncCtx.GetState()
assert.Equal(t, synccommon.OperationRunning, phase)
assert.Len(t, result, 2)
assert.Equal(t, "pod-2", result[1].ResourceKey.Name)
assert.Equal(t, synccommon.ResultCodePruned, result[1].Status)
assert.Len(t, results, 2)
assert.Equal(t, "pod-2", results[1].ResourceKey.Name)
assert.Equal(t, synccommon.ResultCodePruned, results[1].Status)
// add delete timestamp on pod2 to simulate pending delete
pod2.SetDeletionTimestamp(&metav1.Time{Time: time.Now()})
@@ -2253,10 +2253,10 @@ func TestWaitForCleanUpBeforeNextWave(t *testing.T) {
// next sync should wait for deletion of pod2 from cluster,
// it should not move to next wave and prune pod1
syncCtx.Sync()
phase, msg, result = syncCtx.GetState()
phase, msg, results = syncCtx.GetState()
assert.Equal(t, synccommon.OperationRunning, phase)
assert.Equal(t, "waiting for deletion of /Pod/pod-2", msg)
assert.Len(t, result, 2)
assert.Len(t, results, 2)
// simulate successful delete of pod2
syncCtx.resources = groupResources(ReconciliationResult{
@@ -2267,15 +2267,15 @@ func TestWaitForCleanUpBeforeNextWave(t *testing.T) {
// next sync should proceed with next wave
// i.e deletion of pod1
syncCtx.Sync()
phase, _, result = syncCtx.GetState()
phase, _, results = syncCtx.GetState()
assert.Equal(t, synccommon.OperationSucceeded, phase)
assert.Len(t, result, 3)
assert.Equal(t, "pod-3", result[0].ResourceKey.Name)
assert.Equal(t, "pod-2", result[1].ResourceKey.Name)
assert.Equal(t, "pod-1", result[2].ResourceKey.Name)
assert.Equal(t, synccommon.ResultCodePruned, result[0].Status)
assert.Equal(t, synccommon.ResultCodePruned, result[1].Status)
assert.Equal(t, synccommon.ResultCodePruned, result[2].Status)
assert.Len(t, results, 3)
assert.Equal(t, "pod-3", results[0].ResourceKey.Name)
assert.Equal(t, "pod-2", results[1].ResourceKey.Name)
assert.Equal(t, "pod-1", results[2].ResourceKey.Name)
assert.Equal(t, synccommon.ResultCodePruned, results[0].Status)
assert.Equal(t, synccommon.ResultCodePruned, results[1].Status)
assert.Equal(t, synccommon.ResultCodePruned, results[2].Status)
}
func BenchmarkSync(b *testing.B) {

4
test/e2e/cli_test.go
View File

@@ -44,7 +44,7 @@ func TestCliAppCommand(t *testing.T) {
require.NoError(t, err)
vars := map[string]any{"Name": ctx.AppName(), "Namespace": DeploymentNamespace()}
assert.Contains(t, NormalizeOutput(output), Tmpl(t, `Pod {{.Namespace}} pod Synced Progressing pod/pod created`, vars))
assert.Contains(t, NormalizeOutput(output), Tmpl(t, `Pod {{.Namespace}} hook Succeeded Sync pod/hook created`, vars))
assert.Contains(t, NormalizeOutput(output), Tmpl(t, `Pod {{.Namespace}} hook Succeeded Synced Sync pod/hook created`, vars))
}).
Then().
Expect(OperationPhaseIs(OperationSucceeded)).
@@ -88,7 +88,7 @@ func TestNormalArgoCDCommandsExecuteOverPluginsWithSameName(t *testing.T) {
vars := map[string]any{"Name": ctx.AppName(), "Namespace": DeploymentNamespace()}
assert.Contains(t, NormalizeOutput(output), Tmpl(t, `Pod {{.Namespace}} pod Synced Progressing pod/pod created`, vars))
assert.Contains(t, NormalizeOutput(output), Tmpl(t, `Pod {{.Namespace}} hook Succeeded Sync pod/hook created`, vars))
assert.Contains(t, NormalizeOutput(output), Tmpl(t, `Pod {{.Namespace}} hook Succeeded Synced Sync pod/hook created`, vars))
}).
Then().
Expect(OperationPhaseIs(OperationSucceeded)).

2
test/e2e/helm_test.go
View File

@@ -31,7 +31,7 @@ func TestHelmHooksAreCreated(t *testing.T) {
Expect(OperationPhaseIs(OperationSucceeded)).
Expect(HealthIs(health.HealthStatusHealthy)).
Expect(SyncStatusIs(SyncStatusCodeSynced)).
Expect(ResourceResultIs(ResourceResult{Version: "v1", Kind: "Pod", Namespace: fixture.DeploymentNamespace(), Name: "hook", Message: "pod/hook created", Images: []string{"quay.io/argoprojlabs/argocd-e2e-container:0.1"}, HookType: HookTypePreSync, HookPhase: OperationSucceeded, SyncPhase: SyncPhasePreSync}))
Expect(ResourceResultIs(ResourceResult{Version: "v1", Kind: "Pod", Namespace: fixture.DeploymentNamespace(), Name: "hook", Message: "pod/hook created", Images: []string{"quay.io/argoprojlabs/argocd-e2e-container:0.1"}, HookType: HookTypePreSync, Status: ResultCodeSynced, HookPhase: OperationSucceeded, SyncPhase: SyncPhasePreSync}))
}
// make sure we treat Helm weights as a sync wave

10
test/e2e/hook_test.go
View File

@@ -49,7 +49,7 @@ func testHookSuccessful(t *testing.T, hookType HookType) {
Expect(ResourceSyncStatusIs("Pod", "pod", SyncStatusCodeSynced)).
Expect(ResourceHealthIs("Pod", "pod", health.HealthStatusHealthy)).
Expect(ResourceResultNumbering(2)).
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)}))
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, Status: ResultCodeSynced, HookPhase: OperationSucceeded, SyncPhase: SyncPhase(hookType)}))
}
func TestPreDeleteHook(t *testing.T) {
@@ -246,7 +246,7 @@ spec:
CreateApp().
Sync().
Then().
Expect(ResourceResultIs(ResourceResult{Version: "v1", Kind: "Pod", Namespace: DeploymentNamespace(), Images: []string{"quay.io/argoprojlabs/argocd-e2e-container:0.1"}, Name: "sync-fail-hook", Message: "pod/sync-fail-hook created", HookType: HookTypeSyncFail, HookPhase: OperationSucceeded, SyncPhase: SyncPhaseSyncFail})).
Expect(ResourceResultIs(ResourceResult{Version: "v1", Kind: "Pod", Namespace: DeploymentNamespace(), Images: []string{"quay.io/argoprojlabs/argocd-e2e-container:0.1"}, Name: "sync-fail-hook", Message: "pod/sync-fail-hook created", HookType: HookTypeSyncFail, Status: ResultCodeSynced, HookPhase: OperationSucceeded, SyncPhase: SyncPhaseSyncFail})).
Expect(OperationPhaseIs(OperationFailed))
}
@@ -293,8 +293,8 @@ spec:
CreateApp().
Sync().
Then().
Expect(ResourceResultIs(ResourceResult{Version: "v1", Kind: "Pod", Namespace: DeploymentNamespace(), Name: "successful-sync-fail-hook", Images: []string{"quay.io/argoprojlabs/argocd-e2e-container:0.1"}, Message: "pod/successful-sync-fail-hook created", HookType: HookTypeSyncFail, HookPhase: OperationSucceeded, SyncPhase: SyncPhaseSyncFail})).
Expect(ResourceResultIs(ResourceResult{Version: "v1", Kind: "Pod", Namespace: DeploymentNamespace(), Name: "failed-sync-fail-hook", Images: []string{"quay.io/argoprojlabs/argocd-e2e-container:0.1"}, Message: `container "main" failed with exit code 1`, HookType: HookTypeSyncFail, HookPhase: OperationFailed, SyncPhase: SyncPhaseSyncFail})).
Expect(ResourceResultIs(ResourceResult{Version: "v1", Kind: "Pod", Namespace: DeploymentNamespace(), Name: "successful-sync-fail-hook", Images: []string{"quay.io/argoprojlabs/argocd-e2e-container:0.1"}, Message: "pod/successful-sync-fail-hook created", HookType: HookTypeSyncFail, Status: ResultCodeSynced, HookPhase: OperationSucceeded, SyncPhase: SyncPhaseSyncFail})).
Expect(ResourceResultIs(ResourceResult{Version: "v1", Kind: "Pod", Namespace: DeploymentNamespace(), Name: "failed-sync-fail-hook", Images: []string{"quay.io/argoprojlabs/argocd-e2e-container:0.1"}, Message: `container "main" failed with exit code 1`, HookType: HookTypeSyncFail, Status: ResultCodeSynced, HookPhase: OperationFailed, SyncPhase: SyncPhaseSyncFail})).
Expect(OperationPhaseIs(OperationFailed))
}
@@ -522,5 +522,5 @@ func testHookFinalizer(t *testing.T, hookType HookType) {
Expect(ResourceSyncStatusIs("Pod", "pod", SyncStatusCodeSynced)).
Expect(ResourceHealthIs("Pod", "pod", health.HealthStatusHealthy)).
Expect(ResourceResultNumbering(2)).
Expect(ResourceResultIs(ResourceResult{Group: "batch", Version: "v1", Kind: "Job", Namespace: DeploymentNamespace(), Name: "hook", Images: []string{"quay.io/argoprojlabs/argocd-e2e-container:0.1"}, Message: "Resource has finalizer", HookType: hookType, HookPhase: OperationSucceeded, SyncPhase: SyncPhase(hookType)}))
Expect(ResourceResultIs(ResourceResult{Group: "batch", Version: "v1", Kind: "Job", Namespace: DeploymentNamespace(), Name: "hook", Images: []string{"quay.io/argoprojlabs/argocd-e2e-container:0.1"}, Message: "Resource has finalizer", HookType: hookType, Status: ResultCodeSynced, HookPhase: OperationSucceeded, SyncPhase: SyncPhase(hookType)}))
}