Fixed failing gatk-nightly Docker builds. #5759

jamesemery · 2019-03-05T20:17:00Z

Resolves #4965

…lt docker images rather than using the docker build machinery

jamesemery · 2019-03-05T20:52:16Z

https://travis-ci.com/broadinstitute/gatk/builds/103260089 for an example of a running cron job docker build (it uploaded to both repos correctly)

codecov-io · 2019-03-05T20:59:30Z

Codecov Report

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

@@               Coverage Diff               @@
##              master     #5759       +/-   ##
===============================================
- Coverage     86.985%   86.982%   -0.003%     
+ Complexity     31864     31861        -3     
===============================================
  Files           1943      1943               
  Lines         146770    146768        -2     
  Branches       16223     16223               
===============================================
- Hits          127668    127662        -6     
- Misses         13190     13192        +2     
- Partials        5912      5914        +2

Impacted Files	Coverage Δ	Complexity Δ
...utils/smithwaterman/SmithWatermanIntelAligner.java	`50% <0%> (-30%)`	`1% <0%> (-2%)`
...nder/utils/runtime/StreamingProcessController.java	`67.299% <0%> (-1.422%)`	`33% <0%> (-1%)`
...itute/hellbender/tools/walkers/mutect/Mutect2.java	`84.091% <0%> (ø)`	`22% <0%> (ø)`	⬇️
...ender/tools/walkers/annotator/InbreedingCoeff.java	`86.207% <0%> (ø)`	`10% <0%> (ø)`	⬇️
...ithwaterman/SmithWatermanIntelAlignerUnitTest.java	`60% <0%> (ø)`	`2% <0%> (ø)`	⬇️
.../broadinstitute/hellbender/engine/LocusWalker.java	`79.245% <0%> (+1.467%)`	`15% <0%> (ø)`	⬇️
...tute/hellbender/engine/spark/LocusWalkerSpark.java	`87.179% <0%> (+2.179%)`	`13% <0%> (ø)`	⬇️

droazen

@jamesemery Back to you with my comments

droazen · 2019-03-05T20:48:58Z

.travis.yml

 - if [[ $TRAVIS_BRANCH == master && $TRAVIS_EVENT_TYPE == cron && $UPLOAD == true ]]; then
+    docker login --username=$DOCKER_SERVICE_LOGIN --password=$DOCKER_SERVICE_PASS;
    $GCLOUD_HOME/gcloud components -q update gsutil;
    gsutil ls -l gs://gatk-nightly-builds | grep gatk | sort -r -k 2 | grep -o '\S\+$' | tail -n +11 | xargs -I {} gsutil rm {};


Do we currently mention the gs://gatk-nightly-builds bucket anywhere in our README? If not can we mention it?

droazen · 2019-03-05T20:49:19Z

.travis.yml

@@ -175,16 +175,24 @@ after_success:
  fi;

 # This creates and uploads the gatk zip file to the nightly build bucket, only keeping the 10 newest entries
-# This also pokes the Dockerhub web api which is securely stored in the travis API with the variable "DOCKERHUB_URL"
+# This also constructs the Docker image and uploads it to https://cloud.docker.com/u/broadinstitute/repository/docker/broadinstitute/gatk-nightly/


