> On March 28, 2016, 3:13 p.m., Avinash sridharan wrote: > > src/docker/docker.cpp, line 562 > > <https://reviews.apache.org/r/42516/diff/14/?file=1316581#file1316581line562> > > > > We should make sure the user is not trying to specify more than one > > network for this container (multiple `NetworkInfo`). Docker 1.9 supports > > multiple user networks but the container can be connected to more than one > > network only after being started (using `docker network connect`) which is > > kind of useless. Docker 1.10 apparently allows you to attach container to > > multiple network but not using the `docker run` command, so doesn't fit the > > model for `DockerContainerizer`. > > Ezra Silvera wrote: > I'm not sure I follow you point ... Indeed in Docker run it's not > allowed to specify multiple networks this is the reason we use the first > element in the array. In general if you are not going to populate this > structure through inspect I'm not sure you even need an array there .. > > Ezra Silvera wrote: > Let me know if you think we should do something specific in this code, or > it was just a general remark about NetworkInfo > > Avinash sridharan wrote: > Should have been clearer. The `NetworkInfo` field in `ContainerInfo` is a > repeated field. So the framework can potentially ask the container to join > multiple networks (this is supported in CNI > https://issues.apache.org/jira/browse/MESOS-4641) . We should explicitly > disallow it over here. So may be an `if` condition : > if (containerInfo.network_infos_size() > 1) { > return Failure(....); > } > > Avinash sridharan wrote: > Just to add. The reason I think raising this error is important is that > we should explicitly inform the Framework that they are trying to do > something that is not supported, rather than making an assumption that the > first network specified by the `Framework` was the network it intended the > container to join. This is ofcourse valid only in case multiple user-networks > have been specified.
I fully agree. I currently print just a warning I also think we should fail the operation! - Ezra ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42516/#review125635 ----------------------------------------------------------- On March 29, 2016, 11:08 a.m., Ezra Silvera wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/42516/ > ----------------------------------------------------------- > > (Updated March 29, 2016, 11:08 a.m.) > > > Review request for mesos, Jie Yu and Timothy Chen. > > > Bugs: MESOS-4369 > https://issues.apache.org/jira/browse/MESOS-4369 > > > Repository: mesos > > > Description > ------- > > Signed-off-by: Ezra Silvera <[email protected]> > > > Diffs > ----- > > include/mesos/mesos.proto cb68e2c13409620fa4836c12d877488f4333ace7 > include/mesos/v1/mesos.proto af1dc9e11a26b52cfc348324b8dd796c1f72323f > src/docker/docker.cpp 4d35513cdd9c044d37d876a6db7dd9321ceaca53 > > Diff: https://reviews.apache.org/r/42516/diff/ > > > Testing > ------- > > Using Swarm running on Mesos create a network with "docker network create > --driver=bridge myNetwork" and then create a container on that network: > "docker run --net=myNetwork...." > > > Thanks, > > Ezra Silvera > >
