Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[appset in any namespace] ApplicationSet with same name in different namespaces conflict #16207

Closed
3 tasks done
speedfl opened this issue Nov 1, 2023 · 8 comments · Fixed by #16222
Closed
3 tasks done
Labels
apps-in-any-namespace Issues related to the "Apps in any namespace" feature introduced in 2.5 bug Something isn't working component:application-sets Bulk application management related

Comments

@speedfl
Copy link
Contributor

speedfl commented Nov 1, 2023

Checklist:

  • I've searched in the docs and FAQ for my answer: https://bit.ly/argocd-faq.
  • I've included steps to reproduce the bug.
  • I've pasted the output of argocd version.

Describe the bug

When creating two ApplicationSet with same name in two different source namespaces. The name conflict in the applicationset-controller and the source namespace for the application is not taken into account resulting in infinite loop deleting child applications and re-triggering reconcile loop. Logs attached

Quick fix: Ensure there are no name conflicting

To Reproduce

Expected behavior

Screenshots

Version

{
    "Version": "v2.8.4+c279299",
    "BuildDate": "2023-09-13T19:12:09Z",
    "GitCommit": "c27929928104dc37b937764baf65f38b78930e59",
    "GitTreeState": "clean",
    "GoVersion": "go1.20.6",
    "Compiler": "gc",
    "Platform": "linux/amd64",
    "KustomizeVersion": "v5.1.0 2023-06-19T16:58:18Z",
    "HelmVersion": "v3.12.1+gf32a527",
    "KubectlVersion": "v0.24.2",
    "JsonnetVersion": "v0.20.0"
}

Logs

time="2023-11-01T15:44:12Z" level=info msg="Deleted application" app=my-app-one appSet=my-application-set
time="2023-11-01T15:44:12Z" level=info msg="created Application" app=my-app-one appSet=my-application-set
time="2023-11-01T15:44:13Z" level=info msg="unchanged Application" app=my-app-one appSet=my-application-set
@speedfl speedfl added bug Something isn't working component:application-sets Bulk application management related apps-in-any-namespace Issues related to the "Apps in any namespace" feature introduced in 2.5 labels Nov 1, 2023
@speedfl speedfl assigned speedfl and unassigned speedfl Nov 1, 2023
@speedfl
Copy link
Contributor Author

speedfl commented Nov 1, 2023

Maybe a simple error in the applicationset conditions if conflict name are found could be a suitable solution.

Starting my investigation.

@speedfl
Copy link
Contributor Author

speedfl commented Nov 1, 2023

After looking deeper it seems that createInCluster and deleteInCluster are not taking into account appset namespace:

Looking at the other impacts

@reedjosh
Copy link

reedjosh commented Nov 1, 2023

#16114

@speedfl
Copy link
Contributor Author

speedfl commented Nov 1, 2023

Looking at the code I don't think createOrUpdate will cause issues as it relies on ObjectKeyFromObject‎ which is using namespace and name.
I will write an integration test to validate the fix for createInCluster and deleteInCluster. Purpose of the fix is to use the objectKey for the maps

@speedfl
Copy link
Contributor Author

speedfl commented Nov 2, 2023

I reproduced a sample usecase that can be run locally using make start-local.
Looking now at the fix.

https://github.com/speedfl/argocd-example-apps/tree/feature/appset-name-conflict/appset-name-conflict

@speedfl
Copy link
Contributor Author

speedfl commented Nov 2, 2023

I tested a fix. I will push an MR soon

@speedfl
Copy link
Contributor Author

speedfl commented Nov 2, 2023

The fix has been tested locally. Having some issues in writting e2e. Continuing my investigation

@speedfl
Copy link
Contributor Author

speedfl commented Nov 2, 2023

I added an e2e test. It was not so easy to update the environment to support another external namespace.
I let reviewers take a look to it

ishitasequeira added a commit that referenced this issue Nov 6, 2023
* fix(16207): Fix name conflict in appset controller

Signed-off-by: gmuselli <geoffrey.muselli@gmail.com>

* fix(16207): e2e

Signed-off-by: gmuselli <geoffrey.muselli@gmail.com>

* Update test/e2e/fixture/applicationsets/utils/fixture.go

Signed-off-by: Ishita Sequeira <46771830+ishitasequeira@users.noreply.github.com>

---------

Signed-off-by: gmuselli <geoffrey.muselli@gmail.com>
Signed-off-by: Ishita Sequeira <46771830+ishitasequeira@users.noreply.github.com>
Co-authored-by: Ishita Sequeira <46771830+ishitasequeira@users.noreply.github.com>
jmilic1 pushed a commit to jmilic1/argo-cd that referenced this issue Nov 13, 2023
…rgoproj#16222)

