Skip to content
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.

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

Add nerdctl compose stop command #1494

Merged
merged 1 commit into from
Nov 10, 2022
Merged

Conversation

djdongjin
Copy link
Member

@djdongjin djdongjin commented Nov 10, 2022

Fix #666

This implementation uses nerdctl stop [flags] container to stop all containers in given services, similar to how compose down works link.

I didn't use a WaitGroup to parallelize the nerdctl stop calls, as in here, since I'm not sure what forcibly/gracefully means. Thanks for any explanation and I can optimize (seems some refactor/dedup can be done here as well)

Also in the same place, it seems like a bug. This line should be outside of for loop right?

I'll also try to add test as well.

$ cat full.yaml
version: '3.1'

services:
  service1:
    image: alpine:3.13
    command: "sleep infinity"
  service2:
    image: alpine:3.13
    command: "sleep infinity"
$ sudoe nerdctl compose -f full.yaml up -d
INFO[0000] Ensuring image alpine:3.13
...
$ sudoe nerdctl ps
CONTAINER ID    IMAGE                            COMMAND             CREATED           STATUS    PORTS    NAMES
6ab9f4de1c07    docker.io/library/alpine:3.13    "sleep infinity"    17 seconds ago    Up                 orphan_service2_1
e046dbf0bdf3    docker.io/library/alpine:3.13    "sleep infinity"    17 seconds ago    Up                 orphan_service1_1
$ sudoe nerdctl compose -f full.yaml stop service1
INFO[0000] Stopping container orphan_service1_1
$ sudoe nerdctl ps -a
CONTAINER ID    IMAGE                              COMMAND             CREATED               STATUS                         PORTS    NAMES
...
6ab9f4de1c07    docker.io/library/alpine:3.13      "sleep infinity"    About a minute ago    Up                                      orphan_service2_1
e046dbf0bdf3    docker.io/library/alpine:3.13      "sleep infinity"    About a minute ago    Exited (137) 13 seconds ago             orphan_service1_1
...

Signed-off-by: Jin Dong jindon@amazon.com

@djdongjin
Copy link
Member Author

Seems the CI failed due to a flaky test (#767). The issue is due to conflict container names.

runContainer()
base.EnsureContainerStarted(containerName)
base.Cmd("exec", containerName, "cat", "/mnt/file").AssertOutExactly("rev0")
base.Cmd("exec", containerName, "sh", "-euc", "echo -n \"rev1\" >/mnt/file").AssertOK()
base.Cmd("rm", "-f", containerName).AssertOK()
runContainer()

# log
=== CONT  TestRunCopyingUpInitialContentsShouldNotResetTheCopiedContents
    testutil.go:458: buildkitHost="unix:///run/user/1000/buildkit-nerdctl-test/buildkitd.sock"
    run_mount_linux_test.go:233: container nerdctl-testruncopyingupinitialcontentsshouldnotresetthecopiedcontents is now running
    run_mount_linux_test.go:230: assertion failed: res.ExitCode is not exitCode: time="2022-11-10T00:45:37Z" level=fatal msg="name \"nerdctl-testruncopyingupinitialcontentsshouldnotresetthecopiedcontents\" is already used by ID \"7ce7a2031325302a265d2ed8836b5179488fd6835d6938325b52ec18e7e5e7d9\""
        
--- FAIL: TestRunCopyingUpInitialContentsShouldNotResetTheCopiedContents (2.24s)

Within the same file, another test case seems avoid the issue by not specifying containerName in the 2nd run:

"--name", containerName,

v.s.

@djdongjin djdongjin force-pushed the compose-stop branch 2 times, most recently from 46f1900 to 07dfe22 Compare November 10, 2022 04:04
README.md Show resolved Hide resolved
Copy link
Member

@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

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

Thanks

Signed-off-by: Jin Dong <jindon@amazon.com>
Copy link
Member

@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

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

thanks

@AkihiroSuda AkihiroSuda merged commit c0bf117 into containerd:main Nov 10, 2022
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.

nerdctl compose removes containers on termination (stop vs down)
2 participants