Skip to content
Merged
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
4 changes: 2 additions & 2 deletions .claude/CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ The canonical file lives at `.claude/CLAUDE.md`. The root-level `AGENTS.md` is a

Skyhook (being renamed to NodeWright) is a Kubernetes-aware package manager for safely modifying host infrastructure at scale. It coordinates the node lifecycle (cordon → drain → apply package → interrupt/reboot → uncordon) as controlled rollouts gated by interruption budgets and deployment policies.

Rename status: the project is transitioning from Skyhook → NodeWright. The namespace (`skyhook`) still uses `skyhook` pending its migration. Components already moved to `nodewright`: the Go module (`github.com/NVIDIA/nodewright/operator`), the Helm chart (`name: nodewright`, distributed at `oci://ghcr.io/nvidia/nodewright/charts/nodewright`), the operator image (`ghcr.io/nvidia/nodewright/operator`), the agent image (`ghcr.io/nvidia/nodewright/agent`, since `agent/v6.4.2`), the primary CRD group (`nodewright.nvidia.com/v1alpha1 NodeWright`; the legacy `skyhook.nvidia.com` group is kept read-only for a migration window), and the CLI plugin (`kubectl nodewright`, binary `kubectl-nodewright`). Don't "fix" `nodewright` references back to `skyhook`, and don't preemptively rename what hasn't moved yet.
Rename status: the project is transitioning from Skyhook → NodeWright. Components already moved to `nodewright`: the install namespace (`nodewright` for new installs; existing installs stay in `skyhook` forever, since namespaces cannot be renamed in place), the Go module (`github.com/NVIDIA/nodewright/operator`), the Helm chart (`name: nodewright`, distributed at `oci://ghcr.io/nvidia/nodewright/charts/nodewright`), the operator image (`ghcr.io/nvidia/nodewright/operator`), the agent image (`ghcr.io/nvidia/nodewright/agent`, since `agent/v6.4.2`), the primary CRD group (`nodewright.nvidia.com/v1alpha1 NodeWright`; the legacy `skyhook.nvidia.com` group is kept read-only for a migration window), and the CLI plugin (`kubectl nodewright`, binary `kubectl-nodewright`). Don't "fix" `nodewright` references back to `skyhook`, and don't preemptively rename what hasn't moved yet.

## Required reading: `docs/` (load every session)

