[PATCH 1/2] tools: add arguments for home directory and tag in a dockerfile

Fri Nov 1 11:46:43 AEDT 2024

Well the latest tag doesnt work for me, it could be in fact a configuration 
issue but changing the tag to stable-3.1 works fine for me and since this
is a relatively simple change without much consequences I think its worth
It. In fact I see it as enhancement for development, even from point of
View of testing the tags, its a nice little feature where one can just simply
Change the tag on the fly. Similarly I think for the directory of patchwork,
Its less error-prone using a predefined value, rather than hardcoding the
Value everywhere, and it this ever changes? Then it has to be changed in
Many places, instead of one. Even from the point of the if statement error
Highlighted below when I change the if statement the error output need to
Be changed. Last thing to point out I guess its actual linux configuration with
Regards to script it fixes. The home or ~ character resolves differently on
Different linux distros or even systems based on internal system configuration,
And same applies for docker images. Lines below are error prone, because
In the docker-compose we mount /home/patchwork/patchwork/… where as in the
Scripts we use ~/patchwork/patchwork/…. To put this to real life, if my memory
Serves me well, on ubuntu based systems, home directory for root resolves to
/root where as for users to /home/username. Because user management calls
Seem to cause issues on docker when used on Mac and many projects recommend
To comment them out because of it, the use of relative paths in scripts can then cause
Issues. I think its a good enhancement to add the possibility of change if the need arises,
And also it fixes some bugs in the script.

