[bug] fix microVM actors with one character container names

cmd/ateworker-microvm/internal/kata/overlay_linux.go

@@ -67,21 +67,21 @@ func SharedDir(id string) string {
 func VirtiofsdSocketPath(id string) string { return filepath.Join(VMDir(id), "virtiofsd.sock") }
 
 // UpperWorkDirs returns the HOST overlay upperdir and workdir for one container
-// under the actor's rootfs-upper base dir: SIBLING directories, <cid>/fs and
-// <cid>/work. Both properties are load-bearing — the kernel requires upperdir
+// under the actor's rootfs-upper base dir: SIBLING directories, <name>/fs and
+// <name>/work. Both properties are load-bearing — the kernel requires upperdir
 // and workdir on the same filesystem and rejects a nested workdir. The
 // snapshot only carries each upperdir's contents (see
 // cmd/ateworker-microvm/rootfsupper.go); ateworker creates this layout itself.
 // Covered by regression tests.
-func UpperWorkDirs(upperBase, containerID string) (upper, work string) {
-	return filepath.Join(upperBase, containerID, "fs"), filepath.Join(upperBase, containerID, "work")
+func UpperWorkDirs(upperBase, name string) (upper, work string) {
+	return filepath.Join(upperBase, name, "fs"), filepath.Join(upperBase, name, "work")
 }
 
 // MkdirUpperWorkDirs creates UpperWorkDirs under upperBase, which must exist.
-// Only root uses <cid> and the workdir, so they are 0700. The upperdir is
+// Only root uses <name> and the workdir, so they are 0700. The upperdir is
 // 0755: overlayfs presents its mode as the container's /.
-func MkdirUpperWorkDirs(upperBase, containerID string) (upper, work string, err error) {
-	upper, work = UpperWorkDirs(upperBase, containerID)
+func MkdirUpperWorkDirs(upperBase, name string) (upper, work string, err error) {
+	upper, work = UpperWorkDirs(upperBase, name)
 	for _, d := range []struct {
 		path string
 		mode os.FileMode
@@ -240,7 +240,7 @@ func StageImageVolume(ctx context.Context, src, id, cid, volumeName string) erro
 }
 
 // StageMergedRootfs mounts overlay(lower = the OCI image bundle rootfs, upper/work =
-// the actor's host rootfs-upper dirs for cid) at SharedDir(restoreID)/<cid>/rootfs —
+// the actor's host rootfs-upper dirs for name) at SharedDir(restoreID)/<cid>/rootfs —
 // the merged tree the ONE virtiofsd serves and the guest runs the container on
 // directly. The host kernel owns the overlay (the canonical ext4-upper case: no
 // special mount options, whiteouts/opaque markers are ordinary trusted.overlay.*
@@ -249,12 +249,12 @@ func StageImageVolume(ctx context.Context, src, id, cid, volumeName string) erro
 // The merged path is identical on every node — find-paths migration re-opens the
 // guest's open files by path — given a deterministic image unpack plus the upper
 // re-materialized from the snapshot tar (see cmd/ateworker-microvm/rootfsupper.go).
-func StageMergedRootfs(ctx context.Context, bundleRootfs, upperBase, restoreID, cid string) error {
-	if cid == "" {
-		return fmt.Errorf("StageMergedRootfs: empty container id")
+func StageMergedRootfs(ctx context.Context, bundleRootfs, upperBase, name, restoreID, cid string) error {
+	if name == "" || cid == "" {
+		return fmt.Errorf("StageMergedRootfs: empty container name or id")
 	}
 	dst := filepath.Join(SharedDir(restoreID), cid, "rootfs")
-	_, work := UpperWorkDirs(upperBase, cid)
+	_, work := UpperWorkDirs(upperBase, name)
 	// Drop any stale mount first (lazy if busy), then ensure clean mountpoints.
 	Unmount(dst)
 	// The workdir is scratch: wipe it so a volatile mount is never refused by a
@@ -265,7 +265,7 @@ func StageMergedRootfs(ctx context.Context, bundleRootfs, upperBase, restoreID,
 	if err := os.MkdirAll(dst, 0o755); err != nil {
 		return fmt.Errorf("creating %q: %w", dst, err)
 	}
-	upper, work, err := MkdirUpperWorkDirs(upperBase, cid)
+	upper, work, err := MkdirUpperWorkDirs(upperBase, name)
 	if err != nil {
 		return err
 	}

cmd/ateworker-microvm/restore.go

@@ -505,8 +505,8 @@ func (s *AteWorkerService) restoreMemoryFidelity(ctx context.Context, p actorBoo
 	// connection dialed after resume: the restored guest's containers are alive, so
 	// ReadStdout/ReadStderr pick up where they left off.
 	attribution := p.actorAttribution()
-	for _, c := range containers {
-		s.startActorLogForwarding(guestAC, attribution, c.GetName(), c.GetName())
+	for _, c := range ctrs {
+		s.startActorLogForwarding(guestAC, attribution, c.id, c.name)
 	}
 
 	if err := s.tunnel.Activate(p.attribution(), s.sandboxDialer(p.actorUID), egress); err != nil {

cmd/ateworker-microvm/rootfsupper.go

@@ -59,7 +59,7 @@ import (
 )
 
 // rootfsUpperTarFile is the snapshot file holding the tar of one container's
-// rootfs upperdir (<cid>/fs under rootfsUpperDir, see kata.UpperWorkDirs).
+// rootfs upperdir (<name>/fs under rootfsUpperDir, see kata.UpperWorkDirs).
 // Entries are relative to the upperdir, plus a root entry: overlayfs presents
 // the upperdir's own mode, ownership, and xattrs as the container's /. The workdir is never archived: with
 // index=off pinned on the mount (see kata.StageMergedRootfs) a restored

cmd/ateworker-microvm/run.go

@@ -166,23 +166,22 @@ const minGuestMemMiB = 128
 // workloads to signal and wait on, and it must name what the guest actually
 // runs: a container the agent does not know is rejected with InvalidContainerId,
 // and graceful shutdown then gives up without ever reaching the workload.
-//
-// Containers run under their bare name.
 func workloadIDs(ctrs []actorContainer) []string {
 	ids := make([]string, 0, len(ctrs))
 	for _, c := range ctrs {
-		ids = append(ids, c.name)
+		ids = append(ids, c.id)
 	}
 	return ids
 }
 
 // actorContainer is one of the actor's containers prepared for the shared micro-VM:
-// its name (also the kata containerID + the merged rootfs's find-paths subdir), its
+// its name, its kata container ID (also the merged rootfs's find-paths subdir), its
 // host OCI bundle and the bundle rootfs that backs the overlay lower, and its OCI
 // spec. The writable upper is a host directory (see rootfsupper.go); the host kernel
 // merges the two.
 type actorContainer struct {
 	name         string
+	id           string // see ocispec.GuestContainerID
 	bundle       string
 	bundleRootfs string
 	// spec is the container's OCI spec shaped for micro-VM execution.
@@ -608,14 +607,13 @@ func (s *AteWorkerService) coldBootActor(ctx context.Context, p actorBootParams)
 	s.setRunningVM(actorUID, ra)
 
 	// Forward each container's stdout/stderr into the pod logs, keyed by the
-	// container id (== the name; see StartRootfsContainer). The goroutines read
-	// over ac for the actor's lifetime and exit (io.EOF) when teardownActor
-	// closes ac.
+	// container id (see StartRootfsContainer). The goroutines read over ac for
+	// the actor's lifetime and exit (io.EOF) when teardownActor closes ac.
 	workloadIDs := make([]string, 0, len(ctrs))
 	attribution := p.actorAttribution()
 	for _, c := range ctrs {
-		s.startActorLogForwarding(ac, attribution, c.name, c.name)
-		workloadIDs = append(workloadIDs, c.name)
+		s.startActorLogForwarding(ac, attribution, c.id, c.name)
+		workloadIDs = append(workloadIDs, c.id)
 	}
 
 	// Publish the guest to GetWorkloadStats, past every error return above: a
@@ -644,7 +642,7 @@ func (s *AteWorkerService) buildActorContainers(actorUID string, actorDirs *atew
 		cn := c.GetName()
 		bundle := ociBundlePath(actorDirs, cn)
 		spec := ocispec.Build(ocispec.Options{ActorUID: actorUID, ActorDirs: actorDirs, Container: c})
-		if err := ocispec.ShapeMicroVM(spec, ocispec.MicroVMOptions{ActorDirs: actorDirs, ContainerID: cn}); err != nil {
+		if err := ocispec.ShapeMicroVM(spec, ocispec.MicroVMOptions{ActorDirs: actorDirs, ContainerName: cn}); err != nil {
 			return nil, fmt.Errorf("while shaping the OCI spec for %q: %w", cn, err)
 		}
 		// Compose the bundle rootfs from the node's cached image layers (an
@@ -663,6 +661,7 @@ func (s *AteWorkerService) buildActorContainers(actorUID string, actorDirs *atew
 		}
 		ctrs[i] = actorContainer{
 			name:         cn,
+			id:           ocispec.GuestContainerID(cn),
 			bundle:       bundle,
 			bundleRootfs: bundleRootfs,
 			spec:         spec,
@@ -684,12 +683,12 @@ func (s *AteWorkerService) buildActorContainers(actorUID string, actorDirs *atew
 func (s *AteWorkerService) stageMergedRootfs(ctx context.Context, rr resolvedRuntime, id string, actorDirs *ateworkerpb.ActorDirs, ctrs []actorContainer, containers []*ateworkerpb.Container, procAttr *syscall.SysProcAttr) (*exec.Cmd, error) {
 	upperBase := rootfsUpperDir(actorDirs)
 	for _, c := range ctrs {
-		if err := kata.StageMergedRootfs(ctx, c.bundleRootfs, upperBase, id, c.name); err != nil {
+		if err := kata.StageMergedRootfs(ctx, c.bundleRootfs, upperBase, c.name, id, c.id); err != nil {
 			return nil, fmt.Errorf("while staging merged rootfs for %q: %w", c.name, err)
 		}
 		for _, vm := range c.imageMounts {
 			src := imagecache.ImageVolumeMountPath(c.bundle, vm.GetName())
-			if err := kata.StageImageVolume(ctx, src, id, c.name, vm.GetName()); err != nil {
+			if err := kata.StageImageVolume(ctx, src, id, c.id, vm.GetName()); err != nil {
 				return nil, fmt.Errorf("while staging image volume %q for %q: %w", vm.GetName(), c.name, err)
 			}
 		}
@@ -892,19 +891,19 @@ func (s *AteWorkerService) startActorContainers(ctx context.Context, ac *kata.Ag
 }
 
 // startRootfsContainer brings up one container on its host-merged rootfs (the
-// stock kata flow: create + start against shared/<name>/rootfs). On failure it
+// stock kata flow: create + start against shared/<id>/rootfs). On failure it
 // dumps the guest's view of the shared tree.
 //
 // Its spec binds every declared volume at its mount path.
 func (s *AteWorkerService) startRootfsContainer(ctx context.Context, ac *kata.AgentClient, vsockPath string, c actorContainer) error {
 	cCtx, cancel := context.WithTimeout(ctx, 30*time.Second)
-	err := ac.StartRootfsContainer(cCtx, c.name, c.spec)
+	err := ac.StartRootfsContainer(cCtx, c.id, c.spec)
 	cancel()
 	if err != nil {
 		if s.guestDebug {
 			dump := kata.DebugConsoleDump(ctx, vsockPath,
 				"echo '== shared/containers =='; ls -la /run/kata-containers/shared/containers/ 2>&1 | head -40; "+
-					"echo '== rootfs =='; ls /run/kata-containers/shared/containers/"+c.name+"/rootfs/ 2>&1 | head; "+
+					"echo '== rootfs =='; ls /run/kata-containers/shared/containers/"+c.id+"/rootfs/ 2>&1 | head; "+
 					"echo '== mounts =='; grep -E 'kata|virtiofs' /proc/mounts 2>&1")
 			slog.ErrorContext(ctx, "rootfs container failed; dump", slog.String("container", c.name), slog.String("dump", dump))
 		}

cmd/ateworker-microvm/run_test.go

@@ -21,6 +21,7 @@ import (
 	"encoding/json"
 	"errors"
 	"net"
+	"os"
 	"path/filepath"
 	"runtime"
 	"slices"
@@ -29,6 +30,7 @@ import (
 	"time"
 
 	"go.ate.dev/substrate/cmd/ateworker-microvm/internal/kata"
+	"go.ate.dev/substrate/internal/proto/ateworkerpb"
 )
 
 // A vsock socket that has gone missing means cloud-hypervisor stopped the VM
@@ -201,9 +203,20 @@ func TestBuildVMConfigConsole(t *testing.T) {
 // does not know with InvalidContainerId — which aborts the whole graceful
 // shutdown, so a stale id here silently costs the guest its clean exit.
 func TestWorkloadIDs(t *testing.T) {
-	ctrs := []actorContainer{{name: "counter"}, {name: "sidecar"}}
+	dirs := &ateworkerpb.ActorDirs{OciBundleDir: t.TempDir()}
+	var containers []*ateworkerpb.Container
+	for _, name := range []string{"counter", "w"} {
+		if err := os.MkdirAll(filepath.Join(ociBundlePath(dirs, name), "rootfs"), 0o755); err != nil {
+			t.Fatal(err)
+		}
+		containers = append(containers, &ateworkerpb.Container{Name: name, ContainerSpec: &ateworkerpb.ContainerSpec{Args: []string{"/app"}}})
+	}
+	ctrs, err := (&AteWorkerService{}).buildActorContainers("uid", dirs, containers)
+	if err != nil {
+		t.Fatal(err)
+	}
 	got := workloadIDs(ctrs)
-	if want := []string{"counter", "sidecar"}; !slices.Equal(got, want) {
+	if want := []string{"c_counter", "c_w"}; !slices.Equal(got, want) {
 		t.Errorf("workloadIDs() = %v, want %v", got, want)
 	}
 }

internal/ocispec/microvm.go

@@ -39,12 +39,21 @@ const (
 	ShareVolumes    = "volumes"
 )
 
+// GuestContainerID returns the kata container ID for a container name.
+//
+// kata requires IDs of at least two characters, and a name can be one.
+// '_' is not valid in a name, so the ID can't collide with another name or
+// the Share* dirs. It must only depend on the name, since a restored guest
+// keeps the IDs it booted with.
+// https://github.com/kata-containers/kata-containers/blob/4.1.0/src/libs/kata-sys-util/src/validate.rs#L17-L22
+func GuestContainerID(name string) string { return "c_" + name }
+
 // MicroVMOptions describes the micro-VM context of one actor container.
 type MicroVMOptions struct {
 	// ActorDirs are the actor's directories; volume bind sources under them are
 	// staged into the share.
-	ActorDirs   *ateworkerpb.ActorDirs
-	ContainerID string
+	ActorDirs     *ateworkerpb.ActorDirs
+	ContainerName string
 }
 
 // ShapeMicroVM replaces host system mounts with guest mounts, repoints volume
@@ -58,7 +67,7 @@ func ShapeMicroVM(spec *specs.Spec, o MicroVMOptions) error {
 		if m.Type != "bind" {
 			continue
 		}
-		src, err := guestVolumeSource(m.Source, o.ActorDirs, o.ContainerID)
+		src, err := guestVolumeSource(m.Source, o.ActorDirs, o.ContainerName)
 		if err != nil {
 			return fmt.Errorf("mount %q: %w", m.Destination, err)
 		}
@@ -112,12 +121,12 @@ func mergeKataResources(from *specs.LinuxResources) *specs.LinuxResources {
 }
 
 // guestVolumeSource maps a volume's host directory to its guest path.
-func guestVolumeSource(hostPath string, actorDirs *ateworkerpb.ActorDirs, containerID string) (string, error) {
+func guestVolumeSource(hostPath string, actorDirs *ateworkerpb.ActorDirs, containerName string) (string, error) {
 	for _, staged := range []struct{ host, guest string }{
 		{actorDirs.GetDurableDirVolumeMountsDir(), path.Join(GuestSharedDir, ShareDurable)},
 		{actorDirs.GetExternalVolumeMountsDir(), path.Join(GuestSharedDir, ShareCSI)},
 		{actorDirs.GetSystemInfoVolumeRootsDir(), path.Join(GuestSharedDir, ShareSystemInfo)},
-		{imagecache.ImageVolumeMountPath(filepath.Join(actorDirs.GetOciBundleDir(), containerID), ""), path.Join(GuestSharedDir, containerID, ShareVolumes)},
+		{imagecache.ImageVolumeMountPath(filepath.Join(actorDirs.GetOciBundleDir(), containerName), ""), path.Join(GuestSharedDir, GuestContainerID(containerName), ShareVolumes)},
 	} {
 		if rel, ok := strings.CutPrefix(hostPath, staged.host+"/"); ok {
 			return path.Join(staged.guest, rel), nil

internal/ocispec/microvm_test.go

@@ -15,12 +15,31 @@
 package ocispec
 
 import (
+	"regexp"
+	"slices"
+	"strings"
 	"testing"
 
 	"github.com/opencontainers/runtime-spec/specs-go"
 	"go.ate.dev/substrate/internal/proto/ateworkerpb"
 )
 
+// kata's verify_id, see GuestContainerID
+var kataID = regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9._-]+$`)
+
+func TestGuestContainerID(t *testing.T) {
+	shares := []string{ShareDurable, ShareCSI, ShareSystemInfo}
+	for _, name := range append([]string{"w", "app", strings.Repeat("a", 63)}, shares...) {
+		id := GuestContainerID(name)
+		if !kataID.MatchString(id) {
+			t.Errorf("GuestContainerID(%q) = %q, which kata rejects", name, id)
+		}
+		if slices.Contains(shares, id) {
+			t.Errorf("GuestContainerID(%q) = %q, which collides with a share dir", name, id)
+		}
+	}
+}
+
 // The device allowlist and CPU shares from guestResources are the proven-good
 // kata shape. Overlaying a memory limit must not drop them: a container without
 // the allowlist fails in ways that do not point back here.
@@ -125,7 +144,7 @@ func TestShapeMicroVM_KeepsDeclaredContainerLimits(t *testing.T) {
 		Args:      []string{"/app"},
 		Resources: &ateworkerpb.ResourceLimits{MemoryBytes: declared},
 	}}})
-	if err := ShapeMicroVM(spec, MicroVMOptions{ActorDirs: parityActorDirs, ContainerID: "app"}); err != nil {
+	if err := ShapeMicroVM(spec, MicroVMOptions{ActorDirs: parityActorDirs, ContainerName: "app"}); err != nil {
 		t.Fatalf("ShapeMicroVM() = %v", err)
 	}
 
@@ -141,7 +160,7 @@ func TestShapeMicroVM_KeepsDeclaredContainerLimits(t *testing.T) {
 // RAM is the real ceiling, and a cap equal to the whole guest can never bind.
 func TestShapeMicroVM_LeavesUndeclaredContainerUnlimited(t *testing.T) {
 	spec := Build(Options{Container: &ateworkerpb.Container{ContainerSpec: &ateworkerpb.ContainerSpec{Args: []string{"/app"}}}})
-	if err := ShapeMicroVM(spec, MicroVMOptions{ActorDirs: parityActorDirs, ContainerID: "app"}); err != nil {
+	if err := ShapeMicroVM(spec, MicroVMOptions{ActorDirs: parityActorDirs, ContainerName: "app"}); err != nil {
 		t.Fatalf("ShapeMicroVM() = %v", err)
 	}
 

internal/ocispec/parity_test.go

@@ -81,7 +81,7 @@ func TestShapers_PreserveEveryVolumeMount(t *testing.T) {
 	}, {
 		runtime: "microvm",
 		shape: func(s *specs.Spec) error {
-			return ShapeMicroVM(s, MicroVMOptions{ActorDirs: parityActorDirs, ContainerID: "app"})
+			return ShapeMicroVM(s, MicroVMOptions{ActorDirs: parityActorDirs, ContainerName: "app"})
 		},
 	}} {
 		t.Run(tc.runtime, func(t *testing.T) {
@@ -110,14 +110,14 @@ func TestShapers_PreserveEveryVolumeMount(t *testing.T) {
 // ShapeMicroVM rewrites bind sources to their guest share paths.
 func TestShapeMicroVM_TranslatesSourcesIntoTheShare(t *testing.T) {
 	spec := Build(parityOptions)
-	if err := ShapeMicroVM(spec, MicroVMOptions{ActorDirs: parityActorDirs, ContainerID: "app"}); err != nil {
+	if err := ShapeMicroVM(spec, MicroVMOptions{ActorDirs: parityActorDirs, ContainerName: "app"}); err != nil {
 		t.Fatalf("ShapeMicroVM() = %v", err)
 	}
 	for _, tc := range []struct{ dest, wantSource string }{
 		{"/var/data", GuestSharedDir + "/" + ShareDurable + "/data"},
 		{"/run/ate", GuestSharedDir + "/" + ShareSystemInfo + "/sysinfo"},
 		{"/mnt/csi", GuestSharedDir + "/" + ShareCSI + "/csi"},
-		{"/ate", GuestSharedDir + "/app/" + ShareVolumes + "/agent"},
+		{"/ate", GuestSharedDir + "/c_app/" + ShareVolumes + "/agent"},
 	} {
 		if got := mountFor(t, spec, tc.dest).Source; got != tc.wantSource {
 			t.Errorf("%s source = %q, want %q", tc.dest, got, tc.wantSource)
@@ -133,7 +133,7 @@ func TestShapeMicroVM_TranslatesSourcesIntoTheShare(t *testing.T) {
 func TestShapeMicroVM_UnstagedSourceIsAnError(t *testing.T) {
 	spec := Build(Options{Container: &ateworkerpb.Container{ContainerSpec: &ateworkerpb.ContainerSpec{Args: []string{"/app"}}}})
 	spec.Mounts = append(spec.Mounts, specs.Mount{Destination: "/mnt/new", Type: "bind", Source: "/var/lib/ate/new-kind/x"})
-	if err := ShapeMicroVM(spec, MicroVMOptions{ActorDirs: parityActorDirs, ContainerID: "app"}); err == nil {
+	if err := ShapeMicroVM(spec, MicroVMOptions{ActorDirs: parityActorDirs, ContainerName: "app"}); err == nil {
 		t.Fatal("ShapeMicroVM() = nil, want an error for a bind that is not staged into the share")
 	}
 }