HomeMesosphereNo notifications. 4 unresolved issues.
  • Search
Differential D288

only set default network name for container networks
ClosedAll Users
Actions

Authored by jdef on Dec 7 2016, 2:17 PM.

Details

Reviewers
aquamatthias
nfnt
Commits
rMARATHON7362a92c9692: only set default network name for container networks
rMARATHONd6d4d031c4c3: only set default network name for container networks
rMARATHON8f401cebbb05: only set default network name for container networks
Summary

Fixes a bug I found while testing default_network_name w/ some other dcos changes (https://github.com/dcos/dcos/pull/912).

Test Plan

sbt test

Diff Detail

Repository
rMARATHON marathon
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
jdef retitled this revision from to only set default network name for container networks.Dec 7 2016, 2:17 PM
jdef updated this object.
jdef edited the test plan for this revision. (Show Details)
jdef updated this object.Dec 7 2016, 2:34 PM
nfnt accepted this revision.Dec 7 2016, 3:05 PM

LGTM! But what was the problem when providing a network name in HOST mode?

This revision is now accepted and ready to land.Dec 7 2016, 3:05 PM
aquamatthias accepted this revision.Dec 7 2016, 3:22 PM
jdef added a comment.Dec 7 2016, 4:30 PM

host networks are unnamed. validation enforces this. before this change, using a default network name w/ host networking causes the pod to fail validation

Closed by commit rMARATHON7362a92c9692: only set default network name for container networks (authored by jdef). · Explain WhyDec 7 2016, 4:31 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents

Diff 1136

View Options

src/main/scala/mesosphere/marathon/api/v2/PodsResource.scala

Show All 16 Lines
1717import mesosphere.marathon.api.v2.validation.PodsValidation
1818import mesosphere.marathon.api.{ AuthResource, MarathonMediaType, RestResource, TaskKiller }
1919import mesosphere.marathon.core.appinfo.{ PodSelector, PodStatusService, Selector }
2020import mesosphere.marathon.core.base.Clock
2121import mesosphere.marathon.core.event._
2222import mesosphere.marathon.core.instance.Instance
2323import mesosphere.marathon.core.pod.{ PodDefinition, PodManager }
2424import mesosphere.marathon.plugin.auth._
25import mesosphere.marathon.raml.{ Pod, Raml }
25import mesosphere.marathon.raml.{ NetworkMode, Pod, Raml }
2626import mesosphere.marathon.state.{ PathId, Timestamp }
2727import play.api.libs.json.Json
2828
2929@Path("v2/pods")
3030@Consumes(Array(MediaType.APPLICATION_JSON))
3131@Produces(Array(MarathonMediaType.PREFERRED_APPLICATION_JSON))
3232class PodsResource @Inject() (
3333 val config: MarathonConf)(
Show All 10 Lines
4444 import PodsResource._
4545 implicit val podDefValidator = PodsValidation.podDefValidator(config.availableFeatures)
4646
4747 // If we change/add/upgrade the notion of a Pod and can't do it purely in the internal model,
4848 // update the json first
4949 private def normalize(pod: Pod): Pod = {
5050 if (pod.networks.exists(_.name.isEmpty)) {
5151 val networks = pod.networks.map { network =>
52 if (network.name.isEmpty) {
52 if (network.mode == NetworkMode.Container && network.name.isEmpty) {
5353 config.defaultNetworkName.get.fold(network) { name =>
5454 network.copy(name = Some(name))
5555 }
5656 } else {
5757 network
5858 }
5959 }
6060 pod.copy(networks = networks)
▲ Show 20 Lines • Show All 248 Lines • Show Last 20 Lines
View Options

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

Show All 14 Lines
1515import mesosphere.marathon.core.group.GroupManager
1616import mesosphere.marathon.core.instance.Instance.InstanceState
1717import mesosphere.marathon.core.instance.Instance
1818import mesosphere.marathon.core.pod.impl.PodManagerImpl
1919import mesosphere.marathon.core.pod.{ PodDefinition, PodManager }
2020import mesosphere.marathon.core.storage.store.impl.memory.InMemoryPersistenceStore
2121import mesosphere.marathon.metrics.Metrics
2222import mesosphere.marathon.plugin.auth.{ Authenticator, Authorizer }
23import mesosphere.marathon.raml.{ ExecutorResources, FixedPodScalingPolicy, Pod, Raml }
23import mesosphere.marathon.raml.{ ExecutorResources, FixedPodScalingPolicy, NetworkMode, Pod, Raml }
2424import mesosphere.marathon.state.PathId._
2525import mesosphere.marathon.state.Timestamp
2626import mesosphere.marathon.storage.repository.PodRepository
2727import mesosphere.marathon.test.Mockito
2828import mesosphere.marathon.upgrade.DeploymentPlan
2929import play.api.libs.json._
3030
3131import scala.collection.immutable.Seq
Show All 9 Lines
4141 val podSpecJson = """
4242 | { "id": "/mypod", "networks": [ { "mode": "host" } ], "containers": [
4343 | { "name": "webapp",
4444 | "resources": { "cpus": 0.03, "mem": 64 },
4545 | "image": { "kind": "DOCKER", "id": "busybox" },
4646 | "exec": { "command": { "shell": "sleep 1" } } } ] }
4747 """.stripMargin
4848
49 val podSpecJsonWithContainerNetworking = """
50 | { "id": "/mypod", "networks": [ { "mode": "container" } ], "containers": [
51 | { "name": "webapp",
52 | "resources": { "cpus": 0.03, "mem": 64 },
53 | "image": { "kind": "DOCKER", "id": "busybox" },
54 | "exec": { "command": { "shell": "sleep 1" } } } ] }
55 """.stripMargin
56
4957 val podSpecJsonWithExecutorResources = """
5058 | { "id": "/mypod", "networks": [ { "mode": "host" } ], "containers": [
5159 | { "name": "webapp",
5260 | "resources": { "cpus": 0.03, "mem": 64 },
5361 | "image": { "kind": "DOCKER", "id": "busybox" },
5462 | "exec": { "command": { "shell": "sleep 1" } } } ],
5563 | "executorResources": { "cpus": 100, "mem": 100 } }
5664 """.stripMargin
5765
5866 "PodsResource" should {
5967 "support pods" in {
6068 val f = Fixture()
6169 val response = f.podsResource.capability(f.auth.request)
6270 response.getStatus should be(HttpServletResponse.SC_OK)
6371
6472 val body = Option(response.getEntity.asInstanceOf[String])
6573 body should be(None)
6674 }
6775
6876 "be able to create a simple single-container pod from docker image w/ shell command" in {
6977 implicit val podSystem = mock[PodManager]
70 val f = Fixture()
78 val f = Fixture(configArgs = Seq("--default_network_name", "blah")) // should not be injected into host network spec
7179
7280 podSystem.create(any, eq(false)).returns(Future.successful(DeploymentPlan.empty))
7381
7482 val response = f.podsResource.create(podSpecJson.getBytes(), force = false, f.auth.request)
7583
7684 withClue(s"response body: ${response.getEntity}") {
7785 response.getStatus should be(HttpServletResponse.SC_CREATED)
7886
7987 val parsedResponse = Option(response.getEntity.asInstanceOf[String]).map(Json.parse)
8088 parsedResponse should be (defined)
8189 val maybePod = parsedResponse.map(_.as[Pod])
8290 maybePod should be (defined) // validate that we DID get back a pod definition
8391 val pod = maybePod.get
92 pod.networks(0).mode should be (NetworkMode.Host)
93 pod.networks(0).name should not be (defined)
94 pod.executorResources should be (defined) // validate that executor resources are defined
95 pod.executorResources.get should be (ExecutorResources()) // validate that the executor resources has default values
96
97 response.getMetadata.containsKey(RestResource.DeploymentHeader) should be(true)
98 }
99 }
100
101 "create a pod w/ container networking" in {
102 implicit val podSystem = mock[PodManager]
103 val f = Fixture(configArgs = Seq("--default_network_name", "blah")) // required since network name is missing from JSON
104
105 podSystem.create(any, eq(false)).returns(Future.successful(DeploymentPlan.empty))
106
107 val response = f.podsResource.create(podSpecJsonWithContainerNetworking.getBytes(), force = false, f.auth.request)
108
109 withClue(s"response body: ${response.getEntity}") {
110 response.getStatus should be(HttpServletResponse.SC_CREATED)
111
112 val parsedResponse = Option(response.getEntity.asInstanceOf[String]).map(Json.parse)
113 parsedResponse should be (defined)
114 val maybePod = parsedResponse.map(_.as[Pod])
115 maybePod should be (defined) // validate that we DID get back a pod definition
116 val pod = maybePod.get
117 pod.networks(0).mode should be (NetworkMode.Container)
118 pod.networks(0).name should be (Some("blah"))
84119 pod.executorResources should be (defined) // validate that executor resources are defined
85120 pod.executorResources.get should be (ExecutorResources()) // validate that the executor resources has default values
86121
87122 response.getMetadata.containsKey(RestResource.DeploymentHeader) should be(true)
88123 }
89124 }
90125
91126 "create a pod with custom executor resource declaration" in {
▲ Show 20 Lines • Show All 354 Lines • Show Last 20 Lines