Skip to content

fix: Follow-ups to the sandbox integration #491

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jedevc merged 7 commits into from
Sep 4, 2026
Merged

fix: Follow-ups to the sandbox integration #491

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
6ad8b22
fix(sandbox): Merge the command's streams in the command, not the lib…
jedevc Sep 4, 2026
a12e095
fix(sandbox): Exit with the status the remote command exited with
jedevc Sep 4, 2026
6d5a855
fix(sandbox): Wait for an interrupted command unless a WaitDelay is g…
jedevc Sep 4, 2026
6eaed22
fix(sandbox): Close the command's standard input when feeding it fails
jedevc Sep 4, 2026
d27e8e7
feat(sandbox): Take a command as an argument list or as a shell line
jedevc Sep 4, 2026
0a7b03b
refactor(resource): Name the Partition receivers p, not s
jedevc Sep 4, 2026
6153c5a
test(integration): Reuse the shared busybox image for the sandbox tests
jedevc Sep 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 5 additions & 24 deletions cmd/unikraft/integration/sandbox_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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",
Expand Down
13 changes: 12 additions & 1 deletion cmd/unikraft/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}

Comment on lines +57 to +63

@dragosgheorghioiu dragosgheorghioiu Sep 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this does make more sense than what i had before
i had the impression it should just swallow the command completely including the exit code

// Track command completion for telemetry
cmdPath, ok := telemetry.CommandFromContext(ctx)
if ok && (err == nil || errors.Is(err, context.Canceled)) {
Expand Down Expand Up @@ -86,7 +94,10 @@ func main() {
}
}
if err != nil {
os.Exit(1)
exitCode = 1
}
if exitCode != 0 {
os.Exit(exitCode)
}
}

Expand Down
7 changes: 5 additions & 2 deletions internal/cmd/sandbox.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,11 +86,14 @@ func (c *ExecSandboxInstanceCmd) Run(ctx context.Context, stdio config.Stdio, pa
in = strings.NewReader("")
}

cmd := target.CommandLine(ctx, c.Cmd)
out := xio.Unwrap(stdio.Stdout)

cmd := target.CommandArgs(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()
}
Expand Down
108 changes: 54 additions & 54 deletions internal/resource/partition.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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)
}

Expand All @@ -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 {
Expand All @@ -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).
Expand All @@ -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
Expand All @@ -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 {
Expand All @@ -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{}{}
}

Expand All @@ -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
}
}
Expand All @@ -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,
}
}

Expand Down
Loading
Loading