-
Notifications
You must be signed in to change notification settings - Fork 43
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
[Helm] extend functionality #150
base: master
Are you sure you want to change the base?
Conversation
vriabyk
commented
Oct 18, 2022
- add support for already existing secret in cluster with benji configuration
- add fix for templating issue when volumes are empty list
- add support for initContainers
- add Charts.lock which is required for ArgoCD to deploy helm dependencies
- add templates for ceph configuration options
@elemental-lf pls review this when you have time :) |
@vriabyk thanks for your contribution! I will look into it in the next few days. |
I like the idea but I'm not onboard with the same variable value having two different types (boolean and string).
I committed a similar fix also taking into account the new Argo Workflow based cronjobs.
Could you describe your use case for these? I want to understand which pods need init containers and which don't.
I'm still unsure about this. I'm using ArgoCD myself and I don't see this behavior. But I'm not using helm directly but via helmfile so that might be the difference. But I also couldn't find any hints in the ArgoCD documentation that this is required. Could you point me to some info about this? |
@elemental-lf Hey, thanks for your comments! Let me try addressing some of them:
It was not intended to use it as boolean, it's just a matter of the external secret object name pasted there. The value could be just empty and meant to save some space. Otherwise it could be something like:
I think it just adds more lines and doesn't necessary makes it cleaner.
Thanks, I'll check that and rebase our code
We use some init steps to check or initialize PostgreSQL database that we'll be using later on in our set up. As for Charts.lock thing, I'll reply in the next comment |
So as for Charts.lock there is this issue. |
bdfa10a
to
6d5070e
Compare
@elemental-lf hey, we've changed some things here and there, so please check once you have free time |
@elemental-lf hello, is there any news regarding this? |
I will take a look in the next few days. |
83d9d47
to
3071468
Compare
to summarize what we are adding here after the whole discussion:
|
fix templating add initContainers functionality
add secret for ceph configuration adjust values
3071468
to
0c93842
Compare