[PATCH 1/2] tools: add arguments for home directory and tag in a dockerfile
Stephen Finucane
stephen at that.guru
Fri Nov 1 21:47:23 AEDT 2024
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
> 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
> 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.
> 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
❯ docker pull ghcr.io/getpatchwork/pyenv:stable-3.2
❯ docker pull 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 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?
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/de1c07e5/attachment-0001.htm>
More information about the Patchwork
mailing list