From d8fe231da8beef4d979a12fe0027496534d4f6bf Mon Sep 17 00:00:00 2001 From: David Roazen Date: Fri, 8 Dec 2023 15:47:19 -0500 Subject: [PATCH 01/13] Update the GATK base image to a newer LTS ubuntu release --- Dockerfile | 6 ++-- scripts/docker/gatkbase/Dockerfile | 29 +++++++++++++++++-- .../gatkbase/stage_docker_base_in_cloud.sh | 20 +++++++++++++ 3 files changed, 49 insertions(+), 6 deletions(-) create mode 100755 scripts/docker/gatkbase/stage_docker_base_in_cloud.sh diff --git a/Dockerfile b/Dockerfile index a513863128d..253dd82c33d 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -ARG BASE_DOCKER=broadinstitute/gatk:gatkbase-3.1.0 +ARG BASE_DOCKER=us.gcr.io/broad-dsde-methods/gatk-base-image-staging-area:3.2.0rc3 # stage 1 for constructing the GATK zip FROM ${BASE_DOCKER} AS gradleBuild @@ -93,8 +93,8 @@ RUN conda env create -n gatk -f /gatk/gatkcondaenv.yml && \ echo "source activate gatk" >> /gatk/gatkenv.rc && \ echo "source /gatk/gatk-completion.sh" >> /gatk/gatkenv.rc && \ conda clean -afy && \ - find /opt/miniconda/ -follow -type f -name '*.a' -delete && \ - find /opt/miniconda/ -follow -type f -name '*.pyc' -delete && \ + find $CONDA_PATH -follow -type f -name '*.a' -delete && \ + find $CONDA_PATH -follow -type f -name '*.pyc' -delete && \ rm -rf /root/.cache/pip CMD ["bash", "--init-file", "/gatk/gatkenv.rc"] diff --git a/scripts/docker/gatkbase/Dockerfile b/scripts/docker/gatkbase/Dockerfile index 7f4d8cb6a7b..da062cdc173 100644 --- a/scripts/docker/gatkbase/Dockerfile +++ b/scripts/docker/gatkbase/Dockerfile @@ -1,12 +1,13 @@ # Using OpenJDK 17 # This Dockerfile does not require any files that are in the GATK4 repo. -FROM ubuntu:18.04 +FROM ubuntu:22.04 #### Basic image utilities RUN apt-get update && \ apt-get upgrade -y && \ apt-get install -y --no-install-recommends \ python3 \ + python3-pip \ wget \ curl \ bc \ @@ -15,10 +16,13 @@ RUN apt-get update && \ less \ bedtools \ samtools \ + bcftools \ tabix \ + git \ gpg-agent \ build-essential \ openjdk-17-jdk \ + vim \ software-properties-common && \ apt-get -y clean && \ apt-get -y autoclean && \ @@ -55,7 +59,7 @@ ENV JAVA_LIBRARY_PATH /usr/lib/jni # Install miniconda ENV DOWNLOAD_DIR /downloads -ENV CONDA_URL https://repo.anaconda.com/miniconda/Miniconda3-py310_23.1.0-1-Linux-x86_64.sh +ENV CONDA_URL https://repo.anaconda.com/miniconda/Miniconda3-py310_23.1.0-1-Linux-x86_64.sh ENV CONDA_MD5 = "32d73e1bc33fda089d7cd9ef4c1be542616bd8e437d1f77afeeaf7afdb019787" ENV CONDA_PATH /opt/miniconda ENV PATH $CONDA_PATH/bin:$PATH @@ -67,4 +71,23 @@ RUN mkdir $DOWNLOAD_DIR && \ conda clean -afy && \ find /opt/miniconda/ -follow -type f -name '*.a' -delete && \ find /opt/miniconda/ -follow -type f -name '*.pyc' -delete && \ - rm -rf /root/.cache/pip + rm -rf /root/.cache/pip + +# Install miniconda +# Following the instructions from https://docs.conda.io/projects/conda/en/latest/user-guide/install/rpm-debian.html +# Note that this *does* install just miniconda, despite the package being named "conda" +# RUN curl https://repo.anaconda.com/pkgs/misc/gpgkeys/anaconda.asc | gpg --dearmor > /tmp/conda.gpg && \ +# install -o root -g root -m 644 /tmp/conda.gpg /usr/share/keyrings/conda-archive-keyring.gpg && \ +# gpg --keyring /usr/share/keyrings/conda-archive-keyring.gpg --no-default-keyring --fingerprint 34161F5BF5EB1D4BFBBB8F0A8AEB4F8B29D82806 && \ +# rm /tmp/conda.gpg && \ +# echo "deb [arch=amd64 signed-by=/usr/share/keyrings/conda-archive-keyring.gpg] https://repo.anaconda.com/pkgs/misc/debrepo/conda stable main" > /etc/apt/sources.list.d/conda.list && \ +# apt update && \ +# apt install conda && \ +# apt -y clean && \ +# apt -y autoclean && \ +# apt -y autoremove && \ +# . /opt/conda/etc/profile.d/conda.sh && \ +# conda -V +# +# ENV CONDA_PATH /opt/conda +# ENV PATH $CONDA_PATH/bin:$PATH diff --git a/scripts/docker/gatkbase/stage_docker_base_in_cloud.sh b/scripts/docker/gatkbase/stage_docker_base_in_cloud.sh new file mode 100755 index 00000000000..5a2b6fefe40 --- /dev/null +++ b/scripts/docker/gatkbase/stage_docker_base_in_cloud.sh @@ -0,0 +1,20 @@ +#!/bin/bash + +if [ $# -ne 1 ]; then + echo "Usage: $0 docker_image_version" + exit 1 +fi + +IMAGE_VERSION=$1 +IMAGE_NAME="us.gcr.io/broad-dsde-methods/gatk-base-image-staging-area" +DOCKER_IMAGE_TAG="${IMAGE_NAME}:${IMAGE_VERSION}" + +gcloud builds submit --tag ${DOCKER_IMAGE_TAG} --timeout=24h --machine-type n1_highcpu_32 + +if [ $? -ne 0 ]; then + echo "gcloud builds submit failed" + exit 1 +fi + +echo "Successfully published image to staging area at ${DOCKER_IMAGE_TAG}" +exit 0 \ No newline at end of file From 50cfd0b26eccd1e598055f28e8de1a2933642a05 Mon Sep 17 00:00:00 2001 From: David Roazen Date: Sat, 9 Dec 2023 13:00:44 -0500 Subject: [PATCH 02/13] Incorporate base image changes from Sam Lee's PR, fix md5 check, other changes --- Dockerfile | 4 +-- scripts/docker/gatkbase/Dockerfile | 53 ++++++++++-------------------- 2 files changed, 18 insertions(+), 39 deletions(-) diff --git a/Dockerfile b/Dockerfile index 253dd82c33d..6b9fe9e10f1 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -ARG BASE_DOCKER=us.gcr.io/broad-dsde-methods/gatk-base-image-staging-area:3.2.0rc3 +ARG BASE_DOCKER=us.gcr.io/broad-dsde-methods/gatk-base-image-staging-area:3.2.0rc4 # stage 1 for constructing the GATK zip FROM ${BASE_DOCKER} AS gradleBuild @@ -93,8 +93,6 @@ RUN conda env create -n gatk -f /gatk/gatkcondaenv.yml && \ echo "source activate gatk" >> /gatk/gatkenv.rc && \ echo "source /gatk/gatk-completion.sh" >> /gatk/gatkenv.rc && \ conda clean -afy && \ - find $CONDA_PATH -follow -type f -name '*.a' -delete && \ - find $CONDA_PATH -follow -type f -name '*.pyc' -delete && \ rm -rf /root/.cache/pip CMD ["bash", "--init-file", "/gatk/gatkenv.rc"] diff --git a/scripts/docker/gatkbase/Dockerfile b/scripts/docker/gatkbase/Dockerfile index da062cdc173..88fd2a36837 100644 --- a/scripts/docker/gatkbase/Dockerfile +++ b/scripts/docker/gatkbase/Dockerfile @@ -3,11 +3,10 @@ FROM ubuntu:22.04 #### Basic image utilities -RUN apt-get update && \ - apt-get upgrade -y && \ - apt-get install -y --no-install-recommends \ +RUN apt update && \ + apt full-upgrade -y && \ + apt install -y --no-install-recommends \ python3 \ - python3-pip \ wget \ curl \ bc \ @@ -21,12 +20,13 @@ RUN apt-get update && \ git \ gpg-agent \ build-essential \ + libblas-dev \ openjdk-17-jdk \ vim \ software-properties-common && \ - apt-get -y clean && \ - apt-get -y autoclean && \ - apt-get -y autoremove && \ + apt -y clean && \ + apt -y autoclean && \ + apt -y autoremove && \ rm -rf /var/lib/apt/lists/* RUN java -version @@ -35,11 +35,11 @@ RUN java -version RUN echo "deb [signed-by=/usr/share/keyrings/cloud.google.gpg] http://packages.cloud.google.com/apt cloud-sdk main" \ | tee -a /etc/apt/sources.list.d/google-cloud-sdk.list && curl https://packages.cloud.google.com/apt/doc/apt-key.gpg \ | apt-key --keyring /usr/share/keyrings/cloud.google.gpg add - && \ - apt-get update -y && \ - apt-get install -y --no-install-recommends google-cloud-cli && \ - apt-get -y clean && \ - apt-get -y autoclean && \ - apt-get -y autoremove && \ + apt update -y && \ + apt install -y --no-install-recommends google-cloud-cli && \ + apt -y clean && \ + apt -y autoclean && \ + apt -y autoremove && \ rm -rf /var/lib/apt/lists/* && \ # Remove the anthos cli tool and related files since they are very large and we / anyone using the docker are unlikely to use them # Remove the bundled python because we have python installed separately @@ -59,35 +59,16 @@ ENV JAVA_LIBRARY_PATH /usr/lib/jni # Install miniconda ENV DOWNLOAD_DIR /downloads -ENV CONDA_URL https://repo.anaconda.com/miniconda/Miniconda3-py310_23.1.0-1-Linux-x86_64.sh -ENV CONDA_MD5 = "32d73e1bc33fda089d7cd9ef4c1be542616bd8e437d1f77afeeaf7afdb019787" +ENV CONDA_URL https://repo.anaconda.com/miniconda/Miniconda3-py310_23.10.0-1-Linux-x86_64.sh +ENV CONDA_SHA256 "c7a34df472feb69805b64df6e8db58363c5ccab41cd3b40b07e3e6dfb924359a" ENV CONDA_PATH /opt/miniconda ENV PATH $CONDA_PATH/bin:$PATH RUN mkdir $DOWNLOAD_DIR && \ wget -nv -O $DOWNLOAD_DIR/miniconda.sh $CONDA_URL && \ - test "`md5sum $DOWNLOAD_DIR/miniconda.sh | awk -v FS=' ' '{print $1}'` = $CONDA_MD5" && \ + test "$(sha256sum $DOWNLOAD_DIR/miniconda.sh | awk -v FS=' ' -v ORS='' '{print $1}')" = "$CONDA_SHA256" && \ bash $DOWNLOAD_DIR/miniconda.sh -p $CONDA_PATH -b && \ rm $DOWNLOAD_DIR/miniconda.sh && \ + conda config --set auto_update_conda false && \ + conda config --set solver libmamba && \ conda clean -afy && \ - find /opt/miniconda/ -follow -type f -name '*.a' -delete && \ - find /opt/miniconda/ -follow -type f -name '*.pyc' -delete && \ rm -rf /root/.cache/pip - -# Install miniconda -# Following the instructions from https://docs.conda.io/projects/conda/en/latest/user-guide/install/rpm-debian.html -# Note that this *does* install just miniconda, despite the package being named "conda" -# RUN curl https://repo.anaconda.com/pkgs/misc/gpgkeys/anaconda.asc | gpg --dearmor > /tmp/conda.gpg && \ -# install -o root -g root -m 644 /tmp/conda.gpg /usr/share/keyrings/conda-archive-keyring.gpg && \ -# gpg --keyring /usr/share/keyrings/conda-archive-keyring.gpg --no-default-keyring --fingerprint 34161F5BF5EB1D4BFBBB8F0A8AEB4F8B29D82806 && \ -# rm /tmp/conda.gpg && \ -# echo "deb [arch=amd64 signed-by=/usr/share/keyrings/conda-archive-keyring.gpg] https://repo.anaconda.com/pkgs/misc/debrepo/conda stable main" > /etc/apt/sources.list.d/conda.list && \ -# apt update && \ -# apt install conda && \ -# apt -y clean && \ -# apt -y autoclean && \ -# apt -y autoremove && \ -# . /opt/conda/etc/profile.d/conda.sh && \ -# conda -V -# -# ENV CONDA_PATH /opt/conda -# ENV PATH $CONDA_PATH/bin:$PATH From 4a4b5c6ab904dda55c259fd6b89d7dcb0ac525f0 Mon Sep 17 00:00:00 2001 From: David Roazen Date: Sat, 9 Dec 2023 15:32:16 -0500 Subject: [PATCH 03/13] pip --- scripts/docker/gatkbase/Dockerfile | 1 + scripts/gatkcondaenv.yml.template | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/scripts/docker/gatkbase/Dockerfile b/scripts/docker/gatkbase/Dockerfile index 88fd2a36837..b0c2940fa08 100644 --- a/scripts/docker/gatkbase/Dockerfile +++ b/scripts/docker/gatkbase/Dockerfile @@ -7,6 +7,7 @@ RUN apt update && \ apt full-upgrade -y && \ apt install -y --no-install-recommends \ python3 \ + python3-pip \ wget \ curl \ bc \ diff --git a/scripts/gatkcondaenv.yml.template b/scripts/gatkcondaenv.yml.template index fa115de3325..a9c4e5e1392 100644 --- a/scripts/gatkcondaenv.yml.template +++ b/scripts/gatkcondaenv.yml.template @@ -20,7 +20,7 @@ dependencies: # core python dependencies - conda-forge::python=3.6.10 # do not update -- pip=20.0.2 # specifying channel may cause a warning to be emitted by conda +- conda-forge::pip=20.0.2 # specifying channel may cause a warning to be emitted by conda - conda-forge::mkl=2019.5 # MKL typically provides dramatic performance increases for theano, tensorflow, and other key dependencies - conda-forge::mkl-service=2.3.0 - conda-forge::joblib=1.1.1 # must pin joblib - versions after 1.1.1 no longer support python 3.6 From 59762d5460624b1ff55c73ac308f62af7c94d5ef Mon Sep 17 00:00:00 2001 From: David Roazen Date: Sat, 9 Dec 2023 15:51:39 -0500 Subject: [PATCH 04/13] Back out update to newer conda, try 20.04 instead of 22.04 --- Dockerfile | 2 +- scripts/docker/gatkbase/Dockerfile | 12 ++++++------ scripts/gatkcondaenv.yml.template | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Dockerfile b/Dockerfile index 6b9fe9e10f1..6bc421c25f1 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -ARG BASE_DOCKER=us.gcr.io/broad-dsde-methods/gatk-base-image-staging-area:3.2.0rc4 +ARG BASE_DOCKER=us.gcr.io/broad-dsde-methods/gatk-base-image-staging-area:3.2.0rc5 # stage 1 for constructing the GATK zip FROM ${BASE_DOCKER} AS gradleBuild diff --git a/scripts/docker/gatkbase/Dockerfile b/scripts/docker/gatkbase/Dockerfile index b0c2940fa08..cd27c27c120 100644 --- a/scripts/docker/gatkbase/Dockerfile +++ b/scripts/docker/gatkbase/Dockerfile @@ -1,6 +1,9 @@ # Using OpenJDK 17 # This Dockerfile does not require any files that are in the GATK4 repo. -FROM ubuntu:22.04 +FROM ubuntu:20.04 + +# Avoid interactive prompts during apt installs/upgrades +ENV DEBIAN_FRONTEND noninteractive #### Basic image utilities RUN apt update && \ @@ -21,7 +24,6 @@ RUN apt update && \ git \ gpg-agent \ build-essential \ - libblas-dev \ openjdk-17-jdk \ vim \ software-properties-common && \ @@ -60,8 +62,8 @@ ENV JAVA_LIBRARY_PATH /usr/lib/jni # Install miniconda ENV DOWNLOAD_DIR /downloads -ENV CONDA_URL https://repo.anaconda.com/miniconda/Miniconda3-py310_23.10.0-1-Linux-x86_64.sh -ENV CONDA_SHA256 "c7a34df472feb69805b64df6e8db58363c5ccab41cd3b40b07e3e6dfb924359a" +ENV CONDA_URL https://repo.anaconda.com/miniconda/Miniconda3-py310_23.1.0-1-Linux-x86_64.sh +ENV CONDA_SHA256 "32d73e1bc33fda089d7cd9ef4c1be542616bd8e437d1f77afeeaf7afdb019787" ENV CONDA_PATH /opt/miniconda ENV PATH $CONDA_PATH/bin:$PATH RUN mkdir $DOWNLOAD_DIR && \ @@ -69,7 +71,5 @@ RUN mkdir $DOWNLOAD_DIR && \ test "$(sha256sum $DOWNLOAD_DIR/miniconda.sh | awk -v FS=' ' -v ORS='' '{print $1}')" = "$CONDA_SHA256" && \ bash $DOWNLOAD_DIR/miniconda.sh -p $CONDA_PATH -b && \ rm $DOWNLOAD_DIR/miniconda.sh && \ - conda config --set auto_update_conda false && \ - conda config --set solver libmamba && \ conda clean -afy && \ rm -rf /root/.cache/pip diff --git a/scripts/gatkcondaenv.yml.template b/scripts/gatkcondaenv.yml.template index a9c4e5e1392..fa115de3325 100644 --- a/scripts/gatkcondaenv.yml.template +++ b/scripts/gatkcondaenv.yml.template @@ -20,7 +20,7 @@ dependencies: # core python dependencies - conda-forge::python=3.6.10 # do not update -- conda-forge::pip=20.0.2 # specifying channel may cause a warning to be emitted by conda +- pip=20.0.2 # specifying channel may cause a warning to be emitted by conda - conda-forge::mkl=2019.5 # MKL typically provides dramatic performance increases for theano, tensorflow, and other key dependencies - conda-forge::mkl-service=2.3.0 - conda-forge::joblib=1.1.1 # must pin joblib - versions after 1.1.1 no longer support python 3.6 From 87d6706af21ad5954cca7a857bc65daf1da333aa Mon Sep 17 00:00:00 2001 From: David Roazen Date: Sat, 9 Dec 2023 17:03:24 -0500 Subject: [PATCH 05/13] Back to 22.04 --- Dockerfile | 2 +- scripts/docker/gatkbase/Dockerfile | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Dockerfile b/Dockerfile index 6bc421c25f1..4aa45aaff3a 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -ARG BASE_DOCKER=us.gcr.io/broad-dsde-methods/gatk-base-image-staging-area:3.2.0rc5 +ARG BASE_DOCKER=us.gcr.io/broad-dsde-methods/gatk-base-image-staging-area:3.2.0rc6 # stage 1 for constructing the GATK zip FROM ${BASE_DOCKER} AS gradleBuild diff --git a/scripts/docker/gatkbase/Dockerfile b/scripts/docker/gatkbase/Dockerfile index cd27c27c120..61c20a55a83 100644 --- a/scripts/docker/gatkbase/Dockerfile +++ b/scripts/docker/gatkbase/Dockerfile @@ -1,6 +1,6 @@ # Using OpenJDK 17 # This Dockerfile does not require any files that are in the GATK4 repo. -FROM ubuntu:20.04 +FROM ubuntu:22.04 # Avoid interactive prompts during apt installs/upgrades ENV DEBIAN_FRONTEND noninteractive From ccaea7cbfc5a78656a4bd6e7ca3df051484b649e Mon Sep 17 00:00:00 2001 From: David Roazen Date: Sun, 10 Dec 2023 16:26:49 -0500 Subject: [PATCH 06/13] Add an h5diff epsilon + reporting of differences to ScoreVariantAnnotationsIntegrationTest and TrainVariantAnnotationsModelIntegrationTest --- ...ScoreVariantAnnotationsIntegrationTest.java | 14 ++++++++++---- ...VariantAnnotationsModelIntegrationTest.java | 18 ++++++++++++++---- 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/ScoreVariantAnnotationsIntegrationTest.java b/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/ScoreVariantAnnotationsIntegrationTest.java index 1ff89982a9b..4b4b708990a 100644 --- a/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/ScoreVariantAnnotationsIntegrationTest.java +++ b/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/ScoreVariantAnnotationsIntegrationTest.java @@ -36,6 +36,12 @@ public final class ScoreVariantAnnotationsIntegrationTest extends CommandLinePro // diffs in the new expected outputs in git to confirm that they are consistent with expectations. public static final boolean UPDATE_EXACT_MATCH_EXPECTED_OUTPUTS = false; + public static String getH5DiffCommand() { + // -r: Report mode. Print the differences. + // --use-system-epsilon: Return a difference if and only if the difference between two data values exceeds + // the system value for epsilon. + return "h5diff -r --use-system-epsilon"; + } /** * Make sure that someone didn't leave the UPDATE_EXACT_MATCH_EXPECTED_OUTPUTS toggle turned on. */ @@ -165,10 +171,10 @@ private static void assertExpectedOutputs(final String tag, // vcf.idx files are not reproducible SystemCommandUtilsTest.runSystemCommand(String.format("diff %s/%s.vcf %s.vcf", EXPECTED_TEST_FILES_DIR, tag, outputPrefix)); - SystemCommandUtilsTest.runSystemCommand(String.format("h5diff %s/%s.annot.hdf5 %s.annot.hdf5", - EXPECTED_TEST_FILES_DIR, tag, outputPrefix)); - SystemCommandUtilsTest.runSystemCommand(String.format("h5diff %s/%s.scores.hdf5 %s.scores.hdf5", - EXPECTED_TEST_FILES_DIR, tag, outputPrefix)); + SystemCommandUtilsTest.runSystemCommand(String.format("%s %s/%s.annot.hdf5 %s.annot.hdf5", + getH5DiffCommand(), EXPECTED_TEST_FILES_DIR, tag, outputPrefix)); + SystemCommandUtilsTest.runSystemCommand(String.format("%s %s/%s.scores.hdf5 %s.scores.hdf5", + getH5DiffCommand(), EXPECTED_TEST_FILES_DIR, tag, outputPrefix)); } /** diff --git a/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/TrainVariantAnnotationsModelIntegrationTest.java b/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/TrainVariantAnnotationsModelIntegrationTest.java index 4f631c77bf2..4190e003dc1 100644 --- a/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/TrainVariantAnnotationsModelIntegrationTest.java +++ b/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/TrainVariantAnnotationsModelIntegrationTest.java @@ -38,6 +38,12 @@ public final class TrainVariantAnnotationsModelIntegrationTest extends CommandLi // diffs in the new expected outputs in git to confirm that they are consistent with expectations. public static final boolean UPDATE_EXACT_MATCH_EXPECTED_OUTPUTS = false; + public static String getH5DiffCommand() { + // -r: Report mode. Print the differences. + // --use-system-epsilon: Return a difference if and only if the difference between two data values exceeds + // the system value for epsilon. + return "h5diff -r --use-system-epsilon"; } + /** * Make sure that someone didn't leave the UPDATE_EXACT_MATCH_EXPECTED_OUTPUTS toggle turned on. */ @@ -179,11 +185,13 @@ private static void assertExpectedOutputsForVariantType(final String tag, final String tagAndVariantType = String.format("%s.%s", tag, variantType); final String outputPrefixAndVariantType = String.format("%s.%s", outputPrefix, variantType); - SystemCommandUtilsTest.runSystemCommand(String.format("h5diff %s/%s %s", + SystemCommandUtilsTest.runSystemCommand(String.format("%s %s/%s %s", + getH5DiffCommand(), EXPECTED_TEST_FILES_DIR, tagAndVariantType + TrainVariantAnnotationsModel.TRAINING_SCORES_HDF5_SUFFIX, outputPrefixAndVariantType + TrainVariantAnnotationsModel.TRAINING_SCORES_HDF5_SUFFIX)); - SystemCommandUtilsTest.runSystemCommand(String.format("h5diff %s/%s %s", + SystemCommandUtilsTest.runSystemCommand(String.format("%s %s/%s %s", + getH5DiffCommand(), EXPECTED_TEST_FILES_DIR, tagAndVariantType + TrainVariantAnnotationsModel.CALIBRATION_SCORES_HDF5_SUFFIX, outputPrefixAndVariantType + TrainVariantAnnotationsModel.CALIBRATION_SCORES_HDF5_SUFFIX)); @@ -252,10 +260,12 @@ public void testSNPOnlyModelsFromSNPOnlyAndSNPPlusIndelAnnotationsAreIdentical() .apply(argsBuilderSNPPlusIndel); runCommandLine(argsBuilderSNPPlusIndel); - SystemCommandUtilsTest.runSystemCommand(String.format("h5diff %s %s", + SystemCommandUtilsTest.runSystemCommand(String.format("%s %s %s", + getH5DiffCommand(), outputPrefixSNPOnly + ".snp" + TrainVariantAnnotationsModel.TRAINING_SCORES_HDF5_SUFFIX, outputPrefixSNPPlusIndel + ".snp" + TrainVariantAnnotationsModel.TRAINING_SCORES_HDF5_SUFFIX)); - SystemCommandUtilsTest.runSystemCommand(String.format("h5diff %s %s", + SystemCommandUtilsTest.runSystemCommand(String.format("%s %s %s", + getH5DiffCommand(), outputPrefixSNPOnly + ".snp" + TrainVariantAnnotationsModel.CALIBRATION_SCORES_HDF5_SUFFIX, outputPrefixSNPPlusIndel + ".snp" + TrainVariantAnnotationsModel.CALIBRATION_SCORES_HDF5_SUFFIX)); } From 2187cf2f704bc54390da1c1c228666901213b4dc Mon Sep 17 00:00:00 2001 From: David Roazen Date: Mon, 11 Dec 2023 03:16:27 -0500 Subject: [PATCH 07/13] Capture h5diff output in exception messages --- ...coreVariantAnnotationsIntegrationTest.java | 18 ++++++--- ...ariantAnnotationsModelIntegrationTest.java | 38 +++++++++---------- .../hellbender/testutils/BaseTest.java | 37 +++++++++++++++++- 3 files changed, 66 insertions(+), 27 deletions(-) diff --git a/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/ScoreVariantAnnotationsIntegrationTest.java b/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/ScoreVariantAnnotationsIntegrationTest.java index 4b4b708990a..4714d0fa009 100644 --- a/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/ScoreVariantAnnotationsIntegrationTest.java +++ b/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/ScoreVariantAnnotationsIntegrationTest.java @@ -12,6 +12,7 @@ import org.broadinstitute.hellbender.utils.io.IOUtils; import org.broadinstitute.hellbender.utils.io.Resource; import org.broadinstitute.hellbender.utils.python.PythonScriptExecutorException; +import org.broadinstitute.hellbender.utils.runtime.ProcessController; import org.testng.Assert; import org.testng.annotations.DataProvider; import org.testng.annotations.Test; @@ -36,12 +37,17 @@ public final class ScoreVariantAnnotationsIntegrationTest extends CommandLinePro // diffs in the new expected outputs in git to confirm that they are consistent with expectations. public static final boolean UPDATE_EXACT_MATCH_EXPECTED_OUTPUTS = false; - public static String getH5DiffCommand() { + public static void runH5Diff(final String expected, final String actual) { + final ProcessController controller = ProcessController.getThreadLocal(); + // -r: Report mode. Print the differences. // --use-system-epsilon: Return a difference if and only if the difference between two data values exceeds // the system value for epsilon. - return "h5diff -r --use-system-epsilon"; + final String[] command = new String[] { "h5diff", "-r", "--use-system-epsilon", expected, actual }; + + runProcessAndCaptureOutputInExceptionMessage(controller, command); } + /** * Make sure that someone didn't leave the UPDATE_EXACT_MATCH_EXPECTED_OUTPUTS toggle turned on. */ @@ -171,10 +177,10 @@ private static void assertExpectedOutputs(final String tag, // vcf.idx files are not reproducible SystemCommandUtilsTest.runSystemCommand(String.format("diff %s/%s.vcf %s.vcf", EXPECTED_TEST_FILES_DIR, tag, outputPrefix)); - SystemCommandUtilsTest.runSystemCommand(String.format("%s %s/%s.annot.hdf5 %s.annot.hdf5", - getH5DiffCommand(), EXPECTED_TEST_FILES_DIR, tag, outputPrefix)); - SystemCommandUtilsTest.runSystemCommand(String.format("%s %s/%s.scores.hdf5 %s.scores.hdf5", - getH5DiffCommand(), EXPECTED_TEST_FILES_DIR, tag, outputPrefix)); + runH5Diff(String.format("%s/%s.annot.hdf5", EXPECTED_TEST_FILES_DIR, tag), + String.format("%s.annot.hdf5", outputPrefix)); + runH5Diff(String.format("%s/%s.scores.hdf5", EXPECTED_TEST_FILES_DIR, tag), + String.format("%s.scores.hdf5", outputPrefix)); } /** diff --git a/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/TrainVariantAnnotationsModelIntegrationTest.java b/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/TrainVariantAnnotationsModelIntegrationTest.java index 4190e003dc1..b2755d19e86 100644 --- a/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/TrainVariantAnnotationsModelIntegrationTest.java +++ b/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/TrainVariantAnnotationsModelIntegrationTest.java @@ -13,6 +13,7 @@ import org.broadinstitute.hellbender.tools.walkers.vqsr.scalable.modeling.VariantAnnotationsModelBackend; import org.broadinstitute.hellbender.utils.io.IOUtils; import org.broadinstitute.hellbender.utils.io.Resource; +import org.broadinstitute.hellbender.utils.runtime.ProcessController; import org.testng.Assert; import org.testng.annotations.DataProvider; import org.testng.annotations.Test; @@ -38,11 +39,16 @@ public final class TrainVariantAnnotationsModelIntegrationTest extends CommandLi // diffs in the new expected outputs in git to confirm that they are consistent with expectations. public static final boolean UPDATE_EXACT_MATCH_EXPECTED_OUTPUTS = false; - public static String getH5DiffCommand() { + public static void runH5Diff(final String expected, final String actual) { + final ProcessController controller = ProcessController.getThreadLocal(); + // -r: Report mode. Print the differences. // --use-system-epsilon: Return a difference if and only if the difference between two data values exceeds // the system value for epsilon. - return "h5diff -r --use-system-epsilon"; } + final String[] command = new String[] { "h5diff", "-r", "--use-system-epsilon", expected, actual }; + + runProcessAndCaptureOutputInExceptionMessage(controller, command); + } /** * Make sure that someone didn't leave the UPDATE_EXACT_MATCH_EXPECTED_OUTPUTS toggle turned on. @@ -185,16 +191,11 @@ private static void assertExpectedOutputsForVariantType(final String tag, final String tagAndVariantType = String.format("%s.%s", tag, variantType); final String outputPrefixAndVariantType = String.format("%s.%s", outputPrefix, variantType); - SystemCommandUtilsTest.runSystemCommand(String.format("%s %s/%s %s", - getH5DiffCommand(), - EXPECTED_TEST_FILES_DIR, - tagAndVariantType + TrainVariantAnnotationsModel.TRAINING_SCORES_HDF5_SUFFIX, - outputPrefixAndVariantType + TrainVariantAnnotationsModel.TRAINING_SCORES_HDF5_SUFFIX)); - SystemCommandUtilsTest.runSystemCommand(String.format("%s %s/%s %s", - getH5DiffCommand(), - EXPECTED_TEST_FILES_DIR, - tagAndVariantType + TrainVariantAnnotationsModel.CALIBRATION_SCORES_HDF5_SUFFIX, - outputPrefixAndVariantType + TrainVariantAnnotationsModel.CALIBRATION_SCORES_HDF5_SUFFIX)); + runH5Diff(String.format("%s/%s", EXPECTED_TEST_FILES_DIR, tagAndVariantType + TrainVariantAnnotationsModel.TRAINING_SCORES_HDF5_SUFFIX), + String.format("%s", outputPrefixAndVariantType + TrainVariantAnnotationsModel.TRAINING_SCORES_HDF5_SUFFIX)); + + runH5Diff(String.format("%s/%s", EXPECTED_TEST_FILES_DIR, tagAndVariantType + TrainVariantAnnotationsModel.CALIBRATION_SCORES_HDF5_SUFFIX), + String.format("%s", outputPrefixAndVariantType + TrainVariantAnnotationsModel.CALIBRATION_SCORES_HDF5_SUFFIX)); assertScorerExpectedOutputs(tagAndVariantType, outputPrefixAndVariantType); @@ -260,14 +261,11 @@ public void testSNPOnlyModelsFromSNPOnlyAndSNPPlusIndelAnnotationsAreIdentical() .apply(argsBuilderSNPPlusIndel); runCommandLine(argsBuilderSNPPlusIndel); - SystemCommandUtilsTest.runSystemCommand(String.format("%s %s %s", - getH5DiffCommand(), - outputPrefixSNPOnly + ".snp" + TrainVariantAnnotationsModel.TRAINING_SCORES_HDF5_SUFFIX, - outputPrefixSNPPlusIndel + ".snp" + TrainVariantAnnotationsModel.TRAINING_SCORES_HDF5_SUFFIX)); - SystemCommandUtilsTest.runSystemCommand(String.format("%s %s %s", - getH5DiffCommand(), - outputPrefixSNPOnly + ".snp" + TrainVariantAnnotationsModel.CALIBRATION_SCORES_HDF5_SUFFIX, - outputPrefixSNPPlusIndel + ".snp" + TrainVariantAnnotationsModel.CALIBRATION_SCORES_HDF5_SUFFIX)); + runH5Diff(outputPrefixSNPOnly + ".snp" + TrainVariantAnnotationsModel.TRAINING_SCORES_HDF5_SUFFIX, + outputPrefixSNPPlusIndel + ".snp" + TrainVariantAnnotationsModel.TRAINING_SCORES_HDF5_SUFFIX); + + runH5Diff(outputPrefixSNPOnly + ".snp" + TrainVariantAnnotationsModel.CALIBRATION_SCORES_HDF5_SUFFIX, + outputPrefixSNPPlusIndel + ".snp" + TrainVariantAnnotationsModel.CALIBRATION_SCORES_HDF5_SUFFIX); } @Test(expectedExceptions = IllegalArgumentException.class) // python environment is required to run tool diff --git a/src/testUtils/java/org/broadinstitute/hellbender/testutils/BaseTest.java b/src/testUtils/java/org/broadinstitute/hellbender/testutils/BaseTest.java index 7eadf5aa2a1..58d75a35ae4 100644 --- a/src/testUtils/java/org/broadinstitute/hellbender/testutils/BaseTest.java +++ b/src/testUtils/java/org/broadinstitute/hellbender/testutils/BaseTest.java @@ -53,6 +53,17 @@ public static void runProcess(final ProcessController processController, final S runProcess(processController, command, "Process exited with non-zero value. Command: "+ Arrays.toString(command) + "\n"); } + /** + * run an external process and assert that it finishes with exit code 0 + * if it exits with non-zero, include the process stdout/stderr in the exception message + * + * @param processController a ProcessController to use + * @param command command to run, the 0th element must be the executable name + */ + public static void runProcessAndCaptureOutputInExceptionMessage(final ProcessController processController, final String[] command) { + runProcess(processController, command, null,"Process exited with non-zero value. Command: "+ Arrays.toString(command) + "\n", true); + } + /** * run an external process and assert that it finishes with exit code 0 * @param processController a ProcessController to use @@ -71,12 +82,36 @@ public static void runProcess(final ProcessController processController, final S * @param message error message to display on failure */ public static void runProcess(final ProcessController processController, final String[] command, final Map environment, final String message) { + runProcess(processController, command, environment, message, false); + } + + /** + * run an external process and assert that it finishes with exit code 0 + * @param processController a ProcessController to use + * @param command command to run, the 0th element must be the executable name + * @param environment what to use as the process environment variables + * @param message error message to display on failure + * @param includeProcessOutputInExceptionMessage if true, include the process's stdout/stderr (if any) in the exception + * message thrown upon exit with a non-zero value + */ + public static void runProcess(final ProcessController processController, final String[] command, final Map environment, final String message, final boolean includeProcessOutputInExceptionMessage) { final ProcessSettings prs = new ProcessSettings(command); prs.getStderrSettings().printStandard(true); prs.getStdoutSettings().printStandard(true); + + // Capture stdout/stderr to a buffer if we need to include it in the exception message + if ( includeProcessOutputInExceptionMessage ) { + prs.getStderrSettings().setBufferSize(65536); + prs.getStdoutSettings().setBufferSize(65536); + } + prs.setEnvironment(environment); final ProcessOutput output = processController.exec(prs); - Assert.assertEquals(output.getExitValue(), 0, message); + + final String fullMessage = includeProcessOutputInExceptionMessage ? + message + "\n***Process stdout:***\n" + output.getStdout() + "\n***Process stderr:***\n" + output.getStderr() : + message; + Assert.assertEquals(output.getExitValue(), 0, fullMessage); } /** From 5576927b3ab1177ee754d2a6a5f3bc520cc4e383 Mon Sep 17 00:00:00 2001 From: David Roazen Date: Mon, 11 Dec 2023 04:30:06 -0500 Subject: [PATCH 08/13] Remove epsilon temporarily --- .../vqsr/scalable/ScoreVariantAnnotationsIntegrationTest.java | 2 +- .../scalable/TrainVariantAnnotationsModelIntegrationTest.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/ScoreVariantAnnotationsIntegrationTest.java b/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/ScoreVariantAnnotationsIntegrationTest.java index 4714d0fa009..92e1b48a5e9 100644 --- a/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/ScoreVariantAnnotationsIntegrationTest.java +++ b/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/ScoreVariantAnnotationsIntegrationTest.java @@ -43,7 +43,7 @@ public static void runH5Diff(final String expected, final String actual) { // -r: Report mode. Print the differences. // --use-system-epsilon: Return a difference if and only if the difference between two data values exceeds // the system value for epsilon. - final String[] command = new String[] { "h5diff", "-r", "--use-system-epsilon", expected, actual }; + final String[] command = new String[] { "h5diff", "-r", expected, actual }; runProcessAndCaptureOutputInExceptionMessage(controller, command); } diff --git a/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/TrainVariantAnnotationsModelIntegrationTest.java b/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/TrainVariantAnnotationsModelIntegrationTest.java index b2755d19e86..69dbc182ee6 100644 --- a/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/TrainVariantAnnotationsModelIntegrationTest.java +++ b/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/TrainVariantAnnotationsModelIntegrationTest.java @@ -45,7 +45,7 @@ public static void runH5Diff(final String expected, final String actual) { // -r: Report mode. Print the differences. // --use-system-epsilon: Return a difference if and only if the difference between two data values exceeds // the system value for epsilon. - final String[] command = new String[] { "h5diff", "-r", "--use-system-epsilon", expected, actual }; + final String[] command = new String[] { "h5diff", "-r", expected, actual }; runProcessAndCaptureOutputInExceptionMessage(controller, command); } From 636d1391d58c85d12f3371982e98fcf1ca9532c3 Mon Sep 17 00:00:00 2001 From: David Roazen Date: Mon, 11 Dec 2023 05:41:56 -0500 Subject: [PATCH 09/13] restore epsilon, add debugging statement --- .../vqsr/scalable/ScoreVariantAnnotationsIntegrationTest.java | 2 +- .../scalable/TrainVariantAnnotationsModelIntegrationTest.java | 2 +- .../org/broadinstitute/hellbender/testutils/BaseTest.java | 4 ++++ 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/ScoreVariantAnnotationsIntegrationTest.java b/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/ScoreVariantAnnotationsIntegrationTest.java index 92e1b48a5e9..4714d0fa009 100644 --- a/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/ScoreVariantAnnotationsIntegrationTest.java +++ b/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/ScoreVariantAnnotationsIntegrationTest.java @@ -43,7 +43,7 @@ public static void runH5Diff(final String expected, final String actual) { // -r: Report mode. Print the differences. // --use-system-epsilon: Return a difference if and only if the difference between two data values exceeds // the system value for epsilon. - final String[] command = new String[] { "h5diff", "-r", expected, actual }; + final String[] command = new String[] { "h5diff", "-r", "--use-system-epsilon", expected, actual }; runProcessAndCaptureOutputInExceptionMessage(controller, command); } diff --git a/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/TrainVariantAnnotationsModelIntegrationTest.java b/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/TrainVariantAnnotationsModelIntegrationTest.java index 69dbc182ee6..b2755d19e86 100644 --- a/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/TrainVariantAnnotationsModelIntegrationTest.java +++ b/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/TrainVariantAnnotationsModelIntegrationTest.java @@ -45,7 +45,7 @@ public static void runH5Diff(final String expected, final String actual) { // -r: Report mode. Print the differences. // --use-system-epsilon: Return a difference if and only if the difference between two data values exceeds // the system value for epsilon. - final String[] command = new String[] { "h5diff", "-r", expected, actual }; + final String[] command = new String[] { "h5diff", "-r", "--use-system-epsilon", expected, actual }; runProcessAndCaptureOutputInExceptionMessage(controller, command); } diff --git a/src/testUtils/java/org/broadinstitute/hellbender/testutils/BaseTest.java b/src/testUtils/java/org/broadinstitute/hellbender/testutils/BaseTest.java index 58d75a35ae4..45c9c5768b2 100644 --- a/src/testUtils/java/org/broadinstitute/hellbender/testutils/BaseTest.java +++ b/src/testUtils/java/org/broadinstitute/hellbender/testutils/BaseTest.java @@ -111,6 +111,10 @@ public static void runProcess(final ProcessController processController, final S final String fullMessage = includeProcessOutputInExceptionMessage ? message + "\n***Process stdout:***\n" + output.getStdout() + "\n***Process stderr:***\n" + output.getStderr() : message; + + // Temporarily print the stdout/stderr unconditionally, for debugging + System.err.println(fullMessage); + Assert.assertEquals(output.getExitValue(), 0, fullMessage); } From 59346916b100e9b6266f1bd078a7841a3dd4d7b6 Mon Sep 17 00:00:00 2001 From: David Roazen Date: Mon, 11 Dec 2023 08:08:11 -0500 Subject: [PATCH 10/13] Remove debugging, remove obsolete runSystemCommand() method, final refactoring --- ...ractVariantAnnotationsIntegrationTest.java | 21 +++--- ...coreVariantAnnotationsIntegrationTest.java | 26 +++----- .../vqsr/scalable/SystemCommandUtilsTest.java | 64 ++++++++++--------- ...ariantAnnotationsModelIntegrationTest.java | 27 +++----- .../hellbender/testutils/BaseTest.java | 3 - 5 files changed, 65 insertions(+), 76 deletions(-) diff --git a/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/ExtractVariantAnnotationsIntegrationTest.java b/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/ExtractVariantAnnotationsIntegrationTest.java index e3379e91928..7d2e27d27bb 100644 --- a/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/ExtractVariantAnnotationsIntegrationTest.java +++ b/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/ExtractVariantAnnotationsIntegrationTest.java @@ -158,17 +158,18 @@ public void testValidInputs(final String tag, private static void assertOutputs(final String tag, final String outputPrefix) { // vcf.idx files are not reproducible - SystemCommandUtilsTest.runSystemCommand(String.format("h5diff %s/%s %s", - EXPECTED_TEST_FILES_DIR, - tag + LabeledVariantAnnotationsWalker.ANNOTATIONS_HDF5_SUFFIX, - outputPrefix + LabeledVariantAnnotationsWalker.ANNOTATIONS_HDF5_SUFFIX)); - SystemCommandUtilsTest.runSystemCommand(String.format("diff %s/%s.vcf %s.vcf", - EXPECTED_TEST_FILES_DIR, tag, outputPrefix)); + SystemCommandUtilsTest.runH5Diff( + String.format("%s/%s", EXPECTED_TEST_FILES_DIR, tag + LabeledVariantAnnotationsWalker.ANNOTATIONS_HDF5_SUFFIX), + String.format("%s", outputPrefix + LabeledVariantAnnotationsWalker.ANNOTATIONS_HDF5_SUFFIX)); + + SystemCommandUtilsTest.runDiff( + String.format("%s/%s.vcf", EXPECTED_TEST_FILES_DIR, tag), + String.format("%s.vcf", outputPrefix)); + if (tag.contains("posUn")) { - SystemCommandUtilsTest.runSystemCommand(String.format("h5diff %s/%s %s", - EXPECTED_TEST_FILES_DIR, - tag + ExtractVariantAnnotations.UNLABELED_TAG + ExtractVariantAnnotations.ANNOTATIONS_HDF5_SUFFIX, - outputPrefix + ExtractVariantAnnotations.UNLABELED_TAG + ExtractVariantAnnotations.ANNOTATIONS_HDF5_SUFFIX)); + SystemCommandUtilsTest.runH5Diff( + String.format("%s/%s", EXPECTED_TEST_FILES_DIR, tag + ExtractVariantAnnotations.UNLABELED_TAG + ExtractVariantAnnotations.ANNOTATIONS_HDF5_SUFFIX), + String.format("%s", outputPrefix + ExtractVariantAnnotations.UNLABELED_TAG + ExtractVariantAnnotations.ANNOTATIONS_HDF5_SUFFIX)); } else { Assert.assertFalse(new File(outputPrefix + ExtractVariantAnnotations.UNLABELED_TAG + ExtractVariantAnnotations.ANNOTATIONS_HDF5_SUFFIX).exists()); } diff --git a/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/ScoreVariantAnnotationsIntegrationTest.java b/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/ScoreVariantAnnotationsIntegrationTest.java index 4714d0fa009..ffb8f9158ae 100644 --- a/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/ScoreVariantAnnotationsIntegrationTest.java +++ b/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/ScoreVariantAnnotationsIntegrationTest.java @@ -37,17 +37,6 @@ public final class ScoreVariantAnnotationsIntegrationTest extends CommandLinePro // diffs in the new expected outputs in git to confirm that they are consistent with expectations. public static final boolean UPDATE_EXACT_MATCH_EXPECTED_OUTPUTS = false; - public static void runH5Diff(final String expected, final String actual) { - final ProcessController controller = ProcessController.getThreadLocal(); - - // -r: Report mode. Print the differences. - // --use-system-epsilon: Return a difference if and only if the difference between two data values exceeds - // the system value for epsilon. - final String[] command = new String[] { "h5diff", "-r", "--use-system-epsilon", expected, actual }; - - runProcessAndCaptureOutputInExceptionMessage(controller, command); - } - /** * Make sure that someone didn't leave the UPDATE_EXACT_MATCH_EXPECTED_OUTPUTS toggle turned on. */ @@ -175,11 +164,16 @@ public void testValidInputs(final String tag, private static void assertExpectedOutputs(final String tag, final String outputPrefix) { // vcf.idx files are not reproducible - SystemCommandUtilsTest.runSystemCommand(String.format("diff %s/%s.vcf %s.vcf", - EXPECTED_TEST_FILES_DIR, tag, outputPrefix)); - runH5Diff(String.format("%s/%s.annot.hdf5", EXPECTED_TEST_FILES_DIR, tag), - String.format("%s.annot.hdf5", outputPrefix)); - runH5Diff(String.format("%s/%s.scores.hdf5", EXPECTED_TEST_FILES_DIR, tag), + SystemCommandUtilsTest.runDiff( + String.format("%s/%s.vcf", EXPECTED_TEST_FILES_DIR, tag), + String.format("%s.vcf", outputPrefix)); + + SystemCommandUtilsTest.runH5Diff( + String.format("%s/%s.annot.hdf5", EXPECTED_TEST_FILES_DIR, tag), + String.format("%s.annot.hdf5", outputPrefix)); + + SystemCommandUtilsTest.runH5Diff( + String.format("%s/%s.scores.hdf5", EXPECTED_TEST_FILES_DIR, tag), String.format("%s.scores.hdf5", outputPrefix)); } diff --git a/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/SystemCommandUtilsTest.java b/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/SystemCommandUtilsTest.java index 705f292116a..aba550a9b15 100644 --- a/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/SystemCommandUtilsTest.java +++ b/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/SystemCommandUtilsTest.java @@ -2,6 +2,7 @@ import org.broadinstitute.hellbender.GATKBaseTest; import org.broadinstitute.hellbender.exceptions.GATKException; +import org.broadinstitute.hellbender.utils.runtime.ProcessController; import org.testng.Assert; import org.testng.annotations.Test; @@ -16,47 +17,50 @@ public final class SystemCommandUtilsTest extends GATKBaseTest { "org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/extract"); private static final File EXPECTED_TEST_FILES_DIR = new File(TEST_FILES_DIR, "expected"); - static void runSystemCommand(final String command) { - logger.debug(String.format("Testing command: %s", command)); - try { - final ProcessBuilder processBuilder = new ProcessBuilder("sh", "-c", command).redirectErrorStream(true); - final Process process = processBuilder.start(); - - final BufferedReader stdInReader = new BufferedReader(new InputStreamReader(process.getInputStream())); - String stdInLine; - while ((stdInLine = stdInReader.readLine()) != null) { - Assert.fail(String.format("The command \"%s\" resulted in: %s", command, stdInLine)); - } - stdInReader.close(); - - } catch (final IOException e) { - throw new GATKException.ShouldNeverReachHereException(e.getMessage()); - } + static void runH5Diff(final String expected, final String actual) { + final ProcessController controller = ProcessController.getThreadLocal(); + + // -r: Report mode. Print the differences. + // --use-system-epsilon: Return a difference if and only if the difference between two data values exceeds + // the system value for epsilon. + final String[] command = new String[] { "h5diff", "-r", "--use-system-epsilon", expected, actual }; + + runProcessAndCaptureOutputInExceptionMessage(controller, command); + } + + static void runDiff(final String expected, final String actual) { + final ProcessController controller = ProcessController.getThreadLocal(); + final String[] command = new String[] { "diff", expected, actual }; + runProcessAndCaptureOutputInExceptionMessage(controller, command); } @Test(groups = {"python"}) // python environment is required to use h5diff - public void testRunSystemCommand() { - runSystemCommand(String.format("h5diff %s/extract.AS.indel.pos.annot.hdf5 %s/extract.AS.indel.pos.annot.hdf5", - EXPECTED_TEST_FILES_DIR, EXPECTED_TEST_FILES_DIR)); - runSystemCommand(String.format("diff %s/extract.AS.indel.pos.vcf %s/extract.AS.indel.pos.vcf", - EXPECTED_TEST_FILES_DIR, EXPECTED_TEST_FILES_DIR)); + public void testRunH5DiffIdenticalFile() { + runH5Diff(String.format("%s/extract.AS.indel.pos.annot.hdf5", EXPECTED_TEST_FILES_DIR), + String.format("%s/extract.AS.indel.pos.annot.hdf5", EXPECTED_TEST_FILES_DIR)); + } + + @Test + public void testRunDiffIdenticalFile() { + runDiff(String.format("%s/extract.AS.indel.pos.vcf", EXPECTED_TEST_FILES_DIR), + String.format("%s/extract.AS.indel.pos.vcf", EXPECTED_TEST_FILES_DIR)); } @Test(expectedExceptions = AssertionError.class, groups = {"python"}) // python environment is required to use h5diff - public void testRunSystemCommandH5diffException() { - runSystemCommand(String.format("h5diff %s/extract.AS.indel.pos.annot.hdf5 %s/extract.AS.snp.pos.annot.hdf5", - EXPECTED_TEST_FILES_DIR, EXPECTED_TEST_FILES_DIR)); + public void testRunH5DiffException() { + runH5Diff(String.format("%s/extract.AS.indel.pos.annot.hdf5", EXPECTED_TEST_FILES_DIR), + String.format("%s/extract.AS.snp.pos.annot.hdf5", EXPECTED_TEST_FILES_DIR)); } @Test(expectedExceptions = AssertionError.class) - public void testRunSystemCommandDiffException() { - runSystemCommand(String.format("diff %s/extract.AS.indel.pos.vcf %s/extract.AS.snp.pos.vcf", - EXPECTED_TEST_FILES_DIR, EXPECTED_TEST_FILES_DIR)); + public void testRunDiffException() { + runDiff(String.format("%s/extract.AS.indel.pos.vcf", EXPECTED_TEST_FILES_DIR), + String.format("%s/extract.AS.snp.pos.vcf", EXPECTED_TEST_FILES_DIR)); } @Test(expectedExceptions = AssertionError.class) - public void testRunSystemCommandDiffNoSuchFileException() { - runSystemCommand(String.format("diff %s/blahblah %s/extract.AS.snp.pos.vcf", - EXPECTED_TEST_FILES_DIR, EXPECTED_TEST_FILES_DIR)); + public void testRunDiffNoSuchFile() { + runDiff(String.format("%s/blahblah", EXPECTED_TEST_FILES_DIR), + String.format("%s/extract.AS.snp.pos.vcf", EXPECTED_TEST_FILES_DIR)); } } diff --git a/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/TrainVariantAnnotationsModelIntegrationTest.java b/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/TrainVariantAnnotationsModelIntegrationTest.java index b2755d19e86..a9bc55c4f5a 100644 --- a/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/TrainVariantAnnotationsModelIntegrationTest.java +++ b/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/TrainVariantAnnotationsModelIntegrationTest.java @@ -39,17 +39,6 @@ public final class TrainVariantAnnotationsModelIntegrationTest extends CommandLi // diffs in the new expected outputs in git to confirm that they are consistent with expectations. public static final boolean UPDATE_EXACT_MATCH_EXPECTED_OUTPUTS = false; - public static void runH5Diff(final String expected, final String actual) { - final ProcessController controller = ProcessController.getThreadLocal(); - - // -r: Report mode. Print the differences. - // --use-system-epsilon: Return a difference if and only if the difference between two data values exceeds - // the system value for epsilon. - final String[] command = new String[] { "h5diff", "-r", "--use-system-epsilon", expected, actual }; - - runProcessAndCaptureOutputInExceptionMessage(controller, command); - } - /** * Make sure that someone didn't leave the UPDATE_EXACT_MATCH_EXPECTED_OUTPUTS toggle turned on. */ @@ -191,11 +180,13 @@ private static void assertExpectedOutputsForVariantType(final String tag, final String tagAndVariantType = String.format("%s.%s", tag, variantType); final String outputPrefixAndVariantType = String.format("%s.%s", outputPrefix, variantType); - runH5Diff(String.format("%s/%s", EXPECTED_TEST_FILES_DIR, tagAndVariantType + TrainVariantAnnotationsModel.TRAINING_SCORES_HDF5_SUFFIX), - String.format("%s", outputPrefixAndVariantType + TrainVariantAnnotationsModel.TRAINING_SCORES_HDF5_SUFFIX)); + SystemCommandUtilsTest.runH5Diff( + String.format("%s/%s", EXPECTED_TEST_FILES_DIR, tagAndVariantType + TrainVariantAnnotationsModel.TRAINING_SCORES_HDF5_SUFFIX), + String.format("%s", outputPrefixAndVariantType + TrainVariantAnnotationsModel.TRAINING_SCORES_HDF5_SUFFIX)); - runH5Diff(String.format("%s/%s", EXPECTED_TEST_FILES_DIR, tagAndVariantType + TrainVariantAnnotationsModel.CALIBRATION_SCORES_HDF5_SUFFIX), - String.format("%s", outputPrefixAndVariantType + TrainVariantAnnotationsModel.CALIBRATION_SCORES_HDF5_SUFFIX)); + SystemCommandUtilsTest.runH5Diff( + String.format("%s/%s", EXPECTED_TEST_FILES_DIR, tagAndVariantType + TrainVariantAnnotationsModel.CALIBRATION_SCORES_HDF5_SUFFIX), + String.format("%s", outputPrefixAndVariantType + TrainVariantAnnotationsModel.CALIBRATION_SCORES_HDF5_SUFFIX)); assertScorerExpectedOutputs(tagAndVariantType, outputPrefixAndVariantType); @@ -261,10 +252,12 @@ public void testSNPOnlyModelsFromSNPOnlyAndSNPPlusIndelAnnotationsAreIdentical() .apply(argsBuilderSNPPlusIndel); runCommandLine(argsBuilderSNPPlusIndel); - runH5Diff(outputPrefixSNPOnly + ".snp" + TrainVariantAnnotationsModel.TRAINING_SCORES_HDF5_SUFFIX, + SystemCommandUtilsTest.runH5Diff( + outputPrefixSNPOnly + ".snp" + TrainVariantAnnotationsModel.TRAINING_SCORES_HDF5_SUFFIX, outputPrefixSNPPlusIndel + ".snp" + TrainVariantAnnotationsModel.TRAINING_SCORES_HDF5_SUFFIX); - runH5Diff(outputPrefixSNPOnly + ".snp" + TrainVariantAnnotationsModel.CALIBRATION_SCORES_HDF5_SUFFIX, + SystemCommandUtilsTest.runH5Diff( + outputPrefixSNPOnly + ".snp" + TrainVariantAnnotationsModel.CALIBRATION_SCORES_HDF5_SUFFIX, outputPrefixSNPPlusIndel + ".snp" + TrainVariantAnnotationsModel.CALIBRATION_SCORES_HDF5_SUFFIX); } diff --git a/src/testUtils/java/org/broadinstitute/hellbender/testutils/BaseTest.java b/src/testUtils/java/org/broadinstitute/hellbender/testutils/BaseTest.java index 45c9c5768b2..f5fe3b486e7 100644 --- a/src/testUtils/java/org/broadinstitute/hellbender/testutils/BaseTest.java +++ b/src/testUtils/java/org/broadinstitute/hellbender/testutils/BaseTest.java @@ -112,9 +112,6 @@ public static void runProcess(final ProcessController processController, final S message + "\n***Process stdout:***\n" + output.getStdout() + "\n***Process stderr:***\n" + output.getStderr() : message; - // Temporarily print the stdout/stderr unconditionally, for debugging - System.err.println(fullMessage); - Assert.assertEquals(output.getExitValue(), 0, fullMessage); } From 82e8bfbca26a292b2047a61729cbbf06ecf78b00 Mon Sep 17 00:00:00 2001 From: David Roazen Date: Mon, 11 Dec 2023 14:22:19 -0500 Subject: [PATCH 11/13] Incorporate Sam L patch to upgrade conda + pip --- Dockerfile | 2 +- scripts/docker/gatkbase/Dockerfile | 6 ++++-- scripts/gatkcondaenv.yml.template | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/Dockerfile b/Dockerfile index 4aa45aaff3a..17e0c0abd4e 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -ARG BASE_DOCKER=us.gcr.io/broad-dsde-methods/gatk-base-image-staging-area:3.2.0rc6 +ARG BASE_DOCKER=us.gcr.io/broad-dsde-methods/gatk-base-image-staging-area:3.2.0rc7 # stage 1 for constructing the GATK zip FROM ${BASE_DOCKER} AS gradleBuild diff --git a/scripts/docker/gatkbase/Dockerfile b/scripts/docker/gatkbase/Dockerfile index 61c20a55a83..1fb8cad430e 100644 --- a/scripts/docker/gatkbase/Dockerfile +++ b/scripts/docker/gatkbase/Dockerfile @@ -62,8 +62,8 @@ ENV JAVA_LIBRARY_PATH /usr/lib/jni # Install miniconda ENV DOWNLOAD_DIR /downloads -ENV CONDA_URL https://repo.anaconda.com/miniconda/Miniconda3-py310_23.1.0-1-Linux-x86_64.sh -ENV CONDA_SHA256 "32d73e1bc33fda089d7cd9ef4c1be542616bd8e437d1f77afeeaf7afdb019787" +ENV CONDA_URL https://repo.anaconda.com/miniconda/Miniconda3-py310_23.10.0-1-Linux-x86_64.sh +ENV CONDA_SHA256 "c7a34df472feb69805b64df6e8db58363c5ccab41cd3b40b07e3e6dfb924359a" ENV CONDA_PATH /opt/miniconda ENV PATH $CONDA_PATH/bin:$PATH RUN mkdir $DOWNLOAD_DIR && \ @@ -72,4 +72,6 @@ RUN mkdir $DOWNLOAD_DIR && \ bash $DOWNLOAD_DIR/miniconda.sh -p $CONDA_PATH -b && \ rm $DOWNLOAD_DIR/miniconda.sh && \ conda clean -afy && \ + conda config --set auto_update_conda false && \ + conda config --set solver libmamba && \ rm -rf /root/.cache/pip diff --git a/scripts/gatkcondaenv.yml.template b/scripts/gatkcondaenv.yml.template index fa115de3325..a87b2acdda3 100644 --- a/scripts/gatkcondaenv.yml.template +++ b/scripts/gatkcondaenv.yml.template @@ -20,7 +20,7 @@ dependencies: # core python dependencies - conda-forge::python=3.6.10 # do not update -- pip=20.0.2 # specifying channel may cause a warning to be emitted by conda +- conda-forge::pip=21.3.1 - conda-forge::mkl=2019.5 # MKL typically provides dramatic performance increases for theano, tensorflow, and other key dependencies - conda-forge::mkl-service=2.3.0 - conda-forge::joblib=1.1.1 # must pin joblib - versions after 1.1.1 no longer support python 3.6 From 312141b22897c979c3fba817f6ec028d2cf82504 Mon Sep 17 00:00:00 2001 From: David Roazen Date: Mon, 11 Dec 2023 15:46:33 -0500 Subject: [PATCH 12/13] One more test with epsilon off to make sure reporting is still working --- .../tools/walkers/vqsr/scalable/SystemCommandUtilsTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/SystemCommandUtilsTest.java b/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/SystemCommandUtilsTest.java index aba550a9b15..3057ded1a0d 100644 --- a/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/SystemCommandUtilsTest.java +++ b/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/SystemCommandUtilsTest.java @@ -23,7 +23,7 @@ static void runH5Diff(final String expected, final String actual) { // -r: Report mode. Print the differences. // --use-system-epsilon: Return a difference if and only if the difference between two data values exceeds // the system value for epsilon. - final String[] command = new String[] { "h5diff", "-r", "--use-system-epsilon", expected, actual }; + final String[] command = new String[] { "h5diff", "-r", expected, actual }; runProcessAndCaptureOutputInExceptionMessage(controller, command); } From 92ce8ac093e4c9d25f4f19b52f00c55134ec5072 Mon Sep 17 00:00:00 2001 From: David Roazen Date: Mon, 11 Dec 2023 18:06:48 -0500 Subject: [PATCH 13/13] Restore epsilon, switch to release base image, update scripts and docs --- Dockerfile | 2 +- scripts/docker/README.md | 19 +++--- scripts/docker/gatkbase/Dockerfile | 1 + scripts/docker/gatkbase/README.md | 23 ++++--- scripts/docker/gatkbase/build_docker_base.sh | 53 ---------------- .../gatkbase/build_docker_base_cloud.sh | 31 ++++++++++ .../gatkbase/build_docker_base_locally.sh | 33 ++++++++++ .../gatkbase/release_prebuilt_base_image.sh | 60 +++++++++++++++++++ .../gatkbase/stage_docker_base_in_cloud.sh | 20 ------- .../vqsr/scalable/SystemCommandUtilsTest.java | 18 +++++- 10 files changed, 166 insertions(+), 94 deletions(-) delete mode 100755 scripts/docker/gatkbase/build_docker_base.sh create mode 100755 scripts/docker/gatkbase/build_docker_base_cloud.sh create mode 100755 scripts/docker/gatkbase/build_docker_base_locally.sh create mode 100755 scripts/docker/gatkbase/release_prebuilt_base_image.sh delete mode 100755 scripts/docker/gatkbase/stage_docker_base_in_cloud.sh diff --git a/Dockerfile b/Dockerfile index 17e0c0abd4e..22f319086ad 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -ARG BASE_DOCKER=us.gcr.io/broad-dsde-methods/gatk-base-image-staging-area:3.2.0rc7 +ARG BASE_DOCKER=broadinstitute/gatk:gatkbase-3.2.0 # stage 1 for constructing the GATK zip FROM ${BASE_DOCKER} AS gradleBuild diff --git a/scripts/docker/README.md b/scripts/docker/README.md index 01e4bdb0f65..c61b71424f0 100644 --- a/scripts/docker/README.md +++ b/scripts/docker/README.md @@ -21,7 +21,7 @@ This repo contains the scripts for creating and pushing two docker images: - gatkbase -- a basic docker image that we do not expect to change very often. The GATK4 docker image uses this one (``FROM``) - gatk4 -- the official docker image for GATK4. The instructions in this document pertain to this image, unless otherwise stated. -``scripts/docker/gatkbase/build_docker_base.sh`` is a script to create the gatkbase docker image. +``scripts/docker/gatkbase/build_docker_base_cloud.sh`` and ``scripts/docker/gatkbase/build_docker_base_locally.sh`` are scripts to create the gatkbase docker image. ``build_docker.sh`` is a script to create the full gatk4 docker image. ``build_docker_remote.sh`` is a script to create the full gatk4 docker image using google cloud build remotely. This is useful if you can't build the docker image locally (for example if you have an M1 Macbook) NOTE: this requires the user first specify their project with the command `gcloud config set project ` to a project that has access to google cloud build. @@ -148,19 +148,24 @@ exit This is a base image that does not require any files in the gatk repo (except the build script and Dockerfile, obviously). GATK docker images are dependent on this one. **IMPORTANT** -- The gatkbase build script should be run from the ``scripts/docker/gatkbase`` directory. -- If you want to create a new version, you must modify the ``build_docker_base.sh`` script directly. Any changes should be committed to the repo. +- The gatkbase build scripts should be run from the ``scripts/docker/gatkbase`` directory. -#### Create gatkbase docker image and push it to docker hub +#### Build the gatkbase docker image on your local machine: ```bash -build_docker_base.sh -p +build_docker_base_locally.sh ``` -#### Create gatkbase docker image and do not push it to docker hub +#### Build the gatkbase docker image remotely using Google Cloud Build: + +```bash +build_docker_base_cloud.sh +``` + +#### Release a pre-built gatkbase image to the official repositories, after testing it: ```bash -build_docker_base.sh -p +release_prebuilt_base_image.sh ``` diff --git a/scripts/docker/gatkbase/Dockerfile b/scripts/docker/gatkbase/Dockerfile index 1fb8cad430e..7b758d19829 100644 --- a/scripts/docker/gatkbase/Dockerfile +++ b/scripts/docker/gatkbase/Dockerfile @@ -21,6 +21,7 @@ RUN apt update && \ samtools \ bcftools \ tabix \ + jq \ git \ gpg-agent \ build-essential \ diff --git a/scripts/docker/gatkbase/README.md b/scripts/docker/gatkbase/README.md index f020175ae78..9d8b2e81a8f 100644 --- a/scripts/docker/gatkbase/README.md +++ b/scripts/docker/gatkbase/README.md @@ -1,16 +1,15 @@ # How to update the GATK base docker image: -1. choose a new version number for the base image and manually update the version in `scripts/docker/gatkbase/build_docker_base.sh` -2. build the gatkbase image using that script and upload it to the [gatk-dev docker repo](https://hub.docker.com/r/broadinstitute/gatk-dev/) or [gcr-gatk-snapshots](us.gcr.io/broad-dsde-methods/broad-gatk-snapshots) - * cd to scripts/docker/gatkbase - * run `./build_docker_base.sh` - * `docker tag broadinstitute/gatk-dev:your-version-rc1` or whatever the correct tag is for where you want it uploaded - * `docker push tagname` -3. update the Dockerfile in the main gatk to use the image you pushed -4. commit the changes to the two docker files and to a new pull request -5. wait for the tests to pass and show it to a reviewer -6. push the base image to the official [gatk repo](https://hub.docker.com/r/broadinstitute/gatk) with the right name -7. update the main docker to point to the official version you just released -8. wait for tests to pass in travis and merge +1. In a branch, make whatever updates you need to the base image Dockerfile in `scripts/docker/gatkbase` +2. Choose a new version number for the new base image +3. Build the GATK base image by running either `build_docker_base_cloud.sh` (to build in the cloud using Google Cloud Build) or `build_docker_base_locally.sh` (to build on your local machine) from within the `scripts/docker/gatkbase` directory in your GATK clone. + * Both scripts take the version of the new image as their only argument. Eg., `build_docker_base_cloud.sh 3.0.0rc1` will create the remote image `us.gcr.io/broad-dsde-methods/gatk-base-image-staging-area:gatkbase-3.0.0rc1` +4. If you built the image locally, you'll need to manually push it to the staging area at `us.gcr.io/broad-dsde-methods/gatk-base-image-staging-area` +5. Test the new image using the GATK branch with your changes to the base image Dockerfile, by modifying the FROM clause at the top of the main GATK Dockerfile to point to the base image you staged, and submit a PR for the branch to let the test suite run. +6. Once tests pass and everything looks good, push the new base image to the release repositories using the `release_prebuilt_base_image.sh` script + * The release script takes as arguments the prebuilt image you've tested, and a final version number for release. For example: `release_prebuilt_base_image.sh us.gcr.io/broad-dsde-methods/gatk-base-image-staging-area:gatkbase-3.0.0rc1 3.0.0` + * The release script looks for the image locally first, and if not found locally it pulls it from the remote repository before releasing. +7. Update the FROM clause in the main GATK Dockerfile in your GATK PR to point to the officially-released image, and merge it once tests pass. + diff --git a/scripts/docker/gatkbase/build_docker_base.sh b/scripts/docker/gatkbase/build_docker_base.sh deleted file mode 100755 index ca6440c6880..00000000000 --- a/scripts/docker/gatkbase/build_docker_base.sh +++ /dev/null @@ -1,53 +0,0 @@ -#!/usr/bin/env bash - -# Have script stop if there is an error -set -e - - -REPO=broadinstitute -PROJECT=gatk -VERSION=2.3.0 -FULL_PATH=${REPO}/${PROJECT}:gatkbase-${VERSION} - -################################################# -# Parsing arguments -################################################# -while getopts "ph" option; do - case "$option" in - h) IS_HELP=true ;; - p) IS_PUSH=true ;; - esac -done - -if [ -n "$IS_HELP" ]; then - printf "Build the GATK4 base image.\n \ -Usage: %s: [-p] [-h] \n \ -Optional arguments: \n \ --p \t push image to docker hub once complete. \n \n" $0 - exit 1 -fi - -# Output the parameters -echo -e "\n" -echo -e "docker hub repo, project, and tag: ${FULL_PATH}\n\n" -echo "Other options (Blank is false)" -echo "---------------" -echo "Push to dockerhub: ${IS_PUSH}" - -# Login to dockerhub -if [ -n "${IS_PUSH}" ]; then - echo "Please login to dockerhub" - docker login -fi - -# Build -echo "Building image to tag ${FULL_PATH}..." -docker build --squash -t ${FULL_PATH} . - -## Push -if [ -n "${IS_PUSH}" ]; then - docker push ${FULL_PATH} -else - echo "Not pushing to dockerhub" -fi - diff --git a/scripts/docker/gatkbase/build_docker_base_cloud.sh b/scripts/docker/gatkbase/build_docker_base_cloud.sh new file mode 100755 index 00000000000..897b4b2a753 --- /dev/null +++ b/scripts/docker/gatkbase/build_docker_base_cloud.sh @@ -0,0 +1,31 @@ +#!/bin/bash +# +# A script that builds the GATK base image using Google Cloud Build, and pushes it to +# a staging location at us.gcr.io/broad-dsde-methods/gatk-base-image-staging-area +# +# Usage: build_docker_base_cloud.sh +# +# After staging the image, you should test it with GATK before actually releasing it +# using the release_prebuilt_base_image.sh script. You can test it by modifying the +# main GATK Dockerfile FROM clause to point temporarily at the staged base image, and +# submitting that as a PR to trigger a test suite run. +# + +if [ $# -ne 1 ]; then + echo "Usage: $0 " + exit 1 +fi + +IMAGE_VERSION=$1 +IMAGE_NAME="us.gcr.io/broad-dsde-methods/gatk-base-image-staging-area" +DOCKER_IMAGE_TAG="${IMAGE_NAME}:gatkbase-${IMAGE_VERSION}" + +gcloud builds submit --tag ${DOCKER_IMAGE_TAG} --timeout=24h --machine-type n1_highcpu_32 + +if [ $? -ne 0 ]; then + echo "gcloud builds submit failed" + exit 1 +fi + +echo "Successfully published image to staging area at ${DOCKER_IMAGE_TAG}" +exit 0 \ No newline at end of file diff --git a/scripts/docker/gatkbase/build_docker_base_locally.sh b/scripts/docker/gatkbase/build_docker_base_locally.sh new file mode 100755 index 00000000000..bc6acce2011 --- /dev/null +++ b/scripts/docker/gatkbase/build_docker_base_locally.sh @@ -0,0 +1,33 @@ +#!/bin/bash +# +# A script that builds the GATK base image locally (but does not push it anywhere). +# +# Usage: build_docker_base_locally.sh +# +# After building the image, you should test it with GATK before actually releasing it +# using the release_prebuilt_base_image.sh script. You can test it by modifying the +# main GATK Dockerfile FROM clause to point temporarily at the candidate base image, and +# submitting that as a PR to trigger a test suite run. +# + +if [ $# -ne 1 ]; then + echo "Usage: $0 " + exit 1 +fi + +IMAGE_VERSION=$1 +IMAGE_REPO="us.gcr.io/broad-dsde-methods/gatk-base-image-staging-area" +IMAGE_FULL_TAG="${IMAGE_REPO}:gatkbase-${IMAGE_VERSION}" + +# Build +echo "Building image to tag ${IMAGE_FULL_TAG}..." +docker build --squash -t "${IMAGE_FULL_TAG}" . + +if [ $? -ne 0 ]; then + echo "docker build failed" + exit 1 +fi + +echo "docker build succeeded" +exit 0 + diff --git a/scripts/docker/gatkbase/release_prebuilt_base_image.sh b/scripts/docker/gatkbase/release_prebuilt_base_image.sh new file mode 100755 index 00000000000..1647be93810 --- /dev/null +++ b/scripts/docker/gatkbase/release_prebuilt_base_image.sh @@ -0,0 +1,60 @@ +#!/bin/bash +# +# A script that takes a prebuilt GATK base image, and pushes it to the GATK release repositories on +# dockerhub and GCR. Use the build_docker_base_cloud.sh or build_docker_base_locally.sh scripts to +# build the image before running this script. +# +# Usage: release_prebuilt_base_image.sh +# +# If the prebuilt image exists locally, this script will push the local version. Otherwise, +# it will pull the image from the remote repository before pushing it to the GATK release +# repositories. +# +# prebuilt_image: The pre-built image you want to release (make sure you've tested it!) +# May be either local or remote +# version_tag_for_release: The version of the GATK base image you're releasing (eg., 3.1.0) +# + +if [ $# -ne 2 ]; then + echo "Usage: $0 " + exit 1 +fi + +PREBUILT_IMAGE="$1" +VERSION="$2" +DOCKERHUB_REPO="broadinstitute/gatk" +GCR_REPO="us.gcr.io/broad-gatk/gatk" +BASE_IMAGE_FULL_TAG="gatkbase-${VERSION}" + +function fatal_error() { + echo "$1" 1>&2 + exit 1 +} + +function docker_push() { + echo "Pushing to ${1}" + docker push "${1}" + if [ $? -ne 0 ]; then + fatal_error "Failed to push to ${1}" + fi +} + +# Test if the prebuilt image exists locally, and pull it if it doesn't +docker image inspect "${PREBUILT_IMAGE}" > /dev/null 2>&1 +if [ $? -ne 0 ]; then + echo "Image ${PREBUILT_IMAGE} not found locally: attempting to pull it now" + docker pull "${PREBUILT_IMAGE}" + if [ $? -ne 0 ]; then + fatal_error "Failed to pull pre-built image ${PREBUILT_IMAGE}" + fi +else + echo "Image ${PREBUILT_IMAGE} found locally: pushing it to the release repositories" +fi + +docker tag "${PREBUILT_IMAGE}" "${DOCKERHUB_REPO}:${BASE_IMAGE_FULL_TAG}" +docker tag "${PREBUILT_IMAGE}" "${GCR_REPO}:${BASE_IMAGE_FULL_TAG}" + +docker_push "${DOCKERHUB_REPO}:${BASE_IMAGE_FULL_TAG}" +docker_push "${GCR_REPO}:${BASE_IMAGE_FULL_TAG}" + +exit 0 \ No newline at end of file diff --git a/scripts/docker/gatkbase/stage_docker_base_in_cloud.sh b/scripts/docker/gatkbase/stage_docker_base_in_cloud.sh deleted file mode 100755 index 5a2b6fefe40..00000000000 --- a/scripts/docker/gatkbase/stage_docker_base_in_cloud.sh +++ /dev/null @@ -1,20 +0,0 @@ -#!/bin/bash - -if [ $# -ne 1 ]; then - echo "Usage: $0 docker_image_version" - exit 1 -fi - -IMAGE_VERSION=$1 -IMAGE_NAME="us.gcr.io/broad-dsde-methods/gatk-base-image-staging-area" -DOCKER_IMAGE_TAG="${IMAGE_NAME}:${IMAGE_VERSION}" - -gcloud builds submit --tag ${DOCKER_IMAGE_TAG} --timeout=24h --machine-type n1_highcpu_32 - -if [ $? -ne 0 ]; then - echo "gcloud builds submit failed" - exit 1 -fi - -echo "Successfully published image to staging area at ${DOCKER_IMAGE_TAG}" -exit 0 \ No newline at end of file diff --git a/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/SystemCommandUtilsTest.java b/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/SystemCommandUtilsTest.java index 3057ded1a0d..f9a82a8f975 100644 --- a/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/SystemCommandUtilsTest.java +++ b/src/test/java/org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/SystemCommandUtilsTest.java @@ -17,17 +17,33 @@ public final class SystemCommandUtilsTest extends GATKBaseTest { "org/broadinstitute/hellbender/tools/walkers/vqsr/scalable/extract"); private static final File EXPECTED_TEST_FILES_DIR = new File(TEST_FILES_DIR, "expected"); + /** + * Run the h5diff utility to compare the specified files. If h5diff exits with a non-zero + * status code (indicating that there were differences), the differences will get printed + * in the exception message and show up in the TestNG logs on github. + * + * @param expected HDF5 file with the expected results + * @param actual HDF5 file with the actual results + */ static void runH5Diff(final String expected, final String actual) { final ProcessController controller = ProcessController.getThreadLocal(); // -r: Report mode. Print the differences. // --use-system-epsilon: Return a difference if and only if the difference between two data values exceeds // the system value for epsilon. - final String[] command = new String[] { "h5diff", "-r", expected, actual }; + final String[] command = new String[] { "h5diff", "-r", "--use-system-epsilon", expected, actual }; runProcessAndCaptureOutputInExceptionMessage(controller, command); } + /** + * Run the standard diff utility to compare the specified files. If diff exits with a non-zero + * status code (indicating that there were differences), the differences will get printed + * in the exception message and show up in the TestNG logs on github. + * + * @param expected file with the expected results + * @param actual file with the actual results + */ static void runDiff(final String expected, final String actual) { final ProcessController controller = ProcessController.getThreadLocal(); final String[] command = new String[] { "diff", expected, actual };