Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions .github/workflows/helm.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -79,3 +79,46 @@ jobs:

- name: Run chart-testing (install)
run: ct install --config=.github/ci/ct.yaml

# The pre-install guard calls Helm's lookup, which returns nothing during
# `helm template` and `--dry-run`, so the chart unit tests cannot cover it.
# Exercise it against the live kind cluster instead.
#
# The signal is the controller Deployment, not the CRD: Helm applies crds/
# before rendering, so keying on the CRD would refuse every first install.
- name: Verify the duplicate-install pre-install check
run: |
set -euo pipefail
CHART=charts/node-readiness-controller

echo "::group::a fresh install into an empty cluster succeeds with the guard ON"
# This is the path the CRD-based guard broke: crds/ is applied before the
# templates render, so the check must not trip on this install's own CRD.
helm install nrc-guard "$CHART" \
--namespace nrc-guard --create-namespace --wait --timeout 5m
echo "::endgroup::"

echo "::group::a second install is refused while the first is running"
if helm install nrc-guard-2 "$CHART" \
--namespace nrc-guard-2 --create-namespace 2>guard.err; then
echo "expected the second install to be refused, but it succeeded"
exit 1
fi
cat guard.err
grep -q "already installed in this cluster" guard.err
echo "::endgroup::"

echo "::group::escape hatch allows a second install anyway"
helm install nrc-guard-2 "$CHART" \
--namespace nrc-guard-2 --create-namespace \
--set crds.preInstallCheck=false --wait --timeout 5m
echo "::endgroup::"

echo "::group::upgrade of the first release is not blocked, with the guard enabled"
helm upgrade nrc-guard "$CHART" \
--namespace nrc-guard --wait --timeout 5m
echo "::endgroup::"

helm uninstall nrc-guard-2 --namespace nrc-guard-2
helm uninstall nrc-guard --namespace nrc-guard
kubectl delete namespace nrc-guard nrc-guard-2
11 changes: 11 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,17 @@ vet: ## Run go vet against code.
.PHONY: manifests
manifests: $(CONTROLLER_GEN) ## Generate WebhookConfiguration, ClusterRole and CustomResourceDefinition objects.
$(CONTROLLER_GEN) rbac:roleName=manager-role crd webhook paths="./..." output:crd:artifacts:config=config/crd/bases
$(MAKE) sync-chart-crds

.PHONY: sync-chart-crds
sync-chart-crds: ## Regenerate the chart's CRD copy from config, stamping the Helm installer label. Run by `manifests`.
# The chart ships the CRD from crds/ (which Helm does not template), so its only
# difference from controller-gen output is the standard app.kubernetes.io/managed-by
# installer label: kustomize in config/, helm in the chart. Never hand-edit the
# chart CRD; `make manifests` regenerates it and verify-chart-drift.sh checks it in.
sed 's#\(app.kubernetes.io/managed-by:\) kustomize#\1 helm#' \
config/crd/bases/readiness.node.x-k8s.io_nodereadinessrules.yaml \
> charts/node-readiness-controller/crds/nodereadinessrules.readiness.node.x-k8s.io.yaml

.PHONY: generate
generate: $(CONTROLLER_GEN) ## Generate code containing DeepCopy, DeepCopyInto, and DeepCopyObject method implementations.
Expand Down
8 changes: 8 additions & 0 deletions api/v1alpha1/nodereadinessrule_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,14 @@ type DryRunResults struct {
}

// +kubebuilder:object:root=true
// Stamp the CRD with the standard app.kubernetes.io/managed-by label so the
// controlling installation is visible in clusters where the control plane is
// hosted and the controller itself is not. The value is the installer: this
// kustomize package stamps "kustomize" (matching every other resource under
// config/); the Helm chart ships "helm"; providers override it to their own name
// in their install pipeline. See docs/book/src/user-guide/installation.md.
//
// +kubebuilder:metadata:labels=app.kubernetes.io/managed-by=kustomize
// +kubebuilder:subresource:status
// +kubebuilder:resource:scope=Cluster,shortName=nrr
// +kubebuilder:printcolumn:name="Mode",type=string,JSONPath=`.spec.enforcementMode`,description="The enforcement mode of the rule: bootstrap-only or continuous."
Expand Down
31 changes: 31 additions & 0 deletions charts/node-readiness-controller/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,36 @@ The command deploys the _node-readiness-controller_ on the Kubernetes cluster in

