From e0894376ea65b2ed86be8bb1799244a66ce17a55 Mon Sep 17 00:00:00 2001 From: Thales Maciel Date: Sat, 18 Apr 2026 15:10:26 -0300 Subject: [PATCH] vm create: auto-pull image and kernel from catalogs if missing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One-command sandbox: `banger vm run` on a fresh host now Just Works. No prior `banger image pull` or `banger kernel pull` needed. Changes: - Default `default_image_name` flips from "default" to "debian-bookworm" so the golden image is the implicit target when `--image` is omitted. - `CreateVM` resolves the image via a new `findOrAutoPullImage`: try the local store first, and on miss fall back to the embedded imagecat catalog + auto-pull. Emits a vm-create progress stage so the user sees "pulling from image catalog" in the create output. - `resolveKernelInputs` gains context + the same pattern via `readOrAutoPullKernel`: try the local kernelcat, and on miss look up the embedded kernelcat and auto-pull. Fires whenever a bundle's manifest references a kernel the user hasn't pulled yet, not just during image pull — any CreateVM with an image that needs a kernel not yet local will resolve it. - `--image` help text updated on both `vm run` and `vm create`. Six tests cover local-hit-no-pull, auto-pull-on-miss, not-in-catalog error propagation, and a non-ENOENT kernel read error does NOT trigger a misleading "not in catalog" claim. Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/cli/banger.go | 4 +- internal/config/config.go | 2 +- internal/config/config_test.go | 4 +- internal/daemon/autopull_test.go | 136 +++++++++++++++++++++++++++++++ internal/daemon/images.go | 38 +++++++-- internal/daemon/images_pull.go | 4 +- internal/daemon/vm_create.go | 29 ++++++- 7 files changed, 202 insertions(+), 15 deletions(-) create mode 100644 internal/daemon/autopull_test.go diff --git a/internal/cli/banger.go b/internal/cli/banger.go index dccda53..2852819 100644 --- a/internal/cli/banger.go +++ b/internal/cli/banger.go @@ -859,7 +859,7 @@ Three modes: }, } cmd.Flags().StringVar(&name, "name", "", "vm name") - cmd.Flags().StringVar(&imageName, "image", "", "image name or id") + cmd.Flags().StringVar(&imageName, "image", "", "image name or id (defaults to config's default_image_name; auto-pulled from imagecat if missing)") cmd.Flags().IntVar(&vcpu, "vcpu", model.DefaultVCPUCount, "vcpu count") cmd.Flags().IntVar(&memory, "memory", model.DefaultMemoryMiB, "memory in MiB") cmd.Flags().StringVar(&systemOverlaySize, "system-overlay-size", model.FormatSizeBytes(model.DefaultSystemOverlaySize), "system overlay size") @@ -949,7 +949,7 @@ func newVMCreateCommand() *cobra.Command { }, } cmd.Flags().StringVar(&name, "name", "", "vm name") - cmd.Flags().StringVar(&imageName, "image", "", "image name or id") + cmd.Flags().StringVar(&imageName, "image", "", "image name or id (defaults to config's default_image_name; auto-pulled from imagecat if missing)") cmd.Flags().IntVar(&vcpu, "vcpu", model.DefaultVCPUCount, "vcpu count") cmd.Flags().IntVar(&memory, "memory", model.DefaultMemoryMiB, "memory in MiB") cmd.Flags().StringVar(&systemOverlaySize, "system-overlay-size", model.FormatSizeBytes(model.DefaultSystemOverlaySize), "system overlay size") diff --git a/internal/config/config.go b/internal/config/config.go index bfaf926..c2066d7 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -46,7 +46,7 @@ func Load(layout paths.Layout) (model.DaemonConfig, error) { CIDR: model.DefaultCIDR, TapPoolSize: 4, DefaultDNS: model.DefaultDNS, - DefaultImageName: "default", + DefaultImageName: "debian-bookworm", } var file fileConfig diff --git a/internal/config/config_test.go b/internal/config/config_test.go index d4afe50..e22fce5 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -35,8 +35,8 @@ func TestLoadDefaultsResolveFirecrackerAndGenerateSSHKey(t *testing.T) { t.Fatalf("stat %s: %v", path, err) } } - if cfg.DefaultImageName != "default" { - t.Fatalf("DefaultImageName = %q, want default", cfg.DefaultImageName) + if cfg.DefaultImageName != "debian-bookworm" { + t.Fatalf("DefaultImageName = %q, want debian-bookworm", cfg.DefaultImageName) } if cfg.WebListenAddr != "127.0.0.1:7777" { t.Fatalf("WebListenAddr = %q", cfg.WebListenAddr) diff --git a/internal/daemon/autopull_test.go b/internal/daemon/autopull_test.go new file mode 100644 index 0000000..bdf017b --- /dev/null +++ b/internal/daemon/autopull_test.go @@ -0,0 +1,136 @@ +package daemon + +import ( + "context" + "errors" + "os" + "path/filepath" + "strings" + "testing" + + "banger/internal/imagecat" + "banger/internal/model" + "banger/internal/paths" + "banger/internal/system" +) + +func TestFindOrAutoPullImageReturnsLocalWithoutPulling(t *testing.T) { + d := &Daemon{ + layout: paths.Layout{ImagesDir: t.TempDir()}, + store: openDaemonStore(t), + runner: system.NewRunner(), + bundleFetch: func(context.Context, string, imagecat.CatEntry) (imagecat.Manifest, error) { + t.Fatal("bundleFetch should not be called when image is local") + return imagecat.Manifest{}, nil + }, + } + id, _ := model.NewID() + if err := d.store.UpsertImage(context.Background(), model.Image{ + ID: id, + Name: "my-local-image", + CreatedAt: model.Now(), + UpdatedAt: model.Now(), + }); err != nil { + t.Fatal(err) + } + image, err := d.findOrAutoPullImage(context.Background(), "my-local-image") + if err != nil { + t.Fatalf("findOrAutoPullImage: %v", err) + } + if image.Name != "my-local-image" { + t.Fatalf("Name = %q, want my-local-image", image.Name) + } +} + +func TestFindOrAutoPullImagePullsFromCatalog(t *testing.T) { + imagesDir := t.TempDir() + kernelsDir := t.TempDir() + seedKernel(t, kernelsDir, "generic-6.12") + + pullCalls := 0 + d := &Daemon{ + layout: paths.Layout{ImagesDir: imagesDir, KernelsDir: kernelsDir}, + store: openDaemonStore(t), + runner: system.NewRunner(), + bundleFetch: func(ctx context.Context, destDir string, entry imagecat.CatEntry) (imagecat.Manifest, error) { + pullCalls++ + return stubBundleFetch(imagecat.Manifest{KernelRef: "generic-6.12"})(ctx, destDir, entry) + }, + } + // "debian-bookworm" is in the embedded imagecat catalog. + image, err := d.findOrAutoPullImage(context.Background(), "debian-bookworm") + if err != nil { + t.Fatalf("findOrAutoPullImage: %v", err) + } + if image.Name != "debian-bookworm" { + t.Fatalf("Name = %q, want debian-bookworm", image.Name) + } + if pullCalls != 1 { + t.Fatalf("bundleFetch calls = %d, want 1", pullCalls) + } +} + +func TestFindOrAutoPullImageReturnsOriginalErrorWhenNotInCatalog(t *testing.T) { + d := &Daemon{ + layout: paths.Layout{ImagesDir: t.TempDir()}, + store: openDaemonStore(t), + runner: system.NewRunner(), + } + _, err := d.findOrAutoPullImage(context.Background(), "not-in-catalog-or-store") + if err == nil || !strings.Contains(err.Error(), "not found") { + t.Fatalf("err = %v, want not-found", err) + } +} + +func TestReadOrAutoPullKernelReturnsLocalWithoutPulling(t *testing.T) { + kernelsDir := t.TempDir() + seedKernel(t, kernelsDir, "generic-6.12") + d := &Daemon{layout: paths.Layout{KernelsDir: kernelsDir}} + + entry, err := d.readOrAutoPullKernel(context.Background(), "generic-6.12") + if err != nil { + t.Fatalf("readOrAutoPullKernel: %v", err) + } + if entry.Name != "generic-6.12" { + t.Fatalf("Name = %q", entry.Name) + } +} + +func TestReadOrAutoPullKernelErrorsWhenNotInCatalog(t *testing.T) { + d := &Daemon{layout: paths.Layout{KernelsDir: t.TempDir()}} + _, err := d.readOrAutoPullKernel(context.Background(), "nonexistent-kernel") + if err == nil || !strings.Contains(err.Error(), "not found") { + t.Fatalf("err = %v, want not-found", err) + } +} + +// TestReadOrAutoPullKernelSurfacesNonNotExistError covers the path where +// kernelcat.ReadLocal fails for a reason other than missing entry (e.g. +// corrupt manifest); the autopull logic should NOT try to fetch in that +// case since the entry clearly exists in some broken form. +func TestReadOrAutoPullKernelSurfacesNonNotExistError(t *testing.T) { + kernelsDir := t.TempDir() + // Seed a manifest that doesn't match the entry's own Name field — + // kernelcat.ReadLocal returns an error, not os.ErrNotExist. + dir := filepath.Join(kernelsDir, "broken-kernel") + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "manifest.json"), []byte(`{"name":"different-name"}`), 0o644); err != nil { + t.Fatal(err) + } + d := &Daemon{layout: paths.Layout{KernelsDir: kernelsDir}} + _, err := d.readOrAutoPullKernel(context.Background(), "broken-kernel") + if err == nil { + t.Fatal("want error") + } + // Must not be wrapped in an "auto-pull" message — the corrupt-manifest + // failure should surface as the primary cause. + if strings.Contains(err.Error(), "not found in catalog") { + t.Fatalf("err = %v, should not claim 'not in catalog'", err) + } + // Sanity: ensure it's not os.ErrNotExist-compatible. + if errors.Is(err, os.ErrNotExist) { + t.Fatalf("err = %v, should not be os.ErrNotExist", err) + } +} diff --git a/internal/daemon/images.go b/internal/daemon/images.go index 2cdb3dc..f667fed 100644 --- a/internal/daemon/images.go +++ b/internal/daemon/images.go @@ -179,7 +179,7 @@ func (d *Daemon) RegisterImage(ctx context.Context, params api.ImageRegisterPara } } } - kernelPath, initrdPath, modulesDir, err := d.resolveKernelInputs(params.KernelRef, params.KernelPath, params.InitrdPath, params.ModulesDir) + kernelPath, initrdPath, modulesDir, err := d.resolveKernelInputs(ctx, params.KernelRef, params.KernelPath, params.InitrdPath, params.ModulesDir) if err != nil { return model.Image{}, err } @@ -374,7 +374,10 @@ func firstNonEmpty(values ...string) string { // resolveKernelInputs canonicalises user-supplied kernel info: either direct // paths or a kernel-catalog ref. Shared by RegisterImage and PullImage. -func (d *Daemon) resolveKernelInputs(kernelRef, kernelPath, initrdPath, modulesDir string) (string, string, string, error) { +// When kernelRef is given but not yet pulled locally, an auto-pull from the +// embedded kernelcat catalog fires so the caller doesn't have to manage +// kernel/image ordering by hand. +func (d *Daemon) resolveKernelInputs(ctx context.Context, kernelRef, kernelPath, initrdPath, modulesDir string) (string, string, string, error) { kernelRef = strings.TrimSpace(kernelRef) kernelPath = strings.TrimSpace(kernelPath) initrdPath = strings.TrimSpace(initrdPath) @@ -384,12 +387,9 @@ func (d *Daemon) resolveKernelInputs(kernelRef, kernelPath, initrdPath, modulesD if kernelPath != "" || initrdPath != "" || modulesDir != "" { return "", "", "", fmt.Errorf("--kernel-ref is mutually exclusive with --kernel/--initrd/--modules") } - entry, err := kernelcat.ReadLocal(d.layout.KernelsDir, kernelRef) + entry, err := d.readOrAutoPullKernel(ctx, kernelRef) if err != nil { - if os.IsNotExist(err) { - return "", "", "", fmt.Errorf("kernel %q not found in catalog; run 'banger kernel list' to see available entries", kernelRef) - } - return "", "", "", fmt.Errorf("resolve kernel %q: %w", kernelRef, err) + return "", "", "", err } return entry.KernelPath, entry.InitrdPath, entry.ModulesDir, nil } @@ -399,3 +399,27 @@ func (d *Daemon) resolveKernelInputs(kernelRef, kernelPath, initrdPath, modulesD } return kernelPath, initrdPath, modulesDir, nil } + +// readOrAutoPullKernel tries the local kernelcat first; on miss, checks +// the embedded catalog and auto-pulls the bundle. +func (d *Daemon) readOrAutoPullKernel(ctx context.Context, kernelRef string) (kernelcat.Entry, error) { + entry, err := kernelcat.ReadLocal(d.layout.KernelsDir, kernelRef) + if err == nil { + return entry, nil + } + if !os.IsNotExist(err) { + return kernelcat.Entry{}, fmt.Errorf("resolve kernel %q: %w", kernelRef, err) + } + catalog, loadErr := kernelcat.LoadEmbedded() + if loadErr != nil { + return kernelcat.Entry{}, fmt.Errorf("kernel %q not found locally: %w", kernelRef, loadErr) + } + if _, lookupErr := catalog.Lookup(kernelRef); lookupErr != nil { + return kernelcat.Entry{}, fmt.Errorf("kernel %q not found in catalog; run 'banger kernel list --available' to browse", kernelRef) + } + vmCreateStage(ctx, "auto_pull_kernel", fmt.Sprintf("pulling kernel %s from catalog", kernelRef)) + if _, pullErr := d.KernelPull(ctx, api.KernelPullParams{Name: kernelRef}); pullErr != nil { + return kernelcat.Entry{}, fmt.Errorf("auto-pull kernel %q: %w", kernelRef, pullErr) + } + return kernelcat.ReadLocal(d.layout.KernelsDir, kernelRef) +} diff --git a/internal/daemon/images_pull.go b/internal/daemon/images_pull.go index 7ade13a..aac2769 100644 --- a/internal/daemon/images_pull.go +++ b/internal/daemon/images_pull.go @@ -75,7 +75,7 @@ func (d *Daemon) pullFromOCI(ctx context.Context, params api.ImagePullParams) (i return model.Image{}, fmt.Errorf("image %q already exists (id=%s); pick a different --name or delete it first", imgName, existing.ID) } - kernelPath, initrdPath, modulesDir, err := d.resolveKernelInputs(params.KernelRef, params.KernelPath, params.InitrdPath, params.ModulesDir) + kernelPath, initrdPath, modulesDir, err := d.resolveKernelInputs(ctx, params.KernelRef, params.KernelPath, params.InitrdPath, params.ModulesDir) if err != nil { return model.Image{}, err } @@ -181,7 +181,7 @@ func (d *Daemon) pullFromBundle(ctx context.Context, params api.ImagePullParams, if kernelRef == "" && strings.TrimSpace(params.KernelPath) == "" { kernelRef = strings.TrimSpace(entry.KernelRef) } - kernelPath, initrdPath, modulesDir, err := d.resolveKernelInputs(kernelRef, params.KernelPath, params.InitrdPath, params.ModulesDir) + kernelPath, initrdPath, modulesDir, err := d.resolveKernelInputs(ctx, kernelRef, params.KernelPath, params.InitrdPath, params.ModulesDir) if err != nil { return model.Image{}, err } diff --git a/internal/daemon/vm_create.go b/internal/daemon/vm_create.go index 13c1a3f..fce1450 100644 --- a/internal/daemon/vm_create.go +++ b/internal/daemon/vm_create.go @@ -8,6 +8,7 @@ import ( "strings" "banger/internal/api" + "banger/internal/imagecat" "banger/internal/model" "banger/internal/vmdns" ) @@ -35,7 +36,7 @@ func (d *Daemon) CreateVM(ctx context.Context, params api.VMCreateParams) (vm mo imageName = d.config.DefaultImageName } vmCreateStage(ctx, "resolve_image", "resolving image") - image, err := d.FindImage(ctx, imageName) + image, err := d.findOrAutoPullImage(ctx, imageName) if err != nil { return model.VMRecord{}, err } @@ -129,3 +130,29 @@ func (d *Daemon) CreateVM(ctx context.Context, params api.VMCreateParams) (vm mo } return d.startVMLocked(ctx, vm, image) } + +// findOrAutoPullImage tries the local image store first; if the name +// isn't registered but matches an entry in the embedded imagecat +// catalog, it auto-pulls the bundle so `vm create --image foo` (and +// therefore `vm run`) works on a fresh host without the user having +// to run `image pull` first. +func (d *Daemon) findOrAutoPullImage(ctx context.Context, idOrName string) (model.Image, error) { + image, err := d.FindImage(ctx, idOrName) + if err == nil { + return image, nil + } + catalog, loadErr := imagecat.LoadEmbedded() + if loadErr != nil { + return model.Image{}, err + } + entry, lookupErr := catalog.Lookup(idOrName) + if lookupErr != nil { + // Not in the catalog either — surface the original not-found. + return model.Image{}, err + } + vmCreateStage(ctx, "auto_pull_image", fmt.Sprintf("pulling %s from image catalog", entry.Name)) + if _, pullErr := d.PullImage(ctx, api.ImagePullParams{Ref: entry.Name}); pullErr != nil { + return model.Image{}, fmt.Errorf("auto-pull image %q: %w", entry.Name, pullErr) + } + return d.FindImage(ctx, idOrName) +}