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

possibly depricate installplan-approver? #222

Open
itewk opened this issue Jun 16, 2023 · 6 comments
Open

possibly depricate installplan-approver? #222

itewk opened this issue Jun 16, 2023 · 6 comments

Comments

@itewk
Copy link

itewk commented Jun 16, 2023

Over in the helm-charts repo we now have a super charged version of the installplan-approver from this repository that does things even more robustly. makign sure to only approve the correct installplan for instance.

maybe we should depricate the installplan-approver here and/or at least cross link from here to the more advanced helm chart?

https://github.com/redhat-cop/helm-charts/tree/master/charts/operators-installer

thoughts?

@itewk
Copy link
Author

itewk commented Jun 16, 2023

@pittar @zisisli @carlmes @PendaGTP @strangiato thoughts?

@pittar
Copy link
Collaborator

pittar commented Jun 16, 2023

Hey @itewk . I really like the new Helm-based approver. However, not everyone will be using Helm and there's a certain simplicity to referencing the existing approver in your Kustomize file.

I agree that we should at a minimum reference the more robust Helm version in the README for the installplan-approver. I'll have to dig into your code a bit to see how much could be back ported into the simple Kustomize version.

@strangiato
Copy link
Contributor

Agreed with @pittar!

We should align them as best we can and provide both the helm and kustomize options.

Lots of customers are very kustomize driven and avoid helm like it is the plague. Both the helm and kustomize patterns should be maintained as best as possible to support the different options.

@itewk
Copy link
Author

itewk commented Jun 16, 2023

@pittar you could probably backport all of it into customize as well. we are then just maintinaing it in both palces. i went with helm because i wanted all teh approval stuff to be claned up after the fact, and helm hooks (which intigrate with argo hooks) are great for that. additionally, syncing parameters accross resources far easier with helm then kustomize.

minimally you could take the bash in the new Job for the more advanced approval logic if nothing else. maybe leave some comment in both jobs in both repos that they are copies of eachothers logic so if we ever change one someone can go change the other.

@itewk
Copy link
Author

itewk commented Sep 8, 2023

@pittar , @mertel-rh made a PR that ports the functionality from the helm chart (as best as possible) to this customize.

#243

@pittar
Copy link
Collaborator

pittar commented Sep 8, 2023

@itewk , yes - looking at it now! Thanks!

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

No branches or pull requests

3 participants