-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Add production Alpine and Debian Stretch Docker images #225
Conversation
Uses an existing traefik server as a https reverse proxy.
(The base image already contains yarn.)
|
||
# From the project root directory | ||
storage: | ||
avatars: '../data/avatars/' |
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.
So this is /usr/src/data
? You need to adjust the VOLUME
directive in the Dockerfile accordingly. (Yes, I made a similar mistake in my PR ;))
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 is relative to /app
, so it is actually /data/avatars
.
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.
Ah, sorry for the noise.
I like this. Maybe we should go with this PR for now and try to get automated builds working (probably via Travis?). We can add sample Compose / Swarm configs later. |
You haven't any problem with npm build ? There are too many layer but it's cool :D |
@Dryusdan npm build uses scripty, which fails with the default Alpine shell, hence the |
@kaiyou yeah ! You unlock me o/ Actually I make this image (github mirror repo) : https://github.com/Dryusdan/docker-peertube/blob/master/Dockerfile.alpine |
So you can inspired if you would :) |
I have to say I really don't get the obsession of people wanting to minimize the amount of layers at all costs. With today's docker graph drivers, I believe the overhead is minimal. I'd rather optimize for better layer caching, which allows faster updates in production. (See my PR.) |
If you build your image on the same host, minimal layer is usefull... |
I suppose you mean "minimal number of layers". So why should it be better when building on the same host? A multi-layer approach will probably be much faster since you can skip building many of the base layers if only the application code was changed.
Right, with Travis there is no Docker caching currently, although there is an improvement with But does it really matter if a CI build takes 5 or 7 minutes? This is not something you impatiently wait for, do you? |
Use cache is can create some problem with version of paquet, because cache "create" outdated packet (ouch, bad english). The base image first. Fr translation : Le cache Docker est un soucis plus qu'un avantage en production, il utilise des images pas forcément à jour, et des paquets pareil. Cela pose un soucis de sécurité. |
Maybe @kaiyou @Dryusdan @djmaze you could merge your different work in one repository, and make only one PR? For example this alpine image with @djmaze docker compose or whatever. |
I'll try and merge both tonight. I'll update if I cannot deliver in time. |
My work is really different and I don't think it's that well accepted. ^^ |
I have just merged @djmaze commits into the directory structure of this PR. I have not tested yet, so it probably doesn't event build. |
…ich are not strictly required
I just tested, it seems both images build successfully. Now some live testing :) |
Quick update: the Debian image (stretch based) works fine and has been running in production for two days now. The Alpine image still segfaults and I need time or help to fix it (I have been stracing inside the containers for hours now, no success). I suggest we merge but only configure builds for the Debian image at this point. |
@kaiyou Please put your files inside a |
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.
Apart from some minor typos, this LGTM! 👍
support/docker/Dockerfile.alpine
Outdated
@@ -0,0 +1,21 @@ | |||
FROM node:8-alpine | |||
|
|||
# Install dpeendencies |
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.
- Small typo
support/docker/Dockerfile.stretch
Outdated
@@ -0,0 +1,25 @@ | |||
FROM node:8-stretch | |||
|
|||
# Install dpeendencies |
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.
- Small typo
support/docker/Dockerfile.stretch
Outdated
COPY . ./ | ||
RUN yarn install --pure-lockfile && npm run build | ||
|
||
# Configuration the application |
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.
-
Configuring
?
@Chocobozzz I did, thank you @djmaze for the review, I fixed the typos, merged the latest develop, added redis settings accordingly. |
Thanks @kaiyou for your awesome work! |
I've just test.
This other case :
PS : for succefull docker build, i've changed Dockerfile.stretch in Dockerfile and the COPY line in :
I've git cloned in src peertube for package.json and other yarn.lock dependency index availability in building process. |
As-tu essayé de build depuis la racine du projet ?
|
nop, i will try that now. |
In english for the community to be able to read: the docker image seriously lacks documentation. I'll try to remedy that. |
AS: I've just edited my messages to translate them in english.
works, but it dosent change docker-compose behavior. |
I have just updated the compose file in PR #332. Maybe its best we follow up there and make sure the resulting compose example is working properly |
This PR is currently for reference mostly, as it will conflict with #213 anyway. It implements a production Alpine-based image with the following concepts in mind:
support/docker/config
to avoid duplicating a common configuration file