Add support for building storm-mesos docker images based on storm, mesos versions #120

salimane · 2016-03-30T00:23:23Z

This PR makes it possible to automate the building of storm-mesos docker images based on storm, mesos versions.

I've also included in this PR:

an onbuild version of the storm-mesos docker image to import from if needed
ability to build a JAVA 8 runtime version of the docker image
add a makefile for building and pushing docker images
update the function dockerImage in build_release.sh to use the new make target

To build a new storm-mesos docker image, just do:

make help
make images STORM_RELEASE=0.10.0 MESOS_RELEASE=0.28.0 DOCKER_REPO=mesos

This PR needs to be merged before I could finish off #83 #117 with automated docker image building on docker hub

With this PR, we can also close #109 .

cc @erikdw @lfundaro @DarinJ @dsKarthick
Thanks

erikdw · 2016-03-30T00:58:01Z

Awesome, I was gonna take this up since it seems critical to getting the whole build pipeline working, but since you've done it I can just review & test it.

erikdw · 2016-03-30T07:27:09Z

Dockerfile

@@ -1,32 +1,62 @@
 #


Looks great at 1st blush. A little unfortunate that there's so much repetition between the Dockerfile and onbuild/Dockerfile, but I'm not sure how to improve on that. Need a bit more time to grok this, a lot going on here.

yep , I thought about it too, but there is currently no "import" for Dockerfiles :)

So, I need to look at this a bit more to understand if what I'm going to point out is just nonsensical for our use-case:

in the language-examples of onbuild Dockerfiles that I looked at, they use the non-onbuild Docker image as the base for the onbuild Docker image. Maybe that doesn't make sense for us??

Ruby:

https://github.com/docker-library/ruby/blob/master/2.1/Dockerfile

https://github.com/docker-library/ruby/blob/master/2.1/onbuild/Dockerfile

Go-lang:

https://github.com/docker-library/golang/blob/1eab0db63794152b4516dbcb70270eb9dced4cbd/1.5/Dockerfile

https://github.com/docker-library/golang/blob/f1f65c0ab0097a5e3d079d5a74e2468e8d47563d/1.5/onbuild/Dockerfile

So could it make sense for us to follow a similar pattern? Maybe the configurability of all the various fundamental components (mesos, storm, java) makes this an impossible idea?

@erikdw yeah, I also came to the same questions myself. i think since everything is so configurable because we are coupling two systems, MESOS and STORM, it will be hard to have a base image. So the nature of our build is with onbuilds in the end using arguments passed at build time.

yep, same conclusion here

Cool cool, makes sense.

lfundaro · 2016-03-30T21:17:42Z

thank you @salimane, good job ! This PR covers the expectations of PR #109. I will close that one.

erikdw · 2016-04-06T07:10:33Z

Makefile

+	@echo 'ENV'
+	@echo '  STORM_RELEASE 				The targeted release version of Storm'
+	@echo '                 				Default: 0.10.0'
+	@echo '  MESOS_RELEASE      		The targeted release version of MESOS'


the formatting is off here in this whole "help" section -- I'm guessing you're using tabs and have a tab setting which makes this line up nicely for your view here?

(I realize tabs are annoyingly required in Makefiles, but I suggest limiting their use to the required locations, and not proliferating the tab contagion. Don't get me started on go-lang's love of tabs and autoformatting! 😛 )

erikdw · 2016-04-07T03:01:39Z

From my perspective the only question is about whether we are comfortable with the size of the Docker images. Once that is resolved let's merge this!

salimane · 2016-04-07T16:01:20Z

@erikdw well here are the size of the latest version of the dockerfile

0.1.0-SNAPSHOT-0.10.0-0.27.2-jdk8-onbuild         646 MB
0.1.0-SNAPSHOT-0.10.0-0.27.2-jdk7-onbuild         646 MB
0.1.0-SNAPSHOT-0.10.0-0.27.2-jdk8                 618 MB
0.1.0-SNAPSHOT-0.10.0-0.27.2-jdk7                 618 MB
0.1.0-SNAPSHOT-0.10.0-0.28.0-jdk7-onbuild         646 MB
0.1.0-SNAPSHOT-0.10.0-0.28.0-jdk8-onbuild         646 MB
0.1.0-SNAPSHOT-0.10.0-0.28.0-jdk8                 620 MB
0.1.0-SNAPSHOT-0.10.0-0.28.0-jdk7                 620 MB

0.1.0-SNAPSHOT-0.9.6-0.28.0-jdk8-onbuild          419 MB
0.1.0-SNAPSHOT-0.9.6-0.28.0-jdk7-onbuild          419 MB
0.1.0-SNAPSHOT-0.9.6-0.28.0-jdk8                  336 MB
0.1.0-SNAPSHOT-0.9.6-0.28.0-jdk7                  336 MB
0.1.0-SNAPSHOT-0.9.6-0.27.2-jdk8-onbuild          419 MB
0.1.0-SNAPSHOT-0.9.6-0.27.2-jdk7-onbuild          419 MB
0.1.0-SNAPSHOT-0.9.6-0.27.2-jdk8                  334 MB
0.1.0-SNAPSHOT-0.9.6-0.27.2-jdk7                  334 MB

