-
-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
Improve docker images #1359
Improve docker images #1359
Conversation
Hi @dsander, I found these errors below in the log, but these are look like warnings rather than faults.
I keep this container running and let you know if something goes wrong. |
@elvetemedve Thanks for testing! I don't really know what the mysqld stderr message means, but I think it has been there before, the foreman log line are no problem. |
Awesome! I'll admit that I still use Capistrano and don't really know Docker, though. :) Let me know how I can help. |
00fc4ed
to
6e9b690
Compare
@cantino I think two week are enough time for everyone who wanted to test it. If you are OK with having to update the descriptions on Docker Hub manually when we change them, I think we are ready to merge this. The TODO list is updated with a few tasks only you can do to due to the permissions of GitHub and Docker Hub. I will send you the environment variables for the CircleCI configuration via email. Except for disabling the automated builds on docker hub everything can be done whenever you have time (the CircleCI builds will just fail when you enable them before we merge this). Disabling the automated docker hub builds, finalizing the |
@dsander The container you created has been running for 1 weeks and 1 days without any issues, but now it's stopped, because of running out of memory. Looks like a memory leak to me.
Coredump does not tell too much about where did the failure happen:
The machine I'm using to run Huginn has 4 GB of RAM and 4GB of swap memory, while only 1GB is used from it at the moment. |
@elvetemedve I do not think that is related to the Dockerfile changes, we tried to locate the memory leak a few times, but I was never able to find where it happens #1114 #978. |
@dsander In my opinion the long term solution would be tracking the memory usage of the application for each Agent and write it into a log file. That way sooner or later the faulty code block could be revealed. Regarding Docker there is a short term solution you could implement. I noticed that the container was still running after the crash of foreman. It would be nice if you could tell supervisord to restart the process when it dies. There is an |
@elvetemedve Thanks for the tip, it tried changing the |
c67ec48
to
efc9d19
Compare
0f4706b
to
8e894eb
Compare
After trying a bunch of CI services I went back to Travis and finally got it working. Travis allows us to encrypt environment variables, those will only be exposed on branch builds of the Huginn repository, not for pull requests. We only need to make sure not to merge code that would expose the Docker Hub credentials. A build on Travis that pushes the images can be viewed here: https://travis-ci.org/dsander/huginn/builds/122077312 @cantino To merge this we only need to disable the automated builds on Docker Hub and add |
Any idea what the "Coveralls encountered an exception:" warning at the bottom is? Want me to disable the automatic builds now? |
Coveralls is not accepting the data because the builds I linked were done on my fork directly and coveralls only accepts PR builds of this repository.
Sure, I have time to update the credentials in the travis config. |
Should I disable auto builds for both cantino/huginn and cantino/huginn-single-process? |
Yes, if I remember correctly the |
Nicely done! This is a great step forward. 😄 🎉 |
In my opinion our docker images still have a few issues:
This PR tries to address all three problems with one downside, we can not use automated Docker Hub builds anymore.
Problem 3
The image build is now based on the current working directory, thus every change made locally (or checked out on the build server) are included in the image. Building custom images is now easy:
docker build -t myusername/huginn -f docker/multi-process/Dockerfile .
Problem 2
By using CircleCI to build the images we can configure when we want to tag images and push the tags to the Docker Hub after finishing a new build. What I really would like to do is build images for every PR to be able to test the changes locally, but since we need to have the Docker Hub credentials stored in
ENV
this is not possible right now.The build status is viewable here: https://circleci.com/gh/dsander/huginn
We could set up a private Jenkins instance to do the builds (and I would gladly pay the hosting bill), but I was not sure how the community would feel about a build process that is not public. It could be an option to just do the PR builds on a private server.
Problem 1
Image layer caching is leveraged by using the current working directory as the base for the image build. This could also be achieved on Docker Hub, but there the Dockerfiles would have to be placed in the Huginn root directory which would then use the root
README.md
for the image description. When we move away from automated builds the image description needs to be updated manually, but only after we changed it not for every build.For a comparison of image sizes I used e5fcb24 as baseline for the first image, then build images after the merge of #1330 and another one after #1301
Before
I will only show the layer diff for the first change (after the merge of #1330) in the second build the same layers changed.
cantino/huginn-single-process
:This is everything, for every update the user needs to download the whole image, even if the change was just one line. For the
cantino/huginn
image it is even worse:After
After merging #1330 (bf7c2fe), in which only some code was changed. (~13MB uncompressed)
huginn-single-process
:And the same small changeset for the multi-process images 🎉 :
After merging #1301 (9a588e0), we added new gems, thus the gems needed to be reinstalled. (~200MB uncompressed)
huginn-single-process
:The diff stays small for the multi-process image:
Summary
Disadvantages:
Advantages:
Since the
init
script, which does most of the heavy lifting, has been nearly untouched I do not think it should not cause any issues, but we still should get some verification the multi-process image still works (I updated to the updated single-process image myself)@elvetemedve @gdomod @csu @peterseverin @uri @sky-chen @skray @mhow2 @dannysu @xu-cheng @daturkel @kennethkalmer @apopiak If you are still using one of the Huginn Docker images would you mind testing if everything still works when you switch to
dsander/huginn
ordsander/huginn-single-process
(depending on which image you used before)?TODO
.travis.yml
.travis.yml
Needs to be done by @cantino
huginnbuilder
as contributor on Docker Hubcantino/huginn-single-process
and build triggers forcantino/huginn