test: support docker-compose v1 & debian bookworm #2584
Conversation
There was a problem hiding this comment.
You are right there is something wrong with the cleanup logic if an exception occurs. I experienced the same thing no teardown happens and it leaves all ghost containers and networks. Especially in the host-network-mode tests like you also have. I had the same issue on KeyboardInterrupt but I had no time to dig into this. So thanks for also pointing this out.
My first investigation suspects is pytest.fail from L328 and L336
I think this exits the DockerComposer ContextManager and no _down teardown is executed.
But instead of doing docker_compose_down if an exception happens I would recommend to fix the ContextManager.
Sorry I cannot test this right now but my recommendations are:
- Change the pytest.fail from
docker_compose_upanddocker_compose_downto alogging.error - Improve the compose function in the ContextManager with a try except block to make sure teardown happens. And also move the self. declarations up so we are sure they are set even if an exception occurs. (else they are None in _down)
def compose(self, docker_compose_files: List[str], project_name: str):
if docker_compose_files == self._docker_compose_files and project_name == self._project_name:
return
self._down()
if docker_compose_files is None or project_name is None:
return
#Move these states to early top, because we are sure it is set. It never reached it on exception.
self._docker_compose_files = docker_compose_files
self._project_name = project_name
try:
docker_compose_up(docker_compose_files, project_name)
self._networks = connect_to_all_networks()
wait_for_nginxproxy_to_be_ready()
time.sleep(3) # give time to containers to be ready
except KeyboardInterrupt:
logging.warning("KeyboardInterrupt detected! Cleaning up...")
self._down() # Always teardown
raise # Re-raise to allow pytest to exit cleanly
except:
logging.error(f"Docker Compose setup failed")
self._down() # Always teardown
pytest.fail("Docker Compose setup failed")- L487
self._docker_compose_file = None>>self._docker_compose_files = None(files) - Should we make sure
self._networks = []is set in_down?
Well, take your time :) As said before my Python skills are limited. I came up with my rudimentary fix, but I won't be able to do this the right way (tm) as easily as you would. |
|
I am working on this, but I have some new unexpected issues coming up. @buchdag You refactored this a couple of months ago. Do you have a smart hint to solve this? I tried setting the |
Debian bookworm (current stable) still has python3-docker 5.0.3. The reverted commit prevented the test-suite to run natively on this distro. The version check could be bumped again when the next Debian stable (trixie) will be released (by fall 2025 hopefully).
This is to prevent the below error with docker-compose v1: ERROR: for nginx-proxy "host" network_mode is incompatible with port_bindings
3aef586 to
ab49d65
Compare
|
@SchoNie I had issues with the way things where setup in test_fallback too, I almost refactored it to avoid using a self defined @pini-gh could we focus this PR on fixing the test suite on Debian / with Docker Compose v1 ? I think the error handling issue is a separate problem and should be handled in a separate PR, allowing this one to be merged. |
ab49d65 to
c1f1c85
Compare
Fix #2582 #2583 .