Skip to content

Commit

Permalink
fix: not to be deleted a container created with --rm when detatching
Browse files Browse the repository at this point in the history
In the current implementation, detaching from a container started with
`nerdctl run --rm ...` unexpectedly removes it.

The behaviour before this modification is as follows.

```
> nerdctl run --rm -it --detach-keys=ctrl-a,ctrl-b --name test alpine
/ # INFO[0002] read detach keys

> nerdctl ps
CONTAINER ID    IMAGE    COMMAND    CREATED    STATUS    PORTS    NAMES
```

When the same operaton is performed in the Docker CLI, the container will
not be deleted.

This issue is reported in the following:

- containerd#3568

Therefore, this commit resolves this behaviour of a container not being
deleted on detachment.

Note that the behaviour after this modification is as follows.

```
> nerdctl run --rm -it --detach-keys=ctrl-a,ctrl-b --name test alpine
/ # INFO[0010] read detach keys

> nerdctl ps
CONTAINER ID    IMAGE                                        COMMAND      CREATED           STATUS    PORTS    NAMES
46f4c829e5cc    docker.io/library/alpine:latest              "/bin/sh"    15 seconds ago    Up                 test
```

Signed-off-by: Hayato Kiwata <[email protected]>
  • Loading branch information
haytok committed Oct 30, 2024
1 parent 49a19ed commit bc28d41
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 1 deletion.
8 changes: 7 additions & 1 deletion cmd/nerdctl/container/container_run.go
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,8 @@ func processCreateCommandFlagsInRun(cmd *cobra.Command) (types.ContainerCreateOp
// runAction is heavily based on ctr implementation:
// https://github.com/containerd/containerd/blob/v1.4.3/cmd/ctr/commands/run/run.go
func runAction(cmd *cobra.Command, args []string) error {
var isDetatched bool

createOpt, err := processCreateCommandFlagsInRun(cmd)
if err != nil {
return err
Expand Down Expand Up @@ -378,8 +380,11 @@ func runAction(cmd *cobra.Command, args []string) error {
}()

id := c.ID()
if createOpt.Rm && !createOpt.Detach {
if createOpt.Rm {
defer func() {
if isDetatched {
return
}
if err := netManager.CleanupNetworking(ctx, c); err != nil {
log.L.Warnf("failed to clean up container networking: %s", err)
}
Expand Down Expand Up @@ -449,6 +454,7 @@ func runAction(cmd *cobra.Command, args []string) error {
return errors.New("got a nil IO from the task")
}
io.Wait()
isDetatched = true
case status := <-statusC:
if createOpt.Rm {
if _, taskDeleteErr := task.Delete(ctx); taskDeleteErr != nil {
Expand Down
48 changes: 48 additions & 0 deletions cmd/nerdctl/container/container_run_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ import (
"github.com/containerd/nerdctl/v2/pkg/rootlessutil"
"github.com/containerd/nerdctl/v2/pkg/strutil"
"github.com/containerd/nerdctl/v2/pkg/testutil"
"github.com/containerd/nerdctl/v2/pkg/testutil/nerdtest"
"github.com/containerd/nerdctl/v2/pkg/testutil/test"
)

func TestRunCustomRootfs(t *testing.T) {
Expand Down Expand Up @@ -519,3 +521,49 @@ func TestRunWithTtyAndDetached(t *testing.T) {
withTtyContainer := base.InspectContainer(withTtyContainerName)
assert.Equal(base.T, 0, withTtyContainer.State.ExitCode)
}

// TestIssue3568 tests https://github.com/containerd/nerdctl/issues/3568
func TestIssue3568(t *testing.T) {
testCase := nerdtest.Setup()

testCase.SubTests = []*test.Case{
{
Description: "Issue #3568 - Detaching from a container started by using --rm option causes the container to be deleted.",
// When detaching from a container, for a session started with 'docker attach', it prints 'read escape sequence', but for one started with 'docker (run|start)', it prints nothing.
// However, the flag is called '--detach-keys' in all cases, so nerdctl prints 'read detach keys' for all cases, and that's why this test is skipped for Docker.
Require: test.Require(
test.Not(nerdtest.Docker),
),
Command: func(data test.Data, helpers test.Helpers) test.TestableCommand {
cmd := helpers.Command("run", "--rm", "-it", "--detach-keys=ctrl-a,ctrl-b", "--name", data.Identifier(), testutil.CommonImage)
// unbuffer(1) can be installed with `apt-get install expect`.
//
// "-p" is needed because we need unbuffer to read from stdin, and from [1]:
// "Normally, unbuffer does not read from stdin. This simplifies use of unbuffer in some situations.
// To use unbuffer in a pipeline, use the -p flag."
//
// [1] https://linux.die.net/man/1/unbuffer
cmd.WithWrapper("unbuffer", "-p")
cmd.WithStdin(testutil.NewDelayOnceReader(bytes.NewReader([]byte{1, 2}))) // https://www.physics.udel.edu/~watson/scen103/ascii.html
return cmd
},
Cleanup: func(data test.Data, helpers test.Helpers) {
helpers.Anyhow("rm", "-f", data.Identifier())
},
Expected: func(data test.Data, helpers test.Helpers) *test.Expected {
return &test.Expected{
ExitCode: 0,
Errors: []error{},
Output: test.All(
test.Contains("read detach keys"),
func(stdout string, info string, t *testing.T) {
assert.Assert(t, strings.Contains(helpers.Capture("ps"), data.Identifier()))
},
),
}
},
},
}

testCase.Run(t)
}

0 comments on commit bc28d41

Please sign in to comment.