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

feat: support docker compose v2 plugin version #228

Merged
merged 23 commits into from
Apr 28, 2023

Conversation

AlexZeitler
Copy link
Contributor

@AlexZeitler AlexZeitler commented Feb 4, 2023

Solves #177

@AlexZeitler
Copy link
Contributor Author

AlexZeitler commented Feb 8, 2023

Currently I'm facing some issues with docker compose version v2.15.1 with the stop command:

The command is executed without errors but it seems it didn't stop the container when the command exits because a check in our test fails.

  await compose.stop({ cwd: path.join(__dirname), log: logOutput })
  expect(await isContainerRunning('/compose_test_web')).toBeFalsy() // fails
  expect(await isContainerRunning('/compose_test_proxy')).toBeFalsy()

Putting jest.setTimeout(1000) after the compose.stop call fixes the test but that's a bad solution.

@StefanScherer @Steveb-p can I pick your brains here?

@StefanScherer
Copy link

@AlexZeitler Interesting. Is there a simple repro case? I haven't looked into the code, but your failing test would be basically a docker compose up -d with two services, then later the docker compose stop and imediatelly a docker compose ps? And it worked with versions before 2.15.1, right?

@Steveb-p
Copy link
Contributor

Steveb-p commented Feb 9, 2023

I would assume that docker-compose may report a container as stopping, while in reality it is still running. Don't see a reason otherwise. We are waiting for docker-compose process to complete, and adding a timeout fixing it seems to indicate that there is a slight delay between command being run and container being actually stopped.

@AlexZeitler
Copy link
Contributor Author

AlexZeitler commented Feb 9, 2023

@AlexZeitler Interesting. Is there a simple repro case? I haven't looked into the code, but your failing test would be basically a docker compose up -d with two services, then later the docker compose stop and imediatelly a docker compose ps? And it worked with versions before 2.15.1, right?

@StefanScherer thanks for looping in.

That's the failing test which does what you suggested:

https://github.com/PDMLab/docker-compose/pull/228/files#diff-93d57c9e9cda34d9f45e5d57beddbf6436f7338ad591e47b7daa446d911f1201R204

I need to test prior versions of docker compose v2.

@AlexZeitler
Copy link
Contributor Author

I would assume that docker-compose may report a container as stopping, while in reality it is still running. Don't see a reason otherwise. We are waiting for docker-compose process to complete, and adding a timeout fixing it seems to indicate that there is a slight delay between command being run and container being actually stopped.

Yes, that's my assumption as well.

A workaround could be to find the services started by the compose file, add dockerode as dependency as we do in the test and retry the check until the services are really stopped and only then return from our function.

@cristianrgreco
Copy link

A workaround could be to find the services started by the compose file, add dockerode as dependency as we do in the test and retry the check until the services are really stopped and only then return from our function.

Would that involve moving dockerode from a test dependency to a dependency? Without pushing the instantiation of dockerode to the user it may be complex to add it, for example if the user is running a complex docker setup (dind, etc)

@AlexZeitler
Copy link
Contributor Author

AlexZeitler commented Feb 18, 2023

@cristianrgreco Yes, that's true. We should find a better solution.

@AlexZeitler AlexZeitler merged commit 95250be into master Apr 28, 2023
@AlexZeitler AlexZeitler deleted the compose-v2-plugin-support branch April 28, 2023 10:53
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.

4 participants