Merge branch 'develop' into update/minor_patch

broadinstitute · Dec 9, 2024 · b356a67 · b356a67
2 parents a949767 + a085daa
commit b356a67
Show file tree

Hide file tree

Showing 17 changed files with 223 additions and 19 deletions.
diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md
@@ -8,7 +8,10 @@ Ticket: <Link to Jira ticket>
 - [ ] Include the JIRA issue number in the PR description and title
 - [ ] Make sure Swagger is updated if API changes
   - [ ] **...and Orchestration's Swagger too!**
-- [ ] If you changed anything in `model/`, then you should [publish a new official `rawls-model`](https://github.com/broadinstitute/rawls/blob/develop/README.md#publish-rawls-model) and update `rawls-model` in [Orchestration's dependencies](https://github.com/broadinstitute/firecloud-orchestration/blob/develop/project/Dependencies.scala).
+- [ ] If you changed anything in `model/`, then you should publish a new official `rawls-model` and perform the corresponding dependency updates as specified in the [README](https://github.com/broadinstitute/rawls/blob/develop/README.md#publish-rawls-model):
+  - [ ] in the automation subdirectory
+  - [ ] in workbench-libs
+  - [ ] in firecloud-orchestration
 - [ ] Get two thumbsworth of PR review
 - [ ] Verify all tests go green, including CI tests
 - [ ] **Squash commits and merge** to develop (branches are automatically deleted after merging)

diff --git a/Dockerfile b/Dockerfile
@@ -1,5 +1,5 @@
 # TODO: Replace build script with multi-stage Dockerfile that builds its own JAR
-# FROM sbtscala/scala-sbt:eclipse-temurin-17.0.13_11_1.10.5_2.13.15 as jar-builder
+# FROM sbtscala/scala-sbt:eclipse-temurin-17.0.13_11_1.10.6_2.13.15 as jar-builder
 # ...build JAR...
 # FROM us.gcr.io/broad-dsp-gcr-public/base/jre:17-debian
 # COPY --from=jar-builder ./rawls*.jar /rawls

diff --git a/README.md b/README.md
@@ -154,9 +154,10 @@ VAULT_TOKEN=$(cat ~/.vault-token) ARTIFACTORY_USERNAME=dsdejenkins ARTIFACTORY_P
 You can view what is in the artifactory here: https://broadinstitute.jfrog.io/broadinstitute/webapp/#/home
 
 After publishing:
-* update [model/CHANGELOG.md](model/CHANGELOG.md) properly
-* update the rawls-model dependency in the automation subdirectory, and ensure that sbt project is healthy
-* update the rawls-model dependency in workbench-libs serviceTest, and ensure that sbt project is healthy
+* Update [model/CHANGELOG.md](model/CHANGELOG.md) properly
+* Update the rawls-model dependency in the automation subdirectory, and ensure that the sbt project is healthy
+* In workbench-libs, update the rawls-model dependency in the project dependencies, update the README and the serviceTest changelog accordingly, and ensure that the sbt project is healthy
+* In firecloud-orchestration, update the rawls-model dependency in the project dependencies, and ensure that the sbt project is healthy
 
 
 ## Troubleshooting

diff --git a/automation/Dockerfile-tests b/automation/Dockerfile-tests
@@ -1,4 +1,4 @@
-FROM sbtscala/scala-sbt:eclipse-temurin-17.0.13_11_1.10.5_2.13.15
+FROM sbtscala/scala-sbt:eclipse-temurin-17.0.13_11_1.10.6_2.13.15
 
 COPY src /app/src
 COPY test.sh /app

diff --git a/core/src/main/scala/org/broadinstitute/dsde/rawls/Boot.scala b/core/src/main/scala/org/broadinstitute/dsde/rawls/Boot.scala
@@ -299,6 +299,8 @@ object Boot extends IOApp with LazyLogging {
         CromwellBackend(appConfigManager.conf.getString("executionservice.defaultNetworkBackend"))
       val highSecurityNetworkCromwellBackend: CromwellBackend =
         CromwellBackend(appConfigManager.conf.getString("executionservice.highSecurityNetworkBackend"))
+      val gcpBatchBackend: CromwellBackend =
+        CromwellBackend(appConfigManager.conf.getString("executionservice.gcpBatchBackend"))
 
       val wdlParsingConfig = WDLParserConfig(appConfigManager.conf.getConfig("wdl-parsing"))
       def cromwellSwaggerClient = new CromwellSwaggerClient(wdlParsingConfig.serverBasePath)
@@ -599,6 +601,7 @@ object Boot extends IOApp with LazyLogging {
           useWorkflowCollectionLabel,
           defaultNetworkCromwellBackend,
           highSecurityNetworkCromwellBackend,
+          gcpBatchBackend,
           methodConfigResolver,
           bardService,
           workspaceSettingRepository

diff --git a/core/src/main/scala/org/broadinstitute/dsde/rawls/jobexec/WorkflowSubmissionActor.scala b/core/src/main/scala/org/broadinstitute/dsde/rawls/jobexec/WorkflowSubmissionActor.scala
@@ -44,6 +44,7 @@ object WorkflowSubmissionActor {
             useWorkflowCollectionLabel: Boolean,
             defaultNetworkCromwellBackend: CromwellBackend,
             highSecurityNetworkCromwellBackend: CromwellBackend,
+            gcpBatchBackend: CromwellBackend,
             methodConfigResolver: MethodConfigResolver,
             bardService: BardService,
             workspaceSettingRepository: WorkspaceSettingRepository
@@ -69,6 +70,7 @@ object WorkflowSubmissionActor {
         useWorkflowCollectionLabel,
         defaultNetworkCromwellBackend,
         highSecurityNetworkCromwellBackend,
+        gcpBatchBackend,
         methodConfigResolver,
         bardService,
         workspaceSettingRepository
@@ -105,6 +107,7 @@ class WorkflowSubmissionActor(val dataSource: SlickDataSource,
                               val useWorkflowCollectionLabel: Boolean,
                               val defaultNetworkCromwellBackend: CromwellBackend,
                               val highSecurityNetworkCromwellBackend: CromwellBackend,
+                              val gcpBatchBackend: CromwellBackend,
                               val methodConfigResolver: MethodConfigResolver,
                               val bardService: BardService,
                               val workspaceSettingRepository: WorkspaceSettingRepository
@@ -159,6 +162,7 @@ trait WorkflowSubmission extends FutureSupport with LazyLogging with MethodWiths
   val useWorkflowCollectionLabel: Boolean
   val defaultNetworkCromwellBackend: CromwellBackend
   val highSecurityNetworkCromwellBackend: CromwellBackend
+  val gcpBatchBackend: CromwellBackend
   val methodConfigResolver: MethodConfigResolver
   val bardService: BardService
   val workspaceSettingRepository: WorkspaceSettingRepository
@@ -301,7 +305,17 @@ trait WorkflowSubmission extends FutureSupport with LazyLogging with MethodWiths
       // - final_workflow_outputs_dir = submissions/final-outputs
       // - final_workflow_outputs_mode = "copy".
 
+      useCromwellGcpBatchBackend: Boolean = currentSettings.exists {
+        case backendSetting: UseCromwellGcpBatchBackendSetting => backendSetting.config.enabled
+        case _                                                 => false
+      }
+      cromwellSubmissionBackend =
+        if (useCromwellGcpBatchBackend) gcpBatchBackend else highSecurityNetworkCromwellBackend
+
       executionServiceWorkflowOptions = ExecutionServiceWorkflowOptions(
+        // We pass the submission root as the value for two options,
+        // one for the PAPI Cromwell backend and one for the GCP Batch backend.
+        submission.submissionRoot,
         submission.submissionRoot,
         final_workflow_outputs_dir,
         final_workflow_outputs_dir_metadata,
@@ -315,7 +329,7 @@ trait WorkflowSubmission extends FutureSupport with LazyLogging with MethodWiths
         deleteIntermediateOutputFiles,
         useReferenceDisks,
         memoryRetryMultiplier,
-        highSecurityNetworkCromwellBackend,
+        cromwellSubmissionBackend,
         workflowFailureMode,
         google_labels = Map("terra-submission-id" -> s"terra-${submission.id.toString}"),
         ignoreEmptyOutputs,

diff --git a/core/src/main/scala/org/broadinstitute/dsde/rawls/monitor/BootMonitors.scala b/core/src/main/scala/org/broadinstitute/dsde/rawls/monitor/BootMonitors.scala
@@ -94,6 +94,7 @@ object BootMonitors extends LazyLogging {
                    useWorkflowCollectionLabel: Boolean,
                    defaultNetworkCromwellBackend: CromwellBackend,
                    highSecurityNetworkCromwellBackend: CromwellBackend,
+                   gcpBatchBackend: CromwellBackend,
                    methodConfigResolver: MethodConfigResolver,
                    bardService: BardService,
                    workspaceSettingRepository: WorkspaceSettingRepository
@@ -147,6 +148,7 @@ object BootMonitors extends LazyLogging {
         useWorkflowCollectionLabel,
         defaultNetworkCromwellBackend,
         highSecurityNetworkCromwellBackend,
+        gcpBatchBackend,
         methodConfigResolver,
         bardService,
         workspaceSettingRepository
@@ -338,6 +340,7 @@ object BootMonitors extends LazyLogging {
                                             useWorkflowCollectionLabel: Boolean,
                                             defaultNetworkCromwellBackend: CromwellBackend,
                                             highSecurityNetworkCromwellBackend: CromwellBackend,
+                                            gcpBatchBackend: CromwellBackend,
                                             methodConfigResolver: MethodConfigResolver,
                                             bardService: BardService,
                                             workspaceSettingRepository: WorkspaceSettingRepository
@@ -366,6 +369,7 @@ object BootMonitors extends LazyLogging {
           useWorkflowCollectionLabel,
           defaultNetworkCromwellBackend,
           highSecurityNetworkCromwellBackend,
+          gcpBatchBackend,
           methodConfigResolver,
           bardService,
           workspaceSettingRepository

diff --git a/core/src/main/scala/org/broadinstitute/dsde/rawls/submissions/SubmissionsService.scala b/core/src/main/scala/org/broadinstitute/dsde/rawls/submissions/SubmissionsService.scala
@@ -1,6 +1,7 @@
 package org.broadinstitute.dsde.rawls.submissions
 
 import akka.http.scaladsl.model.StatusCodes
+import com.google.common.annotations.VisibleForTesting
 import com.typesafe.scalalogging.LazyLogging
 import org.broadinstitute.dsde.rawls.config.WorkspaceServiceConfig
 import org.broadinstitute.dsde.rawls.dataaccess.slick.{DataAccess, ReadWriteAction, WorkflowRecord}
@@ -531,10 +532,34 @@ class SubmissionsService(
       ps.inputs.filter(_.inputResolutions.forall(_.error.isEmpty))
     )
 
+  @VisibleForTesting
+  def validateCostCap(costCap: Option[BigDecimal]): Unit = {
+    // must be a positive number, no more than two decimal places, and a max of ... 10 billion?
+    val maybeErrorMessage = costCap.collectFirst {
+      case cap if cap.sign != 1 => "per-workflow cost cap must be positive"
+      case cap if cap.compare(BigDecimal.valueOf(10000000000L)) >= 0 =>
+        "per-workflow cost cap must be less than 10,000,000,000"
+      case cap if !(cap * 100).isWhole =>
+        "per-workflow cost cap must have a max of two decimal places"
+    }
+
+    maybeErrorMessage.foreach { msg =>
+      throw new RawlsExceptionWithErrorReport(
+        errorReport = ErrorReport(
+          StatusCodes.BadRequest,
+          msg
+        )
+      )
+    }
+
+  }
+
   private def prepareSubmission(workspaceName: WorkspaceName,
                                 submissionRequest: SubmissionRequest
   ): Future[PreparedSubmission] = {
 
+    validateCostCap(submissionRequest.perWorkflowCostCap)
+
     val submissionId: UUID = UUID.randomUUID()
 
     for {

diff --git a/core/src/test/scala/org/broadinstitute/dsde/rawls/jobexec/WorkflowSubmissionSpec.scala b/core/src/test/scala/org/broadinstitute/dsde/rawls/jobexec/WorkflowSubmissionSpec.scala
@@ -23,9 +23,11 @@ import org.broadinstitute.dsde.rawls.jobexec.WorkflowSubmissionActor.{
 import org.broadinstitute.dsde.rawls.metrics.{BardService, RawlsStatsDTestUtils}
 import org.broadinstitute.dsde.rawls.mock.{MockBardService, MockSamDAO, RemoteServicesMockServer}
 import org.broadinstitute.dsde.rawls.model.ExecutionJsonSupport.ExecutionServiceWorkflowOptionsFormat
-import org.broadinstitute.dsde.rawls.model.WorkspaceSettingConfig.SeparateSubmissionFinalOutputsConfig
-import org.broadinstitute.dsde.rawls.model.WorkspaceSettingTypes.SeparateSubmissionFinalOutputs
-import org.broadinstitute.dsde.rawls.model.{WorkspaceSetting, _}
+import org.broadinstitute.dsde.rawls.model.WorkspaceSettingConfig.{
+  SeparateSubmissionFinalOutputsConfig,
+  UseCromwellGcpBatchBackendConfig
+}
+import org.broadinstitute.dsde.rawls.model._
 import org.broadinstitute.dsde.rawls.util.MockitoTestUtils
 import org.broadinstitute.dsde.rawls.workspace.WorkspaceSettingRepository
 import org.broadinstitute.dsde.rawls.{RawlsExceptionWithErrorReport, RawlsTestUtils}
@@ -120,6 +122,7 @@ class WorkflowSubmissionSpec(_system: ActorSystem)
     val useWorkflowCollectionLabel: Boolean = false,
     val defaultNetworkCromwellBackend: CromwellBackend = CromwellBackend("PAPIv2"),
     val highSecurityNetworkCromwellBackend: CromwellBackend = CromwellBackend("PAPIv2-CloudNAT"),
+    val gcpBatchBackend: CromwellBackend = CromwellBackend("GCPBatch"),
     val methodConfigResolver: MethodConfigResolver = methodConfigResolver,
     val bardService: BardService = mockBardService,
     val workspaceSettingRepository: WorkspaceSettingRepository = mockWorkspaceSettingRepository
@@ -381,6 +384,7 @@ class WorkflowSubmissionSpec(_system: ActorSystem)
         Some(
           ExecutionServiceWorkflowOptions(
             jes_gcs_root = s"gs://${testData.workspace.bucketName}/${testData.submission1.submissionId}",
+            gcp_batch_gcs_root = s"gs://${testData.workspace.bucketName}/${testData.submission1.submissionId}",
             None,
             None,
             google_project = testData.workspace.googleProjectId.value,
@@ -937,6 +941,7 @@ class WorkflowSubmissionSpec(_system: ActorSystem)
           false,
           CromwellBackend("PAPIv2"),
           CromwellBackend("PAPIv2-CloudNAT"),
+          CromwellBackend("GCPBatch"),
           methodConfigResolver,
           mockBardService,
           mockWorkspaceSettingRepository
@@ -1005,6 +1010,7 @@ class WorkflowSubmissionSpec(_system: ActorSystem)
           false,
           CromwellBackend("PAPIv2"),
           CromwellBackend("PAPIv2-CloudNAT"),
+          CromwellBackend("GCPBatch"),
           methodConfigResolver,
           mockBardService,
           mockWorkspaceSettingRepository
@@ -1202,6 +1208,90 @@ class WorkflowSubmissionSpec(_system: ActorSystem)
     }
   }
 
+  it should "submit workflows to Cromwell's GCP Batch backend when UseCromwellGcpBatchBackendSetting is true" in withDefaultTestDatabase {
+    val mockExecCluster = MockShardedExecutionServiceCluster.fromDAO(new MockExecutionServiceDAO(), slickDataSource)
+    val workspaceSettingRepository = mock[WorkspaceSettingRepository]
+    when(workspaceSettingRepository.getWorkspaceSettings(UUID.fromString(testData.workspace.workspaceId))).thenReturn(
+      Future.successful(List(UseCromwellGcpBatchBackendSetting(UseCromwellGcpBatchBackendConfig(true))))
+    )
+
+    val workflowSubmission =
+      new TestWorkflowSubmission(slickDataSource, workspaceSettingRepository = workspaceSettingRepository) {
+        override val executionServiceCluster = mockExecCluster
+      }
+
+    val (workflowRecs, submissionRec, workspaceRec) =
+      getWorkflowSubmissionWorkspaceRecords(testData.regionalSubmission, testData.workspace)
+
+    Await.result(
+      workflowSubmission.submitWorkflowBatch(WorkflowBatch(workflowRecs.map(_.id), submissionRec, workspaceRec)),
+      Duration.Inf
+    )
+
+    val workflowOptions = mockExecCluster.getDefaultSubmitMember
+      .asInstanceOf[MockExecutionServiceDAO]
+      .submitOptions
+      .map(_.parseJson.convertTo[ExecutionServiceWorkflowOptions])
+
+    workflowOptions.get.backend should be(CromwellBackend("GCPBatch"))
+  }
+
+  it should "submit workflows to Cromwell's high security network backend when UseCromwellGcpBatchBackendSetting is false" in withDefaultTestDatabase {
+    val mockExecCluster = MockShardedExecutionServiceCluster.fromDAO(new MockExecutionServiceDAO(), slickDataSource)
+    val workspaceSettingRepository = mock[WorkspaceSettingRepository]
+    when(workspaceSettingRepository.getWorkspaceSettings(UUID.fromString(testData.workspace.workspaceId))).thenReturn(
+      Future.successful(List(UseCromwellGcpBatchBackendSetting(UseCromwellGcpBatchBackendConfig(false))))
+    )
+
+    val workflowSubmission =
+      new TestWorkflowSubmission(slickDataSource, workspaceSettingRepository = workspaceSettingRepository) {
+        override val executionServiceCluster = mockExecCluster
+      }
+
+    val (workflowRecs, submissionRec, workspaceRec) =
+      getWorkflowSubmissionWorkspaceRecords(testData.regionalSubmission, testData.workspace)
+
+    Await.result(
+      workflowSubmission.submitWorkflowBatch(WorkflowBatch(workflowRecs.map(_.id), submissionRec, workspaceRec)),
+      Duration.Inf
+    )
+
+    val workflowOptions = mockExecCluster.getDefaultSubmitMember
+      .asInstanceOf[MockExecutionServiceDAO]
+      .submitOptions
+      .map(_.parseJson.convertTo[ExecutionServiceWorkflowOptions])
+
+    workflowOptions.get.backend should be(CromwellBackend("PAPIv2-CloudNAT"))
+  }
+
+  it should "submit workflows to Cromwell's high security network backend when UseCromwellGcpBatchBackendSetting is not set" in withDefaultTestDatabase {
+    val mockExecCluster = MockShardedExecutionServiceCluster.fromDAO(new MockExecutionServiceDAO(), slickDataSource)
+    val workspaceSettingRepository = mock[WorkspaceSettingRepository]
+    when(workspaceSettingRepository.getWorkspaceSettings(UUID.fromString(testData.workspace.workspaceId))).thenReturn(
+      Future.successful(List())
+    )
+
+    val workflowSubmission =
+      new TestWorkflowSubmission(slickDataSource, workspaceSettingRepository = workspaceSettingRepository) {
+        override val executionServiceCluster = mockExecCluster
+      }
+
+    val (workflowRecs, submissionRec, workspaceRec) =
+      getWorkflowSubmissionWorkspaceRecords(testData.regionalSubmission, testData.workspace)
+
+    Await.result(
+      workflowSubmission.submitWorkflowBatch(WorkflowBatch(workflowRecs.map(_.id), submissionRec, workspaceRec)),
+      Duration.Inf
+    )
+
+    val workflowOptions = mockExecCluster.getDefaultSubmitMember
+      .asInstanceOf[MockExecutionServiceDAO]
+      .submitOptions
+      .map(_.parseJson.convertTo[ExecutionServiceWorkflowOptions])
+
+    workflowOptions.get.backend should be(CromwellBackend("PAPIv2-CloudNAT"))
+  }
+
   private def setWorkflowBatchToQueued(batchSize: Int, submissionId: String): Seq[WorkflowRecord] =
     runAndWait(
       for {

diff --git a/core/src/test/scala/org/broadinstitute/dsde/rawls/model/ExecutionModelSpec.scala b/core/src/test/scala/org/broadinstitute/dsde/rawls/model/ExecutionModelSpec.scala
@@ -126,6 +126,7 @@ class ExecutionModelSpec extends AnyFlatSpec with Matchers {
   "ExecutionServiceWorkflowOptions" should "serialize/deserialize to/from JSON" in {
     val test = ExecutionServiceWorkflowOptions(
       jes_gcs_root = "jes_gcs_root",
+      gcp_batch_gcs_root = "example_gcp_batch_gcs_root",
       final_workflow_outputs_dir = None,
       final_workflow_outputs_dir_metadata = None,
       google_project = "google_project",
@@ -162,6 +163,7 @@ class ExecutionModelSpec extends AnyFlatSpec with Matchers {
       """
         |{
         |  "jes_gcs_root": "jes_gcs_root",
+        |  "gcp_batch_gcs_root": "example_gcp_batch_gcs_root",
         |  "google_project": "google_project",
         |  "account_name": "account_name",
         |  "google_compute_service_account": "[email protected]",
@@ -192,6 +194,7 @@ class ExecutionModelSpec extends AnyFlatSpec with Matchers {
       """
         |{
         |  "jes_gcs_root": "jes_gcs_root",
+        |  "gcp_batch_gcs_root": "example_gcp_batch_gcs_root",
         |  "google_project": "google_project",
         |  "account_name": "account_name",
         |  "google_compute_service_account": "[email protected]",