-
Notifications
You must be signed in to change notification settings - Fork 177
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
Smaller baseimage with Alpine #66
Conversation
We don't need git, pip and the dev libraries for rdd in the production image.
The timezone argument can get seeded alternatively via the TZ env variable or the special GRAPHITE_TIME_ZONE env.
Guess so, latest statsd 'release' doesn't work with newest nodejs. There are some patches in master which fix compatibility. I'm using https://github.com/etsy/statsd/archive/8d5363cb109cc6363661a1d5813e0b96787c4411.tar.gz#/statsd-8d5363c.tar.gz in the Fedora packages. |
Yes, what @piotr1212 said. We can use newer NodeJS with a patch then. |
But much appreciated, @bebehei , I had this idea to do the same, but I'm a bit busy recently. Thanks! |
Ok, statsd seems to be running now properly with the given NodeJS version. Thanks @piotr1212 |
This script shall only run during initialisation
This improves build time, as the base image already contains most of the packages for the dev installation
Even though statsd didn't get updated for a long time, the patches on current master should suffice to make it run on a current NodeJS.
This is absolutely not necessary. If someone really wants to edit and save the config, mount the file.
HERE documents have got the ability to remove leading tabs, when used with a trailing dash (<<-). When using spaces, the indentation wouldn't be possible. http://pubs.opengroup.org/onlinepubs/009695399/utilities/xcu_chap02.html#tag_02_07_04
This allows faster startup time and uses paralell initialisation.
Well, graphite web and graphite do work properly. Redis, statsd and collectd do start, too. But I'm not sure, if they're still properly configured. I don't use them, so I can't judge, if they're still running properly. But for the rest, it seems to work flawlessly. |
@deniszh I'd love to get a review for this code. I know it's a lot. Take your time. But at the current point, I myself consider my work on this PR as "finished" (of course except for fixes of upcoming comments). |
Well, I already did one comment - could you please check? |
@deniszh Where? I don't see anything. May it be, you started a review, but didn't submit it fully? |
Ah, nevermind, just found out that's not relevant. Plz give me some time to properly review it, especially in corner cases. Thanks a lot! |
OK, change looks good, thanks, @bebehei ! |
It builds fine, but I'm currently testing it on my machine. But I'm not considering it mergable yet.
This gets rid of the phusion image and uses Alpine as the base. You don't have to care about security updates anymore, as these are included in Alpine.
Also it's possible to build it on aarch64 (and probably on all other docker supported archs, too). Also it builds much faster on my Odroid C2 (about 1/3 of the time).
This reduces the image size dramatically from ~530MB to 188MB.
Fixes #55
I've got a question: Why did you install NodeJS 6 in the image? Is it because statsd's latest release is from 2016 and NodeJS 6 then was the latest version? I don't find any notice in the commits.