From 47e1a4cefd934aad601e2b177742c10db323ad8b Mon Sep 17 00:00:00 2001 From: Young Joon Lee Date: Tue, 10 Nov 2026 20:47:04 +0900 Subject: [PATCH 1/2] feat(tenant): manage CPU and NUMA pinning without restarting containers --- docs/commands.md | 14 ++ docs/specs/cli-runtime.md | 2 + docs/specs/tenant.md | 15 +- docs/tenant.md | 28 +++ internal/cli/runtime.go | 2 +- internal/cli/runtime_test.go | 2 +- internal/cli/tenant.go | 58 ++++- internal/cli/tenant_cpuset_test.go | 32 +++ internal/tenant/cpuset.go | 205 +++++++++++++++++ internal/tenant/cpuset_test.go | 344 ++++++++++++++++++++++++++++ internal/tenant/import.go | 12 +- internal/tenant/manager.go | 37 ++- internal/tenant/spec.go | 14 ++ tests/scenarios/tenant-lifecycle.sh | 16 ++ 14 files changed, 772 insertions(+), 9 deletions(-) create mode 100644 internal/cli/tenant_cpuset_test.go create mode 100644 internal/tenant/cpuset.go create mode 100644 internal/tenant/cpuset_test.go diff --git a/docs/commands.md b/docs/commands.md index 2b6f717..f467dd5 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -380,12 +380,26 @@ Long-lived tenant containers ([tenant.md](tenant.md)). Every subcommand except ` | `--size` | required | Volume size, such as `500G` or `2T`. | | `--fs` | `ext4` | Volume filesystem: `ext4` (can shrink offline) or `xfs`. | | `--gpus` | none | Comma-separated GPU indices. | +| `--cpus` | unmanaged | Host CPU list (`N` or `N-M`, comma-separated), not a CPU quota. | +| `--mems` | unmanaged | NUMA memory-node list (`N` or `N-M`, comma-separated). | | `--account` | none | `NAME[:UID[:GID]]`, repeatable; UIDs default to 1000, 1001, ... | | `--sudo` | `false` | Give the accounts sudo inside the container. | | `--operator` | none | Host account allowed `sudo rootfiles tenant ops` on this tenant (repeatable). | | `--port` | none | Extra published port `HOST:CONTAINER[/udp]` (repeatable). | | `--bind` | none | Extra bind mount `SRC:DST[:ro\|rw]` outside the volume quota (repeatable). | +### rootfiles tenant cpuset + +`rootfiles tenant cpuset TENANT --cpus 0-3 --mems 0`. Save CPU/NUMA pinning and update the existing running or stopped container in place, without restarting. Mutating. + +| Flag | Default | Purpose | +|---|---|---| +| `--cpus` | retain spec | Host CPU list (`N` or `N-M`, comma-separated); explicit empty makes the key unmanaged. | +| `--mems` | retain spec | NUMA memory-node list; explicit empty makes the key unmanaged. | +| `--clear` | `false` | Remove cpuset from the spec and reset both container keys to the full host lists. Exclusive with the list flags. | + +Absent or empty spec keys are unmanaged, so hand-set pins stay untouched. Other pending settings remain pending; run `rootfiles tenant apply TENANT` separately to recreate for them. A failed update restores the previous spec. On cgroup v2, changing memory nodes migrates existing pages and may take time for large processes. `tenant status` shows `update` for live drift and `status TENANT` shows spec and actual pins. + ### rootfiles tenant gpu `rootfiles tenant gpu TENANT --gpus 0,1`. Change a tenant's GPUs and recreate the container. Mutating. diff --git a/docs/specs/cli-runtime.md b/docs/specs/cli-runtime.md index 4706696..6e076fb 100644 --- a/docs/specs/cli-runtime.md +++ b/docs/specs/cli-runtime.md @@ -41,6 +41,8 @@ Shared plumbing every native `rootfiles` command runs through: the cobra root an ## Behaviour +- `tenant cpuset` is a native mutation: it uses the same root, Linux and global-lock preflight as the other tenant mutations. + - Aliases resolve to one command object: `tenant reset` -> `passwd`, `tunnel install` -> `setup`. Reader and mutation annotations stay on that canonical command. Deprecated backup/user flags retain old invocations; `user passwd --suffix` remains accepted until the next minor release. - Root help includes Rocky/fleet and derives profile names from `config.AvailableProfiles()`. The audited commands carry examples; Use strings contain positional syntax only, checked against Args arity by `cli_surface_test.go`. VLAN help/prompt examples use `203.0.113.10/32`. diff --git a/docs/specs/tenant.md b/docs/specs/tenant.md index 7a65b2b..36b528f 100644 --- a/docs/specs/tenant.md +++ b/docs/specs/tenant.md @@ -35,9 +35,9 @@ Package boundary Spec - One spec per tenant at `/.rootfiles/tenants/.yaml` (`SpecDir`), mode 0600, under the root-only metadata tree checked by `RequireTrustedMetadata`. `LoadSpec` decodes strictly (`KnownFields`) and rejects a file whose `name` differs from its file name. -- Fields (`Spec` in `internal/tenant/spec.go`): `name`, `image`, `gpus` (GPU indices), `ssh_port`, `ports` (`HOST:CONTAINER[/tcp|udp]`), `volume.size` (min 64M), `volume.fs` (ext4 default, or xfs), `users` (name, uid/gid 1000-65533, sudo), `operators`, `extra_binds` (`SRC:DST[:ro|rw]`, absolute, not on `/data`, `/home`, `/tmp`, `/etc/tenant-system`). +- Fields (`Spec` in `internal/tenant/spec.go`): `name`, `image`, `gpus` (GPU indices), `ssh_port`, `ports` (`HOST:CONTAINER[/tcp|udp]`), `volume.size` (min 64M), `volume.fs` (ext4 default, or xfs), `users` (name, uid/gid 1000-65533, sudo), `operators`, `extra_binds` (`SRC:DST[:ro|rw]`, absolute, not on `/data`, `/home`, `/tmp`, `/etc/tenant-system`), optional `cpuset.cpus` and `cpuset.mems` (comma-separated Linux indices `N` or ranges `N-M`, N <= M, no spaces). Empty or absent cpuset keys are unmanaged. - `Validate` reports every problem at once. Tenant names match `^[a-z][a-z0-9-]{0,30}$`, contain no `.` (so sshpiper route names `.` cannot collide) and never end in `-prev`. -- `Spec.Digest` covers only settings a new container can change: name, image, sorted GPUs, SSH port, ports, extra binds. Users, operators and volume size converge without a recreate. Adding a field that needs a new container means adding it to the digest. +- `Spec.Digest` covers only settings a new container can change: name, image, sorted GPUs, SSH port, ports, extra binds. Users, operators, volume size and cpuset converge without a recreate. Cpuset is excluded from the digest; absent or empty cpuset preserves the legacy digest, run arguments and converge steps byte for byte. Adding a field that needs a new container means adding it to the digest. Host layout (root `modules.tenant.root`, default `/raid/tenants`, must be a root-only path chain: `config.RootOnlyHomeBase` in `tenantEnv`) - `//` is 0700 so host users cannot reach setuid files a tenant leaves in its volume. @@ -47,6 +47,7 @@ Host layout (root `modules.tenant.root`, default `/raid/tenants`, must be a root Container (`Manager.RunArgs`) - Created with `docker run`, not compose: name `tenant-`, hostname ``, `--restart=always --init --ipc=host`, labels `rootfiles.tenant`, `rootfiles.tenant.spec=`, `rootfiles.tenant.gpus=`, `-p :22`. +- Managed cpuset keys add `--cpuset-cpus` / `--cpuset-mems` on create and recreate. Multiple tenants may share the same CPUs/nodes. - GPUs always attach with `--runtime=nvidia --gpus "device="`. Evidence (verified 2026-10-20): runc plus `--gpus` loses device access on any `systemctl daemon-reload`, because its devices come only from the prestart hook; the nvidia runtime lists them in the OCI spec. The daemon `default-runtime` is not changed. - `/data`, `/home`, `/tmp` are `--mount type=bind` from `vol/data|home|tmp`; `system` is bound read-only at `/etc/tenant-system`. A bind fails on a missing source, so a container never starts on an unmounted volume and writes past the quota. Keep it a bind, never a named volume. - Entrypoint is `/bin/sh /etc/tenant-system/init.sh` (`InitScript`), the image-agnostic boot contract. The only image requirement is `openssh-server`. On every start it creates users and groups, restores password hashes and host keys, starts sshd (trusting `piper.pub` for every account when present), runs `/data/.tenant/autostart.sh` as the first user, then sleeps. `init.sh users` re-runs only the account step. @@ -57,6 +58,16 @@ Passwords - `SyncShadow` copies usable hashes of spec users from the running container to `system/shadow`; a stopped container keeps the stored hashes. A direct bind of `/etc/shadow` is not used because `passwd` renames the file. - Reads from a tenant container go through `readInContainer`: `timeout 10 head -c 4194304`, inside the container so symlinks resolve against its root. +Live settings (`Manager.UpdateCpuset`, `Manager.ChangeCpuset`) +- Before Apply or Import changes anything, managed CPU and memory-node lists must be subsets of sysfs `cpu/online` and `node/has_memory`. Unreadable/invalid sources block managed changes. Unmanaged keys do not read sysfs. +- Inspect reads `HostConfig.CpusetCpus` and `CpusetMems`. With a current digest, Apply updates only differing managed keys through one `docker update`, before starting a stopped container or syncing accounts. Equivalent list representations do not drift; unmanaged pins remain untouched. +- `tenant cpuset TENANT --cpus LIST --mems LIST` saves the spec and updates the existing running or stopped container only. Omitted flags retain existing spec keys; explicit empty values make them unmanaged. The command never runs full converge, stop, rename, run or restart. A missing or mismatched digest label remains pending and the output points to `rootfiles tenant apply TENANT` for recreation. +- `--clear` is exclusive with the two list flags. It removes the spec field and explicitly resets both live keys to the full host online CPU and memory-node lists. +- A live-update failure restores the previous spec bytes, including comments, through Runner. Dry-run changes neither spec nor container. +- `tenant status` reports `update` for live-only drift; detail shows both desired settings and actual container values, including unmanaged pins. +- Import carries legacy cpuset values into the spec and new container. Before a recreate, unmanaged actual pins warn that they will be lost and name the command to adopt them. +- On cgroup v2 a live memory-node change migrates existing pages and may take time. Older binaries strictly reject the new field; remove it before downgrading (the live pin remains). + Recreate (`Manager.recreate`, on digest change) - Refuses while `tenant--prev` exists, so an earlier rollback copy is never discarded. - Syncs hashes, warns with the writable-layer size, stops, renames to `-prev` with `--restart=no`, runs the new container and verifies it; on failure removes it and restores the old one (name, restart policy, start). On success `-prev` is kept until the operator removes it. diff --git a/docs/tenant.md b/docs/tenant.md index 2734148..1daac3a 100644 --- a/docs/tenant.md +++ b/docs/tenant.md @@ -69,6 +69,8 @@ sudo rootfiles tenant create lab \ | `--size` | Volume size, such as `500G` or `2T` | | `--fs` | Volume filesystem: `ext4` (default; can shrink offline) or `xfs` | | `--gpus` | Comma-separated GPU indices | +| `--cpus` | Host CPU list such as `0-3,8-11` (pinning, not a CPU quota) | +| `--mems` | NUMA memory-node list such as `0` or `0-1` | | `--account NAME[:UID[:GID]]` | Account to create (repeatable); UIDs default to 1000, 1001, ... and the GID to the UID | | `--sudo` | Give the accounts sudo inside the container | | `--operator` | Host account allowed `sudo rootfiles tenant ops` on this tenant (repeatable) | @@ -142,3 +144,29 @@ Import reads the running container: image, published ports (22 becomes the SSH p - One host per command; `fleet` only ships the module config. - The writable layer has no size limit on ext4 hosts. - Not supported on Rocky Linux. + +## CPU and NUMA pinning + +```yaml +cpuset: + cpus: "0-3,8-11" + mems: "0" +``` + +Either key may be set alone. Absent or empty spec keys are unmanaged: apply leaves any hand-set pin untouched. CPU indices must be online (`/sys/devices/system/cpu/online`); memory nodes must have memory (`/sys/devices/system/node/has_memory`). Invalid or unavailable values are refused before any container changes. Several tenants may share the same CPUs/nodes. + +```bash +sudo rootfiles tenant cpuset lab --cpus 0-3 --mems 0 +sudo rootfiles tenant status lab +sudo rootfiles tenant cpuset lab --clear +``` + +`tenant cpuset` saves the spec and uses `docker update` on the same running or stopped container, without a restart. Omitted flags retain their existing spec keys. An explicit `--cpus ""` or `--mems ""` makes that key unmanaged and leaves the live setting alone. `--clear` instead removes the field and resets both live settings to the host's full lists. A failed Docker update restores the previous spec. Dry-run writes nothing. + +If the image, ports or other container settings have pending changes (including a container without a digest label), they remain pending. The command says when `rootfiles tenant apply lab` is still needed; apply may recreate for those other changes. Cpuset alone never changes the recreate digest. Create and recreate use the spec's managed pinning. Before recreating a container with unmanaged hand-set pins, rootfiles warns; adopt the values with `tenant cpuset` first if you want to keep them. `tenant import` carries existing pins into the new spec. + +`tenant status` shows `update` for live-only cpuset drift and `drift` for settings requiring recreation. Detailed status shows the spec and container values, including unmanaged pins. + +On cgroup v2, changing memory nodes migrates existing memory pages to the new nodes. This can take noticeable time for a large process. + +Rollback: older binaries strictly reject specs containing `cpuset`. Before downgrading, remove the field from each spec; the container keeps its current pinning. The release notes for the version introducing this feature must carry this step. diff --git a/internal/cli/runtime.go b/internal/cli/runtime.go index 7b17ec4..4e7b05c 100644 --- a/internal/cli/runtime.go +++ b/internal/cli/runtime.go @@ -37,7 +37,7 @@ var mutatingCommands = [][]string{ {"tunnel", "setup"}, {"tunnel", "restart"}, {"tunnel", "update"}, {"tunnel", "uninstall"}, {"tenant", "create"}, {"tenant", "apply"}, {"tenant", "user", "add"}, - {"tenant", "passwd"}, {"tenant", "gpu"}, {"tenant", "image"}, + {"tenant", "cpuset"}, {"tenant", "passwd"}, {"tenant", "gpu"}, {"tenant", "image"}, {"tenant", "port"}, {"tenant", "volume", "resize"}, {"tenant", "restart"}, {"tenant", "import"}, {"tenant", "ops"}, } diff --git a/internal/cli/runtime_test.go b/internal/cli/runtime_test.go index 8311bb5..ca76ea8 100644 --- a/internal/cli/runtime_test.go +++ b/internal/cli/runtime_test.go @@ -19,7 +19,7 @@ import ( func TestMarkMutating(t *testing.T) { root := NewRootCmd("test", "abc") - for _, path := range [][]string{{"apply"}, {"user", "add"}, {"gpu", "assign"}, {"tunnel", "setup"}} { + for _, path := range [][]string{{"apply"}, {"user", "add"}, {"gpu", "assign"}, {"tunnel", "setup"}, {"tenant", "cpuset"}} { c, _, err := root.Find(path) if err != nil || c.Annotations[annotMutates] != "true" { t.Errorf("%v should be marked mutating", path) diff --git a/internal/cli/tenant.go b/internal/cli/tenant.go index 166803a..81786b1 100644 --- a/internal/cli/tenant.go +++ b/internal/cli/tenant.go @@ -27,7 +27,7 @@ host keys survive image changes. Specs live in /.rootfiles/tenants.`, cmd.AddCommand( newTenantListCmd(), newTenantStatusCmd(), newTenantCreateCmd(), newTenantApplyCmd(), newTenantUserCmd(), newTenantPasswdCmd(), - newTenantGPUCmd(), newTenantImageCmd(), newTenantPortCmd(), newTenantVolumeCmd(), + newTenantGPUCmd(), newTenantCpusetCmd(), newTenantImageCmd(), newTenantPortCmd(), newTenantVolumeCmd(), newTenantRestartCmd(), newTenantLogsCmd(), newTenantImportCmd(), newTenantOpsCmd(), newTenantSyncCmd(), ) @@ -163,6 +163,11 @@ func newTenantCreateCmd() *cobra.Command { s.Ports, _ = cmd.Flags().GetStringArray("port") s.Operators, _ = cmd.Flags().GetStringArray("operator") s.ExtraBinds, _ = cmd.Flags().GetStringArray("bind") + cpus, _ := cmd.Flags().GetString("cpus") + mems, _ := cmd.Flags().GetString("mems") + if cpus != "" || mems != "" { + s.Cpuset = &tenant.Cpuset{CPUs: cpus, Mems: mems} + } if g, _ := cmd.Flags().GetString("gpus"); g != "" { gpus, err := parseGPUList(g) if err != nil { @@ -210,6 +215,8 @@ func newTenantCreateCmd() *cobra.Command { f.String("size", "", "Volume size, e.g. 500G or 2T") f.String("fs", "ext4", "Volume filesystem: ext4 (can shrink offline) or xfs") f.String("gpus", "", "Comma-separated GPU indices") + f.String("cpus", "", "Host CPU list (N or N-M, comma-separated; not a CPU quota)") + f.String("mems", "", "NUMA memory-node list (N or N-M, comma-separated)") f.StringArray("account", nil, "Account NAME[:UID[:GID]] (repeatable; uid defaults to 1000, 1001, ...)") f.Bool("sudo", false, "Give the accounts sudo inside the container") f.StringArray("port", nil, "Extra published port HOST:CONTAINER[/udp] (repeatable)") @@ -376,6 +383,55 @@ func newTenantGPUCmd() *cobra.Command { return cmd } +func newTenantCpusetCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "cpuset TENANT", + Short: "Save CPU/NUMA pinning and update the existing container without restarting", + Long: `Changes only CPU/NUMA pinning, leaving other pending settings for tenant apply. +Empty or omitted keys are unmanaged. --clear removes the spec field and resets +both container settings to the host's full lists. On cgroup v2, changing memory +nodes migrates existing memory pages and can take time for large processes.`, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + cpus, _ := cmd.Flags().GetString("cpus") + mems, _ := cmd.Flags().GetString("mems") + clear, _ := cmd.Flags().GetBool("clear") + if clear && (cmd.Flags().Changed("cpus") || cmd.Flags().Changed("mems")) { + return fmt.Errorf("--clear cannot be combined with --cpus or --mems; use --clear alone to reset both settings") + } + if !clear && !cmd.Flags().Changed("cpus") && !cmd.Flags().Changed("mems") { + return fmt.Errorf("no cpuset change requested; pass --cpus LIST, --mems LIST or --clear") + } + return tenantDo(cmd, args[0], "cpuset", func(ctx context.Context, _ *module.RunContext, m *tenant.Manager) error { + s, err := m.Load(args[0]) + if err != nil { + return err + } + if clear { + s.Cpuset = nil + } else { + pin := tenant.Cpuset{} + if s.Cpuset != nil { + pin = *s.Cpuset + } + if cmd.Flags().Changed("cpus") { + pin.CPUs = cpus + } + if cmd.Flags().Changed("mems") { + pin.Mems = mems + } + s.Cpuset = &pin + } + return m.ChangeCpuset(ctx, s, clear) + }) + }, + } + cmd.Flags().String("cpus", "", "Host CPU list (N or N-M, comma-separated; empty makes it unmanaged)") + cmd.Flags().String("mems", "", "NUMA memory-node list (N or N-M, comma-separated; empty makes it unmanaged)") + cmd.Flags().Bool("clear", false, "Remove cpuset from the spec and reset the container to the full host lists") + return cmd +} + func newTenantImageCmd() *cobra.Command { return &cobra.Command{ Use: "image TENANT IMAGE", diff --git a/internal/cli/tenant_cpuset_test.go b/internal/cli/tenant_cpuset_test.go new file mode 100644 index 0000000..c213a4a --- /dev/null +++ b/internal/cli/tenant_cpuset_test.go @@ -0,0 +1,32 @@ +package cli + +import ( + "strings" + "testing" +) + +func TestTenantCpusetFlagErrorsBeforeHostAccess(t *testing.T) { + for _, args := range [][]string{nil, {"--clear", "--cpus", "0"}, {"--clear", "--mems", ""}} { + cmd := newTenantCpusetCmd() + if err := cmd.ParseFlags(args); err != nil { + t.Fatal(err) + } + err := cmd.RunE(cmd, []string{"lab"}) + if err == nil || (!strings.Contains(err.Error(), "pass --cpus") && !strings.Contains(err.Error(), "use --clear alone")) { + t.Fatalf("%v: %v", args, err) + } + } +} + +func TestTenantCpusetCreateAndCommandFlags(t *testing.T) { + create := newTenantCreateCmd() + cpuset := newTenantCpusetCmd() + for _, flag := range []string{"cpus", "mems"} { + if create.Flags().Lookup(flag) == nil || cpuset.Flags().Lookup(flag) == nil { + t.Fatalf("missing %s", flag) + } + } + if !strings.Contains(cpuset.Long, "migrates existing memory pages") { + t.Fatal("help omits memory migration") + } +} diff --git a/internal/tenant/cpuset.go b/internal/tenant/cpuset.go new file mode 100644 index 0000000..791b893 --- /dev/null +++ b/internal/tenant/cpuset.go @@ -0,0 +1,205 @@ +package tenant + +import ( + "context" + "fmt" + "os" + "path/filepath" + "slices" + "sort" + "strconv" + "strings" +) + +var ( + onlineCPUsPath = "/sys/devices/system/cpu/online" + memoryNodesPath = "/sys/devices/system/node/has_memory" +) + +type cpuRange struct{ first, last int } + +// parseCPUList uses intervals rather than expanding potentially huge ranges. +// Empty values represent unmanaged settings. Non-empty results are merged and sorted. +func parseCPUList(value string) ([]cpuRange, error) { + if value == "" { + return nil, nil + } + var ranges []cpuRange + for _, item := range strings.Split(value, ",") { + parts := strings.Split(item, "-") + if len(parts) > 2 { + return nil, fmt.Errorf("invalid range") + } + nums := make([]int, len(parts)) + for i, part := range parts { + if part == "" || strings.IndexFunc(part, func(r rune) bool { return r < '0' || r > '9' }) >= 0 { + return nil, fmt.Errorf("invalid index") + } + n, err := strconv.ParseUint(part, 10, 31) + if err != nil { + return nil, err + } + nums[i] = int(n) + } + end := nums[len(nums)-1] + if nums[0] > end { + return nil, fmt.Errorf("descending range") + } + ranges = append(ranges, cpuRange{nums[0], end}) + } + sort.Slice(ranges, func(i, j int) bool { return ranges[i].first < ranges[j].first }) + merged := ranges[:0] + for _, r := range ranges { + if len(merged) > 0 && int64(r.first) <= int64(merged[len(merged)-1].last)+1 { + merged[len(merged)-1].last = max(merged[len(merged)-1].last, r.last) + } else { + merged = append(merged, r) + } + } + return merged, nil +} + +func sameCPUList(a, b string) bool { + x, err := parseCPUList(a) + if err != nil { + return false + } + y, err := parseCPUList(b) + return err == nil && slices.Equal(x, y) +} + +func readHostList(file, field string) (string, error) { + b, err := os.ReadFile(file) + if err != nil { + return "", fmt.Errorf("cannot read host %s list %s: %w; check sysfs is mounted and readable before retrying", field, file, err) + } + value := strings.TrimSpace(string(b)) + if _, err := parseCPUList(value); err != nil || value == "" { + return "", fmt.Errorf("host %s list %s is empty or invalid (%q); check sysfs before retrying", field, file, value) + } + return value, nil +} + +func validateHostCpuset(pin *Cpuset) error { + if pin == nil { + return nil + } + for _, field := range []struct{ name, value, file string }{ + {"cpuset.cpus", pin.CPUs, onlineCPUsPath}, {"cpuset.mems", pin.Mems, memoryNodesPath}, + } { + if field.value == "" { + continue + } + want, err := parseCPUList(field.value) + if err != nil { + return fmt.Errorf("%s: invalid list %q; use comma-separated N or N-M with N <= M and no spaces", field.name, field.value) + } + available, err := readHostList(field.file, field.name) + if err != nil { + return err + } + host, _ := parseCPUList(available) + for _, r := range want { + contained := false + for _, h := range host { + if h.first <= r.first && h.last >= r.last { + contained = true + break + } + } + if !contained { + return fmt.Errorf("%s %q is outside the host's available list %q; choose only values from that list and retry", field.name, field.value, available) + } + } + } + return nil +} + +// cpusetArgs emits only managed, differing settings. It also supplies run flags +// when the current settings are empty. +func cpusetArgs(pin *Cpuset, current Cpuset) []string { + if pin == nil { + return nil + } + var args []string + if pin.CPUs != "" && !sameCPUList(pin.CPUs, current.CPUs) { + args = append(args, "--cpuset-cpus", pin.CPUs) + } + if pin.Mems != "" && !sameCPUList(pin.Mems, current.Mems) { + args = append(args, "--cpuset-mems", pin.Mems) + } + return args +} + +func (m *Manager) updateCpuset(ctx context.Context, s *Spec, st containerState) error { + args := cpusetArgs(s.Cpuset, st.Cpuset) + if len(args) == 0 { + return nil + } + args = append(append([]string{"update"}, args...), ContainerName(s.Name)) + if err := m.run(ctx, "docker", args...); err != nil { + return fmt.Errorf("updating cpuset for %s failed: %w; check `docker inspect %s` and the host CPU/node lists, then retry `rootfiles tenant cpuset %s` with the desired flags", s.Name, err, ContainerName(s.Name), s.Name) + } + return nil +} + +// UpdateCpuset applies only live settings, even if a recreate is pending. +func (m *Manager) UpdateCpuset(ctx context.Context, s *Spec) error { + if err := s.Validate(); err != nil { + return err + } + if err := validateHostCpuset(s.Cpuset); err != nil { + return err + } + st := m.inspect(ctx, ContainerName(s.Name)) + if !st.Exists { + return fmt.Errorf("container %s is missing or cannot be inspected; check `docker inspect %s` and run `rootfiles tenant apply %s` to create a missing container before retrying", ContainerName(s.Name), ContainerName(s.Name), s.Name) + } + if err := m.updateCpuset(ctx, s, st); err != nil { + return err + } + if st.Digest != s.Digest() { + m.printf("Container settings are still pending; `rootfiles tenant apply %s` is needed and will recreate the tenant to apply them. Cpuset was updated without a restart.\n", s.Name) + } + return nil +} + +// ChangeCpuset saves the desired spec and runs only the live update. On failure +// it restores the exact previous spec, including comments. Clear is explicitly +// different from an empty (unmanaged) key: it resets both keys to full host lists. +func (m *Manager) ChangeCpuset(ctx context.Context, s *Spec, clear bool) error { + if err := s.Validate(); err != nil { + return err + } + live := *s + if clear { + cpus, err := readHostList(onlineCPUsPath, "cpuset.cpus") + if err != nil { + return err + } + mems, err := readHostList(memoryNodesPath, "cpuset.mems") + if err != nil { + return err + } + live.Cpuset = &Cpuset{CPUs: cpus, Mems: mems} + s.Cpuset = nil + } + if err := validateHostCpuset(live.Cpuset); err != nil { + return err + } + file := filepath.Join(m.SpecDir, s.Name+".yaml") + before, err := os.ReadFile(file) + if err != nil { + return fmt.Errorf("cannot preserve spec %s before cpuset change: %w; check the spec is readable and retry", file, err) + } + if err := m.SaveSpec(s); err != nil { + return err + } + if err := m.UpdateCpuset(ctx, &live); err != nil { + if rerr := m.Runner.WriteFile(file, before, 0o600); rerr != nil { + return fmt.Errorf("%w; restoring spec %s also failed: %v; restore it from the rootfiles backup before retrying", err, file, rerr) + } + return fmt.Errorf("%w; previous spec restored", err) + } + return nil +} diff --git a/internal/tenant/cpuset_test.go b/internal/tenant/cpuset_test.go new file mode 100644 index 0000000..208fd44 --- /dev/null +++ b/internal/tenant/cpuset_test.go @@ -0,0 +1,344 @@ +package tenant + +import ( + "bytes" + "context" + "log/slog" + "os" + "path/filepath" + "reflect" + "strings" + "testing" + + "github.com/STAIxBWLB/rootfiles/internal/exec" +) + +func stubCpusetHost(t *testing.T) { + t.Helper() + oldCPU, oldMems := onlineCPUsPath, memoryNodesPath + t.Cleanup(func() { onlineCPUsPath, memoryNodesPath = oldCPU, oldMems }) + dir := t.TempDir() + onlineCPUsPath, memoryNodesPath = filepath.Join(dir, "online"), filepath.Join(dir, "has_memory") + for file, content := range map[string]string{onlineCPUsPath: "0-7,16-23\n", memoryNodesPath: "0-1\n"} { + if err := os.WriteFile(file, []byte(content), 0600); err != nil { + t.Fatal(err) + } + } +} + +// Only a private docker is executable. Any unexpected ambient tool fails closed. +func cpusetManager(t *testing.T) (*Manager, string, *bytes.Buffer) { + t.Helper() + stubCpusetHost(t) + dir := t.TempDir() + log := filepath.Join(dir, "calls") + t.Setenv("CPUSET_CALLS", log) + script := `#!/bin/sh +printf '%s\n' "$*" >> "$CPUSET_CALLS" +case "$1" in +inspect) + [ -n "$CPUSET_INSPECT" ] || { echo 'container missing' >&2; exit 1; } + printf '%s\n' "$CPUSET_INSPECT" ;; +update) + if [ "$CPUSET_FAIL" = 1 ]; then echo 'cpuset update rejected by fixture' >&2; exit 1; fi ;; +exec) : ;; +*) echo "unexpected docker mutation: $*" >&2; exit 1 ;; +esac +` + if err := os.WriteFile(filepath.Join(dir, "docker"), []byte(script), 0755); err != nil { + t.Fatal(err) + } + t.Setenv("PATH", dir) + t.Setenv("CPUSET_FAIL", "") + t.Setenv("CPUSET_INSPECT", "") + out := &bytes.Buffer{} + m := &Manager{Runner: exec.NewRunner(false, slog.New(slog.NewTextHandler(&bytes.Buffer{}, nil))), Root: t.TempDir(), SpecDir: t.TempDir(), UnitDir: t.TempDir(), Out: out} + m.Runner.QueryGuard = func(file string) error { + if filepath.Dir(file) != dir { + t.Fatalf("ambient query %s", file) + } + return nil + } + return m, log, out +} + +func calls(t *testing.T, log string) string { + t.Helper() + b, err := os.ReadFile(log) + if err != nil && !os.IsNotExist(err) { + t.Fatal(err) + } + return string(b) +} + +func TestCpusetValidation(t *testing.T) { + for _, value := range []string{"", "0", "0-3,16-23", "4,0-2,2-4", "2147483647"} { + s := validSpec() + s.Cpuset = &Cpuset{CPUs: value, Mems: value} + if err := s.Validate(); err != nil { + t.Errorf("%q: %v", value, err) + } + } + for _, value := range []string{"-1", "1-0", "0,", ",0", "0,,1", "0-1-2", " 0", "0, 1", "1\n", "+1", "1.0", "2147483648", "9999999999999999999999999999999"} { + s := validSpec() + s.Cpuset = &Cpuset{CPUs: value, Mems: value} + s.Image = "" + err := s.Validate() + if err == nil { + t.Errorf("accepted %q", value) + continue + } + for _, field := range []string{"cpuset.cpus", "cpuset.mems", "N or N-M", "image"} { + if !strings.Contains(err.Error(), field) { + t.Errorf("missing %s: %v", field, err) + } + } + } +} + +func TestCpusetLegacyGolden(t *testing.T) { + s := validSpec() + // Captured from the pre-cpuset JSON serializer; upgrade must retain it. + const digest = "47f70c9332843d06" + if s.Digest() != digest { + t.Fatalf("legacy digest changed: %s", s.Digest()) + } + m := &Manager{Root: "/raid/tenants"} + want := []string{"run", "-d", "--name", "tenant-est", "--hostname", "est", "--restart=always", "--init", "--ipc=host", "--label", "rootfiles.tenant=est", "--label", "rootfiles.tenant.spec=" + digest, "--label", "rootfiles.tenant.gpus=2,3", "--runtime=nvidia", "--gpus", `"device=2,3"`, "-p", "3889:22", "-p", "8888:8888", "-p", "9000:9000/udp", "--mount", "type=bind,src=/raid/tenants/est/vol/data,dst=/data", "--mount", "type=bind,src=/raid/tenants/est/vol/home,dst=/home", "--mount", "type=bind,src=/raid/tenants/est/vol/tmp,dst=/tmp", "--mount", "type=bind,src=/raid/tenants/est/system,dst=/etc/tenant-system,readonly", "--entrypoint", "/bin/sh", "registry.local/est:1", "/etc/tenant-system/init.sh"} + if got := m.RunArgs(s); !reflect.DeepEqual(got, want) { + t.Fatalf("legacy RunArgs changed: %v", got) + } + s.Cpuset = &Cpuset{} + if s.Digest() != digest || !reflect.DeepEqual(m.RunArgs(s), want) { + t.Fatal("empty cpuset changed legacy behavior") + } + for _, pin := range []Cpuset{{CPUs: "0-3"}, {Mems: "1"}, {CPUs: "0,2", Mems: "0-1"}} { + s.Cpuset = &pin + if s.Digest() != digest { + t.Fatal("cpuset changed recreate digest") + } + got := strings.Join(m.RunArgs(s), " ") + for _, flag := range []struct{ key, value string }{{"--cpuset-cpus", pin.CPUs}, {"--cpuset-mems", pin.Mems}} { + if flag.value != "" && !strings.Contains(got, flag.key+" "+flag.value) { + t.Errorf("missing %s: %s", flag.key, got) + } + if flag.value == "" && strings.Contains(got, flag.key) { + t.Errorf("unmanaged flag %s: %s", flag.key, got) + } + } + } +} + +func TestCpusetConvergeContainer(t *testing.T) { + for _, tc := range []struct { + name, running string + pin *Cpuset + current, want string + }{ + {"live", "true", &Cpuset{CPUs: "2-3", Mems: "1"}, "0-1|0", "update --cpuset-cpus 2-3 --cpuset-mems 1 tenant-est\nexec tenant-est /bin/sh /etc/tenant-system/init.sh users\n"}, + {"matching", "true", &Cpuset{CPUs: "0-1", Mems: "0"}, "0,1|0", "exec tenant-est /bin/sh /etc/tenant-system/init.sh users\n"}, + {"unmanaged", "true", nil, "16-23|1", "exec tenant-est /bin/sh /etc/tenant-system/init.sh users\n"}, + {"empty", "true", &Cpuset{}, "16-23|1", "exec tenant-est /bin/sh /etc/tenant-system/init.sh users\n"}, + {"one-key", "true", &Cpuset{Mems: "1"}, "16-23|0", "update --cpuset-mems 1 tenant-est\nexec tenant-est /bin/sh /etc/tenant-system/init.sh users\n"}, + } { + t.Run(tc.name, func(t *testing.T) { + m, log, _ := cpusetManager(t) + s := validSpec() + s.Cpuset = tc.pin + t.Setenv("CPUSET_INSPECT", tc.running+"|"+s.Digest()+"|always|2,3|"+tc.current) + if err := m.convergeContainer(context.Background(), s); err != nil { + t.Fatal(err) + } + lines := strings.SplitAfter(calls(t, log), "\n") + if got := strings.Join(lines[1:], ""); got != tc.want { + t.Fatalf("steps = %q, want %q", got, tc.want) + } + }) + } +} + +func TestCpusetHostRefusesBeforeAnyApplyChange(t *testing.T) { + for _, pin := range []Cpuset{{CPUs: "8"}, {CPUs: "0-16"}, {Mems: "2"}, {CPUs: "0", Mems: "99"}} { + m, log, _ := cpusetManager(t) + s := validSpec() + s.Cpuset = &pin + reserved := false + m.ReserveGPUs = func(context.Context, string, []int) error { reserved = true; return nil } + err := m.Apply(context.Background(), s) + if err == nil || !strings.Contains(err.Error(), "available list") || !strings.Contains(err.Error(), "choose") { + t.Fatalf("host validation: %v", err) + } + if reserved || calls(t, log) != "" { + t.Fatal("mutated before host validation") + } + if files, _ := os.ReadDir(m.Root); len(files) > 0 { + t.Fatal("wrote tenant data") + } + } + m, log, _ := cpusetManager(t) + s := validSpec() + s.Cpuset = &Cpuset{CPUs: "0"} + if err := os.Remove(onlineCPUsPath); err != nil { + t.Fatal(err) + } + if err := m.Apply(context.Background(), s); err == nil || !strings.Contains(err.Error(), "sysfs") { + t.Fatalf("missing sysfs accepted: %v", err) + } + if calls(t, log) != "" { + t.Fatal("docker ran with missing sysfs") + } + // Unmanaged keys do not even read missing sysfs. + if err := validateHostCpuset(nil); err != nil { + t.Fatal(err) + } + if err := validateHostCpuset(&Cpuset{}); err != nil { + t.Fatal(err) + } +} + +func TestChangeCpusetLiveOnlyRollbackAndClear(t *testing.T) { + for _, tc := range []struct { + name string + fail, clear, dry, missing bool + digest, running string + }{ + {name: "pending", digest: "old", running: "true"}, + {name: "unlabelled", digest: "", running: "true"}, + {name: "stopped", digest: "old", running: "false"}, + {name: "failed", fail: true, digest: "old", running: "true"}, + {name: "missing", missing: true}, + {name: "clear", clear: true, digest: "old", running: "true"}, + {name: "dry", dry: true, digest: "old", running: "true"}, + } { + t.Run(tc.name, func(t *testing.T) { + m, log, out := cpusetManager(t) + s := validSpec() + before, _ := s.Marshal() + before = append([]byte("# retain this operator comment\n"), before...) + file := filepath.Join(m.SpecDir, "est.yaml") + if err := os.WriteFile(file, before, 0600); err != nil { + t.Fatal(err) + } + s.Cpuset = &Cpuset{CPUs: "2-3", Mems: "1"} + if !tc.missing { + t.Setenv("CPUSET_INSPECT", tc.running+"|"+tc.digest+"|always|2,3|0-1|0") + } + if tc.fail { + t.Setenv("CPUSET_FAIL", "1") + } + m.Runner.DryRun = tc.dry + err := m.ChangeCpuset(context.Background(), s, tc.clear) + after, _ := os.ReadFile(file) + if tc.fail || tc.missing { + if err == nil || !strings.Contains(err.Error(), "previous spec restored") { + t.Fatalf("failure = %v", err) + } + if !bytes.Equal(before, after) { + t.Fatal("spec was not restored byte for byte") + } + if tc.fail && !strings.Contains(err.Error(), "cpuset update rejected by fixture") { + t.Fatalf("missing docker cause: %v", err) + } + } else { + if err != nil { + t.Fatal(err) + } + if !strings.Contains(out.String(), "rootfiles tenant apply est") { + t.Fatalf("missing pending recreate message: %s", out) + } + if tc.dry { + if !bytes.Equal(before, after) { + t.Fatal("dry run changed spec") + } + } else { + loaded, err := m.Load("est") + if err != nil { + t.Fatal(err) + } + if tc.clear && loaded.Cpuset != nil { + t.Fatal("clear retained field") + } + if !tc.clear && (loaded.Cpuset == nil || *loaded.Cpuset != *s.Cpuset) { + t.Fatal("pin not saved") + } + } + } + text := calls(t, log) + for _, line := range strings.Split(strings.TrimSpace(text), "\n") { + if !strings.HasPrefix(line, "inspect ") && !strings.HasPrefix(line, "update --cpuset-") { + t.Fatalf("non-live step: %s", line) + } + } + if tc.clear && !strings.Contains(text, "update --cpuset-cpus 0-7,16-23 --cpuset-mems 0-1 tenant-est") { + t.Fatalf("clear did not reset: %s", text) + } + if tc.dry && strings.Contains(text, "update ") { + t.Fatal("dry run updated container") + } + }) + } +} + +func TestCpusetStatusShowsLiveDriftAndUnmanagedPins(t *testing.T) { + m, _, out := cpusetManager(t) + s := validSpec() + s.Cpuset = &Cpuset{CPUs: "2-3"} + if err := m.SaveSpec(s); err != nil { + t.Fatal(err) + } + t.Setenv("CPUSET_INSPECT", "true|"+s.Digest()+"|always|2,3|0-1|1") + if err := m.Status(context.Background(), []string{"est"}, true); err != nil { + t.Fatal(err) + } + for _, want := range []string{"update", "cpuset spec: cpus=2-3 mems=unmanaged", `cpuset container: cpus="0-1" mems="1"`} { + if !strings.Contains(out.String(), want) { + t.Errorf("missing %s: %s", want, out) + } + } + out.Reset() + s.Cpuset = nil + if err := m.SaveSpec(s); err != nil { + t.Fatal(err) + } + if err := m.Status(context.Background(), []string{"est"}, true); err != nil { + t.Fatal(err) + } + if !strings.Contains(out.String(), "current") || !strings.Contains(out.String(), `cpus="0-1"`) { + t.Fatalf("unmanaged pin not visible/current: %s", out) + } +} + +func TestImportCarriesCpuset(t *testing.T) { + stubCpusetHost(t) + dir := t.TempDir() + script := `#!/bin/sh +case "$1 $2" in +"inspect -f") exit 1 ;; +"inspect legacy") printf '%s\n' '[{"Config":{"Image":"ubuntu:test"},"State":{"Running":true},"HostConfig":{"CpusetCpus":"2-3","CpusetMems":"1","PortBindings":{"22/tcp":[{"HostPort":"2201"}]}}}]' ;; +"exec legacy") case "$*" in + */etc/passwd) printf '%s\n' 'alice:x:1000:1000::/home/alice:/bin/bash' ;; + */etc/group) printf '%s\n' 'sudo:x:27:alice' ;; + */etc/shadow) printf '%s\n' 'alice:!:0:0:99999:7:::' ;; + esac ;; +*) exit 1 ;; +esac +` + for name, body := range map[string]string{"docker": script, "rsync": "#!/bin/sh\nexit 0\n", "systemctl": "#!/bin/sh\nprintf 'enabled\\n'\n"} { + if err := os.WriteFile(filepath.Join(dir, name), []byte(body), 0755); err != nil { + t.Fatal(err) + } + } + t.Setenv("PATH", dir) + var log bytes.Buffer + m := &Manager{Runner: exec.NewRunner(true, slog.New(slog.NewTextHandler(&log, nil))), Root: t.TempDir(), SpecDir: t.TempDir(), UnitDir: t.TempDir(), Out: &bytes.Buffer{}} + s, err := m.Import(context.Background(), ImportOptions{Tenant: "imported", From: "legacy", Size: "128M"}) + if err != nil { + t.Fatal(err) + } + if s.Cpuset == nil || *s.Cpuset != (Cpuset{CPUs: "2-3", Mems: "1"}) { + t.Fatalf("legacy pin lost: %+v", s) + } + if !strings.Contains(log.String(), "--cpuset-cpus 2-3 --cpuset-mems 1") { + t.Fatalf("import run lost pin: %s", &log) + } +} diff --git a/internal/tenant/import.go b/internal/tenant/import.go index f06c5cc..0a37703 100644 --- a/internal/tenant/import.go +++ b/internal/tenant/import.go @@ -34,8 +34,9 @@ type legacyContainer struct { Running bool } HostConfig struct { - PortBindings map[string][]struct{ HostPort string } - DeviceRequests []struct { + CpusetCpus, CpusetMems string + PortBindings map[string][]struct{ HostPort string } + DeviceRequests []struct { Driver string Count int DeviceIDs []string @@ -81,6 +82,9 @@ func (m *Manager) Import(ctx context.Context, o ImportOptions) (*Spec, error) { } s := &Spec{Name: o.Tenant, Image: old.Config.Image, Volume: Volume{Size: o.Size, FS: o.FS}, Operators: o.Operators} + if old.HostConfig.CpusetCpus != "" || old.HostConfig.CpusetMems != "" { + s.Cpuset = &Cpuset{CPUs: old.HostConfig.CpusetCpus, Mems: old.HostConfig.CpusetMems} + } for port, binds := range old.HostConfig.PortBindings { if len(binds) == 0 || binds[0].HostPort == "" { continue @@ -124,6 +128,10 @@ func (m *Manager) Import(ctx context.Context, o ImportOptions) (*Spec, error) { return nil, err } + if err := validateHostCpuset(s.Cpuset); err != nil { + return nil, err + } + // Plan and size check before any change. merged := old.GraphDriver.Data["MergedDir"] type copyItem struct{ src, dst string } diff --git a/internal/tenant/manager.go b/internal/tenant/manager.go index 41e76a2..87886d6 100644 --- a/internal/tenant/manager.go +++ b/internal/tenant/manager.go @@ -95,19 +95,23 @@ type containerState struct { Digest string Restart string GPUs string // the GPUs the container was started with + Cpuset Cpuset } func (m *Manager) inspect(ctx context.Context, name string) containerState { res, err := m.Runner.Query(ctx, "docker", "inspect", "-f", - `{{.State.Running}}|{{index .Config.Labels "rootfiles.tenant.spec"}}|{{.HostConfig.RestartPolicy.Name}}|{{index .Config.Labels "rootfiles.tenant.gpus"}}`, name) + `{{.State.Running}}|{{index .Config.Labels "rootfiles.tenant.spec"}}|{{.HostConfig.RestartPolicy.Name}}|{{index .Config.Labels "rootfiles.tenant.gpus"}}|{{.HostConfig.CpusetCpus}}|{{.HostConfig.CpusetMems}}`, name) if err != nil { return containerState{} } f := strings.Split(strings.TrimSpace(res.Stdout), "|") st := containerState{Exists: true} - if len(f) == 4 { + if len(f) >= 4 { st.Running, st.Digest, st.Restart, st.GPUs = f[0] == "true", f[1], f[2], f[3] } + if len(f) >= 6 { + st.Cpuset = Cpuset{CPUs: f[4], Mems: f[5]} + } return st } @@ -122,6 +126,7 @@ func (m *Manager) RunArgs(s *Spec) []string { "--label", "rootfiles.tenant.spec=" + s.Digest(), "--label", "rootfiles.tenant.gpus=" + intList(s.GPUs), } + args = append(args, cpusetArgs(s.Cpuset, Cpuset{})...) if len(s.GPUs) > 0 { ids := make([]string, len(s.GPUs)) for i, g := range s.GPUs { @@ -165,6 +170,9 @@ func (m *Manager) Apply(ctx context.Context, s *Spec) error { if err := s.Validate(); err != nil { return err } + if err := validateHostCpuset(s.Cpuset); err != nil { + return err + } if m.ReserveGPUs != nil { if err := m.ReserveGPUs(ctx, s.Name, s.GPUs); err != nil { return err @@ -200,8 +208,17 @@ func (m *Manager) converge(ctx context.Context, s *Spec) error { if err := m.writeSystem(ctx, s); err != nil { return err } + return m.convergeContainer(ctx, s) +} + +func (m *Manager) convergeContainer(ctx context.Context, s *Spec) error { name := ContainerName(s.Name) st := m.inspect(ctx, name) + if st.Exists && st.Digest == s.Digest() { + if err := m.updateCpuset(ctx, s, st); err != nil { + return err + } + } switch { case !st.Exists: return m.start(ctx, s) @@ -235,6 +252,10 @@ func (m *Manager) recreate(ctx context.Context, s *Spec, st containerState) erro if m.inspect(ctx, prev).Exists { return fmt.Errorf("%s still holds the previous container; check it and remove it (docker rm %s) before recreating", prev, prev) } + if (st.Cpuset.CPUs != "" && (s.Cpuset == nil || s.Cpuset.CPUs == "")) || + (st.Cpuset.Mems != "" && (s.Cpuset == nil || s.Cpuset.Mems == "")) { + m.printf(ui.MarkWarn+" %s has an unmanaged cpuset (cpus=%q mems=%q); unmanaged keys will lose their pin on recreate. Before recreating, adopt them with `rootfiles tenant cpuset %s --cpus LIST --mems LIST`\n", name, st.Cpuset.CPUs, st.Cpuset.Mems, s.Name) + } if st.Running { if err := m.SyncShadow(ctx, s); err != nil { return fmt.Errorf("saving passwords before recreate: %w", err) @@ -635,6 +656,8 @@ func (m *Manager) Status(ctx context.Context, names []string, detail bool) error drift := "current" if st.Exists && st.Digest != s.Digest() { drift = "drift" + } else if st.Exists && len(cpusetArgs(s.Cpuset, st.Cpuset)) > 0 { + drift = "update" } vol := "unmounted/" + s.Volume.Size if u := m.usage(n); u.Mounted { @@ -648,6 +671,16 @@ func (m *Manager) Status(ctx context.Context, names []string, detail bool) error for _, r := range m.Routes(s) { fmt.Fprintf(m.Out, " %s\n", r) } + if detail { + pin := Cpuset{} + if s.Cpuset != nil { + pin = *s.Cpuset + } + m.printf(" cpuset spec: cpus=%s mems=%s (empty keys are unmanaged)\n", orDefault(pin.CPUs, "unmanaged"), orDefault(pin.Mems, "unmanaged")) + if st.Exists { + m.printf(" cpuset container: cpus=%q mems=%q\n", st.Cpuset.CPUs, st.Cpuset.Mems) + } + } if detail && st.Exists { if size := m.writableSize(ctx, ContainerName(n)); size != "" { fmt.Fprintf(m.Out, " writable layer: %s (no quota; keep data in /data, /home or /tmp)\n", size) diff --git a/internal/tenant/spec.go b/internal/tenant/spec.go index e46f8bb..0fde213 100644 --- a/internal/tenant/spec.go +++ b/internal/tenant/spec.go @@ -49,6 +49,7 @@ type Spec struct { Users []User `yaml:"users"` Operators []string `yaml:"operators,omitempty"` ExtraBinds []string `yaml:"extra_binds,omitempty"` + Cpuset *Cpuset `yaml:"cpuset,omitempty"` } // Volume is the tenant's size-capped volume. @@ -57,6 +58,12 @@ type Volume struct { FS string `yaml:"fs,omitempty"` // ext4 (default) or xfs } +// Cpuset contains live CPU and NUMA memory-node settings. Empty keys are unmanaged. +type Cpuset struct { + CPUs string `yaml:"cpus,omitempty"` + Mems string `yaml:"mems,omitempty"` +} + // User is an account created inside the container on every start. type User struct { Name string `yaml:"name"` @@ -114,6 +121,13 @@ func (s *Spec) Validate() error { } seenGPU[g] = true } + if s.Cpuset != nil { + for _, field := range []struct{ name, value string }{{"cpus", s.Cpuset.CPUs}, {"mems", s.Cpuset.Mems}} { + if _, err := parseCPUList(field.value); err != nil { + add("cpuset.%s: invalid list %q; use comma-separated N or N-M with N <= M and no spaces", field.name, field.value) + } + } + } if s.SSHPort < 1 || s.SSHPort > 65535 { add("ssh_port: %d out of range", s.SSHPort) } diff --git a/tests/scenarios/tenant-lifecycle.sh b/tests/scenarios/tenant-lifecycle.sh index bd8271f..b22b2d5 100755 --- a/tests/scenarios/tenant-lifecycle.sh +++ b/tests/scenarios/tenant-lifecycle.sh @@ -128,6 +128,22 @@ docker rm tenant-t1-prev >/dev/null # Back to the running container's settings: nothing to recreate. check "apply is idempotent" bash -c "$RF tenant port t1 2201 | grep -q 'up to date'" +echo "--- #129 AC9: live CPU/NUMA pinning ---" +online_cpus=$(cat /sys/devices/system/cpu/online) +memory_nodes=$(cat /sys/devices/system/node/has_memory) +first_cpu=${online_cpus%%[,-]*} +first_node=${memory_nodes%%[,-]*} +cpuset_identity=$(docker inspect -f '{{.Id}}|{{.State.StartedAt}}|{{.State.Pid}}' tenant-t1) +$RF tenant cpuset t1 --cpus "$first_cpu" --mems "$first_node" +check "cpuset pins requested CPU and memory node" outputs "$first_cpu|$first_node" docker inspect -f '{{.HostConfig.CpusetCpus}}|{{.HostConfig.CpusetMems}}' tenant-t1 +check "pin keeps container ID, start time and PID" outputs "$cpuset_identity" docker inspect -f '{{.Id}}|{{.State.StartedAt}}|{{.State.Pid}}' tenant-t1 +$RF tenant cpuset t1 --cpus "$online_cpus" --mems "$memory_nodes" +check "cpuset changes live to full host lists" outputs "$online_cpus|$memory_nodes" docker inspect -f '{{.HostConfig.CpusetCpus}}|{{.HostConfig.CpusetMems}}' tenant-t1 +check "live change keeps container ID, start time and PID" outputs "$cpuset_identity" docker inspect -f '{{.Id}}|{{.State.StartedAt}}|{{.State.Pid}}' tenant-t1 +check "pinned tenant is still running" outputs true docker inspect -f '{{.State.Running}}' tenant-t1 +$RF tenant cpuset t1 --clear +check "clear removes spec field" bash -c '! sudo grep -q "^cpuset:" /srv/home/.rootfiles/tenants/t1.yaml' + echo "--- sshpiper routing (#80) ---" check "route file written" sudo test -f /etc/rootfiles/sshpiper/tenants/t1.yaml check "routed password login" outputs 2000 piper_as t1.alice Changed-Pass-42 "id -u" From bc827111d3c8291d56a27f6a8c8d1bf5fde221da Mon Sep 17 00:00:00 2001 From: Young Joon Lee Date: Tue, 10 Nov 2026 20:47:58 +0900 Subject: [PATCH 2/2] test(tenant): cover stopped-container updates and unmanaged pin warnings --- internal/cli/tenant.go | 3 ++- internal/tenant/cpuset.go | 2 +- internal/tenant/cpuset_test.go | 23 ++++++++++++++++++++++- 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/internal/cli/tenant.go b/internal/cli/tenant.go index 81786b1..a408469 100644 --- a/internal/cli/tenant.go +++ b/internal/cli/tenant.go @@ -388,7 +388,8 @@ func newTenantCpusetCmd() *cobra.Command { Use: "cpuset TENANT", Short: "Save CPU/NUMA pinning and update the existing container without restarting", Long: `Changes only CPU/NUMA pinning, leaving other pending settings for tenant apply. -Empty or omitted keys are unmanaged. --clear removes the spec field and resets +Empty or absent spec keys are unmanaged; omitted flags retain existing spec keys. +--clear removes the spec field and resets both container settings to the host's full lists. On cgroup v2, changing memory nodes migrates existing memory pages and can take time for large processes.`, Args: cobra.ExactArgs(1), diff --git a/internal/tenant/cpuset.go b/internal/tenant/cpuset.go index 791b893..a3c8a70 100644 --- a/internal/tenant/cpuset.go +++ b/internal/tenant/cpuset.go @@ -159,7 +159,7 @@ func (m *Manager) UpdateCpuset(ctx context.Context, s *Spec) error { return err } if st.Digest != s.Digest() { - m.printf("Container settings are still pending; `rootfiles tenant apply %s` is needed and will recreate the tenant to apply them. Cpuset was updated without a restart.\n", s.Name) + m.printf("Container settings are still pending; `rootfiles tenant apply %s` is needed and will recreate the tenant to apply them.\n", s.Name) } return nil } diff --git a/internal/tenant/cpuset_test.go b/internal/tenant/cpuset_test.go index 208fd44..5722fa4 100644 --- a/internal/tenant/cpuset_test.go +++ b/internal/tenant/cpuset_test.go @@ -41,7 +41,7 @@ inspect) printf '%s\n' "$CPUSET_INSPECT" ;; update) if [ "$CPUSET_FAIL" = 1 ]; then echo 'cpuset update rejected by fixture' >&2; exit 1; fi ;; -exec) : ;; +exec|start) : ;; *) echo "unexpected docker mutation: $*" >&2; exit 1 ;; esac ` @@ -135,6 +135,7 @@ func TestCpusetConvergeContainer(t *testing.T) { pin *Cpuset current, want string }{ + {"stopped", "false", &Cpuset{CPUs: "2-3"}, "0-1|0", "update --cpuset-cpus 2-3 tenant-est\nstart tenant-est\n"}, {"live", "true", &Cpuset{CPUs: "2-3", Mems: "1"}, "0-1|0", "update --cpuset-cpus 2-3 --cpuset-mems 1 tenant-est\nexec tenant-est /bin/sh /etc/tenant-system/init.sh users\n"}, {"matching", "true", &Cpuset{CPUs: "0-1", Mems: "0"}, "0,1|0", "exec tenant-est /bin/sh /etc/tenant-system/init.sh users\n"}, {"unmanaged", "true", nil, "16-23|1", "exec tenant-est /bin/sh /etc/tenant-system/init.sh users\n"}, @@ -342,3 +343,23 @@ esac t.Fatalf("import run lost pin: %s", &log) } } + +func TestRecreateWarnsBeforeLosingUnmanagedPin(t *testing.T) { + m, log, out := cpusetManager(t) + s := validSpec() + // The previous container does not exist. Docker mutations beyond the warning + // fail in the private stub, so no actual recreate or readiness probing occurs. + t.Setenv("CPUSET_INSPECT", "") + err := m.recreate(context.Background(), s, containerState{Exists: true, Cpuset: Cpuset{CPUs: "2-3", Mems: "1"}}) + if err == nil { + t.Fatal("fixture should stop at refused Docker stop") + } + for _, want := range []string{"unmanaged cpuset", "lose their pin", "rootfiles tenant cpuset est"} { + if !strings.Contains(out.String(), want) { + t.Errorf("missing warning %s: %s", want, out) + } + } + if strings.Contains(calls(t, log), "run -d") { + t.Fatal("fixture reached docker run") + } +}