diff --git a/dockerutil/fileretriever.go b/dockerutil/fileretriever.go index 46f8729a1..35d486b10 100644 --- a/dockerutil/fileretriever.go +++ b/dockerutil/fileretriever.go @@ -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, diff --git a/dockerutil/filewriter.go b/dockerutil/filewriter.go index 3db357825..8cfbbfd74 100644 --- a/dockerutil/filewriter.go +++ b/dockerutil/filewriter.go @@ -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, @@ -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 { @@ -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) } diff --git a/dockerutil/volumeowner.go b/dockerutil/volumeowner.go index 8432ab2a6..f3fd12f38 100644 --- a/dockerutil/volumeowner.go +++ b/dockerutil/volumeowner.go @@ -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, @@ -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 { @@ -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) }