From 6ad8b22c7bcf1f30cc9ffb59ce0b97f062ce3d28 Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Fri, 4 Sep 2026 13:43:20 +0100 Subject: [PATCH 1/7] fix(sandbox): Merge the command's streams in the command, not the library Signed-off-by: Justin Chadwell --- internal/cmd/sandbox.go | 5 ++++- internal/sandbox/cmd_test.go | 27 +++++++++++++++++++++++---- internal/sandbox/logs.go | 9 +-------- 3 files changed, 28 insertions(+), 13 deletions(-) diff --git a/internal/cmd/sandbox.go b/internal/cmd/sandbox.go index f21f0b6f..b95c21da 100644 --- a/internal/cmd/sandbox.go +++ b/internal/cmd/sandbox.go @@ -86,11 +86,14 @@ func (c *ExecSandboxInstanceCmd) Run(ctx context.Context, stdio config.Stdio, pa in = strings.NewReader("") } + out := xio.Unwrap(stdio.Stdout) + cmd := target.CommandLine(ctx, c.Cmd) cmd.Dir = c.Dir cmd.Env = env cmd.Stdin = in - cmd.Stdout = xio.Unwrap(stdio.Stdout) + cmd.Stdout = out + cmd.Stderr = out return cmd.Run() } diff --git a/internal/sandbox/cmd_test.go b/internal/sandbox/cmd_test.go index b4d6dc60..3f048f11 100644 --- a/internal/sandbox/cmd_test.go +++ b/internal/sandbox/cmd_test.go @@ -204,9 +204,28 @@ func TestCmdRun(t *testing.T) { assert.Equal(t, map[string]string{"DEBUG": "true"}, *fake.run.Env) } -// TestCmdNilStderrJoinsStreams pins that a caller that set only Stdout gets -// both of the command's streams there, the way a terminal shows them. -func TestCmdNilStderrJoinsStreams(t *testing.T) { +// TestCmdStreamsStayApart pins that the two streams are never folded into +// one: a nil writer discards its stream rather than sending it to the other. +func TestCmdStreamsStayApart(t *testing.T) { + fake := newFakePlugin() + target := newTarget(t, fake) + + fake.write("out", "err") + fake.exit(0) + + var stdout, stderr bytes.Buffer + cmd := target.Command(t.Context(), "sh", "-c", "...") + cmd.Stdout = &stdout + cmd.Stderr = &stderr + + require.NoError(t, cmd.Run()) + assert.Equal(t, "out", stdout.String()) + assert.Equal(t, "err", stderr.String()) +} + +// TestCmdNilStderrDiscards pins that a caller that set only Stdout sees only +// the command's standard output, as os/exec does. +func TestCmdNilStderrDiscards(t *testing.T) { fake := newFakePlugin() target := newTarget(t, fake) @@ -218,7 +237,7 @@ func TestCmdNilStderrJoinsStreams(t *testing.T) { cmd.Stdout = &stdout require.NoError(t, cmd.Run()) - assert.Equal(t, "outerr", stdout.String()) + assert.Equal(t, "out", stdout.String()) } // TestCmdStdin pins that a reader is forwarded in full and that the command's diff --git a/internal/sandbox/logs.go b/internal/sandbox/logs.go index e33d2ab0..a404588e 100644 --- a/internal/sandbox/logs.go +++ b/internal/sandbox/logs.go @@ -91,7 +91,7 @@ func (l *logStream) drainOnce(ctx context.Context) (uint64, error) { out io.Writer }{ {resp.Data.Stdout, &l.stdoutOffset, l.stdout}, - {resp.Data.Stderr, &l.stderrOffset, cmpOr(l.stderr, l.stdout)}, + {resp.Data.Stderr, &l.stderrOffset, l.stderr}, } for _, stream := range outputs { if stream.data == "" { @@ -111,10 +111,3 @@ func (l *logStream) drainOnce(ctx context.Context) (uint64, error) { } return written, nil } - -func cmpOr(w, fallback io.Writer) io.Writer { - if w != nil { - return w - } - return fallback -} From a12e0954d3a4949a4b79f6b32dc79d2273c2a985 Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Fri, 4 Sep 2026 13:44:28 +0100 Subject: [PATCH 2/7] fix(sandbox): Exit with the status the remote command exited with Signed-off-by: Justin Chadwell --- cmd/unikraft/main.go | 13 ++++++++- internal/sandbox/cmd.go | 41 +++++++++++++++++++++++++++-- internal/sandbox/cmd_test.go | 51 ++++++++++++++++++++++++++++++++++-- 3 files changed, 100 insertions(+), 5 deletions(-) diff --git a/cmd/unikraft/main.go b/cmd/unikraft/main.go index bcd7ddb7..378ec884 100644 --- a/cmd/unikraft/main.go +++ b/cmd/unikraft/main.go @@ -23,6 +23,7 @@ import ( "unikraft.com/cli/internal/cmd" "unikraft.com/cli/internal/config" "unikraft.com/cli/internal/logfmt" + "unikraft.com/cli/internal/sandbox" "unikraft.com/cli/internal/telemetry" "unikraft.com/x/colors" "unikraft.com/x/log" @@ -53,6 +54,13 @@ func main() { err = ctx.Err() } + // a command that ran on an instance and failed isn't an error of ours, so + // exit with the status it exited with and print nothing over its output + exitCode := 0 + if exited, ok := errors.AsType[*sandbox.ExitError](err); ok { + exitCode, err = exited.ExitCode(), nil + } + // Track command completion for telemetry cmdPath, ok := telemetry.CommandFromContext(ctx) if ok && (err == nil || errors.Is(err, context.Canceled)) { @@ -86,7 +94,10 @@ func main() { } } if err != nil { - os.Exit(1) + exitCode = 1 + } + if exitCode != 0 { + os.Exit(exitCode) } } diff --git a/internal/sandbox/cmd.go b/internal/sandbox/cmd.go index b4b06cb3..0badd4cd 100644 --- a/internal/sandbox/cmd.go +++ b/internal/sandbox/cmd.go @@ -29,6 +29,17 @@ const WaitForever time.Duration = -1 const PluginName = plugin.PluginName +type ExitError struct { + UUID string + Code int +} + +func (e *ExitError) Error() string { + return fmt.Sprintf("command %s exited with status %d", e.UUID, e.Code) +} + +func (e *ExitError) ExitCode() int { return e.Code } + type Target struct { Client *plugin.Client Instance platform.Instance @@ -59,6 +70,7 @@ type Cmd struct { WaitDelay time.Duration Err error UUID string + ExitCode int ctx context.Context target Target @@ -168,7 +180,11 @@ func (c *Cmd) stream() { c.done <- fmt.Errorf("failed waiting for command: %w", err) return } - c.done <- c.logs.drain(c.waitCtx) + if err := c.logs.drain(c.waitCtx); err != nil { + c.done <- err + return + } + c.done <- c.exitStatus() return case <-timer.C: @@ -203,6 +219,26 @@ func (c *Cmd) stream() { } } +func (c *Cmd) exitStatus() error { + resp, err := c.target.Client.GetCommandByUuid(c.waitCtx, c.target.Instance, c.UUID, c.target.Opts...) + if err != nil { + return fmt.Errorf("failed to read the exit status of command %s: %w", c.UUID, err) + } + if resp.Data == nil { + return fmt.Errorf("failed to read the exit status of command %s: the %q plugin reported no state for it", c.UUID, c.target.Plugin) + } + + c.ExitCode = int(resp.Data.Exitcode) + log.G(c.ctx).Trace(). + Str("cmd", c.UUID). + Int("exit_code", c.ExitCode). + Msg("command ended") + if c.ExitCode != 0 { + return &ExitError{UUID: c.UUID, Code: c.ExitCode} + } + return nil +} + func (c *Cmd) Wait() error { if c.UUID == "" { return errors.New("sandbox: command not started") @@ -248,7 +284,8 @@ func (c *Cmd) Wait() error { case err := <-c.done: c.closed = true if err != nil { - if c.ctx.Err() != nil { + _, exited := errors.AsType[*ExitError](err) + if c.ctx.Err() != nil && !exited { return c.ctx.Err() } return err diff --git a/internal/sandbox/cmd_test.go b/internal/sandbox/cmd_test.go index 3f048f11..e5f1b167 100644 --- a/internal/sandbox/cmd_test.go +++ b/internal/sandbox/cmd_test.go @@ -100,6 +100,12 @@ func (f *mockPlugin) ServeHTTP(w http.ResponseWriter, r *http.Request) { } reply(nil) + case path == "/commands/cmd-1" && r.Method == http.MethodGet: + f.mu.Lock() + code := f.exitCode + f.mu.Unlock() + reply(plugin.GetCommandData{Uuid: "cmd-1", Exitcode: code}) + case path == "/commands/cmd-1/logs": var req plugin.CommandLogsRequest _ = json.NewDecoder(r.Body).Decode(&req) @@ -291,8 +297,47 @@ func TestCmdStdinChunked(t *testing.T) { assert.Equal(t, want, seen) } +// TestCmdExitStatus pins that a command that exits non-zero is reported as +// such, with the status it exited with and everything it wrote. +func TestCmdExitStatus(t *testing.T) { + fake := newFakePlugin() + target := newTarget(t, fake) + + fake.write("", "no such file\n") + fake.exit(2) + + var stdout, stderr bytes.Buffer + cmd := target.Command(t.Context(), "ls", "/nope") + cmd.Stdout = &stdout + cmd.Stderr = &stderr + + var exit *ExitError + err := cmd.Run() + require.ErrorAs(t, err, &exit) + assert.Equal(t, 2, exit.Code) + assert.Equal(t, 2, exit.ExitCode()) + assert.Equal(t, "cmd-1", exit.UUID) + assert.Equal(t, 2, cmd.ExitCode) + assert.Equal(t, "no such file\n", stderr.String()) +} + +// TestCmdExitStatusZero pins that a command that exits zero is no error, and +// that the status is readable all the same. +func TestCmdExitStatusZero(t *testing.T) { + fake := newFakePlugin() + target := newTarget(t, fake) + fake.exit(0) + + cmd := target.Command(t.Context(), "true") + cmd.Stdout = io.Discard + + require.NoError(t, cmd.Run()) + assert.Equal(t, 0, cmd.ExitCode) +} + // TestCmdCancelWithinWaitDelay pins that a command that dies of the interrupt -// within the delay is reported as having ended, with its output. +// within the delay is reported as having ended, with its output and the +// status it died with rather than the cancellation. func TestCmdCancelWithinWaitDelay(t *testing.T) { fake := newFakePlugin() target := newTarget(t, fake) @@ -313,7 +358,9 @@ func TestCmdCancelWithinWaitDelay(t *testing.T) { require.NoError(t, cmd.Start()) cancel() - require.NoError(t, cmd.Wait()) + var exit *ExitError + require.ErrorAs(t, cmd.Wait(), &exit) + assert.Equal(t, 130, exit.Code) assert.Equal(t, "interrupted\n", stdout.String()) } From 6d5a8555cc6cd89666da92d905532a58ac5dfc46 Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Fri, 4 Sep 2026 13:46:05 +0100 Subject: [PATCH 3/7] fix(sandbox): Wait for an interrupted command unless a WaitDelay is given Signed-off-by: Justin Chadwell --- internal/sandbox/cmd.go | 7 +------ internal/sandbox/cmd_test.go | 30 ++++++++++++++++++++++++++++-- 2 files changed, 29 insertions(+), 8 deletions(-) diff --git a/internal/sandbox/cmd.go b/internal/sandbox/cmd.go index 0badd4cd..5624f837 100644 --- a/internal/sandbox/cmd.go +++ b/internal/sandbox/cmd.go @@ -25,8 +25,6 @@ const ( signalTimeout = 5 * time.Second ) -const WaitForever time.Duration = -1 - const PluginName = plugin.PluginName type ExitError struct { @@ -265,10 +263,7 @@ func (c *Cmd) Wait() error { if err := c.cancel(); err != nil { return err } - switch { - case c.WaitDelay == 0: - return c.interrupted() - case c.WaitDelay > 0: + if c.WaitDelay > 0 { delay := time.NewTimer(c.WaitDelay) defer delay.Stop() expired = delay.C diff --git a/internal/sandbox/cmd_test.go b/internal/sandbox/cmd_test.go index e5f1b167..e9572288 100644 --- a/internal/sandbox/cmd_test.go +++ b/internal/sandbox/cmd_test.go @@ -364,8 +364,35 @@ func TestCmdCancelWithinWaitDelay(t *testing.T) { assert.Equal(t, "interrupted\n", stdout.String()) } +// TestCmdCancelWaitsByDefault pins that an unset WaitDelay waits for the +// interrupted command however long it takes, as the zero value of the field +// of the same name of os/exec.Cmd does. +func TestCmdCancelWaitsByDefault(t *testing.T) { + fake := newFakePlugin() + target := newTarget(t, fake) + + ctx, cancel := context.WithCancel(t.Context()) + var stdout bytes.Buffer + cmd := target.Command(ctx, "sleep", "300") + cmd.Stdout = &stdout + cmd.Cancel = func() error { + fake.write("interrupted\n", "") + fake.exit(130) + return nil + } + + require.NoError(t, cmd.Start()) + cancel() + + var exit *ExitError + require.ErrorAs(t, cmd.Wait(), &exit) + assert.Equal(t, 130, exit.Code) + assert.Equal(t, "interrupted\n", stdout.String()) +} + // TestCmdCancelWaitDelayExpires pins that a command that ignores the interrupt -// is given up on once the delay is out, and is still addressable by UUID. +// is given up on once a delay that was asked for is out, and is still +// addressable by UUID. func TestCmdCancelWaitDelayExpires(t *testing.T) { fake := newFakePlugin() target := newTarget(t, fake) @@ -392,7 +419,6 @@ func TestCmdCancelError(t *testing.T) { ctx, cancel := context.WithCancel(t.Context()) cmd := target.Command(ctx, "sleep", "300") cmd.Stdout = io.Discard - cmd.WaitDelay = WaitForever sentinel := errors.New("could not interrupt") cmd.Cancel = func() error { return sentinel } From 6eaed22b0a06e13fb19f918c9ba88863eefdebe8 Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Fri, 4 Sep 2026 13:46:52 +0100 Subject: [PATCH 4/7] fix(sandbox): Close the command's standard input when feeding it fails Signed-off-by: Justin Chadwell --- internal/sandbox/cmd.go | 2 +- internal/sandbox/cmd_test.go | 38 +++++++++++++++++++++++++++++++++++- internal/sandbox/stdin.go | 14 ++++++++++++- 3 files changed, 51 insertions(+), 3 deletions(-) diff --git a/internal/sandbox/cmd.go b/internal/sandbox/cmd.go index 5624f837..cdf33be5 100644 --- a/internal/sandbox/cmd.go +++ b/internal/sandbox/cmd.go @@ -273,7 +273,7 @@ func (c *Cmd) Wait() error { return c.interrupted() case stdinErr := <-c.stdinErr: - log.G(c.ctx).Debug().Err(stdinErr).Str("cmd", c.UUID).Msg("standard input failed") + log.G(c.ctx).Warn().Err(stdinErr).Str("cmd", c.UUID).Msg("standard input failed") continue case err := <-c.done: diff --git a/internal/sandbox/cmd_test.go b/internal/sandbox/cmd_test.go index e9572288..d2e7d1ee 100644 --- a/internal/sandbox/cmd_test.go +++ b/internal/sandbox/cmd_test.go @@ -42,6 +42,10 @@ type mockPlugin struct { stdin []byte stdinEOF bool + // failStdinData is set before the plugin serves anything, so it needs no + // lock of its own. + failStdinData bool + signals []int } @@ -127,9 +131,15 @@ func (f *mockPlugin) ServeHTTP(w http.ResponseWriter, r *http.Request) { _ = json.NewDecoder(r.Body).Decode(&req) decoded, _ := base64.StdEncoding.DecodeString(req.Data) + eof := req.Eof != nil && *req.Eof + if f.failStdinData && !eof { + w.WriteHeader(http.StatusInternalServerError) + return + } + f.mu.Lock() f.stdin = append(f.stdin, decoded...) - if req.Eof != nil && *req.Eof { + if eof { f.stdinEOF = true } f.mu.Unlock() @@ -273,6 +283,32 @@ func TestCmdStdin(t *testing.T) { assert.True(t, eof) } +// TestCmdStdinFailureClosesInput pins that standard input that fails part way +// through still closes the command's input, so that a command reading to EOF +// is not left waiting on input that will never arrive. +func TestCmdStdinFailureClosesInput(t *testing.T) { + fake := newFakePlugin() + fake.failStdinData = true + target := newTarget(t, fake) + + cmd := target.Command(t.Context(), "cat") + cmd.Stdin = strings.NewReader("never arrives\n") + cmd.Stdout = io.Discard + + require.NoError(t, cmd.Start()) + require.Eventually(t, func() bool { + _, eof := fake.stdinSeen() + return eof + }, 5*time.Second, 10*time.Millisecond) + fake.exit(0) + + require.NoError(t, cmd.Wait()) + + seen, eof := fake.stdinSeen() + assert.Empty(t, seen) + assert.True(t, eof) +} + // TestCmdStdinChunked pins that a reader larger than one chunk still arrives // whole. func TestCmdStdinChunked(t *testing.T) { diff --git a/internal/sandbox/stdin.go b/internal/sandbox/stdin.go index f56b1f2b..34a6fa31 100644 --- a/internal/sandbox/stdin.go +++ b/internal/sandbox/stdin.go @@ -11,13 +11,17 @@ import ( "errors" "fmt" "io" + "time" "github.com/docker/go-units" plugin "unikraft.com/cloud/plugins/sandbox" ) -const stdinChunkSize = 32 * units.KiB +const ( + stdinChunkSize = 32 * units.KiB + stdinEOFTimeout = 5 * time.Second +) // stdinPump forwards a reader to a command's standard input, a chunk per // request, and closes that input once the reader is done. @@ -48,6 +52,7 @@ func (p *stdinPump) feed(ctx context.Context, in io.Reader, failed chan<- error) if n > 0 { if err := p.write(ctx, base64.StdEncoding.EncodeToString(buf[:n]), false); err != nil { report(fmt.Errorf("failed to send standard input to the command after %d bytes: %w", sent, err)) + p.close(ctx) return } sent += uint64(n) @@ -56,6 +61,7 @@ func (p *stdinPump) feed(ctx context.Context, in io.Reader, failed chan<- error) if readErr != nil { if !errors.Is(readErr, io.EOF) { report(fmt.Errorf("failed to read standard input after %d bytes: %w", sent, readErr)) + p.close(ctx) return } if ctx.Err() != nil { @@ -69,6 +75,12 @@ func (p *stdinPump) feed(ctx context.Context, in io.Reader, failed chan<- error) } } +func (p *stdinPump) close(ctx context.Context) { + ctx, cancel := context.WithTimeout(context.WithoutCancel(ctx), stdinEOFTimeout) + defer cancel() + _ = p.write(ctx, "", true) +} + func (p *stdinPump) write(ctx context.Context, data string, eof bool) error { req := plugin.CommandStdinRequest{Data: data, Eof: &eof} _, err := p.target.Client.WriteCommandStdin(ctx, p.target.Instance, p.uuid, &req, p.target.Opts...) From d27e8e721ff59829d532d1a79ace1396d7fdbc08 Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Fri, 4 Sep 2026 13:47:52 +0100 Subject: [PATCH 5/7] feat(sandbox): Take a command as an argument list or as a shell line Signed-off-by: Justin Chadwell --- internal/cmd/sandbox.go | 2 +- internal/sandbox/cmd.go | 43 ++++++++++++++++++++-------- internal/sandbox/cmd_test.go | 55 +++++++++++++++++++++++++++++++++++- 3 files changed, 87 insertions(+), 13 deletions(-) diff --git a/internal/cmd/sandbox.go b/internal/cmd/sandbox.go index b95c21da..cf50acd4 100644 --- a/internal/cmd/sandbox.go +++ b/internal/cmd/sandbox.go @@ -88,7 +88,7 @@ func (c *ExecSandboxInstanceCmd) Run(ctx context.Context, stdio config.Stdio, pa out := xio.Unwrap(stdio.Stdout) - cmd := target.CommandLine(ctx, c.Cmd) + cmd := target.CommandArgs(ctx, c.Cmd) cmd.Dir = c.Dir cmd.Env = env cmd.Stdin = in diff --git a/internal/sandbox/cmd.go b/internal/sandbox/cmd.go index cdf33be5..683413dd 100644 --- a/internal/sandbox/cmd.go +++ b/internal/sandbox/cmd.go @@ -46,10 +46,10 @@ type Target struct { } func (t Target) Command(ctx context.Context, name string, args ...string) *Cmd { - return t.CommandLine(ctx, append([]string{name}, args...)) + return t.CommandArgs(ctx, append([]string{name}, args...)) } -func (t Target) CommandLine(ctx context.Context, args []string) *Cmd { +func (t Target) CommandArgs(ctx context.Context, args []string) *Cmd { c := &Cmd{Args: args, ctx: ctx, target: t} if len(args) == 0 { c.Err = errors.New("sandbox: no command given") @@ -57,10 +57,20 @@ func (t Target) CommandLine(ctx context.Context, args []string) *Cmd { return c } +func (t Target) CommandLine(ctx context.Context, cmdline string) *Cmd { + c := &Cmd{Cmdline: cmdline, ctx: ctx, target: t} + if cmdline == "" { + c.Err = errors.New("sandbox: no command given") + } + return c +} + type Cmd struct { - Args []string - Dir string - Env map[string]string + Args []string + Cmdline string + + Dir string + Env map[string]string Stdin io.Reader Stdout, Stderr io.Writer @@ -73,10 +83,6 @@ type Cmd struct { ctx context.Context target Target - // HACK: this will be removed after the plugin api will be able to - // take parsed args instead of a single string - cmdline string - waitCtx context.Context stopWait context.CancelFunc @@ -90,6 +96,21 @@ type Cmd struct { waited bool } +func (c *Cmd) commandLine() (string, error) { + if len(c.Args) == 0 { + if c.Cmdline == "" { + return "", errors.New("sandbox: no command given") + } + return c.Cmdline, nil + } + if c.Cmdline != "" { + return "", errors.New("sandbox: only one of Args and Cmdline may be given") + } + // HACK: this will be removed after the plugin api will be able to + // take parsed args instead of a single string + return Quote(c.Args) +} + func (c *Cmd) Start() error { if c.Err != nil { return c.Err @@ -100,11 +121,11 @@ func (c *Cmd) Start() error { log.G(c.ctx).Trace().Msg("executing command") - cmdline, err := Quote(c.Args) + cmdline, err := c.commandLine() if err != nil { return err } - c.cmdline = cmdline + c.Cmdline = cmdline req := plugin.RunCommandRequest{Cmd: cmdline} if c.Dir != "" { diff --git a/internal/sandbox/cmd_test.go b/internal/sandbox/cmd_test.go index d2e7d1ee..636606e0 100644 --- a/internal/sandbox/cmd_test.go +++ b/internal/sandbox/cmd_test.go @@ -220,6 +220,53 @@ func TestCmdRun(t *testing.T) { assert.Equal(t, map[string]string{"DEBUG": "true"}, *fake.run.Env) } +// TestCmdRunCommandLine pins that a whole shell line reaches the plugin as it +// was written, with nothing quoted on its behalf. +func TestCmdRunCommandLine(t *testing.T) { + fake := newFakePlugin() + target := newTarget(t, fake) + + fake.write("a b\n", "") + fake.exit(0) + + var stdout bytes.Buffer + cmd := target.CommandLine(t.Context(), "echo a b > /dev/stderr; echo a b") + cmd.Stdout = &stdout + + require.NoError(t, cmd.Run()) + assert.Equal(t, "a b\n", stdout.String()) + + fake.mu.Lock() + defer fake.mu.Unlock() + assert.Equal(t, "echo a b > /dev/stderr; echo a b", fake.run.Cmd) +} + +// TestCmdCommandForms pins how the two forms of a command resolve into the +// one line the plugin takes today. +func TestCmdCommandForms(t *testing.T) { + for _, tt := range []struct { + name string + cmd Cmd + want string + wantErr string + }{ + {name: "args", cmd: Cmd{Args: []string{"echo", "a b"}}, want: `'echo' 'a b'`}, + {name: "cmdline", cmd: Cmd{Cmdline: "echo a b"}, want: "echo a b"}, + {name: "neither", cmd: Cmd{}, wantErr: "no command given"}, + {name: "both", cmd: Cmd{Args: []string{"echo"}, Cmdline: "echo"}, wantErr: "only one of Args and Cmdline"}, + } { + t.Run(tt.name, func(t *testing.T) { + got, err := tt.cmd.commandLine() + if tt.wantErr != "" { + assert.ErrorContains(t, err, tt.wantErr) + return + } + require.NoError(t, err) + assert.Equal(t, tt.want, got) + }) + } +} + // TestCmdStreamsStayApart pins that the two streams are never folded into // one: a nil writer discards its stream rather than sending it to the other. func TestCmdStreamsStayApart(t *testing.T) { @@ -505,7 +552,13 @@ func TestCmdMisuse(t *testing.T) { fake.exit(0) t.Run("no-command", func(t *testing.T) { - cmd := target.CommandLine(t.Context(), nil) + cmd := target.CommandArgs(t.Context(), nil) + require.Error(t, cmd.Err) + assert.ErrorContains(t, cmd.Run(), "no command given") + }) + + t.Run("no-command-line", func(t *testing.T) { + cmd := target.CommandLine(t.Context(), "") require.Error(t, cmd.Err) assert.ErrorContains(t, cmd.Run(), "no command given") }) From 0a7b03b6e488b20463ac552cf572b30244b0e0d8 Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Fri, 4 Sep 2026 13:48:33 +0100 Subject: [PATCH 6/7] refactor(resource): Name the Partition receivers p, not s Signed-off-by: Justin Chadwell --- internal/resource/partition.go | 108 ++++++++++++++++----------------- 1 file changed, 54 insertions(+), 54 deletions(-) diff --git a/internal/resource/partition.go b/internal/resource/partition.go index 5ff257f6..6e6b296f 100644 --- a/internal/resource/partition.go +++ b/internal/resource/partition.go @@ -46,17 +46,17 @@ func LoadPartitionFromEnv(resources ...Resource) (*Partition, error) { } func LoadPartition(path string, resources ...Resource) (*Partition, error) { - s := Partition{Path: path, Cleanup: resources} - s.Keys = make(map[string]map[string]struct{}) + p := Partition{Path: path, Cleanup: resources} + p.Keys = make(map[string]map[string]struct{}) for _, r := range resources { - if _, ok := s.Keys[r.Type().Name]; !ok { - s.Keys[r.Type().Name] = make(map[string]struct{}) + if _, ok := p.Keys[r.Type().Name]; !ok { + p.Keys[r.Type().Name] = make(map[string]struct{}) } } f, err := os.Open(path) if errors.Is(err, os.ErrNotExist) { - return &s, nil + return &p, nil } else if err != nil { return nil, fmt.Errorf("failed to open partition file: %w", err) } @@ -68,29 +68,29 @@ func LoadPartition(path string, resources ...Resource) (*Partition, error) { return nil, fmt.Errorf("failed to decode partition file: %w", err) } for rtype, rkeys := range keys { - if _, ok := s.Keys[rtype]; !ok { + if _, ok := p.Keys[rtype]; !ok { continue } for _, rkey := range rkeys { - s.Keys[rtype][rkey] = struct{}{} + p.Keys[rtype][rkey] = struct{}{} } } - return &s, nil + return &p, nil } -func (s *Partition) Save() error { - if s == nil { +func (p *Partition) Save() error { + if p == nil { return nil } - f, err := os.Create(s.Path) + f, err := os.Create(p.Path) if err != nil { return fmt.Errorf("failed to create partition file: %w", err) } defer f.Close() - keys := make(map[string][]string, len(s.Keys)) - for rtype, rkeys := range s.Keys { + keys := make(map[string][]string, len(p.Keys)) + for rtype, rkeys := range p.Keys { keys[rtype] = xmaps.OrderedKeys(rkeys) } @@ -106,14 +106,14 @@ func (s *Partition) Save() error { // Teardown attempts to delete all resources tracked by the partition. Some // resources may not be deletable, in which case they are skipped. -func (s *Partition) Teardown(ctx context.Context) (rerr error) { - if s == nil { +func (p *Partition) Teardown(ctx context.Context) (rerr error) { + if p == nil { return nil } log.G(ctx).Debug(). - Str("path", s.Path). + Str("path", p.Path). Msg("tearing down partition") - for _, r := range s.Cleanup { + for _, r := range p.Cleanup { name := r.Type().Name r, ok := r.(DeletableResource) if !ok { @@ -123,7 +123,7 @@ func (s *Partition) Teardown(ctx context.Context) (rerr error) { continue } - targets := xmaps.OrderedKeys(s.Keys[name]) + targets := xmaps.OrderedKeys(p.Keys[name]) log.G(ctx).Debug(). Str("resource", name). Strs("targets", targets). @@ -144,22 +144,22 @@ func (s *Partition) Teardown(ctx context.Context) (rerr error) { return rerr } -func (s *Partition) Add(ctx context.Context, r Resource) error { - if s == nil { +func (p *Partition) Add(ctx context.Context, r Resource) error { + if p == nil { return nil } - if _, ok := s.Keys[r.Type().Name]; !ok { + if _, ok := p.Keys[r.Type().Name]; !ok { return nil } visited := make(map[string]struct{}) - return s.add(ctx, r, visited) + return p.add(ctx, r, visited) } -func (s *Partition) add(ctx context.Context, r Resource, visited map[string]struct{}) error { - if s == nil { +func (p *Partition) add(ctx context.Context, r Resource, visited map[string]struct{}) error { + if p == nil { return nil } - if _, ok := s.Keys[r.Type().Name]; !ok { + if _, ok := p.Keys[r.Type().Name]; !ok { return nil } typeName := r.Type().Name @@ -169,7 +169,7 @@ func (s *Partition) add(ctx context.Context, r Resource, visited map[string]stru return nil } visited[visitKey] = struct{}{} - s.Keys[typeName][key] = struct{}{} + p.Keys[typeName][key] = struct{}{} fields, err := r.Fields(ctx) if err != nil { @@ -191,11 +191,11 @@ func (s *Partition) add(ctx context.Context, r Resource, visited map[string]stru if key == "" { continue } - for _, r := range s.Cleanup { + for _, r := range p.Cleanup { if r.Type().Name != linkType { continue } - if keys, ok := s.Keys[linkType]; ok { + if keys, ok := p.Keys[linkType]; ok { keys[key] = struct{}{} } @@ -208,7 +208,7 @@ func (s *Partition) add(ctx context.Context, r Resource, visited map[string]stru return fmt.Errorf("failed to get linked resource %s %s: %w", linkType, key, err) } for _, linkedResource := range linkedResources { - if err := s.add(ctx, linkedResource, visited); err != nil { + if err := p.add(ctx, linkedResource, visited); err != nil { return err } } @@ -220,78 +220,78 @@ func (s *Partition) add(ctx context.Context, r Resource, visited map[string]stru return nil } -func (s *Partition) Remove(typeName string, key string) { - if s == nil { +func (p *Partition) Remove(typeName string, key string) { + if p == nil { return } - if _, ok := s.Keys[typeName]; !ok { + if _, ok := p.Keys[typeName]; !ok { return } - delete(s.Keys[typeName], key) + delete(p.Keys[typeName], key) } -func (s *Partition) Has(r Resource) bool { - if s == nil { +func (p *Partition) Has(r Resource) bool { + if p == nil { return true } - if _, ok := s.Keys[r.Type().Name]; !ok { + if _, ok := p.Keys[r.Type().Name]; !ok { return true } - _, ok := s.Keys[r.Type().Name][r.Key().Canonical()] + _, ok := p.Keys[r.Type().Name][r.Key().Canonical()] return ok } -func (s *Partition) Missing(r Resource) bool { - return !s.Has(r) +func (p *Partition) Missing(r Resource) bool { + return !p.Has(r) } -func (s *Partition) WrapGettable(r GettableResource) GettableResource { - if s == nil { +func (p *Partition) WrapGettable(r GettableResource) GettableResource { + if p == nil { return r } return partitionedGettableResource{ GettableResource: r, - partition: s, + partition: p, } } -func (s *Partition) WrapListable(r ListableResource) ListableResource { - if s == nil { +func (p *Partition) WrapListable(r ListableResource) ListableResource { + if p == nil { return r } return partitionedListableResource{ ListableResource: r, - partition: s, + partition: p, } } -func (s *Partition) WrapEditable(r EditableResource) EditableResource { - if s == nil { +func (p *Partition) WrapEditable(r EditableResource) EditableResource { + if p == nil { return r } return partitionedEditableResource{ EditableResource: r, - partition: s, + partition: p, } } -func (s *Partition) WrapCreatable(r CreatableResource) CreatableResource { - if s == nil { +func (p *Partition) WrapCreatable(r CreatableResource) CreatableResource { + if p == nil { return r } return partitionedCreatableResource{ CreatableResource: r, - partition: s, + partition: p, } } -func (s *Partition) WrapDeletable(r DeletableResource) DeletableResource { - if s == nil { +func (p *Partition) WrapDeletable(r DeletableResource) DeletableResource { + if p == nil { return r } return partitionedDeletableResource{ DeletableResource: r, - partition: s, + partition: p, } } From 6153c5a1fd50325b2df8448c54d9aa1bd7c631da Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Fri, 4 Sep 2026 13:49:19 +0100 Subject: [PATCH 7/7] test(integration): Reuse the shared busybox image for the sandbox tests Signed-off-by: Justin Chadwell --- cmd/unikraft/integration/sandbox_test.go | 29 ++++-------------------- 1 file changed, 5 insertions(+), 24 deletions(-) diff --git a/cmd/unikraft/integration/sandbox_test.go b/cmd/unikraft/integration/sandbox_test.go index d8c3805f..a61b5dc1 100644 --- a/cmd/unikraft/integration/sandbox_test.go +++ b/cmd/unikraft/integration/sandbox_test.go @@ -19,34 +19,14 @@ import ( integ "unikraft.com/cli/internal/integration" ) -const ( - sandboxPlugin = sandbox.PluginName - - sandboxKraftfile = ` -spec: v0.7 -name: sandbox-e2e -runtime: base-compat:latest -rootfs: - format: erofs - source: ./Dockerfile - type: dockerfile -cmd: ["tail", "-f", "/dev/null"] -` -) +const sandboxPlugin = sandbox.PluginName -// newSandboxInstance builds the fixture image, creates a running instance -// serving the sandbox plugin on it, and returns the instance's name. +// newSandboxInstance creates a running instance serving the sandbox plugin +// on the shared busybox image, and returns the instance's name. func newSandboxInstance(t *testing.T, r *integ.TestEnv) string { t.Helper() - dir := t.TempDir() - require.NoError(t, fstest.Apply( - fstest.CreateFile("Dockerfile", []byte("FROM busybox:latest\n"), 0o644), - fstest.CreateFile("Kraftfile", []byte(sandboxKraftfile), 0o644), - ).Apply(dir)) - - image := r.Config.Profile.Organization + "/sandbox-e2e:" + uniq() - r.Run(t, []string{"unikraft", "build", ".", "--output", image}, integ.WithWorkDir(dir)) + image := integ.Busybox.Build(t, r) name := "test-" + uniq() r.Run(t, []string{ @@ -55,6 +35,7 @@ func newSandboxInstance(t *testing.T, r *integ.TestEnv) string { "--name", name, "--metro", r.Config.MetroName, "--image", image, + "--args", "tail -f /dev/null", "--plugin", "name=" + sandboxPlugin + ",rom=" + sandboxPluginRom, "--memory", "512", "--vcpus", "1",