fix(local): pin kind worker node names and select the fleet by hostname - #683
Conversation
giuliocalzo
changed the title
Kind derives node names from the cluster name, so the control plane comes up as mokka-control-plane. Pinning it via JoinConfiguration keeps node names stable and aligned with the worker naming used by tooling and docs. Signed-off-by: Giulio Calzolari <gcalzolari@nvidia.com>
Stable worker names make node-targeted commands and docs reproducible across cluster recreations. Drops the nvml-mock/profile worker labels, which are no longer used to select nodes in this cluster shape. Signed-off-by: Giulio Calzolari <gcalzolari@nvidia.com>
The nvml-mock/profile worker labels are gone, so install_fleet() had no node to select and --multi-gpu-profile left every release Pending. Pin each release to a worker by kubernetes.io/hostname, which the kubeadm JoinConfiguration patches now make stable. Profiles map to workers positionally, so a fail() guards asking for more profiles than the cluster has workers. The dot in kubernetes.io is escaped because helm --set otherwise reads it as a path separator and would nest the key instead of setting the label. Signed-off-by: Giulio Calzolari <gcalzolari@nvidia.com>
Worker names are sequential, so the Nth profile maps to worker-N without a parallel list to keep in sync. Signed-off-by: Giulio Calzolari <gcalzolari@nvidia.com>
7674b4f to
7af04ba
Compare
giuliocalzo
added
kind/bug
kubeadm names the bootstrap node from InitConfiguration, so the JoinConfiguration patch never applied and the node stayed mokka-control-plane. Drop the patch rather than convert it: nothing selects the control plane by name. Signed-off-by: Giulio Calzolari <gcalzolari@nvidia.com>
…names Cluster.Nodes() enumerated `kind get nodes`, which reports docker container names, and callers passed those straight to kubectl. Once a cluster config pins nodeRegistration.name the two diverge and every node lookup 404s. Read nodes from the Kubernetes API instead, keeping the container name from Kind's provider ID (kind://docker/<cluster>/<container>) so the `docker exec` call sites in the GPU Operator and NRI scenarios still reach the right container. Roles now come from the control-plane label rather than a "worker" substring, since pinned names need not describe the role. Signed-off-by: Giulio Calzolari <gcalzolari@nvidia.com>
giuliocalzo
changed the title
Node discovery reads a full `kubectl get nodes -o json`, which Run tees to the Ginkgo writer. RunQuiet still captures the body for Result and CmdError, so failures stay diagnosable without the several-hundred-line echo. Signed-off-by: Giulio Calzolari <gcalzolari@nvidia.com>
ArangoGutierrez
left a comment
ArangoGutierrez
left a comment
There was a problem hiding this comment.
Nothing here blocks merge.
The Name/Container split lands cleanly. Every docker exec path now takes Container and the kube-side lookups still take Name, including the one place it would have been easy to miss (nriPluginPodOnNode gets victim.Name, wedgeNRIPlugin gets victim.Container). Reading the role off node-role.kubernetes.io/control-plane rather than a substring of the name is the right move once names are pinned, and containerName's fallback keeps a non-Kind providerID from producing garbage. The helm escaping checks out too: flags reach helm_resource as argv, so the backslash in kubernetes.io is not re-read by a shell. I ran the new package tests and they pass under make test-e2e-framework, which CI does invoke.
- All three inputs here are already decided by the regex, so the length bound is untested: deleting len(name) == 0 || len(name) > 40 leaves the test green. A 41-character lowercase name would pin the bound the doc comment justifies. (tests/e2e/go/framework/cluster/cluster_test.go:99)
Every existing ValidateName case fails on the regex alone, so the 1..40 character check was unpinned: deleting it kept the test green. Add inputs either side of the bound. Signed-off-by: Giulio Calzolari <gcalzolari@nvidia.com>
|
Good catch on the untested length bound — you're right that every existing case fails on the regex alone, so Fixed in 5ac2068 with cases either side of the bound: a 40-character name passes, a 41-character one fails. I verified it actually pins the check by deleting the length guard and confirming the test goes red at the 41-character assertion, then restoring it. |
roma-glushko
left a comment
roma-glushko
left a comment
There was a problem hiding this comment.
I like the idea to keep consistent. I think the other cluster already has stable worker names.
3 participants
Summary
local/kind/default.kind.yamltoworker-0/worker-1viaJoinConfiguration, instead of letting them be derived from the cluster name (mokka-worker,mokka-worker2). Stable names make node-targeted commands and docs reproducible across cluster recreations.install_fleet()inlocal/nvml_mock.tiltfileto pin each per-profile release withnodeSelector.kubernetes.io/hostname=<worker>, and drops the now-redundantnvml-mock/profileworker labels. The mapping is positional: the Nth entry ofFLEET_PROFILESlands onworker-N.Cluster.Nodes()previously enumeratedkind get nodes, which reports docker container names, and passed them straight tokubectl— that works only while the two happen to be identical.Why the harness change is required
Pinning
nodeRegistration.namedecouples the Kubernetes node name from the Kind container name. Without the harness fix, every scenario that resolves a node by name fails withError from server (NotFound): nodes "mokka-worker" not found.cluster.Nodenow carries both identities:Name(Kubernetes, forkubectl) andContainer(docker, fordocker exec). The container name is read from Kind's provider ID,kind://docker/<cluster>/<container>, which keeps pointing at the container after a rename. Thedocker execcall sites in the GPU Operator and NRI scenarios were updated to useContainer. Roles now come from thenode-role.kubernetes.io/control-planelabel rather than a"worker"substring match, since a pinned name need not describe its role.The control-plane node deliberately keeps its Kind-derived name: kubeadm names the bootstrap node from
InitConfiguration, so aJoinConfigurationpatch there is silently ignored.Test plan
make test-e2e-frameworkandmake testpass;golangci-lint --build-tags e2e ./tests/e2e/go/...reports 0 issuescluster_test.gocover renamed nodes, label-derived roles, non-Kind provider IDs, ordering, and malformed inputworker-0resolves to containerrename-probe-workerhelm templaterenders a flatkubernetes.io/hostname: worker-0undernodeSelectortilt alpha tiltfile-result -- --multi-gpu-profilepinsa100toworker-0andt4toworker-1