argoproj / argo-cd

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

Adding a namespace to a non-namespaced resource results in duplicate resources #18669

Open sthomson-wyn opened 5 months ago

sthomson-wyn commented 5 months ago

Checklist:

Describe the bug

When creating a Cluster scoped resource (Like a ClusterRole or ClusterRoleBinding), the application tracks the resource as 2 distinct resources. Due to this, argocd app get incorrectly reports them as having a "Running" sync status

To Reproduce

Add a ClusterRole to an application that also has a namespace

Expected behavior

I think ideally ArgoCD would just ignore the namespace?

Screenshots

image image image (namespace hidden)

Since the tree output doesn't have the namespaces, only one shows up https://github.com/argoproj/argo-cd/blob/48eb7f3608b129c299f8e1a8eddbda2971c8f94e/cmd/argocd/commands/tree.go#L33 image

Version

argocd: v2.11.3+3f344d5
  BuildDate: 2024-06-06T12:31:44Z
  GitCommit: 3f344d54a4e0bbbb4313e1c19cfe1e544b162598
  GitTreeState: clean
  GoVersion: go1.22.4
  Compiler: gc
  Platform: linux/amd64
argocd-server: v2.10.2+fcf5d8c
  BuildDate: 2024-03-01T21:24:51Z
  GitCommit: fcf5d8c2381b68ab1621b90be63913b12cca2eb7
  GitTreeState: clean
  GoVersion: go1.21.3
  Compiler: gc
  Platform: linux/amd64
  Kustomize Version: v5.2.1 2023-10-19T20:13:51Z
  Helm Version: v3.14.2+gc309b6f
  Kubectl Version: v0.26.11
  Jsonnet Version: v0.20.0
T1-leiyang commented 5 months ago

/assign

agaudreault commented 5 months ago

@sthomson-wyn I am having a hard time to understand your issue with the partial screenshots. So let me try to summarize and correct me if I'm wrong! You have a manifest in your application that is a ClusterRole like the one below. When you sync this resource in an application, it is returned twice in the argocd app get command?

apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
  name: storefront
rules:
- apiGroups: [""]
  resources: ["example"]
  verbs: ["get", "watch", "list"]
thecooldrop commented 5 months ago

@sthomson-wyn Does the duplicate resource appear in UI as well?

sthomson-wyn commented 5 months ago

@agaudreault When the resource has a namespace defined

apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
  name: storefront
  namespace: some-namespace
rules:
- apiGroups: [""]
  resources: ["example"]
  verbs: ["get", "watch", "list"]

It appears twice from the argocd app get command, but not in the UI @thecooldrop

thecooldrop commented 4 months ago

I would like to work on this issue.

thecooldrop commented 4 months ago

So here is the rundown of the cause of the bug:

During printing the CLI looks up the resources to print at two locations. Firstly the code looks up all of the resources which synced in last sync from Application.status.operationState.syncResult.resources and from Application.status.resources. The code currently tries to keep track of resources which were already printed from Application.status.operationState.syncResult.resources in order to avoid printing them again from the second source. This fails currently because the cluster-level resources in the first source have namespace set, but in second source they do not have namespace set, and thus the key by which they are tracked is different.

In my opinion this is a lower-level issue, namely the question is why is there a difference in how the resources are maintained in two different places within same status. I have found the place in code which writes the Application.status.operationState.syncResult.resources, and it is within git-ops engine library, and is commented explicitly as intended behavior:

https://github.com/argoproj/gitops-engine/blob/master/pkg/sync/sync_context.go on lines 686 to 700


// enrich target objects with the namespace
for _, task := range tasks {
if task.targetObj == nil {
continue
}
    if task.targetObj.GetNamespace() == "" {
        // If target object's namespace is empty, we set namespace in the object. We do
        // this even though it might be a cluster-scoped resource. This prevents any
        // possibility of the resource from unintentionally becoming created in the
        // namespace during the `kubectl apply`
        task.targetObj = task.targetObj.DeepCopy()
        task.targetObj.SetNamespace(sc.namespace)
    }
}


I still have to find the code which populates the `Application.status.resources` in order to check why there is a difference and if that is valid behaviour.
thecooldrop commented 3 months ago

Turns out that this bug is bigger than though and affects all methods which call waitOnApplicationStatus function. Currently affected are following commands: app rollback, app sync, app wait and app get.

I will be fixing all the places in this bug

andrii-korotkov-verkada commented 3 weeks ago

ArgoCD versions 2.10 and below have reached EOL. Can you upgrade and let us know if the issue is still present, please?

thecooldrop commented 3 weeks ago

ArgoCD versions 2.10 and below have reached EOL. Can you upgrade and let us know if the issue is still present, please?

Hello @andrii-korotkov-verkada , the issue is most likely persistent, since upon assuming the responsibility for this issue I tried it out on main branch. Further the fix is made on main branch. I will check again though to be certain, since the actual problem is caused a bit deeper, then where I have made the workaround. Namely the gitops-engine and ArgoCD handle the namespaces for cluster-scoped resources differently. I have described this behavior in more detail above.