diff --git a/internal/api/handlers.go b/internal/api/handlers.go index 29440aa..8fed759 100644 --- a/internal/api/handlers.go +++ b/internal/api/handlers.go @@ -46,6 +46,7 @@ func (s *Server) handleCreate(w http.ResponseWriter, r *http.Request) { Image: req.Image, CPU: req.Resources.CPU, MemoryMB: req.Resources.MemoryMB, + DiskGB: req.Resources.DiskGB, AutoStopSecs: req.AutoStopSecs, Env: req.Env, Labels: req.Labels, diff --git a/internal/core/service.go b/internal/core/service.go index 4df5948..cab2aa3 100644 --- a/internal/core/service.go +++ b/internal/core/service.go @@ -55,11 +55,16 @@ func (s *Service) Reconcile(ctx context.Context) (int, error) { return s.reg.Rec // Lifecycle // --------------------------------------------------------------------------- +// maxDiskGB bounds CreateParams.DiskGB so the GiB->MiB conversion stays within +// the uint32 the SDK's WithOCIUpperSize option accepts (math.MaxUint32 / 1024). +const maxDiskGB = 4194303 + // CreateParams is the provider-neutral create input. type CreateParams struct { Image string CPU float64 MemoryMB int + DiskGB int // writable overlay upper size, in GiB (OCI images) AutoStopSecs int Env map[string]string Labels map[string]string @@ -124,6 +129,63 @@ func (s *Service) Create(ctx context.Context, p CreateParams) (*Instance, error) name := newName() workdir := strings.TrimSpace(p.Workdir) + opts, err := buildCreateOptions(p, image) + if err != nil { + return nil, err + } + + cctx, cancel := context.WithTimeout(ctx, s.createTO) + defer cancel() + + sb, err := msb.CreateSandbox(cctx, name, opts...) + if err != nil { + return nil, fmt.Errorf("create sandbox: %w", err) + } + s.reg.cache(name, sb) + + // Resolve the box's REAL working directory. + // + // 1. Caller pinned a workdir: ensure it exists in the guest (mkdir -p), + // then use it — the dir gets created if the image didn't ship it, + // rather than the SDK refusing to boot. + // 2. No workdir pinned: trust the image's own WORKDIR by asking the guest + // with `pwd`. + // + // Best-effort: fall back to defaultWorkdir on any error. + resolved := workdir + if resolved != "" { + quoted := shellQuote(resolved) + if _, perr := runShell(cctx, sb, ExecParams{Cmd: "mkdir -p " + quoted}); perr != nil { + // Don't fail Create on mkdir error — fall back to the image's WORKDIR. + resolved = "" + } + } + if resolved == "" { + if out, perr := runShell(cctx, sb, ExecParams{Cmd: "pwd"}); perr == nil { + if wd := strings.TrimSpace(out.Stdout); strings.HasPrefix(wd, "/") { + resolved = wd + } + } + } + resolved = defaultWorkdir(resolved) + s.reg.setWorkdir(name, resolved) + + return &Instance{ + ID: name, + Image: image, + State: StateRunning, + Workdir: resolved, + Labels: p.Labels, + }, nil +} + +// buildCreateOptions translates the provider-neutral CreateParams into the +// microsandbox SDK option slice. It is deterministic and free of side effects +// (no SDK calls beyond constructing options), which keeps the CreateParams -> +// SandboxOption mapping unit-testable without booting a microVM. It returns an +// error for inputs that can't be represented safely (e.g. an out-of-range +// disk size) rather than silently dropping or wrapping them. +func buildCreateOptions(p CreateParams, image string) ([]msb.SandboxOption, error) { opts := []msb.SandboxOption{ msb.WithImage(image), msb.WithDetached(), // survive msbd restart @@ -134,6 +196,17 @@ func (s *Service) Create(ctx context.Context, p CreateParams) (*Instance, error) if p.CPU > 0 { opts = append(opts, msb.WithCPUs(uint8(p.CPU))) } + if p.DiskGB != 0 { + // API field is GiB; SDK option takes MiB (uint32). Reject negatives and + // values that would overflow the MiB conversion instead of wrapping. + if p.DiskGB < 0 { + return nil, fmt.Errorf("invalid disk_gb: %d (must be non-negative)", p.DiskGB) + } + if p.DiskGB > maxDiskGB { + return nil, fmt.Errorf("invalid disk_gb: %d (exceeds maximum of %d GiB)", p.DiskGB, maxDiskGB) + } + opts = append(opts, msb.WithOCIUpperSize(uint32(p.DiskGB)*1024)) + } if len(p.Env) > 0 { opts = append(opts, msb.WithEnv(p.Env)) } @@ -188,50 +261,7 @@ func (s *Service) Create(ctx context.Context, p CreateParams) (*Instance, error) if p.AutoStopSecs > 0 { opts = append(opts, msb.WithIdleTimeout(time.Duration(p.AutoStopSecs)*time.Second)) } - - cctx, cancel := context.WithTimeout(ctx, s.createTO) - defer cancel() - - sb, err := msb.CreateSandbox(cctx, name, opts...) - if err != nil { - return nil, fmt.Errorf("create sandbox: %w", err) - } - s.reg.cache(name, sb) - - // Resolve the box's REAL working directory. - // - // 1. Caller pinned a workdir: ensure it exists in the guest (mkdir -p), - // then use it — the dir gets created if the image didn't ship it, - // rather than the SDK refusing to boot. - // 2. No workdir pinned: trust the image's own WORKDIR by asking the guest - // with `pwd`. - // - // Best-effort: fall back to defaultWorkdir on any error. - resolved := workdir - if resolved != "" { - quoted := shellQuote(resolved) - if _, perr := runShell(cctx, sb, ExecParams{Cmd: "mkdir -p " + quoted}); perr != nil { - // Don't fail Create on mkdir error — fall back to the image's WORKDIR. - resolved = "" - } - } - if resolved == "" { - if out, perr := runShell(cctx, sb, ExecParams{Cmd: "pwd"}); perr == nil { - if wd := strings.TrimSpace(out.Stdout); strings.HasPrefix(wd, "/") { - resolved = wd - } - } - } - resolved = defaultWorkdir(resolved) - s.reg.setWorkdir(name, resolved) - - return &Instance{ - ID: name, - Image: image, - State: StateRunning, - Workdir: resolved, - Labels: p.Labels, - }, nil + return opts, nil } func (s *Service) Get(ctx context.Context, id string) (*Instance, error) { diff --git a/internal/core/service_test.go b/internal/core/service_test.go new file mode 100644 index 0000000..2d5ba50 --- /dev/null +++ b/internal/core/service_test.go @@ -0,0 +1,98 @@ +package core + +import ( + "testing" + + msb "github.com/superradcompany/microsandbox/sdk/go" +) + +// applyOptions folds a slice of SandboxOption into a fresh SandboxConfig so +// tests can assert what buildCreateOptions actually wires through to the SDK. +func applyOptions(opts []msb.SandboxOption) msb.SandboxConfig { + var cfg msb.SandboxConfig + for _, o := range opts { + o(&cfg) + } + return cfg +} + +// TestBuildCreateOptionsDiskGB is the regression test for issue #1: +// resources.disk_gb must be honored and converted from GiB to MiB before being +// handed to the SDK's WithOCIUpperSize option. +func TestBuildCreateOptionsDiskGB(t *testing.T) { + opts, err := buildCreateOptions(CreateParams{DiskGB: 32}, "microsandbox/python") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + cfg := applyOptions(opts) + if got, want := cfg.OCIUpperSizeMiB, uint32(32*1024); got != want { + t.Fatalf("OCIUpperSizeMiB = %d, want %d (32 GiB -> MiB)", got, want) + } +} + +// TestBuildCreateOptionsNoDiskGB confirms that omitting disk_gb leaves the +// overlay size at the SDK/image default (zero, i.e. unset) rather than forcing +// a value. +func TestBuildCreateOptionsNoDiskGB(t *testing.T) { + opts, err := buildCreateOptions(CreateParams{}, "microsandbox/python") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + cfg := applyOptions(opts) + if cfg.OCIUpperSizeMiB != 0 { + t.Fatalf("OCIUpperSizeMiB = %d, want 0 when disk_gb is unset", cfg.OCIUpperSizeMiB) + } +} + +// TestBuildCreateOptionsDiskGBInvalid rejects negative and out-of-range disk_gb +// instead of silently skipping or wrapping the uint32 MiB conversion. +func TestBuildCreateOptionsDiskGBInvalid(t *testing.T) { + for _, tc := range []struct { + name string + diskGB int + }{ + {"negative", -1}, + {"overflow", maxDiskGB + 1}, + } { + t.Run(tc.name, func(t *testing.T) { + if _, err := buildCreateOptions(CreateParams{DiskGB: tc.diskGB}, "img"); err == nil { + t.Fatalf("expected error for disk_gb=%d, got nil", tc.diskGB) + } + }) + } +} + +// TestBuildCreateOptionsDiskGBMax accepts the largest in-range disk_gb and +// converts it without overflowing uint32. +func TestBuildCreateOptionsDiskGBMax(t *testing.T) { + opts, err := buildCreateOptions(CreateParams{DiskGB: maxDiskGB}, "img") + if err != nil { + t.Fatalf("unexpected error at max disk_gb: %v", err) + } + cfg := applyOptions(opts) + if got, want := cfg.OCIUpperSizeMiB, uint32(maxDiskGB)*1024; got != want { + t.Fatalf("OCIUpperSizeMiB = %d, want %d", got, want) + } +} + +// TestBuildCreateOptionsResources covers the CPU/memory mapping alongside disk +// so the conversion test doesn't silently regress the neighbouring knobs. +func TestBuildCreateOptionsResources(t *testing.T) { + opts, err := buildCreateOptions(CreateParams{CPU: 2, MemoryMB: 1024, DiskGB: 8}, "img") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + cfg := applyOptions(opts) + if cfg.CPUs != 2 { + t.Errorf("CPUs = %d, want 2", cfg.CPUs) + } + if cfg.MemoryMiB != 1024 { + t.Errorf("MemoryMiB = %d, want 1024", cfg.MemoryMiB) + } + if cfg.OCIUpperSizeMiB != 8*1024 { + t.Errorf("OCIUpperSizeMiB = %d, want %d", cfg.OCIUpperSizeMiB, 8*1024) + } + if cfg.Image != "img" { + t.Errorf("Image = %q, want %q", cfg.Image, "img") + } +} diff --git a/openapi.yaml b/openapi.yaml index 957d2f5..f969697 100644 --- a/openapi.yaml +++ b/openapi.yaml @@ -883,7 +883,7 @@ components: properties: cpu: { type: number, description: vCPUs } memory_mb: { type: integer, description: Memory in MiB } - disk_gb: { type: integer, description: Disk in GiB } + disk_gb: { type: integer, description: Writable overlay upper size in GiB (OCI images). Zero uses the image default. } CreateRequest: type: object