Deduplicate between Volumes and Mounts in compat API #13540
Conversation
Docker Compose v2.0 passes mount specifications in two different places: Volumes (just the destination) and Mounts (full info provided - source, destination, etc). This was causing Podman to refuse to create containers, as the destination was used twice. Deduplicate between Mounts and Volumes, preferring volumes, to resolve this. Fixes containers#11822 Signed-off-by: Matthew Heon <[email protected]>
openshift-ci
bot
added
the
approved
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cevich, mheon The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Add a pair of new Cirrus test suites using Compose v2 instead of Compose v1 (as is currently packaged in Fedora). They work identically, and run the same tests, as the Compose v1 tests, but with the new v2 binary instead. [NO NEW TESTS NEEDED] This adds an entire Cirrus suite... Signed-off-by: Matthew Heon <[email protected]>
|
Tests added |
Compose v2 uses dashes as separators instead of hyphens. This broke some tests that relied upon container names. Set the name conditionally to make it safe for both. Signed-off-by: Matthew Heon <[email protected]>
|
Tests going green |
|
@containers/podman-maintainers PTAL |
| @@ -272,6 +272,11 @@ case "$TEST_FLAVOR" in | |||
| ;; | |||
| build) make clean ;; | |||
| unit) ;; | |||
| compose_v2) | |||
| dnf -y remove docker-compose | |||
| curl -SL https://github.com/docker/compose/releases/download/v2.2.3/docker-compose-linux-x86_64 -o /usr/local/bin/docker-compose | |||
There was a problem hiding this comment.
I'm always leery to have a specified version in tests rather than latest.
@edsantiago probably has a neater way, but you could get/set the version from GitHub into an envar and then use it:
# export VERSION=`curl -s https://github.com/docker/compose/releases/latest | grep "https://github.com/docker/compose/releases/tag/*" | sed 's/^.*tag\///' | sed 's/\">.*//'`
# echo $VERSION
v2.3.3
# curl -SL https://github.com/docker/compose/releases/download/${VERSION}/docker-compose-linux-x86_64 -o /usr/local/bin/docker-compose
There was a problem hiding this comment.
The latest version can break at any time. Using a fixed version is a good idea IMO.
I think we should download the compose binary at image build time to prevent unnecessary flakes. We could put it at a fixed place and then just mv it here to /usr/bin
There was a problem hiding this comment.
Just FYI, if you do end up deciding to download the latest version using the URL https://github.com/docker/compose/releases/latest/download/docker-compose-linux-x86_64 is much easier (and more robust) than greping and seding through GitHub's HTML :)
| ctr_name="two_networks_con1_1" | ||
| if [ "$TEST_FLAVOR" = "compose_v2" ]; then | ||
| ctr_name="two_networks-con1-1" | ||
| fi |
There was a problem hiding this comment.
Well that is is interesting, they changed the container name to using a dash but below for the network names they still use underscores.
|
/lgtm |
|
@Luap99 I do not, but I can look into it further |
|
I think it was added in v2.1 https://github.com/docker/compose/releases/tag/v2.1.0
|
|
Is there any appetite to backport this fix to v4.1? If not, since I'm not as familiar with the release cycle of podman as I'd like to be, when would the next fork off of main into a production branch actually be? |
|
This is in 4.1, which will be released some time next week. It will not be backported to the current 4.0 release. |
github-actions
bot
added
the
locked - please file new issue/PR
Docker Compose v2.0 passes mount specifications in two different places: Volumes (just the destination) and Mounts (full info provided - source, destination, etc). This was causing Podman to refuse to create containers, as the destination was used twice. Deduplicate between Mounts and Volumes, preferring volumes, to resolve this.
Fixes #11822
Still to-do: Get Compose v2 in our test VMs and add tests