Add helm chart by honghainguyen777 · Pull Request #163 · kubernetes-sigs/node-readiness-controller

honghainguyen777 · 2026-03-12T09:47:51Z

Description

Add a Helm chart for deploying the node-readiness-controller.

This chart installs the node-readiness-controller along with the required
Kubernetes resources, including:

Deployment for the controller
RBAC resources
Metrics and webhook services
ValidatingWebhookConfiguration
NodeReadinessRule CRD
Optional NodeReadinessRule resources configurable via values.yaml

The chart follows conventions used by existing charts in https://github.com/kubernetes-sigs
(e.g. descheduler) and supports customization via Helm values.

Relationship to #128

This PR overlaps with #128 (feat: provision helm chart). I missed that existing PR when opening this one. Depending on maintainer preference, this PR can either supersede #128 or be reconciled with it so we land a single Helm chart implementation.

Related Issue

None

Type of Change

/kind feature

Testing

The chart was tested locally using Helm:

helm lint charts/nrr-controller
helm template charts/nrr-controller
helm install nrr charts/nrr-controller --namespace node-readiness-controller --create-namespace

Verified:

Controller deployment starts successfully
CRD is installed
Optional NodeReadinessRule resources render correctly when defined in values.yaml
Chart renders cleanly with helm template

Checklist

Does this PR introduce a user-facing change?

Adds a Helm chart for deploying the node-readiness-controller, including
CRD installation and optional NodeReadinessRule resources configurable
via Helm values.

Doc #(issue)

linux-foundation-easycla · 2026-03-12T09:47:57Z

The committers listed above are authorized under a signed CLA.

✅ login: honghainguyen777 / name: Hong Hai Nguyen (0448e14, 1e6ebe9, 322ea13, 541447f, 68e3085, 6ee70d7, 7f5f1e6, 82682c0, aa569f7, abd64de, c3c9a0f, ce68415, eae8d21, ff2a3a1)
✅ login: sahitya-chandra / name: Sahitya Chandra (cabd2f7)

k8s-ci-robot · 2026-03-12T09:48:00Z

Welcome @honghainguyen777!

It looks like this is your first PR to kubernetes-sigs/node-readiness-controller 🎉. 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/node-readiness-controller 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. 😃

k8s-ci-robot · 2026-03-12T09:48:01Z

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

netlify · 2026-03-12T09:48:05Z

✅ Deploy Preview for node-readiness-controller ready!

Name	Link
🔨 Latest commit	`68e3085`
🔍 Latest deploy log	https://app.netlify.com/projects/node-readiness-controller/deploys/6a01ef37d2f49b000851d66b
😎 Deploy Preview	https://deploy-preview-163--node-readiness-controller.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

sahitya-chandra

Thanks for working on this chart. I tested the PR locally from the fetched review-pr-163 branch

Commands/results:

helm lint charts/nrr-controller passed
helm template test charts/nrr-controller --namespace node-readiness-system rendered successfully
helm template test charts/nrr-controller --namespace node-readiness-system --set webhook.enabled=true --set validatingWebhook.enabled=true --set certManager.enabled=true rendered successfully
helm package ./charts/nrr-controller --dependency-update --destination /tmp/nrr-chart-package passed
Installed into a 3-node kind cluster using the PR’s make kind-multi-node
helm install nrr-test charts/nrr-controller --namespace node-readiness-system --create-namespace --wait --timeout 2m succeeded
Controller pod became Ready, the CRD was installed, and a sample NodeReadinessRule created through chart values reconciled successfully across all 3 kind nodes

I also installed the same Helm unittest plugin version used by the workflow and ran:
helm unittest charts/nrr-controller --strict

That currently fails with 4 failing tests:

so the rendered Deployment contains annotations by default

the plugin reports these as expect ... to be an array. These assertions likely need to use equal or target the parent arrays instead

Cc @ajaysundark

honghainguyen777 · 2026-05-08T09:54:39Z

Hi @sahitya-chandra, thanks a lot for the detailed testing and feedback!

I’d actually love to get more contributors involved with the chart so we can maintain it well going forward, so a follow-up fix from you would be more than welcome :)

kaisoz · 2026-05-08T09:57:44Z

/assign @ajaysundark

for the final review

kaisoz · 2026-05-08T09:58:30Z

/ok-to-test

sahitya-chandra · 2026-05-08T10:00:48Z

pushing the unittest fix in a bit

sahitya-chandra · 2026-05-08T10:05:08Z

/retest

sahitya-chandra · 2026-05-08T10:09:54Z

Oh, I forgot that I don't have permissions to directly push to this branch, pushed to my fork instead, @honghainguyen777 can cherry-pick this f9b0a6a or I can raise a pr once this pr gets merged

passed locally:

sahitya-chandra · 2026-05-08T10:20:38Z

@@ -0,0 +1 @@
+${CONTAINER_ENGINE:-docker} run -it --rm --network host --workdir=/data --volume ~/.kube/config:/root/.kube/config:ro --volume $(pwd):/data quay.io/helmpack/chart-testing:v3.7.0 /bin/bash -c "git config --global --add safe.directory /data; ct install --config=.github/ci/ct.yaml --helm-extra-set-args=\"--set=kind=Deployment\""