Helm installs CRDs from the chart `crds/` directory during initial install, but Helm does not upgrade or delete CRDs from that directory during `helm upgrade` or `helm uninstall`. Before upgrading to a chart version that changes the `NodeReadinessRule` schema, apply the updated CRD from the release artifacts or from `charts/node-readiness-controller/crds`.

## Avoiding a duplicate installation

The controller manages node taints. Two installations reconciling the same
`NodeReadinessRule` resources will both act on the same nodes, so only one should run
in a cluster.

This is easy to do by accident on a managed cluster: the provider may run the
controller as part of a hosted control plane, where it is not visible to you.

To make the existing installation visible, the CRD carries the standard
`app.kubernetes.io/managed-by` label naming the installer. The CRD is cluster scoped
and shared by every install flow, so its value is whichever installation created it:
`helm` from this chart, `kustomize` from the config manifests, or a provider's own name
when they override it.

```console
$ kubectl get crd nodereadinessrules.readiness.node.x-k8s.io \
-o jsonpath='{.metadata.labels.app\.kubernetes\.io/managed-by}'
helm
```

A fresh `helm install` refuses to proceed when a controller Deployment is already
running in the cluster, and names it in the error. (The guard keys on the running
workload rather than the CRD, because Helm installs the CRD from `crds/` before it
renders templates, so the CRD always exists by the time the check runs.) `helm upgrade`
is unaffected. Set `crds.preInstallCheck=false` to install anyway.

Providers installing the controller should override the label value in their own
pipeline, so the manager shown above identifies them rather than the upstream default.

## Uninstalling the Chart

To uninstall/delete the `my-release` deployment:
Expand Down Expand Up @@ -69,6 +99,7 @@ The following table lists the configurable parameters of the _node-readiness-con
| `leaderElection.enabled` | Enable leader election to support multiple replicas | `true` |
| `leaderElection.namespace` | Namespace for the leader election lease. Defaults to the release namespace when empty. | `""` |
| `priorityClassName` | The name of the priority class to add to pods | `system-cluster-critical` |
| `crds.preInstallCheck` | Fail a fresh install when the `NodeReadinessRule` CRD already exists, so a second controller cannot be installed alongside one you cannot see. No effect on `helm upgrade`. | `true` |
| `rbac.create` | If `true`, create & use RBAC resources | `true` |
| `resources` | Node Readiness Controller container CPU and memory requests/limits | _see values.yaml_ |
| `serviceAccount.create` | If `true`, create a service account | `true` |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ kind: CustomResourceDefinition
metadata:
annotations:
controller-gen.kubebuilder.io/version: v0.19.0
labels:
app.kubernetes.io/managed-by: helm
name: nodereadinessrules.readiness.node.x-k8s.io
spec:
group: readiness.node.x-k8s.io
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
{{- /*
Refuse to install a second node-readiness-controller into a cluster that already
has one. Two controllers reconciling the same NodeReadinessRules compete over the
same node taints. That is easy to do by accident where the control plane is hosted:
the provider's installation is not visible to the user, so nothing signals that the
controller is already running.

Two independent checks run below; either one aborts the install.

1. CRD ownership (value) check. The cluster-scoped NodeReadinessRule CRD carries the
standard app.kubernetes.io/managed-by label naming its installer. This chart's
crds/ copy stamps `helm` (crds.managedBy). Helm applies crds/ *before* it renders
templates, so on a self install the CRD this install just created already carries
our own value -- it matches and passes. A value that is present but DIFFERENT
belongs to another installer: `kustomize`, or a hosted provider's `<provider>`.
This catches an invisible provider install even when its workload is renamed or
hidden, which the Deployment check below cannot see. It also naturally allows a
`helm uninstall` -> `helm install` cycle, since Helm leaves crds/ in place and the
value still matches.

2. Deployment (workload) check. Catches a duplicate that carries the SAME managed-by
value (for example a second Helm release of this chart), which the value check
cannot distinguish. We match any Deployment in any namespace carrying
app.kubernetes.io/name=node-readiness-controller, the label this chart stamps.
The Deployment is applied after rendering, so one found here belongs to a
different, already-running installation. Flows that rename the workload are not
seen here but are caught by check 1 via the CRD label; see the installation guide.

Lookup foot-guns handled below:
* lookup with an empty name returns a *list* that is truthy even when it matched
nothing, so we range .items and count real matches.
* a Deployment (or CRD) may carry no labels at all, so we default the label map
before indexing to avoid a nil panic.

lookup returns nothing during "helm template" and "--dry-run", so this only fires on
a real install.
*/ -}}
{{- if and .Values.crds.preInstallCheck .Release.IsInstall }}
{{- $expected := .Values.crds.managedBy }}
{{- /* 1. CRD ownership (value) check. */ -}}
{{- $crd := lookup "apiextensions.k8s.io/v1" "CustomResourceDefinition" "" "nodereadinessrules.readiness.node.x-k8s.io" }}
{{- if $crd }}
{{- $owner := get ($crd.metadata.labels | default dict) "app.kubernetes.io/managed-by" }}
{{- if and $owner (ne $owner $expected) }}
{{- fail (printf "node-readiness-controller is already installed in this cluster by a different installer.\n\nThe NodeReadinessRule CRD is labelled app.kubernetes.io/managed-by=%s, but this install claims %q.\n\nAnother installation -- often a hosted provider's, running in the control plane where\nit is not visible to you -- already owns the cluster-scoped CRD. Installing this chart\nwould make two controllers manage the same node taints.\n\nTo adopt the existing installation, run 'helm upgrade' against its release.\nTo install anyway, set crds.preInstallCheck=false." $owner $expected) }}
{{- end }}
{{- end }}
{{- /* 2. Deployment (workload) check. */ -}}
{{- $existing := "" }}
{{- range (lookup "apps/v1" "Deployment" "" "").items }}
{{- $labels := .metadata.labels | default dict }}
{{- if eq (get $labels "app.kubernetes.io/name") "node-readiness-controller" }}
{{- $existing = printf "%s/%s" .metadata.namespace .metadata.name }}
{{- end }}
{{- end }}
{{- if $existing }}
{{- fail (printf "node-readiness-controller is already installed in this cluster.\n\nThe Deployment %s is already running the controller.\n\nInstalling a second controller will make both of them manage the same node taints.\nIf this is a managed cluster, your provider may have installed the controller as\npart of the control plane, where it is not visible to you.\n\nTo upgrade the existing installation, run 'helm upgrade' against its release.\nTo install anyway, set crds.preInstallCheck=false." $existing) }}
{{- end }}
{{- end }}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ apiVersion: readiness.node.x-k8s.io/v1alpha1
kind: NodeReadinessRule
metadata:
name: {{ required "nodeReadinessRules[]: name is required" $rule.name | quote }}
labels:
{{- include "node-readiness-controller.labels" $ | nindent 4 }}
spec:
{{- toYaml $spec | nindent 2 }}
{{- end }}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
suite: Test duplicate-install pre-install check

