Skip to content
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

WIP: Docker-ng (alpine) #1919

Closed
wants to merge 28 commits into from
Closed

Conversation

0xdevalias
Copy link
Member

@0xdevalias 0xdevalias commented Mar 2, 2017

This is a WIP to track my efforts on docker-ng (#1874).

FYI, I fully expect this build to fail currently, and am making no efforts to fix that side of things till it's all working well locally. May need to cherry pick/refactor the history depending on how #1876 / etc go.

Would love any comments/feedback/testing/etc.

  • Builds from alpine image supporting therubyracer (usualoma/ruby-with-therubyracer:2.4.0-alpine)
  • Ruby 2.4 (may need to wait on Support Ruby >=2.4 #1876)
  • Currently setup to be tested against mysql:8.0
  • Seems to have issues with utf8m4
  • May not have the full binary dependencies installed for all gems at runtime
  • Unicorn not currently linked to a reverse-proxy
  • See ./compose-huginn-ng-mysql.sh
    • ./compose-huginn-ng-mysql.sh build huginn
    • ./compose-huginn-ng-mysql.sh run —user="root" huginn sh

Sizing

  • Currently includes git as a runtime dependency, unsure if needed (it’s heavy.. ~16mb)
  • Builds to an image ~366 MB (by docker images)
  • usualoma/ruby-with-therubyracer:2.4.0-alpine is ~115mb of that
  • For comparison, current 'legacy' docker builds (ubuntu) are:
    • cantino/huginn ~884 MB
    • cantino/huginn-single-process ~690 MB

* Builds from alpine image supporting therubyracer (usualoma/ruby-with-therubyracer:2.4.0-alpine)
* Ruby 2.4
* Currently setup to be tested against mysql:8.0
* Seems to have issues with utf8m4
* May not have the full binary dependencies installed for all gems at runtime
* Unicorn not currently linked to a reverse-proxy
* Currently includes git as a runtime dependency, unsure if needed (it’s heavy.. ~16mb)
* Builds to an image ~366 MB (by docker images)
* usualoma/ruby-with-therubyracer:2.4.0-alpine is ~115mb of that
* See ./compose-huginn-ng-mysql.sh
    * ./compose-huginn-ng-mysql.sh build huginn
    * ./compose-huginn-ng-mysql.sh run —user=“root” huginn sh
Copy link
Collaborator

@dsander dsander left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very cool @alias1! I did not expect the image to be that much smaller.

Sadly the build failed for me:

tep 11/16 : RUN set -ex     && bundle update rails
 ---> Running in acea035a250e
+ bundle update rails
The dependency tzinfo-data (>= 0) will be unused by any of the platforms Bundler is installing for. Bundler is installing for ruby but the dependency is only for x86-mingw32, x86-mswin32, x64-mingw32. To add those platforms to the bundle, run `bundle lock --add-platform mingw, mswin, x64_mingw`.
Fetching gem metadata from https://rubygems.org/......
Fetching version metadata from https://rubygems.org/..
Fetching dependency metadata from https://rubygems.org/.
You have requested:
  json ~> 1.8.5

The bundle currently has json locked at 1.8.3.
Try running `bundle update json`

If you are updating multiple gems in your Gemfile at once,
try passing them all to `bundle update`
The command '/bin/sh -c set -ex     && bundle update rails' returned a non-zero code: 7

Gemfile Outdated
@@ -117,7 +126,7 @@ gem 'mini_magick'
gem 'multi_xml'
gem 'nokogiri'
gem 'omniauth', '~> 1.3.1'
gem 'rails', '~> 5.0.1'
gem 'rails', '~> 5.0.2'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should update and test the rails upgrade in a separate PR.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Absolutely agree. Not sure this will end up being the final PR as is, might refactor the commit history or something in the end. I tend to test things as I go and commit in small chunks. Bumped this one (i think) due to a requirement in one of the other gems I bumped.

Gemfile Outdated
@@ -192,14 +201,22 @@ ENV['DATABASE_ADAPTER'] ||=
'mysql2'
end

if_true(ENV['DATABASE_ADAPTER'].strip == 'postgresql') do
gem 'pg', '~> 0.18.3'
if_true(ENV['DATABASE_ADAPTER'].strip == 'postgresql' || !!ENV['INSTALL_ALL_DBS']) do
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good idea! I bet the additional space needed in the image is not much compared to the improved startup time? Maybe rename it to INSTALL_ALL_DATABASE_ADAPTERS?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't checked to any great level of detail, but I think it was pretty minimal, maybe a few mb? The main difference comes from needing all of the dev libraries for each DB to compile them, but initial testing seems to imply that we can remove them again once they're compiled, so long as we have the runtime libs installed (much smaller)

Yeah, can definitely change it to INSTALL_ALL_DATABASE_ADAPTERS.

@@ -0,0 +1,213 @@
FROM usualoma/ruby-with-therubyracer:2.4.0-alpine
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you remake this image based on ruby 2.3? I think it would make this PR a bit lighter and separate it from the ruby upgrade in #1876. I would also like to see how much work is required to switch ruby versions in alpine as we will need to move to ruby 2.4.x and 2.x eventually.

Copy link
Member Author

@0xdevalias 0xdevalias Mar 5, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, no. The only image that exists is built on 2.4

That said, can definitely investigate whether the same technique would work on a 2.3.x build, and if so, we can definitely try it. Would be awesome to isolate the changes/differences required, and give us a far better platform for compatibility checking.

Have opened an issue (usualoma/ruby-with-therubyracer#2) to investigate this.

Copy link
Member Author

@0xdevalias 0xdevalias Mar 6, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@usualoma is an absolute legend. Pushed 2.2.x/2.3.x images already!

@dsander Will check them out this evening/when I get a spare second to work on this again.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, sounds great!

# libmysqlclient-dev libpq-dev libsqlite3-dev \
# ruby2.3 ruby2.3-dev

# TODO: Put together a little script that will easily parse out all the .env.example lines to copy into here
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea of dropping the .env file in the docker image. We need to decide how we handle backwards compatibility, the current solution with .env` sometimes requires double escaped strings which will probably not work without converting the values here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can put together a little script to 'update' a double escaped .env file? Or maybe just an 'upgrading' type 'breaking changes' document could explain this process.

docker-compose has built in support for using .env-styled files, and then you can override it further in the environment key:

env_file:
  - ../.env
environment:
  # These values override the env_file
  DATABASE_HOST: db
  #etc


# TODO: Need to tune the .dockerignore, or prevent some things being copied here
# eg.
# Gemfile.lock has caused issues in testing
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What were those issues? I think the Gemfile.lock is essential to ensure the image uses our locked gem version.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Absolutely going to keep the Gemfile.lock for final production version. The issues were more around me bumping gems in the Gemfile, not updating the file on my bare system, then building the docker image; it wasn't passing through the Gemfile.lock in the initial stages (commented out), so the initial gem installs work, but then later when it adds the full huginn source, the Gemfile.lock is added, and so it won't start properly.

So, the main issue is laziness during testing ;p

@0xdevalias
Copy link
Member Author

@dsander Off to a promising start for sure! But we won't count our chickens just yet. There may be additional runtime dependencies that are needed which might bump the image size up more.

Will address most of your comments inline, but the build failing is related to the Gemfile.lock thing.

@0xdevalias
Copy link
Member Author

0xdevalias commented Mar 8, 2017

I'll go back and clean up the history once this is in a working state.

@dsander Should be able to build now, and with less 2.4.x weirdness mixed in. Exposing huginn on 3000, but it didn't seem to be loading resources. ./compose-huginn-ng-mysql.sh

huginn_1  | [b36bf86b-fca8-4575-8864-d0c598dd6d51] Started GET "/images/" for 172.18.0.1 at 2017-03-07 23:51:59 -0800
huginn_1  | [b36bf86b-fca8-4575-8864-d0c598dd6d51]
huginn_1  | [b36bf86b-fca8-4575-8864-d0c598dd6d51] ActionController::RoutingError (No route matches [GET] "/images"):
huginn_1  | [b36bf86b-fca8-4575-8864-d0c598dd6d51]
huginn_1  | [b36bf86b-fca8-4575-8864-d0c598dd6d51] config/initializers/silence_worker_status_logger.rb:3:in `call'

Also maybe some DB errors

db_1      | 2017-03-08T07:58:19.985072Z 7 [Note] Aborted connection 7 to db: 'huginn' user: 'huginn' host: '172.18.0.3' (Got an error reading communication packets)

@dsander
Copy link
Collaborator

dsander commented Mar 9, 2017

@alias1 Nice, it got a bit bigger for me, but still a lot smaller then the current image.

Exposing huginn on 3000, but it didn't seem to be loading resources. ./compose-huginn-ng-mysql.sh

It's probably missing the RAILS_SERVE_STATIC_FILES environment variable. The database errors look normal they are probably from rake db:migrate or some other task that is done before starting the application server.

I am wondering if we can keep most of the old script structure and the init` file for now. By "only" changing the OS and dependencies we would not need to worry about the initialization process because I assume bash in alpine works the same it does in ubuntu.

Putting the docker file in docker/single-process/Dockerfile should also make it build automatically for this PR.

@0xdevalias
Copy link
Member Author

0xdevalias commented Mar 9, 2017

I'll have a look over the scripts again when i get chance. The intention was to encode all of the setup steps into the dockerfile itself, and should largely be using the same init script, but i may have missed some bits.

Note to self: Nice way of handling the DB linking https://github.com/docker-library/redmine/blob/master/docker-entrypoint.sh#L32

@dsander
Copy link
Collaborator

dsander commented Mar 10, 2017

I have seen two different patterns of docker image builds, one is putting everything in the Dockerfile and the other is to put everything that does more then adding or executing a binary into a bash script. I kept the latter because I find it easier to read and modify the bash code when it is in a file with syntax highlighting.
Is there a benefit of having it all in the Dockerfile?

@0xdevalias
Copy link
Member Author

0xdevalias commented Mar 11, 2017 via email

@0xdevalias
Copy link
Member Author

0xdevalias commented Mar 19, 2017

Ok.. had a quick look at the old scripts.. seems I failed to read/copy from them properly. Also.. time gets away so quickly..

  • Add asset compilation steps/etc from ./legacy/scripts/setup to dockerfile

@0xdevalias
Copy link
Member Author

Thinking some more about this.. I wonder if we want/need to keep the ubuntu base image, as opposed to a debian based one? That way we could use the official ruby docker image

@dsander
Copy link
Collaborator

dsander commented Apr 9, 2017

I'm not convinced if we'll be able to get it to a running state at the moment (till some upstream issues are fixed

That's a bummer, but kind of resembles my experiences with complex applications on alpine. The main issue is still libv8?

I figured that once properly setup, they would be minimal effort to maintain, and would give people the option in case they don't want the restrictions of an alpine-based environment.

Not sure about this, I think if we add an alpine image it should support everything the ubuntu/debian based image supported and then there should not be a need to keep the old images. Even if the setup is very clean it still adds environments that we need to support.

I suppose it could satisfy both?

Neat, I have no experience with using docker in development so I don't know how one typically sets uses it.

Thinking some more about this.. I wonder if we want/need to keep the ubuntu base image, as opposed to a debian based one? That way we could use the official ruby docker image

I don't see why we need to keep the ubuntu base image. It would probably add some more layers but the same time reduce the amount of data to be downloaded when our layer cache is invalidated or fails (which seems to happen sometimes on travis).

dsander added a commit to dsander/huginn that referenced this pull request Apr 9, 2017
dsander added a commit to dsander/huginn that referenced this pull request Apr 9, 2017
@0xdevalias
Copy link
Member Author

0xdevalias commented Apr 10, 2017

That's a bummer, but kind of resembles my experiences with complex applications on alpine. The main issue is still libv8?

I believe this is the main issue to watch, as far as therubyracer is concerned: rubyjs/therubyracer#378

Not sure about this, I think if we add an alpine image it should support everything the ubuntu/debian based image supported and then there should not be a need to keep the old images. Even if the setup is very clean it still adds environments that we need to support.

It does add extra environments to support.. but then I would think that we would still want to support 'normal' OS's, for people who aren't going to be using the docker images. And I think the overhead of building the extra image or so would be minimal.

Neat, I have no experience with using docker in development so I don't know how one typically sets uses it.

I suppose we'll find out! :)

I don't see why we need to keep the ubuntu base image. It would probably add some more layers but the same time reduce the amount of data to be downloaded when our layer cache is invalidated or fails (which seems to happen sometimes on travis).

nods that makes sense. Well.. as a proof of concept I think i'll work to convert the current images to the 'new dockerfile layout' anyway (since it's halfway done). If nothing else, it should prove that the new layout is sane.

@cantino Do you have any strong feelings one way or another around keeping/ditching the ubuntu image?

@cantino
Copy link
Member

cantino commented Apr 12, 2017

I don't have enough Docker experience to have an opinion on this.

@0xdevalias
Copy link
Member Author

It's not really a docker question as much as a platforms people can run in docker question.

I'm kind of heavily leaning towards the fact that we should have an image for all of our supported platforms though. If for no other reason than making testing trivial.

@dsander
Copy link
Collaborator

dsander commented Apr 13, 2017

I'm kind of heavily leaning towards the fact that we should have an image for all of our supported platforms though. If for no other reason than making testing trivial.

Not sure how much we would gain from it. The docker images are differing from the manual installation quite a bit. We could ensure that all gems install properly and the processes start, but I can't remember the last change that broke anything like that.
I think Vagrant would be better suited to test the actual installation on different operating systems.

@0xdevalias
Copy link
Member Author

Hrmm.. thats probably fair. When you say it's differing from the manual install processes, I wonder by how much/in what ways? Just due to docker quirks (eg. separated DB/huginn install/etc), or due to some process that we could better align between the two install methods?

@dsander
Copy link
Collaborator

dsander commented Apr 15, 2017

The main difference is that the manual installation uses runit to start the Huginn processes, which causes the majority of debug reports (mostly due to some package oddity or other installed packages causing incompatibilities).

@Skarlso
Copy link
Contributor

Skarlso commented May 10, 2017

@alias1 Seems like the rubyracer isn't maintained any longer. :/

@0xdevalias
Copy link
Member Author

@Skarlso Oh? What makes you say that?

@Skarlso
Copy link
Contributor

Skarlso commented May 10, 2017

@alias1 Mainly because of the lack of responses and significant commits, or any kind of change around the PRs.

But most likely, because of this comment for example four months ago:

It is true that a lot of deadlock issues will be solved with a 1.0 release. However, I have stalled out a bit because I no longer use therubyracer daily.
I'm currently looking for somebody to help push it over the finish line.

And of course there is this, submitted in February:

sstephenson/execjs#205

@0xdevalias
Copy link
Member Author

Interesting, thanks for pointing it out.

@dsander Any thoughts on this? Is there another way we could support the features Huginn needs without therubyracer? A friend said he was able to use node directly these days, but not quite sure what features he was using.

@dsander
Copy link
Collaborator

dsander commented May 10, 2017

@alias1 I don't think we need therubyracer in particular, but don't think calling node directly would work due to the methods are are exposing to the JavascriptAgent.

Apart from the various compilation problems I am not aware of any issues that are related to rubyracer in Huginn.

dsander added a commit to dsander/huginn that referenced this pull request May 21, 2017
dsander added a commit to dsander/huginn that referenced this pull request Jan 24, 2018
@baip
Copy link
Contributor

baip commented May 25, 2018

There seems to be a recipe for therubyracer: see Lesson 1 here.

dsander added a commit to dsander/huginn that referenced this pull request Oct 23, 2018
dsander added a commit to dsander/huginn that referenced this pull request Apr 17, 2019
@0xdevalias
Copy link
Member Author

Super outdated + abandoned

@0xdevalias 0xdevalias closed this Mar 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants