Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions pkg/dind/bindstage.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,3 +75,33 @@ func SweepStagedBinds(dataDir string, log *slog.Logger) {
}
sweepStagedBinds(stagingRootDir(dataDir), log)
}

// SweepStagedBindsForJob removes the bind staging mounts and directory of ONE
// job. Safe to call while other jobs are running, which is what separates it
// from SweepStagedBinds.
//
// This exists because #187's periodic dead-container reaper called the
// whole-tree SweepStagedBinds on its sweeper tick. That function is documented
// STARTUP ONLY for a precise reason, and the reaper violated it: sweeping the
// root unmounts and deletes the staging directory of every LIVE job too. The
// running job's already-started containers keep working (their mounts are
// already in their namespaces), so nothing fails loudly — but the job's NEXT
// `docker run -v` cannot create its mountpoint, because ensureDirLocked has
// already latched m.ready and will not recreate the parent:
//
// docker: Error response from daemon: bind mount ... rejected:
// creating bind staging mountpoint <data>/dind-binds/<job>/2:
// mkdir ...: no such file or directory
//
// Observed on the fleet's Linux amd64 nodes against ephpm's release workflow,
// which runs several `docker run -v "$PWD":/w` steps per job — binds 0 and 1
// stage fine, a reaper tick lands, and bind 2 fails. Scoping the sweep to the
// container actually being reaped keeps the benefit (releasing the mounts that
// pin that container's rootfs snapshot, which is why the reaper swept at all)
// without touching anyone else's.
func SweepStagedBindsForJob(dataDir, jobID string, log *slog.Logger) {
if dataDir == "" || jobID == "" {
return
}
sweepStagedBindsForJob(stagingRootDir(dataDir), jobID, log)
}
19 changes: 19 additions & 0 deletions pkg/dind/bindstage_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,25 @@ func stagingRootParent(jobDir string) string { return filepath.Dir(jobDir) }
// sweepStagedBinds is the startup half of the lifecycle: it unmounts and
// removes staging directories left by a previous ephemerd process. See
// SweepStagedBinds for why leaked staging mounts are worse than untidy.
// sweepStagedBindsForJob unmounts and removes exactly one job's staging
// directory. Unlike sweepStagedBinds it never reads the root, so a live job's
// directory sitting next to this one is untouched.
func sweepStagedBindsForJob(root, jobID string, log *slog.Logger) {
dir := filepath.Join(root, jobID)
if _, err := os.Lstat(dir); err != nil {
return // this job never staged a bind, or it is already gone
}
if err := unmountTreeAndRemove(dir); err != nil {
if log != nil {
log.Warn("failed to sweep dind bind staging dir for reaped job", "path", dir, "error", err)
}
return
}
if log != nil {
log.Info("swept dind bind staging dir for reaped job", "path", dir)
}
}

