Skip to content

update to jdk 11 [AS-594] #1372

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

Merged
merged 12 commits into from
Mar 11, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,10 @@ jobs:
- name: coursier-cache-action
uses: coursier/cache-action@v5

- name: Set up JDK 1.8
- name: Set up JDK 11
uses: actions/setup-java@v1
with:
java-version: 1.8
java-version: 11

- name: Git secrets setup
run: |
Expand Down
2 changes: 1 addition & 1 deletion .java-version
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1.8
11
2 changes: 1 addition & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
FROM us.gcr.io/broad-dsp-gcr-public/base/jre:8-debian
FROM us.gcr.io/broad-dsp-gcr-public/base/jre:11-debian

# To run, must build the jar using ./docker/build.sh

Expand Down
2 changes: 1 addition & 1 deletion automation/Dockerfile-tests
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
FROM bigtruedata/sbt
FROM broadinstitute/scala-baseimage:jdk11-2.12.12-1.4.7
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming container-based compilation is absolutely required, someday I hope this custom image gets pushed to a public GCR or anywhere else besides docker.io. We have so many tests in the terra multi-repo that are failing due to pull rate limits from docker.io these days. 😢

Or for easier patching of individual components, just use a more flexible base container like focal and then download java and the 0.016GB sbt package. While Java gets patches much slower, Scala and SBT get patches once a month. For example: sbt 1.4.8 is already out.

Copy link
Contributor Author

