From e79cd047b81bf95dbbc2db0de1f4472ccf593b58 Mon Sep 17 00:00:00 2001 From: Anand-240 Date: Sun, 2 Aug 2026 15:19:09 +0530 Subject: [PATCH] fix(cmd/urunc): add fail-fast exec stub to fix Argo Workflows hang containerd-shim-runc-v2/go-runc invoke `urunc exec` to signal sidecar containers in a pod (e.g. Argo Workflows' wait container) via kubectl exec. urunc had no exec subcommand at all, so this call failed with an opaque CLI-usage error ("No help topic for 'exec'") that callers could not distinguish from "urunc is broken". This caused Argo workflows using runtimeClassName: urunc to hang in Running forever, since the sidecar could never be signaled to terminate. This adds a minimal exec subcommand that accepts the same CLI shape go-runc sends (--process, --console-socket, --detach, --pid-file, ) and fails fast with a clear "not supported" error and a dedicated exit code (255, matching runc's own convention), instead of the previous unknown-command failure. This is intentionally a fail-fast stub, not a real exec-into-unikernel implementation. See the doc comment on execCommand for why that's a separate, larger problem out of scope here. Fixes #882 Signed-off-by: Anand-240 --- Makefile | 8 ++- cmd/urunc/exec.go | 113 +++++++++++++++++++++++++++++++++++++ cmd/urunc/exec_test.go | 125 +++++++++++++++++++++++++++++++++++++++++ cmd/urunc/main.go | 1 + 4 files changed, 246 insertions(+), 1 deletion(-) create mode 100644 cmd/urunc/exec.go create mode 100644 cmd/urunc/exec_test.go diff --git a/Makefile b/Makefile index b0329d2d..c39d6bed 100644 --- a/Makefile +++ b/Makefile @@ -233,7 +233,7 @@ test: unittest e2etest ## unittest Run all unit tests .PHONY: unittest -unittest: test_unikontainers test_metrics test_network test_hypervisors test_unikernels +unittest: test_unikontainers test_metrics test_network test_hypervisors test_unikernels test_cmd_urunc ## e2etest Run all end-to-end tests .PHONY: e2etest @@ -269,6 +269,12 @@ test_unikernels: @GOFLAGS=$(TEST_FLAGS) $(GO) test $(TEST_OPTS) ./pkg/unikontainers/unikernels -v @echo " " +## test_cmd_urunc Run unit tests for the urunc CLI (cmd/urunc) +test_cmd_urunc: + @echo "Unit testing in cmd/urunc" + @GOFLAGS=$(TEST_FLAGS) $(GO) test $(TEST_OPTS) ./cmd/urunc -v + @echo " " + ## test_nerdctl Run all end-to-end tests with nerdctl .PHONY: test_nerdctl test_nerdctl: diff --git a/cmd/urunc/exec.go b/cmd/urunc/exec.go new file mode 100644 index 00000000..b2a019ad --- /dev/null +++ b/cmd/urunc/exec.go @@ -0,0 +1,113 @@ +// 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 ( + "context" + "errors" + "os" + + "github.com/sirupsen/logrus" + "github.com/urfave/cli/v3" +) + +// ErrExecNotSupported is returned whenever "urunc exec" is invoked. urunc +// containers run as unikernels/VMs: there is no general-purpose process +// namespace to attach a new process to once the unikernel has started, so +// unlike runc, urunc cannot execute an additional process inside a running +// container. +var ErrExecNotSupported = errors.New("exec is not supported by urunc: urunc containers run as " + + "unikernels/VMs and do not support attaching a new process after container start") + +// execNotSupportedExitCode is the exit code urunc uses to report that exec +// is unsupported. It intentionally matches runc's own convention of using a +// dedicated exit code (255, see runc's exec.go) for exec failures, instead +// of falling through to the generic exit code 1 used for other urunc CLI +// errors. This lets callers such as containerd/go-runc, which invoke this +// command as `urunc exec --process [flags] `, +// distinguish "exec is not supported" from a generic urunc failure, rather +// than retrying indefinitely as if it were transient. +const execNotSupportedExitCode = 255 + +// execCommand intentionally implements just enough of the runc-compatible +// "exec" interface required by containerd-shim-runc-v2/go-runc to be +// invoked without a CLI parsing failure: +// +// urunc exec --process [--console-socket ] [--detach] [--pid-file ] +// +// It does not attempt to actually execute a process inside a running +// unikernel: that would require attaching to a VM/unikernel's execution +// environment, which has no equivalent of a container's process namespace +// to exec into. Instead, it fails fast with a clear, distinct error so +// callers can tell "not supported" apart from "urunc is broken" and stop +// retrying instead of hanging indefinitely. +// +// See https://github.com/urunc-dev/urunc/issues/882 for the motivating +// failure mode: Argo Workflows relies on `kubectl exec` into a sidecar +// container to signal step completion, and because Kubernetes RuntimeClass +// is pod-scoped rather than per-container, that exec call was silently +// routed through urunc's shim and retried forever with an opaque failure. +var execCommand = &cli.Command{ + Name: "exec", + Usage: "not supported: urunc unikernels cannot exec an additional process after start", + ArgsUsage: ``, + Description: `The exec command is part of the OCI runtime CLI interface expected by +containerd-shim-runc-v2/go-runc, but urunc does not support it: a running +unikernel/VM has no general-purpose process namespace to attach a new +process to. + +This command exists so that callers relying on the standard OCI runtime CLI +interface (e.g. "kubectl exec" against a urunc-scheduled pod) get a clear, +immediate "not supported" error instead of urunc's CLI parser failing on an +undefined command and callers retrying forever.`, + Flags: []cli.Flag{ + &cli.StringFlag{ + Name: "process", + Usage: "path to the process.json describing the process to exec (unused: exec is not supported)", + }, + &cli.StringFlag{ + Name: "console-socket", + Usage: "path to an AF_UNIX socket for the console pty (unused: exec is not supported)", + }, + &cli.BoolFlag{ + Name: "detach", + Usage: "detach from the container's process (unused: exec is not supported)", + }, + &cli.StringFlag{ + Name: "pid-file", + Usage: "file to write the process id to (unused: exec is not supported)", + }, + }, + Action: func(_ context.Context, cmd *cli.Command) error { + logrus.WithField("command", "EXEC").WithField("args", os.Args).Debug("urunc INVOKED") + + if err := checkArgs(cmd, 1, minArgs); err != nil { + return err + } + + containerID := cmd.Args().First() + if err := validateID(containerID); err != nil { + return err + } + + logrus.WithField("container", containerID).Warn(ErrExecNotSupported) + + // Exit with a dedicated code (see execNotSupportedExitCode) rather + // than returning the error to be handled by main's generic, + // exit-code-1 error path. + fatalWithCode(ErrExecNotSupported, execNotSupportedExitCode) + return nil // unreachable: fatalWithCode calls os.Exit + }, +} diff --git a/cmd/urunc/exec_test.go b/cmd/urunc/exec_test.go new file mode 100644 index 00000000..9d6adc53 --- /dev/null +++ b/cmd/urunc/exec_test.go @@ -0,0 +1,125 @@ +// 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" + "strings" + "testing" + + "github.com/stretchr/testify/assert" +) + +// execHelperEnvVar re-invokes the test binary as the real urunc CLI entry +// point. execCommand's Action calls os.Exit (matching runc's own convention +// for exec failures), so it can't be exercised in-process without killing +// the test run; running it as a subprocess is the standard way to test +// os.Exit paths. +const execHelperEnvVar = "URUNC_TEST_EXEC_HELPER" + +// TestExecCommandRegistered runs the real CLI entry point with "urunc exec" +// as a subprocess and checks that urfave/cli recognizes it as a known +// command rather than falling back to the "No help topic for 'exec'" +// unknown-command error that motivated this stub in the first place (see +// https://github.com/urunc-dev/urunc/issues/882 and the logs quoted in +// https://github.com/urunc-dev/urunc/issues/135). +func TestExecCommandRegistered(t *testing.T) { + assert.NotNil(t, execCommand) + assert.Equal(t, "exec", execCommand.Name) + + if os.Getenv(execHelperEnvVar) == "1" { + os.Args = []string{"urunc", "exec", "some-container-id"} + main() + return + } + + cmd := exec.Command(os.Args[0], "-test.run=TestExecCommandRegistered") //#nosec G204 -- os.Args[0] is this test binary re-invoking itself, not external input + cmd.Env = append(os.Environ(), execHelperEnvVar+"=1") + out, _ := cmd.CombinedOutput() + + assert.NotContains(t, strings.ToLower(string(out)), "no help topic") +} + +// TestExecCommandFlagsAcceptGoRuncShape asserts that execCommand declares +// the flags containerd-shim-runc-v2/go-runc actually pass when invoking +// exec (`exec --process [--console-socket ...] [--detach] +// [--pid-file ...] `), so parsing doesn't fail the way it did +// before this stub existed (see the "-info" parsing failure referenced in +// https://github.com/urunc-dev/urunc/issues/882). +func TestExecCommandFlagsAcceptGoRuncShape(t *testing.T) { + names := make(map[string]bool) + for _, f := range execCommand.Flags { + for _, n := range f.Names() { + names[n] = true + } + } + for _, want := range []string{"process", "console-socket", "detach", "pid-file"} { + assert.True(t, names[want], "expected exec command to define flag %q", want) + } +} + +// TestExecCommandFailsFast runs "urunc exec " in a subprocess +// and checks that it exits immediately with the dedicated "not supported" +// exit code and a clear error message, instead of hanging or returning the +// generic exit code 1 used for other urunc CLI errors. +func TestExecCommandFailsFast(t *testing.T) { + if os.Getenv(execHelperEnvVar) == "1" { + os.Args = []string{"urunc", "exec", "some-container-id"} + main() + return + } + + cmd := exec.Command(os.Args[0], "-test.run=TestExecCommandFailsFast") //#nosec G204 -- os.Args[0] is this test binary re-invoking itself, not external input + cmd.Env = append(os.Environ(), execHelperEnvVar+"=1") + out, runErr := cmd.CombinedOutput() + + var exitErr *exec.ExitError + if assert.ErrorAs(t, runErr, &exitErr, "expected urunc exec to exit with a non-zero status") { + assert.Equal(t, execNotSupportedExitCode, exitErr.ExitCode(), + "expected the dedicated exec-not-supported exit code, not the generic exit code 1") + } + assert.Contains(t, string(out), "exec is not supported by urunc") +} + +// TestExecCommandDoesNotPanicOnGoRuncArgs exercises the exact argument shape +// go-runc/containerd-shim-runc-v2 use when invoking exec, to confirm CLI +// parsing accepts it cleanly rather than crashing or reporting an unknown +// flag. +func TestExecCommandDoesNotPanicOnGoRuncArgs(t *testing.T) { + if os.Getenv(execHelperEnvVar) == "1" { + os.Args = []string{ + "urunc", "exec", + "--process", "/tmp/does-not-matter.json", + "--console-socket", "/tmp/does-not-matter.sock", + "--pid-file", "/tmp/does-not-matter.pid", + "some-container-id", + } + main() + return + } + + cmd := exec.Command(os.Args[0], "-test.run=TestExecCommandDoesNotPanicOnGoRuncArgs") //#nosec G204 -- os.Args[0] is this test binary re-invoking itself, not external input + cmd.Env = append(os.Environ(), execHelperEnvVar+"=1") + out, runErr := cmd.CombinedOutput() + + var exitErr *exec.ExitError + if assert.ErrorAs(t, runErr, &exitErr, "expected urunc exec to exit with a non-zero status") { + assert.Equal(t, execNotSupportedExitCode, exitErr.ExitCode()) + } + lower := strings.ToLower(string(out)) + assert.NotContains(t, lower, "flag provided but not defined") + assert.NotContains(t, lower, "panic") +} diff --git a/cmd/urunc/main.go b/cmd/urunc/main.go index 7a103aac..6fb166d8 100644 --- a/cmd/urunc/main.go +++ b/cmd/urunc/main.go @@ -111,6 +111,7 @@ func main() { Commands: []*cli.Command{ createCommand, deleteCommand, + execCommand, killCommand, runCommand, psCommand,