Fixes d2iq-archive#2865 - Preserve app definition ports order

jdubs · Dec 21, 2015 · 5fc17f7 · 5fc17f7
1 parent d7ef8e0
commit 5fc17f7
Show file tree

Hide file tree

Showing 3 changed files with 45 additions and 1 deletion.
diff --git a/src/main/scala/mesosphere/marathon/tasks/PortsMatcher.scala b/src/main/scala/mesosphere/marathon/tasks/PortsMatcher.scala
@@ -74,7 +74,7 @@ class PortsMatcher(
     */
   private[this] def findPortsInOffer(requiredPorts: Seq[Integer], failLog: Boolean): Option[Seq[PortWithRole]] = {
     takeEnoughPortsOrNone(expectedSize = requiredPorts.size) {
-      requiredPorts.sorted.iterator.map { (port: Integer) =>
+      requiredPorts.iterator.map { (port: Integer) =>
         offeredPortRanges.find(_.contains(port)).map { offeredRange =>
           PortWithRole(offeredRange.role, port)
         } orElse {

diff --git a/src/test/scala/mesosphere/marathon/tasks/PortsMatcherTest.scala b/src/test/scala/mesosphere/marathon/tasks/PortsMatcherTest.scala
@@ -66,6 +66,16 @@ class PortsMatcherTest extends MarathonSpec {
     assert(matcher.portRanges.get.map(_.role) == Seq("*"))
   }
 
+  // #2865 Multiple explicit ports are mixed up in task json
+  test("get ports with requirePorts preserves the ports order") {
+    val app = AppDefinition(ports = Seq(100, 80), requirePorts = true)
+    val offer = makeBasicOffer(beginPort = 70, endPort = 200).build
+    val matcher = new PortsMatcher(app, offer, acceptedResourceRoles = Set("*"))
+
+    assert(matcher.matches)
+    assert(matcher.ports == Seq(100, 80))
+  }
+
   test("get ports from multiple resources, preserving role") {
     val app = AppDefinition(ports = Seq(80, 81, 82, 83, 84))
     val portsResource = RangesResource(

diff --git a/src/test/scala/mesosphere/mesos/TaskBuilderTest.scala b/src/test/scala/mesosphere/mesos/TaskBuilderTest.scala
@@ -1056,6 +1056,40 @@ class TaskBuilderTest extends MarathonSpec with Matchers {
 
   }
 
+  // #2865 Multiple explicit ports are mixed up in task json
+  test("build with requirePorts preserves the port order") {
+    val offer = makeBasicOffer(cpus = 2.0, mem = 128.0, disk = 2000.0, beginPort = 25000, endPort = 26000).build
+
+    val task: Option[(MesosProtos.TaskInfo, Seq[Long])] = buildIfMatches(
+      offer,
+      AppDefinition(
+        id = "/product/frontend".toPath,
+        cmd = Some("foo"),
+        ports = Seq(25552, 25551),
+        requirePorts = true
+      )
+    )
+
+    val Some((taskInfo, _)) = task
+
+    val env: Map[String, String] =
+      taskInfo.getCommand.getEnvironment.getVariablesList.asScala.toList.map(v => v.getName -> v.getValue).toMap
+
+    assert("25552" == env("PORT0"))
+    assert("25552" == env("PORT_25552"))
+    assert("25551" == env("PORT1"))
+    assert("25551" == env("PORT_25551"))
+
+    val portsFromTaskInfo = {
+      val asScalaRanges = for {
+        resource <- taskInfo.getResourcesList.asScala if resource.getName == Resource.PORTS
+        range <- resource.getRanges.getRangeList.asScala
+      } yield range.getBegin to range.getEnd
+      asScalaRanges.flatMap(_.iterator).toList
+    }
+    assert(portsFromTaskInfo == Seq(25552, 25551))
+  }
+
   def buildIfMatches(
     offer: Offer,
     app: AppDefinition,