benji icon indicating copy to clipboard operation
benji copied to clipboard

[Helm] extend functionality

Open vriabyk opened this issue 3 years ago • 9 comments

  • 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

vriabyk avatar Oct 18 '22 11:10 vriabyk

@elemental-lf pls review this when you have time :)

vriabyk avatar Oct 25 '22 11:10 vriabyk

@vriabyk thanks for your contribution! I will look into it in the next few days.

elemental-lf avatar Nov 04 '22 12:11 elemental-lf

add support for already existing secret in cluster with benji configuration

I like the idea but I'm not onboard with the same variable value having two different types (boolean and string).

add fix for templating issue when volumes are empty list

I committed a similar fix also taking into account the new Argo Workflow based cronjobs.

add support for initContainers

Could you describe your use case for these? I want to understand which pods need init containers and which don't.

add Charts.lock which is required for ArgoCD to deploy helm dependencies

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 avatar Nov 14 '22 10:11 elemental-lf

@elemental-lf Hey, thanks for your comments! Let me try addressing some of them:

I like the idea but I'm not onboard with the same variable value having two different types (boolean and string).

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:

benji:
  externalSecretConfig: true
  secretName: secret

I think it just adds more lines and doesn't necessary makes it cleaner.

I committed a similar fix also taking into account the new Argo Workflow based cronjobs.

Thanks, I'll check that and rebase our code

add support for initContainers

We use some init steps to check or initialize PostgreSQL database that we'll be using later on in our set up. Generally, having initContainers just adds some functionality to the chart that can be used or not used by clients.

As for Charts.lock thing, I'll reply in the next comment

JohnnyMastricht avatar Nov 16 '22 09:11 JohnnyMastricht

So as for Charts.lock there is this issue. What I think is that for now actually we could ommit this since we also switched to helmfile and we got to test how it works now. We'll add some commits to this PR soon.

JohnnyMastricht avatar Nov 16 '22 11:11 JohnnyMastricht

@elemental-lf hey, we've changed some things here and there, so please check once you have free time

JohnnyMastricht avatar Nov 28 '22 08:11 JohnnyMastricht

@elemental-lf hello, is there any news regarding this?

JohnnyMastricht avatar Mar 02 '23 15:03 JohnnyMastricht

I will take a look in the next few days.

elemental-lf avatar Mar 29 '23 20:03 elemental-lf

to summarize what we are adding here after the whole discussion:

  • support for already existing secret in cluster with benji configuration
  • ability to set initContainers (for example to initialize database or to pre-create minio bucket) and command for benji maint pod (we wanna start nbd server there using it)
  • templates for ceph configuration options

vriabyk avatar Apr 11 '23 08:04 vriabyk