argoproj / argo-cd

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

2.4 -> 2.6.7 omitted fields are now breaking diffs #13004

Open sandoichi opened 1 year ago

sandoichi commented 1 year ago

Checklist:

Describe the bug

I just upgraded from ArgoCD 2.4.x to 2.6.7 and existing apps are now constantly seeing out of sync statuses because fields that are omitted in the manifest yaml files, even in known types like Deployments, are showing up in the live manifest with their default value and causing the diff to see that this is out of sync.

For example, if a deployment's spec.template.spec.containers[*].resources field is omitted, the container will show up live with resources: {} and cause the resource to be seen as out of sync. If I revert to argo-cd 2.4, this exact same app with nothing else changed will ignore the diff.

To Reproduce Build an app that renders a built-in k8s deployment resource which has a container that omits the resources field.

Sync app and see that the app is healthy and synced.

Render the same app in argocd 2.6.7 and the container's defaulted resources: {} field will now show up in the diff as out of sync, and the app will never see itself as up to date.

Expected behavior I expected the syncing behavior in regards to defaulted fields that got omitted/known fields to still function in 2.6.7 as it did in 2.4

Version

argocd: v2.6.7+5bcd846
  BuildDate: 2023-03-23T15:24:49Z
  GitCommit: 5bcd846fa16e4b19d8f477de7da50ec0aef320e5
  GitTreeState: clean
  GoVersion: go1.18.10
  Compiler: gc
  Platform: linux/amd64
argocd-server: v2.6.7+5bcd846

I checked the migration docs from 2.4 -> 2.5 and 2.5 -> 2.6 but didn't see anything relevant. Is there some new configuration that I need to enable to get the old behavior back? If not, it looks like I would have to either patch every upstream helm chart I use in my apps to always template out every single field, or explicitly ignore everything in diffs; both of which would not be great.

Could there be something different in the latest argocd that causes omitted fields to be explicitly created when manifests are being applied? I tried both server-side apply and client-side apply; no difference.

Thanks.

sandoichi commented 1 year ago

Could this be related to how argocd handles diffing now? I saw in the changes that in 2.5, local server-side diffing became the default, but does this mean that argocd server itself will use server-side diffing now? If so, that maybe could explain why all of a sudden the diffs are seeing fields that get defaulted after applying?

irizzant commented 1 year ago

I'm hitting the same with Prometheus metric relabelings and ArgoCD 2.6.7.

I added server side apply and all of a sudden I started getting the app out-of-sync because of metric relabelings. The default action for relabelings is "replace" and ArgoCD tries to remove this field from the live manifest.

pgier commented 1 year ago

What sync options are you using in your Application and on the specific resources (if any)?

sandoichi commented 1 year ago

What sync options are you using in your Application and on the specific resources (if any)?

- ServerSideApply=true
- CreateNamespace=true
- Replace=true

... though i'm not sure if server-side apply is actually working in 2.4 so that setting is might not be doing anything in my case

pgier commented 1 year ago

... though i'm not sure if server-side apply is actually working in 2.4 so that setting is might not be doing anything in my case

ServerSideApply was added in 2.5, so I don't think it's doing anything in 2.4. In addition, according to the docs, Replace takes precedence over ServerSideApply in the current version, so I'm wondering if it's not taking effect in 2.6 either. Do you still get the same bad diffs if you remove Replace=true in 2.6?

pgier commented 1 year ago

I ran into the same issue using the current 2.7.9, and these two sync options in my Application:

        syncOptions:
        - ServerSideApply=true
        - CreateNamespace=true

I'm using the external-secrets operator and created an ExternalSecret which specifies a key value, but no values for conversionStrategy and decodingStrategy. So I guess the operator or the API server is adding default values for those.

Live manifest

  dataFrom:
    - extract:
        conversionStrategy: Default
        decodingStrategy: None
        key: my-key-name

Desired manifest

  dataFrom:
    - extract:
        key: my-key-name

However, I didn't see the issue with empty resources objects in deployments. So that diff might be fixed in 2.7, or I have something else different in my settings somewhere.

pgier commented 1 year ago

Looks like this is the issue that I'm hitting: #11139

cardoe commented 1 week ago

I see this as well with ExternalSecrets and those same fields on Argo CD 2.11.2