As you can see the major increase of size is coming from the new storm version 0.10.0 . It's independent from the building of the docker image.

The current docker images are actually on par with the reference you provided https://hub.docker.com/r/mesosphere/storm/tags/, because there were using storm 0.9.6. The new docker images for storm 0.9.6 are also in the 300 - 400 MB range

erikdw · 2016-04-07T20:21:20Z

@salimane : ah, thanks for pointing that out!

A couple more questions along these lines:

Why are the onbuild images bigger than non-onbuild? Tried visually comparing, and the answer didn't immediately come to me.
I don't understand why the size difference for 0.9.6 to 0.10.0 is almost 300MB:

https://hub.docker.com/r/salimane/storm-mesos/tags/

0.1.0-SNAPSHOT-0.10.0-0.28.0-jdk8:   620 MB
0.1.0-SNAPSHOT-0.9.6-0.28.0-jdk8:    336 MB

Given that the size difference of the expanded binary releases is only 60MB:

% du -sh apache-storm-0.{9.6,10.0}
 23M    apache-storm-0.9.6
 84M    apache-storm-0.10.0

I'll try to look into this.... an initial guess is the use of the matrix-style building with the COPY . /tmp instruction is resulting in extra unneeded stuff getting copied into the resultant images.

salimane · 2016-04-07T21:33:07Z

