diff --git a/docs/user-guide/sync-waves.md b/docs/user-guide/sync-waves.md index 932ba396d68d2..8b17237c87571 100644 --- a/docs/user-guide/sync-waves.md +++ b/docs/user-guide/sync-waves.md @@ -37,7 +37,7 @@ Hooks and resources are assigned to wave zero by default. The wave can be negati When Argo CD starts a sync, it orders the resources in the following precedence: * The phase -* The wave they are in (lower values first) +* The wave they are in (lower values first for creation & updation and higher values first for deletion) * By kind (e.g. [namespaces first and then other Kubernetes resources, followed by custom resources](https://github.com/argoproj/gitops-engine/blob/bc9ce5764fa306f58cf59199a94f6c968c775a2d/pkg/sync/sync_tasks.go#L27-L66)) * By name @@ -49,6 +49,8 @@ It repeats this process until all phases and waves are in-sync and healthy. Because an application can have resources that are unhealthy in the first wave, it may be that the app can never get to healthy. +During pruning of resources, resources from higher waves are processed first before moving to lower waves. If, for any reason, a resource isn't removed/pruned in a wave, the resources in next waves won't be processed. This is to ensure proper resource cleanup between waves. + Note that there's currently a delay between each sync wave in order give other controllers a chance to react to the spec change that we just applied. This also prevent Argo CD from assessing resource health too quickly (against the stale object), causing hooks to fire prematurely. The current delay between each sync wave is 2 seconds and can be configured via environment diff --git a/go.mod b/go.mod index b8acf2282cdb1..d781c91b47ee5 100644 --- a/go.mod +++ b/go.mod @@ -13,7 +13,7 @@ require ( github.com/TomOnTime/utfutil v0.0.0-20180511104225-09c41003ee1d github.com/alicebob/miniredis/v2 v2.30.4 github.com/antonmedv/expr v1.15.2 - github.com/argoproj/gitops-engine v0.7.1-0.20240122213038-792124280fcc + github.com/argoproj/gitops-engine v0.7.1-0.20240124052710-5fd9f449e757 github.com/argoproj/notifications-engine v0.4.1-0.20231027194313-a8d185ecc0a9 github.com/argoproj/pkg v0.13.7-0.20230626144333-d56162821bd1 github.com/aws/aws-sdk-go v1.44.317 diff --git a/go.sum b/go.sum index 78a085ec08a73..4c7aefc9e7fdf 100644 --- a/go.sum +++ b/go.sum @@ -694,8 +694,8 @@ github.com/apache/thrift v0.12.0/go.mod h1:cp2SuWMxlEZw2r+iP2GNCdIi4C1qmUzdZFSVb github.com/apache/thrift v0.13.0/go.mod h1:cp2SuWMxlEZw2r+iP2GNCdIi4C1qmUzdZFSVb+bacwQ= github.com/apache/thrift v0.16.0/go.mod h1:PHK3hniurgQaNMZYaCLEqXKsYK8upmhPbmdP2FXSqgU= github.com/appscode/go v0.0.0-20191119085241-0887d8ec2ecc/go.mod h1:OawnOmAL4ZX3YaPdN+8HTNwBveT1jMsqP74moa9XUbE= -github.com/argoproj/gitops-engine v0.7.1-0.20240122213038-792124280fcc h1:Fv94Mi2WvtvPkEH5WoWC3iy/VoQRLeSsE0hyg0n2UkY= -github.com/argoproj/gitops-engine v0.7.1-0.20240122213038-792124280fcc/go.mod h1:gWE8uROi7hIkWGNAVM+8FWkMfo0vZ03SLx/aFw/DBzg= +github.com/argoproj/gitops-engine v0.7.1-0.20240124052710-5fd9f449e757 h1:5fKAhTQcTBom0vin56cz/UTPx2GMuvdb+lJRAUOPbHA= +github.com/argoproj/gitops-engine v0.7.1-0.20240124052710-5fd9f449e757/go.mod h1:gWE8uROi7hIkWGNAVM+8FWkMfo0vZ03SLx/aFw/DBzg= github.com/argoproj/notifications-engine v0.4.1-0.20231027194313-a8d185ecc0a9 h1:1lt0VXzmLK7Vv0kaeal3S6/JIfzPyBORkUWXhiqF3l0= github.com/argoproj/notifications-engine v0.4.1-0.20231027194313-a8d185ecc0a9/go.mod h1:E/vv4+by868m0mmflaRfGBmKBtAupoF+mmyfekP8QCk= github.com/argoproj/pkg v0.13.7-0.20230626144333-d56162821bd1 h1:qsHwwOJ21K2Ao0xPju1sNuqphyMnMYkyB3ZLoLtxWpo= diff --git a/test/e2e/sync_waves_test.go b/test/e2e/sync_waves_test.go index ac5db15eee57d..8d0ee14e487d1 100644 --- a/test/e2e/sync_waves_test.go +++ b/test/e2e/sync_waves_test.go @@ -9,6 +9,8 @@ import ( "github.com/argoproj/gitops-engine/pkg/health" . "github.com/argoproj/gitops-engine/pkg/sync/common" + + v1 "k8s.io/api/core/v1" ) func TestFixingDegradedApp(t *testing.T) { @@ -100,3 +102,46 @@ func TestDegradedDeploymentIsSucceededAndSynced(t *testing.T) { Expect(SyncStatusIs(SyncStatusCodeSynced)). Expect(ResourceResultNumbering(1)) } + +// resources should be pruned in reverse of creation order(syncwaves order) +func TestSyncPruneOrderWithSyncWaves(t *testing.T) { + ctx := Given(t).Timeout(60) + + // remove finalizer to ensure proper cleanup if test fails at early stage + defer func() { + _, _ = RunCli("app", "patch-resource", ctx.AppQualifiedName(), + "--kind", "Pod", + "--resource-name", "pod-with-finalizers", + "--patch", `[{"op": "remove", "path": "/metadata/finalizers"}]`, + "--patch-type", "application/json-patch+json", "--all", + ) + }() + + ctx.Path("syncwaves-prune-order"). + When(). + CreateApp(). + // creation order: sa & role -> rolebinding -> pod + Sync(). + Wait(). + Then(). + Expect(SyncStatusIs(SyncStatusCodeSynced)). + Expect(HealthIs(health.HealthStatusHealthy)). + When(). + // delete files to remove resources + DeleteFile("pod.yaml"). + DeleteFile("rbac.yaml"). + Refresh(RefreshTypeHard). + IgnoreErrors(). + Then(). + Expect(SyncStatusIs(SyncStatusCodeOutOfSync)). + When(). + // prune order: pod -> rolebinding -> sa & role + Sync("--prune"). + Wait(). + Then(). + Expect(OperationPhaseIs(OperationSucceeded)). + Expect(SyncStatusIs(SyncStatusCodeSynced)). + Expect(HealthIs(health.HealthStatusHealthy)). + Expect(NotPod(func(p v1.Pod) bool { return p.Name == "pod-with-finalizers" })). + Expect(ResourceResultNumbering(4)) +} diff --git a/test/e2e/testdata/syncwaves-prune-order/README.md b/test/e2e/testdata/syncwaves-prune-order/README.md new file mode 100644 index 0000000000000..92a62fdfe109d --- /dev/null +++ b/test/e2e/testdata/syncwaves-prune-order/README.md @@ -0,0 +1,15 @@ +## Test Scenario + +This test example is for testing the reverse pruning of resources with syncwaves during sync operation. + +Resource creation happens in below order +- wave 0: sa & role +- wave 1: rolebinding +- wave 2: pod + +They are setup in such a way that the resources will be cleaned up properly only if they are deleted in the reverse order of creation i.e +- wave 0: pod +- wave 1: rolebinding +- wave 2: sa & role + +If above delete order is not followed the pod gets stuck in terminating state due to a finalizer which is supposed to be removed by k8s container lifecycle hook on delete if delete order is correct. \ No newline at end of file diff --git a/test/e2e/testdata/syncwaves-prune-order/pod.yaml b/test/e2e/testdata/syncwaves-prune-order/pod.yaml new file mode 100644 index 0000000000000..f801a3992aa37 --- /dev/null +++ b/test/e2e/testdata/syncwaves-prune-order/pod.yaml @@ -0,0 +1,41 @@ +apiVersion: v1 +kind: Pod +metadata: + name: pod-with-finalizers + annotations: + argocd.argoproj.io/sync-wave: "2" + # remove this finalizers using container preStop lifecycle hook on delete + finalizers: + - example.com/block-delete +spec: + serviceAccountName: modify-pods-sa # sa with permissions to modify pods + terminationGracePeriodSeconds: 15 + containers: + - name: container + image: nginx:alpine + command: ["/bin/sh", "-c"] + args: ["sleep 10h"] + env: + - name: POD_NAME + valueFrom: + fieldRef: + fieldPath: metadata.name + - name: NAMESPACE + valueFrom: + fieldRef: + fieldPath: metadata.namespace + lifecycle: + # remove finalizers for successful delete of pod + preStop: + exec: + command: + - /bin/sh + - -c + - | + set -e + + SERVICE_ACCOUNT_TOKEN=$(cat /var/run/secrets/kubernetes.io/serviceaccount/token) + POD_URL="https://kubernetes.default.svc/api/v1/namespaces/$NAMESPACE/pods/$POD_NAME" + PATCH_PAYLOAD='[{"op": "remove", "path": "/metadata/finalizers"}]' + + curl -k -v -H "Authorization: Bearer $SERVICE_ACCOUNT_TOKEN" -H "Content-Type: application/json-patch+json" -X PATCH --data "$PATCH_PAYLOAD" $POD_URL diff --git a/test/e2e/testdata/syncwaves-prune-order/rbac.yaml b/test/e2e/testdata/syncwaves-prune-order/rbac.yaml new file mode 100644 index 0000000000000..9512644b731db --- /dev/null +++ b/test/e2e/testdata/syncwaves-prune-order/rbac.yaml @@ -0,0 +1,37 @@ +apiVersion: v1 +kind: ServiceAccount +metadata: + name: modify-pods-sa + annotations: + argocd.argoproj.io/sync-wave: "0" +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: Role +metadata: + name: modify-pods-role + annotations: + argocd.argoproj.io/sync-wave: "0" +rules: + - apiGroups: [""] + resources: + - pods + verbs: + - get + - list + - delete + - update + - patch +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + name: modify-pods-rolebinding + annotations: + argocd.argoproj.io/sync-wave: "1" +subjects: + - kind: ServiceAccount + name: modify-pods-sa +roleRef: + kind: Role + name: modify-pods-role + apiGroup: rbac.authorization.k8s.io \ No newline at end of file