docs: add Helm installation guide

[tejassinghbhati]

Description

The chart has been in the repo since #163 but Helm is not mentioned anywhere in the book. The install page only covered release manifests, kustomize and static pods, so there was no documented Helm path at all.

Adds a Helm section to the installation guide covering install from a checkout, since OCI chart releases are still WIP, which values turn on metrics, TLS and the validating webhook, and how to ship rules through nodeReadinessRules.

It also documents two things that are specific to Helm and easy to get caught by:

Helm installs the CRD from crds/ on first install only and never touches it again, so a chart bump that changes the schema needs the CRD applied by hand or rules using new fields get rejected while the controller supports them. This is in the chart README already but not where someone following the install guide would see it.

Rules declared in nodeReadinessRules are release resources, so helm uninstall deletes them together with the controller. The taint finalizer then has no controller left to run it, which is the stuck-resource case already documented further down the same page.

Related Issue

Fixes #387

Type of Change

/kind documentation

Testing

Docs only, no code change. I checked every value and default against the chart rather than writing from memory. leaderElection.enabled true, metrics.enabled and metrics.secure false, webhook.enabled and validatingWebhook.enabled false, certManager.enabled false. The ValidatingWebhookConfiguration is gated on and .Values.webhook.enabled .Values.validatingWebhook.enabled, which is why the guide says both are needed.

I also rendered the exact commands in the guide. The default install produces the controller, RBAC and service account only. The full one produces what the table promises:

      2 kind: Certificate
      6 kind: ClusterRole
      2 kind: ClusterRoleBinding
      1 kind: Deployment
      1 kind: Issuer
      1 kind: Role
      1 kind: RoleBinding
      2 kind: Service
      1 kind: ServiceAccount
      1 kind: ValidatingWebhookConfiguration

Checklist

• make test passes
• make lint passes

Does this PR introduce a user-facing change?

NONE

[netlify]

✅ Deploy Preview for node-readiness-controller canceled.

Name	Link
🔨 Latest commit	e65dc47
🔍 Latest deploy log	https://app.netlify.com/projects/node-readiness-controller/deploys/6a9972a4abf0a60008d22e02

[kubernetes-prow]

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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

Details

Needs approval from an approver in each of these files:

• OWNERS

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

[kubernetes-prow]

Hi @tejassinghbhati. 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.

Details

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-sigs/prow repository.

[Ayush4958]

It's great documentation but i got some suggestion for it :-

• we should add helm upgrade section after initial installing it can be great for use
• mention that users can supply their own overrides file using -f custom-values.yaml

[tejassinghbhati]

Good calls, added both in 9fde2a2.

There is a values file section now with helm show values to get a starting point, and an upgrade section covering helm upgrade, --install, and helm diff if you have the plugin. Pointed it at the CRD note too since Helm skips the CRD on upgrade.

[tejassinghbhati]

Rebased on main and added a section for the controller tuning values that landed in #392, since they were not in the chart when I opened this.

Checked all six flags exist in cmd/main.go and rendered them to confirm they only get passed when moved off their defaults. Chart tests are at 31 now, all green.

[tejassinghbhati]

Rebased for the chart rename in #407, so the install, upgrade and uninstall commands and the CRD path all use node-readiness-controller now. Also added a section for the tuning values from #392 since those landed after I opened this.

One extra thing in here that predates the PR, flagging it so it does not look random. The Verification section looked for pods with -l component=node-readiness-controller, and nothing sets a component label, not the chart and not the kustomize manifests. So that command matched nothing on either install path. Both set control-plane=controller-manager so I switched to that. It is right below the new Helm section, so someone following this guide hits it straight after installing.

Happy to pull that into its own PR if you would rather keep this one to Helm.

[tejassinghbhati]

Backed out the verification selector change in f3057ce, and I got it wrong twice over.

#401 was already filed and assigned for this two days before I touched it, so it was not mine to fix. And my analysis was wrong anyway. I said nothing sets a component label. examples/static-pod/node-readiness-controller.yaml sets it, and it is the only place in the repo that does. That pod has component and tier and no control-plane label, so switching the selector to control-plane=controller-manager would have broken verification for static pod installs, the one path where the command currently works.

So this PR is back to just the Helm section plus the rename and tuning updates. Sorry for the noise.

[ajaysundark]

This is now available for 0.5.0. Can you validate these steps and update the PR with changes?

[tejassinghbhati]

Updated in the latest commit to use the official
egistry.k8s.io OCI registry for chart install, values inspection, and upgrades (with a note kept for local checkout development).

[tejassinghbhati]

Validated against the published chart and updated in 0c54cbf.

It pulls fine, appVersion v0.5.0, so the install commands are right as written.

Three things came out of actually running them. The chart version was in a variable called VERSION, which Option 1 already uses for the release tag, and they are different numbers, so that is now CHART_VERSION. The CRD upgrade step reused that same VERSION set way back in Option 1, so anyone arriving via Helm never had it, it now derives the release from the chart appVersion.

The last one is worth a look beyond this PR. Installing from a local checkout deploys v0.4.1 right now, because build-helm injects version and app-version at package time and Chart.yaml keeps a placeholder. Same at the v0.5.0 tag. I have added --set image.tag to that snippet so the docs are not misleading, but the underlying thing might be worth fixing properly.

[ajaysundark]

@vitorfloriano is this because the chart cannot carry the "vX.Y.Z" pattern?

Could you help check if there's room for unifying / fixing this? Having it different would cause discovery issues as we have to keep documenting the mapping in our documentation somewhere.

[tejassinghbhati]

Not a Helm limitation, I checked. Helm takes v0.5.0 as a chart version fine, it packages, lints and renders, and helm show chart reads it back as v0.5.0.

The v gets dropped by one line in the Makefile:

CHART_VERSION ?= $(shell echo "$(RELEASE_VERSION)" | sed 's/^v//')

That came in with #402, so the split is a choice rather than a constraint. Unifying is deleting that sed and setting Chart.yaml to v0.5.0. ct is not in the way either, check-version-increment is already false.

The case for leaving it stripped is that semver itself has no v and most published charts use the plain number, so anything parsing chart versions strictly expects 0.5.0. Helm is lenient, other consumers may not be.

Happy to do it if you want it. It touches the Makefile, Chart.yaml and the version check I added in #453, and it would remove the CHART_VERSION vs VERSION split from these docs, which is the discovery problem you are describing.

[ajaysundark]

We don't need to document this option. You can remove

[tejassinghbhati]

Removed in 9351d16.

[ajaysundark]

I don't think the user installation documention need to capture the make commands used to create the charts

[tejassinghbhati]

Gone with the local checkout note in 9351d16.

[ajaysundark]

We should document the overrides possible, and show a sample how to use it in helm command. We don't need to be opinionated here about whether the user wants to use set or inject values via yaml

[tejassinghbhati]

Reworked in 9351d16. It now points at helm show values for the full list and shows both --set and -f without picking a side.

[ajaysundark]

This feels like a hack. Are we better off not (documenting) supporting the rules array in helm chart for now. we could do a second look at the UX.

[tejassinghbhati]

Agreed, dropped the whole section in 9351d16, along with the uninstall caution that depended on it. The chart still supports the value, it is just not documented until the UX gets a second look.