Add Helm chart

[stevehipwell]

What this PR does / why we need it:

This PR adds an official Helm chart based on the previous stable chart and the deployment yaml from this project.

This chart has been given a major version bump and should be updatable from the existing stable chart but without support for additional containers and volumes (this could be re-added but seemed un-needed).

What is outstanding:

The following items need some discussion.

• CI/CD automation
• Chart maintainer

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 #572.

[k8s-ci-robot]

Hi @stevehipwell. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

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.

[k8s-ci-robot]

Welcome @stevehipwell!

It looks like this is your first PR to kubernetes-sigs/metrics-server 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/metrics-server has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

[pierluigilenoci]

@s-urbaniak @brancz @serathius could you please take a look?

[stevehipwell]

Has anyone had a chance to look at this yet?

[pierluigilenoci]

@s-urbaniak @brancz @serathius please! ❤️

[pierluigilenoci]

@stevehipwell this PR is still in draft, maybe makes sense to try to promote it as real PR to be considered?

[stevehipwell]

@pierluigilenoci the actual chart is complete but I've got outstanding questions around the CI/CD tool to use (I'm familiar with GitHub actiuons for this) and who the chart maintainer needs to be set as. I'm happy to write the readme now and make it an official PR but I was waiting for answers.

[pgollucci]

commenting to track

[stevehipwell]

I've added the chart README and setup GitHub actions for the automation.

The chart lint action will run on PRs when the charts/** path has been modified. The chart will be released when there is a release published with a v prefix. The chart will be released on a tag following the pattern metrics-server-helm-chart-${VERSION} where VERSION is the chart version. Chart version and app version will both need to be incremented as part of the release process (release step number 3).

Before the chart can be released GitHub pages needs to be enabled with an empty gh-pages branch.

[krmichelos]

The chart will be released when there is a release published with a v prefix. The chart will be released on a tag following the pattern metrics-server-helm-chart-${VERSION}

FYI, I have seen in a number of projects where the chart releaser action is triggered when any file in the charts directory is changed on master and will just fail until the chart version in the Chart.yaml is updated to a version that hasn't already been released

[stevehipwell]

@krmichelos that's not a pattern I'm familiar with, specifically the failing part. If a repo is just for Helm charts then releases tend to happen on any change to master where the chart version has been changed (this works with multiple charts). For a repo like this there needs to be a specific trigger and the one I've configured would seem to fit in best with the project release process (for the descheduler I used a different pattern as they have a different release process).

[stevehipwell]

I've rebased, is there anything else I need to do to get this progressed?

[stevehipwell]

@brancz @serathius could you take a look?

[stevehipwell]

@serathius could you approve the lint-test action?

[junior]

This PR does not include the artifacthub-repo.yaml to publish to artifacthub.io. The are any plans to include it?

[stevehipwell]

@junior the artifacthub-repo.yaml file is added to the gh-pages branch so can't be part of this PR.

@serathius before this is merged GitHub pages needs to be enabled on this repo with an empty gh-pages branch. If you plan on registering this repo in Artifact Hub then a artifacthub-repo.yaml file should be added to the empty gh-pages branch.

[pierluigilenoci]

We are so close... what is missing? Just a /lgtm label?

[serathius]

@stevehipwell I have added ghpages. Are there any other prerequisites that are not part of the PR?

As for E2e, I'm not convinced that we should skip them for Helm chart. If we are providing alternative official installation method, we should require same quality. Fact that helm release is independent, doesn't mean that we should not test chart. I think we should run same set of presubmit tests that ensure that both binary and chart changes don't lead to breaking master branch.

This for sure will put maintenance cost on both chart and subproject owners, it will require to introduce and maintain the CI to ensure that PRs don't break compatibility between chart and manifest. However this cost should be smaller than aggregated cost of fixing the release.

Before merging the change I would want to make sure that we have agreement on the test plan for the chart. I think that aside of chart own tests, we should make sure that chart is also covered by MS e2e test. Those test assume that there is a cluster with MS available and validate that:

• Metrics API is available and gives response any other pod in cluster
• Metrics API exposes metrics about all nodes in cluster
• When deploying CPU consumer with preassigned usage, it returns accurate CPU usage
• When deploying Memory consumer with preassigned usage, it returns accurate Memory usage
• Properly handles sidecar and init containers
• Passes health and readiness probes
• Exposes expected metrics

code: https://github.com/kubernetes-sigs/metrics-server/blob/master/test/e2e_test.go

All those tests are independent on MS deployment method and should pass on both manifests and chart. I would want to add running e2e tests for charts to run as part of PR, but not be required to pass. This way we can detect early breakage and assign chart owners to fix it.

[stevehipwell]

@serathius as long as the gh-pages branch isn't protected what you've done should be good. You might also want to configure Artifact Hub both for discovery and to show that the chart is verified.

RE E2E testing that makes sense but could we get the chart merged first so the configuration of this can be added in a separate PR by someone who knows how the MS E2E setup is configured better than I do? That said I'd be happy to review the PR for this to make sure that the chart was being installed with the correct values.

[serathius]

RE E2E testing that makes sense but could we get the chart merged first so the configuration of this can be added in a separate PR by someone who knows how the MS E2E setup is configured better than I do? That said I'd be happy to review the PR for this to make sure that the chart was being installed with the correct values.

Sounds Good,
/lgtm

Leaving approval to @s-urbaniak

[s-urbaniak]

Thank you everybody! 🙇
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: s-urbaniak, stevehipwell

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

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [s-urbaniak]

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

[onedr0p]

Shouting from the top of the roof, a big thanks to everyone involved.

[stevehipwell]

@serathius @s-urbaniak I'll create a PR to fix the release workflow which seems to be broken.

[samhopwell]

This is amazing news, thank you everyone!

[stevehipwell]

@serathius the release workflow is functioning correctly but the gh-pages branch is currently protected which isn't supported, the cluster-autoscaler team, who use the same workflow, started off with a protected gh-pages branch but have now unprotected it. The solution is to un-protect the branch for future releases and then generate and manually add the index.yaml file to gh-pages. I could open a PR to gh-pages with the index in it?

[serathius]

sg, will unprotect the branch.

[serathius]

done

[stevehipwell]

@serathius I've created #822 to publish the chart.

[stevehipwell]

Thanks to the hard work by @serathius we now have the official Metrics Server helm chart published.

helm repo add metrics-server https://kubernetes-sigs.github.io/metrics-server/
helm repo update
helm search repo metrics-server

Now we just need to get the repo registered in Artifact Hub.

[pierluigilenoci]

I can't believe it! ❤️
Thank you all!!!

[stevehipwell]

@serathius would you be willing to sponsor me to join the Kubernetes SIGs GitHub organisation so I can be setup to review Helm chart PRs?