New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

First PoC iteration for Docker build #26

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
3 participants
@stp-ip

stp-ip commented Sep 17, 2018

No description provided.

@stp-ip stp-ip changed the title from [WIP] First PoC iteration for Docker build to First PoC iteration for Docker build Sep 17, 2018

WORKDIR /source
RUN curl -Lo strichliste.tar.gz https://github.com/strichliste/server/releases/download/v0.2.3/strichliste.tar.gz
RUN tar -xf strichliste.tar.gz
RUN rm -r strichliste.tar.gz

This comment has been minimized.

@foosinn

foosinn Sep 18, 2018

I would prefer to put the Dockerfile in the project root and use ADD . /source and .dockerignore.
Using commands to download files leads to issues with dockers caching. If you really want to download it you can use the ADD <url> command.

@foosinn

foosinn Sep 18, 2018

I would prefer to put the Dockerfile in the project root and use ADD . /source and .dockerignore.
Using commands to download files leads to issues with dockers caching. If you really want to download it you can use the ADD <url> command.

This comment has been minimized.

@stp-ip

stp-ip Sep 18, 2018

The release being downloaded has much more than just the plain source code.
It vendors php deps, pulls in the frontend etc.
So ADD . /source is not that easy here. Sidenote: COPY is preferred over ADD.
ADD <url> is usually not recommended as it does quite a bit of magic behind the scenes.

Not sure, what the caching problems would be. Could you elaborate on that?

@stp-ip

stp-ip Sep 18, 2018

The release being downloaded has much more than just the plain source code.
It vendors php deps, pulls in the frontend etc.
So ADD . /source is not that easy here. Sidenote: COPY is preferred over ADD.
ADD <url> is usually not recommended as it does quite a bit of magic behind the scenes.

Not sure, what the caching problems would be. Could you elaborate on that?

This comment has been minimized.

@foosinn

foosinn Sep 18, 2018

AFAIK docker build will use a cache if no change was made to the Dockerfile and the upstream image was not pulled.

@foosinn

foosinn Sep 18, 2018

AFAIK docker build will use a cache if no change was made to the Dockerfile and the upstream image was not pulled.

This comment has been minimized.

@stp-ip

stp-ip Sep 18, 2018

As we are using versioned releases, which should be immutable, a new release means a change to the dockerfile and therefore clearing the cache. But I might be missing something.

@stp-ip

stp-ip Sep 18, 2018

As we are using versioned releases, which should be immutable, a new release means a change to the dockerfile and therefore clearing the cache. But I might be missing something.

COPY entrypoint.sh /source/entrypoint.sh
RUN chmod +x /source/entrypoint.sh
RUN adduser -u 82 -D -S -G www-data www-data

This comment has been minimized.

@foosinn

foosinn Sep 18, 2018

Maybe we should use a environment variable to supply the uid and gid? That simplifies migrations and backups.

@foosinn

foosinn Sep 18, 2018

Maybe we should use a environment variable to supply the uid and gid? That simplifies migrations and backups.

This comment has been minimized.

@stp-ip

stp-ip Sep 18, 2018

For the simple case having them supplied by ENV seems more error prone. It will probably lead to more "database isn't working", "permissions don't work", "it's complaining about my user" problems. Also using ENVs would mean we have to add the user and permissions within the entrypoint script, which leads us to running root for the full container again, which is usually something we should discourage.

@stp-ip

stp-ip Sep 18, 2018

For the simple case having them supplied by ENV seems more error prone. It will probably lead to more "database isn't working", "permissions don't work", "it's complaining about my user" problems. Also using ENVs would mean we have to add the user and permissions within the entrypoint script, which leads us to running root for the full container again, which is usually something we should discourage.

This comment has been minimized.

@foosinn

foosinn Sep 18, 2018

I usually provide default values in the Dockerfile. If you create the user in the entrypoint script there should not be any pitfalls.

ENV UID=1000 \
    GID=1000
@foosinn

foosinn Sep 18, 2018

I usually provide default values in the Dockerfile. If you create the user in the entrypoint script there should not be any pitfalls.

ENV UID=1000 \
    GID=1000

This comment has been minimized.

@stp-ip

stp-ip Sep 18, 2018

Both useradd and chown need root. So using USER www-data wouldn't be possible and the container starts and runs as root. That's the primary argument against setting them as ENVs in my view.

@stp-ip

stp-ip Sep 18, 2018

Both useradd and chown need root. So using USER www-data wouldn't be possible and the container starts and runs as root. That's the primary argument against setting them as ENVs in my view.

Show resolved Hide resolved docker/Dockerfile
@schinken

This comment has been minimized.

Show comment
Hide comment
@schinken

schinken Sep 21, 2018

Contributor

Actually, I would prefer that this docker image:

  • Is "production ready"
  • Is using docker compose with a separate nginx/fpm container

Also we can think about using a "real database" instead of sqlite, if that helps with the uid/gid mapping

Contributor

schinken commented Sep 21, 2018

Actually, I would prefer that this docker image:

  • Is "production ready"
  • Is using docker compose with a separate nginx/fpm container

Also we can think about using a "real database" instead of sqlite, if that helps with the uid/gid mapping

@stp-ip

This comment has been minimized.

Show comment
Hide comment
@stp-ip

stp-ip Sep 21, 2018

Actually, I would prefer that this docker image:

* Is "production ready"

In my view the docker image is "production ready".
We can definitely simplify it by running as root, that removes the uid and gid burden partly, but in my view that's less "production ready".

* Is using docker compose with a separate nginx/fpm container

Single container vs separate nginx and fpm container needs additional tooling and understanding of docker compose, also prevents it running on schedulers such as managed providers such as ECS or Kubernetes (GKE, AKE).

Also we can think about using a "real database" instead of sqlite, if that helps with the uid/gid mapping

The burden is shifted to the database. It's either another container (still needs host permissions) or manually done on the host. Either way a user needs to be created (even just by the package manager), which in my view makes it similar to just letting the user create a www-data user, which in most distributions is one that is available already.

stp-ip commented Sep 21, 2018

Actually, I would prefer that this docker image:

* Is "production ready"

In my view the docker image is "production ready".
We can definitely simplify it by running as root, that removes the uid and gid burden partly, but in my view that's less "production ready".

* Is using docker compose with a separate nginx/fpm container

Single container vs separate nginx and fpm container needs additional tooling and understanding of docker compose, also prevents it running on schedulers such as managed providers such as ECS or Kubernetes (GKE, AKE).

Also we can think about using a "real database" instead of sqlite, if that helps with the uid/gid mapping

The burden is shifted to the database. It's either another container (still needs host permissions) or manually done on the host. Either way a user needs to be created (even just by the package manager), which in my view makes it similar to just letting the user create a www-data user, which in most distributions is one that is available already.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment