argo-cd: Error constructing app status patch: unable to find api field in struct RawExtension for the json field \"autoscaling\"

Any target release planned ?

Is there any progress?

Yes, that’s my thinking as well. I’m working on the change for just this field, I should be ready soon.

On Mon, 18 Sept 2023, 20:01 Michael Crenshaw, @.***> wrote:

tbh I’m fairly concerned about changing the patch calculation mechanism in a patch release since it changes how every single reconciliation works.

I wonder if there’s an interim solution to avoid the problem for just this field, but leave the rest of the patch calculation untouched. Then we can switch over to the new mechanism entirely in 2.9.

— Reply to this email directly, view it on GitHub https://github.com/argoproj/argo-cd/issues/15126#issuecomment-1723997541, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABT5B62FWOUEUJGHDUCMPDX3B45ZANCNFSM6AAAAAA3YB66W4 . You are receiving this because you were mentioned.Message ID: @.***>

I managed to reproduce this consistently. This only happens if the status.sync.comparedTo contains an unquoted boolean. This is why the initial install works. But if you have an unquoted bool field, or you edit the Application and add an unquoted bool, then the Sync breaks.

Steps:

1. Create an app with Helm source and valuesObject.

apiVersion: argoproj.io/v1alpha1
kind: Application
metadata:
  name: grafana
  namespace: argocd
  finalizers:
    - resources-finalizer.argocd.argoproj.io
spec:
  project: default
  source:
    repoURL: https://grafana.github.io/helm-charts
    targetRevision: 6.58.8
    chart: grafana

    helm:
      valuesObject:
        persistence:
          enabled: true

  destination:
    server: https://kubernetes.default.svc
    namespace: default

  syncPolicy:
    automated:
      prune: true
      selfHeal: true

⚠️ It is important not to quote the boolean value.

1. After the app is installed and green, edit the file and disable persistence in Grafana:

    helm:
      valuesObject:
        persistence:
          enabled: false

⚠️ Again, it is important not to quote the boolean value.

1. Apply the file

Expected: App syncs and refreshes correctly.

Actual Result: The app refresh breaks. Go to the UI and hit Refresh. The Refresh button will be greyed out. The error is visible in the controller logs.

I’ve managed to also reproduce this by running Argo locally.

Strategicmerge shouldn’t be used for Unstructured data

This is an older issue also found in Helm, and fixed here. The fix is to use the json merge for Unstructured data, because it seems that the strategicmerge functionality isn’t meant to be used on CRDs or other unstructured inputs. I’ll work on a PR to address it this week.

Meanwhile, the workaround could be to quote all values, including numbers.

currently running on 2.8.5 and seeing this. Also seems to lock up refreshing and messes with application health status. I see that https://github.com/argoproj/argo-cd/pull/15227 hasn’t been merged yet so I imagine this is still an issue in 2.9.X? Are there any plans on getting that merged?

@crenshaw-dev added a PR, tests are passing, feedback is welcomed!

Brilliantly diagnosed. Looking forward to the PR, thanks @vladfr!

@PaulSonOfLars I’ve made the changes to use json for just the valuesObject. Like yourself, I too couldn’t see how to treat a single field separately, so I’m doing two patches, and merging. The end result is… well, if you have time, please take a look on #15227.

Meanwhile, for folks that want to use this now, we worked around the issue by using https://github.com/upsidr/importer to keep Helm values in a separate file.

We encounter the problem too. This prevents us from using the valuesObject, which would simplify our deployment-pipelines a lot

Yep, got some tests ready - will push later tonight,

Also adding support for Sources.Helm on top of Source.Helm; figure thats going to be the next issue report otherwise!

it’s not immediately obvious to me which changes are required to “only” calculate JSON patches for a single field

@PaulSonOfLars that’s a very fair concern. Having seem the updated patch, I worry less about merging it because the impact surface area is smaller. But I still want to understand the issue more thoroughly, and I don’t have time to do that just yet. If folks have time to investigate more, any additional findings would be appreciated.

Using application sets to generate applications, but even quoting in the applicationset template all the valuesObject values is not fix for us, as all the quotes disappear in generated Application manifest. So seems we have to stay with values: only multiline string.