argoproj / argo-cd

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

ArgoCD ignores all labels in yaml if one of the label use boolean value #10489

Open shaoxt opened 2 years ago

shaoxt commented 2 years ago
apiVersion: "autoscaling.k8s.io/v1"
kind: VerticalPodAutoscaler
metadata:
  labels:
    jvm: true
    app: "all_label_will_be_missing"
  name: test-vpa
spec:
  targetRef:
    apiVersion: argoproj.io/v1alpha1
    kind: Rollout
    name:  test-rollout
  resourcePolicy:
    containerPolicies:
      - containerName: 'app'
        minAllowed:
          cpu: 500m
          memory: 1Gi
        maxAllowed:
          cpu: 2
          memory: 2Gi
        controlledResources: ["cpu", "memory"]

Checklist:

Describe the bug

To Reproduce

Expected behavior

Screenshots

Version

Paste the output from `argocd version` here.

Logs

Paste any relevant application logs here.
leoluz commented 2 years ago

*Update: This was in reply to the original issue description where it was suggested a possible ArgoCD bug that VerticalPodAutoscaler label would be applied correctly


It doesn't seem to be a general issue with VPA as I wasn't able to reproduce locally.

apiVersion: autoscaling.k8s.io/v1
kind: VerticalPodAutoscaler
metadata:
  annotations:
    kubectl.kubernetes.io/last-applied-configuration: >
      {"apiVersion":"autoscaling.k8s.io/v1","kind":"VerticalPodAutoscaler","metadata":{"annotations":{},"labels":{"app":"this_label_will_be_missing","app.kubernetes.io/instance":"nginx"},"name":"test-vpa","namespace":"default"},"spec":{"resourcePolicy":{"containerPolicies":[{"containerName":"nginx","controlledResources":["cpu","memory"],"maxAllowed":{"cpu":2,"memory":"2Gi"},"minAllowed":{"cpu":"500m","memory":"1Gi"}}]},"targetRef":{"apiVersion":"apps/v1","kind":"Deployment","name":"nginx-deployment"}}}
  creationTimestamp: '2022-09-01T13:43:14Z'
  generation: 2
  labels:
    app: this_label_will_be_missing     <<< The label was correctly applied
    app.kubernetes.io/instance: nginx
  name: test-vpa
  namespace: default
  resourceVersion: '5499686'
  uid: aec52c16-522e-44b0-95f4-cc2960a9acf4
spec:
  resourcePolicy:
    containerPolicies:
      - containerName: nginx
        controlledResources:
          - cpu
          - memory
        maxAllowed:
...

I also tried configuring ArgoCD locally with the same application label key as defined by the user with:

application.instanceLabelKey: applications.argoproj.io/app-name

Even with that, I am not able to reproduce the issue.

LinAnt commented 2 years ago

Labels need to be strings, ie 'yes', '1' or 'true' works

I think argocd should error out here, instead of dropping invalid labels.

Thoughts?

jdoylei commented 1 year ago

@LinAnt - thanks for the explanation. E.g. in an Application manifest YAML file loaded by an app-of-apps, the label:

It would be good if Argo CD were consistent with e.g. kubectl apply and errored out:

error: unable to decode "src/app-of-apps/test-labels/application.labeled-app.yaml": resource.metadataOnlyObject.ObjectMeta: v1.ObjectMeta.Labels: ReadString: expects " or n, but found t, error found in #10 byte of ...|secrets":true},"name|..., bigger context ...|com/environment":"dbd","uses-foundation-secrets":true},"name":"labeled-app","namespace":"argocd"},"s|...

Otherwise, you may be defining Applications via app-of-apps but not realize they are not valid Kubernetes manifests, and of course, may miss the fact that your labels are missing on some of your apps.