feat: nix hackery#13267
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new debug “host mount namespace” mode that overlays Nix tooling at /nix while running directly against the host rootfs (no nsenter), exposed via a new API profile and a talosctl debug --host-ns flag.
Changes:
- Introduces
PROFILE_HOST_NSin the debug API (proto + generated Go bindings). - Implements host-namespace debug execution using a forked mount namespace + overlayfs + bind-mounting image
/nix. - Updates
talosctl debugto support--host-nsand to default the image when host-ns is used.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/machinery/constants/constants.go | Adds a default image reference for host-ns debug sessions. |
| api/machine/debug.proto | Adds PROFILE_HOST_NS to the DebugContainerRunRequestSpec.Profile enum. |
| pkg/machinery/api/machine/debug.pb.go | Regenerates Go bindings to include the new profile enum value. |
| internal/app/debug/hostns.go | New host-namespace debug runner (mount namespace fork, overlay rootfs, bind /nix, chroot, exec). |
| internal/app/debug/debug.go | Routes debug sessions to host-ns runner when the new profile is selected. |
| internal/app/debug/container_streams.go | Adds host-ns streaming coordinator and recv loop variant. |
| cmd/talosctl/cmd/talos/debug.go | Adds --host-ns, makes image arg optional in host-ns mode, passes profile through to server. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Noel Georgi <git@frezbo.dev>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for _, dir := range []string{"dev", "proc", "sys"} { | ||
| src := "/" + dir | ||
| dst := filepath.Join(merged, dir) | ||
|
|
||
| if mkErr := os.MkdirAll(dst, 0o755); mkErr != nil { |
| case <-ctx.Done(): | ||
| g.stdoutW.Close() //nolint:errcheck | ||
| g.stdinW.Close() //nolint:errcheck | ||
|
|
||
| if recvLoopCh != nil { |
| cgroupFd.Close() //nolint:errcheck | ||
|
|
||
| if launchErr != nil { | ||
| log.Printf("host-ns: launch error: %v", launchErr) |
| // chroot into the overlay. | ||
| // After this point all absolute paths resolve from the merged root: | ||
| // /usr/local/sbin/zpool → host binary ✓ | ||
| // /nix/store/<h>/bin/jq → image tool ✓ | ||
| if err = unix.Chroot(merged); err != nil { |
smira
left a comment
smira
left a comment
There was a problem hiding this comment.
this is crazy amount of code, should we put it under debug build tag?
I wonder if we should hide the flag, and hide the server implementation under a build tag, so it's not available in general? this is powerful, but imho too much crazy to understand all consequences for a production run
After discussion, let's put this under a debug flag and only enable for CI tests or along similar lines |
No description provided.