* fix(16207): Fix name conflict in appset controller

Signed-off-by: gmuselli <geoffrey.muselli@gmail.com>

* fix(16207): e2e

Signed-off-by: gmuselli <geoffrey.muselli@gmail.com>

* Update test/e2e/fixture/applicationsets/utils/fixture.go

Signed-off-by: Ishita Sequeira <46771830+ishitasequeira@users.noreply.github.com>

---------

Signed-off-by: gmuselli <geoffrey.muselli@gmail.com>
Signed-off-by: Ishita Sequeira <46771830+ishitasequeira@users.noreply.github.com>
Co-authored-by: Ishita Sequeira <46771830+ishitasequeira@users.noreply.github.com>
Signed-off-by: jmilic1 <70441727+jmilic1@users.noreply.github.com>
vladfr pushed a commit to vladfr/argo-cd that referenced this issue Dec 13, 2023
…rgoproj#16222)

* fix(16207): Fix name conflict in appset controller

Signed-off-by: gmuselli <geoffrey.muselli@gmail.com>

* fix(16207): e2e

Signed-off-by: gmuselli <geoffrey.muselli@gmail.com>

* Update test/e2e/fixture/applicationsets/utils/fixture.go

Signed-off-by: Ishita Sequeira <46771830+ishitasequeira@users.noreply.github.com>

---------

Signed-off-by: gmuselli <geoffrey.muselli@gmail.com>
Signed-off-by: Ishita Sequeira <46771830+ishitasequeira@users.noreply.github.com>
Co-authored-by: Ishita Sequeira <46771830+ishitasequeira@users.noreply.github.com>
tesla59 pushed a commit to tesla59/argo-cd that referenced this issue Dec 16, 2023
…rgoproj#16222)

* fix(16207): Fix name conflict in appset controller

Signed-off-by: gmuselli <geoffrey.muselli@gmail.com>

* fix(16207): e2e

Signed-off-by: gmuselli <geoffrey.muselli@gmail.com>

* Update test/e2e/fixture/applicationsets/utils/fixture.go

Signed-off-by: Ishita Sequeira <46771830+ishitasequeira@users.noreply.github.com>

---------

Signed-off-by: gmuselli <geoffrey.muselli@gmail.com>
Signed-off-by: Ishita Sequeira <46771830+ishitasequeira@users.noreply.github.com>
Co-authored-by: Ishita Sequeira <46771830+ishitasequeira@users.noreply.github.com>
lyda pushed a commit to lyda/argo-cd that referenced this issue Mar 28, 2024
…rgoproj#16222)

* fix(16207): Fix name conflict in appset controller

Signed-off-by: gmuselli <geoffrey.muselli@gmail.com>

* fix(16207): e2e

Signed-off-by: gmuselli <geoffrey.muselli@gmail.com>

* Update test/e2e/fixture/applicationsets/utils/fixture.go

Signed-off-by: Ishita Sequeira <46771830+ishitasequeira@users.noreply.github.com>

---------

Signed-off-by: gmuselli <geoffrey.muselli@gmail.com>
Signed-off-by: Ishita Sequeira <46771830+ishitasequeira@users.noreply.github.com>
Co-authored-by: Ishita Sequeira <46771830+ishitasequeira@users.noreply.github.com>
Signed-off-by: Kevin Lyda <kevin@lyda.ie>
Hariharasuthan99 pushed a commit to AmadeusITGroup/argo-cd that referenced this issue Jun 16, 2024
…rgoproj#16222)

* fix(16207): Fix name conflict in appset controller

Signed-off-by: gmuselli <geoffrey.muselli@gmail.com>

* fix(16207): e2e

Signed-off-by: gmuselli <geoffrey.muselli@gmail.com>

* Update test/e2e/fixture/applicationsets/utils/fixture.go

Signed-off-by: Ishita Sequeira <46771830+ishitasequeira@users.noreply.github.com>

---------

Signed-off-by: gmuselli <geoffrey.muselli@gmail.com>
Signed-off-by: Ishita Sequeira <46771830+ishitasequeira@users.noreply.github.com>
Co-authored-by: Ishita Sequeira <46771830+ishitasequeira@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
apps-in-any-namespace Issues related to the "Apps in any namespace" feature introduced in 2.5 bug Something isn't working component:application-sets Bulk application management related
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants