-
Notifications
You must be signed in to change notification settings - Fork 602
Fixed failing gatk-nightly Docker builds. #5759
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
Changes from 1 commit
0236eb2
b18d937
e464b16
f7da4eb
7524e0b
e123081
ca1f92d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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/ | ||
- if [[ $TRAVIS_BRANCH == master && $TRAVIS_EVENT_TYPE == cron && $UPLOAD == true ]]; then | ||
docker login --username=$DOCKER_SERVICE_LOGIN --password=$DOCKER_SERVICE_PASS; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are the values of these variables properly secured in travis? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
$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 {}; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we currently mention the |
||
./gradlew bundle; | ||
ZIP_FILE="$(ls build/ | grep .zip)"; | ||
ZIP_FILE="$(ls build/ | grep SNAPSHOT.zip)"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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? |
||
echo "Uploading zip to gs://gatk-nightly-builds/"; | ||
$GCLOUD_HOME/gsutil -m cp build/$ZIP_FILE gs://gatk-nightly-builds/"$(date +%Y-%m-%d)"-$ZIP_FILE; | ||
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; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this need to be run as root? If not, can you change this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, should you be specifying a staging dir via There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
DOCKER_TAG=$(docker images -q | head -n 1); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this guaranteed to be in the right order? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. It only builds one docker image on this machine (it will have downloaded the base image but its in descending order by newest) |
||
DATE=`date +%Y-%m-%d`; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Be consistent in your command substitution convention (backticks vs. $() ) |
||
DOCKER_UPLOAD=broadinstitute/gatk-nightly:$DATE-$(git describe)-SNAPSHOT; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps this should end in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, rename There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay, its inconsistent with the previous syntax but only a handful of people use these so its probably fine chaning things up |
||
|
||
echo "Pushing Docker image"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Pushing nightly docker image |
||
docker tag $DOCKER_TAG $DOCKER_UPLOAD; | ||
docker push $DOCKER_UPLOAD; | ||
fi; | ||
after_failure: | ||
- dmesg | tail -100 | ||
|
This file was deleted.
This file was deleted.
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we mention the nightly dockerhub repo in our README (if we don't already)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Its already mentioned. I added a line about the nightly zip folder in there.