-
Notifications
You must be signed in to change notification settings - Fork 5.5k
Description
Description
I'm running a deployment where the primary container is being created using docker compose up -d. The main container of the deployment is a service called app
. I'm spinning up temporary containers on a per-need basis using e.g docker compose run app bash
.
When a temporary container has been created using run
and is still running, docker compose exec app bash
will enter the run
container instead of the app
container.
I have found various issues and PRs referencing this issue, and I've been able to narrow it down to being most likely connected with the way Docker Compose assigns the container-number
label and the oneoff
label across various versions.
Related issues / PRs and history of changes:
- In 2.15.1, run containers are being incorrectly selected by exec ([BUG] command
exec
of compose v2 cant recognize running service #10359) - In v2.27.1, run containers are being incorrectly selected by exec ([BUG]
exec
command hijacks containers created withrun
#11992) - Starting with 2.29.1, run containers are supposed to be sorted at the end of the selection list so that they have lower priority than the app container (fix(containers): fix sorting logic by adding secondary sorting for one-off containers #11995). However, this sorting logic relies on the run container number being set.
- This seems to have stemmed from a decision to maintain the incorrect behavior in case users started relying on it - while still giving a higher priority to the managed service container(s) if they are running
- I unfortunately haven't been able to test that the bug is NOT present when
2.29.1 < docker compose version < 2.30.0
- In v2.29.7, a user reports that run containers are being incorrectly selected by the
--index
option, which leads to realizing that run containers should probably not have a container number label to begin with ([BUG]--index
fordocker-compose exec
command does not choose right container #12219). - Starting in 2.30.0, run containers no longer have a container number (one-off container are not indexed, and must be ignored by exec --index command #12224)
- this might be breaking the sorting logic introduced in [BUG]
--index
fordocker-compose exec
command does not choose right container #12219: in my understanding, if the container number is not set, the run container will be sorted before the app container (assumingstrconv.Atoi(containers[i].Labels[api.ContainerNumberLabel])
will return 0 - and possibly an error which is being ignored in the sorting logic from [BUG]--index
fordocker-compose exec
command does not choose right container #12219 - but I'm not familiar enough with Go to be 100% sure of this hypothesis).
- this might be breaking the sorting logic introduced in [BUG]
End result: the regression is present again, and run containers are being selected by exec
.
Steps To Reproduce
services:
app:
container_name: app
image: ubuntu
restart: always
command: tail -f /dev/null
docker compose up -d # creates `app` container
docker compose run app bash
In another terminal window:
docker compose exec app bash
Expected behavior: the above command enters the app
container.
Actual behavior: the above command enters the one-off "run" container.
Compose Version
Tried across three different Docker Compose versions:
Docker Compose version v2.18.1
- app container has a container number (set to 1)
- run container has a container number (set to 1)
- docker compose exec app bash enters the run container
Docker Compose version v2.21.0-desktop.1
- app container has a container number (set to 1)
- run container has a container number (set to 1)
- docker compose exec app bash enters the run container
Docker Compose version v2.39.1
- app container has a container number (set to 1)
- run container does not have a container number
- docker compose exec app bash enters the run container
Potential fix proposals:
(Trying to help with brainstorming a possible fix now that I'm this deep into the investigation 😂)
1. Update sorting logic?
Update the sorting logic from #12219 to always sort one-off containers which don't have a label after non-one-off containers which have a container number label?
The final sorting order used by exec
would then be:
- managed containers sorted by container number (if any are present)
- one-off containers sorted in an undetermined order (if any are present)
2. Bring back container numbers on one-off containers and fix --index
option?
This would mean essentially reverting #12224. This would enable the sorting logic to correctly consider the one-off container with container-number=1
as having the same number as the first service container, BUT in this case, the --index 1
selection logic would need to be fixed in order to skip one-off containers.