-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
ApplicationSet generators diff with CLI #10895
Comments
Having something similar to what |
Hi @llavaud / @crenshaw-dev I am working on this issue, will submit a PR very soon. |
Hi @llavaud / @crenshaw-dev Should the difference of every application resource, which is part of the ApplicationSet be displayed as output of the new command ? E.g.: the output shows the difference in the case of modification, addition and deletion respectively.
Do let me know your thoughts on this. |
I posted #9904 some time ago. Reposting here. It could be useful to have a parameter so only the name of the Application are returned. I assume that having diff in the application is expected if the applicationSet template was updated. But if only the generators were updated, then what is most important in my opinion is the result list of applications. |
I am more interested to get only the diff in the result list of applications like @agaudreault-jive said, but if we can have both with a parameter it would be great ! |
I would say yes. It would be nice to have an ability to point to a source file with an application set and get a diff. For example, I am managing helm releases by the So it can be done on the ArgoCD "server" itself, like
But then it would require that the applicationset controller is rendering all the manifests on its side, and I'm afraid that if you add a lot of files to the generators folder it can have some unpleasant performance consequences. So I would rather teach a cli to generate everything on its side. argocd appset diff $APP_SET_NAME --file $FILE_NAME The base for comparing would be taker from the server, as it's already there, and probably rendered. But the wished state will be generated locally. I would be happy to work on this feature myself, actually, but I'm not sure how long will it take then. UPD: Actually, it would be possible to pass the file name without an AppSet name, because we already have a name in files. So like this argocd appset diff --file $FILE_NAME UPD2: |
IMO And obviously, both applicationSet names will need to match to be comparable. |
@agaudreault-jive I'm not 100% sure how argoCD works here. But any ApplicationSet manifest has a name defined in it. apiVersion: argoproj.io/v1alpha1
kind: ApplicationSet
metadata:
name: helm-releases
namespace: argo-system So if name and namespace can be taken from a file, then they could be used for diffing. I think that the Manifest: apiVersion: argoproj.io/v1alpha1
kind: ApplicationSet
metadata:
name: helm-releases
namespace: argo-system
spec:
syncPolicy:
preserveResourcesOnDeletion: true
generators:
- git:
repoURL: git@github.com:user/repo.git
revision: main
files:
- path: "releases/*" Exec And then application set controller would generate a manifest where About the
But now I think that I'm dummy, because I still need to provide a revision somehow, otherwise generator files won't be taken from the needed branch, unless you substitute required values to all revisions in the yaml file. |
@allanger I see a lot of problems if you try to generate the list of Application manifests that an applicationSet will create client-side, starting with authentication. This computing needs to be done server-side. The CLI can be used to "format" the results in different ways. What I have in mind is:
I think that this logic would work for any type of generators, and it would be easy to add different logic to show the diff without having to update the server. So for your use case, what you would send to the server is apiVersion: argoproj.io/v1alpha1
kind: ApplicationSet
metadata:
name: helm-releases
namespace: argo-system
spec:
syncPolicy:
preserveResourcesOnDeletion: true
generators:
- git:
repoURL: git@github.com:user/repo.git
revision: <YOUR_REVISION>
files:
- path: "releases/*" locally, you can use |
@agaudreault-jive
Aye, that's exactly what I meant by substituting, just didn't want to specify that hard :) My only concern about server side rendering for diffs is performance. I have about 20 generators files in the 'releases' folder, (I need to say, that it's only a fun project, so I don't have a very powerful machine), but was able to spot CPU peaks when the AppSet was reconciling. Together with #12314 it may be even cooler. Setting limits to amount of generators files seem not to be very flexible because in the end, I may want 2000 generators files, but I may not be ready for them, when someone is running diff. |
Right now, what I'm doing is listing all the apps belonging to the appset and then running the diffs on those apps. Since argocd don't have a way of listing all the apps of an appset I'm doing it using the method stated #13309 |
My problem is not really to have the diff of existing apps, but to know if the applicationSet will add or remove some of them after an update in the generator configuration. |
@allanger the ApplicationSet controller has efficiency problems in general. Folks have slowly chipped away at those, so I think things are improving. Based on @agaudreault-jive's design above, I think the API server would be taking on most of the load, by running ApplicationSet code as library code. So you'd end up scaling the API server, not the ApplicationSet controller. But either way, scalability is a concern. I'd suggest that we implement the feature, start with human users, and then knock out performance issues one by one as people start using the feature via automation (e.g. diff generation for PRs on git-hosted AppSet manifests). |
It seems like having the diff performed by the server is the barrier here. Would it be a better compromise to just add a |
@blakebarnett what you anticipate the |
Ah true, I was only considering the git-files generator. But you're right I would love to have the matrix generator show cluster parameters, etc as well. Would it be less overhead on the cluster to only do a dry-run vs a diff or is that a negligible difference? |
Couldn't the CLI fetch the server-side data to generate a dry-run output if you're logged in & within the appropriate context/namespace? Seems like it would just gather the current cluster secrets & any other generator data then render the Application YAMLs to stdout. |
I think the difference would be negligible.
That's one way... personally I feel better about leaving the access secrets server-side and just generating the parameters and pass them back for templating. |
Yes, I wasn't suggesting the secret data be pulled/rendered (at least not without explicitly allowing/requesting them). I'd just want labels/annotations & the standard values like |
I am wondering if a lot of folks' usecases would be solved if we were able to pull out just the names of the apps: |
I just want it to dump the |
This doesn't help troubleshoot templating logic too like when using paths or labels, etc. 75% of the time I want to test/verify my templating logic works with my generators, not just get app names. |
I agree; I often just go to online template playgrounds for the appropriate templating implementation for things like this but even there I don't have full insight into the initial objects presented so it's not even that useful and definitely a hacky workaround when I should just be able to get the output from argo on the CLI or something easy so I don't have to constantly test on a live argo instance whether my templating logic and stuff even works :/ |
) * feat(cli): add cmd to preview generated apps Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com> * fix build Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com> * fix local proto gen Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com> * dry run client Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com> * fix: allow to run codegen outside GOPATH Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com> * clientgen Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com> * openapigen Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com> * remove ensure-gopath Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com> * fix tests and templatePatch Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com> * fix build Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com> * convert to interfaces Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com> * codegen Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com> * extract common code Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com> * use appset params in server Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com> * codegen Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com> * fix test build Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com> * unit tests Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com> * move test to new package Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com> * move to correct folders Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com> * fix build Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com> * review Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com> * lint Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> * fix test Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> * fix lint Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> * auto generate mocks Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> * better error handling Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> * more docs Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> * more docs Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> * lint Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> --------- Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com> Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com> Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
…) (argoproj#16781) * feat(cli): add cmd to preview generated apps Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com> * fix build Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com> * fix local proto gen Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com> * dry run client Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com> * fix: allow to run codegen outside GOPATH Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com> * clientgen Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com> * openapigen Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com> * remove ensure-gopath Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com> * fix tests and templatePatch Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com> * fix build Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com> * convert to interfaces Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com> * codegen Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com> * extract common code Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com> * use appset params in server Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com> * codegen Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com> * fix test build Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com> * unit tests Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com> * move test to new package Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com> * move to correct folders Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com> * fix build Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com> * review Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com> * lint Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> * fix test Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> * fix lint Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> * auto generate mocks Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> * better error handling Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> * more docs Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> * more docs Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> * lint Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> --------- Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com> Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com> Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
…) (argoproj#16781) * feat(cli): add cmd to preview generated apps Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com> * fix build Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com> * fix local proto gen Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com> * dry run client Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com> * fix: allow to run codegen outside GOPATH Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com> * clientgen Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com> * openapigen Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com> * remove ensure-gopath Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com> * fix tests and templatePatch Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com> * fix build Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com> * convert to interfaces Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com> * codegen Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com> * extract common code Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com> * use appset params in server Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com> * codegen Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com> * fix test build Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com> * unit tests Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com> * move test to new package Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com> * move to correct folders Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com> * fix build Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com> * review Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com> * lint Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> * fix test Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> * fix lint Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> * auto generate mocks Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> * better error handling Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> * more docs Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> * more docs Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> * lint Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> --------- Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com> Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com> Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> Signed-off-by: Javier Solana <javier.solana@cabify.com> Signed-off-by: Javier Solana <javier.solana@cabify.com>
Summary
Beeing able to diff the ApplicationSet generators
Motivation
Today to push a modification on an application I can do an
argocd app diff --revision ...
in my CI to see the changes I will add.I would love to have something similar to see the list of application that will be added or removed by the applicationset.
Proposal
Implement an
argocd appset diff --revision ...
?The text was updated successfully, but these errors were encountered: