[ 
https://issues.apache.org/jira/browse/MESOS-8534?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16366506#comment-16366506
 ] 

ASF GitHub Bot commented on MESOS-8534:
---------------------------------------

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 (...) {
          ...
        }
      }
    }
    ```


> Allow nested containers in TaskGroups to have separate network namespaces
> -------------------------------------------------------------------------
>
>                 Key: MESOS-8534
>                 URL: https://issues.apache.org/jira/browse/MESOS-8534
>             Project: Mesos
>          Issue Type: Task
>          Components: containerization
>            Reporter: Sagar Sadashiv Patwardhan
>            Priority: Minor
>              Labels: cni
>
> As per the discussion with [~jieyu] and [~avinash.mesos] , I am going to 
> allow nested containers in TaskGroups to have separate namespaces. I am also 
> going to retain the existing functionality, where nested containers can share 
> namespaces with parent/root container.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to