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

feat: Add Helm Chart #630

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Conversation

vyas-n
Copy link

@vyas-n vyas-n commented May 11, 2023

This is a starting point to resolve #629.

Mind if I get some feedback on how this is so far?

  • Is the location of the helm chart code appropriate?

I'd like to start extracting some variables in the helm chart and place it into the values.yaml file. Would it be ok if I proceeded with the following variables:

  • Resources from the deployment
  • container image repo & registry

@vyas-n
Copy link
Author

vyas-n commented May 11, 2023

@timoreimann,

Mind if I get some feedback on the above before continuing?

@vyas-n vyas-n marked this pull request as ready for review May 12, 2023 00:24
@timoreimann
Copy link
Contributor

Re: the location: one interesting question in this context is how the new Helm chart will relate to the static Kubernetes release manifests we currently maintain below the releases/ directory. Not sure if there's a canonical approach on how to go about it, personally I have a seen both of the following in the wild:

  1. A Helm chart is used to generate a default set of release manifests (that is committed to the repo)
  2. Only a Helm chart exists, and users are expected to run it to generate release manifests on their own

I'm slightly leaning towards (1) in order to not make Helm a hard requirement to install CCM, at least not for now. In that vein, it could make sense to move the charts/ directory below releases/ so that the two are more co-located. @vyas-n any thoughts / prior experiences on your end?

Re: the initial two variables: that looks good to me. 👍

(The full Helm adoption should probably cover using Helm for the release as well, meaning we'd want to update our Github Actions workflows. That's not something we need to do in this PR though, and it can also be done by someone else.)

@vyas-n
Copy link
Author

vyas-n commented May 12, 2023

Thanks @timoreimann,

The placement of the helm chart code tends to be developer preference since the resulting artifact is usually hosted separately from the code repository anyway.

I'll go ahead and move the charts folder into the releases directory, create a starter Readme.md, and add some validation in the GitHub Actions CI 👍

@timoreimann
Copy link
Contributor

@vyas-n sounds good, thanks!

@vyas-n vyas-n marked this pull request as draft June 7, 2023 23:43
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

Successfully merging this pull request may close these issues.

Feature Request: Create a Helm chart for DO CCM
2 participants