aws-deepracer-community / deepracer-for-cloud

Creates an AWS DeepRacing training environment which can be deployed in the cloud, or locally on Ubuntu Linux, Windows or Mac.
MIT No Attribution
337 stars 184 forks source link

Fix: Replacing unsupported flag on docker compose command #172

Closed joaocarvoli closed 6 months ago

joaocarvoli commented 6 months ago

Fixing the error unsupported flag -c on docker compose up command. This error happened because varCOMPOSE_FILES was working well to swarm but not to the else (compose) case on this check if [[ "${DR_DOCKER_STYLE,,}" == "swarm" ]];. The docker compose uses a different flag to up multiple docker images.

The solution replaces the -c occurrences with -f.

joaocarvoli commented 6 months ago
docker compose version && docker --version
> Docker Compose version v2.3.3
> Docker version 26.1.2, build 211e74b

So if the problem can be related to the version of docker compose or docker running on the user machine, should we specify the version required on docs or treat this flag based on this dependencies version?

larsll commented 6 months ago

We have updated prepare.sh to use the Ubuntu included docker.io, not the separately provided docker-ce.

On Ubuntu 22.04 I have installed:

docker-compose-v2/jammy-updates,now 2.20.2+ds1-0ubuntu1~22.04.1 amd64 [installed]
docker.io/jammy-updates,now 24.0.5-0ubuntu1~22.04.1 amd64 [installed]
nvidia-docker2/unknown,now 2.14.0-1 all [installed] 

Any PR would have to be compatible with Docker 24.

Please also provide information about what the actual problem is -- what was the wrong value? Normally the COMPOSE_FILES get put together using DR_DOCKER_FILE_SEP, so the root cause (and fix) would be somewhere else.

See https://github.com/aws-deepracer-community/deepracer-for-cloud/blob/2e3e61bd7a286a4cbc8c246a0c3f6568e9fdd3c1/scripts/training/start.sh#L118 as an example.

joaocarvoli commented 6 months ago

Ok, I already had docker and docker-compose before the DRfC. If you are saying that the docker version the user uses impacts the behavior of the DRfC it should at least be indicated on the requirements doc, not?

About the problem, it does not happen in another situation, for example on /scripts/stop.sh:

...
# Check if we will use Docker Swarm or Docker Compose
if [[ "${DR_DOCKER_STYLE,,}" == "swarm" ]]; then
    docker stack rm $STACK_NAME
else
    COMPOSE_FILES=$(echo ${DR_TRAIN_COMPOSE_FILE} | cut -f1-2 -d\ )
    export DR_CURRENT_PARAMS_FILE=""
    export ROBOMAKER_COMMAND=""
    docker compose $COMPOSE_FILES -p $STACK_NAME down
fi
...

This var $COMPOSE_FILES uses the correct -f flag to select the desired docker-compose.yml file. My solution corrects where it should happen automatically but is not happening currently.

joaocarvoli commented 6 months ago

I want to apologize for the pull request I opened regarding the problem with the docker-compose flag. It seems that the issue has resolved itself, and I'm not entirely sure what caused the resolution. As such, the pull request is no longer necessary.

Thank you for your understanding and patience. I appreciate the time you took to review the pull request, and I'm sorry for any inconvenience this may have caused.

joaocarvoli commented 6 months ago

Just for you know if you want...

These were the files that I was using during this test.

I just changed the extensions from .env to .txt to be able to share them. run.txt system.txt

larsll commented 6 months ago

I suspect the issue could have come from switching from swarm to compose; some variables require the bin/activate.sh to be run again to fully take force.

joaocarvoli commented 6 months ago

That makes sense, on my tests I was doing this activation many times and didn't realize when it worked but I found it out when you requested the files last time.