templates:
- "*.yaml"

release:
name: node-readiness-controller

tests:
# The guard calls lookup, which returns nothing outside a live cluster. These
# specs therefore cover rendering only: that the template is well formed, that
# it emits no manifests of its own, and that toggling the value does not break
# the chart. Whether the guard actually allows a first install and refuses a
# second is covered in the Helm CI workflow, which installs against a real cluster.
- it: emits no manifests when the check passes
template: templates/crd-ownership-check.yaml
asserts:
- hasDocuments:
count: 0

- it: emits no manifests when the check is disabled
template: templates/crd-ownership-check.yaml
set:
crds.preInstallCheck: false
asserts:
- hasDocuments:
count: 0

- it: renders cleanly with a custom managed-by identity
template: templates/crd-ownership-check.yaml
set:
crds.managedBy: example-provider
asserts:
- hasDocuments:
count: 0

- it: does not affect the rest of the chart when disabled
template: templates/deployment.yaml
set:
crds.preInstallCheck: false
asserts:
- isKind:
of: Deployment
18 changes: 18 additions & 0 deletions charts/node-readiness-controller/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,24 @@ controller:
# -- Bind address for the pprof endpoint. Leave empty to disable.
pprofBindAddress: ""

# CustomResourceDefinition handling.
#
# The CRD is installed from the chart's crds/ directory, which Helm does not
# template and does not upgrade. See the chart README for upgrade guidance.
crds:
# -- Fail a fresh install when another node-readiness-controller already owns the
# cluster: either the NodeReadinessRule CRD is labelled with a different
# `managed-by` installer, or a controller Deployment is already running. Two
# controllers reconciling the same rules compete over the same node taints, which
# is easy to do by accident on a managed cluster where the provider's installation
# is not visible. Has no effect on `helm upgrade`.
preInstallCheck: true
# -- The installer identity this chart claims, compared against the existing CRD's
# `app.kubernetes.io/managed-by` label by the pre-install check. Must match the
# value stamped in the chart's `crds/` CRD (`helm`); only change it if you also
# re-stamp that CRD (for example when repackaging the chart for a provider).
managedBy: helm

