mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-06 12:03:39 +09:00
Aleksei Sviridkin
924d28cf3d
<!-- Thank you for making a contribution! Here are some tips for you: - Use Conventional Commits for the PR title: `type(scope): description` - Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore - Scopes are not an exhaustive list — pick the most specific scope for the change and extend the list when a genuinely new area appears. Examples: - System components: dashboard, platform, operator, cilium, kube-ovn, linstor, fluxcd, cluster-api - Managed apps: postgres, mariadb, redis, kafka, clickhouse, virtual-machine, kubernetes - Development and maintenance: api, hack, tests, ci, docs, maintenance - Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or add a `BREAKING CHANGE:` footer - If it's a work in progress, consider creating this PR as a draft. - Don't hesistate to ask for opinion and review in the community chats, even if it's still a draft. - Add the label `backport` if it's a bugfix that needs to be backported to a previous version. --> ## What this PR does Closes the gpu-operator half of #3563 — gap 3 (the operator cannot validate a driver it did not install) and gap 4 (nothing routes CUDA libraries to a tenant pod). Tenant clusters have run Talos workers since 1.6.0, where the NVIDIA driver comes from a system extension baked into the worker image rather than from the operator's driver container. The addon default still described the Ubuntu shape, so a GPU node group came up with the operator fighting the OS for the driver and no path at all for the CUDA libraries. Three changes, each closing one silent failure: | Change | Without it | |---|---| | `driver.enabled: false`, `toolkit.enabled: false` | the driver container loads its own module and clashes with the extension's; the toolkit container rewrites `/etc/containerd/config.toml`, which Talos owns and which already registers the `nvidia` runtime with `enable_cdi = true` and `cdi_spec_dirs = ['/run/cdi']` | | `validator.driver.env`: `DRIVER_ROOT=/`, `NVIDIA_DRIVER_ROOT=/`, `DISABLE_DEV_CHAR_SYMLINK_CREATION=true` | the driver validator looks for a driver-container install, finds none, never writes its sentinel, and the device plugin stays blocked behind `toolkit-validation` — the loop on `Attempting to validate a driver container installation` (**gap 3**) | | `devicePlugin.env`: `DEVICE_LIST_STRATEGY=cdi-annotations`, `NVIDIA_CTK_PATH=/usr/local/bin/nvidia-ctk`, plus `DEVICE_ID_STRATEGY`, `NVIDIA_DRIVER_ROOT`, `CDI_ANNOTATION_PREFIX` | tenant pods carry no `runtimeClassName` and did not need one on Ubuntu, where the toolkit made `nvidia` the default containerd runtime. Talos does not, so CDI is the only route: the pod gets the device and no CUDA libraries (**gap 4**) | Every value is the configuration validated on a production Talos GPU node group, minus the parts that only mattered while the driver container was in play. No DaemonSet, no new image, no new package — the fix is values. ### The removed default The previous default fed `NVreg_NvLinkDisable=1` to the driver container through the `nvidia-kernel-module-params` ConfigMap. With that container off the ConfigMap is mounted by nothing, so this removes the default. On Talos the parameter belongs to the worker machine config, which the `kernelModules` field of the worker pool writes: that field is top-level in the KubernetesNodes app (`packages/apps/kubernetes-nodes`), added by #3571, and its automatic set for a pool holding an `nvidia.com/*` GPU already loads `nvidia` with `NVreg_NvLinkDisable=1`. The wrapper chart keeps the ConfigMap feature for an operator who re-enables the driver container through `valuesOverride`. A test pins the absence so the dead default cannot come back and offer a second, inert source for the same setting. ### Relationship to #3571 Independent files, no conflict, but only useful together: this makes `driver.enabled: false` correct, and #3571 is what actually loads the modules the extension ships. Merged alone, this changes a GPU node group from "operator fights the OS" to "operator stays out of the way and the modules still are not loaded" — better, but not yet working. #3571 alone leaves the operator fighting for the driver. Either order is safe; both are needed for a GPU node group to work. ### What is verified, and what is not Verified here: the rendered HelmRelease values, that `valuesOverride` still wins over each new key, and that the removed default is gone. The individual values are the ones running in production today, applied by hand through `addons.gpuOperator.valuesOverride`. Not verified here: this exact default exercised end-to-end as the chart's own output. That needs a Talos GPU node group whose modules load, which needs #3571 released. I would rather say so than imply a green e2e that does not exist — happy to hold this as draft until I have run a single-node L40S cluster against the chart default, and I will report the result on this PR either way. ### Not covered HAMi. When `addons.hami.enabled` the operator's device plugin stays off and HAMi serves the GPUs; HAMi's own plugin needs equivalent CDI wiring on Talos, which this default does not attempt. Flagging rather than guessing — the production cluster this was derived from runs HAMi disabled, so I have no validated configuration for that path. Worth its own issue if a maintainer confirms the shape. Also not in scope, and unchanged from the merge base: `talos.installerRepository` and `talos.version` reach the worker machine config unvalidated (being tracked separately by @lexfrei), and the `nvidia-operator-validator` chain for the *management* cluster's own passthrough/vgpu variants, which never load the host driver at all. ### Screenshots No UI changes. ### Screenshots <!-- REQUIRED for UI changes: attach screenshots or screen recordings demonstrating the visual impact of your changes. PRs with UI changes without screenshots will not be merged. --> ### Downstream repositories <!-- Cozystack is upstream for repositories that are NOT kept in sync with it automatically, and nothing in CI compares the two sides, so a change here can break them silently. Which change forces what, and which file to touch, is the trigger map in docs/agents/contributing.md under "Downstream Repositories". Walk it before you tick anything. IF YOU ARE AN AI AGENT OR AN LLM, THIS IS PART OF THE TASK, NOT A FORM TO FILL IN: 1. Walk the trigger map against the actual diff of this PR, file by file. Do not decide from the PR title or from your memory of what you changed. 2. For every repository you tick, open the follow-up PR there and link it on the same line. A ticked box with no link claims work that does not exist, which is worse than an empty box. Search that repository for an open PR or issue covering it first, and link that instead of filing a duplicate. 3. When the follow-up is out of scope, or needs a decision that is not yours to make, open an issue in that repository instead, link that, and say so in the PR body. Do not push a speculative PR into another repository just to fill a line here. 4. Do not tick "No downstream repository is affected" to make the checklist go away. If you are unsure, leave every box empty and say so in the PR body, so a human decides. --> - [ ] No downstream repository is affected by this change - [x] [cozystack/website](https://github.com/cozystack/website) - follow-up: https://github.com/cozystack/website/issues/643 - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: ### Release note <!-- Write a release note: - Explain what has changed internally and for users. - Start with the same `type(scope):` prefix as in the PR title - Follow the guidelines at https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md. --> ```release-note feat(kubernetes): The GPU Operator addon of tenant Kubernetes clusters now defaults to a driver provided by the operating system, which is the shape of the Talos workers these clusters have run since 1.6.0. `driver` and `toolkit` are disabled (the driver comes from a Talos system extension, and containerd configuration belongs to the machine config), the driver validator is pointed at the host root so it stops looping on "Attempting to validate a driver container installation", and the device plugin is switched to CDI (`DEVICE_LIST_STRATEGY=cdi-annotations`) so tenant pods receive the CUDA libraries — previously they received the GPU device and nothing else, since Talos does not make `nvidia` the default containerd runtime. The previous default, which fed `NVreg_NvLinkDisable=1` to the operator's driver container through a ConfigMap, is removed because that container is disabled; on Talos the parameter is carried by the worker machine config, through the `kernelModules` field of the worker pool in the KubernetesNodes app (`packages/apps/kubernetes-nodes`), which loads `nvidia` with `NVreg_NvLinkDisable=1` by default on a pool holding an `nvidia.com/*` GPU. Operators who overrode `addons.gpuOperator.valuesOverride` to work around this by hand can drop those overrides. NOTE: an override that sets `devicePlugin.env` replaces the list rather than merging it, so restate the full list when overriding it or CDI stops working. HAMi is not covered: when `addons.hami.enabled` the operator's device plugin stays off and HAMi's own plugin needs equivalent CDI wiring, which this change does not attempt. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Configuration** * GPU workloads on Talos workers now use host-provided NVIDIA drivers and tooling by default; the GPU Operator’s driver and toolkit are disabled. * Device discovery uses CDI annotations and UUID-based device IDs. When HAMi is enabled, the Operator’s device plugin is disabled. * Host-root validation no longer creates `/dev/char` symlinks. * Driver kernel-module configuration is no longer created by default. * **Documentation** * Clarified when kernel-module configuration applies and what settings are needed to re-enable the driver. * Noted that `valuesOverride` replaces lists wholesale. <!-- end of auto-generated comment: release notes by coderabbit.ai -->