that Prow failure is because the current file starts directly with:

${CONTAINER_ENGINE:-docker} run ...

It needs a shell header like the repo expects:

#!/usr/bin/env bash # Copyright The Kubernetes Authors. # # Licensed under the Apache License, Version 2.0

Ah, I just added it :)
Reference: https://github.com/kubernetes/kubernetes/blob/master/hack/boilerplate/boilerplate.sh.txt

k8s-ci-robot · 2026-05-08T10:27:50Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: honghainguyen777
Once this PR has been reviewed and has the lgtm label, please ask for approval from ajaysundark. 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

honghainguyen777 · 2026-05-08T10:30:38Z

@sahitya-chandra I have cherry-picked your commit. Thank you very much! <3

honghainguyen777 · 2026-05-08T10:40:56Z

Tests passed:

@sahitya-chandra could you take a second look before @ajaysundark does the final review?

sahitya-chandra · 2026-05-08T10:51:39Z

Re-reviewed, No blocking issues from my side, looks good

kaisoz · 2026-05-08T10:53:06Z

/lgtm

since @sahitya-chandra who is more familiar with the project than me has reviewed it twice

ajaysundark · 2026-05-09T12:10:32Z

@@ -0,0 +1,28 @@
+{{- if and .Values.webhook.enabled .Values.validatingWebhook.enabled }}
+apiVersion: admissionregistration.k8s.io/v1
+kind: ValidatingWebhookConfiguration


When I tried this chart, the webhook service was unreachable by API? Did you get a chance to test with webhook?

Good catch. The issue was that the ValidatingWebhookConfiguration did not set clientConfig.service.port, so the API server defaulted to 443 while the chart service default was 8443. I was too focused on following the install-full.yaml.

I fixed this by aligning the chart default with upstream Kustomize (443 -> 9443) and explicitly wiring .Values.webhook.service.port into the ValidatingWebhookConfiguration.

ajaysundark · 2026-05-09T12:13:31Z

+
+affinity: {}
+
+# NOTE: Before enabling these rules. The CRD `nodereadinessrules.readiness.node.x-k8s.io` must already be installed in the cluster


I think this will have install time race with controller / webhook. how do we ensure cert-manager has issued certificates or webhook is serving?

Agreed. Creating NodeReadinessRule resources in the same install that enables the validating webhook can race with cert-manager issuing certificates and the webhook becoming ready.

I updated the chart comments/README to call this out. The safer flow is to install the controller/webhook first, wait until it is ready, and then apply nodeReadinessRules in a follow-up helm upgrade.

ajaysundark · 2026-05-09T12:14:41Z

+  enforcementMode: {{ $rule.enforcementMode | quote }}
+  nodeSelector:
+{{- toYaml $rule.nodeSelector | nindent 4 }}


this is missing some fields like dryrun etc

Fixed. The template now renders the full rule spec except for name, so fields such as dryRun are preserved instead of being individually enumerated. I added helm-unittest coverage for this.

ajaysundark · 2026-05-09T12:17:17Z

+  taint:
+{{- toYaml $rule.taint | nindent 4 }}
+  enforcementMode: {{ $rule.enforcementMode | quote }}
+  nodeSelector:


what if $rule.nodeSelector was omitted in values?

Fixed. If nodeSelector is omitted from a rule in values, the template now defaults it to {} so the generated resource satisfies the CRD-required field. I added test coverage for omitted and provided nodeSelector.

ajaysundark · 2026-05-09T12:21:12Z

@@ -0,0 +1,18 @@
+kind: Cluster


