From 1ef9e93d14309f4eb665241c2c146a7d246659d1 Mon Sep 17 00:00:00 2001 From: Siddhesh Ghadi Date: Mon, 4 Sep 2023 16:29:24 +0530 Subject: [PATCH] Use waveOverride var instead of directly patching live obj Directly patching live objs results into incorrect wave ordering as the new wave value from live obj is used to perform reordering during next sync Signed-off-by: Siddhesh Ghadi --- pkg/sync/sync_context.go | 22 +++----------------- pkg/sync/sync_context_test.go | 39 +++++++++++++++++++++++++---------- 2 files changed, 31 insertions(+), 30 deletions(-) diff --git a/pkg/sync/sync_context.go b/pkg/sync/sync_context.go index 38fc8747b..477517c28 100644 --- a/pkg/sync/sync_context.go +++ b/pkg/sync/sync_context.go @@ -5,7 +5,6 @@ import ( "encoding/json" "fmt" "sort" - "strconv" "strings" "sync" "time" @@ -781,21 +780,11 @@ func (sc *syncContext) getSyncTasks() (_ syncTasks, successful bool) { endWave := uniquePruneWaves[n-1-i] for _, task := range pruneTasks[startWave] { - annotations := task.liveObj.GetAnnotations() - if annotations == nil { - annotations = make(map[string]string) - } - annotations[common.AnnotationSyncWave] = strconv.Itoa(endWave) - task.liveObj.SetAnnotations(annotations) + task.waveOverride = &endWave } for _, task := range pruneTasks[endWave] { - annotations := task.liveObj.GetAnnotations() - if annotations == nil { - annotations = make(map[string]string) - } - annotations[common.AnnotationSyncWave] = strconv.Itoa(startWave) - task.liveObj.SetAnnotations(annotations) + task.waveOverride = &startWave } } @@ -814,12 +803,7 @@ func (sc *syncContext) getSyncTasks() (_ syncTasks, successful bool) { for _, task := range tasks { if task.isPrune() && (sc.pruneLast || resourceutil.HasAnnotationOption(task.liveObj, common.AnnotationSyncOptions, common.SyncOptionPruneLast)) { - annotations := task.liveObj.GetAnnotations() - if annotations == nil { - annotations = make(map[string]string) - } - annotations[common.AnnotationSyncWave] = strconv.Itoa(syncPhaseLastWave) - task.liveObj.SetAnnotations(annotations) + task.waveOverride = &syncPhaseLastWave } } diff --git a/pkg/sync/sync_context_test.go b/pkg/sync/sync_context_test.go index 83c0ad7b8..2e2740609 100644 --- a/pkg/sync/sync_context_test.go +++ b/pkg/sync/sync_context_test.go @@ -1911,31 +1911,48 @@ func TestWaitForCleanUpBeforeNextWave(t *testing.T) { pod1.SetName("pod-1") pod2 := NewPod() pod2.SetName("pod-2") + pod3 := NewPod() + pod3.SetName("pod-3") pod1.SetAnnotations(map[string]string{synccommon.AnnotationSyncWave: "1"}) pod2.SetAnnotations(map[string]string{synccommon.AnnotationSyncWave: "2"}) + pod3.SetAnnotations(map[string]string{synccommon.AnnotationSyncWave: "3"}) syncCtx := newTestSyncCtx(nil) syncCtx.prune = true - // prune order : pod2 -> pod1 + // prune order : pod3 -> pod2 -> pod1 syncCtx.resources = groupResources(ReconciliationResult{ - Target: []*unstructured.Unstructured{nil, nil}, - Live: []*unstructured.Unstructured{pod1, pod2}, + Target: []*unstructured.Unstructured{nil, nil, nil}, + Live: []*unstructured.Unstructured{pod1, pod2, pod3}, }) var phase common.OperationPhase var msg string var result []common.ResourceSyncResult - // 1st sync should prune only pod2 + // 1st sync should prune only pod3 syncCtx.Sync() phase, _, result = syncCtx.GetState() assert.Equal(t, synccommon.OperationRunning, phase) assert.Equal(t, 1, len(result)) - assert.Equal(t, "pod-2", result[0].ResourceKey.Name) + assert.Equal(t, "pod-3", result[0].ResourceKey.Name) assert.Equal(t, synccommon.ResultCodePruned, result[0].Status) + // simulate successful delete of pod3 + syncCtx.resources = groupResources(ReconciliationResult{ + Target: []*unstructured.Unstructured{nil, nil, }, + Live: []*unstructured.Unstructured{pod1, pod2, }, + }) + + // next sync should prune only pod2 + syncCtx.Sync() + phase, _, result = syncCtx.GetState() + assert.Equal(t, synccommon.OperationRunning, phase) + assert.Equal(t, 2, len(result)) + assert.Equal(t, "pod-2", result[1].ResourceKey.Name) + assert.Equal(t, synccommon.ResultCodePruned, result[1].Status) + // add delete timestamp on pod2 to simulate pending delete pod2.SetDeletionTimestamp(&metav1.Time{Time: time.Now()}) @@ -1945,11 +1962,9 @@ func TestWaitForCleanUpBeforeNextWave(t *testing.T) { phase, msg, result = syncCtx.GetState() assert.Equal(t, synccommon.OperationRunning, phase) assert.Equal(t, "waiting for deletion of /Pod/pod-2", msg) - assert.Equal(t, 1, len(result)) + assert.Equal(t, 2, len(result)) // simulate successful delete of pod2 - pod2.SetDeletionTimestamp(nil) - pod2 = nil syncCtx.resources = groupResources(ReconciliationResult{ Target: []*unstructured.Unstructured{nil, }, Live: []*unstructured.Unstructured{pod1, }, @@ -1960,10 +1975,12 @@ func TestWaitForCleanUpBeforeNextWave(t *testing.T) { syncCtx.Sync() phase, _, result = syncCtx.GetState() assert.Equal(t, synccommon.OperationSucceeded, phase) - assert.Equal(t, 2, len(result)) - assert.Equal(t, "pod-2", result[0].ResourceKey.Name) - assert.Equal(t, "pod-1", result[1].ResourceKey.Name) + assert.Equal(t, 3, len(result)) + 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) }