From d7614a3b2bf9a1ede213bb79ef421a8c144afeab Mon Sep 17 00:00:00 2001 From: Thales Maciel Date: Mon, 20 Apr 2026 20:30:32 -0300 Subject: [PATCH] daemon split (2/5): extract *ImageService service MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second phase of splitting the daemon god-struct. ImageService now owns all image + kernel registry operations: register/promote/delete/pull for images (bundle + OCI paths), the six kernel commands, and the shared SSH-key/work-seed injection helpers. imageOpsMu (the publication-window lock) lives on the service; so do the three OCI pull test seams pullAndFlatten / finalizePulledRootfs / bundleFetch. The four files images.go, images_pull.go, image_seed.go, kernels.go flipped their receivers from *Daemon to *ImageService. FindImage moved with the service. Daemon keeps a thin FindImage forwarder so callers reading the dispatch code see the obvious facade and tests that pre-date the split still compile. flattenNestedWorkHome — called from image_seed.go, vm_authsync.go, and vm_disk.go across future service boundaries — became a package-level helper taking a CommandRunner explicitly. Daemon keeps a deprecated forwarder for now; the other services will use the package form. Lazy-init helper imageSvc() on Daemon mirrors hostNet() from Phase 1, so test literals like &Daemon{store: db, runner: r, ...} that don't spell out an ImageService still get a working one. Tests that override the image test seams (autopull_test, concurrency_test, images_pull_test, images_pull_bundle_test) now assign d.img = &ImageService{...seams...}; the two-statement pattern matches what Phase 1 established for HostNetwork. Dispatch in daemon.go is cleaner now: every image/kernel RPC handler is a single-liner forwarding to d.imageSvc().*. Phase 5 will do the same for VM lifecycle. Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/daemon/autopull_test.go | 16 ++- internal/daemon/concurrency_test.go | 26 +++-- internal/daemon/daemon.go | 61 +++------- internal/daemon/daemon_test.go | 4 +- internal/daemon/image_seed.go | 30 ++--- internal/daemon/image_service.go | 129 +++++++++++++++++++++ internal/daemon/images.go | 60 +++++----- internal/daemon/images_pull.go | 72 ++++++------ internal/daemon/images_pull_bundle_test.go | 66 ++++++++--- internal/daemon/images_pull_test.go | 52 ++++++--- internal/daemon/kernels.go | 34 +++--- internal/daemon/kernels_test.go | 22 ++-- internal/daemon/vm_authsync.go | 2 +- internal/daemon/vm_create.go | 6 +- internal/daemon/vm_disk.go | 18 ++- 15 files changed, 389 insertions(+), 209 deletions(-) create mode 100644 internal/daemon/image_service.go diff --git a/internal/daemon/autopull_test.go b/internal/daemon/autopull_test.go index bdf017b..1c2a8ac 100644 --- a/internal/daemon/autopull_test.go +++ b/internal/daemon/autopull_test.go @@ -19,6 +19,11 @@ func TestFindOrAutoPullImageReturnsLocalWithoutPulling(t *testing.T) { layout: paths.Layout{ImagesDir: t.TempDir()}, store: openDaemonStore(t), runner: system.NewRunner(), + } + d.img = &ImageService{ + layout: d.layout, + store: d.store, + runner: d.runner, 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 @@ -52,6 +57,11 @@ func TestFindOrAutoPullImagePullsFromCatalog(t *testing.T) { layout: paths.Layout{ImagesDir: imagesDir, KernelsDir: kernelsDir}, store: openDaemonStore(t), runner: system.NewRunner(), + } + d.img = &ImageService{ + layout: d.layout, + store: d.store, + runner: d.runner, 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) @@ -87,7 +97,7 @@ func TestReadOrAutoPullKernelReturnsLocalWithoutPulling(t *testing.T) { seedKernel(t, kernelsDir, "generic-6.12") d := &Daemon{layout: paths.Layout{KernelsDir: kernelsDir}} - entry, err := d.readOrAutoPullKernel(context.Background(), "generic-6.12") + entry, err := d.imageSvc().readOrAutoPullKernel(context.Background(), "generic-6.12") if err != nil { t.Fatalf("readOrAutoPullKernel: %v", err) } @@ -98,7 +108,7 @@ func TestReadOrAutoPullKernelReturnsLocalWithoutPulling(t *testing.T) { func TestReadOrAutoPullKernelErrorsWhenNotInCatalog(t *testing.T) { d := &Daemon{layout: paths.Layout{KernelsDir: t.TempDir()}} - _, err := d.readOrAutoPullKernel(context.Background(), "nonexistent-kernel") + _, err := d.imageSvc().readOrAutoPullKernel(context.Background(), "nonexistent-kernel") if err == nil || !strings.Contains(err.Error(), "not found") { t.Fatalf("err = %v, want not-found", err) } @@ -120,7 +130,7 @@ func TestReadOrAutoPullKernelSurfacesNonNotExistError(t *testing.T) { t.Fatal(err) } d := &Daemon{layout: paths.Layout{KernelsDir: kernelsDir}} - _, err := d.readOrAutoPullKernel(context.Background(), "broken-kernel") + _, err := d.imageSvc().readOrAutoPullKernel(context.Background(), "broken-kernel") if err == nil { t.Fatal("want error") } diff --git a/internal/daemon/concurrency_test.go b/internal/daemon/concurrency_test.go index f9eaa99..e36b56a 100644 --- a/internal/daemon/concurrency_test.go +++ b/internal/daemon/concurrency_test.go @@ -65,9 +65,14 @@ func TestPullImageDoesNotSerialiseOnDifferentNames(t *testing.T) { } d := &Daemon{ - layout: paths.Layout{ImagesDir: imagesDir, OCICacheDir: cacheDir}, - store: openDaemonStore(t), - runner: system.NewRunner(), + layout: paths.Layout{ImagesDir: imagesDir, OCICacheDir: cacheDir}, + store: openDaemonStore(t), + runner: system.NewRunner(), + } + d.img = &ImageService{ + layout: d.layout, + store: d.store, + runner: d.runner, pullAndFlatten: slowPullAndFlatten, finalizePulledRootfs: stubFinalizePulledRootfs, } @@ -88,7 +93,7 @@ func TestPullImageDoesNotSerialiseOnDifferentNames(t *testing.T) { wg.Add(1) go func(i int, name string) { defer wg.Done() - _, err := d.PullImage(context.Background(), mkParams(name)) + _, err := d.img.PullImage(context.Background(), mkParams(name)) errs[i] = err }(i, name) } @@ -146,9 +151,14 @@ func TestPullImageRejectsNameClashAtPublish(t *testing.T) { } d := &Daemon{ - layout: paths.Layout{ImagesDir: imagesDir, OCICacheDir: cacheDir}, - store: openDaemonStore(t), - runner: system.NewRunner(), + layout: paths.Layout{ImagesDir: imagesDir, OCICacheDir: cacheDir}, + store: openDaemonStore(t), + runner: system.NewRunner(), + } + d.img = &ImageService{ + layout: d.layout, + store: d.store, + runner: d.runner, pullAndFlatten: pullAndFlatten, finalizePulledRootfs: stubFinalizePulledRootfs, } @@ -167,7 +177,7 @@ func TestPullImageRejectsNameClashAtPublish(t *testing.T) { wg.Add(1) go func(i int) { defer wg.Done() - _, err := d.PullImage(context.Background(), params) + _, err := d.img.PullImage(context.Background(), params) errs[i] = err }(i) } diff --git a/internal/daemon/daemon.go b/internal/daemon/daemon.go index 2acbf8d..f9500d2 100644 --- a/internal/daemon/daemon.go +++ b/internal/daemon/daemon.go @@ -19,8 +19,6 @@ import ( "banger/internal/config" "banger/internal/daemon/opstate" ws "banger/internal/daemon/workspace" - "banger/internal/imagecat" - "banger/internal/imagepull" "banger/internal/model" "banger/internal/paths" "banger/internal/rpc" @@ -35,7 +33,6 @@ type Daemon struct { store *store.Store runner system.CommandRunner logger *slog.Logger - imageOpsMu sync.Mutex createVMMu sync.Mutex createOps opstate.Registry[*vmCreateOperationState] vmLocks vmLockSet @@ -53,14 +50,12 @@ type Daemon struct { // handles.json scratch file and OS inspection. handles *handleCache net *HostNetwork + img *ImageService closing chan struct{} once sync.Once pid int listener net.Listener vmCaps []vmCapability - 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) requestHandler func(context.Context, rpc.Request) rpc.Response guestWaitForSSH func(context.Context, string, string, time.Duration) error guestDial func(context.Context, string, string) (guestSSHClient, error) @@ -449,68 +444,68 @@ func (d *Daemon) dispatch(ctx context.Context, req rpc.Request) rpc.Response { if err != nil { return rpc.NewError("bad_request", err.Error()) } - image, err := d.FindImage(ctx, params.IDOrName) + image, err := d.imageSvc().FindImage(ctx, params.IDOrName) return marshalResultOrError(api.ImageShowResult{Image: image}, err) case "image.register": params, err := rpc.DecodeParams[api.ImageRegisterParams](req) if err != nil { return rpc.NewError("bad_request", err.Error()) } - image, err := d.RegisterImage(ctx, params) + image, err := d.imageSvc().RegisterImage(ctx, params) return marshalResultOrError(api.ImageShowResult{Image: image}, err) case "image.promote": params, err := rpc.DecodeParams[api.ImageRefParams](req) if err != nil { return rpc.NewError("bad_request", err.Error()) } - image, err := d.PromoteImage(ctx, params.IDOrName) + image, err := d.imageSvc().PromoteImage(ctx, params.IDOrName) return marshalResultOrError(api.ImageShowResult{Image: image}, err) case "image.delete": params, err := rpc.DecodeParams[api.ImageRefParams](req) if err != nil { return rpc.NewError("bad_request", err.Error()) } - image, err := d.DeleteImage(ctx, params.IDOrName) + image, err := d.imageSvc().DeleteImage(ctx, params.IDOrName) return marshalResultOrError(api.ImageShowResult{Image: image}, err) case "image.pull": params, err := rpc.DecodeParams[api.ImagePullParams](req) if err != nil { return rpc.NewError("bad_request", err.Error()) } - image, err := d.PullImage(ctx, params) + image, err := d.imageSvc().PullImage(ctx, params) return marshalResultOrError(api.ImageShowResult{Image: image}, err) case "kernel.list": - return marshalResultOrError(d.KernelList(ctx)) + return marshalResultOrError(d.imageSvc().KernelList(ctx)) case "kernel.show": params, err := rpc.DecodeParams[api.KernelRefParams](req) if err != nil { return rpc.NewError("bad_request", err.Error()) } - entry, err := d.KernelShow(ctx, params.Name) + entry, err := d.imageSvc().KernelShow(ctx, params.Name) return marshalResultOrError(api.KernelShowResult{Entry: entry}, err) case "kernel.delete": params, err := rpc.DecodeParams[api.KernelRefParams](req) if err != nil { return rpc.NewError("bad_request", err.Error()) } - err = d.KernelDelete(ctx, params.Name) + err = d.imageSvc().KernelDelete(ctx, params.Name) return marshalResultOrError(api.Empty{}, err) case "kernel.import": params, err := rpc.DecodeParams[api.KernelImportParams](req) if err != nil { return rpc.NewError("bad_request", err.Error()) } - entry, err := d.KernelImport(ctx, params) + entry, err := d.imageSvc().KernelImport(ctx, params) return marshalResultOrError(api.KernelShowResult{Entry: entry}, err) case "kernel.pull": params, err := rpc.DecodeParams[api.KernelPullParams](req) if err != nil { return rpc.NewError("bad_request", err.Error()) } - entry, err := d.KernelPull(ctx, params) + entry, err := d.imageSvc().KernelPull(ctx, params) return marshalResultOrError(api.KernelShowResult{Entry: entry}, err) case "kernel.catalog": - return marshalResultOrError(d.KernelCatalog(ctx)) + return marshalResultOrError(d.imageSvc().KernelCatalog(ctx)) default: return rpc.NewError("unknown_method", req.Method) } @@ -619,35 +614,11 @@ func (d *Daemon) FindVM(ctx context.Context, idOrName string) (model.VMRecord, e return model.VMRecord{}, fmt.Errorf("vm %q not found", idOrName) } +// FindImage stays on Daemon as a thin forwarder to the image service +// lookup so callers reading dispatch code see the obvious facade, and +// tests that pre-date the service split still compile. func (d *Daemon) FindImage(ctx context.Context, idOrName string) (model.Image, error) { - if idOrName == "" { - return model.Image{}, errors.New("image id or name is required") - } - if image, err := d.store.GetImageByName(ctx, idOrName); err == nil { - return image, nil - } - if image, err := d.store.GetImageByID(ctx, idOrName); err == nil { - return image, nil - } - images, err := d.store.ListImages(ctx) - if err != nil { - return model.Image{}, err - } - matchCount := 0 - var match model.Image - for _, image := range images { - if strings.HasPrefix(image.ID, idOrName) || strings.HasPrefix(image.Name, idOrName) { - match = image - matchCount++ - } - } - if matchCount == 1 { - return match, nil - } - if matchCount > 1 { - return model.Image{}, fmt.Errorf("multiple images match %q", idOrName) - } - return model.Image{}, fmt.Errorf("image %q not found", idOrName) + return d.imageSvc().FindImage(ctx, idOrName) } func (d *Daemon) TouchVM(ctx context.Context, idOrName string) (model.VMRecord, error) { diff --git a/internal/daemon/daemon_test.go b/internal/daemon/daemon_test.go index 04b2b98..0120bab 100644 --- a/internal/daemon/daemon_test.go +++ b/internal/daemon/daemon_test.go @@ -23,7 +23,7 @@ func TestRegisterImageRequiresKernel(t *testing.T) { } d := &Daemon{store: openDaemonStore(t)} - _, err := d.RegisterImage(context.Background(), api.ImageRegisterParams{ + _, err := d.imageSvc().RegisterImage(context.Background(), api.ImageRegisterParams{ Name: "missing-kernel", RootfsPath: rootfs, }) @@ -100,7 +100,7 @@ func TestPromoteImageCopiesBootArtifactsIntoArtifactDir(t *testing.T) { store: db, runner: system.NewRunner(), } - got, err := d.PromoteImage(context.Background(), image.Name) + got, err := d.imageSvc().PromoteImage(context.Background(), image.Name) if err != nil { t.Fatalf("PromoteImage: %v", err) } diff --git a/internal/daemon/image_seed.go b/internal/daemon/image_seed.go index b0f47d3..e4e7785 100644 --- a/internal/daemon/image_seed.go +++ b/internal/daemon/image_seed.go @@ -12,48 +12,48 @@ import ( "banger/internal/system" ) -func (d *Daemon) seedAuthorizedKeyOnExt4Image(ctx context.Context, imagePath string) (string, error) { - if strings.TrimSpace(d.config.SSHKeyPath) == "" { +func (s *ImageService) seedAuthorizedKeyOnExt4Image(ctx context.Context, imagePath string) (string, error) { + if strings.TrimSpace(s.config.SSHKeyPath) == "" { return "", nil } - fingerprint, err := guest.AuthorizedPublicKeyFingerprint(d.config.SSHKeyPath) + fingerprint, err := guest.AuthorizedPublicKeyFingerprint(s.config.SSHKeyPath) if err != nil { return "", fmt.Errorf("derive authorized ssh key fingerprint: %w", err) } - publicKey, err := guest.AuthorizedPublicKey(d.config.SSHKeyPath) + publicKey, err := guest.AuthorizedPublicKey(s.config.SSHKeyPath) if err != nil { return "", fmt.Errorf("derive authorized ssh key: %w", err) } - mountDir, cleanup, err := system.MountTempDir(ctx, d.runner, imagePath, false) + mountDir, cleanup, err := system.MountTempDir(ctx, s.runner, imagePath, false) if err != nil { return "", err } defer cleanup() - if err := d.flattenNestedWorkHome(ctx, mountDir); err != nil { + if err := flattenNestedWorkHome(ctx, s.runner, mountDir); err != nil { return "", err } // Same rationale as in ensureAuthorizedKeyOnWorkDisk — the seed's // filesystem root becomes /root inside the guest, and sshd's // StrictModes check walks its ownership and mode. - if err := normaliseHomeDirPerms(ctx, d.runner, mountDir); err != nil { + if err := normaliseHomeDirPerms(ctx, s.runner, mountDir); err != nil { return "", err } sshDir := filepath.Join(mountDir, ".ssh") - if _, err := d.runner.RunSudo(ctx, "mkdir", "-p", sshDir); err != nil { + if _, err := s.runner.RunSudo(ctx, "mkdir", "-p", sshDir); err != nil { return "", err } - if _, err := d.runner.RunSudo(ctx, "chmod", "700", sshDir); err != nil { + if _, err := s.runner.RunSudo(ctx, "chmod", "700", sshDir); err != nil { return "", err } - if _, err := d.runner.RunSudo(ctx, "chown", "0:0", sshDir); err != nil { + if _, err := s.runner.RunSudo(ctx, "chown", "0:0", sshDir); err != nil { return "", err } authorizedKeysPath := filepath.Join(sshDir, "authorized_keys") - existing, err := d.runner.RunSudo(ctx, "cat", authorizedKeysPath) + existing, err := s.runner.RunSudo(ctx, "cat", authorizedKeysPath) if err != nil { existing = nil } @@ -73,17 +73,17 @@ func (d *Daemon) seedAuthorizedKeyOnExt4Image(ctx context.Context, imagePath str return "", err } defer os.Remove(tmpPath) - if _, err := d.runner.RunSudo(ctx, "install", "-m", "600", tmpPath, authorizedKeysPath); err != nil { + if _, err := s.runner.RunSudo(ctx, "install", "-m", "600", tmpPath, authorizedKeysPath); err != nil { return "", err } return fingerprint, nil } -func (d *Daemon) refreshManagedWorkSeedFingerprint(ctx context.Context, image model.Image, fingerprint string) error { +func (s *ImageService) refreshManagedWorkSeedFingerprint(ctx context.Context, image model.Image, fingerprint string) error { if !image.Managed || strings.TrimSpace(image.WorkSeedPath) == "" || strings.TrimSpace(fingerprint) == "" { return nil } - seededFingerprint, err := d.seedAuthorizedKeyOnExt4Image(ctx, image.WorkSeedPath) + seededFingerprint, err := s.seedAuthorizedKeyOnExt4Image(ctx, image.WorkSeedPath) if err != nil { return err } @@ -92,5 +92,5 @@ func (d *Daemon) refreshManagedWorkSeedFingerprint(ctx context.Context, image mo } image.SeededSSHPublicKeyFingerprint = seededFingerprint image.UpdatedAt = model.Now() - return d.store.UpsertImage(ctx, image) + return s.store.UpsertImage(ctx, image) } diff --git a/internal/daemon/image_service.go b/internal/daemon/image_service.go new file mode 100644 index 0000000..73b3800 --- /dev/null +++ b/internal/daemon/image_service.go @@ -0,0 +1,129 @@ +package daemon + +import ( + "context" + "fmt" + "log/slog" + "strings" + "sync" + + "banger/internal/imagecat" + "banger/internal/imagepull" + "banger/internal/model" + "banger/internal/paths" + "banger/internal/store" + "banger/internal/system" +) + +// ImageService owns everything image-registry-related: register / +// promote / delete / pull (bundle + OCI), plus the kernel catalog +// operations that share the same lifecycle primitives. The publication +// lock imageOpsMu lives here so its scope is obvious at the field +// definition, and the three OCI-pull test seams (pullAndFlatten, +// finalizePulledRootfs, bundleFetch) are fields on the service rather +// than mutable globals on Daemon. +// +// Kept unexported except where peer services (VMService) need it, and +// peer access goes through consumer-defined interfaces, not direct +// struct poking. +type ImageService struct { + runner system.CommandRunner + logger *slog.Logger + config model.DaemonConfig + layout paths.Layout + store *store.Store + + // imageOpsMu is the publication-window lock: held only across the + // "recheck name free + atomic rename + UpsertImage" commit. See + // internal/daemon/ARCHITECTURE.md. + imageOpsMu sync.Mutex + + // Test seams; nil → real implementation. + 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) + + // beginOperation is a test seam used by a couple of image ops that + // want structured operation logging. Nil → Daemon's beginOperation, + // injected at construction. + beginOperation func(name string, attrs ...any) *operationLog +} + +// imageServiceDeps names every handle ImageService needs from the +// Daemon composition root. Using a struct (rather than positional args) +// makes the wiring site in Daemon.Open read as a declaration. +type imageServiceDeps struct { + runner system.CommandRunner + logger *slog.Logger + config model.DaemonConfig + layout paths.Layout + store *store.Store + beginOperation func(name string, attrs ...any) *operationLog +} + +func newImageService(deps imageServiceDeps) *ImageService { + return &ImageService{ + runner: deps.runner, + logger: deps.logger, + config: deps.config, + layout: deps.layout, + store: deps.store, + beginOperation: deps.beginOperation, + } +} + +// FindImage is the service-owned lookup helper. It falls back from +// exact-name → exact-id → prefix match, matching the historical +// daemon.FindImage behaviour. Kept on ImageService because image +// lookup is inherently a service concern. +func (s *ImageService) FindImage(ctx context.Context, idOrName string) (model.Image, error) { + if idOrName == "" { + return model.Image{}, fmt.Errorf("image id or name is required") + } + if image, err := s.store.GetImageByName(ctx, idOrName); err == nil { + return image, nil + } + if image, err := s.store.GetImageByID(ctx, idOrName); err == nil { + return image, nil + } + images, err := s.store.ListImages(ctx) + if err != nil { + return model.Image{}, err + } + matchCount := 0 + var match model.Image + for _, image := range images { + if strings.HasPrefix(image.ID, idOrName) || strings.HasPrefix(image.Name, idOrName) { + match = image + matchCount++ + } + } + if matchCount == 1 { + return match, nil + } + if matchCount > 1 { + return model.Image{}, fmt.Errorf("multiple images match %q", idOrName) + } + return model.Image{}, fmt.Errorf("image %q not found", idOrName) +} + +// imageSvc is the Daemon-side getter that lazy-inits ImageService from +// current Daemon fields. Mirrors hostNet() so test literals can keep +// using `&Daemon{store: db, runner: r, ...}` and still end up with a +// working ImageService. +func (d *Daemon) imageSvc() *ImageService { + if d.img != nil { + return d.img + } + d.img = newImageService(imageServiceDeps{ + runner: d.runner, + logger: d.logger, + config: d.config, + layout: d.layout, + store: d.store, + beginOperation: func(name string, attrs ...any) *operationLog { + return d.beginOperation(name, attrs...) + }, + }) + return d.img +} diff --git a/internal/daemon/images.go b/internal/daemon/images.go index d8c5538..6b5a806 100644 --- a/internal/daemon/images.go +++ b/internal/daemon/images.go @@ -20,7 +20,7 @@ import ( // validation + kernel resolution run without imageOpsMu — only the // lookup-then-upsert atom is held under the lock so concurrent // registers of the same name don't race. -func (d *Daemon) RegisterImage(ctx context.Context, params api.ImageRegisterParams) (image model.Image, err error) { +func (s *ImageService) RegisterImage(ctx context.Context, params api.ImageRegisterParams) (image model.Image, err error) { name := strings.TrimSpace(params.Name) if name == "" { return model.Image{}, fmt.Errorf("image name is required") @@ -39,7 +39,7 @@ func (d *Daemon) RegisterImage(ctx context.Context, params api.ImageRegisterPara } } } - kernelPath, initrdPath, modulesDir, err := d.resolveKernelInputs(ctx, params.KernelRef, params.KernelPath, params.InitrdPath, params.ModulesDir) + kernelPath, initrdPath, modulesDir, err := s.resolveKernelInputs(ctx, params.KernelRef, params.KernelPath, params.InitrdPath, params.ModulesDir) if err != nil { return model.Image{}, err } @@ -48,11 +48,11 @@ func (d *Daemon) RegisterImage(ctx context.Context, params api.ImageRegisterPara return model.Image{}, err } - d.imageOpsMu.Lock() - defer d.imageOpsMu.Unlock() + s.imageOpsMu.Lock() + defer s.imageOpsMu.Unlock() now := model.Now() - existing, lookupErr := d.store.GetImageByName(ctx, name) + existing, lookupErr := s.store.GetImageByName(ctx, name) switch { case lookupErr == nil: if existing.Managed { @@ -88,7 +88,7 @@ func (d *Daemon) RegisterImage(ctx context.Context, params api.ImageRegisterPara return model.Image{}, lookupErr } - if err := d.store.UpsertImage(ctx, image); err != nil { + if err := s.store.UpsertImage(ctx, image); err != nil { return model.Image{}, err } return image, nil @@ -99,8 +99,8 @@ func (d *Daemon) RegisterImage(ctx context.Context, params api.ImageRegisterPara // SSH-key seeding, and boot-artifact staging all happen outside // imageOpsMu — only the find/rename/upsert commit atom holds the // lock. -func (d *Daemon) PromoteImage(ctx context.Context, idOrName string) (image model.Image, err error) { - op := d.beginOperation("image.promote") +func (s *ImageService) PromoteImage(ctx context.Context, idOrName string) (image model.Image, err error) { + op := s.beginOperation("image.promote") defer func() { if err != nil { op.fail(err, imageLogAttrs(image)...) @@ -109,7 +109,7 @@ func (d *Daemon) PromoteImage(ctx context.Context, idOrName string) (image model op.done(imageLogAttrs(image)...) }() - image, err = d.FindImage(ctx, idOrName) + image, err = s.FindImage(ctx, idOrName) if err != nil { return model.Image{}, err } @@ -119,21 +119,21 @@ func (d *Daemon) PromoteImage(ctx context.Context, idOrName string) (image model if err := imagemgr.ValidatePromotePaths(image.RootfsPath, image.KernelPath, image.InitrdPath, image.ModulesDir); err != nil { return model.Image{}, err } - if strings.TrimSpace(d.layout.ImagesDir) == "" { + if strings.TrimSpace(s.layout.ImagesDir) == "" { return model.Image{}, errors.New("images dir is not configured") } - if err := os.MkdirAll(d.layout.ImagesDir, 0o755); err != nil { + if err := os.MkdirAll(s.layout.ImagesDir, 0o755); err != nil { return model.Image{}, err } - artifactDir := filepath.Join(d.layout.ImagesDir, image.ID) + artifactDir := filepath.Join(s.layout.ImagesDir, image.ID) if _, statErr := os.Stat(artifactDir); statErr == nil { return model.Image{}, fmt.Errorf("artifact dir already exists: %s", artifactDir) } else if !os.IsNotExist(statErr) { return model.Image{}, statErr } - stageDir, err := os.MkdirTemp(d.layout.ImagesDir, image.ID+".promote-") + stageDir, err := os.MkdirTemp(s.layout.ImagesDir, image.ID+".promote-") if err != nil { return model.Image{}, err } @@ -167,14 +167,14 @@ func (d *Daemon) PromoteImage(ctx context.Context, idOrName string) (image model if err := system.CopyFilePreferClone(image.WorkSeedPath, workSeedPath); err != nil { return model.Image{}, err } - image.SeededSSHPublicKeyFingerprint, err = d.seedAuthorizedKeyOnExt4Image(ctx, workSeedPath) + image.SeededSSHPublicKeyFingerprint, err = s.seedAuthorizedKeyOnExt4Image(ctx, workSeedPath) if err != nil { return model.Image{}, err } } else { image.SeededSSHPublicKeyFingerprint = "" } - _, initrdPath, modulesDir, err := imagemgr.StageBootArtifacts(ctx, d.runner, stageDir, image.KernelPath, image.InitrdPath, image.ModulesDir) + _, initrdPath, modulesDir, err := imagemgr.StageBootArtifacts(ctx, s.runner, stageDir, image.KernelPath, image.InitrdPath, image.ModulesDir) if err != nil { return model.Image{}, err } @@ -191,13 +191,13 @@ func (d *Daemon) PromoteImage(ctx context.Context, idOrName string) (image model image.UpdatedAt = model.Now() op.stage("activate_artifacts", "artifact_dir", artifactDir) - d.imageOpsMu.Lock() - defer d.imageOpsMu.Unlock() + s.imageOpsMu.Lock() + defer s.imageOpsMu.Unlock() if err := os.Rename(stageDir, artifactDir); err != nil { return model.Image{}, err } cleanupStage = false - if err := d.store.UpsertImage(ctx, image); err != nil { + if err := s.store.UpsertImage(ctx, image); err != nil { _ = os.RemoveAll(artifactDir) return model.Image{}, err } @@ -208,22 +208,22 @@ func (d *Daemon) PromoteImage(ctx context.Context, idOrName string) (image model // imageOpsMu so a concurrent CreateVM can't slip an image_id reference // in between the check and the delete. File cleanup happens after the // lock is released — the store row is the authoritative handle. -func (d *Daemon) DeleteImage(ctx context.Context, idOrName string) (model.Image, error) { +func (s *ImageService) DeleteImage(ctx context.Context, idOrName string) (model.Image, error) { image, err := func() (model.Image, error) { - d.imageOpsMu.Lock() - defer d.imageOpsMu.Unlock() - img, err := d.FindImage(ctx, idOrName) + s.imageOpsMu.Lock() + defer s.imageOpsMu.Unlock() + img, err := s.FindImage(ctx, idOrName) if err != nil { return model.Image{}, err } - vms, err := d.store.FindVMsUsingImage(ctx, img.ID) + vms, err := s.store.FindVMsUsingImage(ctx, img.ID) if err != nil { return model.Image{}, err } if len(vms) > 0 { return model.Image{}, fmt.Errorf("image %s is still referenced by %d VM(s)", img.Name, len(vms)) } - if err := d.store.DeleteImage(ctx, img.ID); err != nil { + if err := s.store.DeleteImage(ctx, img.ID); err != nil { return model.Image{}, err } return img, nil @@ -253,7 +253,7 @@ func firstNonEmpty(values ...string) string { // 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) { +func (s *ImageService) 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) @@ -263,7 +263,7 @@ func (d *Daemon) resolveKernelInputs(ctx context.Context, kernelRef, kernelPath, if kernelPath != "" || initrdPath != "" || modulesDir != "" { return "", "", "", fmt.Errorf("--kernel-ref is mutually exclusive with --kernel/--initrd/--modules") } - entry, err := d.readOrAutoPullKernel(ctx, kernelRef) + entry, err := s.readOrAutoPullKernel(ctx, kernelRef) if err != nil { return "", "", "", err } @@ -278,8 +278,8 @@ func (d *Daemon) resolveKernelInputs(ctx context.Context, kernelRef, kernelPath, // 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) +func (s *ImageService) readOrAutoPullKernel(ctx context.Context, kernelRef string) (kernelcat.Entry, error) { + entry, err := kernelcat.ReadLocal(s.layout.KernelsDir, kernelRef) if err == nil { return entry, nil } @@ -294,8 +294,8 @@ func (d *Daemon) readOrAutoPullKernel(ctx context.Context, kernelRef string) (ke 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 { + if _, pullErr := s.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) + return kernelcat.ReadLocal(s.layout.KernelsDir, kernelRef) } diff --git a/internal/daemon/images_pull.go b/internal/daemon/images_pull.go index a97f893..26b0b7c 100644 --- a/internal/daemon/images_pull.go +++ b/internal/daemon/images_pull.go @@ -44,7 +44,7 @@ const minPullExt4Size int64 = 1 << 30 // 1 GiB // staging dir to the final artifact dir, insert the store row. If two // pulls race to the same name, the loser fails fast at the recheck // and its staging dir is cleaned up via defer. -func (d *Daemon) PullImage(ctx context.Context, params api.ImagePullParams) (model.Image, error) { +func (s *ImageService) PullImage(ctx context.Context, params api.ImagePullParams) (model.Image, error) { ref := strings.TrimSpace(params.Ref) if ref == "" { return model.Image{}, errors.New("reference is required") @@ -55,9 +55,9 @@ func (d *Daemon) PullImage(ctx context.Context, params api.ImagePullParams) (mod return model.Image{}, fmt.Errorf("load image catalog: %w", err) } if entry, lookupErr := catalog.Lookup(ref); lookupErr == nil { - return d.pullFromBundle(ctx, params, entry) + return s.pullFromBundle(ctx, params, entry) } - return d.pullFromOCI(ctx, params) + return s.pullFromOCI(ctx, params) } // publishImage is the narrow critical section shared by every image- @@ -71,11 +71,11 @@ func (d *Daemon) PullImage(ctx context.Context, params api.ImagePullParams) (mod // in place, e.g. RegisterImage which only touches the store). When // non-empty the rename is the publication atom: finalDir must not // already exist before the rename fires. -func (d *Daemon) publishImage(ctx context.Context, image model.Image, stagingDir, finalDir string) (model.Image, error) { - d.imageOpsMu.Lock() - defer d.imageOpsMu.Unlock() +func (s *ImageService) publishImage(ctx context.Context, image model.Image, stagingDir, finalDir string) (model.Image, error) { + s.imageOpsMu.Lock() + defer s.imageOpsMu.Unlock() - if existing, err := d.store.GetImageByName(ctx, image.Name); err == nil { + if existing, err := s.store.GetImageByName(ctx, image.Name); err == nil { return model.Image{}, fmt.Errorf("image %q already exists (id=%s); pick a different --name or delete it first", image.Name, existing.ID) } if finalDir != "" { @@ -83,7 +83,7 @@ func (d *Daemon) publishImage(ctx context.Context, image model.Image, stagingDir return model.Image{}, fmt.Errorf("publish artifact dir: %w", err) } } - if err := d.store.UpsertImage(ctx, image); err != nil { + if err := s.store.UpsertImage(ctx, image); err != nil { if finalDir != "" { _ = os.RemoveAll(finalDir) } @@ -94,7 +94,7 @@ func (d *Daemon) publishImage(ctx context.Context, image model.Image, stagingDir // pullFromOCI is the original OCI-registry-pull path. See PullImage for // the intent. -func (d *Daemon) pullFromOCI(ctx context.Context, params api.ImagePullParams) (image model.Image, err error) { +func (s *ImageService) pullFromOCI(ctx context.Context, params api.ImagePullParams) (image model.Image, err error) { ref := strings.TrimSpace(params.Ref) parsed, err := name.ParseReference(ref) if err != nil { @@ -108,11 +108,11 @@ func (d *Daemon) pullFromOCI(ctx context.Context, params api.ImagePullParams) (i return model.Image{}, errors.New("could not derive image name from ref; pass --name") } } - if existing, lookupErr := d.store.GetImageByName(ctx, imgName); lookupErr == nil { + if existing, lookupErr := s.store.GetImageByName(ctx, imgName); lookupErr == nil { 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(ctx, params.KernelRef, params.KernelPath, params.InitrdPath, params.ModulesDir) + kernelPath, initrdPath, modulesDir, err := s.resolveKernelInputs(ctx, params.KernelRef, params.KernelPath, params.InitrdPath, params.ModulesDir) if err != nil { return model.Image{}, err } @@ -124,7 +124,7 @@ func (d *Daemon) pullFromOCI(ctx context.Context, params api.ImagePullParams) (i if err != nil { return model.Image{}, err } - finalDir := filepath.Join(d.layout.ImagesDir, id) + finalDir := filepath.Join(s.layout.ImagesDir, id) stagingDir := finalDir + ".staging" if err := os.MkdirAll(stagingDir, 0o755); err != nil { return model.Image{}, err @@ -144,7 +144,7 @@ func (d *Daemon) pullFromOCI(ctx context.Context, params api.ImagePullParams) (i } defer os.RemoveAll(rootfsTree) - meta, err := d.runPullAndFlatten(ctx, ref, d.layout.OCICacheDir, rootfsTree) + meta, err := s.runPullAndFlatten(ctx, ref, s.layout.OCICacheDir, rootfsTree) if err != nil { return model.Image{}, fmt.Errorf("pull oci image: %w", err) } @@ -162,14 +162,14 @@ func (d *Daemon) pullFromOCI(ctx context.Context, params api.ImagePullParams) (i } rootfsExt4 := filepath.Join(stagingDir, "rootfs.ext4") - if err := imagepull.BuildExt4(ctx, d.runner, rootfsTree, rootfsExt4, sizeBytes); err != nil { + if err := imagepull.BuildExt4(ctx, s.runner, rootfsTree, rootfsExt4, sizeBytes); err != nil { return model.Image{}, fmt.Errorf("build rootfs ext4: %w", err) } - if err := d.runFinalizePulledRootfs(ctx, rootfsExt4, meta); err != nil { + if err := s.runFinalizePulledRootfs(ctx, rootfsExt4, meta); err != nil { return model.Image{}, err } - stagedKernel, stagedInitrd, stagedModules, err := imagemgr.StageBootArtifacts(ctx, d.runner, stagingDir, kernelPath, initrdPath, modulesDir) + stagedKernel, stagedInitrd, stagedModules, err := imagemgr.StageBootArtifacts(ctx, s.runner, stagingDir, kernelPath, initrdPath, modulesDir) if err != nil { return model.Image{}, fmt.Errorf("stage boot artifacts: %w", err) } @@ -187,7 +187,7 @@ func (d *Daemon) pullFromOCI(ctx context.Context, params api.ImagePullParams) (i CreatedAt: now, UpdatedAt: now, } - published, err := d.publishImage(ctx, image, stagingDir, finalDir) + published, err := s.publishImage(ctx, image, stagingDir, finalDir) if err != nil { return model.Image{}, err } @@ -200,12 +200,12 @@ func (d *Daemon) pullFromOCI(ctx context.Context, params api.ImagePullParams) (i // injected at build time), verify its sha256, and register the result // as a managed image. No flatten / mkfs / debugfs work on the daemon // host. -func (d *Daemon) pullFromBundle(ctx context.Context, params api.ImagePullParams, entry imagecat.CatEntry) (image model.Image, err error) { +func (s *ImageService) pullFromBundle(ctx context.Context, params api.ImagePullParams, entry imagecat.CatEntry) (image model.Image, err error) { imgName := strings.TrimSpace(params.Name) if imgName == "" { imgName = entry.Name } - if existing, lookupErr := d.store.GetImageByName(ctx, imgName); lookupErr == nil { + if existing, lookupErr := s.store.GetImageByName(ctx, imgName); lookupErr == nil { return model.Image{}, fmt.Errorf("image %q already exists (id=%s); pick a different --name or delete it first", imgName, existing.ID) } @@ -214,7 +214,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(ctx, kernelRef, params.KernelPath, params.InitrdPath, params.ModulesDir) + kernelPath, initrdPath, modulesDir, err := s.resolveKernelInputs(ctx, kernelRef, params.KernelPath, params.InitrdPath, params.ModulesDir) if err != nil { return model.Image{}, err } @@ -226,7 +226,7 @@ func (d *Daemon) pullFromBundle(ctx context.Context, params api.ImagePullParams, if err != nil { return model.Image{}, err } - finalDir := filepath.Join(d.layout.ImagesDir, id) + finalDir := filepath.Join(s.layout.ImagesDir, id) stagingDir := finalDir + ".staging" if err := os.MkdirAll(stagingDir, 0o755); err != nil { return model.Image{}, err @@ -238,7 +238,7 @@ func (d *Daemon) pullFromBundle(ctx context.Context, params api.ImagePullParams, } }() - if _, err := d.runBundleFetch(ctx, stagingDir, entry); err != nil { + if _, err := s.runBundleFetch(ctx, stagingDir, entry); err != nil { return model.Image{}, fmt.Errorf("fetch bundle: %w", err) } // manifest.json is metadata we only need at fetch time; strip it @@ -246,7 +246,7 @@ func (d *Daemon) pullFromBundle(ctx context.Context, params api.ImagePullParams, _ = os.Remove(filepath.Join(stagingDir, imagecat.ManifestFilename)) rootfsExt4 := filepath.Join(stagingDir, imagecat.RootfsFilename) - stagedKernel, stagedInitrd, stagedModules, err := imagemgr.StageBootArtifacts(ctx, d.runner, stagingDir, kernelPath, initrdPath, modulesDir) + stagedKernel, stagedInitrd, stagedModules, err := imagemgr.StageBootArtifacts(ctx, s.runner, stagingDir, kernelPath, initrdPath, modulesDir) if err != nil { return model.Image{}, fmt.Errorf("stage boot artifacts: %w", err) } @@ -264,7 +264,7 @@ func (d *Daemon) pullFromBundle(ctx context.Context, params api.ImagePullParams, CreatedAt: now, UpdatedAt: now, } - published, err := d.publishImage(ctx, image, stagingDir, finalDir) + published, err := s.publishImage(ctx, image, stagingDir, finalDir) if err != nil { return model.Image{}, err } @@ -273,17 +273,17 @@ func (d *Daemon) pullFromBundle(ctx context.Context, params api.ImagePullParams, } // runBundleFetch is the seam tests substitute. nil → real implementation. -func (d *Daemon) runBundleFetch(ctx context.Context, destDir string, entry imagecat.CatEntry) (imagecat.Manifest, error) { - if d.bundleFetch != nil { - return d.bundleFetch(ctx, destDir, entry) +func (s *ImageService) runBundleFetch(ctx context.Context, destDir string, entry imagecat.CatEntry) (imagecat.Manifest, error) { + if s.bundleFetch != nil { + return s.bundleFetch(ctx, destDir, entry) } return imagecat.Fetch(ctx, nil, destDir, entry) } // runPullAndFlatten is the seam tests substitute. nil → real implementation. -func (d *Daemon) runPullAndFlatten(ctx context.Context, ref, cacheDir, destDir string) (imagepull.Metadata, error) { - if d.pullAndFlatten != nil { - return d.pullAndFlatten(ctx, ref, cacheDir, destDir) +func (s *ImageService) runPullAndFlatten(ctx context.Context, ref, cacheDir, destDir string) (imagepull.Metadata, error) { + if s.pullAndFlatten != nil { + return s.pullAndFlatten(ctx, ref, cacheDir, destDir) } pulled, err := imagepull.Pull(ctx, ref, cacheDir) if err != nil { @@ -293,21 +293,21 @@ func (d *Daemon) runPullAndFlatten(ctx context.Context, ref, cacheDir, destDir s } // runFinalizePulledRootfs applies ownership fixup and injects banger's -// guest agents. Tests substitute via d.finalizePulledRootfs; nil → +// guest agents. Tests substitute via s.finalizePulledRootfs; nil → // real implementation using debugfs + the companion vsock-agent // binary resolved via paths.CompanionBinaryPath. -func (d *Daemon) runFinalizePulledRootfs(ctx context.Context, ext4File string, meta imagepull.Metadata) error { - if d.finalizePulledRootfs != nil { - return d.finalizePulledRootfs(ctx, ext4File, meta) +func (s *ImageService) runFinalizePulledRootfs(ctx context.Context, ext4File string, meta imagepull.Metadata) error { + if s.finalizePulledRootfs != nil { + return s.finalizePulledRootfs(ctx, ext4File, meta) } - if err := imagepull.ApplyOwnership(ctx, d.runner, ext4File, meta); err != nil { + if err := imagepull.ApplyOwnership(ctx, s.runner, ext4File, meta); err != nil { return fmt.Errorf("apply ownership: %w", err) } vsockBin, err := paths.CompanionBinaryPath("banger-vsock-agent") if err != nil { return fmt.Errorf("locate vsock agent binary: %w", err) } - if err := imagepull.InjectGuestAgents(ctx, d.runner, ext4File, imagepull.GuestAgentAssets{ + if err := imagepull.InjectGuestAgents(ctx, s.runner, ext4File, imagepull.GuestAgentAssets{ VsockAgentBin: vsockBin, }); err != nil { return fmt.Errorf("inject guest agents: %w", err) diff --git a/internal/daemon/images_pull_bundle_test.go b/internal/daemon/images_pull_bundle_test.go index f816a09..5130127 100644 --- a/internal/daemon/images_pull_bundle_test.go +++ b/internal/daemon/images_pull_bundle_test.go @@ -63,9 +63,14 @@ func TestPullImageBundlePathRegistersFromCatalog(t *testing.T) { seedKernel(t, kernelsDir, "generic-6.12") d := &Daemon{ - layout: paths.Layout{ImagesDir: imagesDir, KernelsDir: kernelsDir}, - store: openDaemonStore(t), - runner: system.NewRunner(), + layout: paths.Layout{ImagesDir: imagesDir, KernelsDir: kernelsDir}, + store: openDaemonStore(t), + runner: system.NewRunner(), + } + d.img = &ImageService{ + layout: d.layout, + store: d.store, + runner: d.runner, bundleFetch: stubBundleFetch(imagecat.Manifest{KernelRef: "generic-6.12"}), } @@ -77,7 +82,7 @@ func TestPullImageBundlePathRegistersFromCatalog(t *testing.T) { TarballURL: "https://example.com/x.tar.zst", TarballSHA256: "abc", } - image, err := d.pullFromBundle(context.Background(), api.ImagePullParams{Ref: "debian-bookworm"}, entry) + image, err := d.img.pullFromBundle(context.Background(), api.ImagePullParams{Ref: "debian-bookworm"}, entry) if err != nil { t.Fatalf("pullFromBundle: %v", err) } @@ -111,9 +116,14 @@ func TestPullImageBundlePathOverrideNameAndKernelRef(t *testing.T) { } d := &Daemon{ - layout: paths.Layout{ImagesDir: imagesDir, KernelsDir: kernelsDir}, - store: openDaemonStore(t), - runner: system.NewRunner(), + layout: paths.Layout{ImagesDir: imagesDir, KernelsDir: kernelsDir}, + store: openDaemonStore(t), + runner: system.NewRunner(), + } + d.img = &ImageService{ + layout: d.layout, + store: d.store, + runner: d.runner, bundleFetch: stubBundleFetch(imagecat.Manifest{KernelRef: "generic-6.12"}), } @@ -123,7 +133,7 @@ func TestPullImageBundlePathOverrideNameAndKernelRef(t *testing.T) { TarballURL: "https://example.com/x.tar.zst", TarballSHA256: "abc", } - image, err := d.pullFromBundle(context.Background(), api.ImagePullParams{ + image, err := d.img.pullFromBundle(context.Background(), api.ImagePullParams{ Ref: "debian-bookworm", Name: "my-sandbox", KernelRef: "custom-kernel", }, entry) if err != nil { @@ -147,9 +157,14 @@ func TestPullImageBundlePathRejectsExistingName(t *testing.T) { seedKernel(t, kernelsDir, "generic-6.12") d := &Daemon{ - layout: paths.Layout{ImagesDir: imagesDir, KernelsDir: kernelsDir}, - store: openDaemonStore(t), - runner: system.NewRunner(), + layout: paths.Layout{ImagesDir: imagesDir, KernelsDir: kernelsDir}, + store: openDaemonStore(t), + runner: system.NewRunner(), + } + d.img = &ImageService{ + layout: d.layout, + store: d.store, + runner: d.runner, bundleFetch: stubBundleFetch(imagecat.Manifest{KernelRef: "generic-6.12"}), } id, _ := model.NewID() @@ -160,7 +175,7 @@ func TestPullImageBundlePathRejectsExistingName(t *testing.T) { t.Fatal(err) } - _, err := d.pullFromBundle(context.Background(), api.ImagePullParams{Ref: "debian-bookworm"}, imagecat.CatEntry{ + _, err := d.img.pullFromBundle(context.Background(), api.ImagePullParams{Ref: "debian-bookworm"}, imagecat.CatEntry{ Name: "debian-bookworm", KernelRef: "generic-6.12", TarballURL: "https://example.com/x.tar.zst", TarballSHA256: "abc", }) @@ -171,13 +186,18 @@ func TestPullImageBundlePathRejectsExistingName(t *testing.T) { func TestPullImageBundlePathRequiresSomeKernelSource(t *testing.T) { d := &Daemon{ - layout: paths.Layout{ImagesDir: t.TempDir(), KernelsDir: t.TempDir()}, - store: openDaemonStore(t), - runner: system.NewRunner(), + layout: paths.Layout{ImagesDir: t.TempDir(), KernelsDir: t.TempDir()}, + store: openDaemonStore(t), + runner: system.NewRunner(), + } + d.img = &ImageService{ + layout: d.layout, + store: d.store, + runner: d.runner, bundleFetch: stubBundleFetch(imagecat.Manifest{}), } // Catalog entry has no kernel_ref, no --kernel-ref/--kernel passed. - _, err := d.pullFromBundle(context.Background(), api.ImagePullParams{Ref: "x"}, imagecat.CatEntry{ + _, err := d.img.pullFromBundle(context.Background(), api.ImagePullParams{Ref: "x"}, imagecat.CatEntry{ Name: "x", TarballURL: "https://example.com/x.tar.zst", TarballSHA256: "abc", }) if err == nil || !strings.Contains(err.Error(), "kernel") { @@ -194,11 +214,16 @@ func TestPullImageBundleFetchFailurePropagates(t *testing.T) { layout: paths.Layout{ImagesDir: imagesDir, KernelsDir: kernelsDir}, store: openDaemonStore(t), runner: system.NewRunner(), + } + d.img = &ImageService{ + layout: d.layout, + store: d.store, + runner: d.runner, bundleFetch: func(_ context.Context, _ string, _ imagecat.CatEntry) (imagecat.Manifest, error) { return imagecat.Manifest{}, errors.New("r2 exploded") }, } - _, err := d.pullFromBundle(context.Background(), api.ImagePullParams{Ref: "x"}, imagecat.CatEntry{ + _, err := d.img.pullFromBundle(context.Background(), api.ImagePullParams{Ref: "x"}, imagecat.CatEntry{ Name: "x", KernelRef: "generic-6.12", TarballURL: "https://example.com/x.tar.zst", TarballSHA256: "abc", }) @@ -222,6 +247,11 @@ func TestPullImageDispatchFallsThroughToOCIWhenNoCatalogHit(t *testing.T) { layout: paths.Layout{ImagesDir: imagesDir, KernelsDir: kernelsDir, OCICacheDir: t.TempDir()}, store: openDaemonStore(t), runner: system.NewRunner(), + } + d.img = &ImageService{ + layout: d.layout, + store: d.store, + runner: d.runner, pullAndFlatten: func(_ context.Context, ref, _ string, destDir string) (imagepull.Metadata, error) { ociCalled = true if err := os.WriteFile(filepath.Join(destDir, "marker"), []byte("x"), 0o644); err != nil { @@ -233,7 +263,7 @@ func TestPullImageDispatchFallsThroughToOCIWhenNoCatalogHit(t *testing.T) { bundleFetch: stubBundleFetch(imagecat.Manifest{}), } - _, err := d.PullImage(context.Background(), api.ImagePullParams{ + _, err := d.img.PullImage(context.Background(), api.ImagePullParams{ // Not a catalog name (catalog is empty in the embedded default). Ref: "docker.io/library/debian:bookworm", KernelRef: "generic-6.12", diff --git a/internal/daemon/images_pull_test.go b/internal/daemon/images_pull_test.go index 6d89631..41acd06 100644 --- a/internal/daemon/images_pull_test.go +++ b/internal/daemon/images_pull_test.go @@ -71,14 +71,19 @@ func TestPullImageHappyPath(t *testing.T) { kernel, initrd, modules := writeFakeKernelTriple(t) d := &Daemon{ - layout: paths.Layout{ImagesDir: imagesDir, OCICacheDir: cacheDir}, - store: openDaemonStore(t), - runner: system.NewRunner(), + layout: paths.Layout{ImagesDir: imagesDir, OCICacheDir: cacheDir}, + store: openDaemonStore(t), + runner: system.NewRunner(), + } + d.img = &ImageService{ + layout: d.layout, + store: d.store, + runner: d.runner, pullAndFlatten: stubPullAndFlatten, finalizePulledRootfs: stubFinalizePulledRootfs, } - image, err := d.PullImage(context.Background(), api.ImagePullParams{ + image, err := d.img.PullImage(context.Background(), api.ImagePullParams{ Ref: "docker.io/library/debian:bookworm", KernelPath: kernel, InitrdPath: initrd, @@ -116,9 +121,14 @@ func TestPullImageRejectsExistingName(t *testing.T) { kernel, _, _ := writeFakeKernelTriple(t) d := &Daemon{ - layout: paths.Layout{ImagesDir: imagesDir, OCICacheDir: t.TempDir()}, - store: openDaemonStore(t), - runner: system.NewRunner(), + layout: paths.Layout{ImagesDir: imagesDir, OCICacheDir: t.TempDir()}, + store: openDaemonStore(t), + runner: system.NewRunner(), + } + d.img = &ImageService{ + layout: d.layout, + store: d.store, + runner: d.runner, pullAndFlatten: stubPullAndFlatten, finalizePulledRootfs: stubFinalizePulledRootfs, } @@ -133,7 +143,7 @@ func TestPullImageRejectsExistingName(t *testing.T) { t.Fatal(err) } - _, err := d.PullImage(context.Background(), api.ImagePullParams{ + _, err := d.img.PullImage(context.Background(), api.ImagePullParams{ Ref: "docker.io/library/debian:bookworm", KernelPath: kernel, }) @@ -144,13 +154,18 @@ func TestPullImageRejectsExistingName(t *testing.T) { func TestPullImageRequiresKernel(t *testing.T) { d := &Daemon{ - layout: paths.Layout{ImagesDir: t.TempDir(), OCICacheDir: t.TempDir()}, - store: openDaemonStore(t), - runner: system.NewRunner(), + layout: paths.Layout{ImagesDir: t.TempDir(), OCICacheDir: t.TempDir()}, + store: openDaemonStore(t), + runner: system.NewRunner(), + } + d.img = &ImageService{ + layout: d.layout, + store: d.store, + runner: d.runner, pullAndFlatten: stubPullAndFlatten, finalizePulledRootfs: stubFinalizePulledRootfs, } - _, err := d.PullImage(context.Background(), api.ImagePullParams{ + _, err := d.img.PullImage(context.Background(), api.ImagePullParams{ Ref: "docker.io/library/debian:bookworm", }) if err == nil || !strings.Contains(err.Error(), "kernel") { @@ -166,13 +181,18 @@ func TestPullImageCleansStagingOnFailure(t *testing.T) { } d := &Daemon{ - layout: paths.Layout{ImagesDir: imagesDir, OCICacheDir: t.TempDir()}, - store: openDaemonStore(t), - runner: system.NewRunner(), + layout: paths.Layout{ImagesDir: imagesDir, OCICacheDir: t.TempDir()}, + store: openDaemonStore(t), + runner: system.NewRunner(), + } + d.img = &ImageService{ + layout: d.layout, + store: d.store, + runner: d.runner, pullAndFlatten: failureSeam, finalizePulledRootfs: stubFinalizePulledRootfs, } - _, err := d.PullImage(context.Background(), api.ImagePullParams{ + _, err := d.img.PullImage(context.Background(), api.ImagePullParams{ Ref: "docker.io/library/debian:bookworm", KernelPath: kernel, }) diff --git a/internal/daemon/kernels.go b/internal/daemon/kernels.go index 39f0196..1f5e938 100644 --- a/internal/daemon/kernels.go +++ b/internal/daemon/kernels.go @@ -14,8 +14,8 @@ import ( "banger/internal/system" ) -func (d *Daemon) KernelList(_ context.Context) (api.KernelListResult, error) { - entries, err := kernelcat.ListLocal(d.layout.KernelsDir) +func (s *ImageService) KernelList(_ context.Context) (api.KernelListResult, error) { + entries, err := kernelcat.ListLocal(s.layout.KernelsDir) if err != nil { return api.KernelListResult{}, err } @@ -26,19 +26,19 @@ func (d *Daemon) KernelList(_ context.Context) (api.KernelListResult, error) { return result, nil } -func (d *Daemon) KernelShow(_ context.Context, name string) (api.KernelEntry, error) { - entry, err := kernelcat.ReadLocal(d.layout.KernelsDir, name) +func (s *ImageService) KernelShow(_ context.Context, name string) (api.KernelEntry, error) { + entry, err := kernelcat.ReadLocal(s.layout.KernelsDir, name) if err != nil { return api.KernelEntry{}, kernelNotFoundIfMissing(name, err) } return kernelEntryToAPI(entry), nil } -func (d *Daemon) KernelDelete(_ context.Context, name string) error { +func (s *ImageService) KernelDelete(_ context.Context, name string) error { if err := kernelcat.ValidateName(name); err != nil { return err } - return kernelcat.DeleteLocal(d.layout.KernelsDir, name) + return kernelcat.DeleteLocal(s.layout.KernelsDir, name) } // KernelImport copies the kernel / initrd / modules artifacts produced by @@ -46,7 +46,7 @@ func (d *Daemon) KernelDelete(_ context.Context, name string) error { // under params.Name and writes the manifest. It is the primary bridge from // "I built a kernel with the helper scripts" to "banger kernel list shows // it and image register --kernel-ref works." -func (d *Daemon) KernelImport(ctx context.Context, params api.KernelImportParams) (api.KernelEntry, error) { +func (s *ImageService) KernelImport(ctx context.Context, params api.KernelImportParams) (api.KernelEntry, error) { name := strings.TrimSpace(params.Name) if err := kernelcat.ValidateName(name); err != nil { return api.KernelEntry{}, err @@ -61,9 +61,9 @@ func (d *Daemon) KernelImport(ctx context.Context, params api.KernelImportParams return api.KernelEntry{}, fmt.Errorf("discover artifacts under %s: %w", fromDir, err) } - targetDir := kernelcat.EntryDir(d.layout.KernelsDir, name) + targetDir := kernelcat.EntryDir(s.layout.KernelsDir, name) // Overwrite-by-default: clear any prior entry so a re-import is clean. - if err := kernelcat.DeleteLocal(d.layout.KernelsDir, name); err != nil { + if err := kernelcat.DeleteLocal(s.layout.KernelsDir, name); err != nil { return api.KernelEntry{}, fmt.Errorf("clear prior catalog entry %q: %w", name, err) } if err := os.MkdirAll(targetDir, 0o755); err != nil { @@ -85,7 +85,7 @@ func (d *Daemon) KernelImport(ctx context.Context, params api.KernelImportParams if err := os.MkdirAll(modulesTarget, 0o755); err != nil { return api.KernelEntry{}, err } - if err := system.CopyDirContents(ctx, d.runner, discovered.ModulesDir, modulesTarget, false); err != nil { + if err := system.CopyDirContents(ctx, s.runner, discovered.ModulesDir, modulesTarget, false); err != nil { return api.KernelEntry{}, fmt.Errorf("copy modules: %w", err) } } @@ -104,10 +104,10 @@ func (d *Daemon) KernelImport(ctx context.Context, params api.KernelImportParams Source: "import:" + fromDir, ImportedAt: time.Now().UTC(), } - if err := kernelcat.WriteLocal(d.layout.KernelsDir, entry); err != nil { + if err := kernelcat.WriteLocal(s.layout.KernelsDir, entry); err != nil { return api.KernelEntry{}, fmt.Errorf("write manifest: %w", err) } - stored, err := kernelcat.ReadLocal(d.layout.KernelsDir, name) + stored, err := kernelcat.ReadLocal(s.layout.KernelsDir, name) if err != nil { return api.KernelEntry{}, err } @@ -116,14 +116,14 @@ func (d *Daemon) KernelImport(ctx context.Context, params api.KernelImportParams // KernelPull downloads a catalog entry by name into the local catalog. It // refuses to overwrite an existing entry unless params.Force is set. -func (d *Daemon) KernelPull(ctx context.Context, params api.KernelPullParams) (api.KernelEntry, error) { +func (s *ImageService) KernelPull(ctx context.Context, params api.KernelPullParams) (api.KernelEntry, error) { name := strings.TrimSpace(params.Name) if err := kernelcat.ValidateName(name); err != nil { return api.KernelEntry{}, err } if !params.Force { - if _, err := kernelcat.ReadLocal(d.layout.KernelsDir, name); err == nil { + if _, err := kernelcat.ReadLocal(s.layout.KernelsDir, name); err == nil { return api.KernelEntry{}, fmt.Errorf("kernel %q already pulled; pass --force to re-pull", name) } else if !os.IsNotExist(err) { return api.KernelEntry{}, err @@ -139,7 +139,7 @@ func (d *Daemon) KernelPull(ctx context.Context, params api.KernelPullParams) (a return api.KernelEntry{}, fmt.Errorf("kernel %q not in catalog (run 'banger kernel list --available' to browse)", name) } - stored, err := kernelcat.Fetch(ctx, nil, d.layout.KernelsDir, catEntry) + stored, err := kernelcat.Fetch(ctx, nil, s.layout.KernelsDir, catEntry) if err != nil { return api.KernelEntry{}, err } @@ -148,12 +148,12 @@ func (d *Daemon) KernelPull(ctx context.Context, params api.KernelPullParams) (a // KernelCatalog returns every entry from the embedded catalog annotated // with whether it has already been pulled locally. -func (d *Daemon) KernelCatalog(_ context.Context) (api.KernelCatalogResult, error) { +func (s *ImageService) KernelCatalog(_ context.Context) (api.KernelCatalogResult, error) { catalog, err := kernelcat.LoadEmbedded() if err != nil { return api.KernelCatalogResult{}, err } - local, _ := kernelcat.ListLocal(d.layout.KernelsDir) + local, _ := kernelcat.ListLocal(s.layout.KernelsDir) pulled := make(map[string]bool, len(local)) for _, entry := range local { pulled[entry.Name] = true diff --git a/internal/daemon/kernels_test.go b/internal/daemon/kernels_test.go index 7179b5f..ac6cbb3 100644 --- a/internal/daemon/kernels_test.go +++ b/internal/daemon/kernels_test.go @@ -38,7 +38,7 @@ func TestKernelListReturnsSeededEntries(t *testing.T) { seedKernelEntry(t, kernelsDir, "alpine-3.23") d := &Daemon{layout: paths.Layout{KernelsDir: kernelsDir}} - result, err := d.KernelList(context.Background()) + result, err := d.imageSvc().KernelList(context.Background()) if err != nil { t.Fatalf("KernelList: %v", err) } @@ -86,7 +86,7 @@ func TestKernelShowAndDeleteThroughDispatch(t *testing.T) { func TestKernelShowMissingEntry(t *testing.T) { d := &Daemon{layout: paths.Layout{KernelsDir: t.TempDir()}} - _, err := d.KernelShow(context.Background(), "nope") + _, err := d.imageSvc().KernelShow(context.Background(), "nope") if err == nil || !strings.Contains(err.Error(), "not found") { t.Fatalf("KernelShow missing: err=%v", err) } @@ -94,7 +94,7 @@ func TestKernelShowMissingEntry(t *testing.T) { func TestKernelDeleteRejectsInvalidName(t *testing.T) { d := &Daemon{layout: paths.Layout{KernelsDir: t.TempDir()}} - if err := d.KernelDelete(context.Background(), "../escape"); err == nil { + if err := d.imageSvc().KernelDelete(context.Background(), "../escape"); err == nil { t.Fatalf("KernelDelete should reject traversal") } } @@ -113,7 +113,7 @@ func TestRegisterImageResolvesKernelRef(t *testing.T) { store: openDaemonStore(t), } - image, err := d.RegisterImage(context.Background(), api.ImageRegisterParams{ + image, err := d.imageSvc().RegisterImage(context.Background(), api.ImageRegisterParams{ Name: "testbox", RootfsPath: rootfs, KernelRef: "void-6.12", @@ -139,7 +139,7 @@ func TestRegisterImageRejectsKernelRefAndPath(t *testing.T) { layout: paths.Layout{KernelsDir: kernelsDir}, store: openDaemonStore(t), } - _, err := d.RegisterImage(context.Background(), api.ImageRegisterParams{ + _, err := d.imageSvc().RegisterImage(context.Background(), api.ImageRegisterParams{ Name: "testbox", RootfsPath: rootfs, KernelRef: "void-6.12", @@ -175,7 +175,7 @@ func TestKernelImportCopiesArtifactsAndWritesManifest(t *testing.T) { runner: system.NewRunner(), } - entry, err := d.KernelImport(context.Background(), api.KernelImportParams{ + entry, err := d.imageSvc().KernelImport(context.Background(), api.KernelImportParams{ Name: "void-6.12", FromDir: src, Distro: "void", @@ -210,7 +210,7 @@ func TestKernelPullRejectsUnknownCatalogEntry(t *testing.T) { layout: paths.Layout{KernelsDir: t.TempDir()}, runner: system.NewRunner(), } - _, err := d.KernelPull(context.Background(), api.KernelPullParams{Name: "unknown"}) + _, err := d.imageSvc().KernelPull(context.Background(), api.KernelPullParams{Name: "unknown"}) if err == nil || !strings.Contains(err.Error(), "not in catalog") { t.Fatalf("KernelPull unknown: err=%v", err) } @@ -224,7 +224,7 @@ func TestKernelPullRefusesOverwriteWithoutForce(t *testing.T) { layout: paths.Layout{KernelsDir: kernelsDir}, runner: system.NewRunner(), } - _, err := d.KernelPull(context.Background(), api.KernelPullParams{Name: "void-6.12"}) + _, err := d.imageSvc().KernelPull(context.Background(), api.KernelPullParams{Name: "void-6.12"}) if err == nil || !strings.Contains(err.Error(), "already pulled") { t.Fatalf("KernelPull without --force: err=%v", err) } @@ -232,7 +232,7 @@ func TestKernelPullRefusesOverwriteWithoutForce(t *testing.T) { func TestKernelCatalogReportsPulledStatus(t *testing.T) { d := &Daemon{layout: paths.Layout{KernelsDir: t.TempDir()}} - result, err := d.KernelCatalog(context.Background()) + result, err := d.imageSvc().KernelCatalog(context.Background()) if err != nil { t.Fatalf("KernelCatalog: %v", err) } @@ -247,7 +247,7 @@ func TestKernelImportRejectsMissingFromDir(t *testing.T) { layout: paths.Layout{KernelsDir: t.TempDir()}, runner: system.NewRunner(), } - _, err := d.KernelImport(context.Background(), api.KernelImportParams{Name: "x"}) + _, err := d.imageSvc().KernelImport(context.Background(), api.KernelImportParams{Name: "x"}) if err == nil || !strings.Contains(err.Error(), "--from") { t.Fatalf("KernelImport without --from: err=%v", err) } @@ -262,7 +262,7 @@ func TestRegisterImageMissingKernelRef(t *testing.T) { layout: paths.Layout{KernelsDir: t.TempDir()}, store: openDaemonStore(t), } - _, err := d.RegisterImage(context.Background(), api.ImageRegisterParams{ + _, err := d.imageSvc().RegisterImage(context.Background(), api.ImageRegisterParams{ Name: "testbox", RootfsPath: rootfs, KernelRef: "never-imported", diff --git a/internal/daemon/vm_authsync.go b/internal/daemon/vm_authsync.go index ad21b46..7485f78 100644 --- a/internal/daemon/vm_authsync.go +++ b/internal/daemon/vm_authsync.go @@ -94,7 +94,7 @@ func (d *Daemon) ensureAuthorizedKeyOnWorkDisk(ctx context.Context, vm *model.VM } if prep.ClonedFromSeed && image.Managed { vmCreateStage(ctx, "prepare_work_disk", "refreshing managed work seed") - if err := d.refreshManagedWorkSeedFingerprint(ctx, image, fingerprint); err != nil { + if err := d.imageSvc().refreshManagedWorkSeedFingerprint(ctx, image, fingerprint); err != nil { return err } } diff --git a/internal/daemon/vm_create.go b/internal/daemon/vm_create.go index 08302c7..bb96816 100644 --- a/internal/daemon/vm_create.go +++ b/internal/daemon/vm_create.go @@ -175,7 +175,7 @@ func (d *Daemon) reserveVM(ctx context.Context, requestedName string, image mode // 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) + image, err := d.imageSvc().FindImage(ctx, idOrName) if err == nil { return image, nil } @@ -189,8 +189,8 @@ func (d *Daemon) findOrAutoPullImage(ctx context.Context, idOrName string) (mode 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 { + if _, pullErr := d.imageSvc().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) + return d.imageSvc().FindImage(ctx, idOrName) } diff --git a/internal/daemon/vm_disk.go b/internal/daemon/vm_disk.go index 4033f25..f03f8b1 100644 --- a/internal/daemon/vm_disk.go +++ b/internal/daemon/vm_disk.go @@ -190,12 +190,15 @@ func sshdGuestConfig() string { }, "\n") } -func (d *Daemon) flattenNestedWorkHome(ctx context.Context, workMount string) error { +// flattenNestedWorkHome is a package-level helper used by the image, +// workspace-sync, and VM-disk paths, so it takes the runner explicitly +// rather than belonging to any one service struct. +func flattenNestedWorkHome(ctx context.Context, runner system.CommandRunner, workMount string) error { nestedHome := filepath.Join(workMount, "root") if !exists(nestedHome) { return nil } - if _, err := d.runner.RunSudo(ctx, "chmod", "755", nestedHome); err != nil { + if _, err := runner.RunSudo(ctx, "chmod", "755", nestedHome); err != nil { return err } entries, err := os.ReadDir(nestedHome) @@ -204,10 +207,17 @@ func (d *Daemon) flattenNestedWorkHome(ctx context.Context, workMount string) er } for _, entry := range entries { sourcePath := filepath.Join(nestedHome, entry.Name()) - if _, err := d.runner.RunSudo(ctx, "cp", "-a", sourcePath, workMount+"/"); err != nil { + if _, err := runner.RunSudo(ctx, "cp", "-a", sourcePath, workMount+"/"); err != nil { return err } } - _, err = d.runner.RunSudo(ctx, "rm", "-rf", nestedHome) + _, err = runner.RunSudo(ctx, "rm", "-rf", nestedHome) return err } + +// Deprecated forwarder: until every caller learns the package-level +// helper, Daemon keeps a receiver-method form. Will be deleted once +// the last caller is rewritten. +func (d *Daemon) flattenNestedWorkHome(ctx context.Context, workMount string) error { + return flattenNestedWorkHome(ctx, d.runner, workMount) +}