@erikdw yep, but everything is being cleaned up, removed with apt-get clean && rm -rf /var/lib/apt/lists/* /tmp/* /var/tmp/* in the dockerfile

erikdw · 2016-04-18T03:51:43Z

@salimane : so I finally got some time to look into this docker size issue -- I think I understand why the size is so much bigger for storm-0.10.0: the images contain multiple copies of the same storm bits.

The COPY command actually builds a new layer, and the contents of that layer don't disappear just because we delete the dir that they were copied to (/tmp) in the subsequent RUN command.
I enumerated the differences between the COPY layers of 2 of your docker images:
- 0.1.0-SNAPSHOT-0.9.6-0.27.2-jdk8 (334 MB)
- 0.1.0-SNAPSHOT-0.10.0-0.27.2-jdk8 (618 MB)

Total of ~100 MB of extra stuff in the COPY'ed files:

/tmp/_release/storm-mesos-0.1.0-SNAPSHOT-storm0.9.6-mesos0.28.0 31621120
/tmp/_release/storm-mesos-0.1.0-SNAPSHOT-storm0.9.6-mesos0.28.0.tgz 27437073
/tmp/apache-storm-0.9.6.tar.gz 21010966
/tmp/storm-mesos-0.1.0-SNAPSHOT-storm0.9.6-mesos0.28.0.tgz 27437073

31621120 + 27437073 + 21010966 + 27437073 = 107506232

Total of ~335 MB of extra stuff in the COPY'ed files:

/tmp/_release/storm-mesos-0.1.0-SNAPSHOT-storm0.10.0-mesos0.28.0 95404032
/tmp/_release/storm-mesos-0.1.0-SNAPSHOT-storm0.10.0-mesos0.28.0.tgz 84124376
/tmp/apache-storm-0.10.0.tar.gz 77491244
/tmp/storm-mesos-0.1.0-SNAPSHOT-storm0.10.0-mesos0.28.0.tgz 84124376

95404032 + 84124376 + 77491244 + 84124376 = 341144028

So, I believe this will be ok so long as we have a fully clean repo that we do the docker build from. I'm assuming that it would be fully clean when we do this in travis-ci, but I'm not sure exactly how we're planning to integrate the invocation of make images & make push, and whether the repo would be clean.

…sos versions Signed-off-by: Salimane Adjao Moustapha <me@salimane.com>

salimane · 2016-04-18T06:45:43Z

@erikdw nice find.

I've added a .dockerignore to ignore those extra files.

The docker images sizes decreased :

0.1.0-SNAPSHOT-0.10.0-0.27.2-jdk8             431 MB
0.1.0-SNAPSHOT-0.10.0-0.27.2-jdk7             326 MB
0.1.0-SNAPSHOT-0.10.0-0.27.2-jdk8-onbuild     73 MB
0.1.0-SNAPSHOT-0.10.0-0.27.2-jdk7-onbuild     73 MB
0.1.0-SNAPSHOT-0.10.0-0.28.0-jdk8             433 MB
0.1.0-SNAPSHOT-0.10.0-0.28.0-jdk7             328 MB
0.1.0-SNAPSHOT-0.10.0-0.28.0-jdk8-onbuild     73 MB
0.1.0-SNAPSHOT-0.10.0-0.28.0-jdk7-onbuild     73 MB


0.1.0-SNAPSHOT-0.9.6-0.27.2-jdk8              374 MB
0.1.0-SNAPSHOT-0.9.6-0.27.2-jdk7              269 MB
0.1.0-SNAPSHOT-0.9.6-0.27.2-jdk8-onbuild      73 MB
0.1.0-SNAPSHOT-0.9.6-0.27.2-jdk7-onbuild      73 MB
0.1.0-SNAPSHOT-0.9.6-0.28.0-jdk8              376 MB
0.1.0-SNAPSHOT-0.9.6-0.28.0-jdk7              271 MB
0.1.0-SNAPSHOT-0.9.6-0.28.0-jdk7-onbuild      73 MB
0.1.0-SNAPSHOT-0.9.6-0.28.0-jdk8-onbuild      73 MB

Thanks

erikdw · 2016-04-18T07:00:02Z

@salimane : awesome, I didn't know about .dockerignore! I'm gonna merge this, so then after that I presume you'll send a PR for integrating this into the travis build? BTW, locally I had to make a tweak to do an apt-get -y update before apt-get -y install software-properties-common. I'll put that in (along with some tab erasure) after the merge.

erikdw · 2016-04-18T07:23:44Z

Oh, I had a conflict when I merged my changes with yours -- I have a guess that the extra apt-get -y update before apt-get -y install software-properties-common isn't necessary after you removed this cmd from the mesos-distro logic: rm -rf /var/lib/apt/lists/*. So I'm not going to add that in anymore. I'm still going to kill the tabs though!

erikdw · 2016-04-18T09:06:47Z

FYI, #131 contains my tab excising change.

salimane · 2016-04-18T09:16:10Z

@erikdw #130 has the necessary changes to integrate travis ci

erikdw · 2016-04-18T09:26:41Z

thanks @salimane : reviewing now

erikdw mentioned this pull request Mar 30, 2016

onbuild triggers for mesos/storm versioning and building #109

Closed

erikdw reviewed Mar 30, 2016
View reviewed changes

erikdw mentioned this pull request Apr 4, 2016

logviewer support for configurable storm.log.dir #118

Closed

erikdw reviewed Apr 6, 2016
View reviewed changes

salimane force-pushed the docker_builds branch from 074ca70 to 6eeb7ad Compare April 6, 2016 13:33

salimane force-pushed the docker_builds branch from 6eeb7ad to 4c368be Compare April 7, 2016 16:12

erikdw mentioned this pull request Apr 7, 2016

Add automated archive builds and GitHub Releases Uploading #117

Merged

This was referenced Apr 9, 2016

improve docker image tag name #95

Closed

improve handling of mesos version in the build #96

Open

Add support for building storm-mesos docker images based on storm, me…

0d46917

…sos versions Signed-off-by: Salimane Adjao Moustapha <me@salimane.com>

salimane force-pushed the docker_builds branch from 4c368be to 0d46917 Compare April 18, 2016 06:17

erikdw merged commit fa0858f into mesos:master Apr 18, 2016

erikdw mentioned this pull request Apr 18, 2016

Ability to set docker user and docker registry in build-release.sh #92

Closed

salimane deleted the docker_builds branch April 18, 2016 09:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for building storm-mesos docker images based on storm, mesos versions #120

Add support for building storm-mesos docker images based on storm, mesos versions #120

salimane commented Mar 30, 2016

erikdw commented Mar 30, 2016

erikdw Mar 30, 2016

salimane Mar 30, 2016

erikdw Apr 6, 2016

lfundaro Apr 6, 2016

salimane Apr 6, 2016

erikdw Apr 7, 2016

lfundaro commented Mar 30, 2016

erikdw Apr 6, 2016

erikdw commented Apr 7, 2016

salimane commented Apr 7, 2016

erikdw commented Apr 7, 2016

salimane commented Apr 7, 2016

erikdw commented Apr 18, 2016 •

edited

Loading

salimane commented Apr 18, 2016

erikdw commented Apr 18, 2016

erikdw commented Apr 18, 2016 •

edited

Loading

erikdw commented Apr 18, 2016

salimane commented Apr 18, 2016

erikdw commented Apr 18, 2016

Add support for building storm-mesos docker images based on storm, mesos versions #120

Add support for building storm-mesos docker images based on storm, mesos versions #120

Conversation

salimane commented Mar 30, 2016

erikdw commented Mar 30, 2016

erikdw Mar 30, 2016

Choose a reason for hiding this comment

salimane Mar 30, 2016

Choose a reason for hiding this comment

erikdw Apr 6, 2016

Choose a reason for hiding this comment

lfundaro Apr 6, 2016

Choose a reason for hiding this comment

salimane Apr 6, 2016

Choose a reason for hiding this comment

erikdw Apr 7, 2016

Choose a reason for hiding this comment

lfundaro commented Mar 30, 2016

erikdw Apr 6, 2016

Choose a reason for hiding this comment

erikdw commented Apr 7, 2016

salimane commented Apr 7, 2016

erikdw commented Apr 7, 2016

salimane commented Apr 7, 2016

erikdw commented Apr 18, 2016 • edited Loading

salimane commented Apr 18, 2016

erikdw commented Apr 18, 2016

erikdw commented Apr 18, 2016 • edited Loading

erikdw commented Apr 18, 2016

salimane commented Apr 18, 2016

erikdw commented Apr 18, 2016

erikdw commented Apr 18, 2016 •

edited

Loading

erikdw commented Apr 18, 2016 •

edited

Loading