feat: delete in reverse order of sync waves (#3959)

* feat: delete in reverse order of sync waves Signed-off-by: darshanime <[email protected]> * feat: add tests for deletion in order Signed-off-by: darshanime <[email protected]> * feat: fix lint for appcontroller.go Signed-off-by: darshanime <[email protected]> * feat: add comment to explain early return Signed-off-by: darshanime <[email protected]>
argoproj · Jul 21, 2020 · 53a9222 · 53a9222
1 parent dfd7457
commit 53a9222
Show file tree

Hide file tree

Showing 3 changed files with 90 additions and 3 deletions.
diff --git a/controller/appcontroller.go b/controller/appcontroller.go
@@ -601,7 +601,12 @@ func (ctrl *ApplicationController) finalizeApplicationDeletion(app *appv1.Applic
 
 	objs := make([]*unstructured.Unstructured, 0)
 	for k := range objsMap {
-		if ctrl.shouldBeDeleted(app, objsMap[k]) && objsMap[k].GetDeletionTimestamp() == nil {
+		// Wait for objects pending deletion to complete before proceeding with next sync wave
+		if objsMap[k].GetDeletionTimestamp() != nil {
+			logCtx.Infof("%d objects remaining for deletion", len(objsMap))
+			return objs, nil
+		}
+		if ctrl.shouldBeDeleted(app, objsMap[k]) {
 			objs = append(objs, objsMap[k])
 		}
 	}
@@ -612,8 +617,9 @@ func (ctrl *ApplicationController) finalizeApplicationDeletion(app *appv1.Applic
 	}
 	config := metrics.AddMetricsTransportWrapper(ctrl.metricsServer, app, cluster.RESTConfig())
 
-	err = kube.RunAllAsync(len(objs), func(i int) error {
-		obj := objs[i]
+	filteredObjs := FilterObjectsForDeletion(objs)
+	err = kube.RunAllAsync(len(filteredObjs), func(i int) error {
+		obj := filteredObjs[i]
 		return ctrl.kubectl.DeleteResource(config, obj.GroupVersionKind(), obj.GetName(), obj.GetNamespace(), false)
 	})
 	if err != nil {

diff --git a/controller/sort_delete.go b/controller/sort_delete.go
@@ -0,0 +1,40 @@
+package controller
+
+import (
+	"sort"
+
+	"github.com/argoproj/gitops-engine/pkg/sync/syncwaves"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+)
+
+type syncWaveSorter []*unstructured.Unstructured
+
+func (s syncWaveSorter) Len() int {
+	return len(s)
+}
+
+func (s syncWaveSorter) Swap(i, j int) {
+	s[i], s[j] = s[j], s[i]
+}
+
+func (s syncWaveSorter) Less(i, j int) bool {
+	return syncwaves.Wave(s[i]) < syncwaves.Wave(s[j])
+}
+
+func FilterObjectsForDeletion(objs []*unstructured.Unstructured) []*unstructured.Unstructured {
+	if len(objs) <= 1 {
+		return objs
+	}
+
+	sort.Sort(sort.Reverse(syncWaveSorter(objs)))
+
+	currentSyncWave := syncwaves.Wave(objs[0])
+	filteredObjs := make([]*unstructured.Unstructured, 0)
+	for _, obj := range objs {
+		if syncwaves.Wave(obj) != currentSyncWave {
+			break
+		}
+		filteredObjs = append(filteredObjs, obj)
+	}
+	return filteredObjs
+}
diff --git a/controller/sort_delete_test.go b/controller/sort_delete_test.go
@@ -0,0 +1,41 @@
+package controller
+
+import (
+	"reflect"
+	"testing"
+
+	"github.com/argoproj/gitops-engine/pkg/sync/common"
+	. "github.com/argoproj/gitops-engine/pkg/utils/testing"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+)
+
+func TestFilterObjectsForDeletion(t *testing.T) {
+	tests := []struct {
+		input []string
+		want  []string
+	}{
+		{[]string{"1", "5", "7", "7", "4"}, []string{"7", "7"}},
+		{[]string{"1", "5", "2", "2", "4"}, []string{"5"}},
+		{[]string{"1"}, []string{"1"}},
+		{[]string{}, []string{}},
+	}
+	for _, tt := range tests {
+		in := sliceOfObjectsWithSyncWaves(tt.input)
+		need := sliceOfObjectsWithSyncWaves(tt.want)
+		if got := FilterObjectsForDeletion(in); !reflect.DeepEqual(got, need) {
+			t.Errorf("Received unexpected objects for deletion = %v, want %v", got, need)
+		}
+	}
+}
+
+func podWithSyncWave(wave string) *unstructured.Unstructured {
+	return Annotate(NewPod(), common.AnnotationSyncWave, wave)
+}
+
+func sliceOfObjectsWithSyncWaves(waves []string) []*unstructured.Unstructured {
+	objects := make([]*unstructured.Unstructured, 0)
+	for _, wave := range waves {
+		objects = append(objects, podWithSyncWave(wave))
+	}
+	return objects
+}