Skip to content

Conversation

ningmingxiao
Copy link
Contributor

@ningmingxiao ningmingxiao commented Sep 3, 2025

I find containerd will not close pipes until call delete it will cause some ci failed.

func (p *Init) delete(ctx context.Context) error {
	waitTimeout(ctx, &p.wg, 2*time.Second)
	err := p.runtime.Delete(ctx, p.id, nil)
	// ignore errors if a runtime has already deleted the process
	// but we still hold metadata and pipes
	//
	// this is common during a checkpoint, runc will delete the container state
	// after a checkpoint and the container will no longer exist within runc
	if err != nil {
		if strings.Contains(err.Error(), "does not exist") {
			err = nil
		} else {
			err = p.runtimeError(err, "failed to delete task")
		}
	}
	if p.io != nil {
		for _, c := range p.closers {
			c.Close()
		}
		p.io.Close()
	}
	if err2 := mount.UnmountRecursive(p.Rootfs, 0); err2 != nil {
		log.G(ctx).WithError(err2).Warn("failed to cleanup rootfs mount")
		if err == nil {
			err = fmt.Errorf("failed rootfs umount: %w", err2)
		}
	}
	return err
}

failed ci

func TestLogsFollowNoExtraneousLineFeed(t *testing.T) {
	testCase := nerdtest.Setup()
	// This test verifies that `nerdctl logs -f` does not add extraneous line feeds
	testCase.Require = require.Not(require.Windows)

	testCase.Setup = func(data test.Data, helpers test.Helpers) {
		// Create a container that outputs a message without a trailing newline
		helpers.Ensure("run", "--name", data.Identifier(), testutil.CommonImage,
			"sh", "-c", "printf 'Hello without newline'")
	}

@ningmingxiao ningmingxiao marked this pull request as draft September 3, 2025 10:09
@ningmingxiao ningmingxiao force-pushed the live_restore2 branch 7 times, most recently from 9dd72da to 80f0722 Compare September 4, 2025 02:17
Signed-off-by: ningmingxiao <[email protected]>
@ningmingxiao
Copy link
Contributor Author

ningmingxiao commented Sep 25, 2025

can you review this pr containerd/containerd#12281 ? @AkihiroSuda
If this pr merged nerdctl logdriver will no longer need try to connect containerd to get container status. Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant