Skip to content

test:add ci for cleanup fifos #4443

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
merged 1 commit into from
Sep 4, 2025
+42 −19
Merged

test:add ci for cleanup fifos #4443

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
20 changes: 20 additions & 0 deletions cmd/nerdctl/container/container_stop_linux_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,3 +199,23 @@ func TestStopWithTimeout(t *testing.T) {
// The container should get the SIGKILL before the 10s default timeout
assert.Assert(t, elapsed < 10*time.Second, "Container did not respect --timeout flag")
}
func TestStopCleanupFIFOs(t *testing.T) {
if rootlessutil.IsRootless() {
t.Skip("/run/containerd/fifo/ doesn't exist on rootless")
}
testutil.DockerIncompatible(t)
base := testutil.NewBase(t)
testContainerName := testutil.Identifier(t)
oldNumFifos, err := countFIFOFiles("/run/containerd/fifo/")
assert.NilError(t, err)
// Stop the container after 2 seconds
go func() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it possible to avoid goroutine and make sequential check to avoid synchronisation issues and then flaky tests ?

time.Sleep(2 * time.Second)
base.Cmd("stop", testContainerName).AssertOK()
newNumFifos, err := countFIFOFiles("/run/containerd/fifo/")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test compares global counts in /run/containerd/fifo/. If other tests run in parallel and create/cleanup tasks, the global count may fluctuate, making the test flaky:

option: Prefer filtering by container/task ID in the FIFO names, if possible. Otherwise, add a short stabilization poll or mark the test as non-parallel and document the global dependency.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if don't use t.Parallel() the test cases will execute serially. if this testcase is running other test case with t.Parallel() will not run.

assert.NilError(t, err)
assert.Equal(t, oldNumFifos, newNumFifos)
}()
// Start a container that is automatically removed after it exits
base.Cmd("run", "--rm", "--name", testContainerName, testutil.NginxAlpineImage).AssertOK()
}
41 changes: 22 additions & 19 deletions pkg/containerutil/containerutil.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,12 @@ func Stop(ctx context.Context, container containerd.Container, timeout *time.Dur

switch status.Status {
case containerd.Created, containerd.Stopped:
// Cleanup the IO after a successful Stop
if io := task.IO(); io != nil {
if cerr := io.Close(); cerr != nil {
log.G(ctx).Warnf("failed to close IO for container %s: %v", container.ID(), cerr)
}
}
return nil
case containerd.Paused, containerd.Pausing:
paused = true
Expand All @@ -397,6 +403,13 @@ func Stop(ctx context.Context, container containerd.Container, timeout *time.Dur
return err
}

// signal will be sent once resume is finished
if paused {
if err := task.Resume(ctx); err != nil {
log.G(ctx).Errorf("cannot unpause container %s: %s", container.ID(), err)
return err
}
}
if *timeout > 0 {
sig, err := getSignal(signalValue, l)
if err != nil {
Expand All @@ -407,20 +420,10 @@ func Stop(ctx context.Context, container containerd.Container, timeout *time.Dur
return err
}

// signal will be sent once resume is finished
if paused {
if err := task.Resume(ctx); err != nil {
log.G(ctx).Warnf("Cannot unpause container %s: %s", container.ID(), err)
} else {
// no need to do it again when send sigkill signal
paused = false
}
}

sigtermCtx, sigtermCtxCancel := context.WithTimeout(ctx, *timeout)
defer sigtermCtxCancel()

err = waitContainerStop(sigtermCtx, exitCh, container.ID())
err = waitContainerStop(sigtermCtx, task, exitCh, container.ID())
if err == nil {
return nil
}
Expand All @@ -439,13 +442,7 @@ func Stop(ctx context.Context, container containerd.Container, timeout *time.Dur
return err
}

// signal will be sent once resume is finished
if paused {
if err := task.Resume(ctx); err != nil {
log.G(ctx).Warnf("Cannot unpause container %s: %s", container.ID(), err)
}
}
return waitContainerStop(ctx, exitCh, container.ID())
return waitContainerStop(ctx, task, exitCh, container.ID())
}

func getSignal(signalValue string, containerLabels map[string]string) (syscall.Signal, error) {
Expand All @@ -460,14 +457,20 @@ func getSignal(signalValue string, containerLabels map[string]string) (syscall.S
return signal.ParseSignal("SIGTERM")
}

func waitContainerStop(ctx context.Context, exitCh <-chan containerd.ExitStatus, id string) error {
func waitContainerStop(ctx context.Context, task containerd.Task, exitCh <-chan containerd.ExitStatus, id string) error {
select {
case <-ctx.Done():
if err := ctx.Err(); err != nil {
return fmt.Errorf("wait container %v: %w", id, err)
}
return nil
case status := <-exitCh:
// Cleanup the IO after a successful Stop
if io := task.IO(); io != nil {
if cerr := io.Close(); cerr != nil {
log.G(ctx).Warnf("failed to close IO for container %s: %v", id, cerr)
}
}
return status.Error()
}
}
Expand Down
Loading