| zen-dog | |
| timcharper | |
| jdef | |
| jenkins | |
| jeschkies | |
| kensipe |
In the current implementation it is possible to configure a bridged container network with container/bridge, but when building the TaskInfo's for pods these networks are ignored and not passed to mesos. This fix changes the behavior to enable bridged networks for pods.
Deferred work:
sbt test
| Automatic diff as part of commit; lint not applicable. |
| Automatic diff as part of commit; unit tests not applicable. |
REBASED
✔ Build of 3388 completed jenkins-public-marathon-phabricator-146.
You can create a DC/OS with your patched Marathon by creating a new pull
request with the following changes in buildinfo.json:
"url": "https://downloads.mesosphere.io/marathon/snapshots/marathon-1.5.0-SNAPSHOT-553-g224fc0b.tgz", "sha1"" "6646b0974ddc48d065113322631ba74b4f5b007f"
| Commit | Tree | Parents | Author | Summary | Date |
|---|---|---|---|---|---|
| a985158a5cba | ec0ec4b7aaee | d80a48103f31 | Johannes Unterstein | comments | May 29 2017, 2:46 PM |
| d80a48103f31 | 34a671bc5927 | 2ec786629c52 | Johannes Unterstein | adapt feedback | May 29 2017, 12:52 PM |
| 2ec786629c52 | aebdd97e06d0 | 6fe14fc80ec6 | Johannes Unterstein | updated test | May 24 2017, 12:47 PM |
| 6fe14fc80ec6 | 3d54a9b9dfdc | 0122ed7d391f | Aleksey Dukhovniy | Using mesos_bridge_name for bridge networking | May 18 2017, 12:30 PM |
| 0122ed7d391f | 72ab07401d38 | 79e6f912873c | Johannes Unterstein | test for pod bridge networking | May 18 2017, 11:13 AM |
| 79e6f912873c | d4af479d2cc3 | e894da8501fd | Aleksey Dukhovniy | First draft of port mappings for contrainer/bridge network for pods | May 18 2017, 10:41 AM |
| e894da8501fd | 23d1df873ce5 | 224fc0ba2f51 | Johannes Unterstein | added tests | May 17 2017, 4:54 PM |
| Show First 20 Lines • Show All 79 Lines • ▼ Show 20 Line(s) | ||||
| 80 | 80 | def endpointValidator(networks: Seq[Network]) = { | ||
|---|---|---|---|---|
| 81 | 81 | val networkNamess = networks.flatMap(_.name) | ||
| 82 | 82 | val hostPortRequiresNetworkName = isTrue[Endpoint](NetworkNameRequiredForMultipleContainerNetworks) { endpoint => | ||
| 83 | 83 | endpoint.hostPort.isEmpty || endpoint.networkNames.length == 1 | ||
| 84 | 84 | } | ||
| 85 | 85 | |||
| 86 | 86 | val normalValidation = validator[Endpoint] { endpoint => | ||
| 87 | 87 | endpoint.networkNames is every(oneOf(networkNamess: _*)) | ||
| 88 | 88 | |||
| 89 | // host-mode networking implies that hostPort is required | |||
| 89 | // host-mode and container-bridge networking implies that hostPort is required | |||
| 90 | endpoint.hostPort is isTrue("is required when using host-mode networking") { hp => | |||
| 91 | if (networks.exists(_.mode == NetworkMode.Host)) hp.nonEmpty | |||
| 90 | endpoint.hostPort is isTrue("is required when using host-mode or container-bridge networking") { hp => | |||
| 91 | if (networks.exists(n => n.mode == NetworkMode.Host || n.mode == NetworkMode.ContainerBridge)) hp.nonEmpty | |||
| 92 | 92 | else true | ||
| 93 | 93 | } | ||
| 94 | 94 | |||
| 95 | 95 | // host-mode networking implies that containerPort is disallowed | ||
| 96 | 96 | endpoint.containerPort is isTrue("is not allowed when using host-mode networking") { cp => | ||
| 97 | 97 | if (networks.exists(_.mode == NetworkMode.Host)) cp.isEmpty | ||
| 98 | 98 | else true | ||
| 99 | 99 | } | ||
| ▲ Show 20 Lines • Show All 132 Lines • Show Last 20 Lines | ||||
| Show First 20 Lines • Show All 56 Lines • ▼ Show 20 Line(s) | |||
| 57 | 57 | case _ => | |
|---|---|---|---|
| 58 | 58 | throw new IllegalArgumentException(s"unsupported runSpec object ${request.runSpec}") | |
| 59 | 59 | } | |
| 60 | 60 | } | |
| 61 | 61 | ||
| 62 | 62 | protected def inferPodInstanceOp(request: InstanceOpFactory.Request, pod: PodDefinition): OfferMatchResult = { | |
| 63 | 63 | val builderConfig = TaskGroupBuilder.BuilderConfig( | |
| 64 | 64 | config.defaultAcceptedResourceRolesSet, | |
| 65 | config.envVarsPrefix.get) | ||
| 65 | config.envVarsPrefix.get, | ||
| 66 | config.mesosBridgeName()) | ||
| 66 | 67 | ||
| 67 | 68 | val matchedOffer = | |
| 68 | 69 | RunSpecOfferMatcher.matchOffer(pod, request.offer, request.instances, builderConfig.acceptedResourceRoles) | |
| 69 | 70 | ||
| 70 | 71 | matchedOffer match { | |
| 71 | 72 | case matches: ResourceMatchResponse.Match => | |
| 72 | 73 | val (executorInfo, groupInfo, hostPorts, instanceId) = TaskGroupBuilder.build(pod, request.offer, | |
| 73 | 74 | Instance.Id.forRunSpec, builderConfig, runSpecTaskProc, matches.resourceMatch) | |
| ▲ Show 20 Lines • Show All 257 Lines • Show Last 20 Lines | |||
| Show All 21 Lines | |||
| 22 | 22 | // Given that Docker only works under linux with 64bit, | |
|---|---|---|---|
| 23 | 23 | // let's (for now) set these values to reflect that. | |
| 24 | 24 | protected[mesos] val LinuxAmd64 = Map("os" -> "linux", "arch" -> "amd64") | |
| 25 | 25 | ||
| 26 | 26 | private val ephemeralVolPathPrefix = "volumes/" | |
| 27 | 27 | ||
| 28 | 28 | case class BuilderConfig( | |
| 29 | 29 | acceptedResourceRoles: Set[String], | |
| 30 | envVarsPrefix: Option[String]) | ||
| 30 | envVarsPrefix: Option[String], | ||
| 31 | mesosBridgeName: String) | ||
| 31 | 32 | ||
| 32 | 33 | def build( | |
| 33 | 34 | podDefinition: PodDefinition, | |
| 34 | 35 | offer: mesos.Offer, | |
| 35 | 36 | newInstanceId: PathId => Instance.Id, | |
| 36 | 37 | config: BuilderConfig, | |
| 37 | 38 | runSpecTaskProcessor: RunSpecTaskProcessor, | |
| 38 | 39 | resourceMatch: ResourceMatcher.ResourceMatch | |
| 39 | 40 | ): (mesos.ExecutorInfo, mesos.TaskGroupInfo, Seq[Option[Int]], Instance.Id) = { | |
| 40 | 41 | val instanceId = newInstanceId(podDefinition.id) | |
| 41 | 42 | ||
| 42 | 43 | val allEndpoints = podDefinition.containers.flatMap(_.endpoints) | |
| 43 | 44 | ||
| 44 | val mesosNetworks = buildMesosNetworks(podDefinition.networks, allEndpoints, resourceMatch.hostPorts) | ||
| 45 | val mesosNetworks = buildMesosNetworks(podDefinition.networks, allEndpoints, resourceMatch.hostPorts, config.mesosBridgeName) | ||
| 45 | 46 | ||
| 46 | 47 | val executorInfo = computeExecutorInfo( | |
| 47 | 48 | podDefinition, | |
| 48 | 49 | resourceMatch.portsMatch, | |
| 49 | 50 | mesosNetworks, | |
| 50 | 51 | instanceId, | |
| 51 | 52 | offer.getFrameworkId) | |
| 52 | 53 | ||
| Show All 23 Lines | |||
| 76 | 77 | } | |
| 77 | 78 | ||
| 78 | 79 | // The resource match provides us with a list of host ports. | |
| 79 | 80 | // Each port mapping corresponds to an item in that list. | |
| 80 | 81 | // We use that list to swap the dynamic ports (ports set to 0) with the matched ones. | |
| 81 | 82 | private[mesos] def buildMesosNetworks( | |
| 82 | 83 | networks: Seq[Network], | |
| 83 | 84 | endpoints: Seq[raml.Endpoint], | |
| 84 | hostPorts: Seq[Option[Int]]): Seq[mesos.NetworkInfo] = { | ||
| 85 | hostPorts: Seq[Option[Int]], | ||
| 86 | mesosBridgeName: String): Seq[mesos.NetworkInfo] = { | ||
| 85 | 87 | ||
| 86 | 88 | assume( | |
| 87 | 89 | endpoints.size == hostPorts.size, | |
| 88 | 90 | "expected total number of endpoints to match number of optional host ports") | |
| 89 | 91 | val portMappings = endpoints.iterator | |
| 90 | 92 | .zip(hostPorts.iterator) | |
| 91 | 93 | .collect { | |
| 92 | 94 | case (endpoint, Some(hostPort)) => | |
| Show All 27 Lines | |||
| 120 | 122 | .setLabels(containerNetwork.labels.toMesosLabels) | |
| 121 | 123 | portMappings.foreach { | |
| 122 | 124 | case (networkNames, pm) => | |
| 123 | 125 | // if networkNames is empty, then it means associate with all container networks | |
| 124 | 126 | if (networkNames.forall(_ == containerNetwork.name)) | |
| 125 | 127 | b.addPortMappings(pm) | |
| 126 | 128 | } | |
| 127 | 129 | b.build | |
| 130 | case bridgeNetwork: BridgeNetwork => | ||
| 131 | val b = mesos.NetworkInfo.newBuilder | ||
| 132 | .setName(mesosBridgeName) | ||
| 133 | .setLabels(bridgeNetwork.labels.toMesosLabels) | ||
| 134 | portMappings.foreach{ case (_, pm) => b.addPortMappings(pm) } | ||
| 135 | b.build() | ||
| 128 | 136 | } | |
| 129 | 137 | } | |
| 130 | 138 | ||
| 131 | 139 | private[this] def computeTaskInfo( | |
| 132 | 140 | container: MesosContainer, | |
| 133 | 141 | podDefinition: PodDefinition, | |
| 134 | 142 | offer: mesos.Offer, | |
| 135 | 143 | instanceId: Instance.Id, | |
| ▲ Show 20 Lines • Show All 331 Lines • Show Last 20 Lines | |||
| Show All 39 Lines | ||||
| 40 | 40 | val podSpecJson = """ | ||
|---|---|---|---|---|
| 41 | 41 | | { "id": "/mypod", "networks": [ { "mode": "host" } ], "containers": [ | ||
| 42 | 42 | | { "name": "webapp", | ||
| 43 | 43 | | "resources": { "cpus": 0.03, "mem": 64 }, | ||
| 44 | 44 | | "image": { "kind": "DOCKER", "id": "busybox" }, | ||
| 45 | 45 | | "exec": { "command": { "shell": "sleep 1" } } } ] } | ||
| 46 | 46 | """.stripMargin | ||
| 47 | 47 | |||
| 48 | val podSpecJsonWithBridgeNetwork = """ | |||
| 49 | | { "id": "/mypod", "networks": [ { "mode": "container/bridge" } ], "containers": [ | |||
| 50 | | { "name": "webapp", | |||
| 51 | | "resources": { "cpus": 0.03, "mem": 64 }, | |||
| 52 | | "image": { "kind": "DOCKER", "id": "busybox" }, | |||
| 53 | | "exec": { "command": { "shell": "sleep 1" } } } ] } | |||
| 54 | """.stripMargin | |||
| 55 | ||||
| 48 | 56 | val podSpecJsonWithContainerNetworking = """ | ||
| 49 | 57 | | { "id": "/mypod", "networks": [ { "mode": "container" } ], "containers": [ | ||
| 50 | 58 | | { "name": "webapp", | ||
| 51 | 59 | | "resources": { "cpus": 0.03, "mem": 64 }, | ||
| 52 | 60 | | "image": { "kind": "DOCKER", "id": "busybox" }, | ||
| 53 | 61 | | "exec": { "command": { "shell": "sleep 1" } } } ] } | ||
| 54 | 62 | """.stripMargin | ||
| 55 | 63 | |||
| Show All 28 Lines | ||||
| 84 | 92 | response.getStatus should be(HttpServletResponse.SC_CREATED) | ||
| 85 | 93 | |||
| 86 | 94 | val parsedResponse = Option(response.getEntity.asInstanceOf[String]).map(Json.parse) | ||
| 87 | 95 | parsedResponse should be (defined) | ||
| 88 | 96 | val maybePod = parsedResponse.map(_.as[Pod]) | ||
| 89 | 97 | maybePod should be (defined) // validate that we DID get back a pod definition | ||
| 90 | 98 | val pod = maybePod.get | ||
| 91 | 99 | pod.networks(0).mode should be (NetworkMode.Host) | ||
| 100 | pod.networks(0).name should not be (defined) | |||
| 101 | pod.executorResources should be (defined) // validate that executor resources are defined | |||
| 102 | pod.executorResources.get should be (ExecutorResources()) // validate that the executor resources has default values | |||
| 103 | ||||
| 104 | response.getMetadata.containsKey(RestResource.DeploymentHeader) should be(true) | |||
| 105 | } | |||
| 106 | } | |||
| 107 | ||||
| 108 | "be able to create a simple single-container pod with bride network" in { | |||
| 109 | implicit val podSystem = mock[PodManager] | |||
| 110 | val f = Fixture(configArgs = Seq("--default_network_name", "blah")) // should not be injected into host network spec | |||
| 111 | ||||
| 112 | podSystem.create(any, eq(false)).returns(Future.successful(DeploymentPlan.empty)) | |||
| 113 | ||||
| 114 | val response = f.podsResource.create(podSpecJsonWithBridgeNetwork.getBytes(), force = false, f.auth.request) | |||
| 115 | ||||
| 116 | withClue(s"response body: ${response.getEntity}") { | |||
| 117 | response.getStatus should be(HttpServletResponse.SC_CREATED) | |||
| 118 | ||||
| 119 | val parsedResponse = Option(response.getEntity.asInstanceOf[String]).map(Json.parse) | |||
| 120 | parsedResponse should be (defined) | |||
| 121 | val maybePod = parsedResponse.map(_.as[Pod]) | |||
| 122 | maybePod should be (defined) // validate that we DID get back a pod definition | |||
| 123 | val pod = maybePod.get | |||
| 124 | pod.networks(0).mode should be (NetworkMode.ContainerBridge) | |||
| 92 | 125 | pod.networks(0).name should not be (defined) | ||
| 93 | 126 | pod.executorResources should be (defined) // validate that executor resources are defined | ||
| 94 | 127 | pod.executorResources.get should be (ExecutorResources()) // validate that the executor resources has default values | ||
| 95 | 128 | |||
| 96 | 129 | response.getMetadata.containsKey(RestResource.DeploymentHeader) should be(true) | ||
| 97 | 130 | } | ||
| 98 | 131 | } | ||
| 99 | 132 | |||
| ▲ Show 20 Lines • Show All 541 Lines • Show Last 20 Lines | ||||
| Show First 20 Lines • Show All 121 Lines • ▼ Show 20 Line(s) | ||||
| 122 | 122 | |||
|---|---|---|---|---|
| 123 | 123 | "allow portMappings that don't declare hostPort nor networkNames" in { | ||
| 124 | 124 | val app = containerNetworkedPod( | ||
| 125 | 125 | Seq(podContainer(endpoints = Seq(Endpoint("endpoint", containerPort = Some(80))))), | ||
| 126 | 126 | networkCount = 2) | ||
| 127 | 127 | validator(app) shouldBe (aSuccess) | ||
| 128 | 128 | } | ||
| 129 | 129 | |||
| 130 | "allow portMappings for pods with bridge networking" in { | |||
| 131 | val pod = Pod( | |||
| 132 | id = "/bridge", | |||
| 133 | networks = Seq(Network(mode = NetworkMode.ContainerBridge)), | |||
| 134 | containers = Seq(podContainer(endpoints = Seq(Endpoint("endpoint", hostPort = Some(0), containerPort = Some(80))))) | |||
since hostPort normalization was extracted from this PR, we need a validation rule that requires non-empty host port for bridge mode. and a corresponding unit test. related validation code that requires hostPort for host mode is in PodsValidation: // host-mode networking implies that hostPort is required
endpoint.hostPort is isTrue("is required when using host-mode networking") { hp =>
if (networks.exists(_.mode == NetworkMode.Host)) hp.nonEmpty
else true
} | ||||
| 135 | ) | |||
| 136 | ||||
| 137 | validator(pod) shouldBe (aSuccess) | |||
| 138 | } | |||
| 139 | ||||
| 140 | "fail if no hostPort is defined in bridge networking" in { | |||
| 141 | val pod = Pod( | |||
| 142 | id = "/bridge", | |||
| 143 | networks = Seq(Network(mode = NetworkMode.ContainerBridge)), | |||
| 144 | containers = Seq(podContainer(endpoints = Seq(Endpoint("endpoint", containerPort = Some(80))))) | |||
| 145 | ) | |||
| 146 | ||||
| 147 | val result = validator(pod) | |||
| 148 | result shouldBe (aFailure) | |||
| 149 | result.toString should include("is required when using host-mode or container-bridge networking") | |||
| 150 | } | |||
| 151 | ||||
| 130 | 152 | "allow portMappings that both declare a hostPort and a networkNames" in { | ||
| 131 | 153 | val app = containerNetworkedPod( | ||
| 132 | 154 | Seq(podContainer(endpoints = Seq( | ||
| 133 | 155 | Endpoint( | ||
| 134 | 156 | "endpoint", | ||
| 135 | 157 | hostPort = Option(0), | ||
| 136 | 158 | containerPort = Some(80), | ||
| 137 | 159 | networkNames = List("1"))))), | ||
| ▲ Show 20 Lines • Show All 105 Lines • Show Last 20 Lines | ||||
| Show All 18 Lines | |||
| 19 | 19 | ||
|---|---|---|---|
| 20 | 20 | import scala.collection.immutable.Seq | |
| 21 | 21 | import scala.collection.JavaConverters._ | |
| 22 | 22 | import scala.collection.breakOut | |
| 23 | 23 | ||
| 24 | 24 | class TaskGroupBuilderTest extends UnitTest with Inside { | |
| 25 | 25 | val defaultBuilderConfig = TaskGroupBuilder.BuilderConfig( | |
| 26 | 26 | acceptedResourceRoles = Set(ResourceRole.Unreserved), | |
| 27 | envVarsPrefix = None) | ||
| 27 | envVarsPrefix = None, | ||
| 28 | mesosBridgeName = raml.Networks.DefaultMesosBridgeName) | ||
| 28 | 29 | ||
| 29 | 30 | "A TaskGroupBuilder" must { | |
| 30 | 31 | ||
| 31 | 32 | "correlate Endpoint with the appropriate network" when { | |
| 32 | 33 | "multiple container networks are defined" in { | |
| 33 | 34 | ||
| 34 | 35 | val Seq(network1, network2) = TaskGroupBuilder.buildMesosNetworks( | |
| 35 | 36 | List(ContainerNetwork("1"), ContainerNetwork("2")), | |
| 36 | 37 | List( | |
| 37 | 38 | Endpoint("port-1", containerPort = Some(2001), networkNames = List("1")), | |
| 38 | 39 | Endpoint("port-2", containerPort = Some(2002), networkNames = List("2"))), | |
| 39 | 40 | List( | |
| 40 | 41 | Some(1001), | |
| 41 | Some(1002))) | ||
| 42 | Some(1002)), | ||
| 43 | defaultBuilderConfig.mesosBridgeName) | ||
| 42 | 44 | ||
| 43 | 45 | network1.getName shouldBe "1" | |
| 44 | 46 | inside(network1.getPortMappingsList.asScala.toList) { | |
| 45 | 47 | case List(port1) => | |
| 46 | 48 | port1.getContainerPort shouldBe 2001 | |
| 47 | 49 | port1.getHostPort shouldBe 1001 | |
| 48 | 50 | } | |
| 49 | 51 | inside(network2.getPortMappingsList.asScala.toList) { | |
| ▲ Show 20 Lines • Show All 601 Lines • ▼ Show 20 Line(s) | |||
| 651 | 653 | assert(task1HealthCheck.getHttp.getPath == "healthcheck") | |
| 652 | 654 | ||
| 653 | 655 | val task2HealthCheck = taskGroupInfo | |
| 654 | 656 | .getTasksList.find(_.getName == "Foo2").get | |
| 655 | 657 | .getHealthCheck | |
| 656 | 658 | ||
| 657 | 659 | assert(task2HealthCheck.getType == mesos.HealthCheck.Type.TCP) | |
| 658 | 660 | assert(task2HealthCheck.getTcp.getPort == 1235) | |
| 661 | } | ||
| 662 | |||
| 663 | "create pod with container-mode bridge networking" in { | ||
| 664 | val offer = MarathonTestHelper.makeBasicOffer(cpus = 3.1, mem = 416.0, disk = 10.0, beginPort = 1200, endPort = 1300).build | ||
| 665 | |||
| 666 | val podSpec = PodDefinition( | ||
| 667 | id = "/product/frontend".toPath, | ||
| 668 | networks = Seq(BridgeNetwork()), | ||
| 669 | containers = Seq( | ||
| 670 | MesosContainer( | ||
| 671 | name = "Foo1", | ||
| 672 | resources = raml.Resources(cpus = 1.0f, mem = 128.0f), | ||
| 673 | endpoints = Seq( | ||
| 674 | raml.Endpoint( | ||
| 675 | name = "foo1", | ||
| 676 | hostPort = Some(1201), | ||
| 677 | containerPort = Some(1211) | ||
| 678 | ) | ||
| 679 | ) | ||
| 680 | ), | ||
| 681 | MesosContainer( | ||
| 682 | name = "Foo2", | ||
| 683 | resources = raml.Resources(cpus = 1.0f, mem = 128.0f), | ||
| 684 | endpoints = Seq( | ||
| 685 | raml.Endpoint( | ||
| 686 | name = "foo2", | ||
| 687 | hostPort = Some(0), | ||
| 688 | containerPort = Some(1212) | ||
| 689 | ) | ||
| 690 | ) | ||
| 691 | ) | ||
| 692 | ) | ||
| 693 | ) | ||
| 694 | |||
| 695 | val resourceMatch = RunSpecOfferMatcher.matchOffer(podSpec, offer, Seq.empty, defaultBuilderConfig.acceptedResourceRoles) | ||
| 696 | |||
| 697 | val (executorInfo, taskGroupInfo, _, _) = TaskGroupBuilder.build( | ||
| 698 | podSpec, | ||
| 699 | offer, | ||
| 700 | s => Instance.Id.forRunSpec(s), | ||
| 701 | defaultBuilderConfig, | ||
| 702 | RunSpecTaskProcessor.empty, | ||
| 703 | resourceMatch.asInstanceOf[ResourceMatchResponse.Match].resourceMatch | ||
| 704 | ) | ||
| 705 | |||
| 706 | assert(taskGroupInfo.getTasksCount == 2) | ||
| 707 | |||
| 708 | val task1 = taskGroupInfo.getTasksList.find(_.getName == "Foo1").get | ||
| 709 | |||
| 710 | val networkInfo = executorInfo.getContainer.getNetworkInfosList.get(0) | ||
| 711 | assert(networkInfo.getName() == defaultBuilderConfig.mesosBridgeName) | ||
| 712 | |||
| 713 | val portMappings = networkInfo.getPortMappingsList | ||
| 714 | |||
| 715 | assert(portMappings.count(_.getContainerPort == 1211) == 1) | ||
| 716 | assert(portMappings.count(_.getContainerPort == 1212) == 1) | ||
| 717 | assert(portMappings.find(_.getContainerPort == 1211).get.getHostPort == 1201) | ||
| 718 | assert(portMappings.find(_.getContainerPort == 1212).get.getHostPort != 0) | ||
| 659 | 719 | } | |
| 660 | 720 | ||
| 661 | 721 | "support URL artifacts" in { | |
| 662 | 722 | val offer = MarathonTestHelper.makeBasicOffer(cpus = 1.1, mem = 160.0, disk = 10.0).build | |
| 663 | 723 | ||
| 664 | 724 | val podSpec = PodDefinition( | |
| 665 | 725 | id = "/product/frontend".toPath, | |
| 666 | 726 | containers = Seq( | |
| ▲ Show 20 Lines • Show All 272 Lines • Show Last 20 Lines | |||
Is this comment still valid?