argoproj / argo-cd

Declarative Continuous Deployment for Kubernetes
https://argo-cd.readthedocs.io
Apache License 2.0
17.98k stars 5.47k forks source link

Application CRD `status.resources` do not reflect `argocd.argoproj.io/sync-options: Prune=false` #17188

Open thesuperzapper opened 9 months ago

thesuperzapper commented 9 months ago

Checklist:

Describe the bug

The argocd.argoproj.io/sync-options: Prune=false annotations are not respected by the Application controller.

The application controller lists these resources as requiresPruning: true in the CRD status.resources[] field. This is a problem in two significant places:

  1. The ArgoCD UI:
    • The UI will always show these resources in the diff, even if they would not be removed during an actual sync.
    • NOTE: this is also true if argocd.argoproj.io/compare-options: IgnoreExtraneous is set on the resource.
  2. The ArgoCD CLI:
    • When a user runs a argocd app get -o json command.

To Reproduce

  1. Create an ArgoCD application called my-app (it doesn't matter what is in it):
apiVersion: argoproj.io/v1alpha1
kind: Application
metadata:
  name: guestbook
  namespace: argocd
spec:
  project: default
  source:
    repoURL: https://github.com/argoproj/argocd-example-apps.git
    targetRevision: HEAD
    path: guestbook
  destination:
    server: https://kubernetes.default.svc
    namespace: default
  1. Create a ConfigMap that shares the app.kubernetes.io/instance label with the ArgoCD app and argocd.argoproj.io/sync-options: Prune=false annotation:
apiVersion: v1
kind: ConfigMap
metadata:
  name: my-configmap
  namespace: default
  annotations:
    argocd.argoproj.io/sync-options: Prune=false
  labels:
    app.kubernetes.io/instance: guestbook
data:
  SOME_KEY: "SOME_VALUE"
  1. Observe that the argocd app get command shows the ConfigMap as requiresPruning: true:
ARGOCD_NAMESPACE="argocd"
ARGOCD_USERNAME="admin"
ARGOCD_PASSWORD=$(kubectl -n "argocd" get secret "argocd-initial-admin-secret" -o jsonpath="{.data.password}" | base64 -d)

# log in to argocd
export ARGOCD_OPTS="--port-forward --port-forward-namespace '${ARGOCD_NAMESPACE}'"
argocd login --username "${ARGOCD_USERNAME}" --password "${ARGOCD_PASSWORD}"

# get the application as JSON
_app_json=$(argocd app get "guestbook" -o json --refresh)

# extract the `status.resources` array from the JSON
_app_resources=$(echo "$_app_json" | jq -c '.status.resources[]?')

# print the resources
echo "$_app_resources"

# OUTPUT:
# {"version":"v1","kind":"ConfigMap","namespace":"default","name":"my-configmap","status":"OutOfSync","requiresPruning":true}
# {"version":"v1","kind":"Service","namespace":"default","name":"guestbook-ui","status":"Synced","health":{"status":"Healthy"}}
# {"group":"apps","version":"v1","kind":"Deployment","namespace":"default","name":"guestbook-ui","status":"Synced","health":{"status":"Healthy"}}
  1. Observe that the UI shows the resource as requiring pruning:

Screenshot 2024-02-12 at 12 34 18

Expected behavior

Both the CRD itself (and therefore ArgoCD CLI and UI) correctly reflect the presence of argocd.argoproj.io/sync-options: Prune=false.

For example, in the above situation, I would expect to see the status.resources like this:

apiVersion: argoproj.io/v1alpha1
kind: Application
metadata:
  name: guestbook
spec:
  ...
status:
  ...
  resources:
    - version: v1
      kind: ConfigMap
      name: my-configmap
      namespace: default

      ## \/ this part should show as `false`, not `true`  \/ 
      requiresPruning: false

      ## NOTE: this will probably still need to be OutOfSync, until we can create a new "ignored" status:
      ##       https://github.com/argoproj/argo-cd/issues/14338
      status: OutOfSync

Version

argocd: v2.10.0+2175939.dirty
  BuildDate: 2024-02-06T15:31:09Z
  GitCommit: 2175939ed6156ddd743e60f427f7f48118c971bf
  GitTreeState: dirty
  GoVersion: go1.21.6
  Compiler: gc
  Platform: darwin/arm64

Other Notes

There is a similar issue with argocd.argoproj.io/compare-options: IgnoreExtraneous (see https://github.com/argoproj/argo-cd/issues/14338), but that will be more complex to fix, because it probably requires a new "resource status" of "ignored" as it is not correct to say that an ignored resource is "Synced".

thesuperzapper commented 9 months ago

@crenshaw-dev @gdsoumya @ishitasequeira @fengshunli I think this is an important bug to fix, and I am interested in your feedback.

thesuperzapper commented 9 months ago

Is it still the same if the configmap is present in the git source itself instead of being applied manually?

@gdsoumya I don't think that makes sense, because if the ConfigMap was in the git source, it would not require pruning.

And if it were previously in the git source (but was removed), then it would be functionally equivalent to creating the ConfigMap manually with the same app.kubernetes.io/instance label like we do in the example.

EDIT: looks like the comment I was replying to from @gdsoumya got deleted, but I will leave this comment up.

thesuperzapper commented 9 months ago

@pasha-codefresh as this is probably related to argoproj/gitops-engine, I am interested to know your feedback.

Also, I found issue https://github.com/argoproj/argo-cd/issues/8134, which shows that this issue has been impacting users since at least Jan 2022, because argocd app sync will incorrectly fail 100% of the time when there are resources with argocd.argoproj.io/sync-options: Prune=false.

thesuperzapper commented 7 months ago

@crenshaw-dev @ishitasequeira @pasha-codefresh @kostis-codefresh

This is a very important user issue with a relatively simple fix. We need to make it so that the status.resources correctly shows requiresPruning: false when a resource has argocd.argoproj.io/sync-options: Prune=false.

For most users, the biggest impact of the current issue is the ArgoCD UI will show resources with Prune=false as if they are going to be removed in the application diff, and it will make the entire app show as "out of sync".


There is a related issue https://github.com/argoproj/argo-cd/issues/14338 for the argocd.argoproj.io/compare-options: IgnoreExtraneous annotation, but that is slightly more complex, as it will require a new "Ignored" resource status in addition to the "Synced" and "OutOfSync".

crenshaw-dev commented 7 months ago

At a quick glance, what you're saying sounds reasonable. I'm inclined to accept that change.

Talador12 commented 7 months ago

I agree with @crenshaw-dev and @thesuperzapper

cezarsa commented 2 months ago

@thesuperzapper Are you already working on a PR for that behavior change? If not, I should be able to give this a try in the coming weeks.

thesuperzapper commented 2 months ago

@thesuperzapper Are you already working on a PR for that behavior change? If not, I should be able to give this a try in the coming weeks.

@cezarsa actually yes, I have an almost working prototype, I ended up approaching it slightly differently.

The key difference to what I proposed above is that I leave the requiresPruning as true, and added a new pruningDisabled field which is then used by the UI to display is differently.

This is because I still want to actually show these in the UI (with an optional filter on the left), because it's not correct to say that these resources don't "require pruning", and setting it to false just causes it to show as "in sync". It's better if we have a special icon to indicate that we aren't going to actually remove it, but that it is actually extraneous (not in the "desired" state).

I'm also planning to add an ignoreExtraneous field because that concept is separate from disabling pruning, and we can just gray out the icon to indicate that "out of sync"-ness is not propagating to the root app.

andrii-korotkov-verkada commented 1 week ago

@thesuperzapper, are you still working on the PR?

SuperCoolAlan commented 1 day ago

I am seeing similar behavior where an annotated secret is still trying to be pruned by Argo sync.

apiVersion: v1
kind: Secret
metadata:
  name: my-secret
  namespace: my-namespace
  annotations:
    argocd.argoproj.io/sync-options: Prune=false
  labels:
    argocd.argoproj.io/instance: my-argo-app
    createdby: initcontainer
  selfLink: /api/v1/namespaces/my-namespace/secrets/my-secret
data:
  key: <base64 encoded value>
type: Opaque

I am using an old version of Argo

{
    "Version": "v2.8.5+85025e1",
    "BuildDate": "2023-10-27T23:36:30Z",
    "GitCommit": "85025e1dcb683b192ea3599de0b0a196d64c94a7",
    "GitTreeState": "clean",
    "GoVersion": "go1.20.10",
    "Compiler": "gc",
    "Platform": "linux/amd64",
    "KustomizeVersion": "v5.1.0 2023-06-19T16:58:18Z",
    "HelmVersion": "v3.12.1+gf32a527",
    "KubectlVersion": "v0.24.2",
    "JsonnetVersion": "v0.20.0"
}
thesuperzapper commented 12 hours ago

I have raised a PR to fix this issue among other things: