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

fix(appset): normalize app spec before applying #14481

Merged
merged 3 commits into from
Jul 17, 2023

Conversation

crenshaw-dev
Copy link
Member

The AppSet controller can get into a state where it fights with the application controller.

For example, try an appset like this:

apiVersion: argoproj.io/v1alpha1
kind: ApplicationSet
metadata:
  name: test
spec:
  generators:
    - git:
        repoURL: https://github.com/test/test
        directories:
          - path: '*'
  template:
    metadata:
      name: '{{path}}-dev'
    spec:
      project: test
      source:
        repoURL: https://github.intuit.com/test/test
        targetRevision: release/dev
        path: '{{path}}'
        directory:
          jsonnet: {}
      destination:
        server: https://test
        namespace: '{{path}}'

Note that the application controller logs a bunch of stuff like this:

time="2023-07-12T16:28:29Z" level=info msg="Normalized app spec: {\"spec\":{\"source\":{\"directory\":null}}}"

It's fighting with the appset controller.

This PR normalizes before applying so there's nothing to fight about.

Signed-off-by: Michael Crenshaw <[email protected]>
Signed-off-by: Michael Crenshaw <[email protected]>
@codecov
Copy link

codecov bot commented Jul 12, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.02 🎉

Comparison is base (e2e0da7) 49.73% compared to head (2662557) 49.76%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #14481      +/-   ##
==========================================
+ Coverage   49.73%   49.76%   +0.02%     
==========================================
  Files         261      261              
  Lines       44705    44758      +53     
==========================================
+ Hits        22234    22272      +38     
- Misses      20284    20295      +11     
- Partials     2187     2191       +4     
Impacted Files Coverage Δ
...cationset/controllers/applicationset_controller.go 62.64% <100.00%> (+0.10%) ⬆️
util/argo/argo.go 66.49% <100.00%> (+0.04%) ⬆️

... and 12 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@crenshaw-dev crenshaw-dev marked this pull request as ready for review July 12, 2023 23:06
Copy link
Member

@ishitasequeira ishitasequeira left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!!

Copy link
Collaborator

@leoluz leoluz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@leoluz leoluz merged commit b90f3bc into argoproj:master Jul 17, 2023
@crenshaw-dev crenshaw-dev deleted the normalize-appset-app-spec branch July 17, 2023 17:56
@crenshaw-dev
Copy link
Member Author

/cherry-pick release-2.8

@crenshaw-dev
Copy link
Member Author

/cherry-pick release-2.7

gcp-cherry-pick-bot bot pushed a commit that referenced this pull request Jul 17, 2023
* fix(appset): normalize app spec before applying

Signed-off-by: Michael Crenshaw <[email protected]>

* fix nil ref, add test

Signed-off-by: Michael Crenshaw <[email protected]>

* fix another test

Signed-off-by: Michael Crenshaw <[email protected]>

---------

Signed-off-by: Michael Crenshaw <[email protected]>
@crenshaw-dev
Copy link
Member Author

/cherry-pick release-2.6

gcp-cherry-pick-bot bot pushed a commit that referenced this pull request Jul 17, 2023
* fix(appset): normalize app spec before applying

Signed-off-by: Michael Crenshaw <[email protected]>

* fix nil ref, add test

Signed-off-by: Michael Crenshaw <[email protected]>

* fix another test

Signed-off-by: Michael Crenshaw <[email protected]>

---------

Signed-off-by: Michael Crenshaw <[email protected]>
@crenshaw-dev
Copy link
Member Author

/cherry-pick release-2.5

@gcp-cherry-pick-bot
Copy link

Cherry-pick failed with Merge error b90f3bc774b05fc71a1f3cb8803eb66bfd20e5a7 into temp-cherry-pick-0fa364-release-2.6

@gcp-cherry-pick-bot
Copy link

Cherry-pick failed with Merge error b90f3bc774b05fc71a1f3cb8803eb66bfd20e5a7 into temp-cherry-pick-0fa364-release-2.5

crenshaw-dev added a commit that referenced this pull request Jul 17, 2023
* fix(appset): normalize app spec before applying



* fix nil ref, add test



* fix another test



---------

Signed-off-by: Michael Crenshaw <[email protected]>
Co-authored-by: Michael Crenshaw <[email protected]>
crenshaw-dev added a commit that referenced this pull request Jul 17, 2023
* fix(appset): normalize app spec before applying



* fix nil ref, add test



* fix another test



---------

Signed-off-by: Michael Crenshaw <[email protected]>
Co-authored-by: Michael Crenshaw <[email protected]>
Jneville0815 pushed a commit to radiusmethod/argo-cd that referenced this pull request Jul 18, 2023
* fix(appset): normalize app spec before applying

Signed-off-by: Michael Crenshaw <[email protected]>

* fix nil ref, add test

Signed-off-by: Michael Crenshaw <[email protected]>

* fix another test

Signed-off-by: Michael Crenshaw <[email protected]>

---------

Signed-off-by: Michael Crenshaw <[email protected]>
Signed-off-by: Jimmy Neville <[email protected]>
schakrad pushed a commit to schakrad/argo-cd that referenced this pull request Jul 24, 2023
…oproj#14555)

* fix(appset): normalize app spec before applying

* fix nil ref, add test

* fix another test

---------

Signed-off-by: Michael Crenshaw <[email protected]>
Co-authored-by: Michael Crenshaw <[email protected]>
Signed-off-by: schakrad <[email protected]>
yyzxw pushed a commit to yyzxw/argo-cd that referenced this pull request Aug 9, 2023
* fix(appset): normalize app spec before applying

Signed-off-by: Michael Crenshaw <[email protected]>

* fix nil ref, add test

Signed-off-by: Michael Crenshaw <[email protected]>

* fix another test

Signed-off-by: Michael Crenshaw <[email protected]>

---------

Signed-off-by: Michael Crenshaw <[email protected]>
@suzaku suzaku mentioned this pull request Aug 15, 2023
tesla59 pushed a commit to tesla59/argo-cd that referenced this pull request Dec 16, 2023
* fix(appset): normalize app spec before applying

Signed-off-by: Michael Crenshaw <[email protected]>

* fix nil ref, add test

Signed-off-by: Michael Crenshaw <[email protected]>

* fix another test

Signed-off-by: Michael Crenshaw <[email protected]>

---------

Signed-off-by: Michael Crenshaw <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants