-
Notifications
You must be signed in to change notification settings - Fork 768
FEATURE: introduce web_only images #966
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
base: main
Are you sure you want to change the base?
Conversation
f16457d
to
e016fc8
Compare
web_only images do not install postgres or redis, minimizing image sizes for setups that do not require those components. release and dev images still install postgres and redis, but do it later in the process. remove rm runs rm no longer affects image sizes as we are no longer squashing images
e016fc8
to
cc9c36e
Compare
@@ -145,14 +143,6 @@ RUN gem install pups --force &&\ | |||
ADD thpoff.c /src/thpoff.c | |||
RUN gcc -o /usr/local/sbin/thpoff /src/thpoff.c && rm /src/thpoff.c | |||
|
|||
# clean up for docker squash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We no longer use docker squash - that has been gone for a long time. Here, we now desire /tmp to stick around as a way of keeping /tmp/install-redis around for installing later within a build for discourse-dev.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we could delete these after the RUN
command that generates them to keep the image small.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The other option we have is to add a tmpfs have a for these during the build.
Note that I'm keeping /tmp around to ensure install-redis remains as needed - it's called in both this image as well as the dev image.
@@ -130,7 +129,6 @@ ADD install-jemalloc /tmp/install-jemalloc | |||
RUN /tmp/install-jemalloc | |||
|
|||
ADD install-redis /tmp/install-redis |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be moved too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'd need a way to add it to the dev image as well when we build that, as that also needs to be able to run install-redis in a different context.
My concern with copying is that any changes to install-redis could get out of sync in an update.
Probably a preferred solution or way forward that would allow is to give a shared context, IE reorganize things so all dockerfiles live under a common directory. base.dockerfile
dev.dockerfile
test.dockerfile
etc. but I'd prefer that reorganization be left for a potential future refactor stage.
@@ -145,14 +143,6 @@ RUN gem install pups --force &&\ | |||
ADD thpoff.c /src/thpoff.c | |||
RUN gcc -o /usr/local/sbin/thpoff /src/thpoff.c && rm /src/thpoff.c | |||
|
|||
# clean up for docker squash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we could delete these after the RUN
command that generates them to keep the image small.
web_only images would not install postgres or redis, minimizing image sizes for setups that do not require those components.
release and dev images still install postgres and redis, but do it later in the process.
A savings of ~80MB compressed (~260MB uncompressed)