AN-736 Add support for container attribute in WDL 1.1 (#7809)

Author: jgainerdewar

CHANGELOG.md

@@ -30,8 +30,14 @@ This will allow for better grouping of jobs in the Batch UI and ensure determini
 * Added support for specifying an IAM role for AWS Batch job containers via the `aws_batch_job_role_arn` workflow option. This allows containers to access AWS resources based on the permissions granted to the specified role.
 * ECR [pull-through caches](https://docs.aws.amazon.com/AmazonECR/latest/userguide/pull-through-cache.html) can now be used to access Docker images. See [ReadTheDocs](https://cromwell.readthedocs.io/en/develop/backends/AWSBatch/) for details.
 
+### Progress toward WDL 1.1 Support
+ * WDL 1.1 support is in progress. Users that would like to try out the current partial support can do so by using WDL version `development-1.1`. In Cromwell 91, `development-1.1` has been enhanced to include:
+   * Runtime attribute `container`, which may be a single string or an array of strings, is preferred over `docker` for specifying the image a task should run on. If given a list of multiple images, Cromwell will choose the first.
+   * `docker://` is permitted as a prefix for image names, ex. `container: docker://ubuntu:latest`.
+
 ### Other changes
 * Removed unused code related to Azure cloud services.
+* Changed log level from WARN to INFO for messages about unsupported runtime attributes.
 
 ## 90 Release Notes
 

backend/src/main/scala/cromwell/backend/standard/StandardCachingActorHelper.scala

@@ -4,7 +4,7 @@ import akka.actor.{Actor, ActorRef}
 import cromwell.backend._
 import cromwell.backend.io.{JobPaths, WorkflowPaths}
 import cromwell.backend.standard.callcaching.JobCachingActorHelper
-import cromwell.backend.validation.{DockerValidation, RuntimeAttributesValidation, ValidatedRuntimeAttributes}
+import cromwell.backend.validation.{Containers, RuntimeAttributesValidation, ValidatedRuntimeAttributes}
 import cromwell.core.logging.JobLogging
 import cromwell.core.path.Path
 import cromwell.services.metadata.CallMetadataKeys
@@ -57,7 +57,7 @@ trait StandardCachingActorHelper extends JobCachingActorHelper {
   }
 
   lazy val isDockerRun: Boolean =
-    RuntimeAttributesValidation.extractOption(DockerValidation.instance, validatedRuntimeAttributes).isDefined
+    Containers.extractContainerOption(validatedRuntimeAttributes).isDefined
 
   /**
     * Returns the paths to the job.

backend/src/main/scala/cromwell/backend/standard/StandardInitializationActor.scala

@@ -95,7 +95,7 @@ class StandardInitializationActor(val standardParams: StandardInitializationActo
 
       if (notSupportedAttributes.nonEmpty) {
         val notSupportedAttrString = notSupportedAttributes mkString ", "
-        workflowLogger.warn(
+        workflowLogger.info(
           s"Key/s [$notSupportedAttrString] is/are not supported by backend. " +
             s"Unsupported attributes will not be part of job executions."
         )

backend/src/main/scala/cromwell/backend/standard/pollmonitoring/PollResultMonitorActor.scala

@@ -1,14 +1,8 @@
 package cromwell.backend.standard.pollmonitoring
 
 import akka.actor.{Actor, ActorRef}
+import cromwell.backend.validation._
 import cromwell.backend.{BackendJobDescriptor, BackendWorkflowDescriptor, Platform}
-import cromwell.backend.validation.{
-  CpuValidation,
-  DockerValidation,
-  MemoryValidation,
-  RuntimeAttributesValidation,
-  ValidatedRuntimeAttributes
-}
 import cromwell.core.logging.JobLogger
 import cromwell.services.cost.InstantiatedVmInfo
 import cromwell.services.metadata.CallMetadataKeys
@@ -70,8 +64,7 @@ trait PollResultMonitorActor[PollResultType] extends Actor {
     val workflowDescriptor = params.workflowDescriptor
     val jobDescriptor = params.jobDescriptor
     val platform = params.platform.map(_.runtimeKey)
-    val dockerImage =
-      RuntimeAttributesValidation.extractOption(DockerValidation.instance, validatedRuntimeAttributes)
+    val dockerImage = Containers.extractContainerOption(validatedRuntimeAttributes)
     val cpus = RuntimeAttributesValidation.extract(CpuValidation.instance, validatedRuntimeAttributes).value
     val memory = RuntimeAttributesValidation
       .extract(MemoryValidation.instance(), validatedRuntimeAttributes)

backend/src/main/scala/cromwell/backend/validation/Containers.scala

@@ -0,0 +1,87 @@
+package cromwell.backend.validation
+
+import cats.implicits.catsSyntaxValidatedId
+import common.validation.ErrorOr.ErrorOr
+import wdl.draft2.model.LocallyQualifiedName
+import wom.RuntimeAttributesKeys
+import wom.types.{WomStringType, WomType}
+import wom.values.{WomArray, WomString, WomValue}
+
+// Wrapper type for the list of containers that can be provided as 'docker' or 'container' runtime attributes.
+// In WDL, this value can be either a single string or an array of strings, but in the backend we always
+// want to deal with it as a list of strings.
+//
+// Previous to WDL 1.1, only 'docker' was supported. From WDL 1.1 onwards, they are aliases of each other, with
+// `container` being preferred and `docker` deprecated. Only one of they two may be provided in runtime attrs.
+// Note that we strip `container` out of pre-1.1 WDL files during parsing, so at this stage we only see `docker`
+// in those cases.
+case class Containers(values: List[String]) {
+  override def toString: String = values.mkString(", ")
+}
+
+object Containers {
+  val validWdlTypes: Set[wom.types.WomType] =
+    Set(wom.types.WomStringType, wom.types.WomArrayType(wom.types.WomStringType))
+
+  val runtimeAttrKeys = List(RuntimeAttributesKeys.ContainerKey, RuntimeAttributesKeys.DockerKey)
+
+  def apply(value: String): Containers = Containers(List(value))
+
+  def extractContainer(validatedRuntimeAttributes: ValidatedRuntimeAttributes): String =
+    extractContainerOption(validatedRuntimeAttributes).getOrElse {
+      throw new RuntimeException("No container image found in either 'container' or 'docker' runtime attributes.")
+    }
+
+  def extractContainerOption(validatedRuntimeAttributes: ValidatedRuntimeAttributes): Option[String] = {
+    val dockerContainer = RuntimeAttributesValidation
+      .extractOption[Containers](RuntimeAttributesKeys.DockerKey, validatedRuntimeAttributes)
+      .flatMap(_.values.headOption)
+    val containerContainer = RuntimeAttributesValidation
+      .extractOption[Containers](RuntimeAttributesKeys.ContainerKey, validatedRuntimeAttributes)
+      .flatMap(_.values.headOption)
+
+    containerContainer.orElse(dockerContainer)
+  }
+
+  def extractContainerFromPreValidationAttrs(attributes: Map[LocallyQualifiedName, WomValue]): Option[String] = {
+    val containerContainer = attributes.get(RuntimeAttributesKeys.ContainerKey) match {
+      case Some(WomArray(_, values)) =>
+        values.headOption.map(_.valueString)
+      case Some(WomString(value)) => Some(value)
+      case _ => None
+    }
+
+    val dockerContainer = attributes.get(RuntimeAttributesKeys.DockerKey) match {
+      case Some(WomArray(_, values)) =>
+        values.headOption.map(_.valueString)
+      case Some(WomString(value)) => Some(value)
+      case _ => None
+    }
+
+    // TODO enhance to select the best container from the list if multiple are provided.
+    // Currently we always choose the first, should prefer one that matches our platform.
+    // https://broadworkbench.atlassian.net/browse/AN-734
+
+    containerContainer.orElse(dockerContainer)
+  }
+}
+
+/**
+ * Trait to handle validation of both 'docker' and 'container' runtime attributes, which are mutually exclusive
+ * ways of specifying the container image to use for a task.
+ */
+trait ContainersValidation extends OptionalRuntimeAttributesValidation[Containers] {
+  override def coercion: Set[WomType] = Containers.validWdlTypes
+
+  override def usedInCallCaching: Boolean = true
+
+  override protected def missingValueMessage: String = s"Can't find an attribute value for key ${key}"
+
+  override protected def invalidValueMessage(value: WomValue): String = super.missingValueMessage
+
+  override protected def validateOption: PartialFunction[WomValue, ErrorOr[Containers]] = {
+    case WomString(value) => value.validNel.map(v => Containers(v))
+    case WomArray(womType, values) if womType.memberType == WomStringType =>
+      Containers(values.map(_.valueString).toList).validNel
+  }
+}

backend/src/main/scala/cromwell/backend/validation/DockerValidation.scala

@@ -1,32 +1,23 @@
 package cromwell.backend.validation
 
-import cats.syntax.validated._
-import common.validation.ErrorOr.ErrorOr
 import wom.RuntimeAttributesKeys
-import wom.values._
 
 /**
-  * Validates the "docker" runtime attribute as a String, returning it as `String`.
-  *
-  * `instance` returns an validation that errors when no attribute is specified.
-  *
-  * There is no default, however `optional` can be used return the validated value as an `Option`, wrapped in a `Some`,
-  * if present, or `None` if not found.
+  * Different WDL versions support different names for the runtime attribute that specifies the container image to use.
+  * WDL 1.0 supports only `docker`, WDL 1.1 and later support `container` (preferred) and `docker` (deprecated).
   */
 object DockerValidation {
-  lazy val instance: RuntimeAttributesValidation[String] = new DockerValidation
-  lazy val optional: OptionalRuntimeAttributesValidation[String] = instance.optional
+  lazy val instance: OptionalRuntimeAttributesValidation[Containers] = new DockerValidation
 }
 
-class DockerValidation extends StringRuntimeAttributesValidation(RuntimeAttributesKeys.DockerKey) {
-  override def usedInCallCaching: Boolean = true
-
-  override protected def missingValueMessage: String = "Can't find an attribute value for key docker"
+class DockerValidation extends ContainersValidation {
+  override val key: String = RuntimeAttributesKeys.DockerKey
+}
 
-  override protected def invalidValueMessage(value: WomValue): String = super.missingValueMessage
+object ContainerValidation {
+  lazy val instance: OptionalRuntimeAttributesValidation[Containers] = new ContainerValidation
+}
 
-  // NOTE: Docker's current test specs don't like WdlInteger, etc. auto converted to WdlString.
-  override protected def validateValue: PartialFunction[WomValue, ErrorOr[String]] = { case WomString(value) =>
-    value.validNel
-  }
+class ContainerValidation extends ContainersValidation {
+  override val key: String = RuntimeAttributesKeys.ContainerKey
 }

backend/src/main/scala/cromwell/backend/validation/RuntimeAttributesValidation.scala

@@ -26,8 +26,15 @@ object RuntimeAttributesValidation {
     if (unrecognized.nonEmpty) logger.warn(s"Unrecognized runtime attribute keys: $unrecognized")
   }
 
-  def validateDocker(docker: Option[WomValue], onMissingKey: => ErrorOr[Option[String]]): ErrorOr[Option[String]] =
-    validateWithValidation(docker, DockerValidation.instance.optional, onMissingKey)
+  def validateDocker(docker: Option[WomValue],
+                     onMissingKey: => ErrorOr[Option[Containers]]
+  ): ErrorOr[Option[Containers]] =
+    validateWithValidation(docker, DockerValidation.instance, onMissingKey)
+
+  def validateContainer(container: Option[WomValue],
+                        onMissingKey: => ErrorOr[Option[Containers]]
+  ): ErrorOr[Option[Containers]] =
+    validateWithValidation(container, ContainerValidation.instance, onMissingKey)
 
   def validateFailOnStderr(value: Option[WomValue], onMissingKey: => ErrorOr[Boolean]): ErrorOr[Boolean] =
     validateWithValidation(value, FailOnStderrValidation.instance, onMissingKey)

backend/src/test/scala/cromwell/backend/standard/StandardValidatedRuntimeAttributesBuilderSpec.scala

@@ -53,13 +53,16 @@ class StandardValidatedRuntimeAttributesBuilderSpec
 
     "validate a valid Docker entry" in {
       val runtimeAttributes = Map("docker" -> WomString("ubuntu:latest"))
-      val expectedRuntimeAttributes = defaultRuntimeAttributes + (DockerKey -> Option("ubuntu:latest"))
+      val expectedRuntimeAttributes = defaultRuntimeAttributes + (DockerKey -> Option(Containers("ubuntu:latest")))
       assertRuntimeAttributesSuccessfulCreation(runtimeAttributes, expectedRuntimeAttributes)
     }
 
     "fail to validate an invalid Docker entry" in {
       val runtimeAttributes = Map("docker" -> WomInteger(1))
-      assertRuntimeAttributesFailedCreation(runtimeAttributes, "Expecting docker runtime attribute to be a String")
+      assertRuntimeAttributesFailedCreation(
+        runtimeAttributes,
+        "Expecting docker runtime attribute to be a type in Set(WomStringType, WomMaybeEmptyArrayType(WomStringType))"
+      )
     }
 
     "validate a valid failOnStderr entry" in {
@@ -170,7 +173,7 @@ class StandardValidatedRuntimeAttributesBuilderSpec
     val builder = if (includeDockerSupport) {
       StandardValidatedRuntimeAttributesBuilder
         .default(mockBackendRuntimeConfig)
-        .withValidation(DockerValidation.optional)
+        .withValidation(DockerValidation.instance)
     } else {
       StandardValidatedRuntimeAttributesBuilder.default(mockBackendRuntimeConfig)
     }
@@ -185,7 +188,7 @@ class StandardValidatedRuntimeAttributesBuilderSpec
     val continueOnReturnCode =
       RuntimeAttributesValidation.extract(ContinueOnReturnCodeValidation.instance, validatedRuntimeAttributes)
 
-    docker should be(expectedRuntimeAttributes(DockerKey).asInstanceOf[Option[String]])
+    docker should be(expectedRuntimeAttributes(DockerKey).asInstanceOf[Option[Containers]])
     failOnStderr should be(expectedRuntimeAttributes(FailOnStderrKey).asInstanceOf[Boolean])
     continueOnReturnCode should be(
       expectedRuntimeAttributes(ContinueOnReturnCodeKey)
@@ -203,7 +206,7 @@ class StandardValidatedRuntimeAttributesBuilderSpec
       val builder = if (supportsDocker) {
         StandardValidatedRuntimeAttributesBuilder
           .default(mockBackendRuntimeConfig)
-          .withValidation(DockerValidation.optional)
+          .withValidation(DockerValidation.instance)
       } else {
         StandardValidatedRuntimeAttributesBuilder.default(mockBackendRuntimeConfig)
       }

backend/src/test/scala/cromwell/backend/validation/RuntimeAttributesValidationSpec.scala

@@ -28,7 +28,7 @@ class RuntimeAttributesValidationSpec
         "Failed to get Docker mandatory key from runtime attributes".invalidNel
       )
       result match {
-        case Valid(x) => assert(x.get == "someImage")
+        case Valid(x) => assert(x.get.values.head == "someImage")
         case Invalid(e) => fail(e.toList.mkString(" "))
       }
     }
@@ -62,7 +62,35 @@ class RuntimeAttributesValidationSpec
       )
       result match {
         case Valid(_) => fail("A failure was expected.")
-        case Invalid(e) => assert(e.head == "Expecting docker runtime attribute to be a String")
+        case Invalid(e) =>
+          assert(
+            e.head == "Expecting docker runtime attribute to be a type in Set(WomStringType, WomMaybeEmptyArrayType(WomStringType))"
+          )
+      }
+    }
+
+    "return success when tries to validate a valid container entry" in {
+      val imageValue = Some(WomString("someImage"))
+      val result = RuntimeAttributesValidation.validateContainer(
+        imageValue,
+        "Failed to get container key from runtime attributes".invalidNel
+      )
+      result match {
+        case Valid(x) => assert(x.get.values == List("someImage"))
+        case Invalid(e) => fail(e.toList.mkString(" "))
+      }
+    }
+
+    "return success when tries to validate a valid container entry containing a list" in {
+      val imageValue =
+        Some(WomArray(WomArrayType(WomStringType), Seq(WomString("someImage"), WomString("someOtherImage"))))
+      val result = RuntimeAttributesValidation.validateContainer(
+        imageValue,
+        "Failed to get container key from runtime attributes".invalidNel
+      )
+      result match {
+        case Valid(x) => assert(x.get.values == Seq("someImage", "someOtherImage"))
+        case Invalid(e) => fail(e.toList.mkString(" "))
       }
     }
 

centaur/src/main/resources/standardTestCases/container_attr/container_attr_wdl10.test

@@ -0,0 +1,11 @@
+name: container_attr_wdl10
+testFormat: workflowsuccess
+
+files {
+  workflow: wdl/container_attr_wdl10.wdl
+}
+
+metadata {
+  "calls.container_attr_wdl10.dockerOnly.dockerImageUsed": "ubuntu@sha256:d0afa9fbcf16134b776fbba4a04c31d476eece2d080c66c887fdd2608e4219a9",
+  "calls.container_attr_wdl10.dockerAndContainer.dockerImageUsed": "ubuntu@sha256:d0afa9fbcf16134b776fbba4a04c31d476eece2d080c66c887fdd2608e4219a9"
+}