Skip to content

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 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 previously approved these changes Mar 29, 2024
@eddumelendez eddumelendez added this to the next milestone Mar 29, 2024
@eddumelendez eddumelendez merged commit e2b2d85 into testcontainers:main Mar 29, 2024
@eddumelendez
Copy link
Member

Thanks for your contribution and detailed explanation, @mmusenbr !

fokion pushed a commit to fokion/testcontainers-java that referenced this pull request Jan 18, 2025
…#8493)

This issue was introduced in 7112db5. `DockerComposeContainer` and 
`ComposeContainer` now passes the env to the stop command in both 
implementations.

Fixes testcontainers#8492

---------

Signed-off-by: Michael Musenbrock <[email protected]>
Co-authored-by: Eddú Meléndez Gonzales <[email protected]>
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