is there a reason why existing config/testing/kind/*.configs cannot be leveraged?

Good point. I removed the new hack/kind_config.yaml and updated kind-multi-node to reuse the existing config/testing/kind/kind-3node-config.yaml.

ajaysundark · 2026-05-09T12:23:03Z

+## TL;DR:
+
+```shell
+helm repo add node-readiness-controller https://kubernetes-sigs.github.io/node-readiness-controller/


would this work for helm upgrade? how does future schema changes reach existing installs?

Documented this in the chart README. Helm installs CRDs from the chart crds/ directory during initial install, but does not upgrade or delete those CRDs during helm upgrade or helm uninstall.

For future schema changes, users need to apply the updated CRD before upgrading to a chart version that depends on it. Moving CRDs into templates/ solves the problem, but the CRD lifecycle becomes more dangerous. Alternatively, we can add a pre-install/pre-upgrade hook Job that runs kubectl apply for CRDs. For this PR I kept Helm’s standard crds/ behavior and documented that schema-changing upgrades require applying the updated CRD first.

ajaysundark · 2026-05-09T12:33:55Z

@@ -0,0 +1,17 @@
+#!/usr/bin/env bash


Can we also add a hack/verify-chart-drift.sh and wire it into both hack/verify-all.sh and .github/workflows/helm.yaml. It doesnt have to be exhaustive check, maybe a simple diff check to match controller-gen output to chart?

Added hack/verify-chart-drift.sh and wired it into both hack/verify-all.sh and .github/workflows/helm.yaml. The check runs make manifests and diffs the controller-gen CRD against the chart CRD.

ajaysundark · 2026-05-11T04:47:17Z

+    admissionReviewVersions:
+      {{- toYaml .Values.validatingWebhook.admissionReviewVersions | nindent 6 }}
+    clientConfig:
+      service:


@honghainguyen777 Your values.yaml sets this default to 8443, but this service config doesnt have a wiring for 'port' field. This seems why webhook was failing.

Can we fix this and also add tests to include coverage for webhook service?

Fixed and added the tests

ajaysundark · 2026-05-11T04:58:56Z

+    port: 8443
+    targetPort: 9443


Note: this conflicts with current upstream config/webhook/service.yaml that uses 443 -> 9443. This is why we should have guardrails to maintain configuration consistency between helm and kustomize artifacts.

Fixed. The chart default now matches upstream config/webhook/service.yaml: 443 -> 9443.

k8s-ci-robot · 2026-05-11T14:40:49Z

New changes are detected. LGTM label has been removed.

k8s-ci-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Mar 12, 2026

k8s-ci-robot requested review from ajaysundark and tallclair March 12, 2026 09:47

k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Mar 12, 2026

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Mar 12, 2026

k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Mar 12, 2026

honghainguyen777 force-pushed the add-helm-chart branch from eb0d1d7 to 9035670 Compare March 20, 2026 12:39

sahitya-chandra reviewed May 8, 2026

View reviewed changes

k8s-ci-robot assigned ajaysundark May 8, 2026

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels May 8, 2026

sahitya-chandra reviewed May 8, 2026

View reviewed changes

honghainguyen777 requested a review from sahitya-chandra May 8, 2026 10:35

ajaysundark reviewed May 9, 2026

View reviewed changes

ajaysundark reviewed May 11, 2026

View reviewed changes

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 11, 2026

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 11, 2026

honghainguyen777 and others added 15 commits May 11, 2026 16:48

Add nrr-controller Helm chart

eae8d21

Update README.md & remove temp files

82682c0

Add test config

6ee70d7

Add missing eof

1e6ebe9

Fix wrong helm branch - github workflows

c3c9a0f

Update crds & rbac & tolerations for image v0.3.0

541447f

Fix Helm chart unit test assertions

cabd2f7

Align pod annotation assertions with defaults

aa569f7

Add boilerplate header to verify-chart.sh

abd64de

Fix wire validating webhook service port

ff2a3a1

Render full NodeReadinessRule specs

ce68415

Add CRD drift verification

322ea13

Reuse existing kind multi-node config

0448e14

Add dryRun to rule example

7f5f1e6

Sync chart CRD with generated output

68e3085

honghainguyen777 force-pushed the add-helm-chart branch from 711cd53 to 68e3085 Compare May 11, 2026 15:01

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 11, 2026

honghainguyen777 requested a review from ajaysundark May 11, 2026 15:07

		@@ -0,0 +1 @@
		${CONTAINER_ENGINE:-docker} run -it --rm --network host --workdir=/data --volume ~/.kube/config:/root/.kube/config:ro --volume $(pwd):/data quay.io/helmpack/chart-testing:v3.7.0 /bin/bash -c "git config --global --add safe.directory /data; ct install --config=.github/ci/ct.yaml --helm-extra-set-args=\"--set=kind=Deployment\""


		affinity: {}

		# NOTE: Before enabling these rules. The CRD `nodereadinessrules.readiness.node.x-k8s.io` must already be installed in the cluster

Conversation

honghainguyen777 commented Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Relationship to #128

Related Issue

Type of Change

Testing

Checklist

Does this PR introduce a user-facing change?

Uh oh!

linux-foundation-easycla Bot commented Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k8s-ci-robot commented Mar 12, 2026

Uh oh!

k8s-ci-robot commented Mar 12, 2026

Uh oh!

netlify Bot commented Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for node-readiness-controller ready!

Uh oh!

sahitya-chandra left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

honghainguyen777 commented May 8, 2026

Uh oh!

kaisoz commented May 8, 2026

Uh oh!

kaisoz commented May 8, 2026

Uh oh!

sahitya-chandra commented May 8, 2026

Uh oh!

sahitya-chandra commented May 8, 2026

Uh oh!

sahitya-chandra commented May 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented May 8, 2026

Uh oh!

honghainguyen777 commented May 8, 2026

Uh oh!

honghainguyen777 commented May 8, 2026

Uh oh!

sahitya-chandra commented May 8, 2026

Uh oh!

kaisoz commented May 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

honghainguyen777 commented Mar 12, 2026 •

edited

Loading

linux-foundation-easycla Bot commented Mar 12, 2026 •

edited

Loading

netlify Bot commented Mar 12, 2026 •

edited

Loading

sahitya-chandra left a comment •

edited

Loading

sahitya-chandra commented May 8, 2026 •

edited

Loading

kaisoz commented May 8, 2026 •

edited

Loading