Can we mention the nightly dockerhub repo in our README (if we don't already)?

Its already mentioned. I added a line about the nightly zip folder in there.

droazen · 2019-03-05T20:50:32Z

.travis.yml

 - if [[ $TRAVIS_BRANCH == master && $TRAVIS_EVENT_TYPE == cron && $UPLOAD == true ]]; then
+    docker login --username=$DOCKER_SERVICE_LOGIN --password=$DOCKER_SERVICE_PASS;


Are the values of these variables properly secured in travis?

Yes, if you go into settings on travis you can see them. There is no way to extract the plaintext for the password (louis entered the username as unsecured). Furthermore perusing the logs it doesn't expose it when I type that command. Travis internally hides these variable names from the logs through some magic as well.

droazen · 2019-03-05T20:51:21Z

.travis.yml

-    echo "Triggering an automatic build on dockerhub";
-    curl -H "Content-Type:application/json" --data '{"build":true}' -X POST ${DOCKERHUB_URL};
+
+    sudo bash build_docker.sh  -e ${TRAVIS_COMMIT} -s -u;


Does this need to be run as root? If not, can you change this?

Also, should you be specifying a staging dir via -d, or did you deliberately want to use the working dir?

The working dir in this case saves us having to pull from lfs again. The travis environment is also essentially an untouched gatk repo at this point anyway.

droazen · 2019-03-05T20:53:00Z

.travis.yml

    $GCLOUD_HOME/gcloud components -q update gsutil;
    gsutil ls -l gs://gatk-nightly-builds | grep gatk | sort -r -k 2 | grep -o '\S\+$' | tail -n +11 | xargs -I {} gsutil rm {};
    ./gradlew bundle;
-    ZIP_FILE="$(ls build/ | grep .zip)";
+    ZIP_FILE="$(ls build/ | grep SNAPSHOT.zip)";


I think this grep will fail if we happen to be on a commit in master with a tag (eg., we are on version 4.1.0.0 exactly). Can you just grep for a zip file that starts with "gatk" instead?

droazen · 2019-03-05T20:56:16Z

.travis.yml

-    curl -H "Content-Type:application/json" --data '{"build":true}' -X POST ${DOCKERHUB_URL};
+
+    sudo bash build_docker.sh  -e ${TRAVIS_COMMIT} -s -u;
+    DOCKER_TAG=$(docker images -q | head -n 1);


Is this guaranteed to be in the right order?

Yes. It only builds one docker image on this machine (it will have downloaded the base image but its in descending order by newest)

droazen · 2019-03-05T20:58:02Z

.travis.yml

+
+    sudo bash build_docker.sh  -e ${TRAVIS_COMMIT} -s -u;
+    DOCKER_TAG=$(docker images -q | head -n 1);
+    DATE=`date +%Y-%m-%d`;


Be consistent in your command substitution convention (backticks vs. $() )

droazen · 2019-03-05T20:58:59Z

.travis.yml

+    sudo bash build_docker.sh  -e ${TRAVIS_COMMIT} -s -u;
+    DOCKER_TAG=$(docker images -q | head -n 1);
+    DATE=`date +%Y-%m-%d`;
+    DOCKER_UPLOAD=broadinstitute/gatk-nightly:$DATE-$(git describe)-SNAPSHOT;


Perhaps this should end in -NIGHTLY-SNAPSHOT?

Also, rename DOCKER_UPLOAD to DOCKER_NIGHTLY_TAG

Okay, its inconsistent with the previous syntax but only a handful of people use these so its probably fine chaning things up

droazen · 2019-03-05T20:59:26Z

.travis.yml

+    DATE=`date +%Y-%m-%d`;
+    DOCKER_UPLOAD=broadinstitute/gatk-nightly:$DATE-$(git describe)-SNAPSHOT;
+
+    echo "Pushing Docker image";


Pushing nightly docker image

jamesemery · 2019-03-05T21:27:27Z

@droazen responded to your comments

lbergelson

Seems good to me.

lbergelson · 2019-03-05T22:19:15Z

README.md

@@ -103,7 +103,7 @@ releases of the toolkit.

 You can download and run pre-built versions of GATK4 from the following places:

-* A zip archive with everything you need to run GATK4 can be downloaded for each release from the [github releases page](https://github.com/broadinstitute/gatk/releases).
+* A zip archive with everything you need to run GATK4 can be downloaded for each release from the [github releases page](https://github.com/broadinstitute/gatk/releases). We also host unstable archives generated nightly in the Google bucket gs://gatk-nightly-builds.


Do nightly builds ever age away or is dockerhub hosting all of our builds for all eternity?

They just sit forever I believe. Its smart about compressing them/reusing layers but it doesn't seem like dockerhubs pricing scheme cares about the number of images hosted compared the the number of repositories but I could be wrong about that.

lbergelson · 2019-03-05T22:21:38Z

.travis.yml

    $GCLOUD_HOME/gcloud components -q update gsutil;
    gsutil ls -l gs://gatk-nightly-builds | grep gatk | sort -r -k 2 | grep -o '\S\+$' | tail -n +11 | xargs -I {} gsutil rm {};
    ./gradlew bundle;
-    ZIP_FILE="$(ls build/ | grep .zip)";
+    ZIP_FILE="$(ls build/ | grep '^gatk.*.zip$' | grep -iv python)";


One other option... we could make building /pushing the docker a gradle command. Probably best saved for another time though.

Thats not a bad idea. There are some nuances that wold be a bit of a pain to resolve

lbergelson · 2019-03-05T22:23:42Z

.travis.yml

+
+    echo "Pushing Nightly Docker image";
+    docker tag $DOCKER_TAG $DOCKER_NIGHTLY_TAG;
+    docker push $DOCKER_NIGHTLY_TAG;


Can we push these to our gcr test repo as well?

Having a discussion about this with @droazen I think we should do this in a separate PR.

jamesemery · 2019-03-06T21:35:00Z

@droazen do you have any final thoughts on this branch? I would like to get this in so I can reactivate the cron job tonight

droazen

👍 Merge when ready @jamesemery

…ng to dockerhub

Changed the travis cron job configuration to support uploading prebui…

0236eb2

…lt docker images rather than using the docker build machinery

jamesemery assigned droazen and lbergelson Mar 5, 2019

jamesemery requested review from droazen and lbergelson March 5, 2019 20:17

droazen suggested changes Mar 5, 2019

View reviewed changes

droazen assigned jamesemery and unassigned droazen and lbergelson Mar 5, 2019

responded to comments

b18d937

lbergelson approved these changes Mar 5, 2019

View reviewed changes

removing the sudo

e464b16

droazen approved these changes Mar 6, 2019

View reviewed changes

jamesemery added 4 commits March 7, 2019 09:38

trying something that should hopefully fail for the sake

f7da4eb

travis.yml not being parsable

7524e0b

i hate travis .yml

e123081

moved the task so it will properly fail if there is a problem uploadi…

ca1f92d

…ng to dockerhub

jamesemery merged commit bd61efb into master Mar 7, 2019

jamesemery deleted the je_restoreKnightlyImages branch March 7, 2019 20:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed failing gatk-nightly Docker builds. #5759

Fixed failing gatk-nightly Docker builds. #5759

jamesemery commented Mar 5, 2019

jamesemery commented Mar 5, 2019

codecov-io commented Mar 5, 2019 •

edited

Loading

droazen left a comment

droazen Mar 5, 2019

droazen Mar 5, 2019

jamesemery Mar 5, 2019

droazen Mar 5, 2019

jamesemery Mar 5, 2019

droazen Mar 5, 2019

droazen Mar 5, 2019

jamesemery Mar 5, 2019

droazen Mar 5, 2019

droazen Mar 5, 2019

jamesemery Mar 5, 2019

droazen Mar 5, 2019

droazen Mar 5, 2019

droazen Mar 5, 2019

jamesemery Mar 5, 2019

droazen Mar 5, 2019

jamesemery commented Mar 5, 2019

lbergelson left a comment

lbergelson Mar 5, 2019

jamesemery Mar 6, 2019

lbergelson Mar 5, 2019

jamesemery Mar 6, 2019

lbergelson Mar 5, 2019

jamesemery Mar 6, 2019

jamesemery commented Mar 6, 2019

droazen left a comment

		- if [[ $TRAVIS_BRANCH == master && $TRAVIS_EVENT_TYPE == cron && $UPLOAD == true ]]; then
		docker login --username=$DOCKER_SERVICE_LOGIN --password=$DOCKER_SERVICE_PASS;

Fixed failing gatk-nightly Docker builds. #5759

Fixed failing gatk-nightly Docker builds. #5759

Conversation

jamesemery commented Mar 5, 2019

jamesemery commented Mar 5, 2019

codecov-io commented Mar 5, 2019 • edited Loading

Codecov Report

droazen 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jamesemery commented Mar 5, 2019

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

jamesemery commented Mar 6, 2019

droazen left a comment

Choose a reason for hiding this comment

codecov-io commented Mar 5, 2019 •

edited

Loading