argoproj / argo-cd

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

Label CRD Resources by Default or Conditionally #18565

Closed acudovs closed 2 months ago

acudovs commented 3 months ago

Summary

Currently, CustomResourceDefinition (CRD) resources are not labeled by ArgoCD, which leads to SharedResourceWarning issues when managing CRDs. This issue suggests that CRD resources should be labeled by default or conditionally, similar to other ArgoCD managed resources.

https://github.com/argoproj/argo-cd/blob/3703a1ee/cmd/argocd/commands/app.go#L1358

Motivation

In scenarios where CRDs are part of Application, not labeling them correctly causes SharedResourceWarning issues. For example, when installing cert-manager CRDs they are managed by both argocd/my-cert-manager and cert-manager applications. Proper labeling of CRDs will ensure consistent management and reduce conflicts and warnings, making it easier to track and manage CRDs within the ArgoCD environment.

Proposal

To address the issue, CRDs should be labeled by default or conditionally based on user configurations.

Questions

Why CDR resources are not labeled? Can you consider labeling CDRs by default or conditionally?

agaudreault commented 3 months ago

@acudovs Just to clarify, you mention

Currently, CustomResourceDefinition (CRD) resources are not labeled by ArgoCD, which leads to SharedResourceWarning issues when managing CRDs.

Does that mean you currently have SharedResourceWarning?

I think that is the expected behavior. When a CRD is managed by two application and the CRD is different, if the two applications use a different version, the version of the CRD will constantly change based on which application was sync the last. SharedResourceWarning with FailOnSharedResource will prevent that behavior.

acudovs commented 3 months ago

All resources, including CRDs, are managed by a single ArgoCD application named "my-cert-manager".

Let's consider the Deployment resource: https://github.com/cert-manager/cert-manager/blob/master/deploy/charts/cert-manager/templates/deployment.yaml#L9

Originally, {{ .Release.Name }} is set to cert-manager. However, ArgoCD overrides app.kubernetes.io/instance: {{ .Release.Name }} to app.kubernetes.io/instance: my-cert-manager, so all resources become part of the ArgoCD application.

An exception to this is the CRDs: https://github.com/cert-manager/cert-manager/blob/master/deploy/crds/crd-certificaterequests.yaml#L14

For CRDs, {{ .Release.Name }} is not overridden by ArgoCD and remains cert-manager (the Chart name). As a result, ArgoCD displays a SharedResourceWarning in the UI.

It seems FailOnSharedResource does not applicable in this case, since there is a single ArgoCD application but different app.kubernetes.io/instance label values.

agaudreault commented 3 months ago

@acudovs can you share your application definition? When using helm, Argo will use the helm.releaseName from the config, or it will default to the application name. Since the label app.kubernetes.io/instance: '{{ .Release.Name }}' is part of the helm template and Argo will not modify this label to use the application name, the value used will be the releaseName.

Is it possible that you specify a releaseName in the applicationSpec that is different to the application Name?

I added this issue to the next contributor meeting to discuss and document why CRDs are not tracked.

You could switch to the newer tracking method to resolve this issue, or specify the application name as the release name.

acudovs commented 3 months ago

Here is the ArgoCD Application definition

apiVersion: argoproj.io/v1alpha1
kind: Application
metadata:
  name: my-cert-manager
  namespace: argocd
  labels:
    app: cert-manager
    env: my
  finalizers:
    - resources-finalizer.argocd.argoproj.io
spec:
  project: cert-manager
  source:
    repoURL: https://gitlab.example.com/helm/cert-manager.git
    targetRevision: master
    path: charts/v1.15.0
    helm:
      releaseName: cert-manager
      valueFiles:
        - ./environments/all/values.yaml
        - ./environments/my/values.yaml
  destination:
    server: https://k8s.example.com
    namespace: cert-manager
  syncPolicy:
    syncOptions:
      - CreateNamespace=true

By default, helm.releaseName is set to the Helm Chart's default value of "cert-manager." If I change helm.releaseName to "my-cert-manager," all associated resources will be renamed accordingly, and the CRDs will be labeled correctly. For instance, the default Deployment name is "cert-manager," and the pod name follows the pattern "cert-manager-xxx." After changing the release name, the Deployment name will become "my-cert-manager," and the pod name will be "my-cert-manager-xxx."