> On 31 Oct 2024, at 21:56, Stephen Finucane <stephen at that.guru> wrote:
> 
> On Tue, 2024-10-29 at 19:09 +0000, Herakliusz Lipiec wrote:
>> Currently to change the home directory for docker image it has
>> to be changed in many places. Dockerfile allows for use of
>> arguments and env variables, by using those now it can be changed
>> by passing arguments to docker-compose. Also adding TAG argument
>> for the same reason.
> 
> Could I ask why you want to do this? Changing the tag shouldn't be necessary,
> since 'latest' should always contain the runtimes needed to test Patchwork.
> Similarly, there should be no need to customise the directory that Patchwork
> runs under since those paths are relative to the container, not the host? I
> don't have a Mac to test this on, but this feels like a local configuration
> issue more so than anything else?  We could probably remove the 'PROJECT_HOME'
> configuration variable though since it's not used any more ('git log -S
> PROJECT_HOME' shows I should have removed it in commit a0db473213 some time
> back).
> 
> Cheers,
> Stephen
> 
>> 
>> Signed-off-by: Herakliusz Lipiec <hero24 at icloud.com>
>> ---
>> docker-compose-pg.yml      |  2 ++
>> docker-compose-sqlite3.yml |  2 ++
>> docker-compose.yml         |  2 ++
>> tools/docker/Dockerfile    | 10 ++++++----
>> tools/docker/entrypoint.sh |  9 +++++----
>> 5 files changed, 17 insertions(+), 8 deletions(-)
>> 
>> diff --git a/docker-compose-pg.yml b/docker-compose-pg.yml
>> index 44bc3ec..a9593bd 100644
>> --- a/docker-compose-pg.yml
>> +++ b/docker-compose-pg.yml
>> @@ -15,6 +15,8 @@ services:
>>       args:
>>         - UID
>>         - GID
>> +        - TAG
>> +        - PATCHWORK_DIR
>>     depends_on:
>>       - db
>>     volumes:
>> diff --git a/docker-compose-sqlite3.yml b/docker-compose-sqlite3.yml
>> index 900cb71..d26e74b 100644
>> --- a/docker-compose-sqlite3.yml
>> +++ b/docker-compose-sqlite3.yml
>> @@ -7,6 +7,8 @@ services:
>>       args:
>>         - UID
>>         - GID
>> +        - TAG
>> +        - PATCHWORK_DIR
>>     command: python3 manage.py runserver 0.0.0.0:8000
>>     volumes:
>>       - .:/home/patchwork/patchwork/
>> diff --git a/docker-compose.yml b/docker-compose.yml
>> index 73f080a..f6a59b2 100644
>> --- a/docker-compose.yml
>> +++ b/docker-compose.yml
>> @@ -20,6 +20,8 @@ services:
>>       args:
>>         - UID
>>         - GID
>> +        - TAG
>> +        - PATCHWORK_DIR
>>     depends_on:
>>       - db
>>     volumes:
>> diff --git a/tools/docker/Dockerfile b/tools/docker/Dockerfile
>> index 0a55b54..4a18c44 100644
>> --- a/tools/docker/Dockerfile
>> +++ b/tools/docker/Dockerfile
>> @@ -1,12 +1,14 @@
>> -FROM ghcr.io/getpatchwork/pyenv:latest
>> +ARG TAG="latest"
>> +FROM ghcr.io/getpatchwork/pyenv:${TAG}
>> 
>> ARG UID=1000
>> ARG GID=1000
>> -
>> +ARG PATCHWORK_DIR="/home/patchwork/patchwork"
>> ARG TZ="Australia/Canberra"
>> +
>> ENV DEBIAN_FRONTEND noninteractive
>> ENV PYTHONUNBUFFERED 1
>> -ENV PROJECT_HOME /home/patchwork/patchwork
>> +ENV PROJECT_HOME ${PATCHWORK_DIR}
>> ENV DJANGO_SETTINGS_MODULE patchwork.settings.dev
>> 
>> RUN groupadd --gid=$GID patchwork && \
>> @@ -38,4 +40,4 @@ COPY tools/docker/entrypoint.sh /usr/local/bin/entrypoint.sh
>> ENTRYPOINT ["/usr/local/bin/entrypoint.sh"]
>> CMD ["python3", "manage.py", "runserver", "0.0.0.0:8000"]
>> USER patchwork
>> -WORKDIR /home/patchwork/patchwork
>> +WORKDIR $PATCHWORK_DIR
>> diff --git a/tools/docker/entrypoint.sh b/tools/docker/entrypoint.sh
>> index c78c058..5a4085d 100755
>> --- a/tools/docker/entrypoint.sh
>> +++ b/tools/docker/entrypoint.sh
>> @@ -5,6 +5,7 @@ export DATABASE_HOST=${DATABASE_HOST:-}
>> export DATABASE_PORT=${DATABASE_PORT:-}
>> export DATABASE_USER=${DATABASE_USER:-patchwork}
>> export DATABASE_PASSWORD=${DATABASE_PASSWORD:-password}
>> +export PROJECT_HOME=${PROJECT_HOME:-/home/patchwork/patchwork/}
>> 
>> case "${DATABASE_TYPE:-}" in
>> postgres)
>> @@ -40,13 +41,13 @@ test_database() {
>> 
>> # check if patchwork is mounted. Checking if we exist is a
>> # very good start!
>> -if [ ! -f ~patchwork/patchwork/tools/docker/entrypoint.sh ]; then
>> +if [ ! -f $PROJECT_HOME/tools/docker/entrypoint.sh ]; then
To get this running, I need to change the above because ~patchwork doesnt resolve correctly
>>     cat << EOF
>> The patchwork directory doesn't seem to be mounted!
>> 
>> Are you using docker-compose? If so, you may need to create an SELinux rule.
>> Refer to the development installation documentation for more information.
>> -If not, you need -v PATH_TO_PATCHWORK:/home/patchwork/patchwork
>> +If not, you need -v PATH_TO_PATCHWORK:$PROJECT_HOME
Error here is outputs different value than the actual test without this patch.
Even if the path is correct in the if statement and it has the missing slash,
~/patchwork/ will not always resolve to /home/patchwork
I know its an edge case, but I actually think this is a quality change to the code.
>> EOF
>>     exit 1
>> fi
>> @@ -55,7 +56,7 @@ set +e
>> 
>> # check if we need to rebuild because requirements changed
>> for x in /opt/requirements-*.txt; do
>> -    if ! cmp $x ~/patchwork/$(basename $x); then
>> +    if ! cmp $x $PROJECT_HOME/$(basename $x); then
>>         cat << EOF
>> A requirements file has changed.
>> 
>> @@ -63,7 +64,7 @@ You may need to rebuild the patchwork image:
>> 
>>     docker-compose build web
>> EOF
>> -        diff -u $x ~/patchwork/$(basename $x)
>> +        diff -u $x $PROJECT_HOME/$(basename $x)
>>     fi
>> done
>> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ozlabs.org/pipermail/patchwork/attachments/20241101/ce7d6652/attachment.htm>