Refactored the docker build script to only only include the gatk bundle in order to shrink the docker image size #4955

jamesemery · 2018-06-27T14:20:52Z

lbergelson · 2018-06-27T14:48:35Z

@jamesemery How big was the docker and how big is it now?

jamesemery · 2018-06-27T16:46:44Z

@lbergelson
now: 3.9GB (without ~0.3-5 GB extra off from the base image changes currently in the branch)
then: 5.22GB
A moderate but not irrelevant reduction in size. With these changes some more drastic changes will be easier, namely trying to slice down the python packages or refactoring the build jars should be easier. I intend to open another PR to upgrade the base image shortly.

droazen · 2018-06-27T17:45:19Z

@jamesemery So to clarify: including the size reductions to the base image, the total reduction will be around 1.6-1.8 GB, correct?

codecov-io · 2018-06-27T18:47:32Z

Codecov Report

Merging #4955 into master will decrease coverage by 20.624%.
The diff coverage is n/a.

@@               Coverage Diff                @@
##              master     #4955        +/-   ##
================================================
- Coverage     80.797%   60.173%   -20.624%     
+ Complexity     17952     12779      -5173     
================================================
  Files           1095      1095                
  Lines          64609     64609                
  Branches       10394     10394                
================================================
- Hits           52202     38877     -13325     
- Misses          8379     21497     +13118     
- Partials        4028      4235       +207

Impacted Files	Coverage Δ	Complexity Δ
...institute/hellbender/cmdline/TestProgramGroup.java	`0% <0%> (-100%)`	`0% <0%> (-3%)`
...er/utils/smithwaterman/SWNativeAlignerWrapper.java	`0% <0%> (-100%)`	`0% <0%> (-6%)`
...llbender/engine/filters/CountingVariantFilter.java	`0% <0%> (-100%)`	`0% <0%> (-18%)`
...ne/programgroups/CoverageAnalysisProgramGroup.java	`0% <0%> (-100%)`	`0% <0%> (-3%)`
...mlineContigPloidyHybridADVIArgumentCollection.java	`0% <0%> (-100%)`	`0% <0%> (-3%)`
...ecaller/graphs/DeadEndKBestSubHaplotypeFinder.java	`0% <0%> (-100%)`	`0% <0%> (-10%)`
...sformers/MisencodedBaseQualityReadTransformer.java	`0% <0%> (-100%)`	`0% <0%> (-4%)`
...ctions/RequiredFeatureInputArgumentCollection.java	`0% <0%> (-100%)`	`0% <0%> (-1%)`
...ls/copynumber/gcnv/GermlineCNVVariantComposer.java	`0% <0%> (-100%)`	`0% <0%> (-4%)`
...groups/StructuralVariantDiscoveryProgramGroup.java	`0% <0%> (-100%)`	`0% <0%> (-3%)`
... and 583 more

jamesemery · 2018-06-27T19:14:30Z

@droazen correct.
Generally, one issue is that this slows down the docker image creation in a somewhat substantial way. Around 10 minutes currently. Half of this is unzipping the bundled jar, and another piece is some redundant gradle downloading that can be alleviated with cache shenanigans.

lbergelson

@jamesemery A bunch of cleanup type things. I do think you should rename that second gradle script. Sharing code between them might be tricky, but at least put a comment in.

Looks good. Ready to merge once the final changes are resolved and the base image is updated.

lbergelson · 2018-06-27T14:49:03Z

.travis.yml

@@ -20,7 +20,8 @@ env:
  #gradle needs this
  - TERM=dumb
  #limit gradle jvm memory and disable daemon
-  - GRADLE_OPTS="-Xmx1024m -Dorg.gradle.daemon=false"
+  - GRADLE_OPTS="-Xmx1024m"
+   #Dorg.gradle.daemon=false"


lbergelson · 2018-06-27T15:28:19Z

scripts/docker/gatkbase/Dockerfile

+    apt-get upgrade -y && \
+    apt-get install -y python && \
+    apt-get install -y python3-pip && \
+    apt-get install --no-install-recommends -y wget curl unzip gcc python-dev python-setuptools git less lynx ## hdfview && \


remove hdfview

lbergelson · 2018-06-27T15:29:33Z

build_docker.sh

@@ -113,8 +112,22 @@ if [ -n "${IS_PUSH}" ]; then
 else
    RELEASE=false
 fi
+./gradlew clean bundle pythonPackageArchive shadowTestClassJar shadowTestJar -Drelease=$DRELEASE


it shouldn't do all the test jar stuff if you're not running tests

lbergelson · 2018-06-27T15:39:03Z

build.gradle

-     * cloud, integration, unit  : run one of the three disjoint partitions of the test suite
-     * all                       : run all the tests
-     * anything else             : run the non-cloud tests
+     * cloud, integration, unit, spark, python, bucket, funcotatorValidation  : run one of the several disjoint partitions of the test suite


bucket and funcotatorValidation aren't valid options to TEST_TYPE

