Github user jieyu commented on a diff in the pull request:
https://github.com/apache/mesos/pull/263#discussion_r168647358
--- Diff: src/slave/containerizer/mesos/isolators/network/cni/cni.cpp ---
@@ -570,10 +570,17 @@ Future<Option<ContainerLaunchInfo>>
NetworkCniIsolatorProcess::prepare(
return Failure("Container has already been prepared");
}
+ bool needsSeparateNs = false;
+ if ((containerConfig.has_container_info() &&
+ containerConfig.container_info().network_infos().size() > 0) ||
+ !containerId.has_parent()) {
--- End diff --
This is misleading. It's possible that the top level container joins host
network (thus does not require a separate network namespace). Calling the
boolean `needsSeparateNs` is misleading.
I think basically the first step in this function is to calculate
`containerNetworks` and `hostname` for the container. I'd suggest making it
more explicit:
```c++
hashmap<string, ContainerNetwork> containerNetworks;
Option<string> hostname;
bool isNestedContainer = containerId.has_parent();
bool isDebugContainer = containerConfig.container_class() ==
ContainerClass::DEBUG;
// Not setting network infos for a nested container means that it'll join
its parent's networks.
bool joinParentNetwork =
!containerConfig.has_container_info() ||
containerConfig.container_info().network_infos().empty();
if (isDebugContainer || (isNestedContainer && joinParentNetwork) {
ContainerID rootContainerId = protobuf::getRootContainerId(containerId);
if (infos.contains(rootContainerId)) {
containerNetworks = infos[rootContainerId]->containerNetworks;
}
} else {
// Top level container, or nested container joining separate network than
the parent.
if (containerConfig.has_container_info()) {
const ContainerInfo& containerInfo = containerConfig.container_info();
if (containerInfo.type() != ContainerInfo::MESOS) {
return Failure("...");
}
if (containerInfo.has_hostname()) {
hostname = containerInfo.hostname();
}
int ifIndex = 0;
foreach (...) {
...
}
}
}
```
---
