diff --git a/centaur/src/main/resources/standardTestCases/ignore_noAddress.test b/centaur/src/main/resources/standardTestCases/ignore_noAddress.test new file mode 100644 index 00000000000..4c4f0270e75 --- /dev/null +++ b/centaur/src/main/resources/standardTestCases/ignore_noAddress.test @@ -0,0 +1,14 @@ +# NB: To request this test by name, make it lowercase, eg sbt "centaur/it:testOnly * -- -n ignore_noaddress" +name: ignore_noAddress +backends: [Papi,Papiv2] +backendsMode: any +testFormat: workflowsuccess + +files { + workflow: ignore_noAddress/ignore_noAddress.wdl +} + +metadata { + workflowName: ignore_noAddress + status: Succeeded +} diff --git a/centaur/src/main/resources/standardTestCases/ignore_noAddress/ignore_noAddress.wdl b/centaur/src/main/resources/standardTestCases/ignore_noAddress/ignore_noAddress.wdl new file mode 100644 index 00000000000..112f2b79a70 --- /dev/null +++ b/centaur/src/main/resources/standardTestCases/ignore_noAddress/ignore_noAddress.wdl @@ -0,0 +1,17 @@ +task task_with_noAddress { + + command { echo "hello, world" } + + output { String s = read_string(stdout()) } + + runtime { + docker: "ubuntu:latest" + # NB: This is ignored because of the config in the centaur configuration. + # If we didn't have that, this task would run forever - or at least until PAPI times it out. + noAddress: true + } +} + +workflow ignore_noAddress { + call task_with_noAddress +} diff --git a/cromwell.example.backends/PAPIv2.conf b/cromwell.example.backends/PAPIv2.conf index ee332615d8d..68bfa4b25d4 100644 --- a/cromwell.example.backends/PAPIv2.conf +++ b/cromwell.example.backends/PAPIv2.conf @@ -28,6 +28,11 @@ backend { # Emit a warning if jobs last longer than this amount of time. This might indicate that something got stuck in PAPI. slow-job-warning-time: 24 hours + # Whether to allow users to specify the 'noAddress' runtime attribute when submitting jobs. + # Defaults to true. + # If set to false, the noAddress attribute will be ignored and public addresses will always be assigned. + allow-noAddress-attribute: true + # Set this to the lower of the two values "Queries per 100 seconds" and "Queries per 100 seconds per user" for # your project. # diff --git a/src/ci/resources/papi_v1_v2alpha1_provider_config.inc.conf b/src/ci/resources/papi_v1_v2alpha1_provider_config.inc.conf index e7a0f2e258e..20cb2aaf459 100644 --- a/src/ci/resources/papi_v1_v2alpha1_provider_config.inc.conf +++ b/src/ci/resources/papi_v1_v2alpha1_provider_config.inc.conf @@ -16,3 +16,5 @@ filesystems { } slow-job-warning-time: 20 minutes + +allow-noAddress-attribute: false diff --git a/src/ci/resources/papi_v2beta_provider_config.inc.conf b/src/ci/resources/papi_v2beta_provider_config.inc.conf index d9c8f9c4775..0c426ae49fb 100644 --- a/src/ci/resources/papi_v2beta_provider_config.inc.conf +++ b/src/ci/resources/papi_v2beta_provider_config.inc.conf @@ -17,3 +17,5 @@ filesystems { } slow-job-warning-time: 20 minutes + +allow-noAddress-attribute: false \ No newline at end of file diff --git a/supportedBackends/google/pipelines/common/src/main/scala/cromwell/backend/google/pipelines/common/PipelinesApiAsyncBackendJobExecutionActor.scala b/supportedBackends/google/pipelines/common/src/main/scala/cromwell/backend/google/pipelines/common/PipelinesApiAsyncBackendJobExecutionActor.scala index 03960ebdcfc..e2816180671 100644 --- a/supportedBackends/google/pipelines/common/src/main/scala/cromwell/backend/google/pipelines/common/PipelinesApiAsyncBackendJobExecutionActor.scala +++ b/supportedBackends/google/pipelines/common/src/main/scala/cromwell/backend/google/pipelines/common/PipelinesApiAsyncBackendJobExecutionActor.scala @@ -458,6 +458,7 @@ class PipelinesApiAsyncBackendJobExecutionActor(override val standardParams: Sta virtualPrivateCloudConfiguration = jesAttributes.virtualPrivateCloudConfiguration, retryWithMoreMemoryKeys = jesAttributes.memoryRetryConfiguration.map(_.errorKeys), fuseEnabled = fuseEnabled(jobDescriptor.workflowDescriptor), + allowNoAddress = pipelinesConfiguration.papiAttributes.allowNoAddress ) case Some(other) => throw new RuntimeException(s"Unexpected initialization data: $other") diff --git a/supportedBackends/google/pipelines/common/src/main/scala/cromwell/backend/google/pipelines/common/PipelinesApiConfigurationAttributes.scala b/supportedBackends/google/pipelines/common/src/main/scala/cromwell/backend/google/pipelines/common/PipelinesApiConfigurationAttributes.scala index a10bb50fe1c..95d87c82027 100644 --- a/supportedBackends/google/pipelines/common/src/main/scala/cromwell/backend/google/pipelines/common/PipelinesApiConfigurationAttributes.scala +++ b/supportedBackends/google/pipelines/common/src/main/scala/cromwell/backend/google/pipelines/common/PipelinesApiConfigurationAttributes.scala @@ -43,7 +43,8 @@ case class PipelinesApiConfigurationAttributes(project: String, gcsTransferConfiguration: GcsTransferConfiguration, virtualPrivateCloudConfiguration: Option[VirtualPrivateCloudConfiguration], batchRequestTimeoutConfiguration: BatchRequestTimeoutConfiguration, - memoryRetryConfiguration: Option[MemoryRetryConfiguration]) + memoryRetryConfiguration: Option[MemoryRetryConfiguration], + allowNoAddress: Boolean) object PipelinesApiConfigurationAttributes { @@ -64,6 +65,8 @@ object PipelinesApiConfigurationAttributes { lazy val DefaultMemoryRetryFactor: GreaterEqualRefined = refineMV[GreaterEqualOne](2.0) + val allowNoAddressAttributeKey = "allow-noAddress-attribute" + private val papiKeys = CommonBackendConfigurationAttributes.commonValidConfigurationAttributeKeys ++ Set( "project", "root", @@ -98,6 +101,7 @@ object PipelinesApiConfigurationAttributes { "virtual-private-cloud.auth", "memory-retry.error-keys", "memory-retry.multiplier", + allowNoAddressAttributeKey ) private val deprecatedJesKeys: Map[String, String] = Map( @@ -149,6 +153,7 @@ object PipelinesApiConfigurationAttributes { val genomicsAuthName: ErrorOr[String] = validate { backendConfig.as[String]("genomics.auth") } val genomicsRestrictMetadataAccess: ErrorOr[Boolean] = validate { backendConfig.as[Option[Boolean]]("genomics.restrict-metadata-access").getOrElse(false) } val genomicsEnableFuse: ErrorOr[Boolean] = validate { backendConfig.as[Option[Boolean]]("genomics.enable-fuse").getOrElse(false) } + val allowNoAddress: ErrorOr[Boolean] = validate { backendConfig.as[Option[Boolean]](allowNoAddressAttributeKey).getOrElse(true) } val gcsFilesystemAuthName: ErrorOr[String] = validate { backendConfig.as[String]("filesystems.gcs.auth") } val qpsValidation = validateQps(backendConfig) val duplicationStrategy = validate { backendConfig.as[Option[String]]("filesystems.gcs.caching.duplication-strategy").getOrElse("copy") match { @@ -217,7 +222,8 @@ object PipelinesApiConfigurationAttributes { gcsTransferConfiguration: GcsTransferConfiguration, virtualPrivateCloudConfiguration: Option[VirtualPrivateCloudConfiguration], batchRequestTimeoutConfiguration: BatchRequestTimeoutConfiguration, - memoryRetryConfig: Option[MemoryRetryConfiguration]): ErrorOr[PipelinesApiConfigurationAttributes] = + memoryRetryConfig: Option[MemoryRetryConfiguration], + allowNoAddress: Boolean): ErrorOr[PipelinesApiConfigurationAttributes] = (googleConfig.auth(genomicsName), googleConfig.auth(gcsName)) mapN { (genomicsAuth, gcsAuth) => PipelinesApiConfigurationAttributes( @@ -239,6 +245,7 @@ object PipelinesApiConfigurationAttributes { virtualPrivateCloudConfiguration = virtualPrivateCloudConfiguration, batchRequestTimeoutConfiguration = batchRequestTimeoutConfiguration, memoryRetryConfiguration = memoryRetryConfig, + allowNoAddress ) } @@ -257,6 +264,7 @@ object PipelinesApiConfigurationAttributes { virtualPrivateCloudConfiguration, batchRequestTimeoutConfigurationValidation, memoryRetryConfig, + allowNoAddress ) flatMapN authGoogleConfigForPapiConfigurationAttributes match { case Valid(r) => r case Invalid(f) => diff --git a/supportedBackends/google/pipelines/common/src/main/scala/cromwell/backend/google/pipelines/common/api/PipelinesApiRequestFactory.scala b/supportedBackends/google/pipelines/common/src/main/scala/cromwell/backend/google/pipelines/common/api/PipelinesApiRequestFactory.scala index 0bc7919a7ce..7fcbb6eceaf 100644 --- a/supportedBackends/google/pipelines/common/src/main/scala/cromwell/backend/google/pipelines/common/api/PipelinesApiRequestFactory.scala +++ b/supportedBackends/google/pipelines/common/src/main/scala/cromwell/backend/google/pipelines/common/api/PipelinesApiRequestFactory.scala @@ -82,10 +82,14 @@ object PipelinesApiRequestFactory { adjustedSizeDisks: Seq[PipelinesApiAttachedDisk], virtualPrivateCloudConfiguration: Option[VirtualPrivateCloudConfiguration], retryWithMoreMemoryKeys: Option[List[String]], - fuseEnabled: Boolean) { + fuseEnabled: Boolean, + allowNoAddress: Boolean) { def literalInputs = inputOutputParameters.literalInputParameters def inputParameters = inputOutputParameters.fileInputParameters def outputParameters = inputOutputParameters.fileOutputParameters def allParameters = inputParameters ++ outputParameters + + // Takes into account the 'noAddress' runtime attribute and the allowNoAddress configuration option: + def effectiveNoAddressValue: Boolean = allowNoAddress && runtimeAttributes.noAddress } } diff --git a/supportedBackends/google/pipelines/common/src/test/scala/cromwell/backend/google/pipelines/common/PipelinesApiConfigurationSpec.scala b/supportedBackends/google/pipelines/common/src/test/scala/cromwell/backend/google/pipelines/common/PipelinesApiConfigurationSpec.scala index 98a552f0cc9..786832d417b 100644 --- a/supportedBackends/google/pipelines/common/src/test/scala/cromwell/backend/google/pipelines/common/PipelinesApiConfigurationSpec.scala +++ b/supportedBackends/google/pipelines/common/src/test/scala/cromwell/backend/google/pipelines/common/PipelinesApiConfigurationSpec.scala @@ -126,6 +126,21 @@ class PipelinesApiConfigurationSpec extends FlatSpec with Matchers with TableDri dockerConf.get.token shouldBe "dockerToken" } + it should "correctly default allowNoAddress to true" in { + val noAddressConf = new PipelinesApiConfiguration(BackendConfigurationDescriptor(backendConfig, globalConfig), genomicsFactory, googleConfiguration, papiAttributes) + noAddressConf.papiAttributes.allowNoAddress should be(true) + } + + it should "be able to set allowNoAddress to false" in { + val updatedBackendConfig = backendConfig.withValue( + PipelinesApiConfigurationAttributes.allowNoAddressAttributeKey, + ConfigValueFactory.fromAnyRef(false) + ) + val updatedPapiAttributes = PipelinesApiConfigurationAttributes(googleConfiguration, updatedBackendConfig, "papi") + val noAddressConf = new PipelinesApiConfiguration(BackendConfigurationDescriptor(updatedBackendConfig, globalConfig), genomicsFactory, googleConfiguration, updatedPapiAttributes) + noAddressConf.papiAttributes.allowNoAddress should be(false) + } + it should "have correct needAuthFileUpload" in { val configs = Table( ("backendConfig", "globalConfig"), diff --git a/supportedBackends/google/pipelines/common/src/test/scala/cromwell/backend/google/pipelines/common/callcaching/PipelinesApiBackendCacheHitCopyingActorSpec.scala b/supportedBackends/google/pipelines/common/src/test/scala/cromwell/backend/google/pipelines/common/callcaching/PipelinesApiBackendCacheHitCopyingActorSpec.scala index f77289b52d1..883533d5195 100644 --- a/supportedBackends/google/pipelines/common/src/test/scala/cromwell/backend/google/pipelines/common/callcaching/PipelinesApiBackendCacheHitCopyingActorSpec.scala +++ b/supportedBackends/google/pipelines/common/src/test/scala/cromwell/backend/google/pipelines/common/callcaching/PipelinesApiBackendCacheHitCopyingActorSpec.scala @@ -422,6 +422,7 @@ class PipelinesApiBackendCacheHitCopyingActorSpec extends TestKitSuite("Pipeline virtualPrivateCloudConfiguration = None, batchRequestTimeoutConfiguration = null, memoryRetryConfiguration = None, + allowNoAddress = true ) val papiConfiguration = mock[PipelinesApiConfiguration] diff --git a/supportedBackends/google/pipelines/v1alpha2/src/main/scala/cromwell/backend/google/pipelines/v1alpha2/GenomicsFactory.scala b/supportedBackends/google/pipelines/v1alpha2/src/main/scala/cromwell/backend/google/pipelines/v1alpha2/GenomicsFactory.scala index 10fca78c104..03d2f304695 100644 --- a/supportedBackends/google/pipelines/v1alpha2/src/main/scala/cromwell/backend/google/pipelines/v1alpha2/GenomicsFactory.scala +++ b/supportedBackends/google/pipelines/v1alpha2/src/main/scala/cromwell/backend/google/pipelines/v1alpha2/GenomicsFactory.scala @@ -37,7 +37,7 @@ case class GenomicsFactory(applicationName: String, authMode: GoogleAuthMode, en val commandLine = s"/bin/bash ${createPipelineParameters.commandScriptContainerPath.pathAsString}" val pipelineInfoBuilder = if (createPipelineParameters.preemptible) PreemptiblePipelineInfoBuilder else NonPreemptiblePipelineInfoBuilder - val pipelineInfo = pipelineInfoBuilder.build(commandLine, createPipelineParameters.runtimeAttributes, createPipelineParameters.dockerImage) + val pipelineInfo = pipelineInfoBuilder.build(commandLine, createPipelineParameters) val inputParameters: Map[String, PipelinesApiInput] = createPipelineParameters.inputParameters.map({ i => i.name -> i }).toMap @@ -57,7 +57,7 @@ case class GenomicsFactory(applicationName: String, authMode: GoogleAuthMode, en // disks cannot have mount points at runtime, so set them null val runtimePipelineResources = { - val resources = pipelineInfoBuilder.build(commandLine, createPipelineParameters.runtimeAttributes, createPipelineParameters.dockerImage).resources + val resources = pipelineInfoBuilder.build(commandLine, createPipelineParameters).resources val disksWithoutMountPoint = resources.getDisks.asScala map { _.setMountPoint(null) } diff --git a/supportedBackends/google/pipelines/v1alpha2/src/main/scala/cromwell/backend/google/pipelines/v1alpha2/api/PipelinesApiInfo.scala b/supportedBackends/google/pipelines/v1alpha2/src/main/scala/cromwell/backend/google/pipelines/v1alpha2/api/PipelinesApiInfo.scala index d1474fef391..d56984cccaa 100644 --- a/supportedBackends/google/pipelines/v1alpha2/src/main/scala/cromwell/backend/google/pipelines/v1alpha2/api/PipelinesApiInfo.scala +++ b/supportedBackends/google/pipelines/v1alpha2/src/main/scala/cromwell/backend/google/pipelines/v1alpha2/api/PipelinesApiInfo.scala @@ -1,6 +1,7 @@ package cromwell.backend.google.pipelines.v1alpha2.api import com.google.api.services.genomics.model.{DockerExecutor, PipelineResources} +import cromwell.backend.google.pipelines.common.api.PipelinesApiRequestFactory.CreatePipelineParameters import cromwell.backend.google.pipelines.common.{GpuResource, PipelinesApiRuntimeAttributes} import cromwell.backend.google.pipelines.v1alpha2.PipelinesConversions._ import wdl4s.parser.MemoryUnit @@ -27,32 +28,33 @@ trait PipelineInfoBuilder { } } - def buildResources(runtimeAttributes: PipelinesApiRuntimeAttributes): PipelineResources = { + def buildResources(createPipelineParameters: CreatePipelineParameters): PipelineResources = { + val runtimeAttributes =createPipelineParameters.runtimeAttributes val resources = new PipelineResources() .setMinimumRamGb(runtimeAttributes.memory.to(MemoryUnit.GB).amount) .setMinimumCpuCores(runtimeAttributes.cpu.value) .setZones(runtimeAttributes.zones.asJava) .setDisks(runtimeAttributes.disks.map(_.toGoogleDisk).asJava) .setBootDiskSizeGb(runtimeAttributes.bootDiskSize) - .setNoAddress(runtimeAttributes.noAddress) + .setNoAddress(createPipelineParameters.effectiveNoAddressValue) setGpu(resources, runtimeAttributes) } - def build(commandLine: String, runtimeAttributes: PipelinesApiRuntimeAttributes, docker: String): PipelineInfo + def build(commandLine: String, createPipelineParameters: CreatePipelineParameters): PipelineInfo } object NonPreemptiblePipelineInfoBuilder extends PipelineInfoBuilder { - def build(commandLine: String, runtimeAttributes: PipelinesApiRuntimeAttributes, dockerImage: String): PipelineInfo = { - val resources = buildResources(runtimeAttributes).setPreemptible(false) - new NonPreemptiblePipelineInfoBuilder(resources, buildDockerExecutor(commandLine, dockerImage)) + def build(commandLine: String, createPipelineParameters: CreatePipelineParameters): PipelineInfo = { + val resources = buildResources(createPipelineParameters).setPreemptible(false) + new NonPreemptiblePipelineInfoBuilder(resources, buildDockerExecutor(commandLine, createPipelineParameters.dockerImage)) } } object PreemptiblePipelineInfoBuilder extends PipelineInfoBuilder { - def build(commandLine: String, runtimeAttributes: PipelinesApiRuntimeAttributes, dockerImage: String): PipelineInfo = { - val resources = buildResources(runtimeAttributes).setPreemptible(true) - new PreemptiblePipelineInfoBuilder(resources, buildDockerExecutor(commandLine, dockerImage)) + def build(commandLine: String, createPipelineParameters: CreatePipelineParameters): PipelineInfo = { + val resources = buildResources(createPipelineParameters).setPreemptible(true) + new PreemptiblePipelineInfoBuilder(resources, buildDockerExecutor(commandLine, createPipelineParameters.dockerImage)) } } diff --git a/supportedBackends/google/pipelines/v2alpha1/src/main/scala/cromwell/backend/google/pipelines/v2alpha1/GenomicsFactory.scala b/supportedBackends/google/pipelines/v2alpha1/src/main/scala/cromwell/backend/google/pipelines/v2alpha1/GenomicsFactory.scala index 7a05455262f..3ac5d948839 100644 --- a/supportedBackends/google/pipelines/v2alpha1/src/main/scala/cromwell/backend/google/pipelines/v2alpha1/GenomicsFactory.scala +++ b/supportedBackends/google/pipelines/v2alpha1/src/main/scala/cromwell/backend/google/pipelines/v2alpha1/GenomicsFactory.scala @@ -96,7 +96,7 @@ case class GenomicsFactory(applicationName: String, authMode: GoogleAuthMode, en networkLabelOption match { case Some(networkLabel) => val network = new Network() - .setUsePrivateAddress(createPipelineParameters.runtimeAttributes.noAddress) + .setUsePrivateAddress(createPipelineParameters.effectiveNoAddressValue) .setName(VirtualPrivateCloudNetworkPath.format(createPipelineParameters.projectId, networkLabel._2)) subnetworkLabelOption foreach { case(_, subnet) => network.setSubnetwork(subnet) } @@ -104,7 +104,7 @@ case class GenomicsFactory(applicationName: String, authMode: GoogleAuthMode, en case None => // Falling back to running the job on default network since the project does not provide the custom network // specifying keys in its metadata - new Network().setUsePrivateAddress(createPipelineParameters.runtimeAttributes.noAddress) + new Network().setUsePrivateAddress(createPipelineParameters.effectiveNoAddressValue) } } @@ -120,7 +120,7 @@ case class GenomicsFactory(applicationName: String, authMode: GoogleAuthMode, en def createNetwork(): Network = { createPipelineParameters.virtualPrivateCloudConfiguration match { - case None => new Network().setUsePrivateAddress(createPipelineParameters.runtimeAttributes.noAddress) + case None => new Network().setUsePrivateAddress(createPipelineParameters.effectiveNoAddressValue) case Some(vpcConfig) => createNetworkWithVPC(vpcConfig).handleErrorWith { e => IO.raiseError(new RuntimeException(s"Failed to create Network object for project `${createPipelineParameters.projectId}`. " + diff --git a/supportedBackends/google/pipelines/v2beta/src/main/scala/cromwell/backend/google/pipelines/v2beta/LifeSciencesFactory.scala b/supportedBackends/google/pipelines/v2beta/src/main/scala/cromwell/backend/google/pipelines/v2beta/LifeSciencesFactory.scala index 5ed6544a785..5681e12ecd5 100644 --- a/supportedBackends/google/pipelines/v2beta/src/main/scala/cromwell/backend/google/pipelines/v2beta/LifeSciencesFactory.scala +++ b/supportedBackends/google/pipelines/v2beta/src/main/scala/cromwell/backend/google/pipelines/v2beta/LifeSciencesFactory.scala @@ -88,7 +88,6 @@ case class LifeSciencesFactory(applicationName: String, authMode: GoogleAuthMode } } - def networkFromLabels(vpcConfig: VirtualPrivateCloudConfiguration, projectLabels: ProjectLabels): Network = { val networkLabelOption = projectLabels.labels.find(l => l._1.equals(vpcConfig.name)) val subnetworkLabelOption = vpcConfig.subnetwork.flatMap(s => projectLabels.labels.find(l => l._1.equals(s))) @@ -96,7 +95,7 @@ case class LifeSciencesFactory(applicationName: String, authMode: GoogleAuthMode networkLabelOption match { case Some(networkLabel) => val network = new Network() - .setUsePrivateAddress(createPipelineParameters.runtimeAttributes.noAddress) + .setUsePrivateAddress(createPipelineParameters.effectiveNoAddressValue) .setNetwork(VirtualPrivateCloudNetworkPath.format(createPipelineParameters.projectId, networkLabel._2)) subnetworkLabelOption foreach { case(_, subnet) => network.setSubnetwork(subnet) } @@ -104,7 +103,7 @@ case class LifeSciencesFactory(applicationName: String, authMode: GoogleAuthMode case None => // Falling back to running the job on default network since the project does not provide the custom network // specifying keys in its metadata - new Network().setUsePrivateAddress(createPipelineParameters.runtimeAttributes.noAddress) + new Network().setUsePrivateAddress(createPipelineParameters.effectiveNoAddressValue) } } @@ -120,7 +119,7 @@ case class LifeSciencesFactory(applicationName: String, authMode: GoogleAuthMode def createNetwork(): Network = { createPipelineParameters.virtualPrivateCloudConfiguration match { - case None => new Network().setUsePrivateAddress(createPipelineParameters.runtimeAttributes.noAddress) + case None => new Network().setUsePrivateAddress(createPipelineParameters.effectiveNoAddressValue) case Some(vpcConfig) => createNetworkWithVPC(vpcConfig).handleErrorWith { e => IO.raiseError(new RuntimeException(s"Failed to create Network object for project `${createPipelineParameters.projectId}`. " +