From db1f36120cdf9edb15e1a5325055c936834404f5 Mon Sep 17 00:00:00 2001 From: mornyx Date: Fri, 2 Oct 2026 12:50:09 +0800 Subject: [PATCH 1/3] fix(fs): wait for mount readability before returning background mounts The --ready-timeout flag on ti fs mount-file-system and ti fs-vault mount-vault was parsed but never consumed on the Drive9 companion path: the commands returned as soon as the companion mount process exited successfully, so first reads during the companion warmup window could surface EAGAIN from the freshly mounted path. After the companion mount succeeds and the mount locator is written, poll the mount path until directory listing succeeds, bounded by --ready-timeout (default 30s). On timeout or cancellation the mount is left running, the locator is preserved, and the error carries the exact unmount command. The fake companions used by unit and e2e tests now create a readable mount path so they model a live background mount. --- README.md | 2 + e2e/testdata/fake-drive9.go | 4 + internal/fs/control.go | 1 + internal/fs/drive9_companion.go | 6 + internal/fs/drive9_companion_test.go | 4 + internal/fs/mountready.go | 109 +++++++++++++++++ internal/fs/mountready_test.go | 173 +++++++++++++++++++++++++++ 7 files changed, 299 insertions(+) create mode 100644 internal/fs/mountready.go create mode 100644 internal/fs/mountready_test.go diff --git a/README.md b/README.md index 662924c..1372412 100644 --- a/README.md +++ b/README.md @@ -179,6 +179,8 @@ Automatic mounting uses FUSE on Linux and WebDAV on macOS and Windows. macOS use Mount commands start the companion runtime in the background, wait until the mount is ready, and then return a structured result. Use `ti fs unmount-file-system` or `ti fs-vault unmount-vault` to end a mount. The public CLI does not expose a foreground mount mode. +Mount readiness means the mounted path is readable: `ti fs mount-file-system` and `ti fs-vault mount-vault` keep polling the mount path until directory listing succeeds, bounded by `--ready-timeout` (default `30s`). If the mount path never becomes readable in time, the command fails with `fs.mount_ready_timeout`, the background mount is left running, and the mount locator is preserved so the matching unmount command can stop it. + Filesystem layers can fork copy-on-write child timelines without copying a workspace. Layer and checkpoint mounts require FUSE; checkpoint mounts are always read-only. Drive9 does not support combining recursive copy with a layer, so seed a directory tree through a writable layer mount, drain it, and then create the checkpoint: ```shell diff --git a/e2e/testdata/fake-drive9.go b/e2e/testdata/fake-drive9.go index 30a7acc..7fd0e39 100644 --- a/e2e/testdata/fake-drive9.go +++ b/e2e/testdata/fake-drive9.go @@ -143,6 +143,10 @@ func main() { } if hasPrefix(args, "mount") { fmt.Fprintln(os.Stderr, "drive9: mount mode: "+mountMode(args)) + if mountPath := args[len(args)-1]; len(args) >= 2 && len(mountPath) > 0 && mountPath[0] != '-' { + _ = os.MkdirAll(mountPath, 0o755) + _ = os.WriteFile(mountPath+string(os.PathSeparator)+".drive9-mounted", []byte("fake mount ready\n"), 0o644) + } return } } diff --git a/internal/fs/control.go b/internal/fs/control.go index 81f47ac..1f3fbc4 100644 --- a/internal/fs/control.go +++ b/internal/fs/control.go @@ -29,6 +29,7 @@ type Service struct { Timeout time.Duration FSReadyWaitTimeout time.Duration FSReadyWaitPollInterval time.Duration + MountReadyPollInterval time.Duration Debug bool DebugWriter io.Writer HomeDir string diff --git a/internal/fs/drive9_companion.go b/internal/fs/drive9_companion.go index 949f496..a116c54 100644 --- a/internal/fs/drive9_companion.go +++ b/internal/fs/drive9_companion.go @@ -1193,6 +1193,9 @@ func (s Service) drive9MountVault(ctx context.Context, opts VaultMountOptions) ( _, _ = s.drive9Run(ctx, opts.Profile, []string{"umount", opts.MountPath}, false) return MountResult{}, err } + if err := s.waitForMountReady(ctx, opts.MountPath, opts.ReadyTimeout, fmt.Sprintf("; to stop it run: ti fs-vault unmount-vault --mount-path %q", opts.MountPath)); err != nil { + return MountResult{}, err + } return MountResult{Status: "mounted", Profile: profileName(opts.Profile), FileSystemName: "vault", MountPath: opts.MountPath, RemotePath: "/n/vault", Driver: "fuse"}, nil } @@ -1296,6 +1299,9 @@ func (s Service) drive9MountFileSystem(ctx context.Context, opts MountFileSystem _, _ = s.drive9Run(ctx, opts.Profile, []string{"umount", opts.MountPath}, false) return MountResult{}, err } + if err := s.waitForMountReady(ctx, opts.MountPath, opts.ReadyTimeout, fmt.Sprintf("; to stop it run: ti fs unmount-file-system --mount-path %q", opts.MountPath)); err != nil { + return MountResult{}, err + } endpoint, _ := s.resolveFS(opts.Profile) driver := drive9MountedDriver(result.Stderr, opts.Driver) if driver == "" { diff --git a/internal/fs/drive9_companion_test.go b/internal/fs/drive9_companion_test.go index 5d9eb45..70e3325 100644 --- a/internal/fs/drive9_companion_test.go +++ b/internal/fs/drive9_companion_test.go @@ -866,6 +866,10 @@ func main() { fmt.Fprintln(os.Stderr, "mount: drive9 mount: background mount exited before becoming ready") os.Exit(1) } + if mountPath := args[len(args)-1]; len(args) >= 2 && len(mountPath) > 0 && mountPath[0] != '-' { + _ = os.MkdirAll(mountPath, 0o755) + _ = os.WriteFile(mountPath+string(os.PathSeparator)+".drive9-mounted", []byte("fake mount ready\n"), 0o644) + } fmt.Fprintln(os.Stderr, "drive9: mount running in background") fmt.Fprintln(os.Stderr, "drive9: unmount with drive9 umount /workspace") case len(args) >= 3 && args[0] == "admin" && args[1] == "tenant" && args[2] == "delete": diff --git a/internal/fs/mountready.go b/internal/fs/mountready.go new file mode 100644 index 0000000..65b2259 --- /dev/null +++ b/internal/fs/mountready.go @@ -0,0 +1,109 @@ +package fs + +import ( + "context" + "errors" + "fmt" + "io" + "os" + "time" + + "github.com/tidbcloud/ti-cli/internal/apperr" +) + +const ( + defaultMountReadyTimeout = 30 * time.Second + defaultMountReadyPollInterval = 100 * time.Millisecond + mountReadyProbeTimeout = 2 * time.Second +) + +// probeMountPointReady reports whether mountPath is a directory that the +// kernel can list through the mounted filesystem. A background mount is only +// usable once stat and readdir are served from the mount root, which can lag +// the mount process exit while the companion runtime warms up. +func probeMountPointReady(mountPath string) error { + info, err := os.Stat(mountPath) + if err != nil { + return err + } + if !info.IsDir() { + return fmt.Errorf("mount path %q is not a directory", mountPath) + } + dir, err := os.Open(mountPath) + if err != nil { + return err + } + defer dir.Close() + if _, err := dir.Readdirnames(1); err != nil && !errors.Is(err, io.EOF) { + return err + } + return nil +} + +// probeMountPointOnce bounds one readiness probe. A wedged mount can block +// readdir indefinitely, so slow probes are abandoned and treated as not +// ready; the abandoned goroutine finishes and closes its handle once the +// syscall returns. +func probeMountPointOnce(mountPath string) error { + done := make(chan error, 1) + go func() { + done <- probeMountPointReady(mountPath) + }() + select { + case err := <-done: + return err + case <-time.After(mountReadyProbeTimeout): + return fmt.Errorf("mount readiness probe did not complete within %s", mountReadyProbeTimeout) + } +} + +func (s Service) waitForMountReady(ctx context.Context, mountPath string, timeout time.Duration, stopHint string) error { + if timeout <= 0 { + timeout = defaultMountReadyTimeout + } + interval := s.MountReadyPollInterval + if interval <= 0 { + interval = defaultMountReadyPollInterval + } + deadline := time.Now().Add(timeout) + var lastErr error + for { + if err := probeMountPointOnce(mountPath); err != nil { + lastErr = err + } else { + return nil + } + remaining := time.Until(deadline) + if remaining <= 0 { + break + } + wait := interval + if remaining < wait { + wait = remaining + } + timer := time.NewTimer(wait) + select { + case <-ctx.Done(): + timer.Stop() + return mountReadyCanceled(mountPath, stopHint, ctx.Err()) + case <-timer.C: + } + } + return apperr.Wrap( + "fs.mount_ready_timeout", + "runtime", + 1, + fmt.Sprintf("background mount at %q did not become readable within %s; the mount is still running%s", mountPath, timeout, stopHint), + lastErr, + ) +} + +func mountReadyCanceled(mountPath, stopHint string, cause error) error { + return apperr.Wrap( + "fs.mount_ready_canceled", + "runtime", + 1, + fmt.Sprintf("waiting for the background mount at %q to become readable was canceled; the mount is still running%s", mountPath, stopHint), + cause, + ) +} diff --git a/internal/fs/mountready_test.go b/internal/fs/mountready_test.go new file mode 100644 index 0000000..c995030 --- /dev/null +++ b/internal/fs/mountready_test.go @@ -0,0 +1,173 @@ +package fs + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/tidbcloud/ti-cli/internal/apperr" + "github.com/tidbcloud/ti-cli/internal/fs/mountlocator" +) + +func TestProbeMountPointReady(t *testing.T) { + t.Run("directory with entries is ready", func(t *testing.T) { + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "entry.txt"), []byte("x"), 0o644); err != nil { + t.Fatal(err) + } + if err := probeMountPointReady(dir); err != nil { + t.Fatalf("expected ready, got %v", err) + } + }) + t.Run("empty directory is ready", func(t *testing.T) { + if err := probeMountPointReady(t.TempDir()); err != nil { + t.Fatalf("expected ready, got %v", err) + } + }) + t.Run("missing path is not ready", func(t *testing.T) { + if err := probeMountPointReady(filepath.Join(t.TempDir(), "missing")); err == nil { + t.Fatal("expected error for missing path") + } + }) + t.Run("file path is not a mount point", func(t *testing.T) { + path := filepath.Join(t.TempDir(), "not-a-dir") + if err := os.WriteFile(path, []byte("x"), 0o644); err != nil { + t.Fatal(err) + } + if err := probeMountPointReady(path); err == nil { + t.Fatal("expected error for file path") + } + }) +} + +func TestWaitForMountReadySucceedsOnceReadable(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "mp") + go func() { + time.Sleep(30 * time.Millisecond) + if err := os.MkdirAll(path, 0o755); err != nil { + t.Error(err) + } + }() + service := Service{MountReadyPollInterval: 5 * time.Millisecond} + if err := service.waitForMountReady(context.Background(), path, 2*time.Second, ""); err != nil { + t.Fatalf("expected ready, got %v", err) + } +} + +func TestWaitForMountReadyTimeout(t *testing.T) { + service := Service{MountReadyPollInterval: 5 * time.Millisecond} + path := filepath.Join(t.TempDir(), "missing") + err := service.waitForMountReady(context.Background(), path, 50*time.Millisecond, "; to stop it run: ti fs unmount-file-system --mount-path x") + if err == nil { + t.Fatal("expected timeout error") + } + if apperr.CodeFor(err) != "fs.mount_ready_timeout" { + t.Fatalf("unexpected error code %q: %v", apperr.CodeFor(err), err) + } + if !strings.Contains(err.Error(), "ti fs unmount-file-system --mount-path x") { + t.Fatalf("error lacks stop hint: %v", err) + } +} + +func TestWaitForMountReadyCanceled(t *testing.T) { + service := Service{MountReadyPollInterval: 5 * time.Millisecond} + ctx, cancel := context.WithCancel(context.Background()) + go func() { + time.Sleep(20 * time.Millisecond) + cancel() + }() + err := service.waitForMountReady(ctx, filepath.Join(t.TempDir(), "missing"), 2*time.Second, "") + if apperr.CodeFor(err) != "fs.mount_ready_canceled" { + t.Fatalf("unexpected error code %q: %v", apperr.CodeFor(err), err) + } +} + +func TestDrive9MountWaitsUntilMountPointIsReadable(t *testing.T) { + home := t.TempDir() + companion, recordPath := buildFakeDrive9(t) + t.Setenv("TI_FAKE_DRIVE9_RECORD", recordPath) + service := testCompanionService(home, companion) + service.MountReadyPollInterval = 5 * time.Millisecond + mountPath := filepath.Join(t.TempDir(), "workspace") + + result, err := service.MountFileSystem(context.Background(), MountFileSystemOptions{ + Profile: dataProfile(), + FileSystemName: "workspace", + MountPath: mountPath, + RemotePath: "/", + ReadyTimeout: 2 * time.Second, + }) + if err != nil { + t.Fatalf("MountFileSystem failed: %v", err) + } + if result.Status != "mounted" { + t.Fatalf("unexpected status %q", result.Status) + } + if info, err := os.Stat(mountPath); err != nil || !info.IsDir() { + t.Fatalf("mount path was not made readable: %v", err) + } +} + +func TestDrive9MountTimeoutPreservesMountLocator(t *testing.T) { + home := t.TempDir() + companion, recordPath := buildFakeDrive9(t) + t.Setenv("TI_FAKE_DRIVE9_RECORD", recordPath) + service := testCompanionService(home, companion) + service.MountReadyPollInterval = 5 * time.Millisecond + // A file at the mount path never becomes a readable mount root, so the + // readiness wait must time out while keeping the mount and its locator. + mountPath := filepath.Join(t.TempDir(), "not-a-dir") + if err := os.WriteFile(mountPath, []byte("x"), 0o644); err != nil { + t.Fatal(err) + } + + _, err := service.MountFileSystem(context.Background(), MountFileSystemOptions{ + Profile: dataProfile(), + FileSystemName: "workspace", + MountPath: mountPath, + RemotePath: "/", + ReadyTimeout: 80 * time.Millisecond, + }) + if apperr.CodeFor(err) != "fs.mount_ready_timeout" { + t.Fatalf("unexpected error code %q: %v", apperr.CodeFor(err), err) + } + if !strings.Contains(err.Error(), "ti fs unmount-file-system --mount-path") { + t.Fatalf("error lacks unmount guidance: %v", err) + } + if _, _, locErr := mountlocator.Read(home, mountPath); locErr != nil { + t.Fatalf("mount locator must survive a readiness timeout: %v", locErr) + } + requireFakeDrive9Call(t, recordPath, "mount") +} + +func TestDrive9VaultMountTimeoutPreservesMountLocator(t *testing.T) { + home := t.TempDir() + companion, _ := buildFakeDrive9(t) + t.Setenv("TI_FAKE_DRIVE9_RECORD", filepath.Join(t.TempDir(), "unused.jsonl")) + service := testCompanionService(home, companion) + service.MountReadyPollInterval = 5 * time.Millisecond + mountPath := filepath.Join(t.TempDir(), "not-a-dir") + if err := os.WriteFile(mountPath, []byte("x"), 0o644); err != nil { + t.Fatal(err) + } + + _, err := service.MountVault(context.Background(), VaultMountOptions{ + Profile: dataProfile(), + MountPath: mountPath, + VaultToken: "vault-token", + ReadyTimeout: 80 * time.Millisecond, + }) + if apperr.CodeFor(err) != "fs.mount_ready_timeout" { + t.Fatalf("unexpected error code %q: %v", apperr.CodeFor(err), err) + } + if !strings.Contains(err.Error(), "ti fs-vault unmount-vault --mount-path") { + t.Fatalf("error lacks unmount guidance: %v", err) + } + if _, _, locErr := mountlocator.Read(home, mountPath); locErr != nil { + t.Fatalf("mount locator must survive a readiness timeout: %v", locErr) + } +} From d668e2e0a7e9a85076ed2ff143e701328ff8777f Mon Sep 17 00:00:00 2001 From: mornyx Date: Fri, 2 Oct 2026 13:15:35 +0800 Subject: [PATCH 2/3] fix(fs): allow slow cold WebDAV first readdir in mount readiness probe Live verification against a real backend caught a false timeout: a WebDAV mount's first directory listing crosses the companion proxy and the remote region and can legitimately exceed the previous 2s per-probe bound, so every probe was abandoned as not-ready until the overall --ready-timeout fired even though the mount was healthy. Raise the per-probe bound to 10s; its purpose is only to stop a wedged mount from blocking a probe forever. --- internal/fs/mountready.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/internal/fs/mountready.go b/internal/fs/mountready.go index 65b2259..83bab6f 100644 --- a/internal/fs/mountready.go +++ b/internal/fs/mountready.go @@ -14,7 +14,11 @@ import ( const ( defaultMountReadyTimeout = 30 * time.Second defaultMountReadyPollInterval = 100 * time.Millisecond - mountReadyProbeTimeout = 2 * time.Second + // mountReadyProbeTimeout only guards against a wedged mount blocking a + // probe forever. It must stay well above a healthy cold first readdir: + // a WebDAV mount's initial directory listing crosses the companion + // proxy and the remote region and can legitimately take seconds. + mountReadyProbeTimeout = 10 * time.Second ) // probeMountPointReady reports whether mountPath is a directory that the From bf07eb0f55f15b99cd77f45bbb9d63350403ef15 Mon Sep 17 00:00:00 2001 From: mornyx Date: Fri, 2 Oct 2026 14:30:41 +0800 Subject: [PATCH 3/3] fix(fs): bound mount probes by ready-timeout and require real mount evidence Address review feedback on the mount readiness wait: - Every probe is now capped by min(10s, remaining --ready-timeout) and observes the command context, so a short --ready-timeout no longer waits a full probe bound and Ctrl-C during a blocked probe returns promptly instead of after the probe budget. - Readiness now requires active-mount evidence, not just a readable directory: st_dev comparison against the parent on macOS and /proc/self/mountinfo on Linux. Unsupported platforms fall back to readability only because ti mounts are unsupported there. - New regression: a companion that exits 0 without mounting leaves a plain directory that must not satisfy readiness. A darwin test mounts a real mount_webdav volume and proves the predicate passes only after the kernel mount exists. - Black-box e2e fake-companion runs enable mount evidence via the hidden TI_TEST_FAKE_MOUNT_READY control, gated by TI_ALLOW_TEST_ENDPOINTS like the other TI_TEST_* overrides. --- e2e/cli_test.go | 4 + internal/fs/control.go | 5 + internal/fs/drive9_companion_test.go | 8 +- internal/fs/mountready.go | 78 ++++++++--- internal/fs/mountready_darwin.go | 31 +++++ internal/fs/mountready_linux.go | 70 ++++++++++ internal/fs/mountready_other.go | 10 ++ internal/fs/mountready_test.go | 197 +++++++++++++++++++++++++-- internal/fs/mountready_testenv.go | 12 ++ 9 files changed, 379 insertions(+), 36 deletions(-) create mode 100644 internal/fs/mountready_darwin.go create mode 100644 internal/fs/mountready_linux.go create mode 100644 internal/fs/mountready_other.go create mode 100644 internal/fs/mountready_testenv.go diff --git a/e2e/cli_test.go b/e2e/cli_test.go index 6def1a8..8f06bff 100644 --- a/e2e/cli_test.go +++ b/e2e/cli_test.go @@ -825,6 +825,7 @@ func TestFSRemoteInventoryAndIDCredentialSelectionAcrossCommandFamilies(t *testi baseEnv := []string{ "HOME=" + home, "TI_DRIVE9_BIN=" + companion, + "TI_TEST_FAKE_MOUNT_READY=1", "FAKE_DRIVE9_RECORD=" + recordPath, "TI_ALLOW_TEST_ENDPOINTS=1", "TI_TEST_FS_MANIFEST_URL=" + manifestServer.URL, @@ -1097,6 +1098,7 @@ func TestFSConfigurationFreeAccess(t *testing.T) { "HOME=" + home, "TI_LOGGING=on", "TI_DRIVE9_BIN=" + companion, + "TI_TEST_FAKE_MOUNT_READY=1", "FAKE_DRIVE9_RECORD=" + recordPath, "TI_ALLOW_TEST_ENDPOINTS=1", "TI_TEST_FS_MANIFEST_URL=" + manifestServer.URL, @@ -1207,6 +1209,7 @@ func TestFSLayerForkWorkflowCommands(t *testing.T) { env := []string{ "HOME=" + home, "TI_DRIVE9_BIN=" + companion, + "TI_TEST_FAKE_MOUNT_READY=1", "FAKE_DRIVE9_RECORD=" + recordPath, "TI_ALLOW_TEST_ENDPOINTS=1", "TI_TEST_FS_MANIFEST_URL=" + manifestServer.URL, @@ -1295,6 +1298,7 @@ func TestFSImportFileSystemToken(t *testing.T) { baseEnv := []string{ "HOME=" + home, "TI_DRIVE9_BIN=" + companion, + "TI_TEST_FAKE_MOUNT_READY=1", "FAKE_DRIVE9_RECORD=" + recordPath, "FAKE_DRIVE9_EXPECT_API_KEY=" + token, "TI_ALLOW_TEST_ENDPOINTS=1", diff --git a/internal/fs/control.go b/internal/fs/control.go index 1f3fbc4..ac9df78 100644 --- a/internal/fs/control.go +++ b/internal/fs/control.go @@ -38,6 +38,11 @@ type Service struct { Stdin io.Reader Stdout io.Writer Stderr io.Writer + + // mountPointActive overrides active-mount evidence detection. It exists + // for tests of unrelated mount behaviors whose fake companion cannot + // create a real kernel mount; production leaves it nil. + mountPointActive func(string) (bool, error) } type CreateFileSystemOptions struct { diff --git a/internal/fs/drive9_companion_test.go b/internal/fs/drive9_companion_test.go index 70e3325..e38ce11 100644 --- a/internal/fs/drive9_companion_test.go +++ b/internal/fs/drive9_companion_test.go @@ -414,7 +414,9 @@ func TestDrive9LayerCheckpointMountIsReadOnlyAndRecorded(t *testing.T) { companion, recordPath := buildFakeDrive9(t) t.Setenv("TI_FAKE_DRIVE9_RECORD", recordPath) mountPath := filepath.Join(t.TempDir(), "checkpoint") - result, err := testCompanionService(home, companion).MountFileSystem(context.Background(), MountFileSystemOptions{ + checkpointService := testCompanionService(home, companion) + checkpointService.mountPointActive = trueMountEvidence + result, err := checkpointService.MountFileSystem(context.Background(), MountFileSystemOptions{ Profile: dataProfile(), MountPath: mountPath, RemotePath: "/research/q3-market", Driver: "fuse", LayerRef: "style-analyst", CheckpointID: "v5", }) if err != nil { @@ -535,6 +537,7 @@ func TestDrive9MountLocatorRoutesDrainAndUnmountWithoutCredentials(t *testing.T) companion, recordPath := buildFakeDrive9(t) t.Setenv("TI_FAKE_DRIVE9_RECORD", recordPath) service := testCompanionService(home, companion) + service.mountPointActive = trueMountEvidence mountPath := filepath.Join(t.TempDir(), "workspace") profile := dataProfile() @@ -599,6 +602,7 @@ func TestDrive9VaultMountUsesBackgroundMode(t *testing.T) { companion, recordPath := buildFakeDrive9(t) t.Setenv("TI_FAKE_DRIVE9_RECORD", recordPath) service := testCompanionService(home, companion) + service.mountPointActive = trueMountEvidence service.Stdout = &bytes.Buffer{} service.Stderr = &bytes.Buffer{} mountPath := filepath.Join(t.TempDir(), "vault") @@ -631,6 +635,7 @@ func TestDrive9MountSuppressesCompanionSuccessChatter(t *testing.T) { var stdout bytes.Buffer var stderr bytes.Buffer service := testCompanionService(home, companion) + service.mountPointActive = trueMountEvidence service.Stdout = &stdout service.Stderr = &stderr @@ -673,6 +678,7 @@ func TestDrive9FailedUnmountPreservesMountLocator(t *testing.T) { companion, recordPath := buildFakeDrive9(t) t.Setenv("TI_FAKE_DRIVE9_RECORD", recordPath) service := testCompanionService(home, companion) + service.mountPointActive = trueMountEvidence mountPath := filepath.Join(t.TempDir(), "workspace") if _, err := service.MountFileSystem(context.Background(), MountFileSystemOptions{ Profile: dataProfile(), diff --git a/internal/fs/mountready.go b/internal/fs/mountready.go index 83bab6f..7985af5 100644 --- a/internal/fs/mountready.go +++ b/internal/fs/mountready.go @@ -14,18 +14,24 @@ import ( const ( defaultMountReadyTimeout = 30 * time.Second defaultMountReadyPollInterval = 100 * time.Millisecond - // mountReadyProbeTimeout only guards against a wedged mount blocking a - // probe forever. It must stay well above a healthy cold first readdir: - // a WebDAV mount's initial directory listing crosses the companion - // proxy and the remote region and can legitimately take seconds. + // mountReadyProbeTimeout bounds a single probe so a wedged mount cannot + // block one probe forever. It is an upper bound only: every probe is also + // capped by the remaining --ready-timeout budget and the command context, + // so a short --ready-timeout never waits a full probe bound. It must stay + // well above a healthy cold first readdir: a WebDAV mount's initial + // directory listing crosses the companion proxy and the remote region and + // can legitimately take seconds. mountReadyProbeTimeout = 10 * time.Second ) -// probeMountPointReady reports whether mountPath is a directory that the -// kernel can list through the mounted filesystem. A background mount is only -// usable once stat and readdir are served from the mount root, which can lag -// the mount process exit while the companion runtime warms up. -func probeMountPointReady(mountPath string) error { +// errMountReadyProbeExhausted marks a probe whose budget elapsed before the +// probe answered. It means "not ready yet", not a mount failure. +var errMountReadyProbeExhausted = errors.New("mount readiness probe budget exhausted") + +// probeMountPointReady reports whether mountPath is an active mount that the +// kernel can list. A background mount is only usable once the mount exists, +// is visible in the mount table, and readdir is served from the mount root. +func probeMountPointReady(mountPath string, mounted func(string) (bool, error)) error { info, err := os.Stat(mountPath) if err != nil { return err @@ -33,6 +39,13 @@ func probeMountPointReady(mountPath string) error { if !info.IsDir() { return fmt.Errorf("mount path %q is not a directory", mountPath) } + active, err := mounted(mountPath) + if err != nil { + return fmt.Errorf("mount evidence for %q: %w", mountPath, err) + } + if !active { + return fmt.Errorf("mount path %q is not an active mount", mountPath) + } dir, err := os.Open(mountPath) if err != nil { return err @@ -44,23 +57,33 @@ func probeMountPointReady(mountPath string) error { return nil } -// probeMountPointOnce bounds one readiness probe. A wedged mount can block -// readdir indefinitely, so slow probes are abandoned and treated as not -// ready; the abandoned goroutine finishes and closes its handle once the -// syscall returns. -func probeMountPointOnce(mountPath string) error { +// probeMountPointOnce runs one probe bounded by budget and ctx. A blocked or +// wedged probe is abandoned when either expires; the abandoned goroutine +// finishes and closes its handle once the underlying syscall returns. +func probeMountPointOnce(ctx context.Context, mountPath string, budget time.Duration, mounted func(string) (bool, error)) error { done := make(chan error, 1) go func() { - done <- probeMountPointReady(mountPath) + done <- probeMountPointReady(mountPath, mounted) }() + timer := time.NewTimer(budget) + defer timer.Stop() select { case err := <-done: return err - case <-time.After(mountReadyProbeTimeout): - return fmt.Errorf("mount readiness probe did not complete within %s", mountReadyProbeTimeout) + case <-ctx.Done(): + return ctx.Err() + case <-timer.C: + return errMountReadyProbeExhausted } } +func (s Service) mountEvidence() func(string) (bool, error) { + if s.mountPointActive != nil { + return s.mountPointActive + } + return defaultMountPointActive +} + func (s Service) waitForMountReady(ctx context.Context, mountPath string, timeout time.Duration, stopHint string) error { if timeout <= 0 { timeout = defaultMountReadyTimeout @@ -69,15 +92,28 @@ func (s Service) waitForMountReady(ctx context.Context, mountPath string, timeou if interval <= 0 { interval = defaultMountReadyPollInterval } + mounted := s.mountEvidence() deadline := time.Now().Add(timeout) var lastErr error for { - if err := probeMountPointOnce(mountPath); err != nil { - lastErr = err - } else { + remaining := time.Until(deadline) + if remaining <= 0 { + break + } + budget := mountReadyProbeTimeout + if remaining < budget { + budget = remaining + } + err := probeMountPointOnce(ctx, mountPath, budget, mounted) + switch { + case err == nil: return nil + case errors.Is(err, context.Canceled), errors.Is(err, context.DeadlineExceeded): + return mountReadyCanceled(mountPath, stopHint, err) + default: + lastErr = err } - remaining := time.Until(deadline) + remaining = time.Until(deadline) if remaining <= 0 { break } diff --git a/internal/fs/mountready_darwin.go b/internal/fs/mountready_darwin.go new file mode 100644 index 0000000..84e1fdc --- /dev/null +++ b/internal/fs/mountready_darwin.go @@ -0,0 +1,31 @@ +//go:build darwin + +package fs + +import ( + "os" + "path/filepath" + "syscall" +) + +// defaultMountPointActive reports whether mountPath is the root of an active +// mount. On macOS a mount root sits on a different filesystem than its parent +// directory, so unequal st_dev values prove an active mount (FUSE, WebDAV, +// disk images); a plain subdirectory shares the parent's device. +func defaultMountPointActive(mountPath string) (bool, error) { + if testFakeMountReady() { + return true, nil + } + if mountPath == string(os.PathSeparator) { + return true, nil + } + info, err := os.Stat(mountPath) + if err != nil { + return false, err + } + parent, err := os.Stat(filepath.Dir(mountPath)) + if err != nil { + return false, err + } + return info.Sys().(*syscall.Stat_t).Dev != parent.Sys().(*syscall.Stat_t).Dev, nil +} diff --git a/internal/fs/mountready_linux.go b/internal/fs/mountready_linux.go new file mode 100644 index 0000000..2828941 --- /dev/null +++ b/internal/fs/mountready_linux.go @@ -0,0 +1,70 @@ +//go:build linux + +package fs + +import ( + "os" + "path/filepath" + "strconv" + "strings" +) + +// defaultMountPointActive reports whether mountPath is the root of an active +// mount by checking /proc/self/mountinfo, the kernel's authoritative mount +// table. The companion mounts FUSE filesystems, which always appear there. +func defaultMountPointActive(mountPath string) (bool, error) { + if testFakeMountReady() { + return true, nil + } + candidates := mountPathCandidates(mountPath) + data, err := os.ReadFile("/proc/self/mountinfo") + if err != nil { + return false, err + } + for _, line := range strings.Split(string(data), "\n") { + fields := strings.Split(line, " ") + if len(fields) < 5 { + continue + } + if _, ok := candidates[unescapeMountinfoPath(fields[4])]; ok { + return true, nil + } + } + return false, nil +} + +func mountPathCandidates(mountPath string) map[string]struct{} { + candidates := map[string]struct{}{} + for _, path := range []string{mountPath, filepath.Clean(mountPath)} { + if path != "" { + candidates[path] = struct{}{} + } + } + if resolved, err := filepath.EvalSymlinks(mountPath); err == nil { + candidates[resolved] = struct{}{} + } + if abs, err := filepath.Abs(mountPath); err == nil { + candidates[abs] = struct{}{} + } + return candidates +} + +// unescapeMountinfoPath decodes the octal escapes (for example \040 for a +// space) that /proc/self/mountinfo uses for special characters. +func unescapeMountinfoPath(path string) string { + if !strings.Contains(path, "\\") { + return path + } + var out strings.Builder + for i := 0; i < len(path); i++ { + if path[i] == '\\' && i+3 < len(path) { + if value, err := strconv.ParseUint(path[i+1:i+4], 8, 8); err == nil { + out.WriteByte(byte(value)) + i += 3 + continue + } + } + out.WriteByte(path[i]) + } + return out.String() +} diff --git a/internal/fs/mountready_other.go b/internal/fs/mountready_other.go new file mode 100644 index 0000000..dc9d282 --- /dev/null +++ b/internal/fs/mountready_other.go @@ -0,0 +1,10 @@ +//go:build !darwin && !linux + +package fs + +// ti fs background mounts are supported on macOS and Linux only. On other +// platforms there is no supported driver to detect, so mount evidence cannot +// be observed and readiness falls back to the readability probe alone. +func defaultMountPointActive(string) (bool, error) { + return true, nil +} diff --git a/internal/fs/mountready_test.go b/internal/fs/mountready_test.go index c995030..c179fdc 100644 --- a/internal/fs/mountready_test.go +++ b/internal/fs/mountready_test.go @@ -2,33 +2,50 @@ package fs import ( "context" + "fmt" + "net" + "net/http" "os" + "os/exec" "path/filepath" + "runtime" "strings" "testing" "time" "github.com/tidbcloud/ti-cli/internal/apperr" "github.com/tidbcloud/ti-cli/internal/fs/mountlocator" + "golang.org/x/net/webdav" ) +func trueMountEvidence(string) (bool, error) { return true, nil } + +func blockingMountEvidence(d time.Duration) func(string) (bool, error) { + return func(string) (bool, error) { + time.Sleep(d) + return true, nil + } +} + func TestProbeMountPointReady(t *testing.T) { - t.Run("directory with entries is ready", func(t *testing.T) { + t.Run("plain directory is not ready without mount evidence", func(t *testing.T) { dir := t.TempDir() if err := os.WriteFile(filepath.Join(dir, "entry.txt"), []byte("x"), 0o644); err != nil { t.Fatal(err) } - if err := probeMountPointReady(dir); err != nil { - t.Fatalf("expected ready, got %v", err) + err := probeMountPointReady(dir, defaultMountPointActive) + if err == nil || !strings.Contains(err.Error(), "not an active mount") { + t.Fatalf("plain directory must not be ready, got %v", err) } }) - t.Run("empty directory is ready", func(t *testing.T) { - if err := probeMountPointReady(t.TempDir()); err != nil { - t.Fatalf("expected ready, got %v", err) + t.Run("empty directory is not ready without mount evidence", func(t *testing.T) { + err := probeMountPointReady(t.TempDir(), defaultMountPointActive) + if err == nil || !strings.Contains(err.Error(), "not an active mount") { + t.Fatalf("empty plain directory must not be ready, got %v", err) } }) t.Run("missing path is not ready", func(t *testing.T) { - if err := probeMountPointReady(filepath.Join(t.TempDir(), "missing")); err == nil { + if err := probeMountPointReady(filepath.Join(t.TempDir(), "missing"), trueMountEvidence); err == nil { t.Fatal("expected error for missing path") } }) @@ -37,10 +54,79 @@ func TestProbeMountPointReady(t *testing.T) { if err := os.WriteFile(path, []byte("x"), 0o644); err != nil { t.Fatal(err) } - if err := probeMountPointReady(path); err == nil { + if err := probeMountPointReady(path, trueMountEvidence); err == nil { t.Fatal("expected error for file path") } }) + t.Run("readability with injected evidence", func(t *testing.T) { + dir := t.TempDir() + if err := probeMountPointReady(dir, trueMountEvidence); err != nil { + t.Fatalf("expected ready with evidence, got %v", err) + } + }) +} + +func TestDefaultMountPointActiveRejectsPlainDirectory(t *testing.T) { + active, err := defaultMountPointActive(t.TempDir()) + if err != nil { + t.Fatalf("detection failed: %v", err) + } + if active { + t.Fatal("a plain directory must not count as an active mount") + } +} + +// TestDefaultMountPointActiveAcceptsRealWebDAVMount proves the evidence +// predicate passes only after a real kernel mount exists. It failed against +// the pre-regression behavior where any readable directory counted as ready. +func TestDefaultMountPointActiveAcceptsRealWebDAVMount(t *testing.T) { + if runtime.GOOS != "darwin" { + t.Skip("real-mount evidence test uses macOS mount_webdav") + } + if _, err := exec.LookPath("mount_webdav"); err != nil { + t.Skip("mount_webdav is not available") + } + // The served tree must not contain the mountpoint: listing a share that + // contains its own mount root recurses kernel WebDAV into itself and + // deadlocks the readdir. + serveRoot := t.TempDir() + if err := os.WriteFile(filepath.Join(serveRoot, "seed.txt"), []byte("x"), 0o644); err != nil { + t.Fatal(err) + } + mountParent := t.TempDir() + mp := filepath.Join(mountParent, "mp") + if err := os.MkdirAll(mp, 0o755); err != nil { + t.Fatal(err) + } + + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Skipf("cannot listen for WebDAV server: %v", err) + } + server := &http.Server{Handler: &webdav.Handler{ + FileSystem: webdav.Dir(serveRoot), + LockSystem: webdav.NewMemLS(), + }} + go func() { _ = server.Serve(listener) }() + defer func() { _ = server.Close() }() + + serverURL := fmt.Sprintf("http://127.0.0.1:%d/", listener.Addr().(*net.TCPAddr).Port) + mountCmd := exec.Command("mount_webdav", serverURL, mp) + if out, err := mountCmd.CombinedOutput(); err != nil { + _ = exec.Command("umount", mp).Run() + t.Skipf("mount_webdav failed in this environment (%v): %s", err, out) + } + defer func() { _ = exec.Command("umount", mp).Run() }() + + if active, err := defaultMountPointActive(mp); err != nil || !active { + t.Fatalf("real WebDAV mount must count as active: active=%v err=%v", active, err) + } + if err := probeMountPointOnce(context.Background(), mp, 10*time.Second, defaultMountPointActive); err != nil { + t.Fatalf("real WebDAV mount must be ready: %v", err) + } + if _, err := os.ReadDir(mp); err != nil { + t.Fatalf("real WebDAV mount must list entries: %v", err) + } } func TestWaitForMountReadySucceedsOnceReadable(t *testing.T) { @@ -52,14 +138,17 @@ func TestWaitForMountReadySucceedsOnceReadable(t *testing.T) { t.Error(err) } }() - service := Service{MountReadyPollInterval: 5 * time.Millisecond} + service := Service{ + MountReadyPollInterval: 5 * time.Millisecond, + mountPointActive: trueMountEvidence, + } if err := service.waitForMountReady(context.Background(), path, 2*time.Second, ""); err != nil { t.Fatalf("expected ready, got %v", err) } } func TestWaitForMountReadyTimeout(t *testing.T) { - service := Service{MountReadyPollInterval: 5 * time.Millisecond} + service := Service{MountReadyPollInterval: 5 * time.Millisecond, mountPointActive: trueMountEvidence} path := filepath.Join(t.TempDir(), "missing") err := service.waitForMountReady(context.Background(), path, 50*time.Millisecond, "; to stop it run: ti fs unmount-file-system --mount-path x") if err == nil { @@ -74,7 +163,7 @@ func TestWaitForMountReadyTimeout(t *testing.T) { } func TestWaitForMountReadyCanceled(t *testing.T) { - service := Service{MountReadyPollInterval: 5 * time.Millisecond} + service := Service{MountReadyPollInterval: 5 * time.Millisecond, mountPointActive: trueMountEvidence} ctx, cancel := context.WithCancel(context.Background()) go func() { time.Sleep(20 * time.Millisecond) @@ -86,12 +175,65 @@ func TestWaitForMountReadyCanceled(t *testing.T) { } } +// TestWaitForMountReadyBoundsBlockedProbeByTimeout pins the review blocker: +// a blocked probe must be abandoned at the --ready-timeout budget, not at the +// full per-probe bound. Against the previous implementation (10s fixed probe +// budget) this test failed because the wait took the full probe bound. +func TestWaitForMountReadyBoundsBlockedProbeByTimeout(t *testing.T) { + service := Service{ + MountReadyPollInterval: 5 * time.Millisecond, + mountPointActive: blockingMountEvidence(3 * time.Second), + } + dir := t.TempDir() + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + start := time.Now() + err := service.waitForMountReady(context.Background(), dir, 200*time.Millisecond, "") + elapsed := time.Since(start) + if apperr.CodeFor(err) != "fs.mount_ready_timeout" { + t.Fatalf("unexpected error code %q: %v", apperr.CodeFor(err), err) + } + if elapsed > 1500*time.Millisecond { + t.Fatalf("short --ready-timeout must bound a blocked probe: took %s", elapsed) + } +} + +// TestWaitForMountReadyCancelsBlockedProbe pins the second half of the review +// blocker: Ctrl-C during a blocked probe must return promptly instead of +// waiting out the full probe budget. +func TestWaitForMountReadyCancelsBlockedProbe(t *testing.T) { + service := Service{ + MountReadyPollInterval: 5 * time.Millisecond, + mountPointActive: blockingMountEvidence(5 * time.Second), + } + dir := t.TempDir() + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + ctx, cancel := context.WithCancel(context.Background()) + go func() { + time.Sleep(100 * time.Millisecond) + cancel() + }() + start := time.Now() + err := service.waitForMountReady(ctx, dir, 30*time.Second, "") + elapsed := time.Since(start) + if apperr.CodeFor(err) != "fs.mount_ready_canceled" { + t.Fatalf("unexpected error code %q: %v", apperr.CodeFor(err), err) + } + if elapsed > 1500*time.Millisecond { + t.Fatalf("cancellation during a blocked probe must be prompt: took %s", elapsed) + } +} + func TestDrive9MountWaitsUntilMountPointIsReadable(t *testing.T) { home := t.TempDir() companion, recordPath := buildFakeDrive9(t) t.Setenv("TI_FAKE_DRIVE9_RECORD", recordPath) service := testCompanionService(home, companion) service.MountReadyPollInterval = 5 * time.Millisecond + service.mountPointActive = trueMountEvidence mountPath := filepath.Join(t.TempDir(), "workspace") result, err := service.MountFileSystem(context.Background(), MountFileSystemOptions{ @@ -112,10 +254,38 @@ func TestDrive9MountWaitsUntilMountPointIsReadable(t *testing.T) { } } +// TestDrive9MountRequiresActiveMountEvidence is the companion-exits-0-but- +// nothing-mounted regression: the fake companion creates only a plain +// directory, which must not satisfy readiness even though it is readable. +// The previous implementation reported status "mounted" here. +func TestDrive9MountRequiresActiveMountEvidence(t *testing.T) { + home := t.TempDir() + companion, _ := buildFakeDrive9(t) + service := testCompanionService(home, companion) + service.MountReadyPollInterval = 5 * time.Millisecond + mountPath := filepath.Join(t.TempDir(), "workspace") + + _, err := service.MountFileSystem(context.Background(), MountFileSystemOptions{ + Profile: dataProfile(), + FileSystemName: "workspace", + MountPath: mountPath, + RemotePath: "/", + ReadyTimeout: 80 * time.Millisecond, + }) + if apperr.CodeFor(err) != "fs.mount_ready_timeout" { + t.Fatalf("plain directory must not satisfy mount readiness, got %v", err) + } + if !strings.Contains(err.Error(), "not an active mount") && !strings.Contains(err.Error(), "did not become readable") { + t.Fatalf("unexpected error detail: %v", err) + } + if _, _, locErr := mountlocator.Read(home, mountPath); locErr != nil { + t.Fatalf("mount locator must survive a readiness timeout: %v", locErr) + } +} + func TestDrive9MountTimeoutPreservesMountLocator(t *testing.T) { home := t.TempDir() - companion, recordPath := buildFakeDrive9(t) - t.Setenv("TI_FAKE_DRIVE9_RECORD", recordPath) + companion, _ := buildFakeDrive9(t) service := testCompanionService(home, companion) service.MountReadyPollInterval = 5 * time.Millisecond // A file at the mount path never becomes a readable mount root, so the @@ -141,7 +311,6 @@ func TestDrive9MountTimeoutPreservesMountLocator(t *testing.T) { if _, _, locErr := mountlocator.Read(home, mountPath); locErr != nil { t.Fatalf("mount locator must survive a readiness timeout: %v", locErr) } - requireFakeDrive9Call(t, recordPath, "mount") } func TestDrive9VaultMountTimeoutPreservesMountLocator(t *testing.T) { diff --git a/internal/fs/mountready_testenv.go b/internal/fs/mountready_testenv.go new file mode 100644 index 0000000..505afc3 --- /dev/null +++ b/internal/fs/mountready_testenv.go @@ -0,0 +1,12 @@ +package fs + +import "os" + +// testFakeMountReady allows black-box e2e runs against the fake ti-drive9 +// companion to pass mount evidence checks. The fake companion only creates a +// plain directory instead of a real kernel mount. Like the other TI_TEST_* +// controls it is hidden test configuration, never documented for users, and +// requires the explicit test-endpoint opt-in. +func testFakeMountReady() bool { + return os.Getenv("TI_ALLOW_TEST_ENDPOINTS") == "1" && os.Getenv("TI_TEST_FAKE_MOUNT_READY") == "1" +}