diff --git a/CubeMaster/pkg/templatecenter/image/disk.go b/CubeMaster/pkg/templatecenter/image/disk.go index f09fdb99f..c2e539f83 100644 --- a/CubeMaster/pkg/templatecenter/image/disk.go +++ b/CubeMaster/pkg/templatecenter/image/disk.go @@ -16,6 +16,7 @@ import ( "strconv" "strings" "sync" + "time" ) const ( @@ -68,12 +69,57 @@ func createExt4ImageStreaming(ctx context.Context, source *PreparedSource, workD // Use context.Background() for cleanup so it runs even after request cancellation. cleanupCtx := context.Background() + var loopDevice string + var mounted bool + var mountStuck bool var unmountOnce sync.Once var detachOnce sync.Once - cleanup := func() { - unmountOnce.Do(func() { - _ = runCommand(cleanupCtx, "", "umount", "--", mountPoint) + detachLoop := func() { + if loopDevice == "" { + return + } + detachOnce.Do(func() { + // A lazy umount releases the mount asynchronously, so the detach can fail + // EBUSY for a short window afterwards; retry briefly. When even the lazy + // umount failed the mount is still there and every attempt is guaranteed to + // fail, so don't spend the budget. + attempts := 5 + if mountStuck { + attempts = 1 + } + var err error + for attempt := 0; attempt < attempts; attempt++ { + if attempt > 0 { + time.Sleep(200 * time.Millisecond) + } + if err = runCommand(cleanupCtx, "", "losetup", "--detach", "--", loopDevice); err == nil { + return + } + } + // Be precise about how narrow recovery is from here: reclaim only picks a + // device up once its backing file has been unlinked — which happens on the + // build's failure path only — and the mount is gone. On the success path the + // image stays, so the device stays attached until the host reboots. + log.G(ctx).Warnf("losetup --detach %s failed; device stays attached, reclaimable later only if its backing file is unlinked and the mount released: %v", loopDevice, err) }) + } + // A single cleanup closure, so the detach always runs *after* the umount: + // detaching a still-mounted device fails EBUSY and pins the loop device — and, + // once the backing file is unlinked, its disk space too — until the host + // reboots. Two separate defers would run in LIFO order and get this backwards. + cleanup := func() { + if mounted { + unmountOnce.Do(func() { + if err := runCommand(cleanupCtx, "", "umount", "--", mountPoint); err != nil { + log.G(ctx).Warnf("umount %s failed, retrying lazily: %v", mountPoint, err) + if lazyErr := runCommand(cleanupCtx, "", "umount", "-l", "--", mountPoint); lazyErr != nil { + mountStuck = true + log.G(ctx).Warnf("lazy umount %s also failed, loop device will leak: %v", mountPoint, lazyErr) + } + } + }) + } + detachLoop() if err := os.RemoveAll(mountPoint); err != nil { log.G(ctx).Warnf("cleanup mount point %s failed: %v", mountPoint, err) } @@ -86,25 +132,47 @@ func createExt4ImageStreaming(ctx context.Context, source *PreparedSource, workD // parsed device path — corrupting the loopDevice string so every later mount // and detach receives a garbage argument (mount fails "bad option", detach // fails → loop device leaks). - losetupCmd := exec.CommandContext(ctx, "losetup", "--find", "--show", "--", ext4Path) - var losetupErr bytes.Buffer - losetupCmd.Stderr = &losetupErr - loopOut, err := losetupCmd.Output() - if err != nil { - return fmt.Errorf("losetup --find --show %s failed: %w: %s", ext4Path, err, strings.TrimSpace(losetupErr.String())) + findLoop := func() ([]byte, error) { + cmd := exec.CommandContext(ctx, "losetup", "--find", "--show", "--", ext4Path) + var stderr bytes.Buffer + cmd.Stderr = &stderr + out, err := cmd.Output() + if err != nil { + return nil, fmt.Errorf("losetup --find --show %s failed: %w: %s", ext4Path, err, strings.TrimSpace(stderr.String())) + } + return out, nil } - loopDevice := strings.TrimSpace(string(loopOut)) - detachLoop := func() { - detachOnce.Do(func() { - _ = runCommand(cleanupCtx, "", "losetup", "--detach", "--", loopDevice) - }) + loopOut, err := findLoop() + if err != nil { + // Running out of loop devices can be self-inflicted: a detach that could not + // be completed (e.g. the umount above only succeeded lazily) keeps one pinned + // for the lifetime of the host, so the fast path would be lost permanently + // once enough builds have leaked. Reclaim devices whose backing file is + // already gone and retry before degrading to the much slower phase-1 build. + // This is best-effort and only meaningful for exhaustion; losetup reports + // every failure the same way, so other causes (cancellation, permissions) + // pay one extra `losetup --list` and then fail the retry as they should. + // It also only frees devices this process managed to attach, i.e. ones whose + // /dev node exists here — it cannot create the node that LOOP_CTL_GET_FREE + // returned, so a container whose /dev tmpfs is short of loop nodes and has no + // orphans to reclaim still degrades to phase-1 (see the deployment note). + if n := reclaimOrphanLoopDevices(cleanupCtx, artifactStoreDirOf(ext4Path)); n > 0 { + log.G(ctx).Warnf("loop device allocation failed (%v); reclaimed %d orphaned loop device(s), retrying", err, n) + retryOut, retryErr := findLoop() + if retryErr != nil { + return fmt.Errorf("%w (retry after reclaiming %d device(s) also failed: %v)", err, n, retryErr) + } + loopOut = retryOut + } else { + return err + } } - defer detachLoop() + loopDevice = strings.TrimSpace(string(loopOut)) if err := runCommand(ctx, "", "mount", "-o", "nosuid,noexec,nodev,noatime", "--", loopDevice, mountPoint); err != nil { - detachLoop() // explicit detach on mount failure (defer will be a no-op via sync.Once) return fmt.Errorf("mount loop device %s: %w", loopDevice, err) } + mounted = true // 4. Stream export directly into the mounted ext4. if source.ExportMode == ExportModeNative { @@ -133,7 +201,7 @@ func createExt4ImageStreaming(ctx context.Context, source *PreparedSource, workD } } - // 5. Unmount (via cleanup). + // 5. Unmount and detach (via cleanup). cleanup() // 6. Shrink the ext4 filesystem to minimum size (best-effort). @@ -176,6 +244,74 @@ func createExt4ImageStreaming(ctx context.Context, source *PreparedSource, workD return nil } +// artifactStoreDirOf returns the artifact store root for an ext4 image path, +// mirroring the //.ext4 layout BuildExt4 constructs +// (including when it falls back to ArtifactFallbackStoreRootDir, so a fallback +// build only ever reclaims fallback-store orphans). A path of another shape just +// yields a prefix that matches nothing in orphanLoopCandidates, i.e. reclaim +// no-ops rather than reaching outside the store. Nested store roots are the one +// case where the scope is wider than the build's own store: the shallower root's +// prefix also covers the deeper one's orphans, still bounded to devices whose +// backing file is already gone. +func artifactStoreDirOf(ext4Path string) string { + return filepath.Dir(filepath.Dir(ext4Path)) +} + +// orphanLoopCandidates parses the output of +// `losetup --list --noheadings --raw --output NAME,BACK-FILE` and returns the +// devices whose backing file lives under storeDir and has already been unlinked +// (losetup marks those with a trailing " (deleted)"). +// +// Requiring the unlink is what makes the reclaim safe against a build running +// concurrently: the image file is only unlinked once a build has given up on it, +// so a live build's device can never become a candidate — not even in the +// attach→mount window, where the device is not yet in the mount table and the +// kernel's EBUSY refusal would not protect it either. +func orphanLoopCandidates(losetupList, storeDir string) []string { + if storeDir == "" || storeDir == "/" { + return nil + } + prefix := strings.TrimSuffix(storeDir, "/") + "/" + var names []string + for _, line := range strings.Split(losetupList, "\n") { + name, backing, ok := strings.Cut(strings.TrimSpace(line), " ") + if !ok || name == "" { + continue + } + backing = strings.TrimSpace(backing) + if !strings.HasSuffix(backing, " (deleted)") { + continue + } + backing = strings.TrimSpace(strings.TrimSuffix(backing, " (deleted)")) + if strings.HasPrefix(backing, prefix) { + names = append(names, name) + } + } + return names +} + +// reclaimOrphanLoopDevices detaches loop devices whose backing file under +// storeDir has already been unlinked, i.e. devices no build can still be using. +// Returns how many were freed. +func reclaimOrphanLoopDevices(ctx context.Context, storeDir string) int { + out, err := exec.CommandContext(ctx, "losetup", "--list", "--noheadings", "--raw", + "--output", "NAME,BACK-FILE").Output() + if err != nil { + log.G(ctx).Warnf("losetup --list for orphan reclaim failed: %v", err) + return 0 + } + reclaimed := 0 + for _, name := range orphanLoopCandidates(string(out), storeDir) { + if err := runCommand(ctx, "", "losetup", "--detach", "--", name); err != nil { + log.G(ctx).Debugf("orphan loop device %s not reclaimable (likely still in use): %v", name, err) + continue + } + log.G(ctx).Debugf("reclaimed orphaned loop device %s", name) + reclaimed++ + } + return reclaimed +} + // pipeExportToDir streams the docker export of a container directly into a target // directory via tar -xf -. func pipeExportToDir(ctx context.Context, containerID, destDir string) error { diff --git a/CubeMaster/pkg/templatecenter/image/disk_test.go b/CubeMaster/pkg/templatecenter/image/disk_test.go new file mode 100644 index 000000000..834898869 --- /dev/null +++ b/CubeMaster/pkg/templatecenter/image/disk_test.go @@ -0,0 +1,352 @@ +// Copyright (c) 2026 Tencent Inc. +// SPDX-License-Identifier: Apache-2.0 +// + +package image + +import ( + "context" + "os" + "path/filepath" + "reflect" + "strings" + "testing" + + "github.com/agiledragon/gomonkey/v2" +) + +func TestArtifactStoreDirOf(t *testing.T) { + got := artifactStoreDirOf("/var/lib/cubemaster/storage/rfs-abc/rfs-abc.ext4") + if want := "/var/lib/cubemaster/storage"; got != want { + t.Fatalf("artifactStoreDirOf = %q, want %q", got, want) + } +} + +func TestOrphanLoopCandidates(t *testing.T) { + const store = "/var/lib/cubemaster/storage" + + cases := []struct { + name string + list string + storeDir string + want []string + }{ + { + name: "empty output", + list: "", + storeDir: store, + }, + { + name: "deleted backing file is a candidate", + list: "/dev/loop3 /var/lib/cubemaster/storage/rfs-a/rfs-a.ext4 (deleted)\n", + storeDir: store, + want: []string{"/dev/loop3"}, + }, + { + name: "existing backing file under the store is left alone (may be a live build)", + list: "/dev/loop1 /var/lib/cubemaster/storage/rfs-b/rfs-b.ext4\n", + storeDir: store, + }, + { + name: "devices outside the store are never touched", + list: "/dev/loop0 /var/lib/snapd/snaps/core.snap\n/dev/loop2 /srv/other/disk.img (deleted)\n", + storeDir: store, + }, + { + name: "the store dir itself must not match as a prefix of a sibling", + list: "/dev/loop4 /var/lib/cubemaster/storage-backup/rfs-c/rfs-c.ext4 (deleted)\n", + storeDir: store, + }, + { + name: "blank and malformed lines are skipped", + list: "\n \n/dev/loop5\n/dev/loop6 /var/lib/cubemaster/storage/rfs-d/rfs-d.ext4 (deleted)\n", + storeDir: store, + want: []string{"/dev/loop6"}, + }, + { + name: "a root store dir is refused rather than detaching everything", + list: "/dev/loop0 /any/file.img (deleted)\n", + storeDir: "/", + }, + { + name: "trailing slash on the store dir behaves the same", + list: "/dev/loop7 /var/lib/cubemaster/storage/rfs-e/rfs-e.ext4 (deleted)\n", + storeDir: store + "/", + want: []string{"/dev/loop7"}, + }, + { + name: "a backing path containing spaces is still matched", + list: "/dev/loop8 /var/lib/cubemaster/storage/rfs f/rfs f.ext4 (deleted)\n", + storeDir: store, + want: []string{"/dev/loop8"}, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := orphanLoopCandidates(tc.list, tc.storeDir) + if len(got) == 0 && len(tc.want) == 0 { + return + } + if !reflect.DeepEqual(got, tc.want) { + t.Fatalf("orphanLoopCandidates = %v, want %v", got, tc.want) + } + }) + } +} + +// setupStreamingFakes puts fake truncate/mkfs.ext4/mount/umount/resize2fs on +// PATH (each appending its argv to a trace file) and lets the streaming build +// believe loop mounts are usable. The caller installs its own `losetup` fake, +// which is where every interesting behaviour lives. +func setupStreamingFakes(t *testing.T) (binDir, tracePath, ext4Path string) { + t.Helper() + binDir = t.TempDir() + tracePath = filepath.Join(binDir, "trace.log") + store := filepath.Join(t.TempDir(), "storage") + ext4Path = filepath.Join(store, "rfs-a", "rfs-a.ext4") + + t.Setenv("PATH", binDir) + t.Setenv("FAKE_TRACE", tracePath) + t.Setenv("FAKE_STORE", store) + t.Setenv("FAKE_STATE", filepath.Join(binDir, "state")) + + for _, name := range []string{"truncate", "mkfs.ext4", "mount", "umount", "resize2fs"} { + installFakeCommand(t, binDir, name, `echo "`+name+` $*" >> "$FAKE_TRACE"`) + } + + patches := gomonkey.NewPatches() + patches.ApplyFuncReturn(canUseLoopMount, true) + patches.ApplyFuncReturn(StreamRegistryToDir, nil) + t.Cleanup(patches.Reset) + return binDir, tracePath, ext4Path +} + +func runStreamingBuild(t *testing.T, ext4Path string) error { + t.Helper() + source := &PreparedSource{LocalRef: "local/img:latest", ExportMode: ExportModeNative} + return createExt4ImageStreaming(context.Background(), source, t.TempDir(), ext4Path, 1024, nil) +} + +func traceLines(t *testing.T, tracePath string) []string { + t.Helper() + data, err := os.ReadFile(tracePath) + if err != nil { + t.Fatalf("read command trace: %v", err) + } + var lines []string + for _, line := range strings.Split(string(data), "\n") { + if line = strings.TrimSpace(line); line != "" { + lines = append(lines, line) + } + } + return lines +} + +func traceIndex(lines []string, prefix string) int { + for i, line := range lines { + if strings.HasPrefix(line, prefix) { + return i + } + } + return -1 +} + +func traceCount(lines []string, prefix string) int { + n := 0 + for _, line := range lines { + if strings.HasPrefix(line, prefix) { + n++ + } + } + return n +} + +// TestStreamingUnmountsBeforeDetach guards the actual leak: detaching a device +// that is still mounted fails EBUSY and pins it for the lifetime of the host. +func TestStreamingUnmountsBeforeDetach(t *testing.T) { + binDir, tracePath, ext4Path := setupStreamingFakes(t) + installFakeCommand(t, binDir, "losetup", `echo "losetup $*" >> "$FAKE_TRACE" +case "$1" in --find) echo /dev/loop9 ;; esac`) + + if err := runStreamingBuild(t, ext4Path); err != nil { + t.Fatalf("streaming build failed: %v", err) + } + + lines := traceLines(t, tracePath) + umountAt := traceIndex(lines, "umount ") + detachAt := traceIndex(lines, "losetup --detach") + if umountAt < 0 || detachAt < 0 { + t.Fatalf("expected both umount and detach in trace, got %v", lines) + } + if umountAt > detachAt { + t.Fatalf("detach ran before umount (would fail EBUSY and leak the device): %v", lines) + } +} + +// TestStreamingRetriesDetachAfterBusy covers the lazy-umount window, where the +// mount is released asynchronously and the first detaches legitimately fail. +func TestStreamingRetriesDetachAfterBusy(t *testing.T) { + binDir, tracePath, ext4Path := setupStreamingFakes(t) + installFakeCommand(t, binDir, "losetup", `echo "losetup $*" >> "$FAKE_TRACE" +case "$1" in +--find) echo /dev/loop9 ;; +--detach) + if [ -f "$FAKE_STATE.2" ]; then exit 0; fi + if [ -f "$FAKE_STATE.1" ]; then > "$FAKE_STATE.2"; else > "$FAKE_STATE.1"; fi + echo "device or resource busy" >&2; exit 1 ;; +esac`) + + if err := runStreamingBuild(t, ext4Path); err != nil { + t.Fatalf("streaming build failed: %v", err) + } + + if got := traceCount(traceLines(t, tracePath), "losetup --detach"); got != 3 { + t.Fatalf("detach attempts = %d, want 3 (retry until it succeeds)", got) + } +} + +// TestStreamingSkipsDetachRetriesWhenMountStuck: once even the lazy umount +// failed the mount is still there, so every detach is guaranteed to fail — +// spending the retry budget only delays the build. +func TestStreamingSkipsDetachRetriesWhenMountStuck(t *testing.T) { + binDir, tracePath, ext4Path := setupStreamingFakes(t) + installFakeCommand(t, binDir, "umount", `echo "umount $*" >> "$FAKE_TRACE" +echo "target is busy" >&2; exit 1`) + installFakeCommand(t, binDir, "losetup", `echo "losetup $*" >> "$FAKE_TRACE" +case "$1" in +--find) echo /dev/loop9 ;; +--detach) echo "device or resource busy" >&2; exit 1 ;; +esac`) + + if err := runStreamingBuild(t, ext4Path); err != nil { + t.Fatalf("streaming build failed: %v", err) + } + + lines := traceLines(t, tracePath) + if got := traceCount(lines, "umount -l"); got != 1 { + t.Fatalf("lazy umount attempts = %d, want 1", got) + } + if got := traceCount(lines, "losetup --detach"); got != 1 { + t.Fatalf("detach attempts = %d, want 1 (mount is stuck, retrying is pointless)", got) + } +} + +// TestStreamingDetachesAfterLazyUmount is the case the detach retry budget +// exists for: the plain umount fails, the lazy one succeeds, and the device is +// released once the kernel has dropped the mount. +func TestStreamingDetachesAfterLazyUmount(t *testing.T) { + binDir, tracePath, ext4Path := setupStreamingFakes(t) + installFakeCommand(t, binDir, "umount", `echo "umount $*" >> "$FAKE_TRACE" +case "$1" in -l) exit 0 ;; *) echo "target is busy" >&2; exit 1 ;; esac`) + installFakeCommand(t, binDir, "losetup", `echo "losetup $*" >> "$FAKE_TRACE" +case "$1" in +--find) echo /dev/loop9 ;; +--detach) + if [ -f "$FAKE_STATE" ]; then exit 0; fi + > "$FAKE_STATE"; echo "device or resource busy" >&2; exit 1 ;; +esac`) + + if err := runStreamingBuild(t, ext4Path); err != nil { + t.Fatalf("streaming build failed: %v", err) + } + + lines := traceLines(t, tracePath) + if got := traceCount(lines, "umount -l"); got != 1 { + t.Fatalf("lazy umount attempts = %d, want 1", got) + } + if got := traceCount(lines, "losetup --detach"); got != 2 { + t.Fatalf("detach attempts = %d, want 2 (retried once the lazy umount landed)", got) + } +} + +// TestStreamingDetachesWhenMountFails guards the removal of the explicit detach +// on the mount-failure path: it now happens only through the deferred cleanup. +func TestStreamingDetachesWhenMountFails(t *testing.T) { + binDir, tracePath, ext4Path := setupStreamingFakes(t) + installFakeCommand(t, binDir, "mount", `echo "mount $*" >> "$FAKE_TRACE" +echo "bad option" >&2; exit 32`) + installFakeCommand(t, binDir, "losetup", `echo "losetup $*" >> "$FAKE_TRACE" +case "$1" in --find) echo /dev/loop9 ;; esac`) + + if err := runStreamingBuild(t, ext4Path); err == nil { + t.Fatal("expected the build to fail when the mount fails") + } + + lines := traceLines(t, tracePath) + if traceIndex(lines, "losetup --detach -- /dev/loop9") < 0 { + t.Fatalf("allocated device must be detached even though it was never mounted: %v", lines) + } + if traceIndex(lines, "umount ") >= 0 { + t.Fatalf("nothing was mounted, so no umount should be attempted: %v", lines) + } +} + +// TestStreamingReclaimsOrphansOnAllocationFailure: exhaustion caused by earlier +// leaks must self-heal instead of permanently degrading to the phase-1 build. +func TestStreamingReclaimsOrphansOnAllocationFailure(t *testing.T) { + binDir, tracePath, ext4Path := setupStreamingFakes(t) + installFakeCommand(t, binDir, "losetup", `echo "losetup $*" >> "$FAKE_TRACE" +case "$1" in +--find) + if [ -f "$FAKE_STATE" ]; then echo /dev/loop9; else > "$FAKE_STATE"; echo "could not find any free loop device" >&2; exit 1; fi ;; +--list) echo "/dev/loop3 $FAKE_STORE/rfs-gone/rfs-gone.ext4 (deleted)" ;; +esac`) + + if err := runStreamingBuild(t, ext4Path); err != nil { + t.Fatalf("streaming build failed instead of reclaiming and retrying: %v", err) + } + + lines := traceLines(t, tracePath) + if got := traceCount(lines, "losetup --find"); got != 2 { + t.Fatalf("losetup --find attempts = %d, want 2 (retry after reclaim)", got) + } + if traceIndex(lines, "losetup --detach -- /dev/loop3") < 0 { + t.Fatalf("expected the orphaned device to be detached, got %v", lines) + } +} + +// TestStreamingAllocationFailureWithoutOrphans: with nothing to reclaim the +// original allocation error must be returned unchanged (and no retry spent), so +// causes other than exhaustion still surface as themselves. +func TestStreamingAllocationFailureWithoutOrphans(t *testing.T) { + binDir, tracePath, ext4Path := setupStreamingFakes(t) + installFakeCommand(t, binDir, "losetup", `echo "losetup $*" >> "$FAKE_TRACE" +case "$1" in +--find) echo "could not find any free loop device" >&2; exit 1 ;; +--list) ;; +esac`) + + err := runStreamingBuild(t, ext4Path) + if err == nil { + t.Fatal("expected the build to fail when no loop device can be allocated") + } + if strings.Contains(err.Error(), "retry") { + t.Fatalf("nothing was reclaimed, so no retry should be reported: %v", err) + } + + if got := traceCount(traceLines(t, tracePath), "losetup --find"); got != 1 { + t.Fatalf("losetup --find attempts = %d, want 1 (nothing to reclaim)", got) + } +} + +// TestStreamingReportsBothAllocationFailures: when the retry after a reclaim +// fails too, both the original and the retry error must be visible — otherwise +// the reclaim hides the real reason the fast path was lost. +func TestStreamingReportsBothAllocationFailures(t *testing.T) { + binDir, _, ext4Path := setupStreamingFakes(t) + installFakeCommand(t, binDir, "losetup", `echo "losetup $*" >> "$FAKE_TRACE" +case "$1" in +--find) echo "could not find any free loop device" >&2; exit 1 ;; +--list) echo "/dev/loop3 $FAKE_STORE/rfs-gone/rfs-gone.ext4 (deleted)" ;; +esac`) + + err := runStreamingBuild(t, ext4Path) + if err == nil { + t.Fatal("expected the build to fail when the allocation fails even after reclaiming") + } + if !strings.Contains(err.Error(), "could not find any free loop device") || + !strings.Contains(err.Error(), "retry after reclaiming 1 device(s)") { + t.Fatalf("error must report both the original and the retry failure, got %v", err) + } +}