@zarsky-broad zarsky-broad Mar 9, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So:

  1. Eventually, there's going to be Broad GCR coordinates for this image, @choover-broad is working on something for that...
  2. This is basically just git (which I debated about) and that little sbt package on top of the base adoptopenjdk image, all the existing images were running mystery meat openjdk's, the idea was to get us on the one we're officially using.
  3. SBT will automatically download the version of itself that you specify, so it matters a lot less what version of sbt (and scala for that matter, although it's bigger) is baked into the image.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my take is that even though sbt will automatically download whatever it needs, the entire point of adding sbt to a baseimage is to preload as many dependencies as possible at docker-build time, and avoid re-downloading those dependencies on every build of every service that uses this image. So, counting on sbt to download what it needs defeats the purpose - if we're relying on that, I could see omitting it entirely and downloading it at runtime, avoiding the maintenance of the baseimage.


COPY src /app/src
COPY minnie-kenny.sh /app
Expand Down
3 changes: 1 addition & 2 deletions automation/project/Settings.scala
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ object Settings {
//coreDefaultSettings + defaultConfigs = the now deprecated defaultSettings
val commonBuildSettings = Defaults.coreDefaultSettings ++ Defaults.defaultConfigs ++ Seq(
javaOptions += "-Xmx2G",
javacOptions ++= Seq("-source", "1.8", "-target", "1.8"),
javacOptions ++= Seq("--release", "11"),
addCompilerPlugin(scalafixSemanticdb)
)

Expand All @@ -24,7 +24,6 @@ object Settings {
"-deprecation",
"-feature",
"-encoding", "utf8",
"-target:jvm-1.8",
"-Xmax-classfile-name", "100"
)

Expand Down
2 changes: 1 addition & 1 deletion automation/project/build.properties
Original file line number Diff line number Diff line change
@@ -1 +1 @@
sbt.version = 1.4.6
sbt.version = 1.4.7
1 change: 1 addition & 0 deletions build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ lazy val rawls = project.in(file("."))
// This appears to do some magic to configure itself. It consistently fails in some environments
// unless it is loaded after the settings definitions above.
Revolver.settings
Global / excludeLintKeys += debugSettings // To avoid lint warning

mainClass in reStart := Some("org.broadinstitute.dsde.rawls.Boot")

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package org.broadinstitute.dsde.rawls
import akka.http.scaladsl.model.{StatusCode, StatusCodes}
import org.broadinstitute.dsde.rawls.dataaccess.slick.TestDriverComponent
import org.broadinstitute.dsde.rawls.model.Workspace
import org.mockserver.model.StringBody
import org.mockserver.model.RegexBody
import org.scalatest.exceptions.TestFailedException
import org.scalatest.Suite

Expand Down Expand Up @@ -71,8 +71,8 @@ trait RawlsTestUtils extends Suite with TestDriverComponent with Matchers {
}

// MockServer's .withBody doesn't have a built-in string contains feature. This serves that purpose.
def mockServerContains(text: String): StringBody = {
val anythingWithNewlines = "((.|\n|\r)*)"
StringBody.regex(anythingWithNewlines + Regex.quote(text.toString) + anythingWithNewlines)
def mockServerContains(text: String): RegexBody = {
// "(?s)" turns on DOTALL mode, where a "." matches a line break as well as any character
new RegexBody("(?s).*" + Regex.quote(text) + ".*")
}
}
4 changes: 2 additions & 2 deletions docker/build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ function make_jar()
if [ "$SKIP_TESTS" != "skip-tests" ]; then
DOCKER_RUN="$DOCKER_RUN --link mysql:mysql"
fi
DOCKER_RUN="$DOCKER_RUN -e SKIP_TESTS=$SKIP_TESTS -e GIT_MODEL_HASH=$GIT_MODEL_HASH -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 broadinstitute/scala-baseimage /working/docker/install.sh /working"
DOCKER_RUN="$DOCKER_RUN -e SKIP_TESTS=$SKIP_TESTS -e GIT_MODEL_HASH=$GIT_MODEL_HASH -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 broadinstitute/scala-baseimage:jdk11-2.12.11-1.4.7 /working/docker/install.sh /working"
JAR_CMD=$($DOCKER_RUN 1>&2)
EXIT_CODE=$?

Expand All @@ -130,7 +130,7 @@ function artifactory_push()
ARTIFACTORY_USERNAME=dsdejenkins
ARTIFACTORY_PASSWORD=$(docker run -e VAULT_TOKEN=$VAULT_TOKEN broadinstitute/dsde-toolbox vault read -field=password secret/dsp/accts/artifactory/dsdejenkins)
echo "Publishing to artifactory..."
docker run --rm -v $PWD:/$PROJECT -v sbt-cache:/root/.sbt -v jar-cache:/root/.ivy2 -v coursier-cache:/root/.cache/coursier -w="/$PROJECT" -e ARTIFACTORY_USERNAME=$ARTIFACTORY_USERNAME -e ARTIFACTORY_PASSWORD=$ARTIFACTORY_PASSWORD broadinstitute/scala-baseimage:scala-2.11.8 /$PROJECT/core/src/bin/publishSnapshot.sh
docker run --rm -v $PWD:/$PROJECT -v sbt-cache:/root/.sbt -v jar-cache:/root/.ivy2 -v coursier-cache:/root/.cache/coursier -w="/$PROJECT" -e ARTIFACTORY_USERNAME=$ARTIFACTORY_USERNAME -e ARTIFACTORY_PASSWORD=$ARTIFACTORY_PASSWORD broadinstitute/scala-baseimage:jdk11-2.12.11-1.4.7 /$PROJECT/core/src/bin/publishSnapshot.sh
}

function docker_cmd()
Expand Down
2 changes: 1 addition & 1 deletion docker/install.sh
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ if [ "$SKIP_TESTS" = "skip-tests" ]; then
echo skipping tests
else
echo "starting sbt test ..."
sbt -J-Xms5g -J-Xmx5g -J-XX:MaxMetaspaceSize=5g test -Dmysql.host=mysql -Dmysql.port=3306
sbt -J-Xms5g -J-Xmx5g -J-XX:MaxMetaspaceSize=5g test -Dmysql.host=mysql -Dmysql.port=3306 -Dsecrets.skip=true
echo "sbt test done"
fi

Expand Down
8 changes: 5 additions & 3 deletions project/Dependencies.scala
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ object Dependencies {
val scalaUri: ModuleID = "io.lemonlabs" %% "scala-uri" % "3.0.0"
val scalatest: ModuleID = "org.scalatest" %% "scalatest" % "3.2.2" % "test"
val mockito: ModuleID = "org.scalatestplus" %% "mockito-3-4" % "3.2.2.0" % Test
val mockserverNetty: ModuleID = "org.mock-server" % "mockserver-netty" % "3.9.2" % "test"
val mockserverNetty: ModuleID = "org.mock-server" % "mockserver-netty" % "5.11.2" % "test"
val ficus: ModuleID = "com.iheart" %% "ficus" % "1.4.0"
val scalaCache: ModuleID = "com.github.cb372" %% "scalacache-caffeine" % "0.24.2"
val apacheCommonsIO: ModuleID = "commons-io" % "commons-io" % "2.6"
Expand All @@ -108,8 +108,10 @@ object Dependencies {

val accessContextManager = "com.google.apis" % "google-api-services-accesscontextmanager" % "v1beta-rev55-1.25.0"

val workspaceManager = excludeGuavaJDK5("bio.terra" % "workspace-manager-client" % "0.13.0-SNAPSHOT")
val dataRepo = excludeGuavaJDK5("bio.terra" % "datarepo-client" % "1.0.44-SNAPSHOT")
def excludeJakartaActivationApi(m: ModuleID): ModuleID = m.exclude("jakarta.activation", "jakarta.activation-api")

val workspaceManager = excludeJakartaActivationApi("bio.terra" % "workspace-manager-client" % "0.13.0-SNAPSHOT")
val dataRepo = excludeJakartaActivationApi("bio.terra" % "datarepo-client" % "1.0.44-SNAPSHOT")

val opencensusScalaCode: ModuleID = "com.github.sebruck" %% "opencensus-scala-core" % "0.7.0-M2"
val opencensusAkkaHttp: ModuleID = "com.github.sebruck" %% "opencensus-scala-akka-http" % "0.7.0-M2"
Expand Down
15 changes: 10 additions & 5 deletions project/Settings.scala
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ object Settings {
javaOptions += "-Xmx2G"
)

val java11BuildSettings = Seq( // can be wrapped into commonBuildSettings when rawls-model can publish java 11
javacOptions ++= Seq("--release", "11")
)

val commonCompilerSettings = Seq(
"-unchecked",
"-feature",
Expand Down Expand Up @@ -78,13 +82,14 @@ object Settings {
val googleSettings = commonSettings ++ List(
name := "workbench-google",
libraryDependencies ++= googleDependencies
) ++ versionSettings ++ noPublishSettings
) ++ versionSettings ++ noPublishSettings ++ java11BuildSettings

//the full list of settings for the rawlsModel project (see build.sbt)
//coreDefaultSettings (inside commonSettings) sets the project name, which we want to override, so ordering is important.
//thus commonSettings needs to be added first.
val modelSettings = cross212and213 ++ commonSettings ++ List(
name := "rawls-model",
javacOptions ++= Seq("--release", "8"), // has to publish a java 8 artifact
libraryDependencies ++= modelDependencies
) ++ versionSettings ++ publishSettings

Expand All @@ -94,15 +99,15 @@ object Settings {
val utilSettings = commonSettings ++ List(
name := "workbench-util",
libraryDependencies ++= utilDependencies
) ++ versionSettings ++ noPublishSettings
) ++ versionSettings ++ noPublishSettings ++ java11BuildSettings

//the full list of settings for the workbenchMetrics project (see build.sbt)
//coreDefaultSettings (inside commonSettings) sets the project name, which we want to override, so ordering is important.
//thus commonSettings needs to be added first.
val metricsSettings = commonSettings ++ List(
name := "workbench-metrics",
libraryDependencies ++= metricsDependencies
) ++ versionSettings ++ noPublishSettings
) ++ versionSettings ++ noPublishSettings ++ java11BuildSettings

//the full list of settings for the rawlsCore project (see build.sbt)
//coreDefaultSettings (inside commonSettings) sets the project name, which we want to override, so ordering is important.
Expand All @@ -111,7 +116,7 @@ object Settings {
name := "rawls-core",
version := "0.1",
libraryDependencies ++= rawlsCoreDependencies
) ++ antlr4CodeGenerationSettings ++ rawlsAssemblySettings ++ noPublishSettings ++ rawlsCompileSettings
) ++ antlr4CodeGenerationSettings ++ rawlsAssemblySettings ++ noPublishSettings ++ rawlsCompileSettings ++ java11BuildSettings
//NOTE: rawlsCoreCompileSettings above has to be last, because something in commonSettings or rawlsAssemblySettings
//overwrites it if it's before them. I (hussein) don't know what that is and I don't care to poke the bear to find out.

Expand All @@ -121,6 +126,6 @@ object Settings {
val rootSettings = commonSettings ++ List(
name := "rawls",
version := "0.1"
) ++ rawlsAssemblySettings ++ noPublishSettings ++ rawlsCompileSettings
) ++ rawlsAssemblySettings ++ noPublishSettings ++ rawlsCompileSettings ++ java11BuildSettings
//See immediately above NOTE.
}
6 changes: 5 additions & 1 deletion project/Testing.scala
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import MinnieKenny.testSettings
import sbt.Keys._
import sbt._

Expand All @@ -22,6 +23,9 @@ object Testing {

val commonTestSettings: Seq[Setting[_]] = List(

testOptions in Test += Tests.Setup(() =>
sys.props += "mockserver.logLevel" -> "WARN"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default log verbosity for mockserver logs every mock defined, for every test that defines it. Since this goes to the console for jenkins jobs, it makes the logs huge, to the point that the browser has trouble searching through them.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome!!

),
// SLF4J initializes itself upon the first logging call. Because sbt
// runs tests in parallel it is likely that a second thread will
// invoke a second logging call before SLF4J has completed
Expand Down Expand Up @@ -55,7 +59,7 @@ object Testing {
(testOnly in Test) := ((testOnly in Test) dependsOn validMySqlHost).evaluated,

parallelExecution in Test := false
) ++ MinnieKenny.testSettings
) ++ (if (sys.props.getOrElse("secrets.skip", "false") != "true") MinnieKenny.testSettings else List())

implicit class ProjectTestSettings(val project: Project) extends AnyVal {
def withTestSettings: Project = project
Expand Down
2 changes: 1 addition & 1 deletion project/build.properties
Original file line number Diff line number Diff line change
@@ -1 +1 @@
sbt.version=1.4.6
sbt.version=1.4.7