From e53be603499826d33977c9dcfc7c5a7f873f8dec Mon Sep 17 00:00:00 2001 From: Chennamma-Hotkar Date: Sat, 30 May 2026 14:42:40 +0000 Subject: [PATCH] fix(ps): return all host-visible PIDs via /proc walk Previously ps only returned the monitor PID. Now getAllDescendants() recursively walks /proc//task//children to include all VMM sub-processes and virtiofsd in the output. Closes #637 Signed-off-by: Chennamma-Hotkar --- cmd/urunc/ps.go | 44 ++++++++++++++++---- cmd/urunc/ps_test.go | 76 ++++++++++++++++++++++++++++++++++ pkg/unikontainers/shared_fs.go | 7 ++-- pkg/unikontainers/utils.go | 6 +-- 4 files changed, 119 insertions(+), 14 deletions(-) create mode 100644 cmd/urunc/ps_test.go diff --git a/cmd/urunc/ps.go b/cmd/urunc/ps.go index 47de3c2fd..f15b8afd7 100644 --- a/cmd/urunc/ps.go +++ b/cmd/urunc/ps.go @@ -19,18 +19,49 @@ import ( "encoding/json" "fmt" "os" + "strconv" + "strings" "github.com/sirupsen/logrus" "github.com/urfave/cli/v3" ) +// getAllDescendants returns the given pid and all its descendant PIDs +// by walking /proc//task//children recursively. +func getAllDescendants(rootPid int) []int { + var pids []int + queue := []int{rootPid} + + for len(queue) > 0 { + pid := queue[0] + queue = queue[1:] + pids = append(pids, pid) + + childrenPath := fmt.Sprintf("/proc/%d/task/%d/children", pid, pid) + data, err := os.ReadFile(childrenPath) + if err != nil { + // process may have exited, skip silently + continue + } + + for _, field := range strings.Fields(string(data)) { + childPid, err := strconv.Atoi(field) + if err == nil { + queue = append(queue, childPid) + } + } + } + + return pids +} + var psCommand = &cli.Command{ Name: "ps", Usage: "displays the host-visible monitor processes associated with a container", ArgsUsage: ``, Description: `The ps command displays the host-visible process IDs associated -with a urunc container. This currently returns the host-visible monitor PID -stored in urunc state.json. +with a urunc container. It returns all host-visible PIDs including the monitor +process and all its descendants (e.g. VMM sub-processes, virtiofsd). This command intentionally implements the runc-compatible interface required by containerd-shim-runc-v2/go-runc: @@ -61,12 +92,9 @@ The JSON format must be a JSON array of integers, for example: return err } - // The host-visible process for the current implementation is the - // monitor process saved in state.json as State.Pid. - // - // Keep the return value as []int to match runc's ps implementation - // and containerd/go-runc's expectation for `ps --format json`. - pids := []int{unikontainer.State.Pid} + // Return all host-visible PIDs (monitor + descendants) to match runc's + // ps implementation and containerd/go-runc's expectation for `ps --format json`. + pids := getAllDescendants(unikontainer.State.Pid) switch cmd.String("format") { case "json": diff --git a/cmd/urunc/ps_test.go b/cmd/urunc/ps_test.go new file mode 100644 index 000000000..5c2931877 --- /dev/null +++ b/cmd/urunc/ps_test.go @@ -0,0 +1,76 @@ +// Copyright (c) 2023-2026, Nubificus LTD +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package main + +import ( + "os" + "os/exec" + "testing" +) + +// TestGetAllDescendants_IncludesRoot verifies that the root PID +// itself is always included in the result. +func TestGetAllDescendants_IncludesRoot(t *testing.T) { + pid := os.Getpid() + pids := getAllDescendants(pid) + + if len(pids) == 0 { + t.Fatal("expected at least one PID, got none") + } + if pids[0] != pid { + t.Errorf("expected first PID to be root %d, got %d", pid, pids[0]) + } +} + +// TestGetAllDescendants_NonExistentPID verifies that a non-existent +// PID returns only itself without crashing. +func TestGetAllDescendants_NonExistentPID(t *testing.T) { + pids := getAllDescendants(99999999) + + if len(pids) != 1 { + t.Errorf("expected 1 PID for non-existent process, got %d", len(pids)) + } + if pids[0] != 99999999 { + t.Errorf("expected PID 99999999, got %d", pids[0]) + } +} + +// TestGetAllDescendants_IncludesChild verifies that a spawned child +// process appears in the descendants list. +func TestGetAllDescendants_IncludesChild(t *testing.T) { + // Spawn a child process that sleeps long enough for us to inspect + child := exec.Command("sleep", "10") + if err := child.Start(); err != nil { + t.Fatalf("failed to start child process: %v", err) + } + defer child.Process.Kill() //nolint:errcheck + + childPid := child.Process.Pid + parentPid := os.Getpid() + + pids := getAllDescendants(parentPid) + + found := false + for _, p := range pids { + if p == childPid { + found = true + break + } + } + + if !found { + t.Errorf("child PID %d not found in descendants of %d: %v", childPid, parentPid, pids) + } +} diff --git a/pkg/unikontainers/shared_fs.go b/pkg/unikontainers/shared_fs.go index 54d3e23f3..6d6eff232 100644 --- a/pkg/unikontainers/shared_fs.go +++ b/pkg/unikontainers/shared_fs.go @@ -101,11 +101,12 @@ func (s sharedfsRootfs) preStart() error { args = append(args, strings.Fields(s.vfsdConfig.Options)...) } - err := spawnProcess(s.vfsdConfig.Path, args) + vfsdPid, err := spawnProcess(s.vfsdConfig.Path, args) if err != nil { - err = fmt.Errorf("failed to start virtiofsd: %w", err) + return fmt.Errorf("failed to start virtiofsd: %w", err) } - return err + uniklog.WithField("virtiofsd_pid", vfsdPid).Debug("virtiofsd started") + return nil } func chooseTmpfsSize(sfsType string, mem uint64) string { diff --git a/pkg/unikontainers/utils.go b/pkg/unikontainers/utils.go index 3f2c64bc6..2f49fb018 100644 --- a/pkg/unikontainers/utils.go +++ b/pkg/unikontainers/utils.go @@ -203,17 +203,17 @@ func convertUint32ToIntSlice(valSlice []uint32, size int) []int { // return data.Bytes(), nil // } -func spawnProcess(binaryPath string, args []string) error { +func spawnProcess(binaryPath string, args []string) (int, error) { cmd := exec.Command(binaryPath, args...) cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr if err := cmd.Start(); err != nil { - return err + return -1, err } - return nil + return cmd.Process.Pid, nil } func resolveAgainstBase(base string, path string) (string, error) {