-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60558/#review181816
-----------------------------------------------------------




src/docker/docker.cpp
Lines 755-765 (patched)
<https://reviews.apache.org/r/60558/#comment257574>

    This looks really complicated with the `wildcard entry` thrown into play. 
Can I suggest storing the DNS entries into a private variable within `Docker` 
on the following lines?
    
    Option<mesos::internal::ContainerDNSInfo::DockerInfo> bridgeDNS;
    hashmap<string, mesos::internal::ContainerDNSInfo::DockerInfo> userDNS;
    
    For wildcard we use the key as "*" in the userDNS network (or you can keep 
an explicit variable for user nertworks. 
    
    If we have these variables and populate them from the flags passed to 
docker executor, the code here becomes relatively simple which will be 
something along the lines of:
    
    if (dockerInfo.network() == ContainerInfo::DocckerInfo::BRIDGE && 
bridgeDNS.isSome() {
      ....
    } else if (dockerInfo.network() == ContainerInfo::DockerInfo::USER && 
userDNS.contains(options.network.get()) {
      ...
    } else if (dockerInfo.network() == ContainerInfo::DockerInfo::USER && 
userDNS.contains("*") {
    }



src/docker/docker.cpp
Lines 770 (patched)
<https://reviews.apache.org/r/60558/#comment257575>

    s/apples/applies?



src/docker/docker.cpp
Lines 783-793 (patched)
<https://reviews.apache.org/r/60558/#comment257583>

    We should convert this into a lambda and then invoke the lambda in the `if` 
and the `else if` conditional below. Will avoid code duplication.


- Avinash sridharan


On July 28, 2017, 2:32 p.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60558/
> -----------------------------------------------------------
> 
> (Updated July 28, 2017, 2:32 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan and Jie Yu.
> 
> 
> Bugs: MESOS-7709
>     https://issues.apache.org/jira/browse/MESOS-7709
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Set container DNS with `--default_container_dns` in Docker executor.
> 
> 
> Diffs
> -----
> 
>   src/docker/docker.hpp 5593cb635e073334c6c2566be3d803cd7febb1c3 
>   src/docker/docker.cpp 8081c0203bf62cf62aa3b93d745f0e829ad65509 
>   src/docker/executor.cpp e03f24461ec7b19cacae79c020406f0a475f2d19 
> 
> 
> Diff: https://reviews.apache.org/r/60558/diff/6/
> 
> 
> Testing
> -------
> 
> sudo make check
> 
> 1. Start Mesos master.
> ```
> $ sudo ./bin/mesos-master.sh --work_dir=/opt/mesos
> ```
> 
> 2. Start Mesos agent.
> ```
> $ sudo ./bin/mesos-slave.sh --master=192.168.122.216:5050 
> --containerizers=mesos,docker --image_providers=docker 
> --image_provisioner_backend=aufs 
> --isolation=filesystem/linux,docker/runtime,network/cni,cgroups/cpu,cgroups/mem
>  --network_cni_config_dir=/opt/cni/net_configs 
> --network_cni_plugins_dir=/opt/cni/plugins --work_dir=/opt/mesos 
> --docker_store_dir=/opt/mesos/store/docker 
> --executor_registration_timeout=60mins 
> --default_container_dns=file:///home/stack/dns.json
> 
> $ cat /home/stack/dns.json    
> {
>   "mesos": [
>     {
>       "network_mode": "CNI",
>       "network_name": "net1",
>       "dns": {
>         "nameservers": [ "8.8.8.8", "8.8.4.4" ]
>       }
>     }
>   ],
>   "docker": [
>     {
>       "network_mode": "BRIDGE",
>       "dns": {
>         "nameservers": [ "8.8.8.8", "8.8.4.4" ],
>         "search": [ "xxx.com", "yyy.com" ],
>         "options": [ "timeout:3", "attempts:2" ]
>       }
>     },
>     {
>       "network_mode": "USER",
>       "network_name": "net2",
>       "dns": {
>         "nameservers": [ "8.8.8.8", "8.8.4.4" ]
>       }
>     }
>   ]
> }
> ```
> 
> 3. Launch a Docker container with `mesos-execute`.
> ```
> $ sudo src/mesos-execute --master=192.168.122.216:5050 
> --task=file:///home/stack/task-docker.json
> 
> $cat /home/stack/task-docker.json 
> {
>   "name": "test",
>   "task_id": {"value" : "test"},
>   "agent_id": {"value" : ""},
>   "resources": [
>     {
>       "name": "cpus",
>       "type": "SCALAR",
>       "scalar": {
>         "value": 0.1
>       }
>     },
>     {
>       "name": "mem",
>       "type": "SCALAR",
>       "scalar": {
>         "value": 32
>       }
>     }
>   ],
>   "command": {
>     "shell": false
>   },
>   "container": {
>     "type": "DOCKER",
>     "docker": {
>       "image": "nginx",
>       "network": "BRIDGE"
>     }
>   }
> }
> ```
> 
> 4. Check the DNS configuration of the Docker container.
> ```
> $ docker ps 
> CONTAINER ID        IMAGE               COMMAND                  CREATED      
>         STATUS              PORTS               NAMES
> ca642bf31a9f        nginx               "nginx -g 'daemon off"   About a 
> minute ago   Up About a minute   80/tcp, 443/tcp     
> mesos-1d48fc32-a138-4c31-a5a9-fd7d279231da
> 
> $ docker exec ca642bf31a9f cat /etc/resolv.conf 
> search xxx.com yyy.com
> nameserver 8.8.8.8
> nameserver 8.8.4.4
> options timeout:3 attempts:2
> ```
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Reply via email to