Official Helm Chart

[stevehipwell]

Now that there is an official Docker image (#144) it would be really useful if there was an official Helm chart.

[seanmalloy]

/kind feature
/good-first-issue

[seanmalloy]

Here are the required details that are needed to make this issue qualify as a good first issue.

Please use helm v3, not v2.

The helm chart needs to create a CronJob, ConfigMap, ClusterRole, ServiceAccount, and ClusterRoleBinding in the kube-system namespace. The .spec.schedule field needs to be configurable. The ConfigMap needs to support specifying a custom descheduler policy. See the files in the kubernetes directory for a good starting point.

The descheduler container image can be pulled from the below registry locations.

asia.gcr.io/k8s-artifacts-prod/descheduler/descheduler:v0.10.0
eu.gcr.io/k8s-artifacts-prod/descheduler/descheduler:v0.10.0
us.gcr.io/k8s-artifacts-prod/descheduler/descheduler:v0.10.0

[stevehipwell]

@seanmalloy I'm interested in picking this up and am slightly curious around the Helm v3 requirement? I understand that in a perfect world we'd all have moved to Helm v3 and all of the chart authors would have upgraded their charts to v3. However based on the current available charts it seems that most people are still using v2 charts even if they are applying them via v3. My suggestion to add this chart was to get closer alignment and it seems like only supporting Helm v3 might not be the best way to do this?

Would it be worth creating two charts (v2 & v3) or due to the limited scope of this chart would it be better sticking with Helm v2 for now?

Also do you have a repository for this new chart as the stable repo is no longer taking new chart submissions?

[Odernor]

Hi,
i have written an small descheduler helm chart for me. Im newbie in this..
Perhaps it can help an little: https://github.com/Odernor/descheduler_helm
its helm v2 but is installable with v3 too

[stevehipwell]

The following chart has been around for a while as the author also created a Docker image.
https://github.com/komljen/helm-charts/tree/master/descheduler

[seanmalloy]

I'm interested in picking this up and am slightly curious around the Helm v3 requirement?

I'm fine with Helm v2 if you think that makes the most sense.

Also do you have a repository for this new chart as the stable repo is no longer taking new chart submissions?

I'm not aware of an official location for storing k8s project helm charts. I'll reach out to wg-k8s-infra on slack to see if there is an official k8s helm repo.

Other ideas ...
We could use GitHub pages in this repo as a helm repo. I've set this up for one of the other projects I maintain. There was a GCS bucket setup for descheduler when GCR was setup. Maybe the GCS bucket could be used as helm repo. I believe the staging GCS bucket for descheduler is gs://k8s-staging-descheduler.

[seanmalloy]

I'm interested in picking this up

@stevehipwell feel free to start working on this. Let me know if you have any additional questions.

[seanmalloy]

@stevehipwell I've asked in the #wg-k8s-infra slack channel to see if the k8s project has an official helm repo that can be used. I'll let you know once I get an answer.

[seanmalloy]

I did not get any responses in the #wg-k8s-infra slack channel. My suggestion is that we use GitHub Pages in this repo to host the helm repo. See this link for an example of what this would look like.

@ravisantoshgudimetla @aveshagarwal @damemi any concerns with using GitHub pages to host the helm repo for descheduler?

[stevehipwell]

@seanmalloy I've been on holiday for the last week but will pick this up in the next couple of days. I'm happy with the GitHub pages repo suggestion and would look at using GitHub Actions to achieve this as I've used them successfully before.

[ndegory]

@seanmalloy , @stevehipwell any reason not to submit a new chart in https://github.com/helm/charts? (probably incubator). I'm using the chart mentioned above, but it's out of date, and PR are waiting for consideration, if that can help speed up to initiate the official chart komljen/helm-charts#30

[stevehipwell]

@ndegory see previous comment.

Also do you have a repository for this new chart as the stable repo is no longer taking new chart submissions?

[ndegory]

@stevehipwell did you have time to work on it?

[stevehipwell]

@ndegory I've not had a chance yet but I'm hoping to find some time this week. Once I have the time it should be very quick to get something done.

[stevehipwell]

@seanmalloy how does compatibility between releases work? Is it the same as usual with v0.17 being compatible with K8s v1.16, v1.17 and v1.18?

[seanmalloy]

@seanmalloy how does compatibility between releases work? Is it the same as usual with v0.17 being compatible with K8s v1.16, v1.17 and v1.18?

@stevehipwell yes that is correct for now. See #288.

[stevehipwell]

Thanks @seanmalloy.

RE Helm, I'm thinking a top level Helm directory containing the chart?

[seanmalloy]

RE Helm, I'm thinking a top level Helm directory containing the chart?

That sounds good to me.

[stevehipwell]

@seanmalloy I've opened #298 which is close to being finished. I need to know what the release process for the descheduler is so I can try and align the release GitHub action to that, assuming this is what's wanted. There will also need to be a token added to the config for the release to use (I'll add instructions to the PR when ready). Finally someone might want to update the chart docs with a better overview before actually creating a release.

[ThoTischner]

Hey just check out our helm chart, its super simple but made in germany: https://bitsbeats.github.io/helm-charts/

[damemi]

Hey just check out our helm chart, its super simple but made in germany: https://bitsbeats.github.io/helm-charts/

@ThoTischner thanks for the example! We were actually looking for examples of how other people are publishing their charts, would you mind taking a look at some of the discussion in #298 (it's a bit long, warning) and give your insight?

It looks like we're going to go with publishing the chart on a separate tagged branch, which will automatically trigger publishing through Github actions when PRs are merged to it. Does that sound similar to your implementation?

[ThoTischner]

@damemi I had a look at the PR. So it looks good to me. I did not know the chart- tagging logic. It seems to be a good idea if you have no separate repo for your charts, like we have. Our chart repo is based on the chart repo from velero: https://github.com/vmware-tanzu/helm-charts
Maybe it is a good idea to have separate repos for your app / code and the helm chart to keep things easy and do not mix up to many stuff in one big repo ;)

[damemi]

@ThoTischner I appreciate you taking the time to look at our PR, having the input from several different sources has been very helpful.

I agree that a separate repo might be the cleanest way, though I don't know if it's feasible for us to request an entirely new repo as a sig project. So, based on your feedback I think the branching implementation in the PR is a good compromise on that level. It keeps the charts "separate" from the project code while still being reasonably managed in the same space.

Thanks for your feedback!

[seanmalloy]

Thanks to @stevehipwell the descheduler helm chart is merged into the master branch!

I've opened the below two pull requests to wrap up the last few steps before releasing the helm chart.

• Cherry Pick: Add helm chart release-1.18 #352
• Update Release Documentation For Helm Charts #351

[seanmalloy]

All of the code is merged into the master and release-1.18 branches. I've asked @damemi to create the chart-0.18.0 tag from the release-1.18 branch.

/remove-good-first-issue

[damemi]

Another possibility: the actions/checkout action that runs before chart-releaser by default checks out the ref that triggered the workflow. In this case, that would mean it's checking out the chart-* tag, which the chart-releaser flow is comparing to itself.

So maybe we want to configure the checkout action (https://github.com/kubernetes-sigs/descheduler/blob/master/.github/workflows/release.yaml#L13) to have ref point to a different branch, either master or gh-pages

[stevehipwell]

@damemi it looks like the assumption that I made over swapping out the default trigger of push to master to pushing a tag was incorrect as the action is hardcoded to compare the current ref to the latest tag. I'll try to think of an alternative and get back to you.

[damemi]

I got it to work in my own repo by switching the push: tags to push on master and release-* branches. This successfully created a tag and published a release of the helm chart (the tag and release are both descheduler-<version> (changes in #356). (see successful runs to release-1.18 and master

Reading more into the helm releaser tool, the tool itself is what creates the new tag/release when publishing the chart. So we shouldn't be trying to create/push the tag (from what I can tell this is inherently how the cr tool works).

Downsides to this:

• I don't see any way we can change what the tag name or release title is. That means they'll always be descheduler-<version> (see https://github.com/damemi/descheduler/releases), which will get confusing against our regular releases. Maybe if we change the repo name in the chart to something like descheduler-helm
• We have a broken tag: https://github.com/kubernetes-sigs/descheduler/tree/0.9.0. This tag is missing the leading v, so it doesn't show up in order with the others. In fact, it shows as the "latest" tag, and if you look at my release runs I linked above you'll see that they always compare to this as the "latest". Imo, it is a bug in the chart-releaser-action to assume that the "latest" (alphabetically) tag is what should be compared against, and it doesn't have any logic to actually check the latest chart release tag. I don't think we can just delete/rename this tag since that could seriously break anyone that is currently depending on it. The release action works, but it's just always going to compare against an outdated tag edit:this was wrong, git tags aren't alphabetical it's just the most recent tag on master

[damemi]

I'm going to keep playing with it in my fork to see if I can figure out some of these problems.

Anyone else should also be able to test it in their forks too, which you should have write access too (since it's your fork).

[damemi]

I was wrong about the tag being alphabetical, I think 0.9.0 is just the most recent tag from the master branch (we've been tagging other releases from their release- branch). See this run where I added a new tag to my release-1.18 branch and it checked against that: https://github.com/damemi/descheduler/runs/899793702?check_suite_focus=true

[damemi]

Downside to running against master: every merged PR (without a helm version bump) will trigger a chart-releaser action that fails (https://github.com/damemi/descheduler/runs/899803075?check_suite_focus=true#step:4:23). As it should, because we'll only be bumping the chart version during a release (or a backport).

So I think we need to 1) change the action to act on pushes to release-* branches and 2) change our normal (non-release) flow to the following:

1. tag the new release from master
2. cut the release-1.x branch

We need to tag before we cut the branch because if not, just cutting the release-1.x branch will trigger a new action. Since the last tag on master (and thus the release-1.x branch) is 0.9.0, it will compare against that and always succeed.

This flow also means that we can merge the helm chart version bump to master in a normal PR as part of our release process.

@seanmalloy @stevehipwell what do you think of this? It's a lot simpler than the flow we originally imagined and still keeps the separate tags for charts (these are created by the action). I updated my pr in #356 with these changes

[stevehipwell]

@damemi that sounds like a sensible solution if it works with the project flow. You could take a look at how https://github.com/fluxcd/flux do theirs. They use a different action, but it also includes the lint process.

[damemi]

@damemi that sounds like a sensible solution if it works with the project flow. You could take a look at how https://github.com/fluxcd/flux do theirs. They use a different action, but it also includes the lint process.

Flux seems to be using a different chart releaser from stefanprodan/helm-gh-pages (https://github.com/fluxcd/flux/blob/master/.github/workflows/chart-release.yml#L12), as opposed to the official helm/chart-releaser-action we're trying to use here. I'm assuming that action takes some different steps that don't require the assumptions of the helm one here

[stevehipwell]

@damemi yes, the flux flow is the same as the initial one we planned. It looks like the action they're using checks the version in Chart.yaml against the repo metadata on the gh-pages branch when triggered by a chart-* tag.

[seanmalloy]

Looks like @damemi got us closer ...

• https://github.com/kubernetes-sigs/descheduler/releases/tag/descheduler-helm-chart-0.18.1
• https://github.com/kubernetes-sigs/descheduler/releases/download/descheduler-helm-chart-0.18.1/descheduler-helm-chart-0.18.1.tgz

[damemi]

@stevehipwell @seanmalloy Success! With the changes in #357, we have:

• A successful run of the chart-releaser-action
• A published helm chart release
• An updated index.yaml pushed to the gh-pages branch (this required removing the CLA check from this branch, or the push failed from the action. Since any changes that could trigger this will need to be first merged to another branch that does require the CLA, I think this is ok. No PRs should ever merge without the CLA, and we shouldn't allow any to be opened directly against this branch anyway).

[seanmalloy]

Cool ... thanks for your help @damemi.

I think the final step is to verify that the install instructions work now:

• https://github.com/kubernetes-sigs/descheduler/tree/descheduler-helm-chart-0.18.1/charts/descheduler#tldr
• https://github.com/kubernetes-sigs/descheduler/tree/descheduler-helm-chart-0.18.1/charts/descheduler#installing-the-chart

I think we could also remove all of the files from the gh-pages branch except for the file index.yaml, but I guess this is optional. After that we can close this issue!

[damemi]

@seanmalloy I was able to install the chart using:

$ helm repo add descheduler https://kubernetes-sigs.github.io/descheduler/
"descheduler" has been added to your repositorie

$ helm repo update
Hang tight while we grab the latest from your chart repositories...
...Successfully got an update from the "descheduler" chart repository
Update Complete. ⎈ Happy Helming!⎈ 

$ helm install descheduler descheduler/descheduler-helm-chart
NAME: descheduler
LAST DEPLOYED: Thu Jul 23 15:39:32 2020
NAMESPACE: default
STATUS: deployed
REVISION: 1
TEST SUITE: None
NOTES:
Descheduler installed as a cron job.

Note that because we changed the chart name from descheduler to descheduler-helm-chart I had to use that instead of what's in https://github.com/kubernetes-sigs/descheduler/tree/descheduler-helm-chart-0.18.1/charts/descheduler#tldr, so I opened #358 to fix that.

Also, really thanks again @stevehipwell for working with us on this and for helping to debug it. It's great to see this working and your and @seanmalloy's efforts to iron out the kinks with me were crucial. It's great to see it working! 🎉

[jyaworski]

Is it not possible to update the action to publish the correct name? helm install descheduler descheduler/descheduler-helm-chart seems quite strange.

[damemi]

Is it not possible to update the action to publish the correct name? helm install descheduler descheduler/descheduler-helm-chart seems quite strange.

We switched the name to avoid confusion between the descheduler releases and the chart releases (which are separate but both hosted in this repo, and they'd both be called "descheduler"). I couldn't find any way to have the action publish the release under a different name than the helm chart name but if anyone else does please feel free to make that change :)

[stevehipwell]

Great job @seanmalloy & @damemi, it was a pleasure working with you!!

[seanmalloy]

Final PR to adjust some documentation ... #359. I will close this issue once that PR is merged.

[seanmalloy]

/close

Thanks @stevehipwell and @damemi for your help with the helm chart. The helm chart is available and all the documentation has been updated.

[k8s-ci-robot]

@seanmalloy: Closing this issue.

In response to this:

/close

Thanks @stevehipwell and @damemi for your help with the helm chart. The helm chart is available and all the documentation has been updated.

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.

[stevehipwell]

@seanmalloy have you thought about adding the repo to https://hub.helm.sh/?

[seanmalloy]

@seanmalloy have you thought about adding the repo to https://hub.helm.sh/?

I think it is fine if we add the descheduler helm chat there. What is the process to add it? If it's a pull request would you be willing to submit the pull request?

[stevehipwell]

@seanmalloy I'm happy to make the PR, I'll just need to know what the contact details should be for the repo (as required). You will also want to make sure that the maintainer section of the Chart.yaml file is correct.

[seanmalloy]

@seanmalloy I'm happy to make the PR, I'll just need to know what the contact details should be for the repo (as required). You will also want to make sure that the maintainer section of the Chart.yaml file is correct.

@stevehipwell I think we should use email address kubernetes-sig-scheduling@googlegroups.com for the contact information. This is the mailing list for SIG Scheduling.

[stevehipwell]

FYI @seanmalloy helm/hub#435