Replace base image to alpine #240
Conversation
NingZhou-NZ
changed the title
|
Haven't had a chance to look at the full amount of changes but there's definitely too many different things put into one PR/commit. Can the changes to alpine be separated from the introduction of new containers? Any change not related should be in it's own PR. Ideally each new feature should have its own commit and the pr clearly needs to reflect that |
|
@codyfinegan @derschatta I seperated my original commit to four different commits |
|
I also added method for creating image according to the cpu architecture |
|
Let me know if you want me to make PR for each commits. |
There was a problem hiding this comment.
@NingZhou-NZ I think we should get some of the changes in here in. Although this now requires a rebase and some changes (see my comments).
| @@ -1,7 +1,6 @@ | |||
| log_errors = On | |||
| error_log = /dev/stderr | |||
| error_reporting = E_ALL & ~E_DEPRECATED | |||
| max_input_vars = 2000 | |||
There was a problem hiding this comment.
why was this change necessary?
There was a problem hiding this comment.
Changes in the PHP Dockerfiles are obsolete, they have been updated to bullseye in #248.
Moving the PHP containers to alpine is possible I think but requires a bit more work. I have experimented with it locally and can add a followup patch at a later stage.
| @@ -1,8 +1,9 @@ | |||
| FROM nginx:1.20 | |||
| ARG ARCH=amd64 | |||
| FROM $ARCH/nginx:alpine | |||
There was a problem hiding this comment.
remove ARCH (we don't want to optimise for only one architecture, rather build an image which supports both, arm and amd).
And change image to nginx:1.25-alpine
| @@ -2,7 +2,7 @@ version: "3.7" | |||
| services: | |||
|
|
|||
| pgsql93: | |||
| image: postgres:9.3 | |||
| image: postgres:9.3-alpine | |||
There was a problem hiding this comment.
all the pgsql images have been changed to alpine in #250
| container_name: totara_pgsql10 | ||
| ports: | ||
| - "5410:5432" | ||
| environment: | ||
| TZ: ${TIME_ZONE} | ||
| PGDATA: /var/lib/postgresql/data/pgdata | ||
| POSTGRES_HOST_AUTH_METHOD: trust |
There was a problem hiding this comment.
what happens without this environment variable?
There was a problem hiding this comment.
I think I try to remove the db password from local image.
| # Check the OS and make image for that OS | ||
| if [ $(uname -m) == "arm64" ]; then |
There was a problem hiding this comment.
doesn't it automatically use the one you are currently running?
| @@ -1,6 +1,7 @@ | |||
| FROM httpd:2.4 | |||
| ARG ARCH=amd64 | |||
| FROM $ARCH/httpd:2.4-alpine | |||
There was a problem hiding this comment.
remove the ARCH, we don't want to build only for one architecture.
There was a problem hiding this comment.
also docker will automatically pick the right arch depending on the host I believe.
| @@ -8,7 +8,7 @@ services: | |||
| # docker-compose -f docker-compose.yml -f compose/nginx.yml -f compose/pgsql.yml -f compose/php.yml up -d pgsql php-7.3 | |||
|
|
|||
| nodejs: | |||
| image: node:16 | |||
| image: node:16-alpine3.16 | |||
There was a problem hiding this comment.
maybe should be node:16-alpine?
| @@ -28,7 +28,8 @@ services: | |||
| - totara | |||
|
|
|||
| redis: | |||
| image: redis | |||
| image: redis:alpine | |||
| container_name: totara_redis | |||
There was a problem hiding this comment.
this could create problems if someone has been running it before, conflicts with existing containers. Not strictly necessay anyway, so maybe it's safer to not add container names in this patch.
|
actually I just found out that alpine has difference uid/guid for www-data. In the entrypoint.sh for nginx we are setting the ownership of the data root folder to www-data. This is problematic if not all images accessing it use alpine. I think we should hold off mixing alpine and non-alpine for now. It only works if all images are based on alpine I think. Maybe except the database containers as they are pretty self-contained. |
|
was able to convert two of the PHP images to use the alpine base images, see #252 as a draft |
I replced the base image of apache, nginx, postgresql, nodejs, redis and memcache to alpine linux.
The reason for replacement were that alpine linux is smaller and consume less resource and less vulnerability.
I will replace those php image in next commit.