move Container.Docker.PortMapping to Container.PortMapping

Summary:
Port mappings are supported (by Mesos) for more than just Docker, so move PortMapping into Container (as a first step).

related to #4548

Test Plan: sbt test

Reviewers: nfnt, jasongilanfarr

Reviewed By: jasongilanfarr

Subscribers: jenkins, marathon-team

Differential Revision: https://phabricator.mesosphere.com/D123

src/main/scala/mesosphere/marathon/api/serialization/ContainerSerializer.scala

@@ -2,7 +2,7 @@ package mesosphere.marathon
 package api.serialization
 
 import mesosphere.marathon.core.externalvolume.ExternalVolumes
-import mesosphere.marathon.state.Container.Docker.PortMapping
+import mesosphere.marathon.state.Container.PortMapping
 import mesosphere.marathon.state._
 import mesosphere.marathon.stream._
 import org.apache.mesos
@@ -214,7 +214,7 @@ object LabelsSerializer {
 }
 
 object PortMappingSerializer {
-  def toProto(mapping: Container.Docker.PortMapping): Protos.ExtendedContainerInfo.DockerInfo.PortMapping = {
+  def toProto(mapping: Container.PortMapping): Protos.ExtendedContainerInfo.DockerInfo.PortMapping = {
     val builder = Protos.ExtendedContainerInfo.DockerInfo.PortMapping.newBuilder
       .setContainerPort(mapping.containerPort)
       .setProtocol(mapping.protocol)
@@ -237,7 +237,7 @@ object PortMappingSerializer {
       proto.getLabelsList.map { p => p.getKey -> p.getValue }(collection.breakOut)
     )
 
-  def toMesos(mapping: Container.Docker.PortMapping): Seq[mesos.Protos.ContainerInfo.DockerInfo.PortMapping] = {
+  def toMesos(mapping: Container.PortMapping): Seq[mesos.Protos.ContainerInfo.DockerInfo.PortMapping] = {
     def mesosPort(protocol: String, hostPort: Int) = {
       mesos.Protos.ContainerInfo.DockerInfo.PortMapping.newBuilder
         .setContainerPort (mapping.containerPort)

src/main/scala/mesosphere/marathon/api/v2/json/Formats.scala

@@ -223,14 +223,14 @@ trait ContainerFormats {
   implicit lazy val DockerNetworkFormat: Format[DockerInfo.Network] =
     enumFormat(DockerInfo.Network.valueOf, str => s"$str is not a valid network type")
 
-  implicit lazy val PortMappingFormat: Format[Container.Docker.PortMapping] = (
+  implicit lazy val PortMappingFormat: Format[Container.PortMapping] = (
     (__ \ "containerPort").formatNullable[Int].withDefault(AppDefinition.RandomPortValue) ~
     (__ \ "hostPort").formatNullable[Int] ~
     (__ \ "servicePort").formatNullable[Int].withDefault(AppDefinition.RandomPortValue) ~
     (__ \ "protocol").formatNullable[String].withDefault("tcp") ~
     (__ \ "name").formatNullable[String] ~
     (__ \ "labels").formatNullable[Map[String, String]].withDefault(Map.empty[String, String])
-  )(Container.Docker.PortMapping(_, _, _, _, _, _), unlift(Container.Docker.PortMapping.unapply))
+  )(Container.PortMapping(_, _, _, _, _, _), unlift(Container.PortMapping.unapply))
 
   implicit lazy val CredentialFormat: Format[Container.Credential] = (
     (__ \ "principal").format[String] ~
@@ -291,7 +291,7 @@ trait ContainerFormats {
     case class DockerContainerParameters(
       image: String,
       network: Option[ContainerInfo.DockerInfo.Network],
-      portMappings: Option[Seq[Container.Docker.PortMapping]],
+      portMappings: Option[Seq[Container.PortMapping]],
       privileged: Boolean,
       parameters: Seq[Parameter],
       credential: Option[Container.Credential],
@@ -300,7 +300,7 @@ trait ContainerFormats {
     implicit lazy val DockerContainerParametersFormat: Format[DockerContainerParameters] = (
       (__ \ "image").format[String] ~
       (__ \ "network").formatNullable[DockerInfo.Network] ~
-      (__ \ "portMappings").formatNullable[Seq[Container.Docker.PortMapping]] ~
+      (__ \ "portMappings").formatNullable[Seq[Container.PortMapping]] ~
       (__ \ "privileged").formatNullable[Boolean].withDefault(false) ~
       (__ \ "parameters").formatNullable[Seq[Parameter]].withDefault(Seq.empty) ~
       (__ \ "credential").formatNullable[Container.Credential] ~

src/main/scala/mesosphere/marathon/state/Container.scala

@@ -1,4 +1,5 @@
-package mesosphere.marathon.state
+package mesosphere.marathon
+package state
 
 import com.wix.accord.dsl._
 import com.wix.accord._
@@ -20,7 +21,7 @@ sealed trait Container {
   }
 
   // TODO(jdef): Someone should really fix this to not be Option[Seq[]] - we can't express that in protos anyways!
-  def portMappings: Option[Seq[Container.Docker.PortMapping]] = None
+  def portMappings: Option[Seq[Container.PortMapping]] = None
 
   def hostPorts: Option[Seq[Option[Int]]] =
     for (pms <- portMappings) yield pms.map(_.hostPort)
@@ -37,7 +38,7 @@ object Container {
     volumes: Seq[Volume] = Seq.empty,
     image: String = "",
     network: Option[ContainerInfo.DockerInfo.Network] = None,
-    override val portMappings: Option[Seq[Docker.PortMapping]] = None,
+    override val portMappings: Option[Seq[PortMapping]] = None,
     privileged: Boolean = false,
     parameters: Seq[Parameter] = Nil,
     forcePullImage: Boolean = false) extends Container
@@ -48,7 +49,7 @@ object Container {
       volumes: Seq[Volume],
       image: String = "",
       network: Option[ContainerInfo.DockerInfo.Network] = None,
-      portMappings: Option[Seq[Docker.PortMapping]] = None,
+      portMappings: Option[Seq[PortMapping]] = None,
       privileged: Boolean = false,
       parameters: Seq[Parameter] = Seq.empty,
       forcePullImage: Boolean = false): Docker = Docker(
@@ -69,63 +70,63 @@ object Container {
       parameters = parameters,
       forcePullImage = forcePullImage)
 
-    /**
-      * @param containerPort The container port to expose
-      * @param hostPort      The host port to bind
-      * @param servicePort   The well-known port for this service
-      * @param protocol      Layer 4 protocol to expose (i.e. "tcp", "udp" or "udp,tcp" for both).
-      * @param name          Name of the service hosted on this port.
-      * @param labels        This can be used to decorate the message with metadata to be
-      *                      interpreted by external applications such as firewalls.
-      */
-    case class PortMapping(
-      containerPort: Int = AppDefinition.RandomPortValue,
-      hostPort: Option[Int] = None, // defaults to HostPortDefault for BRIDGE mode, None for USER mode
-      servicePort: Int = AppDefinition.RandomPortValue,
-      protocol: String = "tcp",
-      name: Option[String] = None,
-      labels: Map[String, String] = Map.empty[String, String])
-
-    object PortMapping {
-      val TCP = "tcp"
-      val UDP = "udp"
-
-      val HostPortDefault = AppDefinition.RandomPortValue // HostPortDefault only applies when in BRIDGE mode
-
-      implicit val uniqueProtocols: Validator[Iterable[String]] =
-        isTrue[Iterable[String]]("protocols must be unique.") { protocols =>
-          protocols.size == protocols.toSet.size
-        }
+    val validDockerContainer = validator[Docker] { docker =>
+      docker.image is notEmpty
+      docker.portMappings is optional(PortMapping.portMappingsValidator and PortMapping.validForDocker(docker))
+    }
+  }
 
-      implicit val portMappingValidator = validator[PortMapping] { portMapping =>
-        portMapping.protocol.split(',').toIterable is uniqueProtocols and every(oneOf(TCP, UDP))
-        portMapping.containerPort should be >= 0
-        portMapping.hostPort.each should be >= 0
-        portMapping.servicePort should be >= 0
-        portMapping.name is optional(matchRegexFully(PortAssignment.PortNamePattern))
+  /**
+    * @param containerPort The container port to expose
+    * @param hostPort      The host port to bind
+    * @param servicePort   The well-known port for this service
+    * @param protocol      Layer 4 protocol to expose (i.e. "tcp", "udp" or "udp,tcp" for both).
+    * @param name          Name of the service hosted on this port.
+    * @param labels        This can be used to decorate the message with metadata to be
+    *                      interpreted by external applications such as firewalls.
+    */
+  case class PortMapping(
+    containerPort: Int = AppDefinition.RandomPortValue,
+    hostPort: Option[Int] = None, // defaults to HostPortDefault for BRIDGE mode, None for USER mode
+    servicePort: Int = AppDefinition.RandomPortValue,
+    protocol: String = "tcp",
+    name: Option[String] = None,
+    labels: Map[String, String] = Map.empty[String, String])
+
+  object PortMapping {
+    val TCP = "tcp"
+    val UDP = "udp"
+
+    val HostPortDefault = AppDefinition.RandomPortValue // HostPortDefault only applies when in BRIDGE mode
+
+    implicit val uniqueProtocols: Validator[Iterable[String]] =
+      isTrue[Iterable[String]]("protocols must be unique.") { protocols =>
+        protocols.size == protocols.toSet.size
       }
 
-      def networkHostPortValidator(docker: Docker): Validator[PortMapping] =
-        isTrue[PortMapping]("hostPort is required for BRIDGE mode.") { pm =>
-          docker.network match {
-            case Some(ContainerInfo.DockerInfo.Network.BRIDGE) => pm.hostPort.isDefined
-            case _ => true
-          }
-        }
+    implicit val portMappingValidator = validator[PortMapping] { portMapping =>
+      portMapping.protocol.split(',').toIterable is uniqueProtocols and every(oneOf(TCP, UDP))
+      portMapping.containerPort should be >= 0
+      portMapping.hostPort.each should be >= 0
+      portMapping.servicePort should be >= 0
+      portMapping.name is optional(matchRegexFully(PortAssignment.PortNamePattern))
+    }
 
-      val portMappingsValidator = validator[Seq[PortMapping]] { portMappings =>
-        portMappings is every(valid)
-        portMappings is elementsAreUniqueByOptional(_.name, "Port names must be unique.")
+    def networkHostPortValidator(docker: Docker): Validator[PortMapping] =
+      isTrue[PortMapping]("hostPort is required for BRIDGE mode.") { pm =>
+        docker.network match {
+          case Some(ContainerInfo.DockerInfo.Network.BRIDGE) => pm.hostPort.isDefined
+          case _ => true
+        }
       }
 
-      def validForDocker(docker: Docker): Validator[Seq[PortMapping]] = validator[Seq[PortMapping]] { pm =>
-        pm is every(valid(PortMapping.networkHostPortValidator(docker)))
-      }
+    val portMappingsValidator = validator[Seq[PortMapping]] { portMappings =>
+      portMappings is every(valid)
+      portMappings is elementsAreUniqueByOptional(_.name, "Port names must be unique.")
     }
 
-    val validDockerContainer = validator[Docker] { docker =>
-      docker.image is notEmpty
-      docker.portMappings is optional(PortMapping.portMappingsValidator and PortMapping.validForDocker(docker))
+    def validForDocker(docker: Docker): Validator[Seq[PortMapping]] = validator[Seq[PortMapping]] { pm =>
+      pm is every(valid(PortMapping.networkHostPortValidator(docker)))
     }
   }
 

src/main/scala/mesosphere/marathon/tasks/PortsMatcher.scala

@@ -51,7 +51,7 @@ class PortsMatcher private[tasks] (
 
         mappedPortRanges(requiredPorts)
       case app: AppDefinition =>
-        val portMappings: Option[Seq[Container.Docker.PortMapping]] =
+        val portMappings: Option[Seq[Container.PortMapping]] =
           for {
             c <- app.container
             pms <- c.portMappings if pms.nonEmpty

src/test/scala/mesosphere/marathon/api/v2/AppsResourceTest.scala

@@ -1,4 +1,5 @@
-package mesosphere.marathon.api.v2
+package mesosphere.marathon
+package api.v2
 
 import java.util
 import javax.ws.rs.core.Response
@@ -319,7 +320,7 @@ class AppsResourceTest extends MarathonSpec with MarathonActorSupport with Match
         network = Some(Mesos.ContainerInfo.DockerInfo.Network.USER),
         image = "jdef/helpme",
         portMappings = Some(Seq(
-          Container.Docker.PortMapping(containerPort = 0, protocol = "tcp")
+          Container.PortMapping(containerPort = 0, protocol = "tcp")
         ))
       )),
       portDefinitions = Seq.empty
@@ -351,7 +352,7 @@ class AppsResourceTest extends MarathonSpec with MarathonActorSupport with Match
       network = Some(Mesos.ContainerInfo.DockerInfo.Network.BRIDGE),
       image = "jdef/helpme",
       portMappings = Some(Seq(
-        Container.Docker.PortMapping(containerPort = 0, protocol = "tcp")
+        Container.PortMapping(containerPort = 0, protocol = "tcp")
       ))
     )
 
@@ -384,7 +385,7 @@ class AppsResourceTest extends MarathonSpec with MarathonActorSupport with Match
         versionInfo = VersionInfo.OnlyVersion(clock.now()),
         container = Some(container.copy(
           portMappings = Some(Seq(
-            Container.Docker.PortMapping(containerPort = 0, hostPort = Some(0), protocol = "tcp")
+            Container.PortMapping(containerPort = 0, hostPort = Some(0), protocol = "tcp")
           ))
         ))
       ),
@@ -410,7 +411,7 @@ class AppsResourceTest extends MarathonSpec with MarathonActorSupport with Match
         network = Some(Mesos.ContainerInfo.DockerInfo.Network.USER),
         image = "jdef/helpme",
         portMappings = Some(Seq(
-          Container.Docker.PortMapping(containerPort = 0, protocol = "tcp")
+          Container.PortMapping(containerPort = 0, protocol = "tcp")
         ))
       )
       ),

src/test/scala/mesosphere/marathon/api/v2/ModelValidationTest.scala

@@ -1,10 +1,10 @@
-package mesosphere.marathon.api.v2
+package mesosphere.marathon
+package api.v2
 
 import com.wix.accord._
 import com.wix.accord.dsl._
 import mesosphere.marathon.api.v2.Validation._
 import mesosphere.marathon.api.v2.json.GroupUpdate
-import mesosphere.marathon.state.Container.Docker.PortMapping
 import mesosphere.marathon.state.Container._
 import mesosphere.marathon.state.PathId._
 import mesosphere.marathon.state._

src/test/scala/mesosphere/marathon/api/v2/json/AppDefinitionFormatsTest.scala

@@ -1,4 +1,5 @@
-package mesosphere.marathon.api.v2.json
+package mesosphere.marathon
+package api.v2.json
 
 import mesosphere.marathon.Protos
 import mesosphere.marathon.Protos.Constraint
@@ -329,7 +330,7 @@ class AppDefinitionFormatsTest
     appDef.container.get shouldBe a[Container.Docker]
     appDef.container.flatMap(_.docker.flatMap(_.network.map(_.toString))) should equal (Some("USER"))
     appDef.container.flatMap(_.portMappings) should equal (Some(Seq(
-      Container.Docker.PortMapping(containerPort = 123, servicePort = 80, name = Some("foobar"))
+      Container.PortMapping(containerPort = 123, servicePort = 80, name = Some("foobar"))
     )))
   }
 

src/test/scala/mesosphere/marathon/api/v2/json/AppDefinitionTest.scala

@@ -1,4 +1,5 @@
-package mesosphere.marathon.api.v2.json
+package mesosphere.marathon
+package api.v2.json
 
 import com.wix.accord._
 import mesosphere.marathon.Protos
@@ -10,6 +11,7 @@ import mesosphere.marathon.core.plugin.PluginManager
 import mesosphere.marathon.core.readiness.ReadinessCheckTestHelper
 import mesosphere.marathon.raml.Resources
 import mesosphere.marathon.state.Container.Docker
+import mesosphere.marathon.state.Container.PortMapping
 import mesosphere.marathon.state.DiscoveryInfo.Port
 import mesosphere.marathon.state.EnvVarValue._
 import mesosphere.marathon.state.PathId._
@@ -108,8 +110,8 @@ class AppDefinitionTest extends MarathonSpec with Matchers {
         image = "mesosphere/marathon",
         network = Some(mesos.ContainerInfo.DockerInfo.Network.BRIDGE),
         portMappings = Some(Seq(
-          Docker.PortMapping(8080, Some(0), 0, "tcp", Some("foo")),
-          Docker.PortMapping(8081, Some(0), 0, "tcp", Some("foo"))
+          PortMapping(8080, Some(0), 0, "tcp", Some("foo")),
+          PortMapping(8081, Some(0), 0, "tcp", Some("foo"))
         ))
       )),
       portDefinitions = Nil
@@ -140,8 +142,8 @@ class AppDefinitionTest extends MarathonSpec with Matchers {
       container = Some(Docker(
         image = "mesosphere/marathon",
         portMappings = Some(Seq(
-          Docker.PortMapping(8080, Some(0), 0, "tcp", Some("foo")),
-          Docker.PortMapping(8081, Some(0), 0, "tcp", Some("bar"))
+          PortMapping(8080, Some(0), 0, "tcp", Some("foo")),
+          PortMapping(8081, Some(0), 0, "tcp", Some("bar"))
         ))
       )),
       portDefinitions = Nil)
@@ -156,7 +158,7 @@ class AppDefinitionTest extends MarathonSpec with Matchers {
         image = "mesosphere/marathon",
         network = Some(mesos.ContainerInfo.DockerInfo.Network.USER),
         portMappings = Some(Seq(
-          Docker.PortMapping(8080, None, 0, "tcp", Some("foo"))
+          PortMapping(8080, None, 0, "tcp", Some("foo"))
         ))
       )),
       portDefinitions = Nil)
@@ -171,7 +173,7 @@ class AppDefinitionTest extends MarathonSpec with Matchers {
         image = "mesosphere/marathon",
         network = Some(mesos.ContainerInfo.DockerInfo.Network.BRIDGE),
         portMappings = Some(Seq(
-          Docker.PortMapping(8080, None, 0, "tcp", Some("foo"))
+          PortMapping(8080, None, 0, "tcp", Some("foo"))
         ))
       )),
       portDefinitions = Nil)
@@ -186,8 +188,8 @@ class AppDefinitionTest extends MarathonSpec with Matchers {
         image = "mesosphere/marathon",
         network = Some(mesos.ContainerInfo.DockerInfo.Network.USER),
         portMappings = Some(Seq(
-          Docker.PortMapping(8080, Some(0), 0, "tcp", Some("foo")),
-          Docker.PortMapping(8081, Some(0), 0, "tcp", Some("bar"))
+          PortMapping(8080, Some(0), 0, "tcp", Some("foo")),
+          PortMapping(8081, Some(0), 0, "tcp", Some("bar"))
         ))
       )),
       portDefinitions = Nil)
@@ -203,8 +205,8 @@ class AppDefinitionTest extends MarathonSpec with Matchers {
         image = "mesosphere/marathon",
         network = Some(mesos.ContainerInfo.DockerInfo.Network.USER),
         portMappings = Some(Seq(
-          Docker.PortMapping(8080, Some(0), 0, "tcp", Some("foo")),
-          Docker.PortMapping(8081, Some(0), 0, "tcp", Some("foo"))
+          PortMapping(8080, Some(0), 0, "tcp", Some("foo")),
+          PortMapping(8081, Some(0), 0, "tcp", Some("foo"))
         ))
       )),
       portDefinitions = Nil)
@@ -326,8 +328,8 @@ class AppDefinitionTest extends MarathonSpec with Matchers {
       container = Some(Docker(
         network = Some(mesos.ContainerInfo.DockerInfo.Network.BRIDGE),
         portMappings = Some(Seq(
-          Docker.PortMapping(8080, Some(0), 0, "tcp"),
-          Docker.PortMapping(8081, Some(0), 0, "tcp")
+          PortMapping(8080, Some(0), 0, "tcp"),
+          PortMapping(8081, Some(0), 0, "tcp")
         ))
       )),
       portDefinitions = Nil,
@@ -645,7 +647,6 @@ class AppDefinitionTest extends MarathonSpec with Matchers {
   }
 
   test("Read app with container definition and port mappings") {
-    import mesosphere.marathon.state.Container.Docker.PortMapping
     import org.apache.mesos.Protos.ContainerInfo.DockerInfo.Network
 
     val app4 = AppDefinition(