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

fix: prefer docker-compose-v1 to work around race condition changes in Compose V2 RC3 #304

Merged
merged 3 commits into from
Sep 17, 2021

Conversation

jbinto
Copy link
Contributor

@jbinto jbinto commented Sep 16, 2021

Slack context: https://touchbistro.slack.com/archives/CU7JQ7ZPS/p1631815389225500

After upgrading to Docker for Mac 4.0.1, it appears something changed in Docker Compose V2 RC3 that causes some of our preRun scripts to fail with the following:

DEBU Error response from daemon: You cannot remove a running container 96107f4682cc81f739d74a696b4a1cee4ddce7663f468d94658c5224be8c8725. Stop the container before attempting removal or force remove  id=docker-compose-run

We have some fairly complex interdependencies with preRun scripts and we may have been depending on racy behaviour that was fixed.

This PR changes tb to prefer docker-compose-v1 which is still distributed as a binary with Docker for Mac (but for how long, who can tell).

@@ -15,14 +16,25 @@ const (
stopTimeoutSecs = 2
)

func composeCommand() string {
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like we should cache the result vs looking it up every time so a single run is consistent (and doesn't produce these logs multiple times)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in a190a16

@jbinto jbinto merged commit 9fa1de5 into master Sep 17, 2021
@jbinto jbinto deleted the put_out_docker_compose_v2_rc3_fire branch September 17, 2021 16:07
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.

2 participants