func sweepStagedBinds(root string, log *slog.Logger) {
entries, err := os.ReadDir(root)
if err != nil {
Expand Down
47 changes: 47 additions & 0 deletions pkg/dind/bindstage_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -726,3 +726,50 @@ func TestBindStaging_MountpointTypeMatchesSource(t *testing.T) {
t.Errorf("staged file reads %q (err %v), want %q", body, err, "body")
}
}

// TestSweepStagedBindsForJob_SparesOtherJobs pins the fix for the regression
// #187 introduced: its periodic reaper called the whole-tree
// SweepStagedBinds, which is documented STARTUP ONLY because it cannot tell a
// live job's staging dir from a dead one. On the fleet that deleted running
// jobs' directories, and their next `docker run -v` failed with
// "creating bind staging mountpoint <data>/dind-binds/<job>/2:
//
// mkdir ...: no such file or directory"
//
// because ensureDirLocked had already latched m.ready and would not recreate
// the parent. No mounts are needed to prove the scoping: plain directories
// exercise the same path selection.
func TestSweepStagedBindsForJob_SparesOtherJobs(t *testing.T) {
data := t.TempDir()
root := stagingRootDir(data)

dead := filepath.Join(root, "job-dead")
live := filepath.Join(root, "job-live")
for _, d := range []string{filepath.Join(dead, "0"), filepath.Join(live, "0")} {
if err := os.MkdirAll(d, 0o700); err != nil {
t.Fatal(err)
}
}

SweepStagedBindsForJob(data, "job-dead", discardLog())

if _, err := os.Lstat(dead); !os.IsNotExist(err) {
t.Errorf("reaped job's staging dir still present (err=%v), want removed", err)
}
// The whole point: a live job sitting next to the reaped one must survive,
// mountpoint subdirectory and all, or its next docker run cannot stage.
if _, err := os.Lstat(filepath.Join(live, "0")); err != nil {
t.Errorf("LIVE job's staging dir was destroyed by a scoped sweep: %v — "+
"this is the #187 regression: the next `docker run -v` in that job "+
"fails with mkdir ENOENT", err)
}
}

// TestSweepStagedBindsForJob_ToleratesMissing: a job that never staged a bind
// (no -v in the whole job) must not make the reaper log or fail.
func TestSweepStagedBindsForJob_ToleratesMissing(t *testing.T) {
data := t.TempDir()
SweepStagedBindsForJob(data, "never-staged", discardLog())
SweepStagedBindsForJob("", "job", discardLog())
SweepStagedBindsForJob(data, "", discardLog())
}
7 changes: 7 additions & 0 deletions pkg/dind/bindstage_other.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package dind
import (
"log/slog"
"os"
"path/filepath"
)

// newBindStager returns the off-Linux stager. There is no mount(2) here and,
Expand Down Expand Up @@ -35,3 +36,9 @@ func (passthroughStager) teardown() {}
func sweepStagedBinds(root string, log *slog.Logger) {
_ = os.RemoveAll(root)
}

// sweepStagedBindsForJob is the one-job form; same "nothing to unmount"
// reasoning as above.
func sweepStagedBindsForJob(root, jobID string, log *slog.Logger) {
_ = os.RemoveAll(filepath.Join(root, jobID))
}
26 changes: 17 additions & 9 deletions pkg/runtime/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -563,12 +563,11 @@ func (r *Runtime) ReapDeadContainers(ctx context.Context) error {
}

now := time.Now()
// Release leaked dind bind-staging mounts lazily — only once, and only
// if we actually find something to reap. Each leaked staging mount pins
// the runner rootfs it was bound from, which makes WithSnapshotCleanup
// fail with "device or resource busy"; CleanOrphans sweeps them for the
// same reason. Skipped entirely on an idle node with nothing to reap.
stagedSwept := false
// Each leaked staging mount pins the runner rootfs it was bound from,
// which makes WithSnapshotCleanup fail with "device or resource busy",
// so a container being reaped needs its staging dir released first. That
// happens per-container below, NOT once per pass over the whole tree —
// see the comment at the call site.
for _, c := range containers {
id := c.ID()
log := r.cfg.Log.With("id", id)
Expand All @@ -592,9 +591,18 @@ func (r *Runtime) ReapDeadContainers(ctx context.Context) error {
continue
}

if !stagedSwept {
dind.SweepStagedBinds(r.cfg.DataDir, r.cfg.Log)
stagedSwept = true
// Sweep ONLY this container's staging dir. The whole-tree
// SweepStagedBinds used to run here once per pass, which broke live
// jobs: it is documented STARTUP ONLY precisely because it cannot
// tell a live job's staging dir from a dead one, and deleting a
// running job's dir makes its NEXT `docker run -v` fail with
// "creating bind staging mountpoint ...: no such file or directory"
// (ensureDirLocked has latched m.ready and will not recreate the
// parent). The reaping motive is unchanged — a leaked staged mount
// pins this container's rootfs so its snapshot cannot be deleted —
// and scoping to the container being reaped keeps that.
{
dind.SweepStagedBindsForJob(r.cfg.DataDir, id, r.cfg.Log)
}

log.Info("reaping dead job container", "reason", "task dead past grace, snapshot pinned", "grace", grace)
Expand Down