WX-1711 local Docker build fixes, streamlining (#2993)

* Allow building on branches that have not been pushed * Remove Docker Hub, clean up - Supply default the correct GCR registry as default - Drop additional `DOCKERTAG_SAFE_NAME` that was computed using a `sed -e` invocation that does not work on BSD - Per deprecation warning, replace `gcloud docker push` with `docker push` - Remove `rawls-tests` image as we abandoned it in April with the GCR move * Fix bash bug that always pushed the image Unbelievable * Remove tests from `build.sh` - Delete `install.sh` which is now unused - Update README reference to `clean_install.sh` * Dockerfile comment * Fix JAR explosion * `scalafmtAll` despite no Scala
broadinstitute · Aug 14, 2024 · 478ee29 · 478ee29
1 parent f7a7970
commit 478ee29
Show file tree

Hide file tree

Showing 6 changed files with 67 additions and 99 deletions.
diff --git a/Dockerfile b/Dockerfile
@@ -1,3 +1,9 @@
+# TODO: Replace build script with multi-stage Dockerfile that builds its own JAR
+# FROM sbtscala/scala-sbt:eclipse-temurin-jammy-17.0.10_7_1.10.0_2.13.14 as jar-builder
+# ...build JAR...
+# FROM us.gcr.io/broad-dsp-gcr-public/base/jre:17-debian
+# COPY --from=jar-builder ./rawls*.jar /rawls
+
 FROM us.gcr.io/broad-dsp-gcr-public/base/jre:17-debian
 
 # To run, must build the jar using ./docker/build.sh

diff --git a/core/src/main/scala/org/broadinstitute/dsde/rawls/methods/MethodConfigurationUtils.scala b/core/src/main/scala/org/broadinstitute/dsde/rawls/methods/MethodConfigurationUtils.scala
@@ -21,7 +21,7 @@ object MethodConfigurationUtils {
       case Success(None) =>
         throw new RawlsExceptionWithErrorReport(
           errorReport = ErrorReport(StatusCodes.NotFound,
-            s"Cannot get ${methodConfig.methodRepoMethod.methodUri} from method repo."
+                                    s"Cannot get ${methodConfig.methodRepoMethod.methodUri} from method repo."
           )
         )
       case Success(Some(wdl)) =>
@@ -34,8 +34,8 @@ object MethodConfigurationUtils {
       case Failure(throwable) =>
         throw new RawlsExceptionWithErrorReport(
           errorReport = ErrorReport(StatusCodes.BadGateway,
-            s"Unable to query the method repo.",
-            methodRepoDAO.toErrorReport(throwable)
+                                    s"Unable to query the method repo.",
+                                    methodRepoDAO.toErrorReport(throwable)
           )
         )
     }

diff --git a/core/src/test/scala/org/broadinstitute/dsde/rawls/methods/MethodConfigurationUtilsSpec.scala b/core/src/test/scala/org/broadinstitute/dsde/rawls/methods/MethodConfigurationUtilsSpec.scala
@@ -4,15 +4,27 @@ import akka.actor.ActorSystem
 import akka.http.scaladsl.model.StatusCodes
 import akka.http.scaladsl.model.headers.OAuth2BearerToken
 import com.typesafe.config.ConfigFactory
-import org.broadinstitute.dsde.rawls.{RawlsException, RawlsExceptionWithErrorReport, TestExecutionContext, jobexec}
+import org.broadinstitute.dsde.rawls.{jobexec, RawlsException, RawlsExceptionWithErrorReport, TestExecutionContext}
 import org.broadinstitute.dsde.rawls.config.{MethodRepoConfig, WDLParserConfig}
 import org.broadinstitute.dsde.rawls.dataaccess.{HttpMethodRepoDAO, MockCromwellSwaggerClient}
 import org.broadinstitute.dsde.rawls.dataaccess.slick.TestDriverComponent
 import org.broadinstitute.dsde.rawls.jobexec.MethodConfigResolver
 import org.broadinstitute.dsde.rawls.jobexec.MethodConfigResolver.GatherInputsResult
 import org.broadinstitute.dsde.rawls.jobexec.wdlparsing.CachingWDLParser
 import org.broadinstitute.dsde.rawls.mock.RemoteServicesMockServer
-import org.broadinstitute.dsde.rawls.model.{Agora, AgoraMethod, Dockstore, ErrorReport, ErrorReportSource, MethodConfiguration, RawlsRequestContext, RawlsUser, RawlsUserEmail, RawlsUserSubjectId, UserInfo}
+import org.broadinstitute.dsde.rawls.model.{
+  Agora,
+  AgoraMethod,
+  Dockstore,
+  ErrorReport,
+  ErrorReportSource,
+  MethodConfiguration,
+  RawlsRequestContext,
+  RawlsUser,
+  RawlsUserEmail,
+  RawlsUserSubjectId,
+  UserInfo
+}
 import org.mockito.ArgumentMatchers.any
 import org.mockito.Mockito.when
 import org.scalatest.flatspec.AnyFlatSpec
@@ -30,34 +42,43 @@ class MethodConfigurationUtilsSpec extends AnyFlatSpec with Matchers with TestDr
 
   val mockServer: RemoteServicesMockServer = RemoteServicesMockServer()
   val methodRepoDAO: HttpMethodRepoDAO = mock[HttpMethodRepoDAO]
-  val ctx: RawlsRequestContext = RawlsRequestContext(UserInfo(testData.userProjectOwner.userEmail, OAuth2BearerToken("foo"), 0, testData.userProjectOwner.userSubjectId))
+  val ctx: RawlsRequestContext = RawlsRequestContext(
+    UserInfo(testData.userProjectOwner.userEmail, OAuth2BearerToken("foo"), 0, testData.userProjectOwner.userSubjectId)
+  )
 
   val agoraMethodConf: MethodConfiguration = MethodConfiguration("dsde",
-    "no_input",
-    Some("Sample"),
-    None,
-    Map.empty,
-    Map.empty,
-    AgoraMethod("dsde", "no_input", 1))
+                                                                 "no_input",
+                                                                 Some("Sample"),
+                                                                 None,
+                                                                 Map.empty,
+                                                                 Map.empty,
+                                                                 AgoraMethod("dsde", "no_input", 1)
+  )
 
   behavior of "MethodConfigurationUtils"
 
   it should "return results when method when found" in {
     when(methodRepoDAO.getMethod(any(), any())).thenReturn(Future.successful(Option(meth1WDL)))
 
-    val future = MethodConfigurationUtils.gatherMethodConfigInputs(ctx, methodRepoDAO, agoraMethodConf, methodConfigResolver)
+    val future =
+      MethodConfigurationUtils.gatherMethodConfigInputs(ctx, methodRepoDAO, agoraMethodConf, methodConfigResolver)
     Await.ready(future, 30.seconds)
     val Success(result) = future.value.get
 
     // verify that it returns GatherInputsResult
-    result shouldBe a [GatherInputsResult]
+    result shouldBe a[GatherInputsResult]
     result.missingInputs shouldBe Set("meth1.method1.i1")
   }
 
   it should "return 404 if method is not found" in {
     when(methodRepoDAO.getMethod(any(), any())).thenReturn(Future.successful(None))
 
-    val exception = intercept[RawlsExceptionWithErrorReport](Await.result(MethodConfigurationUtils.gatherMethodConfigInputs(ctx, methodRepoDAO, agoraMethodConf, methodConfigResolver), 30.seconds))
+    val exception = intercept[RawlsExceptionWithErrorReport](
+      Await.result(
+        MethodConfigurationUtils.gatherMethodConfigInputs(ctx, methodRepoDAO, agoraMethodConf, methodConfigResolver),
+        30.seconds
+      )
+    )
 
     // assert that if method is not found it returns 404
     exception shouldBe a[RawlsExceptionWithErrorReport]
@@ -66,11 +87,17 @@ class MethodConfigurationUtilsSpec extends AnyFlatSpec with Matchers with TestDr
   }
 
   it should "return 502 when something unexpected happens" in {
-    when(methodRepoDAO.getMethod(any(), any())).thenReturn(Future.failed(new RawlsException("exception thrown for testing purposes")))
+    when(methodRepoDAO.getMethod(any(), any()))
+      .thenReturn(Future.failed(new RawlsException("exception thrown for testing purposes")))
     when(methodRepoDAO.errorReportSource).thenReturn(ErrorReportSource("agora"))
     when(methodRepoDAO.toErrorReport(any())).thenCallRealMethod()
 
-    val exception = intercept[RawlsExceptionWithErrorReport](Await.result(MethodConfigurationUtils.gatherMethodConfigInputs(ctx, methodRepoDAO, agoraMethodConf, methodConfigResolver), 30.seconds))
+    val exception = intercept[RawlsExceptionWithErrorReport](
+      Await.result(
+        MethodConfigurationUtils.gatherMethodConfigInputs(ctx, methodRepoDAO, agoraMethodConf, methodConfigResolver),
+        30.seconds
+      )
+    )
 
     // assert that when Future fails, a 502 is returned with wrapped exception
     exception shouldBe a[RawlsExceptionWithErrorReport]

diff --git a/docker/README.md b/docker/README.md
@@ -4,7 +4,7 @@
 * `run-mysql.sh`
   * which calls `sql_validate.sh`
 
-_See https://github.com/broadinstitute/rawls/blob/develop/.github/workflows/rawls-build-tag-publish-and-run-tests.yaml
+_See [`rawls-build-tag-publish-and-run-tests`](https://github.com/broadinstitute/rawls/blob/develop/.github/workflows/rawls-build-tag-publish-and-run-tests.yaml) and [`rawls-build`](https://github.com/broadinstitute/terra-github-workflows/blob/main/.github/workflows/rawls-build.yaml)
 for GitHub Actions_
 
 
@@ -15,8 +15,7 @@ for GitHub Actions_
 
 ### Available for building a Docker image locally:
 * `build.sh`
-  * which calls `install.sh`
+  * which calls `clean_install.sh`
 
 _See instructions for local development and building Docker images in
 https://github.com/broadinstitute/rawls/blob/develop/README.md_
-
diff --git a/docker/build.sh b/docker/build.sh
@@ -26,14 +26,8 @@ echo "rawls docker/build.sh starting ..."
 # Set default variables
 DOCKER_CMD=
 BRANCH=${BRANCH:-$(git rev-parse --abbrev-ref HEAD)}  # default to current branch
-REGEX_TO_REPLACE_ILLEGAL_CHARACTERS_WITH_DASHES="s/[^a-zA-Z0-9_.\-]/-/g"
-REGEX_TO_REMOVE_DASHES_AND_PERIODS_FROM_BEGINNING="s/^[.\-]*//g"
-DOCKERTAG_SAFE_NAME=$(echo $BRANCH | sed -e $REGEX_TO_REPLACE_ILLEGAL_CHARACTERS_WITH_DASHES -e $REGEX_TO_REMOVE_DASHES_AND_PERIODS_FROM_BEGINNING | cut -c 1-127)  # https://docs.docker.com/engine/reference/commandline/tag/#:~:text=A%20tag%20name%20must%20be,a%20maximum%20of%20128%20characters.
-DOCKERHUB_REGISTRY=${DOCKERHUB_REGISTRY:-broadinstitute/$PROJECT}
-DOCKERHUB_TESTS_REGISTRY=${DOCKERHUB_REGISTRY}-tests
-GCR_REGISTRY=""
+GCR_REGISTRY=${GCR_REGISTRY:-gcr.io/broad-dsp-gcr-public/rawls}
 ENV=${ENV:-""}
-SKIP_TESTS=${SKIP_TESTS:-""}
 SERVICE_ACCT_KEY_FILE=""
 
 MAKE_JAR=false
@@ -98,27 +92,19 @@ fi
 function make_jar()
 {
     echo "building jar..."
-    if [ "$SKIP_TESTS" != "skip-tests" ]; then
-        echo "starting mysql..."
-        bash ./docker/run-mysql.sh start
-    fi
 
     # make jar.  cache sbt dependencies. capture output and stop db before returning.
     DOCKER_RUN="docker run --rm"
-    if [ "$SKIP_TESTS" != "skip-tests" ]; then
-        DOCKER_RUN="$DOCKER_RUN --link mysql:mysql"
-    fi
-    DOCKER_RUN="$DOCKER_RUN -e SKIP_TESTS=$SKIP_TESTS -e DOCKER_TAG -e GIT_COMMIT -e BUILD_NUMBER -v $PWD:/working -v sbt-cache:/root/.sbt -v jar-cache:/root/.ivy2 -v coursier-cache:/root/.cache/coursier sbtscala/scala-sbt:eclipse-temurin-jammy-17.0.10_7_1.10.0_2.13.14 /working/docker/install.sh /working"
+
+    # TODO: DOCKER_TAG hack until JAR build migrates to Dockerfile. Tell SBT to name the JAR
+    # `rawls-assembly-local-SNAP.jar` instead of including the commit hash. Otherwise we get
+    # an explosion of JARs that all get copied to the image; and which one runs is undefined.
+    DOCKER_RUN="$DOCKER_RUN -e DOCKER_TAG=local -e GIT_COMMIT -e BUILD_NUMBER -v $PWD:/working -v sbt-cache:/root/.sbt -v jar-cache:/root/.ivy2 -v coursier-cache:/root/.cache/coursier sbtscala/scala-sbt:eclipse-temurin-jammy-17.0.10_7_1.10.0_2.13.14 /working/docker/clean_install.sh /working"
+
     JAR_CMD=$($DOCKER_RUN 1>&2)
     EXIT_CODE=$?
 
-    if [ "$SKIP_TESTS" != "skip-tests" ]; then
-        # stop mysql
-        echo "stopping mysql..."
-        bash ./docker/run-mysql.sh stop
-    fi
-
-    # if tests were a fail, fail script
+    # if jar is a fail, fail script
     if [ $EXIT_CODE != 0 ]; then
         exit $EXIT_CODE
     fi
@@ -136,35 +122,16 @@ function artifactory_push()
 function docker_cmd()
 {
     if [ $DOCKER_CMD = "build" ] || [ $DOCKER_CMD = "push" ]; then
-        GIT_SHA=$(git rev-parse origin/${BRANCH})
+        GIT_SHA=$(git rev-parse ${BRANCH})
         echo GIT_SHA=$GIT_SHA > env.properties
         HASH_TAG=${GIT_SHA:0:12}
 
-        echo "building ${DOCKERHUB_REGISTRY}:${HASH_TAG}..."
-        docker build --pull -t $DOCKERHUB_REGISTRY:${HASH_TAG} .
-
-        echo "building ${DOCKERHUB_TESTS_REGISTRY}:${HASH_TAG}..."
-        cd automation
-        docker build -f Dockerfile-tests -t $DOCKERHUB_TESTS_REGISTRY:${HASH_TAG} .
-        cd ..
-
-        if [ $DOCKER_CMD="push" ]; then
-            echo "pushing ${DOCKERHUB_REGISTRY}:${HASH_TAG}..."
-            docker push $DOCKERHUB_REGISTRY:${HASH_TAG}
-            docker tag $DOCKERHUB_REGISTRY:${HASH_TAG} $DOCKERHUB_REGISTRY:${DOCKERTAG_SAFE_NAME}
-            docker push $DOCKERHUB_REGISTRY:${DOCKERTAG_SAFE_NAME}
-
-            echo "pushing ${DOCKERHUB_TESTS_REGISTRY}:${HASH_TAG}..."
-            docker push $DOCKERHUB_TESTS_REGISTRY:${HASH_TAG}
-            docker tag $DOCKERHUB_TESTS_REGISTRY:${HASH_TAG} $DOCKERHUB_TESTS_REGISTRY:${DOCKERTAG_SAFE_NAME}
-            docker push $DOCKERHUB_TESTS_REGISTRY:${DOCKERTAG_SAFE_NAME}
-
-            if [[ -n $GCR_REGISTRY ]]; then
-                echo "pushing $GCR_REGISTRY:${HASH_TAG}..."
-                docker tag $DOCKERHUB_REGISTRY:${HASH_TAG} $GCR_REGISTRY:${HASH_TAG}
-                gcloud docker -- push $GCR_REGISTRY:${HASH_TAG}
-                gcloud container images add-tag $GCR_REGISTRY:${HASH_TAG} $GCR_REGISTRY:${DOCKERTAG_SAFE_NAME}
-            fi
+        echo "building ${GCR_REGISTRY}:${HASH_TAG}..."
+        docker build --pull -t ${GCR_REGISTRY}:${HASH_TAG} .
+
+        if [ $DOCKER_CMD = "push" ]; then
+            echo "pushing ${GCR_REGISTRY}:${HASH_TAG}..."
+            docker push ${GCR_REGISTRY}:${HASH_TAG}
         fi
     else
         echo "Not a valid docker option!  Choose either build or push (which includes build)"

diff --git a/docker/install.sh b/docker/install.sh