From 3edd7c6de700b9264f5feee8f2c329d8c6332139 Mon Sep 17 00:00:00 2001 From: Thales Maciel Date: Thu, 23 Apr 2026 20:24:10 -0300 Subject: [PATCH] daemon: build a work-seed during image pull, refresh doctor check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before this change `banger image pull` (both OCI-direct and bundle paths) shipped images with an empty WorkSeedPath — the BuildWorkSeedImage helper existed only behind the hidden `banger internal work-seed` CLI. Every pulled image hit ensureWorkDisk's no-seed branch, and the guest booted with a bare /root (no .bashrc, no .profile, none of the distro defaults). Pull now calls BuildWorkSeedImage after the rootfs is finalised (OCI) or fetched (bundle). The builder is behind a new `workSeedBuilder` test seam so existing pull tests don't accidentally demand sudo mount. The build failure is non-fatal: any error logs a warning and leaves WorkSeedPath empty — images stay publishable even if the pulled rootfs has no /root to extract. Verified end-to-end by wiping the cached smoke image and re-pulling: work-seed.ext4 lands in the artifact dir next to rootfs.ext4, and all 21 smoke scenarios pass. Also refreshes the "feature /root work disk" fallback tooling check — the no-seed path no longer touches mount/umount/cp after commit 0e28504, so the doctor check now only requires truncate + mkfs.ext4. The warn copy updates from "new VM creates will be slower" to "guest /root will be empty", which matches the actual tradeoff post-refactor. Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/daemon/autopull_test.go | 1 + internal/daemon/capabilities.go | 4 +-- internal/daemon/concurrency_test.go | 2 ++ internal/daemon/image_service.go | 1 + internal/daemon/images_pull.go | 33 +++++++++++++++++++ internal/daemon/images_pull_bundle_test.go | 38 +++++++++++++--------- internal/daemon/images_pull_test.go | 13 ++++++++ internal/daemon/vm_disk.go | 1 - 8 files changed, 74 insertions(+), 19 deletions(-) diff --git a/internal/daemon/autopull_test.go b/internal/daemon/autopull_test.go index c10eb63..6907eff 100644 --- a/internal/daemon/autopull_test.go +++ b/internal/daemon/autopull_test.go @@ -67,6 +67,7 @@ func TestFindOrAutoPullImagePullsFromCatalog(t *testing.T) { pullCalls++ return stubBundleFetch(imagecat.Manifest{KernelRef: "generic-6.12"})(ctx, destDir, entry) }, + workSeedBuilder: stubWorkSeedBuilder, } wireServices(d) // "debian-bookworm" is in the embedded imagecat catalog. diff --git a/internal/daemon/capabilities.go b/internal/daemon/capabilities.go index d4037c2..730be93 100644 --- a/internal/daemon/capabilities.go +++ b/internal/daemon/capabilities.go @@ -251,11 +251,11 @@ func (c workDiskCapability) AddDoctorChecks(_ context.Context, report *system.Re } } checks := system.NewPreflight() - for _, command := range []string{"mkfs.ext4", "mount", "umount", "cp"} { + for _, command := range []string{"truncate", "mkfs.ext4"} { checks.RequireCommand(command, toolHint(command)) } report.AddPreflight("feature /root work disk", checks, "fallback /root work disk tooling available") - report.AddWarn("feature /root work disk", "default image has no work-seed artifact; new VM creates will be slower until the image is rebuilt") + report.AddWarn("feature /root work disk", "default image has no work-seed artifact; guest /root will be empty until the image is rebuilt") } // dnsCapability publishes + removes .vm records on the in-process diff --git a/internal/daemon/concurrency_test.go b/internal/daemon/concurrency_test.go index 2ef3478..ed0d59a 100644 --- a/internal/daemon/concurrency_test.go +++ b/internal/daemon/concurrency_test.go @@ -75,6 +75,7 @@ func TestPullImageDoesNotSerialiseOnDifferentNames(t *testing.T) { runner: d.runner, pullAndFlatten: slowPullAndFlatten, finalizePulledRootfs: stubFinalizePulledRootfs, + workSeedBuilder: stubWorkSeedBuilder, } wireServices(d) @@ -162,6 +163,7 @@ func TestPullImageRejectsNameClashAtPublish(t *testing.T) { runner: d.runner, pullAndFlatten: pullAndFlatten, finalizePulledRootfs: stubFinalizePulledRootfs, + workSeedBuilder: stubWorkSeedBuilder, } wireServices(d) diff --git a/internal/daemon/image_service.go b/internal/daemon/image_service.go index d1d4e84..ea0be21 100644 --- a/internal/daemon/image_service.go +++ b/internal/daemon/image_service.go @@ -42,6 +42,7 @@ type ImageService struct { pullAndFlatten func(ctx context.Context, ref, cacheDir, destDir string) (imagepull.Metadata, error) finalizePulledRootfs func(ctx context.Context, ext4File string, meta imagepull.Metadata) error bundleFetch func(ctx context.Context, destDir string, entry imagecat.CatEntry) (imagecat.Manifest, error) + workSeedBuilder func(ctx context.Context, rootfsExt4, outPath string) error // beginOperation is a test seam used by a couple of image ops that // want structured operation logging. Nil → Daemon's beginOperation, diff --git a/internal/daemon/images_pull.go b/internal/daemon/images_pull.go index 26b0b7c..3f8a8d4 100644 --- a/internal/daemon/images_pull.go +++ b/internal/daemon/images_pull.go @@ -16,6 +16,7 @@ import ( "banger/internal/imagepull" "banger/internal/model" "banger/internal/paths" + "banger/internal/system" "github.com/google/go-containerregistry/pkg/name" ) @@ -168,6 +169,7 @@ func (s *ImageService) pullFromOCI(ctx context.Context, params api.ImagePullPara if err := s.runFinalizePulledRootfs(ctx, rootfsExt4, meta); err != nil { return model.Image{}, err } + workSeedExt4 := s.runBuildWorkSeed(ctx, rootfsExt4, stagingDir) stagedKernel, stagedInitrd, stagedModules, err := imagemgr.StageBootArtifacts(ctx, s.runner, stagingDir, kernelPath, initrdPath, modulesDir) if err != nil { @@ -187,6 +189,9 @@ func (s *ImageService) pullFromOCI(ctx context.Context, params api.ImagePullPara CreatedAt: now, UpdatedAt: now, } + if workSeedExt4 != "" { + image.WorkSeedPath = filepath.Join(finalDir, filepath.Base(workSeedExt4)) + } published, err := s.publishImage(ctx, image, stagingDir, finalDir) if err != nil { return model.Image{}, err @@ -245,6 +250,7 @@ func (s *ImageService) pullFromBundle(ctx context.Context, params api.ImagePullP // so the final artifact dir contains only boot-relevant files. _ = os.Remove(filepath.Join(stagingDir, imagecat.ManifestFilename)) rootfsExt4 := filepath.Join(stagingDir, imagecat.RootfsFilename) + workSeedExt4 := s.runBuildWorkSeed(ctx, rootfsExt4, stagingDir) stagedKernel, stagedInitrd, stagedModules, err := imagemgr.StageBootArtifacts(ctx, s.runner, stagingDir, kernelPath, initrdPath, modulesDir) if err != nil { @@ -264,6 +270,9 @@ func (s *ImageService) pullFromBundle(ctx context.Context, params api.ImagePullP CreatedAt: now, UpdatedAt: now, } + if workSeedExt4 != "" { + image.WorkSeedPath = filepath.Join(finalDir, filepath.Base(workSeedExt4)) + } published, err := s.publishImage(ctx, image, stagingDir, finalDir) if err != nil { return model.Image{}, err @@ -315,6 +324,30 @@ func (s *ImageService) runFinalizePulledRootfs(ctx context.Context, ext4File str return nil } +// runBuildWorkSeed extracts /root from the pulled rootfs into a +// sibling work-seed ext4 image. Any failure is treated as non-fatal: +// the image is still publishable without a seed, and VM create falls +// back to the empty-work-disk path (losing distro dotfiles but keeping +// every other guarantee). Returns the work-seed path on success, "" on +// failure (with a warn logged). Tests substitute via s.workSeedBuilder. +func (s *ImageService) runBuildWorkSeed(ctx context.Context, rootfsExt4, stagingDir string) string { + outPath := filepath.Join(stagingDir, "work-seed.ext4") + var err error + if s.workSeedBuilder != nil { + err = s.workSeedBuilder(ctx, rootfsExt4, outPath) + } else { + err = system.BuildWorkSeedImage(ctx, s.runner, rootfsExt4, outPath) + } + if err != nil { + if s.logger != nil { + s.logger.Warn("work-seed build failed; VMs using this image will start with an empty /root", "rootfs", rootfsExt4, "error", err.Error()) + } + _ = os.Remove(outPath) + return "" + } + return outPath +} + // nameSanitize keeps lowercase alphanumerics + hyphens, collapses runs. var nameSanitizeRE = regexp.MustCompile(`[^a-z0-9]+`) diff --git a/internal/daemon/images_pull_bundle_test.go b/internal/daemon/images_pull_bundle_test.go index 57ee5db..2e2ea29 100644 --- a/internal/daemon/images_pull_bundle_test.go +++ b/internal/daemon/images_pull_bundle_test.go @@ -68,10 +68,11 @@ func TestPullImageBundlePathRegistersFromCatalog(t *testing.T) { runner: system.NewRunner(), } d.img = &ImageService{ - layout: d.layout, - store: d.store, - runner: d.runner, - bundleFetch: stubBundleFetch(imagecat.Manifest{KernelRef: "generic-6.12"}), + layout: d.layout, + store: d.store, + runner: d.runner, + bundleFetch: stubBundleFetch(imagecat.Manifest{KernelRef: "generic-6.12"}), + workSeedBuilder: stubWorkSeedBuilder, } wireServices(d) @@ -122,10 +123,11 @@ func TestPullImageBundlePathOverrideNameAndKernelRef(t *testing.T) { runner: system.NewRunner(), } d.img = &ImageService{ - layout: d.layout, - store: d.store, - runner: d.runner, - bundleFetch: stubBundleFetch(imagecat.Manifest{KernelRef: "generic-6.12"}), + layout: d.layout, + store: d.store, + runner: d.runner, + bundleFetch: stubBundleFetch(imagecat.Manifest{KernelRef: "generic-6.12"}), + workSeedBuilder: stubWorkSeedBuilder, } wireServices(d) @@ -164,10 +166,11 @@ func TestPullImageBundlePathRejectsExistingName(t *testing.T) { runner: system.NewRunner(), } d.img = &ImageService{ - layout: d.layout, - store: d.store, - runner: d.runner, - bundleFetch: stubBundleFetch(imagecat.Manifest{KernelRef: "generic-6.12"}), + layout: d.layout, + store: d.store, + runner: d.runner, + bundleFetch: stubBundleFetch(imagecat.Manifest{KernelRef: "generic-6.12"}), + workSeedBuilder: stubWorkSeedBuilder, } wireServices(d) id, _ := model.NewID() @@ -194,10 +197,11 @@ func TestPullImageBundlePathRequiresSomeKernelSource(t *testing.T) { runner: system.NewRunner(), } d.img = &ImageService{ - layout: d.layout, - store: d.store, - runner: d.runner, - bundleFetch: stubBundleFetch(imagecat.Manifest{}), + layout: d.layout, + store: d.store, + runner: d.runner, + bundleFetch: stubBundleFetch(imagecat.Manifest{}), + workSeedBuilder: stubWorkSeedBuilder, } wireServices(d) // Catalog entry has no kernel_ref, no --kernel-ref/--kernel passed. @@ -226,6 +230,7 @@ func TestPullImageBundleFetchFailurePropagates(t *testing.T) { bundleFetch: func(_ context.Context, _ string, _ imagecat.CatEntry) (imagecat.Manifest, error) { return imagecat.Manifest{}, errors.New("r2 exploded") }, + workSeedBuilder: stubWorkSeedBuilder, } wireServices(d) _, err := d.img.pullFromBundle(context.Background(), api.ImagePullParams{Ref: "x"}, imagecat.CatEntry{ @@ -266,6 +271,7 @@ func TestPullImageDispatchFallsThroughToOCIWhenNoCatalogHit(t *testing.T) { }, finalizePulledRootfs: stubFinalizePulledRootfs, bundleFetch: stubBundleFetch(imagecat.Manifest{}), + workSeedBuilder: stubWorkSeedBuilder, } wireServices(d) diff --git a/internal/daemon/images_pull_test.go b/internal/daemon/images_pull_test.go index 8b99592..f65c046 100644 --- a/internal/daemon/images_pull_test.go +++ b/internal/daemon/images_pull_test.go @@ -45,6 +45,15 @@ func stubFinalizePulledRootfs(_ context.Context, _ string, _ imagepull.Metadata) return nil } +// stubWorkSeedBuilder returns an error so runBuildWorkSeed treats +// the step as non-fatal and proceeds without a work-seed. Keeps tests +// off sudo mount without asserting on WorkSeedPath. +func stubWorkSeedBuilder(_ context.Context, _ string, _ string) error { + return errWorkSeedBuilderStub +} + +var errWorkSeedBuilderStub = errors.New("work-seed builder stubbed in tests") + // stubPullAndFlatten writes a fixed file tree into destDir, simulating a // successful OCI pull without the network or tarball machinery. func stubPullAndFlatten(_ context.Context, _ string, _ string, destDir string) (imagepull.Metadata, error) { @@ -81,6 +90,7 @@ func TestPullImageHappyPath(t *testing.T) { runner: d.runner, pullAndFlatten: stubPullAndFlatten, finalizePulledRootfs: stubFinalizePulledRootfs, + workSeedBuilder: stubWorkSeedBuilder, } wireServices(d) @@ -132,6 +142,7 @@ func TestPullImageRejectsExistingName(t *testing.T) { runner: d.runner, pullAndFlatten: stubPullAndFlatten, finalizePulledRootfs: stubFinalizePulledRootfs, + workSeedBuilder: stubWorkSeedBuilder, } wireServices(d) // Seed a preexisting image with the would-be derived name. @@ -166,6 +177,7 @@ func TestPullImageRequiresKernel(t *testing.T) { runner: d.runner, pullAndFlatten: stubPullAndFlatten, finalizePulledRootfs: stubFinalizePulledRootfs, + workSeedBuilder: stubWorkSeedBuilder, } wireServices(d) _, err := d.img.PullImage(context.Background(), api.ImagePullParams{ @@ -194,6 +206,7 @@ func TestPullImageCleansStagingOnFailure(t *testing.T) { runner: d.runner, pullAndFlatten: failureSeam, finalizePulledRootfs: stubFinalizePulledRootfs, + workSeedBuilder: stubWorkSeedBuilder, } wireServices(d) _, err := d.img.PullImage(context.Background(), api.ImagePullParams{ diff --git a/internal/daemon/vm_disk.go b/internal/daemon/vm_disk.go index e4ff38c..a8b84be 100644 --- a/internal/daemon/vm_disk.go +++ b/internal/daemon/vm_disk.go @@ -175,4 +175,3 @@ func sshdGuestConfig() string { "", }, "\n") } -