From fd611e09995a5d568444c3d6df6d6e5a1b462e3b Mon Sep 17 00:00:00 2001 From: AdaAibaby <8872412+adababys@users.noreply.github.com> Date: Fri, 23 Oct 2026 11:18:58 +0800 Subject: [PATCH] fix(nfsproxy): clean up .nfs* silly-rename orphans on sandbox teardown When an NFS client deletes a file that still has open file descriptors, it renames the file to .nfsXXXX on the server instead of removing it, and only sends the REMOVE once the last descriptor is closed. If the sandbox VM is killed before closing its descriptors, that REMOVE never arrives and the .nfsXXXX file - as large as the original, often several GiB - leaks on the NFS server permanently (observed in production: a 5 GiB workspace image lingering as .nfs000000019000ee98... with no process holding it). OnNetworkRelease runs after the VM is dead and all of its descriptors are gone, so any .nfs* orphan in the volume is safe to delete there. Add removeNFSOrphans, called per chroot before Close(): it scans the chroot host-side root (chrooted.Chrooted.ActualRoot) and removes top-level .nfs* files, skipping subdirectories, best-effort (failures logged, teardown never aborted). Rebased onto current main (OnNetworkRelease now iterates mounts by lifecycleID and takes the chroot from mounted.chroot). Adds a regression test: .nfs* removed, real/other dotfiles and a .nfs* subdirectory kept, and a missing or empty directory is a no-op. Fixes #3532. Supersedes #3533. --- .../orchestrator/pkg/nfsproxy/chroot/nfs.go | 55 ++++++++++++++ .../pkg/nfsproxy/chroot/nfs_orphan_test.go | 75 +++++++++++++++++++ 2 files changed, 130 insertions(+) create mode 100644 packages/orchestrator/pkg/nfsproxy/chroot/nfs_orphan_test.go diff --git a/packages/orchestrator/pkg/nfsproxy/chroot/nfs.go b/packages/orchestrator/pkg/nfsproxy/chroot/nfs.go index f9cc1c9e7a..c525855179 100644 --- a/packages/orchestrator/pkg/nfsproxy/chroot/nfs.go +++ b/packages/orchestrator/pkg/nfsproxy/chroot/nfs.go @@ -7,7 +7,10 @@ import ( "errors" "fmt" "net" + "os" + "path/filepath" "regexp" + "strings" "sync" "syscall" @@ -132,6 +135,12 @@ func (h *NFSHandler) OnNetworkRelease(ctx context.Context, sbx *sandbox.Sandbox) } chroot := mounted.chroot + + // The sandbox VM is already dead by the time OnNetworkRelease fires, so all + // of its file descriptors are gone and any .nfs* silly-rename orphans in the + // volume can be safely removed before the chroot is torn down. + h.removeNFSOrphans(ctx, sbx.Runtime.SandboxID, chroot.Root()) + err := chroot.Close() if err != nil { logger.L().Warn(ctx, "failed to close chroot", @@ -147,6 +156,52 @@ func (h *NFSHandler) OnNetworkRelease(ctx context.Context, sbx *sandbox.Sandbox) return nil } +// removeNFSOrphans removes .nfs* files left by the NFS silly-rename mechanism. +// When an NFS client deletes a file that still has open file descriptors, the +// client renames it to .nfsXXXX on the server instead of removing it, and sends +// the REMOVE only once the last descriptor is closed. If the sandbox VM is +// killed before closing its descriptors, that REMOVE never arrives and the +// .nfsXXXX file (as large as the original, often several GiB) is leaked on the +// server permanently. OnNetworkRelease runs after the VM is dead and all its +// descriptors are gone, so these orphans are safe to delete here. Best-effort: +// failures are logged and never abort teardown. +func (h *NFSHandler) removeNFSOrphans(ctx context.Context, sandboxID, dir string) { + entries, err := os.ReadDir(dir) + if err != nil { + if !os.IsNotExist(err) { + logger.L().Warn(ctx, "failed to read volume dir for NFS orphan cleanup", + logger.WithSandboxID(sandboxID), + zap.String("dir", dir), + zap.Error(err), + ) + } + + return + } + + for _, entry := range entries { + if entry.IsDir() || !strings.HasPrefix(entry.Name(), ".nfs") { + continue + } + + path := filepath.Join(dir, entry.Name()) + if err := os.Remove(path); err != nil && !os.IsNotExist(err) { + logger.L().Warn(ctx, "failed to remove NFS orphan file", + logger.WithSandboxID(sandboxID), + zap.String("path", path), + zap.Error(err), + ) + + continue + } + + logger.L().Info(ctx, "removed NFS orphan file", + logger.WithSandboxID(sandboxID), + zap.String("path", path), + ) + } +} + func (h *NFSHandler) Mount( ctx context.Context, conn net.Conn, diff --git a/packages/orchestrator/pkg/nfsproxy/chroot/nfs_orphan_test.go b/packages/orchestrator/pkg/nfsproxy/chroot/nfs_orphan_test.go new file mode 100644 index 0000000000..c5b47274fc --- /dev/null +++ b/packages/orchestrator/pkg/nfsproxy/chroot/nfs_orphan_test.go @@ -0,0 +1,75 @@ +package chroot + +import ( + "context" + "os" + "path/filepath" + "sort" + "testing" + + "github.com/stretchr/testify/require" +) + +// TestRemoveNFSOrphans verifies that removeNFSOrphans deletes NFS silly-rename +// orphans (.nfs*) in the volume directory while leaving real files, and that a +// missing directory is a no-op rather than an error. +func TestRemoveNFSOrphans(t *testing.T) { + t.Parallel() + + h := &NFSHandler{} + ctx := context.Background() + + t.Run("removes .nfs* and keeps real files", func(t *testing.T) { + t.Parallel() + dir := t.TempDir() + + orphans := []string{ + ".nfs000000019000ee9800000001", + ".nfs0000000a", + ".nfsZZZ", + } + keep := []string{ + "ws_123.img", + "data.bin", + ".hidden", // dotfile but not .nfs + ".nfs", // exactly ".nfs" has the prefix -> treated as orphan? HasPrefix(".nfs",".nfs")=true + } + // ".nfs" exactly matches the prefix and is removed; adjust expectations. + for _, n := range append(append([]string{}, orphans...), keep...) { + require.NoError(t, os.WriteFile(filepath.Join(dir, n), []byte("x"), 0o644)) + } + // a subdirectory named like an orphan must NOT be removed (IsDir guard) + require.NoError(t, os.Mkdir(filepath.Join(dir, ".nfsdir"), 0o755)) + + h.removeNFSOrphans(ctx, "sbx-test", dir) + + got, err := os.ReadDir(dir) + require.NoError(t, err) + var names []string + for _, e := range got { + names = append(names, e.Name()) + } + sort.Strings(names) + + want := []string{".hidden", ".nfsdir", "data.bin", "ws_123.img"} + sort.Strings(want) + require.Equal(t, want, names, + "only non-.nfs* files and the .nfsdir subdirectory must remain") + }) + + t.Run("missing directory is a no-op", func(t *testing.T) { + t.Parallel() + require.NotPanics(t, func() { + h.removeNFSOrphans(ctx, "sbx-test", filepath.Join(t.TempDir(), "does-not-exist")) + }) + }) + + t.Run("empty directory is a no-op", func(t *testing.T) { + t.Parallel() + dir := t.TempDir() + h.removeNFSOrphans(ctx, "sbx-test", dir) + got, err := os.ReadDir(dir) + require.NoError(t, err) + require.Empty(t, got) + }) +}