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) + }) +}