[PATCH 1/2] tools: add arguments for home directory and tag in a dockerfile
Herakliusz Lipiec
hero24 at icloud.com
Fri Nov 1 22:20:17 AEDT 2024
> On 1 Nov 2024, at 10:47, Stephen Finucane <stephen at that.guru> wrote:
>
> On Fri, 2024-11-01 at 00:46 +0000, Herakliusz Lipiec wrote:
>>
>> 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.
>
> But that's the point: it's not. The stable-3.2 tag (which FYI just got created yesterday in order to fix build failures on that branch) is missing Python 3.13, which is part of the test matrix for current main, and has Python 3.8, which is not. The stable-3.1 tag is even older and is missing Python 3.12 but has Python 3.7. By using these older tags, you don't have the ability to run the full test matrix (i.e. running `tox` without arguments will fail on e.g. the `py313-*` testenvs). That kind of defeats the point of having that Dockerfile, since it exists solely to provide us with all the Python runtimes needed for our test matrix.
>
>> 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.
>
> This shouldn't ever be necessary though. There should never be any tags except for `latest` and various `stable-*` tags, and there's a 1:1 mapping between tags for the Docker image and branches for the git repo (effectively `latest:main` and `stable-X.Y:stable/X.Y`). If we need to change the tag, it should be because we have created a new stable branch. See [1] for example.
>
> [1] https://github.com/getpatchwork/patchwork/commit/0da30213d42e8ec5067eb598cd0ebffc0cae74a1
Comment to these below.
>
>> 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.
>
> Eh. 🤷
>
> ❯ sed -i 's/\/home\/patchwork\/patchwork/\/home\/foo\/bar/' tools/docker/Dockerfile docker-compose*
> ❯ sed -i 's/~\/patchwork/\/home\/foo\/bar/' tools/docker/Dockerfile docker-compose* tools/docker/entrypoint.sh
See this is the point. This is not going to work, its only going to change some of the paths, and others are going to fail,
Because whoever wrote these scrips, and it was probably done by few people over some period of time used a mix of
Relational and absolute paths, and some paths are even wrong, because they’ve been shorten down.
I actually think, that this whole mess-up with paths could’ve been long avoided if the path was stored in a variable (single place),
Even if lets say we dont want to give the people ability to change (which I dont understand fully why we wouldn’t),
I still think keeping the paths in env values makes is neater and less error prone.
>> 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.
>
> fwiw, I'm fine replacing `~/patchwork` with `/home/patchwork/patchwork`. In fact, I'll go do that right now.
It probably solves the issue mentioned above, but is this the right solution, in 5months time, someone gonna come
And again add a ~/patchwork/ piece, and then another someone is going to run sed -I ’s/\/home/patchwork\/foo/bar’,
And the strangling path that has a ~ is gonna cause issues.
>
>> 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.
>
> I'm afraid I think this is a case of You're Not Gonna Need It (YAGNI), and I think it would be better to diagnose the issue you're having with Docker. Are you able to pull the images outside of docker-compose? Each of the below should work just fine for you:
>
> ❯ docker pull ghcr.io/getpatchwork/pyenv:stable-3.1 <http://ghcr.io/getpatchwork/pyenv:stable-3.1>
> ❯ docker pull ghcr.io/getpatchwork/pyenv:stable-3.2 <http://ghcr.io/getpatchwork/pyenv:stable-3.2>
> ❯ docker pull ghcr.io/getpatchwork/pyenv:latest <http://ghcr.io/getpatchwork/pyenv:latest>
>
> If they're not, that's an issue you should look into because it suggests a local configuration issue that is almost certainly going to affect you elsewhere: pulling images shouldn't require any special permissions. However, if that works, you should test if you're? For example:
>
> ❯ docker run -it ghcr.io/getpatchwork/pyenv:latest <http://ghcr.io/getpatchwork/pyenv:latest> python3.13
>
> That should open the Python REPL. If that works then it confirms the issue lies elsewhere. I wasn't able to find anything from a quick web search RE: permissions issue but my money is on the fact that we mount the local directory as a volume. Have you tried for example cloning the repo to '/tmp' and running docker-compose from there? That would confirm if its file permission issue related to running from your home directory. Also, are you using docker-tools or Docker for Mac? Have you tried podman?
>
Ok, thanks to suggestions I now know what is the issue, when using docker pull it seems giving more informative errors.
The reason why I can’t pull latest or stable-3.1 is the manifest which doesn’t have support for my platform.
docker pull ghcr.io/getpatchwork/pyenv:latest
Error response from daemon: no match for platform in manifest: not found
Looking at pyenv repository with stable-3.2 there was a manifest added for OS/Arch linux/amd64
In this case its bound to fail, because Apple’s M3 is arm based, when I come to think of it its bound to cause problems on any
Arm platform. Out of curiosity I wonder how would this behave on non-unix system like Windows, but I guess thats behind the
Scope of this conversation.
I hear you on the python versioning and that it should not be necessary to change tags, Im not going to be pushy with the tag,
Although I also think it’s no harm to have it anyway, it provides an enhancement of configurability and doesn’t really add any overhead. I could understand pushback against it, if this was affecting performance in any way, but I dont see an impact like this.
Thanks,
Hero
> Hope this helps,
> Stephen
>
>>
>>> 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/8dc23a20/attachment-0001.htm>
More information about the Patchwork
mailing list