lbergelson · 2018-06-27T15:41:17Z

.travis.yml

      DOCKER_TAG=$TRAVIS_COMMIT;
    fi;
    sudo docker images;
    echo ${TEST_TYPE};
    sudo mkdir -p build/reports/;
    sudo chmod -R a+w build/reports/;
-    sudo docker run -v $(pwd)/src/test/resources:/testdata -v $(pwd)/build/reports/:/gatk/build/reports/ --rm -e "TEST_VERBOSITY=minimal" -e "TEST_TYPE=${TEST_TYPE}" -t broadinstitute/gatk:${DOCKER_TAG} bash --init-file /gatk/gatkenv.rc /root/run_unit_tests.sh;
+    sudo docker run -v $(pwd):/gatksrc -v $(pwd)/testJars:/jars --rm -e "TEST_VERBOSITY=minimal" -e "TEST_TYPE=${TEST_TYPE}" -t broadinstitute/gatk:${DOCKER_TAG} bash --init-file /gatk/gatkenv.rc /root/run_unit_tests.sh;


Would this be better named gatk_root? its not just the source is it?

lbergelson · 2018-07-06T15:34:17Z

scripts/docker/build.gradle

+
+plugins {
+    id "java"           // set up default java compile and test tasks
+    id 'maven'


I doubt we need maven here

i think we need it to download jacoco? we point to maven central still.

The maven plugin is for releasing to maven, not downloading from it. That's a core gradle thing I think.

lbergelson · 2018-07-06T15:34:18Z

scripts/docker/build.gradle

+buildscript {
+    repositories {
+        mavenCentral()
+        jcenter() // for shadow plugin


You can drop jcenter from this one

lbergelson · 2018-07-06T15:34:33Z

scripts/docker/build.gradle

+}
+
+repositories {
+    mavenCentral()


You can remove this whole block I think.

lbergelson · 2018-07-06T15:35:51Z

scripts/docker/build.gradle

+
+task unpackTestJar(type: Copy){
+    String testClassesJar = "$System.env.TEST_JAR"
+    String testClassesDir = "$System.env.CP_DIR"


Add a comment explaining why this is necessary for future generations.

and a description line

lbergelson · 2018-07-06T15:39:36Z

build.gradle

+    }
+}
+
+tasks.withType(Test) {


You can put this back as just a modification of the test task since there's no second one here anymore.

i'd actually rather leave it, because we are using warnings i would rather the two test methods that need to be updated in lock step are byte identical so its as easy as possible for someone to update.

…r necessary files for space concerns

jamesemery requested a review from lbergelson June 27, 2018 14:20

droazen assigned lbergelson Jun 27, 2018

jamesemery force-pushed the je_newShrinkDockerSize branch 2 times, most recently from 4a44ec7 to f2c2253 Compare July 6, 2018 15:14

lbergelson approved these changes Jul 6, 2018

View reviewed changes

Refactored the docker image to contain only the runtime jars and othe…

63ec7e4

…r necessary files for space concerns

jamesemery force-pushed the je_newShrinkDockerSize branch from 0f0958d to 63ec7e4 Compare July 6, 2018 19:52

allelic_counts_file

69003d1

droazen merged commit d6a7fcd into master Jul 6, 2018

droazen deleted the je_newShrinkDockerSize branch July 6, 2018 21:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactored the docker build script to only only include the gatk bundle in order to shrink the docker image size #4955

Refactored the docker build script to only only include the gatk bundle in order to shrink the docker image size #4955

jamesemery commented Jun 27, 2018

lbergelson commented Jun 27, 2018

jamesemery commented Jun 27, 2018 •

edited

Loading

droazen commented Jun 27, 2018

codecov-io commented Jun 27, 2018 •

edited

Loading

jamesemery commented Jun 27, 2018

lbergelson left a comment

lbergelson Jun 27, 2018

lbergelson Jun 27, 2018

lbergelson Jun 27, 2018

lbergelson Jun 27, 2018

lbergelson Jun 27, 2018

lbergelson Jul 6, 2018

jamesemery Jul 6, 2018

lbergelson Jul 6, 2018

lbergelson Jul 6, 2018

lbergelson Jul 6, 2018

lbergelson Jul 6, 2018

lbergelson Jul 6, 2018

lbergelson Jul 6, 2018

jamesemery Jul 6, 2018

Refactored the docker build script to only only include the gatk bundle in order to shrink the docker image size #4955

Refactored the docker build script to only only include the gatk bundle in order to shrink the docker image size #4955

Conversation

jamesemery commented Jun 27, 2018

lbergelson commented Jun 27, 2018

jamesemery commented Jun 27, 2018 • edited Loading

droazen commented Jun 27, 2018

codecov-io commented Jun 27, 2018 • edited Loading

Codecov Report

jamesemery commented Jun 27, 2018

lbergelson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jamesemery commented Jun 27, 2018 •

edited

Loading

codecov-io commented Jun 27, 2018 •

edited

Loading