Skip to content

Commit 91f54c6

Browse files
committed
Handle read-only trees in data dir cleanup
Go module cache sets files to 0444 and directories to 0555, causing os.RemoveAll to fail with permission denied when cleaning the data dir on next VM boot. Replace the default removeAll with forceRemoveAll which retries after making directories writable, matching how go clean -modcache works.
1 parent 98e51b5 commit 91f54c6

File tree

3 files changed

+87
-1
lines changed

3 files changed

+87
-1
lines changed

options.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ func defaultConfig() *config {
9494
preflight: preflight.Default(),
9595
imageCache: image.NewCache(filepath.Join(dataDir, "cache")),
9696
dataDir: dataDir,
97-
removeAll: os.RemoveAll,
97+
removeAll: forceRemoveAll,
9898
stat: os.Stat,
9999
}
100100
}

propolis.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package propolis
1818
import (
1919
"context"
2020
"fmt"
21+
"io/fs"
2122
"log/slog"
2223
"os"
2324
"path/filepath"
@@ -258,6 +259,30 @@ func cleanDataDir(cfg *config) error {
258259
return nil
259260
}
260261

262+
// forceRemoveAll removes the given path, handling read-only directory trees
263+
// such as Go module caches (whose entries are set to 0444/0555). It first
264+
// attempts a plain os.RemoveAll; on failure it walks the tree making every
265+
// directory user-writable, then retries.
266+
func forceRemoveAll(path string) error {
267+
err := os.RemoveAll(path)
268+
if err == nil {
269+
return nil
270+
}
271+
272+
// Make every directory writable so entries can be unlinked.
273+
_ = filepath.WalkDir(path, func(p string, d fs.DirEntry, walkErr error) error {
274+
if walkErr != nil {
275+
return nil // best-effort
276+
}
277+
if d.IsDir() {
278+
_ = os.Chmod(p, 0o700)
279+
}
280+
return nil
281+
})
282+
283+
return os.RemoveAll(path)
284+
}
285+
261286
func cacheDir(cfg *config) string {
262287
if cfg == nil || cfg.imageCache == nil {
263288
return ""

propolis_test.go

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,67 @@ func TestRun_WithCleanDataDir_RemovesStaleState(t *testing.T) {
245245
require.NoError(t, err)
246246
}
247247

248+
func TestRun_WithCleanDataDir_ReadOnlyTree(t *testing.T) {
249+
t.Parallel()
250+
251+
dataDir := t.TempDir()
252+
rootfsDir := filepath.Join(dataDir, "rootfs")
253+
require.NoError(t, os.MkdirAll(rootfsDir, 0o755))
254+
255+
// Simulate a Go module cache tree: read-only dirs and files.
256+
modDir := filepath.Join(dataDir, "rootfs-work", "go", "pkg", "mod", "example.com@v1.0.0")
257+
require.NoError(t, os.MkdirAll(modDir, 0o755))
258+
require.NoError(t, os.WriteFile(filepath.Join(modDir, "README.md"), []byte("ro"), 0o444))
259+
// Lock down directories to read+execute only, like Go module cache.
260+
require.NoError(t, os.Chmod(modDir, 0o555))
261+
require.NoError(t, os.Chmod(filepath.Dir(modDir), 0o555))
262+
263+
handle := &mockVMHandle{id: "ro-test", alive: true}
264+
netProv := &mockNetProvider{sockPath: "/tmp/fake.sock"}
265+
266+
vm, err := Run(context.Background(), "test:latest",
267+
WithDataDir(dataDir),
268+
WithCleanDataDir(),
269+
WithPreflightChecker(preflight.NewEmpty()),
270+
WithRootFSPath(rootfsDir),
271+
WithNetProvider(netProv),
272+
WithBackend(&mockBackend{startHandle: handle}),
273+
)
274+
require.NoError(t, err)
275+
require.NotNil(t, vm)
276+
277+
// The read-only tree should have been removed.
278+
_, err = os.Stat(filepath.Join(dataDir, "rootfs-work"))
279+
assert.True(t, os.IsNotExist(err))
280+
281+
// Rootfs (kept) should still exist.
282+
_, err = os.Stat(rootfsDir)
283+
require.NoError(t, err)
284+
}
285+
286+
func TestForceRemoveAll(t *testing.T) {
287+
t.Parallel()
288+
289+
t.Run("removes read-only tree", func(t *testing.T) {
290+
t.Parallel()
291+
dir := t.TempDir()
292+
nested := filepath.Join(dir, "a", "b", "c")
293+
require.NoError(t, os.MkdirAll(nested, 0o755))
294+
require.NoError(t, os.WriteFile(filepath.Join(nested, "file.txt"), []byte("x"), 0o444))
295+
require.NoError(t, os.Chmod(nested, 0o555))
296+
require.NoError(t, os.Chmod(filepath.Dir(nested), 0o555))
297+
298+
require.NoError(t, forceRemoveAll(dir))
299+
_, err := os.Stat(dir)
300+
assert.True(t, os.IsNotExist(err))
301+
})
302+
303+
t.Run("no error on nonexistent path", func(t *testing.T) {
304+
t.Parallel()
305+
require.NoError(t, forceRemoveAll("/tmp/does-not-exist-propolis-test"))
306+
})
307+
}
308+
248309
func TestRun_Success(t *testing.T) {
249310
t.Parallel()
250311

0 commit comments

Comments
 (0)