Add cleanup IO process after nerdctl stop

[liuchangyan]

Fix #4315
In the ctr and CRI call paths of containerd, the remaining FIFO files (e.g., /run/containerd/fifo) are primarily cleaned up through the Delete interface of the task. However, in the nerdctl stop path, this interface is not invoked, so an explicit call to io.Close is required to clean them up.

[fahedouch]

can we have a test please ?

[liuchangyan]

can we have a test please ?

Sure

[fahedouch]

Sequentially stopping the container makes things more deterministic. AssertOk here is necessary to ensure proper stop

[liuchangyan]

Sequentially stopping the container makes things more deterministic. AssertOk here is necessary to ensure proper stop

Thanks! Since the container is long-running with --rm, we use a goroutine to trigger stop — otherwise run blocks and we can’t proceed. Already updated the stop call to use AssertOK to ensure it completes properly.

[apostasie]

Hey @liuchangyan

Looks like the new tests are failing?

[ningmingxiao]

I can help to create a ci for this bug.

[fahedouch]

I can help to create a ci for this bug.

@ningmingxiao please go ahead

[ningmingxiao]

I think we should mv io.close in waitContainerStop it will make sure container is really stopped.
I create a pr #4443

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()
	}
}

[fahedouch]

Hi @liuchangyan,

What is the next step for this PR?

[liuchangyan]

Hi @liuchangyan,

What is the next step for this PR?
I've removed my test sample since it looks like this PR4443 already covers that part. I think my original code still has value to keep. Should we merge both PRs together~ WDYT?