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

Helm-Chart for kubevirt-hostpath-provisioner #326

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

kubelize
Copy link

What this PR does / why we need it:
Helm is a very practical method to deploy, update and rollback applications and operators in Kubernetes. This PR adds a fully functional helm-chart that allows for a single command to deploy, and by specifying your own values.yaml modify any of the values listed in the README.md for the chart.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

Special notes for your reviewer:
A github-pages deployment which would server as the endpoint for the helm-chart releases would add additional ease of use and quality to the chart. But I leave that up to you if it's something that would interest you :)

Release note:

Adds a helm-chart to the repository. Will have no effect on the functionality of other content. No user action required.
Chart: kubevirt-hostpath-provisioner
AppVersion: 0.18.0
Chart Version: 0.1.0

@kubevirt-bot kubevirt-bot added release-note-action-required Denotes a PR that introduces potentially breaking changes that require user action. dco-signoff: no Indicates the PR's author has not DCO signed all their commits. labels Feb 21, 2024
@kubevirt-bot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign aglitke for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kubevirt-bot
Copy link

Hi @moffoso. Thanks for your PR.

PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test all.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@awels
Copy link
Member

awels commented Feb 21, 2024

To make the DCO happy, can you git --amend -s the -s will sign the commit and make the bot happy.

@awels
Copy link
Member

awels commented Feb 21, 2024

So I have little to no experience with helm charts. So I can't promise I will maintain it properly. Do we want to create some release artifacts when I release a new version of the provisioner related to the helm charts?

Signed-off-by: Daniel Hendricken <daniel.hendricken@bedag.ch>
Signed-off-by: Daniel Hendricken <daniel.hendricken@bedag.ch>
@kubevirt-bot kubevirt-bot added dco-signoff: yes Indicates the PR's author has DCO signed all their commits. and removed dco-signoff: no Indicates the PR's author has not DCO signed all their commits. labels Feb 21, 2024
@kubelize
Copy link
Author

kubelize commented Feb 21, 2024

Hey @awels

Unless there are breaking changes to the provisioner, the chart wont need to be updated. The image-tag is set to "latest" per default. (The exeption being if the functionality- of the chart should be extended or improved in some manner)

A release artifact would increase quality of life, especially for example in a CI/CD setup.
Optimally the workflow with helm would be that you can add the repository the chart is in e.g:

helm repo add kubevirt https://kubevirt.github.io/kubevirt-hostpath-provisioner

and then install while specifying your own values

helm install kubevirt-hostpath-provisioner kubevirt/kubevirt-hostpath-provisioner -f my-custom-values.yam

The repo/branch containing the helm-packages then needs and index.yaml used for indexing the charts in the repository.
index.yaml

apiVersion: v1
entries:
  kubevirt-hostpath-provisioner:
  - apiVersion: v2
    appVersion: 0.18.0
    created: "2024-02-21T16:32:46.506729413+01:00"
    description: A Helm chart for Kubernetes
    digest: 350a43cb10758aad5e46bd5bf1a01a7b727d95536a27cbce0c80a20b73041014
    name: kubevirt-hostpath-provisioner
    type: application
    urls:
    - https://kubevirt.github.io/kubevirt-hostpath-provisioner/releases/download/kubevirt-hostpath-provisioner-0.1.0.tgz
    version: 0.1.0
generated: "2024-02-21T16:32:46.506427115+01:00"

Branch contents

.
├── index.yaml
├── LICENSE
└── README.md

And finally a release that contains the individual helm-packages.

If that's not something you would want to maintain, that's understandable. In that case a github-release of the chart would suffice for most use-cases. Alternatively I could host a repository for the chart and maintain it. I just thought it could potentially be of interest to you.

Cheers

@kubevirt-bot kubevirt-bot added dco-signoff: no Indicates the PR's author has not DCO signed all their commits. and removed dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels Feb 22, 2024
Signed-off-by: Daniel Hendricken <daniel.hendricken@bedag.ch>
…ubstituted correctly

Signed-off-by: Daniel Hendricken <daniel.hendricken@bedag.ch>
…ubstituted correctly

Signed-off-by: Daniel Hendricken <daniel.hendricken@bedag.ch>
@kubevirt-bot kubevirt-bot added dco-signoff: yes Indicates the PR's author has DCO signed all their commits. and removed dco-signoff: no Indicates the PR's author has not DCO signed all their commits. labels Feb 22, 2024
@kubelize
Copy link
Author

kubelize commented Feb 22, 2024

I did some testing, and found that pvc's would stay pending due to an an error in the rendering of the daemonset manifest. For some reason that I haven't yet got to the bottom of yet. The issue has been resolved, and pv's are provisioned as expected now.
-> Bumped the chart version to 0.1.1

@awels
Copy link
Member

awels commented Feb 26, 2024

Okay I have no issues with this, just one question, what is in the tgz file? Is it just a tarred and gzipped version of the helm chart?

@kubelize
Copy link
Author

kubelize commented Feb 26, 2024

Okay I have no issues with this, just one question, what is in the tgz file? Is it just a tarred and gzipped version of the helm chart?

Indeed it is, it's purely for convenience sake. So that you can install the chart by simply downloading the archive and then running

helm install kubevirt-hostpath-provisioner-0.1.1.tgz

Optionally you can append --namespace yournamespace

@awels
Copy link
Member

awels commented Feb 26, 2024

So maybe instead of putting that in the repo, when I make the release, we create the tgz as an artifact, and you can download that to install it?

@kubelize
Copy link
Author

That would be ideal!

@awels
Copy link
Member

awels commented Feb 26, 2024

If you look at https://github.com/kubevirt/hostpath-provisioner/blob/main/hack/release.sh#L45-L59 that is what is called when it is building the release. I don't know what goes in the tgz file, but if you could modify that to create the file, and put it in the list of things to push that would be great.

@kubelize
Copy link
Author

Alright sounds good, I'll probably find time to do that tomorrow.

@awels
Copy link
Member

awels commented Mar 21, 2024

I guess you didn't find time, which is fine. Just bumping this so the bot won't start putting stale on it.

@Bernix01
Copy link

Friendly ping to @kubelize for this PR this feature is very much appreciated :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dco-signoff: yes Indicates the PR's author has DCO signed all their commits. release-note-action-required Denotes a PR that introduces potentially breaking changes that require user action. size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants