Skip to content

fix(dockerutil): drop AutoRemove on one-off helper containers #2

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
whalelephant merged 1 commit into from
Jul 31, 2026
Merged

fix(dockerutil): drop AutoRemove on one-off helper containers #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 6 additions & 2 deletions dockerutil/fileretriever.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,12 @@ func (r *FileRetriever) SingleFileContent(ctx context.Context, volumeName, relPa
Labels: map[string]string{CleanupLabel: r.testName},
},
&container.HostConfig{
Binds: []string{volumeName + ":" + mountPath},
AutoRemove: true,
// No AutoRemove: this container is never started (we only
// CopyFromContainer its filesystem), and the deferred
// ContainerRemove(Force) below cleans it up. Keeping the one-off
// helpers uniformly AutoRemove-free avoids the ContainerWait race
// that AutoRemove causes on newer Docker Engines (see volumeowner.go).
Binds: []string{volumeName + ":" + mountPath},
},
nil, // No networking necessary.
nil,
Expand Down
16 changes: 6 additions & 10 deletions dockerutil/filewriter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,7 @@ func (w *FileWriter) WriteFile(ctx context.Context, volumeName, relPath string,
Labels: map[string]string{CleanupLabel: w.testName},
},
&container.HostConfig{
Binds: []string{volumeName + ":" + mountPath},
AutoRemove: true,
Binds: []string{volumeName + ":" + mountPath},
},
nil, // No networking necessary.
nil,
Expand All @@ -70,13 +69,12 @@ func (w *FileWriter) WriteFile(ctx context.Context, volumeName, relPath string,
return fmt.Errorf("creating container: %w", err)
}

autoRemoved := false
// We intentionally do NOT set AutoRemove on the container. With AutoRemove
// the daemon reaps the container the instant its (near-instant) chown exits,
// which on newer Docker Engines (25+) races the ContainerWait below and
// surfaces as "No such container". Instead we let the container persist and
// always remove it ourselves in this defer.
defer func() {
if autoRemoved {
// No need to attempt removing the container if we successfully started and waited for it to complete.
return
}

if err := w.cli.ContainerRemove(ctx, cc.ID, types.ContainerRemoveOptions{
Force: true,
}); err != nil {
Expand Down Expand Up @@ -127,8 +125,6 @@ func (w *FileWriter) WriteFile(ctx context.Context, volumeName, relPath string,
case err := <-errCh:
return err
case res := <-waitCh:
autoRemoved = true

if res.Error != nil {
return fmt.Errorf("waiting for write-file container: %s", res.Error.Message)
}
Expand Down
16 changes: 6 additions & 10 deletions dockerutil/volumeowner.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,7 @@ func SetVolumeOwner(ctx context.Context, opts VolumeOwnerOptions) error {
Labels: map[string]string{CleanupLabel: opts.TestName},
},
&container.HostConfig{
Binds: []string{opts.VolumeName + ":" + mountPath},
AutoRemove: true,
Binds: []string{opts.VolumeName + ":" + mountPath},
},
nil, // No networking necessary.
nil,
Expand All @@ -69,13 +68,12 @@ func SetVolumeOwner(ctx context.Context, opts VolumeOwnerOptions) error {
return fmt.Errorf("creating container: %w", err)
}

autoRemoved := false
// We intentionally do NOT set AutoRemove on the container. With AutoRemove
// the daemon reaps the container the instant its (near-instant) chown exits,
// which on newer Docker Engines (25+) races the ContainerWait below and
// surfaces as "No such container". Instead we let the container persist and
// always remove it ourselves in this defer.
defer func() {
if autoRemoved {
// No need to attempt removing the container if we successfully started and waited for it to complete.
return
}

if err := opts.Client.ContainerRemove(ctx, cc.ID, types.ContainerRemoveOptions{
Force: true,
}); err != nil {
Expand All @@ -94,8 +92,6 @@ func SetVolumeOwner(ctx context.Context, opts VolumeOwnerOptions) error {
case err := <-errCh:
return err
case res := <-waitCh:
autoRemoved = true

if res.Error != nil {
return fmt.Errorf("waiting for volume-owner container: %s", res.Error.Message)
}
Expand Down