tighten / takeout

Docker-based development-only dependency manager. macOS, Linux, and WSL2-only and installs via PHP's Composer... for now.
MIT License
1.59k stars 83 forks source link

Docker run command parameter order #299

Closed delta1186 closed 1 year ago

delta1186 commented 1 year ago

@mattstauffer I believe that I have identified a bug with the new implementation of #218. Currently the implementation appends the command to the end of the $dockerRunTemplate for each service.

protected $dockerRunTemplate = '-p "${:port}":1433 \
        -e ACCEPT_EULA=Y \
        -e "TZ=America/Chicago" \
        -e SA_PASSWORD="${:sa_password}" \
        -v "${:volume}":/var/opt/mssql \
        "${:organization}"/"${:image_name}":"${:tag}"';

So say I was trying to run the following command: takeout enable mssql --default -- -e '"TZ=America/Chicago"' I would get the following output if I was dumping the console commands.

docker run -d --name "${:container_name}" --network=takeout --network-alias="${:alias}" --label com.tighten.takeout.Full_Alias=mssql2019-GA-ubuntu-16.04 --network-alias="mssql/server" --label=com.tighten.takeout.Base_Alias=mssql/s
erver -p "${:port}":1433 \n
        -e ACCEPT_EULA=Y \n
        -e SA_PASSWORD="${:sa_password}" \n
        -v "${:volume}":/var/opt/mssql \n
        "${:organization}"/"${:image_name}":"${:tag}" -e "TZ=America/Chicago"

The command will run without error; however, the container will not start and will give the following error.

/opt/mssql/bin/permissions_check.sh: line 59: exec: -e: invalid option
exec: usage: exec [-cl] [-a name] [command [arguments ...]] [redirection ...]
SQL Server 2019 will run as non-root by default.
This container is running as user mssql.
Your master database file is owned by mssql.
To learn more visit https://go.microsoft.com/fwlink/?linkid=2099216.

This is because per the Docker docs for the run command (https://docs.docker.com/engine/reference/run/) the options should come before the image. $ docker run [OPTIONS] IMAGE[:TAG|@DIGEST] [COMMAND] [ARG...]

I would like to suggest the following implementation to fix this issue.

 protected $dockerRunTemplate = '-p "${:port}":1433 \
        -e ACCEPT_EULA=Y \
        -e SA_PASSWORD="${:sa_password}" \
        -v "${:volume}":/var/opt/mssql \
>>>     $optional_parameters \
        "${:organization}"/"${:image_name}":"${:tag}"';

All of the Service templates would need to be updated and code added to inject the new variable into the template.

I have verified this works by manually updating the template for mssql to the following and ran and the container started successfully and the timezone value was updated for the container.

protected $dockerRunTemplate = '-p "${:port}":1433 \
        -e ACCEPT_EULA=Y \
        -e "TZ=America/Chicago" \
        -e SA_PASSWORD="${:sa_password}" \
        -v "${:volume}":/var/opt/mssql \
        "${:organization}"/"${:image_name}":"${:tag}"';

Let me know if you would like for me to work on a PR to address this issue.

Regards, Kevin