rbac:
create: true

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ kind: CustomResourceDefinition
metadata:
annotations:
controller-gen.kubebuilder.io/version: v0.19.0
labels:
app.kubernetes.io/managed-by: kustomize
name: nodereadinessrules.readiness.node.x-k8s.io
spec:
group: readiness.node.x-k8s.io
Expand Down
85 changes: 85 additions & 0 deletions docs/book/src/user-guide/installation.md
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,91 @@ example on deploying the controller as a static pod in a kind cluster.
This is typically handled via a bootstrap script or post-install job in a `kubeadm` setup.

---
## Avoiding a Duplicate Installation

The controller manages node taints, so only one installation should run in a cluster.
Two installations reconciling the same `NodeReadinessRule` resources will both act on
the same nodes.

This is easy to do by accident on a managed cluster. Where the control plane is hosted,
the provider may already run the controller as part of it, and that installation is not
visible to you in the usual places.

### Finding the existing installation

The CRD is cluster-scoped and singleton, so it is the one artifact every installation
shares. It carries the standard `app.kubernetes.io/managed-by` label naming the
installer that owns it:

```sh
kubectl get crd nodereadinessrules.readiness.node.x-k8s.io \
-o jsonpath='{.metadata.labels.app\.kubernetes\.io/managed-by}'
```

The value is the installer: `helm` from the chart, `kustomize` from the config
manifests, or a provider's own name when they override it (see below).

If the CRD is not present, the controller is not installed.

### For providers

Providers installing the controller should override the label value so it identifies
them rather than the `helm`/`kustomize` default, which makes the hosted installation
self-describing to users:

```yaml
# kustomize patch
apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
name: nodereadinessrules.readiness.node.x-k8s.io
labels:
app.kubernetes.io/managed-by: example-provider
```

### Helm guardrail

A fresh `helm install` runs two checks and aborts if either one finds an existing
installation:

1. **CRD ownership.** If the `NodeReadinessRule` CRD already carries a
`app.kubernetes.io/managed-by` value different from the one this chart claims
(`helm`, set by `crds.managedBy`), another installer owns it — a `kustomize`
install, or a provider's `<provider>`. This catches a hosted provider install
even when its workload is renamed or hidden. A self reinstall passes, because the
CRD Helm leaves behind still carries `helm`.
2. **Running workload.** If a controller Deployment labelled
`app.kubernetes.io/name=node-readiness-controller` is already running, the install
is refused and the Deployment is named. This catches a second Helm release, which
check 1 cannot distinguish because it carries the same `managed-by` value.

The workload check names the offending Deployment in the error:

```console
$ helm install nrc ./charts/node-readiness-controller
Error: node-readiness-controller is already installed in this cluster.

The Deployment kube-system/node-readiness-controller-manager is already running the
controller.
...
```

The guard keys on the running controller Deployment, not on the CRD: Helm installs the
CRD from `crds/` before it renders templates, so the CRD always exists by the time the
check runs. The Deployment is applied after rendering, so one found during the check
belongs to a different, already-running installation.

`helm upgrade` is unaffected. To install anyway, set `crds.preInstallCheck=false`.

> [!NOTE]
> The check only runs during a real install. It is skipped by `helm template` and
> `helm install --dry-run`, which do not query the cluster. It matches Deployments
> labelled `app.kubernetes.io/name=node-readiness-controller`; an installation that
> renames the workload, or does not use this chart, is not detected.

The other install paths -- `kubectl apply` of the release manifests, or Kustomize --
do not have an equivalent guard. Check the label above before installing.

## Verification

After installation, verify that the controller is running successfully.
Expand Down
6 changes: 5 additions & 1 deletion hack/verify-chart-drift.sh
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,12 @@ set -o pipefail
KUBE_ROOT="$(dirname "${BASH_SOURCE[0]}")/.."
cd "${KUBE_ROOT}"

# `make manifests` regenerates config/crd/bases AND the chart's crds/ copy (via
# sync-chart-crds). So this check just runs it and asserts nothing changed: if the
# committed CRDs are stale, or the chart copy was hand-edited, regeneration moves
# them and git diff fails. Both files are generated; neither is edited by hand.
make manifests

diff -u \
git diff --exit-code -- \
config/crd/bases/readiness.node.x-k8s.io_nodereadinessrules.yaml \
charts/node-readiness-controller/crds/nodereadinessrules.readiness.node.x-k8s.io.yaml