----------------------------------------------------------- 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 > >