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(chart): Create initial Chart for csi-cloudscale #43

Merged
merged 47 commits into from
Sep 22, 2022

Conversation

eyenx
Copy link
Contributor

@eyenx eyenx commented Jun 22, 2022

Creates a first initial draft of a helm chart for easily deploying csi-cloudscale via helm

Signed-off-by: Toni Tauro [email protected]

@alakae
Copy link
Contributor

alakae commented Jul 5, 2022

Hi @eyenx
Thank you for your pull-request. We'll have a look at your contribution soon and get back to you.
Feel free to contact us, if you have any questions.

Copy link
Contributor

@alakae alakae left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi again!

I had a more in-depth look into your pull request. Thanks, again, for your contribution. I think publishing the chart is definitely an option for the future.

I have added a number of comments to your code, where the release generated by the chart differs from the current YAML. Some of them seem to be typos, etc., whereas for others I am uncertain if they were intentional.

Additionally, I think we should set up a Helm chart repo and set up GitHub actions to publish the chart to it.

If you add to the PR, I'll have a look again. Otherwise, I'll probably try to finish it in a few weeks.

charts/csi-cloudscale/templates/daemonset.yaml Outdated Show resolved Hide resolved
charts/csi-cloudscale/templates/daemonset.yaml Outdated Show resolved Hide resolved
charts/csi-cloudscale/templates/rbac.yaml Outdated Show resolved Hide resolved
charts/csi-cloudscale/templates/serviceaccount.yaml Outdated Show resolved Hide resolved
charts/csi-cloudscale/templates/statefulset.yaml Outdated Show resolved Hide resolved
charts/csi-cloudscale/values.yaml Show resolved Hide resolved
charts/csi-cloudscale/values.yaml Show resolved Hide resolved
charts/csi-cloudscale/Chart.yaml Outdated Show resolved Hide resolved
Signed-off-by: Toni Tauro <[email protected]>
@eyenx eyenx force-pushed the feat/add-helm-chart branch from 146f661 to 71a5e47 Compare July 19, 2022 19:14
@eyenx eyenx requested a review from alakae July 19, 2022 19:15
@eyenx
Copy link
Contributor Author

eyenx commented Jul 19, 2022

Additionally, I think we should set up a Helm chart repo and set up GitHub actions to publish the chart to it.

Setting this up needs elevate admin rights to the repository. Feel free to check out our github actions and how we publish charts on https://github.com/adfinis-sygroup/helm-charts

@davidhalter
Copy link
Contributor

@alain One more thing: If we merge this, I think we should strongly consider generating the static manifests by the helm charts for each release, because having two separate deployment sets (with almost the same content) is probably a bad idea. Not sure how people are doing this, though.

@alakae
Copy link
Contributor

alakae commented Jul 21, 2022

@alain One more thing: If we merge this, I think we should strongly consider generating the static manifests by the helm charts for each release, because having two separate deployment sets (with almost the same content) is probably a bad idea. Not sure how people are doing this, though.

I had this on my mind as well.

Copy link

@simu simu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great overall. I've left some suggestions for improvements inline.

We'll be able to switch to the Helm chart without any big issues (see also projectsyn/component-csi-cloudscale#47)

charts/csi-cloudscale/values.yaml Outdated Show resolved Hide resolved
charts/csi-cloudscale/values.yaml Outdated Show resolved Hide resolved
charts/csi-cloudscale/templates/statefulset.yaml Outdated Show resolved Hide resolved
charts/csi-cloudscale/templates/statefulset.yaml Outdated Show resolved Hide resolved
@alakae alakae force-pushed the feat/add-helm-chart branch from a82b55a to fcc1661 Compare September 22, 2022 11:25
make NEW_VERSION=v3.3.0 bump-version
make NEW_CHART_VERSION=v1.0.0 bump-chart-version
@alakae alakae merged commit 15e6c32 into cloudscale-ch:master Sep 22, 2022
@alakae
Copy link
Contributor

alakae commented Sep 22, 2022

Thank you @eyenx @simu and @megian .

@eyenx eyenx deleted the feat/add-helm-chart branch September 22, 2022 15:20
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.

5 participants