Expand Down Expand Up @@ -100,7 +100,7 @@ E2E tests use [chainsaw](https://kyverno.github.io/chainsaw/) against a real clu

### CRDs (see `operator/api/v1alpha1/`)

- **Skyhook** (namespaced) — desired state: a DAG of `packages` (container image + version + configMap + optional `dependsOn`), node selector, interruption budget, additional tolerations, runtime-required flag, priority/sequencing, optional `DeploymentPolicy` reference.
- **Skyhook** / **NodeWright** (cluster-scoped) — desired state: a DAG of `packages` (container image + version + configMap + optional `dependsOn`), node selector, interruption budget, additional tolerations, runtime-required flag, priority/sequencing, optional `DeploymentPolicy` reference. Both CRDs are cluster-scoped, so the install namespace only ever determines where the operator and its package pods live, never which CRs a client sees.
- **DeploymentPolicy** (cluster-scoped) — rollout shape: batch sizing, pause/resume windows, cross-Skyhook ordering.

CRD types are kubebuilder-annotated (`//+kubebuilder:…`). Any change to these files **must** be followed by `make manifests generate` — the generated `zz_generated.deepcopy.go`, CRD YAML under `config/crd/bases/`, and webhook config are all consumed at build time.
Expand Down
24 changes: 13 additions & 11 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -76,13 +76,15 @@ Install NodeWright quickly using Helm without downloading the repository:
# Helm 3.8+ supports OCI natively — no `helm repo add` needed.
helm install nodewright oci://ghcr.io/nvidia/nodewright/charts/nodewright \
--version v0.17.1 \
--namespace skyhook \
--namespace nodewright \
--create-namespace
```

> **Where things live:** chart at `oci://ghcr.io/nvidia/nodewright/charts/nodewright`, operator image at `ghcr.io/nvidia/nodewright/operator`, agent image at `ghcr.io/nvidia/nodewright/agent`. NGC / `nvcr.io` distribution is paused — see [docs/release-process.md#distribution-ghcrio-only-for-now](docs/release-process.md#distribution-ghcrio-only-for-now).
>
> **Migrating from `helm repo add skyhook https://helm.ngc.nvidia.com/...`?** Run `helm repo remove skyhook` and use the OCI install above. If you also want to keep the existing in-cluster release name (e.g. `skyhook`), substitute it for `nodewright` in the `helm install` command — the chart works either way.
>
> **Already installed in the `skyhook` namespace?** Stay there. The documented namespace for **new** installs moved from `skyhook` to `nodewright`, but Kubernetes namespaces cannot be renamed in place and Helm cannot move a release between namespaces, so there is nothing to migrate and no deadline. `kubectl nodewright` finds the operator in either namespace automatically. See [docs/nodewright-migration.md#install-namespace](docs/nodewright-migration.md#install-namespace-skyhook---nodewright).

### Configure Image Pull Secrets (if needed)

Expand All @@ -92,7 +94,7 @@ If you're using private container registries, create the necessary secrets:
kubectl create secret generic node-init-secret \
--from-file=.dockerconfigjson=${HOME}/.docker/config.json \
--type=kubernetes.io/dockerconfigjson \
--namespace skyhook
--namespace nodewright
```

**Note:** NodeWright currently uses a single shared image pull secret for all packages, and agent/operator containers. If you need access to multiple registries, combine the credentials into one `dockerconfigjson` secret with multiple registry auths.
Expand All @@ -101,16 +103,16 @@ kubectl create secret generic node-init-secret \

```bash
# Check that the operator is running
kubectl get pods -n skyhook
kubectl get pods -n nodewright

# or Wait for the deployment to be available first
kubectl wait --for=condition=Available deployment -l control-plane=controller-manager -n skyhook --timeout=300s
kubectl wait --for=condition=Available deployment -l control-plane=controller-manager -n nodewright --timeout=300s

# Then wait for the operator pod to be ready
kubectl wait --for=condition=Ready pod -l control-plane=controller-manager -n skyhook --timeout=300s
kubectl wait --for=condition=Ready pod -l control-plane=controller-manager -n nodewright --timeout=300s

# Verify the Ready condition
kubectl get pods -l control-plane=controller-manager -n skyhook -o jsonpath='{.items[0].status.conditions[?(@.type=="Ready")].status}'
kubectl get pods -l control-plane=controller-manager -n nodewright -o jsonpath='{.items[0].status.conditions[?(@.type=="Ready")].status}'

# Verify the CRDs are installed
kubectl get crd | grep nodewright
Expand Down Expand Up @@ -154,7 +156,7 @@ kubectl describe nodewright nodewright-sample

```bash
# Uninstall the chart (cleanup happens automatically)
helm uninstall nodewright --namespace skyhook
helm uninstall nodewright --namespace nodewright
```

The pre-delete hook will:
Expand All @@ -170,13 +172,13 @@ The pre-delete hook will:
To disable automatic cleanup and manage resources manually:

```bash
helm install nodewright ./chart --namespace skyhook --set cleanup.enabled=false
helm install nodewright ./chart --namespace nodewright --set cleanup.enabled=false
```

To adjust the job timeout:

```bash
helm install nodewright ./chart --namespace skyhook \
helm install nodewright ./chart --namespace nodewright \
--set cleanup.jobTimeoutSeconds=180
```

Expand All @@ -193,7 +195,7 @@ kubectl delete deploymentpolicies.nodewright.nvidia.com --all
kubectl delete deploymentpolicies --all

# Then uninstall the chart
helm uninstall nodewright --namespace skyhook
helm uninstall nodewright --namespace nodewright
```

**Why cleanup matters:** If you uninstall while NodeWright CRs with finalizers still exist, it can leave resources in a broken state that may cause reinstall issues.
Expand All @@ -203,7 +205,7 @@ helm uninstall nodewright --namespace skyhook
### Watch NodeWright apply packages

```
kubectl get pods -w -n skyhook
kubectl get pods -w -n nodewright
```
There will be a pod for each lifecycle stage (apply, config, etc.) per package per node matching the selector.

Expand Down
4 changes: 2 additions & 2 deletions chart/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ By default, the Helm chart includes a pre-delete hook that automatically cleans

```bash
# Uninstall with automatic cleanup (default)
helm uninstall nodewright --namespace skyhook
helm uninstall nodewright --namespace nodewright
```

The pre-delete hook will:
Expand All @@ -121,7 +121,7 @@ When disabled, you must manually delete resources before uninstalling to avoid i
# Manual cleanup when automatic cleanup is disabled
kubectl delete skyhooks --all
kubectl delete deploymentpolicies --all
helm uninstall nodewright --namespace skyhook
helm uninstall nodewright --namespace nodewright
```

### Configuring Timeout Values
Expand Down
9 changes: 9 additions & 0 deletions chart/RELEASE_NOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,15 @@ For the full commit-level log see CHANGELOG.md.

## Unreleased

### Other Changes

- **The documented install namespace for new installs is now `nodewright`.** This is a
documentation and example change only: the chart has always sourced the namespace from
`.Release.Namespace` and installs cleanly into any namespace. **An existing release in
the `skyhook` namespace needs no action** — a namespace cannot be renamed in place and
Helm cannot move a release between namespaces, so `helm upgrade` there keeps working
unchanged and is supported indefinitely.

### Bug Fixes

- **Helm upgrade no longer fails on the immutable Deployment selector after the
Expand Down
38 changes: 37 additions & 1 deletion docs/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,42 @@ If the cluster serves only the legacy `skyhook.nvidia.com` group and not
naming both groups and telling you to upgrade to a NodeWright-capable operator,
rather than a confusing `NotFound`.

### Namespace resolution

The install namespace for new deployments moved from `skyhook` to `nodewright` with
the rename. Namespaces cannot be renamed in place, and Helm cannot move a release
between namespaces, so an install that predates the rename legitimately stays in
`skyhook` indefinitely. The CLI therefore does **not** simply default to
`nodewright`.

When `--namespace` is **not** passed, the CLI resolves the operator's namespace in
this order and caches the answer for the invocation:

1. `nodewright` — if a NodeWright operator Deployment is there, use it.
2. `skyhook` — if the operator is there instead, use it and print a one-line note
to stderr saying the legacy namespace was used and that new installs default to
`nodewright`. Pass `--namespace skyhook` to silence the note.
3. A cluster-wide Deployment sweep, for installs in some other namespace. This
needs cluster-scoped list permission; if you don't have it, the step is skipped
silently rather than failing.
4. If nothing is found, `nodewright` is used so the command's own lookup produces
the specific error (for example, "no operator deployment found in namespace
\"nodewright\"").

When `--namespace` **is** passed, it is used verbatim with no discovery and no
note. Nothing else about the CLI is version-gated on this: the namespace is only
used to locate the operator Deployment and the package pods.

The `NodeWright` and `DeploymentPolicy` CRDs are cluster-scoped, so `--namespace`
never affects which CRs a command sees.

| Situation | `--namespace` omitted | `--namespace` passed |
|-----------|----------------------|----------------------|
| Operator in `nodewright` | ✅ Discovered | ✅ Used as given |
| Operator in `skyhook` (pre-rename install) | ✅ Discovered, with a deprecation note | ✅ Used as given |
| Operator in some other namespace | ✅ Discovered when the user can list Deployments cluster-wide, otherwise falls back to `nodewright` and the command reports the miss | ✅ Used as given |
| No operator installed | ⚠️ Falls back to `nodewright`; the command reports the miss | ⚠️ Command reports the miss |

### Minimum Operator Version

The CLI requires **operator version v0.8.0 or later** for full functionality of all commands.
Expand Down Expand Up @@ -105,7 +141,7 @@ kubectl nodewright [global-flags] <command> [subcommand] [flags] [arguments]

- `-h, --help` - Show help for any command
- `--version` - Show version information
- `-n, --namespace` - Kubernetes namespace (default: "skyhook")
- `-n, --namespace` - Kubernetes namespace the operator is installed in. When omitted, the CLI discovers it (see [Namespace resolution](#namespace-resolution)); the fallback default is `nodewright`.
- `-o, --output` - Output format: table|json|yaml|wide
- `-v, --verbose` - Enable verbose output
- `--dry-run` - Preview changes without applying them
Expand Down
16 changes: 10 additions & 6 deletions docs/designs/webhook-bootstrap-lease.md
Original file line number Diff line number Diff line change
Expand Up @@ -102,17 +102,21 @@ specifically, because v0.7.x has no knowledge of the new lease. Operators
on v0.7.x still need the manual workaround:

```bash
# In the operator's namespace:
kubectl -n skyhook get pods # identify old-version pods
kubectl -n skyhook delete pod <old-pod-1> <old-pod-2> # free the lease
# The operator's namespace. A v0.7.x install predates the skyhook -> nodewright
# namespace default, so it is almost certainly still `skyhook`; `nodewright` is
# the default only for installs created after that change.
NS=skyhook

kubectl -n "$NS" get pods # identify old-version pods
kubectl -n "$NS" delete pod <old-pod-1> <old-pod-2> # free the lease
# Or, equivalently:
kubectl -n skyhook delete lease 3c22c1ae.nvidia.com
kubectl -n "$NS" delete lease 3c22c1ae.nvidia.com

# Verify recovery:
kubectl -n skyhook get secret webhook-cert -w
kubectl -n "$NS" get secret webhook-cert -w
kubectl get mutatingwebhookconfiguration skyhook-operator-mutating-webhook \
-o jsonpath='{.webhooks[0].clientConfig.caBundle}' | wc -c # must be > 0
kubectl -n skyhook rollout status deploy/skyhook-operator-controller-manager
kubectl -n "$NS" rollout status deploy/skyhook-operator-controller-manager
```

The runbook above should be added to release notes for any future major
Expand Down
14 changes: 7 additions & 7 deletions docs/metrics/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -84,18 +84,18 @@ Note: When a NodeWright is deleted all metrics for that NodeWright are no longer

See the script [metrics_test.py](../../k8s-tests/chainsaw/metrics_test.py) that will let you test for the existence or absence of metrics based on name and labels. The metrics endpoint requires a bearer token authorized for the `/metrics` non-resource URL. Create a scraper identity and bind it to the chart's metrics-reader role:
```bash
kubectl -n skyhook create serviceaccount metrics-reader
kubectl -n nodewright create serviceaccount metrics-reader
kubectl create clusterrolebinding metrics-reader-access \
--clusterrole=skyhook-operator-metrics-reader \
--serviceaccount=skyhook:metrics-reader
--serviceaccount=nodewright:metrics-reader
```

Then port-forward the HTTPS Service and scrape it with a short-lived token. TLS verification must be skipped because controller-runtime generates an in-memory self-signed certificate for each operator pod and does not publish a stable CA:

```bash
kubectl -n skyhook port-forward \
kubectl -n nodewright port-forward \
svc/skyhook-operator-controller-manager-metrics-service 8443:8443 &
METRICS_TOKEN="$(kubectl -n skyhook create token metrics-reader)"
METRICS_TOKEN="$(kubectl -n nodewright create token metrics-reader)"
Comment thread
coderabbitai[bot] marked this conversation as resolved.
curl --insecure --header "Authorization: Bearer ${METRICS_TOKEN}" \
https://127.0.0.1:8443/metrics
```
Expand All @@ -104,7 +104,7 @@ curl --insecure --header "Authorization: Bearer ${METRICS_TOKEN}" \
namespace are provided by flags or environment variables:

```bash
SKYHOOK_NAMESPACE=skyhook \
SKYHOOK_NAMESPACE=nodewright \
METRICS_TEST_SERVICE_ACCOUNT=metrics-reader \
./k8s-tests/chainsaw/metrics_test.py \
skyhook_node_target_count 1 -t skyhook_name=my-nodewright
Expand All @@ -114,7 +114,7 @@ For repeated checks, mint one token and reuse it instead of making a
TokenRequest for every invocation:

```bash
METRICS_TOKEN="$(kubectl -n skyhook create token metrics-reader)"
METRICS_TOKEN="$(kubectl -n nodewright create token metrics-reader)"
export METRICS_TOKEN
./k8s-tests/chainsaw/metrics_test.py \
skyhook_node_target_count 1 -t skyhook_name=my-nodewright
Expand Down Expand Up @@ -177,7 +177,7 @@ make create-kind-cluster

# Install the operator through the helm chart so that
# the /metrics endpoint is setup
helm install skyhook ../chart --namespace skyhook \
helm install skyhook ../chart --namespace nodewright \
--set metrics.addServiceAccountBinding=true \
--set metrics.serviceAccountName=prometheus \
--set metrics.serviceAccountNamespace=default
Expand Down
38 changes: 38 additions & 0 deletions docs/nodewright-migration.md
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,44 @@ runtime-required **taint** key is likewise untouched: it did not move in the ren
This is transition-only behavior and is removed together with the `skyhook.nvidia.com` group at the
removal release.

## Install namespace (`skyhook` -> `nodewright`)

The documented install namespace for **new** deployments is now `nodewright`. The kustomize
overlay moved from `skyhook-operator-system` to `nodewright-operator-system` to match, and the
operator's own `NAMESPACE` default (used only when nothing sets it, such as a bare binary or
`make run`) moved from `skyhook` to `nodewright`.

**There is nothing to migrate, and no deadline.** Kubernetes namespaces cannot be renamed in
place, and Helm cannot move a release between namespaces, so moving an existing install would
mean deleting and recreating every namespaced object the operator owns: a real outage in
exchange for a cosmetic name. **Staying in `skyhook` remains fully supported and correct.** The
chart takes the namespace from `.Release.Namespace` throughout and installs cleanly into any
namespace, so `helm upgrade` against an existing `skyhook`-namespace release is unaffected by
this change.

What this does change:

- **New installs.** The README and chart docs now use `--namespace nodewright --create-namespace`.
Substitute your own namespace freely; nothing depends on the name.
- **`kubectl nodewright`.** With no `--namespace`, the CLI discovers the operator's namespace
rather than assuming one, so it keeps working against a `skyhook`-namespace install. It checks
`nodewright`, then `skyhook` (printing a one-line note), then sweeps cluster-wide. See
[the CLI reference](cli.md#namespace-resolution).

If you do want to move an existing install, treat it as an uninstall and reinstall, not a
migration:

1. Confirm every NodeWright is `complete` with no nodes in progress.
2. `helm uninstall <release> -n skyhook`. The chart's pre-delete hook removes the NodeWright and
DeploymentPolicy CRs; on-node package state (the `nodewright.nvidia.com/*` node annotations)
is **not** removed by this, so it survives.
3. `helm install <release> oci://ghcr.io/nvidia/nodewright/charts/nodewright -n nodewright --create-namespace`.
4. Re-apply your NodeWright CRs. Because the per-node state annotations survived, packages are
not re-run.

There is no reason to do this on a running cluster unless you have an external requirement on
the namespace name.

## Downstream consumers (e.g. aicr)

Projects that ship or depend on Skyhook CRs need a coordinated update. For example
Expand Down
Loading
Loading