Unique container names are bad

[arnuschky]

I know I can override container names, but I think the default implementation is bad.

Currently containers are named "pytest{}".format(os.getpid()). This leads to the following problems:

• If test runs fail or are interrupted (ctrl-c), containers may stay around. Due to the unique names they start to accumulate. Same for images.
• Docker-compose doesn't recognize that things belong to each other, and has conflicts. For example, if you use a static network subnet, then pytest-docker fails as different runs try to use the same network, but it's named separately.

This cost me days to debug (no docker wiz here).

Is there a specific reason why to use the pid-based naming? I would follow simply the behavior of docker-compose:

@pytest.fixture(scope='session')
def docker_compose_project_name(pytestconfig):
    """ Generate a project name using the projects root directory.

    Override this fixture in your tests if you need a particular project name.
    """
    return "{}pytest".format(os.path.basename(str(pytestconfig.rootdir)))

[AndreLouisCaron]

Thanks for reporting this and sorry it caused you some pain!

Since you have a workaround, I'd like to take the time to fully understand the issues you're facing so we can work out the right solution :-)

Is there a specific reason why to use the pid-based naming?

The rationale behind unique container names is that we needed to support parallel builds on the same machine. I was under the impression that this was a safer default than having parallel builds interfere with each other (this has also caused me lost days to debug in other circumstances).

Note: there is no reason to use the PID specifically, just some value that is locally unique for the duration of the test run.

I would follow simply the behavior of docker-compose:

I agree that we should try to honour docker-compose's behaviour to minimise surprises.

At the very least, I think we should honour $COMPOSE_PROJECT_NAME as an override. I guess that if we had our build machines inject a unique value in this environment variable, we'd get our parallel builds without forcing unique container names on everyone.

Would that work for you?

If test runs fail or are interrupted (ctrl-c), containers may stay around.

Using fixed container names prevents build-up, which provides a partial solution to your problem, but I'm not sure we can solve the CTRL-C issue within a pytest plug-in.

I get proper cleanup when tests fail. What do you mean by "if test runs fail"?

Same for images

Do you also see build-up when using fixed container names?

I automate daily cleanup of dangling images on all my machines that should run unsupervised for long periods of time (see this gist for several recipes). I wish the Docker daemon did this automatically because it really helps all projects, not just tests run using pytest-docker.

[arnuschky]

The rationale behind unique container names is that we needed to support parallel builds on the same machine. I was under the impression that this was a safer default than having parallel builds interfere with each other (this has also caused me lost days to debug in other circumstances).

Ah ok. Admittedly, I haven't tested parallel builds on my dev machine, even though I need that for our CI server.

At the very least, I think we should honour $COMPOSE_PROJECT_NAME as an override. I guess that if we had our build machines inject a unique value in this environment variable, we'd get our parallel builds without forcing unique container names on everyone.
Would that work for you?

I'll read up / test and get back to you. I am not sure what it does tbh.

Using fixed container names prevents build-up, which provides a partial solution to your problem, but I'm not sure we can solve the CTRL-C issue within a pytest plug-in.

True, but having unique names limits the problem to one set.

I get proper cleanup when tests fail. What do you mean by "if test runs fail"?

Sorry, this was unspecific. During getting started with docker and pytest, we had many tests failing during setup rather than test run. For example, problems in the docker-compose file etc. Those aren't cleaned up properly afaik. However, I would need to re-test for details, after days of trial and error everything become a bit hazy ;)

Re images: Admittedly this is the smallest problem and can be easily solved with a script like you mention.

The main issue for us was using a static network. I've chosen to put our docker networks on a rarely used network as I constantly had problems with overlaps of local networks, vpns, wifis etc. I don't know how other people deal with this (I am rather new to docker), but I configured this as follows:

networks:
  default:
    ipam:
      config:
        - subnet: 172.31.219.1/24

This configuration supports only a single instance, and thus fails with multiple instances with unique names due to a subnet overlap error.

[danodonovan]

The rationale behind unique container names is that we needed to support parallel builds on the same machine

Just thinking out loud - would this be better handled by having unique tags and repeatable image names?

I don't believe docker-compose can tag the images it builds, but it would be possible with a separate call to docker

$ docker tag <image> <tag>

Of course this would require a new DockerExecutor like DockerComposeExecutor etc. If I can get it working I'll submit a PR for discussion.

[AndreLouisCaron]

@danodonovan I'm not sure I understand how your proposal works. Can you tell me more?

[danodonovan]

Sorry for delay. The idea would be to have a static image name (ie the project name sans pid) and then have unique tags for different builds (to cover the support for parallel builds).

1. docker-compose build builds the docker image(s) with a static image name
2. docker tag tags the built image(s) with a unique id(s) (ie not latest)
3. docker-compose up runs the containers with these tags

But, having investigated further, docker-compose explicitly doesn't support tagging of images docker/compose#213 (comment) - So I don't think docker-compose could be made to run a specific <image>:<tag> if the tag wasn't specified in the compose file.

TL;DR I don't think this will work with docker-compose.

[mikemol]

Wouldn't it make more sense to use labels to group containers, and have a garbage collection step identifying running containers associated with test runs where the test process has terminated?

[danodonovan]

@mikemol ... and you would identify the test runs where the test process has terminated from the os.getpid() attached to the container name?

Who would execute the garbage collection step? If it's the test process then it's containers are still valid (as the PID is still valid) - and so wouldn't get cleared.