argo-cd
argo-cd copied to clipboard
feat(cli): add cmd to preview generated apps of appsets (#10895)
- depends on https://github.com/argoproj/argo-cd/pull/15741 having the list of application (must be merged and this code updated to use the resource field)
Closes #10895
The goal of this PR is to update the argocd appset create
command to add a --dry-run
argument. When it is specified, the server will call the applicationSet templating and app generation logic to evaluate and transform all the generator. It will use the resulting applications to generate a list of "generated" application.
./dist/argocd appset create --dry-run ./appset.yaml -o json | jq -r '.status.applicationStatus[].application'
ApplicationSet 'tests.simple-application' unchanged (dry-run)
tests.simple-application.dev
tests.simple-application.production-1
tests.simple-application.production-2
tests.simple-application.staging
The output of this command will allow to compare the list of resulting applications with the output of argocd appset get tests.simple-application -o json | jq -r '.status.applicationStatus[].application
to know which app will be added or removed once the manifest is applied.
Suggestions
I think the CLI command could be argocd appset template ./appset.yaml -o json
and return the list of templated applications with the full object and not only their name. Some people expressed the need to have the whole applications so they could validate the logic client-side.
With the current implementation, only the app name is returned in create and the get.
An implementation to be able to do client-side diff would be to have the template command return the current AppSet with all the applications templated, and modify the get to return the full templated application as well (this would avoid an argocd app get
for each generated apps).
Checklist:
- [x] Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes.
- [x] The title of the PR states what changed and the related issues number (used for the release note).
- [x] The title of the PR conforms to the Toolchain Guide
- [x] I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue.
- [x] I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them.
- [x] Does this PR require documentation updates?
- [x] I've updated documentation as required by this PR.
- [x] I have signed off all my commits as required by DCO
- [x] I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
- [x] My build is green (troubleshooting builds).
- [x] My new feature complies with the feature status guidelines.
- [x] I have added a brief description of why this PR is necessary and/or what this PR solves.
- [x] Optional. My organization is added to USERS.md.
- [x] Optional. For bug fixes, I've indicated what older releases this fix should be cherry-picked into (this may or may not happen depending on risk/complexity).
Codecov Report
Attention: Patch coverage is 62.45955%
with 116 lines
in your changes missing coverage. Please review.
Project coverage is 50.24%. Comparing base (
4c6ad9d
) to head (19b0871
). Report is 11 commits behind head on master.
:exclamation: Current head 19b0871 differs from pull request most recent head 02bb52a
Please upload reports for the commit 02bb52a to get more accurate results.
Additional details and impacted files
@@ Coverage Diff @@
## master #16781 +/- ##
==========================================
- Coverage 50.28% 50.24% -0.05%
==========================================
Files 312 315 +3
Lines 43020 43120 +100
==========================================
+ Hits 21633 21665 +32
- Misses 18905 18971 +66
- Partials 2482 2484 +2
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
This feature is really needed in order to do any meaningful dryrun if using applicationset.
In production environment, we have to review the dryrun changes before any updates to applicationset could be landed. With this PR in place, we could get a list of applications, and then we could iterate the list of applications to get its diff via "argocd app diff" command at least.
Without this PR in place, there's no way to do any meaningful dryrun for applicationset.
This is the most needed feature for argocd applicationset.
@argoproj/argocd-approvers ~~what do you think if we merge this as an "alpha" feature, and when the dependency is merged, we can change the output of the command to use the new fields? Maybe with a warning in the CLI doc?~~ Never mind, I forgot that the current get command does not return the list of applications when progressive sync is not present. So it does need the dependency.
The dependency seems to be coupled with a few other PRs and may take some time to merge. @alexymantha @csantanapr @reggie-k
👍 Long-waited feature to allow simple and reliable validations on any non-trivial manifests repo that uses appset
@agaudreault
The dependency seems to be coupled with a few other PRs and may take some time to merge. @alexymantha @csantanapr @reggie-k
Is this related to the sharding algorithms based on apps? Could you list of PRs that is holding this back?
Initially, this PR was treated as a dependency for https://github.com/argoproj/argo-cd/pull/15731/files/19c5c0812ce859e97fb85ef1e9e63a2d38d32bb1 But lately I got an update from @crenshaw-dev that it was de-coupled and treated as an independent PR.
@agaudreault
The dependency seems to be coupled with a few other PRs and may take some time to merge. @alexymantha @csantanapr @reggie-k
Is this related to the sharding algorithms based on apps? Could you list of PRs that is holding this back?
It only depends on https://github.com/argoproj/argo-cd/pull/15741 having the list of application afaik. When it is merged, i'll get back into it to make the final changes.
https://github.com/argoproj/argo-cd/pull/15741 just got merged!
@agaudreault any update on this? no pressure but I'm really waiting for this feature!
@agaudreault any update on this? no pressure but I'm really waiting for this feature!
I should be able to resume the work next week so it is in the next release :)
This feature was marked as having been completed in 2.12, but it is not anywhere on the 2.12 branch. I think we need to move it to 2.13 now.