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

Use docker compose v2 by default in test-integration-docker-compatibility #1644

Merged
merged 1 commit into from
Jun 8, 2023

Conversation

djdongjin
Copy link
Member

@djdongjin djdongjin commented Dec 12, 2022

Fix #1577

Signed-off-by: Jin Dong djdongjin95@gmail.com

@djdongjin djdongjin force-pushed the test-compose-v2 branch 3 times, most recently from a1c514e to 2088215 Compare December 13, 2022 17:13
@AkihiroSuda
Copy link
Member

@djdongjin Do you still plan to work on this?

@djdongjin djdongjin force-pushed the test-compose-v2 branch 16 times, most recently from 2783754 to a451630 Compare June 3, 2023 01:23
@djdongjin djdongjin marked this pull request as ready for review June 3, 2023 01:31
@djdongjin djdongjin changed the title [WIP] Use docker compose v2 by default in test-integration-docker-compatibility Use docker compose v2 by default in test-integration-docker-compatibility Jun 3, 2023
@djdongjin
Copy link
Member Author

@djdongjin Do you still plan to work on this?

@AkihiroSuda the PR is ready for review, thanks

@AkihiroSuda AkihiroSuda added this to the v1.4.1 (tentative) milestone Jun 7, 2023
@djdongjin djdongjin force-pushed the test-compose-v2 branch 2 times, most recently from 2f528f4 to f3c923e Compare June 7, 2023 19:23
Signed-off-by: Jin Dong <djdongjin95@gmail.com>

Resolve comments

Signed-off-by: Jin Dong <djdongjin95@gmail.com>
@@ -51,6 +51,7 @@ services:
t.Logf("projectName=%q", projectName)
defer base.ComposeCmd("-f", comp.YAMLFullPath(), "down", "-v").Run()

defer base.Cmd("rm", "-f", "-v", containerName).Run()
Copy link
Member

@AkihiroSuda AkihiroSuda Jun 8, 2023

Choose a reason for hiding this comment

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

Isn't defer base.ComposeCmd("-f", comp.YAMLFullPath(), "down", "-v").Run() (L52) enough?

Copy link
Member Author

Choose a reason for hiding this comment

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

It didn't work in docker (compose), e.g., containers from docker compose run (i.e., not from docker compose up) won't be covered by docker compose down:

$ cat docker-compose.yaml
version: '3.1'

services:
  svc0:
    image: nginx
  svc1:
    image: rabbitmq

# docker compose down cannot down container from docker compose run
$ docker compose run -d svc0
[+] Running 1/0
 ✔ Network tmp_default  Created                                                                                                                                                                                                                                                                                                       0.0s
103f148596f2d629f2b875d83da6ac5ea3682ad85b8c61d58e73ddc7931f3b11
$ docker compose down -v
[+] Running 1/0
 ✘ Network tmp_default  Error                                                                                                                                                                                                                                                                                                         0.0s
failed to remove network tmp_default: Error response from daemon: error while removing network: network tmp_default id 3795360bd3e1e9a11def2a76f140597f328009fb101bd7d67490bdeebb8b54f9 has active endpoints
$ docker ps
CONTAINER ID   IMAGE     COMMAND                  CREATED          STATUS          PORTS     NAMES
103f148596f2   nginx     "/docker-entrypoint.…"   24 seconds ago   Up 23 seconds   80/tcp    tmp-svc0-run-db8a05f1f3e6

# works fine if from docker compose up
$ docker compose up -d
[+] Running 2/2
 ✔ Container tmp-svc1-1  Started                                                                                                                                                                                                                                                                                                      0.2s
 ✔ Container tmp-svc0-1  Started                                                                                                                                                                                                                                                                                                      0.3s
$ docker compose down -v
[+] Running 3/2
 ✔ Container tmp-svc0-1  Removed                                                                                                                                                                                                                                                                                                      0.1s
 ✔ Container tmp-svc1-1  Removed                                                                                                                                                                                                                                                                                                      6.3s
 ✔ Network tmp_default   Removed

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 3a7909d into containerd:main Jun 8, 2023
@djdongjin djdongjin deleted the test-compose-v2 branch June 8, 2023 01:05
@AkihiroSuda AkihiroSuda mentioned this pull request Jul 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

test-integration-docker-compatibility: switch away from docker-compose (v1) to docker compose (v2)
2 participants