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

Pass env to ComposeDelegate in DockerComposeContainer#stop #8493

Merged
merged 4 commits into from
Mar 29, 2024

Conversation

mmusenbr
Copy link
Contributor

This fixes issues on stopping when ENV-Variables are used inside the compose file. Issue was introduced in 7112db5.

Fixes #8492

This fixes issues on stopping when ENV-Variables are used inside the compose file.
Issue was introduced in 7112db5.

Fixes testcontainers#8492

Signed-off-by: Michael Musenbrock <[email protected]>
@mmusenbr mmusenbr requested a review from a team as a code owner March 27, 2024 21:36
@eddumelendez
Copy link
Member

Thanks for your contribution, @mmusenbr! I wonder why this test has not failed. I see during the refactor this was omitted but would be nice to have a test to make sure this is not happening again.

This is the same change as in DockerComposeContainer, even though
no error is reported here, because COMPOSE_PROJECT_NAME seems sufficient

Signed-off-by: Michael Musenbrock <[email protected]>
@mmusenbr
Copy link
Contributor Author

Hi @eddumelendez , I need to answer why ComposePassthroughTest is passing with two aspects:

  • Running via ComposeContainer does not seem to need all the details set, as is uses the information from the COMPOSE_PROJECT_NAME

For example having the following yaml:

services:
  redis:
    image: redis
    ports:
      - ${REDIS_PORT}

And then running:
COMPOSE_PROJECT_NAME=randprojname REDIS_PORT=8272 docker compose -f compose.yml up
and then:
COMPOSE_PROJECT_NAME=randprojname docker compose -f compose.yml down
works just with a warning: WARN[0000] The "REDIS_PORT" variable is not set. Defaulting to a blank string.

Whereas running both commands without COMPOSE_PROJECT_NAME, the down command fails with 1 error(s) decoding: * 'services[redis].ports[0]' expected a map, got 'string'.

So, even with this yaml DockerComposePassthroughTest would pass, BUT not ComposePassthroughTest

For example having the following yaml:

services:
  redis:
    image: redis
    environment:
      test=${REDIS_PORT}

And running WITHOUT COMPOSE_PROJECT_NAME the commands:
REDIS_PORT=8272 docker compose -f compose.yml up
and then:
docker compose -f compose.yml down
it succeeds and only a warning is printed again.

@mmusenbr
Copy link
Contributor Author

I have added two tests, as described above, the ComposeContainerPortViaEnvTest would also not fail with the current code, as only the warning is visible in the logs, but stopping works fine. But DockerComposeContainerPortViaEnvTest is the one, which would fail now, but is fixed with this PR.

@eddumelendez eddumelendez added this to the next milestone Mar 29, 2024
@eddumelendez eddumelendez merged commit e2b2d85 into testcontainers:main Mar 29, 2024
100 checks passed
@eddumelendez
Copy link
Member

Thanks for your contribution and detailed explanation, @mmusenbr !

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.

[Bug]: DockerComposeContainer: Stop exception when variable substitution is used in compose file
2 participants