fix(server): Application sync fails when `CreateNamespace` and `ServerSideApply` flags are set in the sync options

anandf commented 10 months ago

Root Cause:

When server side apply is set to true, dry run is run in server mode, if the dry run fails in server mode, then it is re-run in client mode. But the server side apply is still set. This is not a supported configuration in kubectl when --dry-run is set to client, --server-side flag is not supported.

# kubectl apply -k kustomize-guestbook --dry-run=client --server-side -n guestbook -o json
error: --dry-run=client doesn't work with --server-side (did you mean --dry-run=server instead?)

Solution

When kubectl apply --dry-run=server --server-side fails, then re-run the dry run in client mode and with server-side flag removed as below


# kubectl apply -k kustomize-guestbook --dry-run=client -n guestbook -o json

codecov[bot] commented 10 months ago

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (aba3819) 54.47% compared to head (4357008) 54.47%.

Files	Patch %	Lines
pkg/sync/sync_context.go	66.66%	2 Missing :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #563 +/- ## ========================================== - Coverage 54.47% 54.47% -0.01% ========================================== Files 41 41 Lines 4793 4795 +2 ========================================== + Hits 2611 2612 +1 - Misses 1969 1970 +1 Partials 213 213 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

anandf commented 10 months ago

Steps to reproduce the error

Create the kind cluster
```
kind create cluster --name argocd
```

Install the 2.10.0-rc1 version of argocd in argocd namespace

kubectl create ns argocd && kubectl apply -f https://raw.githubusercontent.com/argoproj/argo-cd/v2.10.0-rc1/manifests/install.yaml -n argocd

Update the image containing the fix


kubectl config set-context --current --namespace argocd

argocd app create guestbook --core --repo https://github.com/anandf/argocd-example-apps.git \ --path kustomize-guestbook \ --dest-namespace guestbook \ --dest-server https://kubernetes.default.svc \ --sync-policy automated \ --self-heal \ --sync-option CreateNamespace=true \ --sync-option ServerSideApply=true

- List the application and notice the status and health of the application.

argocd app list --core


The argocd application `guestbook` will fail to sync and will have the sync failed status with message`namespace \'guestbook\' not found`

anandf commented 10 months ago

Steps to build the fix

Clone the argo-cd repo for branch release-2.10

git clone -b release-2.10 git@github.com:argoproj/argo-cd

Update the go.mod file to pick the gitops-engine library containing the fix and update the modules

go mod edit -replace github.com/argoproj/gitops-engine=github.com/anandf/gitops-engine@b39d57e63236b3f430a8b229514f76a5a53f5d46
go mod tidy

Build the argocd image and push it to quay or docker>

export QUAY_USER=<quay_userid>
IMAGE_PREFIX=quay.io/$QUAY_USER/ make image && docker push quay.io/$QUAY_USER/argocd:latest

Steps to apply the fix and validate

Delete and recreate the kind cluster

kind delete cluster --name argocd
kind create cluster --name argocd

Install the 2.10.0-rc1 version of ArgoCD in argocd namespace

kubectl create ns argocd && kubectl apply -f https://raw.githubusercontent.com/argoproj/argo-cd/v2.10.0-rc1/manifests/install.yaml -n argocd

Update the image containing the fix

kubectl set image -n argocd statefulset argocd-application-controller \*=quay.io/$QUAY_USER/argocd:latest

Wait for the rollout to complete and create the application

kubectl rollout status statefulset/argocd-application-controller -n argocd

Creat the application with syncOptions CreateNamespace=true and ServerSideApply=true as below
```
kubectl config set-context --current --namespace argocd
```

argocd app create guestbook --core --repo https://github.com/anandf/argocd-example-apps.git \ --path kustomize-guestbook \ --dest-namespace guestbook \ --dest-server https://kubernetes.default.svc \ --sync-policy automated \ --self-heal \ --sync-option CreateNamespace=true \ --sync-option ServerSideApply=true

- List the application and ensure that the application gets synced and reach an healthy state.

argocd app list --core

pasha-codefresh commented 10 months ago

@anandf thank you for your PR. Last time when i have fixed this issue https://github.com/argoproj/argo-cd/issues/16177 in these PRs https://github.com/argoproj/gitops-engine/pull/548 https://github.com/argoproj/gitops-engine/pull/546

In high level Argocd always used client side dry run for both, client and server apply. I have noticed only issue with hooks that i described originally and for compatibility we decided not to change how it worked before if error occur. Before my fix it always took server side apply with client side dry run.

Could you please confirm that before my changes you were not able reproduce this issue? Because with your change you also going to change how you applying resources and customer even will not know that they doing it with client side apply.

Also important to say that this issue was opened before my fix https://github.com/argoproj/argo-cd/issues/13874

So looks like it is just another not covered case with ServerSideApply

Thanks!

leoluz commented 10 months ago

Could you please confirm that before my changes you were not able reproduce this issue? Because with your change you also going to change how you applying resources and customer even will not know that they doing it with client side apply.

@pasha-codefresh This change will only affect the sync during dryrun with strategy DryRunServer. It doesn't seem to change how Argo CD actually applies the resource in the cluster. Can you please take another look in his changes and confirm?

pasha-codefresh commented 10 months ago

Could you please confirm that before my changes you were not able reproduce this issue? Because with your change you also going to change how you applying resources and customer even will not know that they doing it with client side apply.

@pasha-codefresh This change will only affect the sync during dryrun with strategy DryRunServer. It doesn't seem to change how Argo CD actually applies the resource in the cluster. Can you please take another look in his changes and confirm?

Yes, you are right , lgtm