Re: Review Request 63796: Made `CheckerProcess` support IPv6 for HTTP/TCP check.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63796/#review191853 --- Ship it! Ship It! - Avinash sridharan On Nov. 16, 2017, 7:22 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63796/ > --- > > (Updated Nov. 16, 2017, 7:22 a.m.) > > > Review request for mesos, Alexander Rukletsov and Avinash sridharan. > > > Bugs: MESOS-8050 > https://issues.apache.org/jira/browse/MESOS-8050 > > > Repository: mesos > > > Description > --- > > Made `CheckerProcess` support IPv6 for HTTP/TCP check. > > > Diffs > - > > src/checks/checker_process.cpp 7985e8d4dc6115f81a4daef0ce4eee67f4233c9f > > > Diff: https://reviews.apache.org/r/63796/diff/2/ > > > Testing > --- > > sudo make check > > > Thanks, > > Qian Zhang > >
Re: Review Request 63795: Made `mesos-tcp-connect` support IPv6.
> On Nov. 25, 2017, 11:49 a.m., Avinash sridharan wrote: > > src/checks/tcp_connect.cpp > > Lines 88 (patched) > > <https://reviews.apache.org/r/63795/diff/4/?file=1900074#file1900074line94> > > > > Why do we need this? `net::IP` will always be `AF_INET` or `AF_INET6`? > > Qian Zhang wrote: > That's from Alex's comment > (https://reviews.apache.org/r/63795/#comment268744), I think it is because we > do not want to be tightly coupled with the internal implementation of > `net::IP::parse()`. I don't think that comment is valid with the use of `net::IP` and `network::address`. `net::IP` can only be `AF_INET` or AF_INET6`. Its a fundamentatl property of IP addresses. So this check is redundant here. I think we should remove it. https://github.com/apache/mesos/blob/bd5e874c22f0d16fc5494213d319065cf9107d0f/3rdparty/stout/include/stout/ip.hpp#L417 - Avinash --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63795/#review191849 --- On Nov. 22, 2017, 3:14 p.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63795/ > --- > > (Updated Nov. 22, 2017, 3:14 p.m.) > > > Review request for mesos, Alexander Rukletsov and Avinash sridharan. > > > Bugs: MESOS-8050 > https://issues.apache.org/jira/browse/MESOS-8050 > > > Repository: mesos > > > Description > --- > > Made `mesos-tcp-connect` support IPv6. > > > Diffs > - > > src/checks/tcp_connect.cpp 2514f4aebcc634b18cd2b3c36529222fe456e903 > > > Diff: https://reviews.apache.org/r/63795/diff/4/ > > > Testing > --- > > > Thanks, > > Qian Zhang > >
Re: Review Request 63795: Made `mesos-tcp-connect` support IPv6.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63795/#review191849 --- Fix it, then Ship it! Ship It! src/checks/tcp_connect.cpp Lines 88 (patched) <https://reviews.apache.org/r/63795/#comment269785> Why do we need this? `net::IP` will always be `AF_INET` or `AF_INET6`? - Avinash sridharan On Nov. 22, 2017, 3:14 p.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63795/ > --- > > (Updated Nov. 22, 2017, 3:14 p.m.) > > > Review request for mesos, Alexander Rukletsov and Avinash sridharan. > > > Bugs: MESOS-8050 > https://issues.apache.org/jira/browse/MESOS-8050 > > > Repository: mesos > > > Description > --- > > Made `mesos-tcp-connect` support IPv6. > > > Diffs > - > > src/checks/tcp_connect.cpp 2514f4aebcc634b18cd2b3c36529222fe456e903 > > > Diff: https://reviews.apache.org/r/63795/diff/4/ > > > Testing > --- > > > Thanks, > > Qian Zhang > >
Re: Review Request 63910: Added 3 tests for TCP/HTTP(S) health check support for Docker container.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63910/#review191724 --- Ship it! Ship It! - Avinash sridharan On Nov. 19, 2017, 1:41 p.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63910/ > --- > > (Updated Nov. 19, 2017, 1:41 p.m.) > > > Review request for mesos, Alexander Rukletsov and Avinash sridharan. > > > Bugs: MESOS-8050 > https://issues.apache.org/jira/browse/MESOS-8050 > > > Repository: mesos > > > Description > --- > > Removed 3 tests which only run on IPv4 Docker network. > ROOT_DOCKER_DockerHealthyTaskViaHTTP > ROOT_DOCKER_DockerHealthyTaskViaHTTPS > ROOT_DOCKER_DockerHealthyTaskViaTCP > > Added 3 tests which run on both IPv4 and IPv6 Docker networks, so they > should cover the above 3 tests. > ROOT_DOCKER_USERNETWORK_HealthyTaskViaHTTP > ROOT_DOCKER_USERNETWORK_HealthyTaskViaHTTPS > ROOT_DOCKER_USERNETWORK_HealthyTaskViaTCP > > > Diffs > - > > src/tests/containerizer/docker_containerizer_tests.cpp > 67945ddc4f98ffa072f584af8106967e7ff336d3 > src/tests/health_check_tests.cpp c0dcba265363f2149b217b189ee5a8e925e40ea1 > src/tests/mesos.hpp 345b883a8c629bf5bed83e9236632c277f2eb0eb > > > Diff: https://reviews.apache.org/r/63910/diff/2/ > > > Testing > --- > > Ran all newly added tests repeatedly (20 times), they all succeeded. > > > Thanks, > > Qian Zhang > >
Re: Review Request 63910: Added 3 tests for TCP/HTTP(S) health check support for Docker container.
> On Nov. 17, 2017, 7:25 p.m., Avinash sridharan wrote: > > src/tests/health_check_tests.cpp > > Lines 2450 (patched) > > <https://reviews.apache.org/r/63910/diff/1/?file=1896789#file1896789line2558> > > > > We should try and see if we can publish this image to > > :https://hub.docker.com/u/mesos/ > > > > Also, I assuming this a custorm image. We should commit the > > `Dockerfile` for this image to our test repo. More importantly want to the > > cert being used in this test to be part of the repo. > > Qian Zhang wrote: > Agree, do you know who has the permission to publish image to > https://hub.docker.com/u/mesos/? And for the Dockerfile & the cert (and even > the Python code `https_server.py`), I think we can just put them in the > description of the Docker image so that we can see all of them in the same > place. Qian for the time being lets just commit the Dockerfile. We can figure out the uploading of the image in a separate commit. We need to get consensus from AlexR as to where the Dockerfile should reside in the repo. If necessary we can this as a separate patch after we commit this patch. Please feel free to drop this if we are going to do this in a separate patch. > On Nov. 17, 2017, 7:25 p.m., Avinash sridharan wrote: > > src/tests/health_check_tests.cpp > > Lines 2525 (patched) > > <https://reviews.apache.org/r/63910/diff/1/?file=1896789#file1896789line2648> > > > > As you mentioned, given that this test is identical to the HTTP test > > above, can we just parameterize the `HealthCheck.type` field as well in the > > above test case? > > Qian Zhang wrote: > Yeah, I thought about that before, but the problem is, besides > `ROOT_DOCKER_USERNETWORK_HealthyTaskViaHTTP` and > `ROOT_DOCKER_USERNETWORK_HealthyTaskViaTCP`, we have another test > `ROOT_DOCKER_USERNETWORK_HealthyTaskViaHTTPS`, I am not sure how it can work > if we parameterize the `HealthCheck.type` field as well. Understood, we take this up as an optimization in a separate commit. - Avinash --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63910/#review191375 --- On Nov. 19, 2017, 1:41 p.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63910/ > --- > > (Updated Nov. 19, 2017, 1:41 p.m.) > > > Review request for mesos, Alexander Rukletsov and Avinash sridharan. > > > Bugs: MESOS-8050 > https://issues.apache.org/jira/browse/MESOS-8050 > > > Repository: mesos > > > Description > --- > > Removed 3 tests which only run on IPv4 Docker network. > ROOT_DOCKER_DockerHealthyTaskViaHTTP > ROOT_DOCKER_DockerHealthyTaskViaHTTPS > ROOT_DOCKER_DockerHealthyTaskViaTCP > > Added 3 tests which run on both IPv4 and IPv6 Docker networks, so they > should cover the above 3 tests. > ROOT_DOCKER_USERNETWORK_HealthyTaskViaHTTP > ROOT_DOCKER_USERNETWORK_HealthyTaskViaHTTPS > ROOT_DOCKER_USERNETWORK_HealthyTaskViaTCP > > > Diffs > - > > src/tests/containerizer/docker_containerizer_tests.cpp > 67945ddc4f98ffa072f584af8106967e7ff336d3 > src/tests/health_check_tests.cpp c0dcba265363f2149b217b189ee5a8e925e40ea1 > src/tests/mesos.hpp 345b883a8c629bf5bed83e9236632c277f2eb0eb > > > Diff: https://reviews.apache.org/r/63910/diff/2/ > > > Testing > --- > > Ran all newly added tests repeatedly (20 times), they all succeeded. > > > Thanks, > > Qian Zhang > >
Re: Review Request 63795: Made `mesos-tcp-connect` support IPv6.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63795/#review191722 --- src/checks/tcp_connect.cpp Lines 90 (patched) <https://reviews.apache.org/r/63795/#comment269606> Should have bought this up earlier. I think this code will be a lot more simpler if you use `network::Address` to represent the ip:port address. https://github.com/apache/mesos/blob/master/3rdparty/libprocess/include/process/address.hpp#L277 You wouldn't need to maintain family specific code here and you can directly use the `sock_storage` cast operator https://github.com/apache/mesos/blob/master/3rdparty/libprocess/include/process/address.hpp#L377 - Avinash sridharan On Nov. 17, 2017, 12:57 p.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63795/ > --- > > (Updated Nov. 17, 2017, 12:57 p.m.) > > > Review request for mesos, Alexander Rukletsov and Avinash sridharan. > > > Bugs: MESOS-8050 > https://issues.apache.org/jira/browse/MESOS-8050 > > > Repository: mesos > > > Description > --- > > Made `mesos-tcp-connect` support IPv6. > > > Diffs > - > > src/checks/tcp_connect.cpp 2514f4aebcc634b18cd2b3c36529222fe456e903 > > > Diff: https://reviews.apache.org/r/63795/diff/3/ > > > Testing > --- > > > Thanks, > > Qian Zhang > >
Re: Review Request 63795: Made `mesos-tcp-connect` support IPv6.
> On Nov. 16, 2017, 6:54 p.m., Avinash sridharan wrote: > > src/checks/tcp_connect.cpp > > Lines 90 (patched) > > <https://reviews.apache.org/r/63795/diff/2/?file=1894182#file1894182line92> > > > > Why do we need this? > > > > `net::IP::in` and `net::IP::in6` already give this functionality we > > should just use those? > > Qian Zhang wrote: > We need to call `parse->family()` to know it is `AF_INET` or `AF_INET6`, > and then call `net::IP::in()` or `net::IP::in6()` accordingly. agreed. Was wondering if we can add a `storage` call to `net::IP` and get `storage` from `net::IP` itself instead of higher level code needing to deal with `family`. - Avinash --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63795/#review191196 --- On Nov. 17, 2017, 12:57 p.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63795/ > --- > > (Updated Nov. 17, 2017, 12:57 p.m.) > > > Review request for mesos, Alexander Rukletsov and Avinash sridharan. > > > Bugs: MESOS-8050 > https://issues.apache.org/jira/browse/MESOS-8050 > > > Repository: mesos > > > Description > --- > > Made `mesos-tcp-connect` support IPv6. > > > Diffs > - > > src/checks/tcp_connect.cpp 2514f4aebcc634b18cd2b3c36529222fe456e903 > > > Diff: https://reviews.apache.org/r/63795/diff/3/ > > > Testing > --- > > > Thanks, > > Qian Zhang > >
Re: Review Request 63910: Added 3 tests for TCP/HTTP(S) health check support for Docker container.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63910/#review191375 --- src/tests/health_check_tests.cpp Lines 1972 (patched) <https://reviews.apache.org/r/63910/#comment269159> What is `local mode`? :) src/tests/health_check_tests.cpp Lines 2040 (patched) <https://reviews.apache.org/r/63910/#comment269160> Feels like we should have a helper function for this, similar to `createTask`? src/tests/health_check_tests.cpp Line 2043 (original), 2050 (patched) <https://reviews.apache.org/r/63910/#comment269161> why s/->/get()? src/tests/health_check_tests.cpp Line 2062 (original), 2070 (patched) <https://reviews.apache.org/r/63910/#comment269162> Not yours but s/mesos/Mesos src/tests/health_check_tests.cpp Line 2080 (original), 2077 (patched) <https://reviews.apache.org/r/63910/#comment269163> s/CMD/command src/tests/health_check_tests.cpp Line 2268 (original), 2308 (patched) <https://reviews.apache.org/r/63910/#comment269164> This test is a user network test, so we technically don't need to do this. We could set the port statically since the container is going to get its own network namespace. src/tests/health_check_tests.cpp Line 2272 (original), 2312 (patched) <https://reviews.apache.org/r/63910/#comment269165> Just a thought: Can we just use `nginx:alpine` image? We won't need to rely on `nc` to do this stuff. It would work cleanly since the container is on its network namespace. src/tests/health_check_tests.cpp Lines 2450 (patched) <https://reviews.apache.org/r/63910/#comment269166> We should try and see if we can publish this image to :https://hub.docker.com/u/mesos/ Also, I assuming this a custorm image. We should commit the `Dockerfile` for this image to our test repo. More importantly want to the cert being used in this test to be part of the repo. src/tests/health_check_tests.cpp Lines 2525 (patched) <https://reviews.apache.org/r/63910/#comment269167> As you mentioned, given that this test is identical to the HTTP test above, can we just parameterize the `HealthCheck.type` field as well in the above test case? - Avinash sridharan On Nov. 17, 2017, 1:35 p.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63910/ > --- > > (Updated Nov. 17, 2017, 1:35 p.m.) > > > Review request for mesos, Alexander Rukletsov and Avinash sridharan. > > > Bugs: MESOS-8050 > https://issues.apache.org/jira/browse/MESOS-8050 > > > Repository: mesos > > > Description > --- > > Removed 3 tests which only run on IPv4 Docker network. > ROOT_DOCKER_DockerHealthyTaskViaHTTP > ROOT_DOCKER_DockerHealthyTaskViaHTTPS > ROOT_DOCKER_DockerHealthyTaskViaTCP > > Added 3 tests which run on both IPv4 and IPv6 Docker networks, so they > should cover the above 3 tests. > ROOT_DOCKER_USERNETWORK_HealthyTaskViaHTTP > ROOT_DOCKER_USERNETWORK_HealthyTaskViaHTTPS > ROOT_DOCKER_USERNETWORK_HealthyTaskViaTCP > > > Diffs > - > > src/tests/containerizer/docker_containerizer_tests.cpp > 67945ddc4f98ffa072f584af8106967e7ff336d3 > src/tests/health_check_tests.cpp c0dcba265363f2149b217b189ee5a8e925e40ea1 > src/tests/mesos.hpp 345b883a8c629bf5bed83e9236632c277f2eb0eb > > > Diff: https://reviews.apache.org/r/63910/diff/1/ > > > Testing > --- > > Ran all newly added tests repeatedly (20 times), they all succeeded. > > > Thanks, > > Qian Zhang > >
Re: Review Request 63794: Added a new member field `ipv6` to the `CheckerProcess` class.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63794/#review191226 --- Fix it, then Ship it! Ship It! src/checks/checker_process.hpp Lines 148 (patched) <https://reviews.apache.org/r/63794/#comment268895> s/temp/temporary - Avinash sridharan On Nov. 16, 2017, 7:20 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63794/ > --- > > (Updated Nov. 16, 2017, 7:20 a.m.) > > > Review request for mesos, Alexander Rukletsov and Avinash sridharan. > > > Bugs: MESOS-8050 > https://issues.apache.org/jira/browse/MESOS-8050 > > > Repository: mesos > > > Description > --- > > Added a new member field `ipv6` to the `CheckerProcess` class. > > > Diffs > - > > src/checks/checker_process.hpp 5d0c36dd3e1edf68527fc109d8c0708f09b7e12f > src/checks/checker_process.cpp 7985e8d4dc6115f81a4daef0ce4eee67f4233c9f > src/checks/health_checker.cpp d4bda6ed1747ae4c970619bbfa336321aeea52ea > > > Diff: https://reviews.apache.org/r/63794/diff/2/ > > > Testing > --- > > > Thanks, > > Qian Zhang > >
Re: Review Request 63795: Made `mesos-tcp-connect` support IPv6.
> On Nov. 16, 2017, 6:54 p.m., Avinash sridharan wrote: > > src/checks/tcp_connect.cpp > > Line 86 (original), 84 (patched) > > <https://reviews.apache.org/r/63795/diff/2/?file=1894182#file1894182line86> > > > > `AF_UNSPEC` is default. You can just do `net::IP::parse` > > > > Also instead of `parse` can we use `_ip`? Looks like AlexR already vetoed the use of `_ip`, so I am good with that. - Avinash --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63795/#review191196 --- On Nov. 16, 2017, 7:21 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63795/ > --- > > (Updated Nov. 16, 2017, 7:21 a.m.) > > > Review request for mesos, Alexander Rukletsov and Avinash sridharan. > > > Bugs: MESOS-8050 > https://issues.apache.org/jira/browse/MESOS-8050 > > > Repository: mesos > > > Description > --- > > Made `mesos-tcp-connect` support IPv6. > > > Diffs > - > > src/checks/tcp_connect.cpp 2514f4aebcc634b18cd2b3c36529222fe456e903 > > > Diff: https://reviews.apache.org/r/63795/diff/2/ > > > Testing > --- > > > Thanks, > > Qian Zhang > >
Re: Review Request 63795: Made `mesos-tcp-connect` support IPv6.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63795/#review191196 --- src/checks/tcp_connect.cpp Line 86 (original), 84 (patched) <https://reviews.apache.org/r/63795/#comment268831> `AF_UNSPEC` is default. You can just do `net::IP::parse` Also instead of `parse` can we use `_ip`? src/checks/tcp_connect.cpp Lines 90 (patched) <https://reviews.apache.org/r/63795/#comment268890> Why do we need this? `net::IP::in` and `net::IP::in6` already give this functionality we should just use those? - Avinash sridharan On Nov. 16, 2017, 7:21 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63795/ > --- > > (Updated Nov. 16, 2017, 7:21 a.m.) > > > Review request for mesos, Alexander Rukletsov and Avinash sridharan. > > > Bugs: MESOS-8050 > https://issues.apache.org/jira/browse/MESOS-8050 > > > Repository: mesos > > > Description > --- > > Made `mesos-tcp-connect` support IPv6. > > > Diffs > - > > src/checks/tcp_connect.cpp 2514f4aebcc634b18cd2b3c36529222fe456e903 > > > Diff: https://reviews.apache.org/r/63795/diff/2/ > > > Testing > --- > > > Thanks, > > Qian Zhang > >
Re: Review Request 63434: Added IPv6 capabilities for TCP and HTTP healthchecks.
> On Nov. 7, 2017, 2:22 p.m., Qian Zhang wrote: > > I think you need to do the similar changes to the `CheckInfo` message. `CheckInfo` is specific to the `default-executor` and hence UCR. We don't support IPv6 for CNI and hence UCR at this point, so hence not adding this to `CheckInfo` at this point. - Avinash --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63434/#review190303 --- On Nov. 3, 2017, 12:38 a.m., Avinash sridharan wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63434/ > --- > > (Updated Nov. 3, 2017, 12:38 a.m.) > > > Review request for mesos, Alexander Rukletsov, Qian Zhang, and Vinod Kone. > > > Bugs: MESOS-8050 > https://issues.apache.org/jira/browse/MESOS-8050 > > > Repository: mesos > > > Description > --- > > Added IPv6 capabilities for TCP and HTTP healthchecks. > > > Diffs > - > > include/mesos/mesos.proto 12652fe2804efdd2228c8e8c95b88e1f4b762f7b > include/mesos/v1/mesos.proto e0797aea6ffe200aab5b0bde5450dbd7c332618b > > > Diff: https://reviews.apache.org/r/63434/diff/2/ > > > Testing > --- > > make > > > Thanks, > > Avinash sridharan > >
Re: Review Request 63434: Added IPv6 capabilities for TCP and HTTP healthchecks.
> On Nov. 2, 2017, 6:48 p.m., Alexander Rukletsov wrote: > > include/mesos/mesos.proto > > Lines 512-516 (patched) > > <https://reviews.apache.org/r/63434/diff/1/?file=1873100#file1873100line512> > > > > When I see this enum, I have several questions, for example, "how this > > fits into overall IPv6 story?", "shall this enum be global?", "what about > > `CheckInfo` message?". Since we are going to deprecate `HealchCheck` at > > some point, I'm fine with adding an ad-hoc solution here, but I would like > > to have a note here about what our long-term plan is (and that provides > > some sorts of answers to the questions above). > > Vinod Kone wrote: > Yea, one option is to directly use "NetworkInfo::Protocol" instead of > re-declaring it here. I like that option, since it will be tied to `NetworkInfo` which will encapsulate our support for IPv4 and IPv6. - Avinash --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63434/#review189946 --- On Nov. 3, 2017, 12:38 a.m., Avinash sridharan wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63434/ > --- > > (Updated Nov. 3, 2017, 12:38 a.m.) > > > Review request for mesos, Alexander Rukletsov, Qian Zhang, and Vinod Kone. > > > Bugs: MESOS-8050 > https://issues.apache.org/jira/browse/MESOS-8050 > > > Repository: mesos > > > Description > --- > > Added IPv6 capabilities for TCP and HTTP healthchecks. > > > Diffs > - > > include/mesos/mesos.proto 12652fe2804efdd2228c8e8c95b88e1f4b762f7b > include/mesos/v1/mesos.proto e0797aea6ffe200aab5b0bde5450dbd7c332618b > > > Diff: https://reviews.apache.org/r/63434/diff/2/ > > > Testing > --- > > make > > > Thanks, > > Avinash sridharan > >
Re: Review Request 63434: Added IPv6 capabilities for TCP and HTTP healthchecks.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63434/ --- (Updated Nov. 3, 2017, 12:38 a.m.) Review request for mesos, Alexander Rukletsov, Qian Zhang, and Vinod Kone. Changes --- Addressed Vinod's and AlexR's comments. Bugs: MESOS-8050 https://issues.apache.org/jira/browse/MESOS-8050 Repository: mesos Description --- Added IPv6 capabilities for TCP and HTTP healthchecks. Diffs (updated) - include/mesos/mesos.proto 12652fe2804efdd2228c8e8c95b88e1f4b762f7b include/mesos/v1/mesos.proto e0797aea6ffe200aab5b0bde5450dbd7c332618b Diff: https://reviews.apache.org/r/63434/diff/2/ Changes: https://reviews.apache.org/r/63434/diff/1-2/ Testing --- make Thanks, Avinash sridharan
Review Request 63434: Added IPv6 capabilities for TCP and HTTP healthchecks.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63434/ --- Review request for mesos, Alexander Rukletsov, Qian Zhang, and Vinod Kone. Bugs: MESOS-8050 https://issues.apache.org/jira/browse/MESOS-8050 Repository: mesos Description --- Added IPv6 capabilities for TCP and HTTP healthchecks. Diffs - include/mesos/mesos.proto 12652fe2804efdd2228c8e8c95b88e1f4b762f7b include/mesos/v1/mesos.proto e0797aea6ffe200aab5b0bde5450dbd7c332618b Diff: https://reviews.apache.org/r/63434/diff/1/ Testing --- make Thanks, Avinash sridharan
Re: Review Request 62913: Fixed flaky ROOT_DynamicAddDelofCniConfig test.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62913/#review187715 --- Ship it! Ship It! - Avinash sridharan On Oct. 11, 2017, 11:21 p.m., Alexander Rukletsov wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/62913/ > --- > > (Updated Oct. 11, 2017, 11:21 p.m.) > > > Review request for mesos, Avinash sridharan, Gaston Kleiman, and Vinod Kone. > > > Bugs: MESOS-7511 > https://issues.apache.org/jira/browse/MESOS-7511 > > > Repository: mesos > > > Description > --- > > Due to clock manipulation, a TASK_RUNNING status update could have > been resent by the status update manager, triggering an expectation > failure. > > > Diffs > - > > src/tests/containerizer/cni_isolator_tests.cpp > 636695f368d1c8acd9ed4ccfd1a449f375af8282 > > > Diff: https://reviews.apache.org/r/62913/diff/1/ > > > Testing > --- > > make check > > > Thanks, > > Alexander Rukletsov > >
Re: Review Request 62017: Allows port mapper plugin to have optional args.
> On Sept. 6, 2017, 2:09 a.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp
> > Lines 149 (patched)
> > <https://reviews.apache.org/r/62017/diff/4/?file=1816054#file1816054line149>
> >
> > Why are we setting this empty `args` here instead of setting up a
> > `NetworkInfo networkInfo` object and passing it down into the constructor
> > of the port-mapper?
> >
> > the `port-mapper` really doesn't care about `args` it cares about
> > `NetworkInfo`. This code seems to conflate the whole need for not setting
> > up `args` here.
> >
> > Also setting up `mesos.values` seems to tie the port-mapper plugin back
> > to Mesos semantics of setting up the args which seems like a bit of a hack.
>
> Deepak Goel wrote:
> Yeah, you are right. Initially, I thought of doing that but then it led
> to making exceptions to various checks that happens before we reach to the
> constructor and the code looked even more hacky and error prone. This patch
> achieves the same goal with fewer lines of code change
I don't follow. Why would it be more hacky?
If we just need to setup the `NetworkInfo` all we would do:
```
NetworkInfo networkInfo
if (args.isSome()) {
// Move current code into this segement
// If `NetworkInfo` is set:
networkInfo =
}
// create `port-mapper` with `networkInfo`
```
Not sure whats so complicated about the above ^^
- Avinash
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62017/#review184620
---
On Sept. 6, 2017, 12:25 a.m., Deepak Goel wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62017/
> -------
>
> (Updated Sept. 6, 2017, 12:25 a.m.)
>
>
> Review request for mesos and Avinash sridharan.
>
>
> Bugs: mesos-7923
> https://issues.apache.org/jira/browse/mesos-7923
>
>
> Repository: mesos
>
>
> Description
> ---
>
> Mesos port mapper cni plugin is a wrapper around bridge plugin
> to add port mapping functionality to bridge plugin. However, in
> certain cases the network creator doesn't need port mapping
> functionality and just want to access bridge plugin. In this case,
> the creator may not supply `args` in cni config which will makes
> mesos port mapper plugin to fail. This patch makes `args` in cni
> config optional for mesos port mapper plugin
>
>
> Diffs
> -
>
>
> src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp
> 43cf3e44a55c56dc8195c9cd05f6edd8bf13d448
>
>
> Diff: https://reviews.apache.org/r/62017/diff/4/
>
>
> Testing
> ---
>
>
> Thanks,
>
> Deepak Goel
>
>
Re: Review Request 62017: Allows port mapper plugin to have optional args.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62017/#review184620 --- src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp Lines 149 (patched) <https://reviews.apache.org/r/62017/#comment260797> Why are we setting this empty `args` here instead of setting up a `NetworkInfo networkInfo` object and passing it down into the constructor of the port-mapper? the `port-mapper` really doesn't care about `args` it cares about `NetworkInfo`. This code seems to conflate the whole need for not setting up `args` here. Also setting up `mesos.values` seems to tie the port-mapper plugin back to Mesos semantics of setting up the args which seems like a bit of a hack. - Avinash sridharan On Sept. 6, 2017, 12:25 a.m., Deepak Goel wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/62017/ > --- > > (Updated Sept. 6, 2017, 12:25 a.m.) > > > Review request for mesos and Avinash sridharan. > > > Bugs: mesos-7923 > https://issues.apache.org/jira/browse/mesos-7923 > > > Repository: mesos > > > Description > --- > > Mesos port mapper cni plugin is a wrapper around bridge plugin > to add port mapping functionality to bridge plugin. However, in > certain cases the network creator doesn't need port mapping > functionality and just want to access bridge plugin. In this case, > the creator may not supply `args` in cni config which will makes > mesos port mapper plugin to fail. This patch makes `args` in cni > config optional for mesos port mapper plugin > > > Diffs > - > > > src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp > 43cf3e44a55c56dc8195c9cd05f6edd8bf13d448 > > > Diff: https://reviews.apache.org/r/62017/diff/4/ > > > Testing > --- > > > Thanks, > > Deepak Goel > >
Re: Review Request 61874: Added a test for IPv6 containers on docker user networks.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61874/ --- (Updated Aug. 25, 2017, 6:59 a.m.) Review request for mesos, Jie Yu and Qian Zhang. Changes --- Addressed Qian's comments. Bugs: MESOS-7807 https://issues.apache.org/jira/browse/MESOS-7807 Repository: mesos Description --- The test creates a IPv6 docker user network. It than launches an alpine image on the docker user network. Finally it checks if the IP address allocated to the container are reflected correctly in state. Diffs (updated) - src/tests/containerizer/docker_containerizer_tests.cpp 6856ca22570183b022d401d1fa8f59d819783eed Diff: https://reviews.apache.org/r/61874/diff/3/ Changes: https://reviews.apache.org/r/61874/diff/2-3/ Testing --- sudo bin/mesos-tests.sh --gtest_filter="*.ROOT_DOCKER_USERNETWORK_LaunchIPv6Container" [==] Running 1 test from 1 test case. [--] Global test environment set-up. [--] 1 test from DockerContainerizerIPv6UserNetworkTest [ RUN ] DockerContainerizerIPv6UserNetworkTest.ROOT_DOCKER_USERNETWORK_LaunchIPv6Container [ OK ] DockerContainerizerIPv6UserNetworkTest.ROOT_DOCKER_USERNETWORK_LaunchIPv6Container (15660 ms) [--] 1 test from DockerContainerizerIPv6UserNetworkTest (15664 ms total) [--] Global test environment tear-down [==] 1 test from 1 test case ran. (15713 ms total) [ PASSED ] 1 test. Thanks, Avinash sridharan
Re: Review Request 61874: Added a test for IPv6 containers on docker user networks.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61874/ --- (Updated Aug. 25, 2017, 6:56 a.m.) Review request for mesos, Jie Yu and Qian Zhang. Changes --- Addressed Qian's comments. Bugs: MESOS-7807 https://issues.apache.org/jira/browse/MESOS-7807 Repository: mesos Description --- The test creates a IPv6 docker user network. It than launches an alpine image on the docker user network. Finally it checks if the IP address allocated to the container are reflected correctly in state. Diffs (updated) - src/tests/containerizer/docker_containerizer_tests.cpp 6856ca22570183b022d401d1fa8f59d819783eed Diff: https://reviews.apache.org/r/61874/diff/2/ Changes: https://reviews.apache.org/r/61874/diff/1-2/ Testing --- sudo bin/mesos-tests.sh --gtest_filter="*.ROOT_DOCKER_USERNETWORK_LaunchIPv6Container" [==] Running 1 test from 1 test case. [--] Global test environment set-up. [--] 1 test from DockerContainerizerIPv6UserNetworkTest [ RUN ] DockerContainerizerIPv6UserNetworkTest.ROOT_DOCKER_USERNETWORK_LaunchIPv6Container [ OK ] DockerContainerizerIPv6UserNetworkTest.ROOT_DOCKER_USERNETWORK_LaunchIPv6Container (15660 ms) [--] 1 test from DockerContainerizerIPv6UserNetworkTest (15664 ms total) [--] Global test environment tear-down [==] 1 test from 1 test case ran. (15713 ms total) [ PASSED ] 1 test. Thanks, Avinash sridharan
Re: Review Request 61873: Added test filter for docker user network tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61873/ --- (Updated Aug. 25, 2017, 6:54 a.m.) Review request for mesos, Jie Yu and Qian Zhang. Changes --- Fixed Qian's comments. Bugs: MESOS-7807 https://issues.apache.org/jira/browse/MESOS-7807 Repository: mesos Description --- Added test filter for docker user network tests. Diffs (updated) - src/tests/environment.cpp a7262cd97361b55ff31238341657e764df6c9ea5 Diff: https://reviews.apache.org/r/61873/diff/2/ Changes: https://reviews.apache.org/r/61873/diff/1-2/ Testing --- make, make check Thanks, Avinash sridharan
Re: Review Request 61237: Updated docker executor to return IPv6 address of a container.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61237/
---
(Updated Aug. 25, 2017, 6:54 a.m.)
Review request for mesos, Jie Yu and Qian Zhang.
Changes
---
Fixed Qian's comments.
Bugs: MESOS-7807
https://issues.apache.org/jira/browse/MESOS-7807
Repository: mesos
Description
---
A docker container can have a v4 and a v6 address. Currently the docker
executor was returning only the IPv4 address. This changes allows the
executor to return the IPv4 and IPv6 address of the container.
Diffs (updated)
-
src/docker/docker.hpp 95e60a7dbbd6ccc659f70ca3dca8d13433f3ea07
src/docker/docker.cpp 192e170acce895f80df6c83e437da020489de468
src/docker/executor.cpp 99a62244b7805d25b926d0293cb62574981d2878
Diff: https://reviews.apache.org/r/61237/diff/4/
Changes: https://reviews.apache.org/r/61237/diff/3-4/
Testing
---
Start agent:
sudo ./bin/mesos-agent.sh --ip=10.0.2.15 --master=10.0.2.15:5050
--work_dir=/tmp/mesos --containerizers=mesos,docker
Start master:
sudo ./bin/mesos-master.sh --ip=10.0.2.15 --work_dir=/tmp/mesos-master
Use mesos-execute to launch a docker container on an IPv6 network:
sudo src/mesos-execute --master=10.0.2.15:5050
--task=file:///home/vagrant/dev/mesos_tasks/docker.json 2> log.txt
cat /home/vagrant/dev/mesos_tasks/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": "USER"
},
"network_infos": [
{
"name": "ipv6"
}
]
}
}
Curl the Mesos state to make sure that the right IPv6 addresses are reflected
for the container:
curl http://10.0.2.15:5051/state | jq
"role": "*",
"statuses": [
{
"state": "TASK_RUNNING",
"timestamp": 1501288232.79384,
"container_status": {
"container_id": {
"value": "59319a5b-a28b-46bf-a20f-8472e6598abd"
},
"network_infos": [
{
"ip_addresses": [
{
"protocol": "IPv4",
"ip_address": "172.19.0.2"
},
{
"protocol": "IPv6",
"ip_address": "fd00:0:0:1::2"
}
],
"name": "ipv6"
}
]
}
}
],
"container": {
"type": "DOCKER",
"docker": {
"image": "nginx",
"network": "USER",
"privileged": false
},
"network_infos": [
{
"name": "ipv6"
}
]
}
}
],
Thanks,
Avinash sridharan
Re: Review Request 61874: Added a test for IPv6 containers on docker user networks.
> On Aug. 24, 2017, 10:21 a.m., Qian Zhang wrote: > > src/tests/containerizer/docker_containerizer_tests.cpp > > Lines 4753 (patched) > > <https://reviews.apache.org/r/61874/diff/1/?file=1802826#file1802826line4753> > > > > I think we should call this method in the end rather than at the > > beginning. We need the containers to be removed from the network before the network can be deleted. So need call tear down on the base class before deleting the network. - Avinash --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61874/#review183724 ------- On Aug. 24, 2017, 12:59 a.m., Avinash sridharan wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/61874/ > --- > > (Updated Aug. 24, 2017, 12:59 a.m.) > > > Review request for mesos, Jie Yu and Qian Zhang. > > > Bugs: MESOS-7807 > https://issues.apache.org/jira/browse/MESOS-7807 > > > Repository: mesos > > > Description > --- > > The test creates a IPv6 docker user network. It than launches an alpine > image on the docker user network. Finally it checks if the IP address > allocated to the container are reflected correctly in state. > > > Diffs > - > > src/tests/containerizer/docker_containerizer_tests.cpp > 6856ca22570183b022d401d1fa8f59d819783eed > > > Diff: https://reviews.apache.org/r/61874/diff/1/ > > > Testing > --- > > sudo bin/mesos-tests.sh > --gtest_filter="*.ROOT_DOCKER_USERNETWORK_LaunchIPv6Container" > [==] Running 1 test from 1 test case. > [--] Global test environment set-up. > [--] 1 test from DockerContainerizerIPv6UserNetworkTest > [ RUN ] > DockerContainerizerIPv6UserNetworkTest.ROOT_DOCKER_USERNETWORK_LaunchIPv6Container > [ OK ] > DockerContainerizerIPv6UserNetworkTest.ROOT_DOCKER_USERNETWORK_LaunchIPv6Container > (15660 ms) > [--] 1 test from DockerContainerizerIPv6UserNetworkTest (15664 ms > total) > > [--] Global test environment tear-down > [==] 1 test from 1 test case ran. (15713 ms total) > [ PASSED ] 1 test. > > > Thanks, > > Avinash sridharan > >
Review Request 61873: Added test filter for docker user network tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61873/ --- Review request for mesos, Jie Yu and Qian Zhang. Bugs: MESOS-7807 https://issues.apache.org/jira/browse/MESOS-7807 Repository: mesos Description --- Added test filter for docker user network tests. Diffs - src/tests/environment.cpp a7262cd97361b55ff31238341657e764df6c9ea5 Diff: https://reviews.apache.org/r/61873/diff/1/ Testing --- make, make check Thanks, Avinash sridharan
Review Request 61874: Added a test for IPv6 containers on docker user networks.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61874/ --- Review request for mesos, Jie Yu and Qian Zhang. Bugs: MESOS-7807 https://issues.apache.org/jira/browse/MESOS-7807 Repository: mesos Description --- The test creates a IPv6 docker user network. It than launches an alpine image on the docker user network. Finally it checks if the IP address allocated to the container are reflected correctly in state. Diffs - src/tests/containerizer/docker_containerizer_tests.cpp 6856ca22570183b022d401d1fa8f59d819783eed Diff: https://reviews.apache.org/r/61874/diff/1/ Testing --- sudo bin/mesos-tests.sh --gtest_filter="*.ROOT_DOCKER_USERNETWORK_LaunchIPv6Container" [==] Running 1 test from 1 test case. [--] Global test environment set-up. [--] 1 test from DockerContainerizerIPv6UserNetworkTest [ RUN ] DockerContainerizerIPv6UserNetworkTest.ROOT_DOCKER_USERNETWORK_LaunchIPv6Container [ OK ] DockerContainerizerIPv6UserNetworkTest.ROOT_DOCKER_USERNETWORK_LaunchIPv6Container (15660 ms) [--] 1 test from DockerContainerizerIPv6UserNetworkTest (15664 ms total) [--] Global test environment tear-down [==] 1 test from 1 test case ran. (15713 ms total) [ PASSED ] 1 test. Thanks, Avinash sridharan
Re: Review Request 61237: Updated docker executor to return IPv6 address of a container.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61237/
---
(Updated Aug. 24, 2017, 12:48 a.m.)
Review request for mesos, Jie Yu and Qian Zhang.
Changes
---
Rebased.
Bugs: MESOS-7807
https://issues.apache.org/jira/browse/MESOS-7807
Repository: mesos
Description
---
A docker container can have a v4 and a v6 address. Currently the docker
executor was returning only the IPv4 address. This changes allows the
executor to return the IPv4 and IPv6 address of the container.
Diffs (updated)
-
src/docker/docker.hpp 95e60a7dbbd6ccc659f70ca3dca8d13433f3ea07
src/docker/docker.cpp 192e170acce895f80df6c83e437da020489de468
src/docker/executor.cpp 99a62244b7805d25b926d0293cb62574981d2878
Diff: https://reviews.apache.org/r/61237/diff/3/
Changes: https://reviews.apache.org/r/61237/diff/2-3/
Testing
---
Start agent:
sudo ./bin/mesos-agent.sh --ip=10.0.2.15 --master=10.0.2.15:5050
--work_dir=/tmp/mesos --containerizers=mesos,docker
Start master:
sudo ./bin/mesos-master.sh --ip=10.0.2.15 --work_dir=/tmp/mesos-master
Use mesos-execute to launch a docker container on an IPv6 network:
sudo src/mesos-execute --master=10.0.2.15:5050
--task=file:///home/vagrant/dev/mesos_tasks/docker.json 2> log.txt
cat /home/vagrant/dev/mesos_tasks/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": "USER"
},
"network_infos": [
{
"name": "ipv6"
}
]
}
}
Curl the Mesos state to make sure that the right IPv6 addresses are reflected
for the container:
curl http://10.0.2.15:5051/state | jq
"role": "*",
"statuses": [
{
"state": "TASK_RUNNING",
"timestamp": 1501288232.79384,
"container_status": {
"container_id": {
"value": "59319a5b-a28b-46bf-a20f-8472e6598abd"
},
"network_infos": [
{
"ip_addresses": [
{
"protocol": "IPv4",
"ip_address": "172.19.0.2"
},
{
"protocol": "IPv6",
"ip_address": "fd00:0:0:1::2"
}
],
"name": "ipv6"
}
]
}
}
],
"container": {
"type": "DOCKER",
"docker": {
"image": "nginx",
"network": "USER",
"privileged": false
},
"network_infos": [
{
"name": "ipv6"
}
]
}
}
],
Thanks,
Avinash sridharan
Re: Review Request 61434: Updated health-checks.md to include description of generalized checks.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61434/#review182495 --- docs/health-checks.md Line 29 (original), 29 (patched) <https://reviews.apache.org/r/61434/#comment258403> s/defines/which defines? docs/health-checks.md Lines 162 (patched) <https://reviews.apache.org/r/61434/#comment258408> s/the following/to the following? docs/health-checks.md Lines 204 (patched) <https://reviews.apache.org/r/61434/#comment258410> I think we also need to mention that the expectation here is that task is listening on the `loopback` interface along with any other routeable interface to which it might be listening. docs/health-checks.md Lines 208 (patched) <https://reviews.apache.org/r/61434/#comment258409> how is the `host` resolved? It's not necessary that it will resolve to `127.0.0.1`? docs/health-checks.md Lines 232 (patched) <https://reviews.apache.org/r/61434/#comment258411> Ditto to the comments on the `HTTP Checks` section. docs/health-checks.md Line 265 (original), 494 (patched) <https://reviews.apache.org/r/61434/#comment258412> Why do we need to share the `mnt` namespace? This is already done by the executor for `MesosContainerizer` so why does the `checker` need to do this if it is running as part of the executor? docs/health-checks.md Line 267 (original), 496 (patched) <https://reviews.apache.org/r/61434/#comment258413> A bit confused here? I thought only `Health checks` are supported for `docker executor` and not `checks`? - Avinash sridharan On Aug. 4, 2017, 6:14 p.m., Alexander Rukletsov wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/61434/ > --- > > (Updated Aug. 4, 2017, 6:14 p.m.) > > > Review request for mesos, Gastón Kleiman, Greg Mann, and Vinod Kone. > > > Bugs: MESOS-7349 > https://issues.apache.org/jira/browse/MESOS-7349 > > > Repository: mesos > > > Description > --- > > See summary. > > > Diffs > - > > docs/health-checks.md d0749347d1f9398004e56b03503d655b5fba75f4 > > > Diff: https://reviews.apache.org/r/61434/diff/1/ > > > Testing > --- > > https://gist.github.com/rukletsov/517a136cb6a0e8b909db30b36b13eded > > Additionally rendered in MacDown. > > > Thanks, > > Alexander Rukletsov > >
Re: Review Request 60500: Introduced `--default_container_dns` agent flag.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60500/#review182068 --- Ship it! Ship It! - Avinash sridharan On Aug. 3, 2017, 2:57 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60500/ > --- > > (Updated Aug. 3, 2017, 2:57 a.m.) > > > Review request for mesos, Avinash sridharan and Jie Yu. > > > Bugs: MESOS-7709 > https://issues.apache.org/jira/browse/MESOS-7709 > > > Repository: mesos > > > Description > --- > > Introduced `--default_container_dns` agent flag. > > > Diffs > - > > docs/configuration.md 5449b92409baa6118d1ca215f771a30bbbf9f96e > src/messages/flags.hpp 70ad58c33067a0b058deafdcc4ffc5f554b6ca72 > src/messages/flags.proto e87075f19714739b93d9f4aa33ea8686a1bb5613 > src/slave/flags.hpp bf9adf03847a113306c1df2d71e839b90ada7dd3 > src/slave/flags.cpp a4c1a0c62af5506bbeb1d26d2fd9ca0ba11d8ad7 > > > Diff: https://reviews.apache.org/r/60500/diff/8/ > > > Testing > --- > > > Thanks, > > Qian Zhang > >
Re: Review Request 61237: Updated docker executor to return IPv6 address of a container.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61237/
---
(Updated Aug. 2, 2017, 6:57 p.m.)
Review request for mesos, Jie Yu and Qian Zhang.
Changes
---
Addressed Qian's comments.
Bugs: MESOS-7807
https://issues.apache.org/jira/browse/MESOS-7807
Repository: mesos
Description
---
A docker container can have a v4 and a v6 address. Currently the docker
executor was returning only the IPv4 address. This changes allows the
executor to return the IPv4 and IPv6 address of the container.
Diffs (updated)
-
src/docker/docker.hpp 5593cb635e073334c6c2566be3d803cd7febb1c3
src/docker/docker.cpp 8081c0203bf62cf62aa3b93d745f0e829ad65509
src/docker/executor.cpp e03f24461ec7b19cacae79c020406f0a475f2d19
Diff: https://reviews.apache.org/r/61237/diff/2/
Changes: https://reviews.apache.org/r/61237/diff/1-2/
Testing
---
Start agent:
sudo ./bin/mesos-agent.sh --ip=10.0.2.15 --master=10.0.2.15:5050
--work_dir=/tmp/mesos --containerizers=mesos,docker
Start master:
sudo ./bin/mesos-master.sh --ip=10.0.2.15 --work_dir=/tmp/mesos-master
Use mesos-execute to launch a docker container on an IPv6 network:
sudo src/mesos-execute --master=10.0.2.15:5050
--task=file:///home/vagrant/dev/mesos_tasks/docker.json 2> log.txt
cat /home/vagrant/dev/mesos_tasks/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": "USER"
},
"network_infos": [
{
"name": "ipv6"
}
]
}
}
Curl the Mesos state to make sure that the right IPv6 addresses are reflected
for the container:
curl http://10.0.2.15:5051/state | jq
"role": "*",
"statuses": [
{
"state": "TASK_RUNNING",
"timestamp": 1501288232.79384,
"container_status": {
"container_id": {
"value": "59319a5b-a28b-46bf-a20f-8472e6598abd"
},
"network_infos": [
{
"ip_addresses": [
{
"protocol": "IPv4",
"ip_address": "172.19.0.2"
},
{
"protocol": "IPv6",
"ip_address": "fd00:0:0:1::2"
}
],
"name": "ipv6"
}
]
}
}
],
"container": {
"type": "DOCKER",
"docker": {
"image": "nginx",
"network": "USER",
"privileged": false
},
"network_infos": [
{
"name": "ipv6"
}
]
}
}
],
Thanks,
Avinash sridharan
Re: Review Request 61245: Added a test `DefaultContainerDNSFlagTest.ValidateFlag`.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61245/#review181997
---
Fix it, then Ship it!
Ship It!
src/tests/slave_tests.cpp
Lines 7942 (patched)
<https://reviews.apache.org/r/61245/#comment257812>
We could do something like:
```
string network_mode = (containerizer == "mesos" ? "CNI" : "USER");
defaultContainerDNSInfo =
"--default_container_dns={"
" \"" + containerizer + "\": [\n"
"{\n"
" \"network_mode\": + network_mode+ ",\n"
" \"dns\": {\n"
"\"nameservers\": [ \"8.8.8.8\" ]\n"
" }\n"
"},\n"
"{\n"
" \"network_mode\": \"CNI\",\n"
" \"dns\": {\n"
"\"nameservers\": [ \"8.8.8.8\" ]\n"
" }\n"
"}\n"
" ]\n"
"}";
```
That way we wouldn't need the `else` ocondtitional below ?
- Avinash sridharan
On Aug. 2, 2017, 8:15 a.m., Qian Zhang wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61245/
> ---
>
> (Updated Aug. 2, 2017, 8:15 a.m.)
>
>
> Review request for mesos, Avinash sridharan and Jie Yu.
>
>
> Bugs: MESOS-7709
> https://issues.apache.org/jira/browse/MESOS-7709
>
>
> Repository: mesos
>
>
> Description
> ---
>
> Added a test `DefaultContainerDNSFlagTest.ValidateFlag`.
>
>
> Diffs
> -
>
> src/tests/slave_tests.cpp a8c3a7749472739c72de2deebe6220c724e635bb
>
>
> Diff: https://reviews.apache.org/r/61245/diff/3/
>
>
> Testing
> ---
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 60793: Added a test `DefaultContainerDNSCniTest.ROOT_VerifyDefaultDNS`.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60793/#review181994
---
src/tests/containerizer/cni_isolator_tests.cpp
Lines 1777 (patched)
<https://reviews.apache.org/r/60793/#comment257811>
Can we add one more test to this sequence? A test where we are setting the
default and specific DNS entry. The expectation would be to always see the
specific DNS entry. So the extra parameter would look like:
```
{
"mesos": [
{
"network_mode": "CNI",
"network_name": "__MESOS_TEST__",
"dns": {
"nameservers": [ "8.8.8.8", "8.8.4.4" ],
"domain": "mesos.apache.org",
"search": [ "a.mesos.apache.org", "a.mesos.apache.org" ],
"options": [ "timeout:3", "attempts:2" ]
}
},
{
"network_mode": "CNI",
"dns": {
"nameservers": [ "8.8.8.9", "8.8.4.5" ],
"domain": "new.mesos.apache.org",
"search": [ "b.mesos.apache.org", "b.mesos.apache.org" ],
"options": [ "timeout:10", "attempts:2" ]
}
}
]
- Avinash sridharan
On Aug. 2, 2017, 8:13 a.m., Qian Zhang wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60793/
> ---
>
> (Updated Aug. 2, 2017, 8:13 a.m.)
>
>
> Review request for mesos, Avinash sridharan and Jie Yu.
>
>
> Bugs: MESOS-7709
> https://issues.apache.org/jira/browse/MESOS-7709
>
>
> Repository: mesos
>
>
> Description
> ---
>
> Added a test `DefaultContainerDNSCniTest.ROOT_VerifyDefaultDNS`.
>
>
> Diffs
> -
>
> src/tests/containerizer/cni_isolator_tests.cpp
> ae0980bd671849fcd3e19941b33c7d3b09fdae7c
>
>
> Diff: https://reviews.apache.org/r/60793/diff/4/
>
>
> Testing
> ---
>
> sudo make check
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 60600: Set container DNS with `--default_container_dns` in CNI isolator.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60600/#review181937
---
Ship it!
Ship It!
- Avinash sridharan
On Aug. 1, 2017, 8:49 a.m., Qian Zhang wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60600/
> ---
>
> (Updated Aug. 1, 2017, 8:49 a.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 CNI isolator.
>
>
> Diffs
> -
>
> src/slave/containerizer/mesos/isolators/network/cni/cni.hpp
> a04700bda15c1f060375e502330e94246a9f554e
> src/slave/containerizer/mesos/isolators/network/cni/cni.cpp
> 831bc7d0774a0ad3c8cbd7f42d4a3f8bd34d3243
>
>
> Diff: https://reviews.apache.org/r/60600/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 /opt/cni/net_configs/net1
> {
> "name": "net1",
> "type": "bridge",
> "bridge": "br1",
> "isGateway": true,
> "ipMasq": true,
> "ipam": {
> "type": "host-local",
> "subnet": "192.168.1.0/24",
> "routes": [
> { "dst": "0.0.0.0/0" }
> ]
> }
> }
>
> $ cat /opt/cni/net_configs/net2
> {
> "name": "net2",
> "type": "bridge",
> "bridge": "br2",
> "isGateway": true,
> "ipMasq": true,
> "ipam": {
> "type": "host-local",
> "subnet": "192.168.2.0/24",
> "routes": [
> { "dst": "0.0.0.0/0" }
> ]
> },
> "dns": {
> "nameservers": [ "8.8.4.4" ],
> "domain": "net2.com",
> "search": [ "yyy.com" ],
> "options": [ "attempts:3" ]
> }
> }
>
> $ cat /home/stack/dns.json
> {
> "mesos": [
> {
> "network_mode": "CNI",
> "network_name": "net1",
> "dns": {
> "nameservers": [ "8.8.8.8" ],
> "search": [ "xxx.com" ],
> "options": [ "timeout:4" ]
> }
> }
> ]
> }
> ```
>
> 3. Launch a unified container with `mesos-execute`.
> ```
> $ sudo src/mesos-execute --master=192.168.122.216:5050
> --task=file:///home/stack/task.json
>
> $ cat /home/stack/task.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": {
> "value": "sleep 300"
> },
> "container": {
> "type": "MESOS",
> "mesos": {
> "image": {
> "type": "DOCKER",
> "docker": {
> "name": "busybox"
> }
> }
> },
> "network_infos": [
> {
> "name": "net1"
> },
> {
> "name": "net2"
> }
> ]
> }
> }
> ```
>
> 4. Check the DNS configuration of the unified container.
> ```
> $ ps -ef | grep sleep
> root 20060 20037 2 21:45 ?00:00:00 sh -c sleep 300
> root 20074 20060 0 21:45 ?00:00:00 sleep 300
>
> $ sudo nsenter -t 20060 -m -u -n cat /etc/resolv.conf
> domain net2.com
> search yyy.com xxx.com
> options attempts:3 timeout:4
> nameserver 8.8.4.4
> nameserver 8.8.8.8
> ```
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 60558: Set container DNS with `--default_container_dns` in Docker executor.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60558/#review181933
---
Fix it, then Ship it!
Ship It!
src/docker/docker.cpp
Lines 748 (patched)
<https://reviews.apache.org/r/60558/#comment257757>
Minor nit, maybe declare:
`using mesos::internal::ContainerDNSInfo`? Will shorten the code here.
src/docker/docker.cpp
Lines 764 (patched)
<https://reviews.apache.org/r/60558/#comment257758>
s/,it applies/. It applies
src/docker/docker.cpp
Lines 909 (patched)
<https://reviews.apache.org/r/60558/#comment257761>
Failure(
"--dns option ..."); ?
- Avinash sridharan
On Aug. 1, 2017, 8:41 a.m., Qian Zhang wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60558/
> ---
>
> (Updated Aug. 1, 2017, 8:41 a.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/7/
>
>
> 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 IDIMAGE COMMAND CREATED
> STATUS PORTS NAMES
> ca642bf31a9fnginx "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
>
>
Re: Review Request 61245: Added a test `SlaveTest.ValidateDefaultContainerDNSFlag`.
> On Aug. 1, 2017, 6:51 a.m., Avinash sridharan wrote:
> > src/tests/slave_tests.cpp
> > Lines 7934 (patched)
> > <https://reviews.apache.org/r/61245/diff/1/?file=1785603#file1785603line7934>
> >
> > This code seems to be duplicated. I think you can use a parameterized
> > test over here similar to this:
> >
> > https://github.com/apache/mesos/blob/master/src/tests/default_executor_tests.cpp#L104
>
> Qian Zhang wrote:
> Can you elaborate a bit about this? Did you mean the strings I assigned
> to `argv[3]` for CNI and CNM are duplicated? They are very similar but still
> have difference, e.g., for CNI, the field name is `mesos`, but for CNM, the
> field name is `docker`, and also the network modes are different (`CNI` v.s.
> `USER`). So we need different strings anyway, I am not sure how we can
> parameterize them.
>
> Qian Zhang wrote:
> Or the duplicated you were talking about is:
> ```
> slave::Flags flags;
> Try load = flags.load(None(), argc, argv);
> EXPECT_ERROR(load);
> ```
> And then move all those strings into `INSTANTIATE_TEST_CASE_P()`?
Qian, I was talking about this JSON:
```
--default_container_dns={"
" \"mesos\": [\n"
"{\n"
" \"network_mode\": \"UNKNOWN\",\n"
" \"dns\": {\n"
"\"nameservers\": [ \"8.8.8.8\" ]\n"
" }\n"
"}\n"
" ]\n"
"}";
```
Since this is just a string and the only thing that seems change is the key
(mesos|docker) thought making parameterized will simplify the code.
- Avinash
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61245/#review181880
---
On Aug. 1, 2017, 10:07 a.m., Qian Zhang wrote:
>
> -----------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61245/
> ---
>
> (Updated Aug. 1, 2017, 10:07 a.m.)
>
>
> Review request for mesos, Avinash sridharan and Jie Yu.
>
>
> Bugs: MESOS-7709
> https://issues.apache.org/jira/browse/MESOS-7709
>
>
> Repository: mesos
>
>
> Description
> ---
>
> Added a test `SlaveTest.ValidateDefaultContainerDNSFlag`.
>
>
> Diffs
> -
>
> src/tests/slave_tests.cpp a089cc47eae41cd6baeffd3f4a7ee7c7984aacbd
>
>
> Diff: https://reviews.apache.org/r/61245/diff/2/
>
>
> Testing
> ---
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 61245: Added a test `SlaveTest.ValidateDefaultContainerDNSFlag`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61245/#review181880 --- src/tests/slave_tests.cpp Lines 7878 (patched) <https://reviews.apache.org/r/61245/#comment257671> s/inexistent/non-existent src/tests/slave_tests.cpp Lines 7898 (patched) <https://reviews.apache.org/r/61245/#comment257672> s/inexistent/non-existent src/tests/slave_tests.cpp Lines 7934 (patched) <https://reviews.apache.org/r/61245/#comment257674> This code seems to be duplicated. I think you can use a parameterized test over here similar to this: https://github.com/apache/mesos/blob/master/src/tests/default_executor_tests.cpp#L104 src/tests/slave_tests.cpp Lines 7955-7972 (patched) <https://reviews.apache.org/r/61245/#comment257675> Ditto for using parameterized tests here for CNI and CNM. - Avinash sridharan On July 30, 2017, 3:32 p.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/61245/ > --- > > (Updated July 30, 2017, 3: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 > --- > > Added a test `SlaveTest.ValidateDefaultContainerDNSFlag`. > > > Diffs > - > > src/tests/slave_tests.cpp a089cc47eae41cd6baeffd3f4a7ee7c7984aacbd > > > Diff: https://reviews.apache.org/r/61245/diff/1/ > > > Testing > --- > > > Thanks, > > Qian Zhang > >
Re: Review Request 61219: Added a test `CniIsolatorTest.ROOT_VerifyDefaultDNSWildcardMatch`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61219/#review181879 --- Fix it, then Ship it! Ship It! src/tests/containerizer/cni_isolator_tests.cpp Lines 1383-1385 (patched) <https://reviews.apache.org/r/61219/#comment257670> Can we change this to: A DNS information with `network_mode = CNI`, but without a network name, acts as a wild card match making it the default DNS for any CNI network not specified in the `--default_container_dns` flag. - Avinash sridharan On July 28, 2017, 2:35 p.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/61219/ > --- > > (Updated July 28, 2017, 2:35 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 > --- > > Added a test `CniIsolatorTest.ROOT_VerifyDefaultDNSWildcardMatch`. > > > Diffs > - > > src/tests/containerizer/cni_isolator_tests.cpp > ae0980bd671849fcd3e19941b33c7d3b09fdae7c > > > Diff: https://reviews.apache.org/r/61219/diff/1/ > > > Testing > --- > > > Thanks, > > Qian Zhang > >
Re: Review Request 60793: Added a test `CniIsolatorTest.ROOT_VerifyDefaultDNS`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60793/#review181876 --- Ship it! Ship It! - Avinash sridharan On July 25, 2017, 6:08 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60793/ > --- > > (Updated July 25, 2017, 6:08 a.m.) > > > Review request for mesos, Avinash sridharan and Jie Yu. > > > Bugs: MESOS-7709 > https://issues.apache.org/jira/browse/MESOS-7709 > > > Repository: mesos > > > Description > --- > > Added a test `CniIsolatorTest.ROOT_VerifyDefaultDNS`. > > > Diffs > - > > src/tests/containerizer/cni_isolator_tests.cpp > ae0980bd671849fcd3e19941b33c7d3b09fdae7c > > > Diff: https://reviews.apache.org/r/60793/diff/3/ > > > Testing > --- > > sudo make check > > > Thanks, > > Qian Zhang > >
Re: Review Request 60761: Added a test `DockerContainerizerTest.ROOT_DOCKER_DefaultDNS`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60761/#review181864 --- Ship it! Ship It! - Avinash sridharan On July 25, 2017, 6:07 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60761/ > --- > > (Updated July 25, 2017, 6:07 a.m.) > > > Review request for mesos, Avinash sridharan and Jie Yu. > > > Bugs: MESOS-7709 > https://issues.apache.org/jira/browse/MESOS-7709 > > > Repository: mesos > > > Description > --- > > Added a test `DockerContainerizerTest.ROOT_DOCKER_DefaultDNS`. > > > Diffs > - > > src/tests/containerizer/docker_containerizer_tests.cpp > 1e85a79f812399270575ea4a64db10e72f40e648 > > > Diff: https://reviews.apache.org/r/60761/diff/3/ > > > Testing > --- > > sudo make check > > > Thanks, > > Qian Zhang > >
Re: Review Request 60600: Set container DNS with `--default_container_dns` in CNI isolator.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60600/#review181863
---
src/slave/containerizer/mesos/isolators/network/cni/cni.cpp
Lines 1008-1028 (patched)
<https://reviews.apache.org/r/60600/#comment257641>
Just a thought? Storing the CNI network DNS entries in a map and the
defaultDNS also in an `Option` at startup might simplify this code a lot over
here?
- Avinash sridharan
On July 28, 2017, 2:34 p.m., Qian Zhang wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60600/
> ---
>
> (Updated July 28, 2017, 2:34 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 CNI isolator.
>
>
> Diffs
> -
>
> src/slave/containerizer/mesos/isolators/network/cni/cni.cpp
> 831bc7d0774a0ad3c8cbd7f42d4a3f8bd34d3243
>
>
> Diff: https://reviews.apache.org/r/60600/diff/5/
>
>
> 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 /opt/cni/net_configs/net1
> {
> "name": "net1",
> "type": "bridge",
> "bridge": "br1",
> "isGateway": true,
> "ipMasq": true,
> "ipam": {
> "type": "host-local",
> "subnet": "192.168.1.0/24",
> "routes": [
> { "dst": "0.0.0.0/0" }
> ]
> }
> }
>
> $ cat /opt/cni/net_configs/net2
> {
> "name": "net2",
> "type": "bridge",
> "bridge": "br2",
> "isGateway": true,
> "ipMasq": true,
> "ipam": {
> "type": "host-local",
> "subnet": "192.168.2.0/24",
> "routes": [
> { "dst": "0.0.0.0/0" }
> ]
> },
> "dns": {
> "nameservers": [ "8.8.4.4" ],
> "domain": "net2.com",
> "search": [ "yyy.com" ],
> "options": [ "attempts:3" ]
> }
> }
>
> $ cat /home/stack/dns.json
> {
> "mesos": [
> {
> "network_mode": "CNI",
> "network_name": "net1",
> "dns": {
> "nameservers": [ "8.8.8.8" ],
> "search": [ "xxx.com" ],
> "options": [ "timeout:4" ]
> }
> }
> ]
> }
> ```
>
> 3. Launch a unified container with `mesos-execute`.
> ```
> $ sudo src/mesos-execute --master=192.168.122.216:5050
> --task=file:///home/stack/task.json
>
> $ cat /home/stack/task.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": {
> "value": "sleep 300"
> },
> "container": {
> "type": "MESOS",
> "mesos": {
> "image": {
> "type": "DOCKER",
> "docker": {
> "name": "busybox"
> }
> }
> },
> "network_infos": [
> {
> "name": "net1"
> },
> {
> "name": "net2"
> }
> ]
> }
> }
> ```
>
> 4. Check the DNS configuration of the unified container.
> ```
> $ ps -ef | grep sleep
> root 20060 20037 2 21:45 ?00:00:00 sh -c sleep 300
> root 20074 20060 0 21:45 ?00:00:00 sleep 300
>
> $ sudo nsenter -t 20060 -m -u -n cat /etc/resolv.conf
> domain net2.com
> search yyy.com xxx.com
> options attempts:3 timeout:4
> nameserver 8.8.4.4
> nameserver 8.8.8.8
> ```
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 60558: Set container DNS with `--default_container_dns` in Docker executor.
---
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 bridgeDNS;
hashmap 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": {
&g
Review Request 61237: Updated docker executor to return IPv6 address of a container.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61237/
---
Review request for mesos, Jie Yu and Qian Zhang.
Bugs: MESOS-7807
https://issues.apache.org/jira/browse/MESOS-7807
Repository: mesos
Description
---
A docker container can have a v4 and a v6 address. Currently the docker
executor was returning only the IPv4 address. This changes allows the
executor to return the IPv4 and IPv6 address of the container.
Diffs
-
src/docker/docker.hpp 5593cb635e073334c6c2566be3d803cd7febb1c3
src/docker/docker.cpp 8081c0203bf62cf62aa3b93d745f0e829ad65509
src/docker/executor.cpp e03f24461ec7b19cacae79c020406f0a475f2d19
Diff: https://reviews.apache.org/r/61237/diff/1/
Testing
---
Start agent:
sudo ./bin/mesos-agent.sh --ip=10.0.2.15 --master=10.0.2.15:5050
--work_dir=/tmp/mesos --containerizers=mesos,docker
Start master:
sudo ./bin/mesos-master.sh --ip=10.0.2.15 --work_dir=/tmp/mesos-master
Use mesos-execute to launch a docker container on an IPv6 network:
sudo src/mesos-execute --master=10.0.2.15:5050
--task=file:///home/vagrant/dev/mesos_tasks/docker.json 2> log.txt
cat /home/vagrant/dev/mesos_tasks/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": "USER"
},
"network_infos": [
{
"name": "ipv6"
}
]
}
}
Curl the Mesos state to make sure that the right IPv6 addresses are reflected
for the container:
curl http://10.0.2.15:5051/state | jq
"role": "*",
"statuses": [
{
"state": "TASK_RUNNING",
"timestamp": 1501288232.79384,
"container_status": {
"container_id": {
"value": "59319a5b-a28b-46bf-a20f-8472e6598abd"
},
"network_infos": [
{
"ip_addresses": [
{
"protocol": "IPv4",
"ip_address": "172.19.0.2"
},
{
"protocol": "IPv6",
"ip_address": "fd00:0:0:1::2"
}
],
"name": "ipv6"
}
]
}
}
],
"container": {
"type": "DOCKER",
"docker": {
"image": "nginx",
"network": "USER",
"privileged": false
},
"network_infos": [
{
"name": "ipv6"
}
]
}
}
],
Thanks,
Avinash sridharan
Re: Review Request 60761: Added a test `DockerContainerizerTest.ROOT_DOCKER_DefaultDNS`.
> On July 28, 2017, 12:35 a.m., Avinash sridharan wrote: > > src/tests/containerizer/docker_containerizer_tests.cpp > > Lines 4357 (patched) > > <https://reviews.apache.org/r/60761/diff/3/?file=1782044#file1782044line4357> > > > > Can we have a test for checking validation error on setting up the HOST > > MODE? > > Qian Zhang wrote: > Did you mean adding a test to test the validation code in the lambda that > we added in `src/slave/flags.cpp` for the `--default_container_dns` flag? If > so, then I think `docker_containerizer_tests.cpp` may not be a good place for > such test because `docker_containerizer_tests.cpp` should contain the > end-to-end tests, but for testing the validation code we do not even need to > start the agent. Maybe just add a test in `flags_tests.cpp`? And in another > hand, for the other agent flags which has the validation code in lambda, it > seems there is no related test for them, so do we really need such test for > testing the validation code of `--default_container_dns`? > > BTW, I was thinking to add a test for wildcard match of Docker > user-defined network, but the problem is we can not assume any user-defined > networks in the test env, but there must be a bridge network, so currently I > only have the test for the bridge network. Agreed !! if we can add the flag validation tests for the flags tests that should be good enough. Wildcard match test for BRIDGE mode is good enough I think. - Avinash --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60761/#review181628 --- On July 25, 2017, 6:07 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60761/ > ------- > > (Updated July 25, 2017, 6:07 a.m.) > > > Review request for mesos, Avinash sridharan and Jie Yu. > > > Bugs: MESOS-7709 > https://issues.apache.org/jira/browse/MESOS-7709 > > > Repository: mesos > > > Description > --- > > Added a test `DockerContainerizerTest.ROOT_DOCKER_DefaultDNS`. > > > Diffs > - > > src/tests/containerizer/docker_containerizer_tests.cpp > 1e85a79f812399270575ea4a64db10e72f40e648 > > > Diff: https://reviews.apache.org/r/60761/diff/3/ > > > Testing > --- > > sudo make check > > > Thanks, > > Qian Zhang > >
Re: Review Request 60558: Set container DNS with `--default_container_dns` in Docker executor.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60558/#review181634
---
src/docker/docker.cpp
Lines 755 (patched)
<https://reviews.apache.org/r/60558/#comment257257>
I think we need to add the wild card `*` match for `USER` network here.
Basically if mode is `USER` and `network_name` is present we do a wild card
match for all networks.
We want to set the DNS for the most specific match.
- Avinash sridharan
On July 25, 2017, 6:05 a.m., Qian Zhang wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60558/
> ---
>
> (Updated July 25, 2017, 6:05 a.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/5/
>
>
> 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 IDIMAGE COMMAND CREATED
> STATUS PORTS NAMES
> ca642bf31a9fnginx "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
>
>
Re: Review Request 60600: Set container DNS with `--default_container_dns` in CNI isolator.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60600/#review181633
---
src/slave/containerizer/mesos/isolators/network/cni/cni.cpp
Lines 1011 (patched)
<https://reviews.apache.org/r/60600/#comment257255>
We are missing the `*` wildcard match option here,.i.e, if the
`network_mode = CNI` and the `network_name` is not set it means it is the
default DNS config for all the CNI networks. While setting up DNS we want to
setup the DNS for the most specific match.
- Avinash sridharan
On July 25, 2017, 6:06 a.m., Qian Zhang wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60600/
> ---
>
> (Updated July 25, 2017, 6:06 a.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 CNI isolator.
>
>
> Diffs
> -
>
> src/slave/containerizer/mesos/isolators/network/cni/cni.cpp
> 831bc7d0774a0ad3c8cbd7f42d4a3f8bd34d3243
>
>
> Diff: https://reviews.apache.org/r/60600/diff/4/
>
>
> 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 /opt/cni/net_configs/net1
> {
> "name": "net1",
> "type": "bridge",
> "bridge": "br1",
> "isGateway": true,
> "ipMasq": true,
> "ipam": {
> "type": "host-local",
> "subnet": "192.168.1.0/24",
> "routes": [
> { "dst": "0.0.0.0/0" }
> ]
> }
> }
>
> $ cat /opt/cni/net_configs/net2
> {
> "name": "net2",
> "type": "bridge",
> "bridge": "br2",
> "isGateway": true,
> "ipMasq": true,
> "ipam": {
> "type": "host-local",
> "subnet": "192.168.2.0/24",
> "routes": [
> { "dst": "0.0.0.0/0" }
> ]
> },
> "dns": {
> "nameservers": [ "8.8.4.4" ],
> "domain": "net2.com",
> "search": [ "yyy.com" ],
> "options": [ "attempts:3" ]
> }
> }
>
> $ cat /home/stack/dns.json
> {
> "mesos": [
> {
> "network_mode": "CNI",
> "network_name": "net1",
> "dns": {
> "nameservers": [ "8.8.8.8" ],
> "search": [ "xxx.com" ],
> "options": [ "timeout:4" ]
> }
> }
> ]
> }
> ```
>
> 3. Launch a unified container with `mesos-execute`.
> ```
> $ sudo src/mesos-execute --master=192.168.122.216:5050
> --task=file:///home/stack/task.json
>
> $ cat /home/stack/task.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": {
> "value": "sleep 300"
> },
> "container": {
> "type": "MESOS",
> "mesos": {
> "image": {
> "type": "DOCKER",
> "docker": {
> "name": "busybox"
> }
> }
> },
> "network_infos": [
> {
> "name": "net1"
> },
> {
> "name": "net2"
> }
> ]
> }
> }
> ```
>
> 4. Check the DNS configuration of the unified container.
> ```
> $ ps -ef | grep sleep
> root 20060 20037 2 21:45 ?00:00:00 sh -c sleep 300
> root 20074 20060 0 21:45 ?00:00:00 sleep 300
>
> $ sudo nsenter -t 20060 -m -u -n cat /etc/resolv.conf
> domain net2.com
> search yyy.com xxx.com
> options attempts:3 timeout:4
> nameserver 8.8.4.4
> nameserver 8.8.8.8
> ```
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 60793: Added a test `CniIsolatorTest.ROOT_VerifyDefaultDNS`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60793/#review181630 --- src/tests/containerizer/cni_isolator_tests.cpp Lines 1226 (patched) <https://reviews.apache.org/r/60793/#comment257253> Similar to the `DockerContainerizer` can we add a test for the flags themselves to check that we don't allow setting DNS for HOST mode? Also, would be great to test the matching algorithm we use to setup the DNS when we have multiple DNS entries. - Avinash sridharan On July 25, 2017, 6:08 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60793/ > --- > > (Updated July 25, 2017, 6:08 a.m.) > > > Review request for mesos, Avinash sridharan and Jie Yu. > > > Bugs: MESOS-7709 > https://issues.apache.org/jira/browse/MESOS-7709 > > > Repository: mesos > > > Description > --- > > Added a test `CniIsolatorTest.ROOT_VerifyDefaultDNS`. > > > Diffs > - > > src/tests/containerizer/cni_isolator_tests.cpp > ae0980bd671849fcd3e19941b33c7d3b09fdae7c > > > Diff: https://reviews.apache.org/r/60793/diff/3/ > > > Testing > --- > > sudo make check > > > Thanks, > > Qian Zhang > >
Re: Review Request 60761: Added a test `DockerContainerizerTest.ROOT_DOCKER_DefaultDNS`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60761/#review181628 --- Fix it, then Ship it! Ship It! src/tests/containerizer/docker_containerizer_tests.cpp Lines 4357 (patched) <https://reviews.apache.org/r/60761/#comment257252> Can we have a test for checking validation error on setting up the HOST MODE? - Avinash sridharan On July 25, 2017, 6:07 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60761/ > --- > > (Updated July 25, 2017, 6:07 a.m.) > > > Review request for mesos, Avinash sridharan and Jie Yu. > > > Bugs: MESOS-7709 > https://issues.apache.org/jira/browse/MESOS-7709 > > > Repository: mesos > > > Description > --- > > Added a test `DockerContainerizerTest.ROOT_DOCKER_DefaultDNS`. > > > Diffs > - > > src/tests/containerizer/docker_containerizer_tests.cpp > 1e85a79f812399270575ea4a64db10e72f40e648 > > > Diff: https://reviews.apache.org/r/60761/diff/3/ > > > Testing > --- > > sudo make check > > > Thanks, > > Qian Zhang > >
Re: Review Request 60760: Parsed DNS related info from the output of `docker inspect`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60760/#review181627 --- Ship it! Ship It! - Avinash sridharan On July 11, 2017, 9:01 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60760/ > --- > > (Updated July 11, 2017, 9:01 a.m.) > > > Review request for mesos, Avinash sridharan and Jie Yu. > > > Bugs: MESOS-7709 > https://issues.apache.org/jira/browse/MESOS-7709 > > > Repository: mesos > > > Description > --- > > Parsed DNS related info from the output of `docker inspect`. > > > Diffs > - > > src/docker/docker.hpp 5593cb635e073334c6c2566be3d803cd7febb1c3 > src/docker/docker.cpp 8ca0c68836ea5d1a1186e79942f6daccf6c6da46 > > > Diff: https://reviews.apache.org/r/60760/diff/1/ > > > Testing > --- > > > Thanks, > > Qian Zhang > >
Re: Review Request 61075: Set container DNS with `--default_container_dns` in DockerContainerizer.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61075/#review181626 --- Fix it, then Ship it! Ship It! src/slave/containerizer/docker.cpp Lines 1333-1339 (patched) <https://reviews.apache.org/r/61075/#comment257251> Some edits to the comment: We need to pass `flags.default_container_dns` only when the agent is not running in a Docker container. This is to handle the case of launching a custom executor in a Docker container. If the agent is running in a Docker container (i.e., `flags.docker_mesos_image.isSome() == true`), that is the case of launching `mesos-docker-executor` in a Docker container with the Docker image `flags.docker_mesos_image`. In that case we already set `flags.default_container_dns` in the method `dockerFlags()`. - Avinash sridharan On July 24, 2017, 8 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/61075/ > --- > > (Updated July 24, 2017, 8 a.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 DockerContainerizer. > > > Diffs > - > > src/slave/containerizer/docker.cpp 2fe92272d7ac6d916371c55affe24598255f10eb > > > Diff: https://reviews.apache.org/r/61075/diff/1/ > > > Testing > --- > > > Thanks, > > Qian Zhang > >
Re: Review Request 60500: Introduced `--default_container_dns` agent flag.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60500/#review181437 --- Ship it! Ship It! - Avinash sridharan On July 26, 2017, 2:41 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60500/ > --- > > (Updated July 26, 2017, 2:41 a.m.) > > > Review request for mesos, Avinash sridharan and Jie Yu. > > > Bugs: MESOS-7709 > https://issues.apache.org/jira/browse/MESOS-7709 > > > Repository: mesos > > > Description > --- > > Introduced `--default_container_dns` agent flag. > > > Diffs > - > > docs/configuration.md a06ac82e7b908601b9ba97011931276e6292102d > src/messages/flags.hpp 70ad58c33067a0b058deafdcc4ffc5f554b6ca72 > src/messages/flags.proto e87075f19714739b93d9f4aa33ea8686a1bb5613 > src/slave/flags.hpp bf9adf03847a113306c1df2d71e839b90ada7dd3 > src/slave/flags.cpp a4c1a0c62af5506bbeb1d26d2fd9ca0ba11d8ad7 > > > Diff: https://reviews.apache.org/r/60500/diff/7/ > > > Testing > --- > > > Thanks, > > Qian Zhang > >
Re: Review Request 60500: Introduced `--default_container_dns` agent flag.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60500/#review181349 --- src/slave/main.cpp Lines 411-457 (patched) <https://reviews.apache.org/r/60500/#comment256895> Shouldn't this be a \lambda? Similar to how we do error handling for the flags here: https://github.com/apache/mesos/blob/master/src/slave/flags.cpp#L329 Any specific reason we have it part of the code initialization? - Avinash sridharan On July 25, 2017, 6:03 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60500/ > --- > > (Updated July 25, 2017, 6:03 a.m.) > > > Review request for mesos, Avinash sridharan and Jie Yu. > > > Bugs: MESOS-7709 > https://issues.apache.org/jira/browse/MESOS-7709 > > > Repository: mesos > > > Description > --- > > Introduced `--default_container_dns` agent flag. > > > Diffs > - > > docs/configuration.md a06ac82e7b908601b9ba97011931276e6292102d > src/messages/flags.hpp 70ad58c33067a0b058deafdcc4ffc5f554b6ca72 > src/messages/flags.proto e87075f19714739b93d9f4aa33ea8686a1bb5613 > src/slave/flags.hpp bf9adf03847a113306c1df2d71e839b90ada7dd3 > src/slave/flags.cpp a4c1a0c62af5506bbeb1d26d2fd9ca0ba11d8ad7 > src/slave/main.cpp a4a8ced6825ece1b25003824d987ff83fb799ad7 > > > Diff: https://reviews.apache.org/r/60500/diff/6/ > > > Testing > --- > > > Thanks, > > Qian Zhang > >
Re: Review Request 60500: Introduced `--default_container_dns` agent flag.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60500/#review181234 --- src/slave/flags.cpp Lines 773 (patched) <https://reviews.apache.org/r/60500/#comment256729> We need a validation for the `HOST` mode here? - Avinash sridharan On July 24, 2017, 1:39 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60500/ > --- > > (Updated July 24, 2017, 1:39 a.m.) > > > Review request for mesos, Avinash sridharan and Jie Yu. > > > Bugs: MESOS-7709 > https://issues.apache.org/jira/browse/MESOS-7709 > > > Repository: mesos > > > Description > --- > > Introduced `--default_container_dns` agent flag. > > > Diffs > - > > docs/configuration.md a06ac82e7b908601b9ba97011931276e6292102d > src/messages/flags.hpp 70ad58c33067a0b058deafdcc4ffc5f554b6ca72 > src/messages/flags.proto e87075f19714739b93d9f4aa33ea8686a1bb5613 > src/slave/flags.hpp bf9adf03847a113306c1df2d71e839b90ada7dd3 > src/slave/flags.cpp a4c1a0c62af5506bbeb1d26d2fd9ca0ba11d8ad7 > src/slave/main.cpp a4a8ced6825ece1b25003824d987ff83fb799ad7 > > > Diff: https://reviews.apache.org/r/60500/diff/5/ > > > Testing > --- > > > Thanks, > > Qian Zhang > >
Re: Review Request 60500: Introduced `--default_container_dns` agent flag.
> On July 23, 2017, 3:22 p.m., Avinash sridharan wrote: > > src/messages/flags.proto > > Lines 69 (patched) > > <https://reviews.apache.org/r/60500/diff/4/?file=1781100#file1781100line69> > > > > Qian, sorry, should have pointed this out earlier. I think we decided > > that for Docker we can support HOST mode as well? So we can remove this > > comment. > > Qian Zhang wrote: > I remember that we decided to support BRIDGE mode for Docker, but not > support HOST mode for both CNI and Docker so that they can have a consistent > behavior regarding HOST mode. Can you please double confirm with Jie? Ah ok!! I am fine with not supporting HOST mode for docker. The use-case isn't that strong. Probably the confusion arose on my part because your patches already did support HOST mode for Docker so though we were planning to keep that support. Lets drop this in that case and explicitly not support HOST mode for docker. - Avinash --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60500/#review181181 --- On July 24, 2017, 1:39 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60500/ > ------- > > (Updated July 24, 2017, 1:39 a.m.) > > > Review request for mesos, Avinash sridharan and Jie Yu. > > > Bugs: MESOS-7709 > https://issues.apache.org/jira/browse/MESOS-7709 > > > Repository: mesos > > > Description > --- > > Introduced `--default_container_dns` agent flag. > > > Diffs > - > > docs/configuration.md a06ac82e7b908601b9ba97011931276e6292102d > src/messages/flags.hpp 70ad58c33067a0b058deafdcc4ffc5f554b6ca72 > src/messages/flags.proto e87075f19714739b93d9f4aa33ea8686a1bb5613 > src/slave/flags.hpp bf9adf03847a113306c1df2d71e839b90ada7dd3 > src/slave/flags.cpp a4c1a0c62af5506bbeb1d26d2fd9ca0ba11d8ad7 > src/slave/main.cpp a4a8ced6825ece1b25003824d987ff83fb799ad7 > > > Diff: https://reviews.apache.org/r/60500/diff/5/ > > > Testing > --- > > > Thanks, > > Qian Zhang > >
Re: Review Request 60500: Introduced `--default_container_dns` agent flag.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60500/#review181181 --- src/messages/flags.proto Lines 69 (patched) <https://reviews.apache.org/r/60500/#comment256633> Qian, sorry, should have pointed this out earlier. I think we decided that for Docker we can support HOST mode as well? So we can remove this comment. src/messages/flags.proto Lines 90 (patched) <https://reviews.apache.org/r/60500/#comment256634> Seems a bit odd that we are re-using CNI over here for the DNS semantics. I am inclined to define a CNM/DNS protobuf that we can re-use here, following the semantics of keeping these pieces completely segregated? - Avinash sridharan On July 23, 2017, 9:29 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60500/ > --- > > (Updated July 23, 2017, 9:29 a.m.) > > > Review request for mesos, Avinash sridharan and Jie Yu. > > > Bugs: MESOS-7709 > https://issues.apache.org/jira/browse/MESOS-7709 > > > Repository: mesos > > > Description > --- > > Introduced `--default_container_dns` agent flag. > > > Diffs > - > > docs/configuration.md a06ac82e7b908601b9ba97011931276e6292102d > src/messages/flags.hpp 70ad58c33067a0b058deafdcc4ffc5f554b6ca72 > src/messages/flags.proto e87075f19714739b93d9f4aa33ea8686a1bb5613 > src/slave/flags.hpp bf9adf03847a113306c1df2d71e839b90ada7dd3 > src/slave/flags.cpp a4c1a0c62af5506bbeb1d26d2fd9ca0ba11d8ad7 > src/slave/main.cpp a4a8ced6825ece1b25003824d987ff83fb799ad7 > > > Diff: https://reviews.apache.org/r/60500/diff/4/ > > > Testing > --- > > > Thanks, > > Qian Zhang > >
Re: Review Request 60500: Introduced `--default_container_dns` agent flag.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60500/#review181121 --- src/messages/flags.proto Lines 41 (patched) <https://reviews.apache.org/r/60500/#comment256547> Can we do `MesosDNS`? Following the cadence of `ContainerInfo` and `MesosInfo`, this would be `ContainerDNS` and `MesosDNS`? src/messages/flags.proto Lines 43 (patched) <https://reviews.apache.org/r/60500/#comment256550> s/Not supported yet/Currently not supported. src/messages/flags.proto Lines 44 (patched) <https://reviews.apache.org/r/60500/#comment256548> I know we are using this `USER` mode from the docker terminology, but I would really like to move away from that terminology if possible and be explicit here that this is a CNI network? Instead of calling this mode `USER` can we just call it `CNI? src/messages/flags.proto Lines 49-54 (patched) <https://reviews.apache.org/r/60500/#comment256549> Can we change this to: This field is valid only when the `network_mode` is set to `CNI`. It is invalid when the `network_mode` is `HOST`. When the mode is `CNI` this field informs the `network/cni` isolator about the `CNI` network to which the DNS configuration applies. Also, if the mode is `CNI` and this field is not set it implies wild card semantics for the CNI network name, informing the `network/cni` isolator that the DNS configuration applies to "all" CNI networks. NOTE: If there are multiple `MesosDNS` configuration specified, when applyig a DNS configuration to a CNI network the container will always choose the most specific match based on the CNI network name. src/messages/flags.proto Lines 63 (patched) <https://reviews.apache.org/r/60500/#comment256551> s/Not supported yet/Currently not supported. src/messages/flags.proto Lines 70-75 (patched) <https://reviews.apache.org/r/60500/#comment256554> This field is valid only when the mode is `USER`. For `USER` mode this specified the CNM network to which this DNS configuration applies. Also, if the mode is `USER` and this field is not set it implies wild card semantics for CNM networks, i.e, this DNS configuration applies to all CNM networks. NOTE: In case there are multiple `DockerDNS` specified, when appliying the DNS configuration to a CNM network the containerizer will always choose the most specific match. - Avinash sridharan On July 20, 2017, 12:21 p.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60500/ > --- > > (Updated July 20, 2017, 12:21 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 > --- > > Introduced `--default_container_dns` agent flag. > > > Diffs > - > > docs/configuration.md a06ac82e7b908601b9ba97011931276e6292102d > src/messages/flags.hpp 70ad58c33067a0b058deafdcc4ffc5f554b6ca72 > src/messages/flags.proto e87075f19714739b93d9f4aa33ea8686a1bb5613 > src/slave/flags.hpp bf9adf03847a113306c1df2d71e839b90ada7dd3 > src/slave/flags.cpp a4c1a0c62af5506bbeb1d26d2fd9ca0ba11d8ad7 > > > Diff: https://reviews.apache.org/r/60500/diff/3/ > > > Testing > --- > > > Thanks, > > Qian Zhang > >
Review Request 61005: Added a test to check for copy assignment of `net::IP::Network`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61005/ --- Review request for mesos and Benjamin Hindman. Bugs: MESOS-7806 https://issues.apache.org/jira/browse/MESOS-7806 Repository: mesos Description --- Added a test to check for copy assignment of `net::IP::Network`. Diffs - 3rdparty/stout/tests/ip_tests.cpp dba0de6f0b6483f41e10ad38fe0d87946f186b9b Diff: https://reviews.apache.org/r/61005/diff/1/ Testing --- ~/dev/mesosphere/mesos/build$ ./3rdparty/stout/tests/stout-tests --gtest_filter="NetTest.CopyIPNetwork" Note: Google Test filter = NetTest.CopyIPNetwork- [==] Running 1 test from 1 test case. [--] Global test environment set-up. [--] 1 test from NetTest [ RUN ] NetTest.CopyIPNetwork [ OK ] NetTest.CopyIPNetwork (0 ms) [--] 1 test from NetTest (0 ms total) [--] Global test environment tear-down [==] 1 test from 1 test case ran. (0 ms total) [ PASSED ] 1 test. Thanks, Avinash sridharan
Review Request 61004: Added a copy assignment operator to `net::IP::Network`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61004/ --- Review request for mesos and Benjamin Hindman. Bugs: MESOS-7806 https://issues.apache.org/jira/browse/MESOS-7806 Repository: mesos Description --- Added a copy assignment operator to `net::IP::Network`. Diffs - 3rdparty/stout/include/stout/ip.hpp 51000b4f275466d6ed29105c15941c920af73516 Diff: https://reviews.apache.org/r/61004/diff/1/ Testing --- make check Thanks, Avinash sridharan
Re: Review Request 60500: Introduced `--default_container_dns` agent flag.
> On July 17, 2017, 11:12 p.m., Avinash sridharan wrote:
> > src/messages/flags.proto
> > Lines 56 (patched)
> > <https://reviews.apache.org/r/60500/diff/2/?file=1769366#file1769366line56>
> >
> > Had an internal discussion on this with Jie, and seems like having
> > different DNS options for CNI and CNM is not something we need to worry
> > about, so we should merge the fields into one.
>
> Qian Zhang wrote:
> OK, so the protobuf message should be like the below, right?
> ```
> message ContainerDNS {
> message DNSInfo {
> // Specify CNI network name or CNM network name as the value of
> // of this field. For CNM host network, its name is `host`, for
> // CNM default bridge network, its name is `bridge`, for a CNM
> // user-defined network, its name is specified by:
> // `ContainerInfo.network_infos(0).name`.
> required string network = 1;
>
> // For CNI network, all four fields in `slave.cni.spec.DNS` are
> // supported, but for CNM network, we only support three fields:
> // `nameservers`, `search` and `options` but not `domain` since
> // Docker only has `--dns`, `--dns-search`, `--dns-option` options.
> required slave.cni.spec.DNS dns = 2;
> }
>
> repeated DNSInfo dns = 1;
> }
> ```
That looks about right. Another thing that came out of the discussion is that
we should probably make the `network` field optional. This would then follow
the semantic of the `network/cni` isolator for missing `network` field implying
host networks. That said, currently, we should not be supporting setting DNS
for the host networking since the behavior is different for CNI and CNM. Also,
changing DNS for host networking doesn't make that much sense. To protect
against host networking we can add a validation check as a lambda in the
`--default_container_dns` flag?
For any unsupported DNS options (`domain` in CNM) we should throw a
`LOG(WARNING)` while configuring the container's DNS.
- Avinash
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60500/#review180764
---
On July 5, 2017, 7:03 a.m., Qian Zhang wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60500/
> ---
>
> (Updated July 5, 2017, 7:03 a.m.)
>
>
> Review request for mesos, Avinash sridharan and Jie Yu.
>
>
> Bugs: MESOS-7709
> https://issues.apache.org/jira/browse/MESOS-7709
>
>
> Repository: mesos
>
>
> Description
> ---
>
> Introduced `--default_container_dns` agent flag.
>
>
> Diffs
> -
>
> docs/configuration.md 007d0f556375f331148c9ad09be8b301ab31a01c
> src/messages/flags.hpp 70ad58c33067a0b058deafdcc4ffc5f554b6ca72
> src/messages/flags.proto e87075f19714739b93d9f4aa33ea8686a1bb5613
> src/slave/flags.hpp c6803eb54e09a5497755e1e5fef2872193eacba6
> src/slave/flags.cpp 398768656b5fa3b7c85474de2b4b008bf7b85cb3
>
>
> Diff: https://reviews.apache.org/r/60500/diff/2/
>
>
> Testing
> ---
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 60500: Introduced `--default_container_dns` agent flag.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60500/#review180764 --- src/messages/flags.proto Lines 56 (patched) <https://reviews.apache.org/r/60500/#comment255988> Had an internal discussion on this with Jie, and seems like having different DNS options for CNI and CNM is not something we need to worry about, so we should merge the fields into one. - Avinash sridharan On July 5, 2017, 7:03 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60500/ > --- > > (Updated July 5, 2017, 7:03 a.m.) > > > Review request for mesos, Avinash sridharan and Jie Yu. > > > Bugs: MESOS-7709 > https://issues.apache.org/jira/browse/MESOS-7709 > > > Repository: mesos > > > Description > --- > > Introduced `--default_container_dns` agent flag. > > > Diffs > - > > docs/configuration.md 007d0f556375f331148c9ad09be8b301ab31a01c > src/messages/flags.hpp 70ad58c33067a0b058deafdcc4ffc5f554b6ca72 > src/messages/flags.proto e87075f19714739b93d9f4aa33ea8686a1bb5613 > src/slave/flags.hpp c6803eb54e09a5497755e1e5fef2872193eacba6 > src/slave/flags.cpp 398768656b5fa3b7c85474de2b4b008bf7b85cb3 > > > Diff: https://reviews.apache.org/r/60500/diff/2/ > > > Testing > --- > > > Thanks, > > Qian Zhang > >
Re: Review Request 60500: Introduced `--default_container_dns` agent flag.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60500/#review180728 --- src/messages/flags.proto Lines 56 (patched) <https://reviews.apache.org/r/60500/#comment255947> I see Jie's point. From a framework's standpoint a name is a single domain of IP connectivity, so ideally if we have a network name that is both a CNI and CNM network (DC/OS overlay or Calico comes to mind here), it implies that it should be a single network and hence having a common DNS configuration across CNI/CNM is not incorrect. That said instead of using Mesos and Docker as the fields if we use CNI and CNM as the field names would that make sense? We would then be pegging against the standard rather than the `Containerizer`? The reason I am proposing to keep two different fields here is primarily to take into consideration any incosistencies that might pop up in these standards at some point (right now there don't seem to be any for DNS). - Avinash sridharan On July 5, 2017, 7:03 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60500/ > --- > > (Updated July 5, 2017, 7:03 a.m.) > > > Review request for mesos, Avinash sridharan and Jie Yu. > > > Bugs: MESOS-7709 > https://issues.apache.org/jira/browse/MESOS-7709 > > > Repository: mesos > > > Description > --- > > Introduced `--default_container_dns` agent flag. > > > Diffs > - > > docs/configuration.md 007d0f556375f331148c9ad09be8b301ab31a01c > src/messages/flags.hpp 70ad58c33067a0b058deafdcc4ffc5f554b6ca72 > src/messages/flags.proto e87075f19714739b93d9f4aa33ea8686a1bb5613 > src/slave/flags.hpp c6803eb54e09a5497755e1e5fef2872193eacba6 > src/slave/flags.cpp 398768656b5fa3b7c85474de2b4b008bf7b85cb3 > > > Diff: https://reviews.apache.org/r/60500/diff/2/ > > > Testing > --- > > > Thanks, > > Qian Zhang > >
Re: Review Request 60720: Fixed initialization of `__address__` in the abscense of `--ip` flag.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60720/#review180249 --- Will addressing the changes suggested by Yan in a separate patch, so closing this out. - Avinash sridharan On July 11, 2017, 9:56 p.m., Avinash sridharan wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60720/ > --- > > (Updated July 11, 2017, 9:56 p.m.) > > > Review request for mesos, Benjamin Hindman and Jiang Yan Xu. > > > Bugs: MESOS-7769 > https://issues.apache.org/jira/browse/MESOS-7769 > > > Repository: mesos > > > Description > --- > > When we introduced the `__address6__ optional IPv6 storage into > `libprocess` we also introduced a regression because of which the `port` > doesn't get initialized to whatever is specified in the `--port` flag > until the `--ip` flag is specified. > > This change fixes the initialization of the `__address__` in the > absence of the `--ip` flag. > > > Diffs > - > > 3rdparty/libprocess/src/process.cpp > 264298c16199ded4ccbf948f6893f4209c23aed0 > > > Diff: https://reviews.apache.org/r/60720/diff/1/ > > > Testing > --- > > sudo ./build/bin/mesos-agent.sh --port=5059 --master=10.0.2.15:5050 > --work_dir=/tmp/mesos > tcp0 0 0.0.0.0:50590.0.0.0:* LISTEN > > sudo ./build/bin/mesos-agent.sh --ip=10.0.2.15 --port=5059 > --master=10.0.2.15:5050 --work_dir=/tmp/mesos > tcp0 0 10.0.2.15:5059 0.0.0.0:* LISTEN > > > Thanks, > > Avinash sridharan > >
Re: Review Request 60720: Fixed initialization of `__address__` in the abscense of `--ip` flag.
> On July 7, 2017, 11:18 p.m., Jiang Yan Xu wrote: > > 3rdparty/libprocess/src/process.cpp > > Line 1135 (original), 1135 (patched) > > <https://reviews.apache.org/r/60720/diff/1/?file=1771682#file1771682line1135> > > > > You are (re)initalizing/resetting this `__address__` but not > > `__address6__` (to None). Should they be consistent? > > > > Given it's initialized at definition and reset during `finailize` > > perhaps it doesn't need to be done here? > > > > If it needs to be done, can it be moved down closer to where the > > modifications/reassignments are done? > > Avinash sridharan wrote: > `__address6__` is optional hence we don't initialize it (in fact its > initalized to `None()`) > > Jiang Yan Xu wrote: > Yeah I understand so that's why I used "(re)initalizing/resetting" in the > comment. My comment is about the inconsistency: > > `__address__` is (re)set to ANY_ANY: > > - At definition. > - In finalize(). > - In initialize(). > > `__address6__` is (re)set to None: > > - At definition. > - In finalize(). > > Seems like there's no reason they need to be different despite > `__address6__` is `Option`al. Ah !! I c your point. This makes sense, for consistency we should initialize it to `None()`. > On July 7, 2017, 11:18 p.m., Jiang Yan Xu wrote: > > 3rdparty/libprocess/src/process.cpp > > Line 1191 (original), 1192 (patched) > > <https://reviews.apache.org/r/60720/diff/1/?file=1771682#file1771682line1192> > > > > Should `advertise_ip` be treated the same way as `ip`? i.e., flag > > validation and choice between reassignment and modification. > > Avinash sridharan wrote: > Once we move to the pattern of doing `__address__.ip = > libprocess_flags->ip.get()` I think handling of `__address__` for `--ip` and > `--advertise_ip` will be the same. > > Jiang Yan Xu wrote: > What about flag vaidation? Yeah you are right we should validate `advertise_ip` the same way we validate `ip`. Will add this as well. - Avinash ------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60720/#review179958 --- On July 8, 2017, 12:08 a.m., Avinash sridharan wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60720/ > --- > > (Updated July 8, 2017, 12:08 a.m.) > > > Review request for mesos and Jiang Yan Xu. > > > Bugs: MESOS-7769 > https://issues.apache.org/jira/browse/MESOS-7769 > > > Repository: mesos > > > Description > --- > > When we introduced the `__address6__ optional IPv6 storage into > `libprocess` we also introduced a regression because of which the `port` > doesn't get initialized to whatever is specified in the `--port` flag > until the `--ip` flag is specified. > > This change fixes the initialization of the `__address__` in the > absence of the `--ip` flag. > > > Diffs > - > > 3rdparty/libprocess/src/process.cpp > 264298c16199ded4ccbf948f6893f4209c23aed0 > > > Diff: https://reviews.apache.org/r/60720/diff/1/ > > > Testing > --- > > sudo ./build/bin/mesos-agent.sh --port=5059 --master=10.0.2.15:5050 > --work_dir=/tmp/mesos > tcp0 0 0.0.0.0:50590.0.0.0:* LISTEN > > sudo ./build/bin/mesos-agent.sh --ip=10.0.2.15 --port=5059 > --master=10.0.2.15:5050 --work_dir=/tmp/mesos > tcp0 0 10.0.2.15:5059 0.0.0.0:* LISTEN > > > Thanks, > > Avinash sridharan > >
Re: Review Request 60724: Fixed initialization of `LIBPROCESS_IP6`.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60724/
---
(Updated July 9, 2017, 8:23 p.m.)
Review request for mesos, Benjamin Bannier and Benjamin Hindman.
Changes
---
Addressed Benjamin's comments.
Bugs: MESOS-7772
https://issues.apache.org/jira/browse/MESOS-7772
Repository: mesos
Description
---
Fixed initialization of `LIBPROCESS_IP6`.
Diffs
-
src/slave/main.cpp 358a4394d27d2d123c9cdc9ed3e5295ecbaf9130
Diff: https://reviews.apache.org/r/60724/diff/1/
Testing (updated)
---
make check
MANUAL TESTING
~/dev/mesosphere/mesos$ sudo ./build/bin/mesos-agent.sh --ip=10.0.2.15
--ip6=1::1 --master=10.0.2.15:5050 --work_dir=/tmp/mesos
--containerizers=mesos,docker
~/dev/mesosphere/mesos$ sudo ./build/src/mesos-execute --master=10.0.2.15:5050
--task=file:///home/vagrant/dev/mesos_apps/docker_host.json
WARNING: Logging before InitGoogleLogging() is written to STDERR
W0709 20:01:42.282433 4953 scheduler.cpp:169]
**
Scheduler driver bound to loopback interface! Cannot communicate with remote
master(s). You might want to set 'LIBPROCESS_IP' environment variable to use a
routable IP address.
**
I0709 20:01:42.282627 4953 scheduler.cpp:184] Version: 1.4.0
I0709 20:01:42.300417 4972 scheduler.cpp:470] New master detected at
master@10.0.2.15:5050
Subscribed with ID 70b8df2f-4b14-4597-a3c2-01f16ccd5b6f-0002
Submitted task 'test' to agent '70b8df2f-4b14-4597-a3c2-01f16ccd5b6f-S0'
Received status update TASK_RUNNING for task 'test_docker'
source: SOURCE_EXECUTOR
vagrant@centos7:~$ curl http://10.0.2.15:5051/state | jq
"frameworks": [
{
"id": "70b8df2f-4b14-4597-a3c2-01f16ccd5b6f-0001",
"name": "mesos-execute instance",
"user": "root",
"failover_timeout": 0,
"checkpoint": false,
"hostname": "",
"role": "*",
"executors": [
{
"id": "test_docker",
"name": "Command Executor (Task: test_docker) (Command: NO
EXECUTABLE)",
"source": "test_docker",
"container": "8a51cdfb-2039-407a-942e-52bb66bd1a4c",
"directory":
"/tmp/mesos/slaves/70b8df2f-4b14-4597-a3c2-01f16ccd5b6f-S0/frameworks/70b8df2f-4b14-4597-a3c2-01f16ccd5b6f-0001/executors/test_docker/runs/8a51cdfb-2039-407a-942e-52bb66bd1a4c",
"resources": {
"disk": 0,
"mem": 64,
"gpus": 0,
"cpus": 0.2
},
"role": "*",
"tasks": [
{
"id": "test_docker",
"name": "test",
"framework_id": "70b8df2f-4b14-4597-a3c2-01f16ccd5b6f-0001",
"executor_id": "",
"slave_id": "70b8df2f-4b14-4597-a3c2-01f16ccd5b6f-S0",
"state": "TASK_RUNNING",
"resources": {
"disk": 0,
"mem": 32,
"gpus": 0,
"cpus": 0.1
},
"role": "*",
"statuses": [
{
"state": "TASK_RUNNING",
"timestamp": 1499630245.73584,
"container_status": {
"container_id": {
"value": "8a51cdfb-2039-407a-942e-52bb66bd1a4c"
},
"network_infos": [
{
"ip_addresses": [
{
"protocol": "IPv4",
"ip_address": "10.0.2.15"
},
{
"protocol": "IPv6",
"ip_address": "1::1"
Re: Review Request 60724: Fixed initialization of `LIBPROCESS_IP6`.
> On July 8, 2017, 10:23 a.m., Benjamin Bannier wrote: > > Since this wasn't caught by `make check` previously, do we have manual > > steps we could perform to verify that the `--ip6` flag of libprocess works? Valid point. Have added the output from the manual testing when the `--ip6` flag is set on the agent. The `task` output in agent and master state clearly shows that the IPv6 set by the operator is set for the container running on the host network. - Avinash --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60724/#review179980 --- On July 8, 2017, 4:45 a.m., Avinash sridharan wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60724/ > --- > > (Updated July 8, 2017, 4:45 a.m.) > > > Review request for mesos, Benjamin Bannier and Benjamin Hindman. > > > Bugs: MESOS-7772 > https://issues.apache.org/jira/browse/MESOS-7772 > > > Repository: mesos > > > Description > --- > > Fixed initialization of `LIBPROCESS_IP6`. > > > Diffs > - > > src/slave/main.cpp 358a4394d27d2d123c9cdc9ed3e5295ecbaf9130 > > > Diff: https://reviews.apache.org/r/60724/diff/1/ > > > Testing > --- > > make check > > > Thanks, > > Avinash sridharan > >
Re: Review Request 60724: Fixed initialization of `LIBPROCESS_IP6` on agent.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60724/
---
(Updated July 9, 2017, 8:23 p.m.)
Review request for mesos, Benjamin Bannier and Benjamin Hindman.
Changes
---
Fixed the summary.
Summary (updated)
-
Fixed initialization of `LIBPROCESS_IP6` on agent.
Bugs: MESOS-7772
https://issues.apache.org/jira/browse/MESOS-7772
Repository: mesos
Description
---
Fixed initialization of `LIBPROCESS_IP6`.
Diffs
-
src/slave/main.cpp 358a4394d27d2d123c9cdc9ed3e5295ecbaf9130
Diff: https://reviews.apache.org/r/60724/diff/1/
Testing
---
make check
MANUAL TESTING
~/dev/mesosphere/mesos$ sudo ./build/bin/mesos-agent.sh --ip=10.0.2.15
--ip6=1::1 --master=10.0.2.15:5050 --work_dir=/tmp/mesos
--containerizers=mesos,docker
~/dev/mesosphere/mesos$ sudo ./build/src/mesos-execute --master=10.0.2.15:5050
--task=file:///home/vagrant/dev/mesos_apps/docker_host.json
WARNING: Logging before InitGoogleLogging() is written to STDERR
W0709 20:01:42.282433 4953 scheduler.cpp:169]
**
Scheduler driver bound to loopback interface! Cannot communicate with remote
master(s). You might want to set 'LIBPROCESS_IP' environment variable to use a
routable IP address.
**
I0709 20:01:42.282627 4953 scheduler.cpp:184] Version: 1.4.0
I0709 20:01:42.300417 4972 scheduler.cpp:470] New master detected at
master@10.0.2.15:5050
Subscribed with ID 70b8df2f-4b14-4597-a3c2-01f16ccd5b6f-0002
Submitted task 'test' to agent '70b8df2f-4b14-4597-a3c2-01f16ccd5b6f-S0'
Received status update TASK_RUNNING for task 'test_docker'
source: SOURCE_EXECUTOR
vagrant@centos7:~$ curl http://10.0.2.15:5051/state | jq
"frameworks": [
{
"id": "70b8df2f-4b14-4597-a3c2-01f16ccd5b6f-0001",
"name": "mesos-execute instance",
"user": "root",
"failover_timeout": 0,
"checkpoint": false,
"hostname": "",
"role": "*",
"executors": [
{
"id": "test_docker",
"name": "Command Executor (Task: test_docker) (Command: NO
EXECUTABLE)",
"source": "test_docker",
"container": "8a51cdfb-2039-407a-942e-52bb66bd1a4c",
"directory":
"/tmp/mesos/slaves/70b8df2f-4b14-4597-a3c2-01f16ccd5b6f-S0/frameworks/70b8df2f-4b14-4597-a3c2-01f16ccd5b6f-0001/executors/test_docker/runs/8a51cdfb-2039-407a-942e-52bb66bd1a4c",
"resources": {
"disk": 0,
"mem": 64,
"gpus": 0,
"cpus": 0.2
},
"role": "*",
"tasks": [
{
"id": "test_docker",
"name": "test",
"framework_id": "70b8df2f-4b14-4597-a3c2-01f16ccd5b6f-0001",
"executor_id": "",
"slave_id": "70b8df2f-4b14-4597-a3c2-01f16ccd5b6f-S0",
"state": "TASK_RUNNING",
"resources": {
"disk": 0,
"mem": 32,
"gpus": 0,
"cpus": 0.1
},
"role": "*",
"statuses": [
{
"state": "TASK_RUNNING",
"timestamp": 1499630245.73584,
"container_status": {
"container_id": {
"value": "8a51cdfb-2039-407a-942e-52bb66bd1a4c"
},
"network_infos": [
{
"ip_addresses": [
{
"protocol": "IPv4",
"ip_address": "10.0.2.15"
},
{
"protocol": "IPv6",
"ip_address": "1::1"
Re: Review Request 60600: Set container DNS with `--default_container_dns` in CNI isolator.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60600/#review179534
---
Ship it!
Ship It!
src/slave/containerizer/mesos/isolators/network/cni/cni.cpp
Lines 1003 (patched)
<https://reviews.apache.org/r/60600/#comment254295>
s/even the/even though the
- Avinash sridharan
On July 3, 2017, 2:45 p.m., Qian Zhang wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60600/
> ---
>
> (Updated July 3, 2017, 2:45 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 CNI isolator.
>
>
> Diffs
> -
>
> src/slave/containerizer/mesos/isolators/network/cni/cni.cpp
> b386fbed17698dd6c59bc6c53edfe8a7ece6a3eb
>
>
> Diff: https://reviews.apache.org/r/60600/diff/1/
>
>
> 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 /opt/cni/net_configs/net1
> {
> "name": "net1",
> "type": "bridge",
> "bridge": "br1",
> "isGateway": true,
> "ipMasq": true,
> "ipam": {
> "type": "host-local",
> "subnet": "192.168.1.0/24",
> "routes": [
> { "dst": "0.0.0.0/0" }
> ]
> }
> }
>
> $ cat /opt/cni/net_configs/net2
> {
> "name": "net2",
> "type": "bridge",
> "bridge": "br2",
> "isGateway": true,
> "ipMasq": true,
> "ipam": {
> "type": "host-local",
> "subnet": "192.168.2.0/24",
> "routes": [
> { "dst": "0.0.0.0/0" }
> ]
> },
> "dns": {
> "nameservers": [ "8.8.4.4" ],
> "domain": "net2.com",
> "search": [ "yyy.com" ],
> "options": [ "attempts:3" ]
> }
> }
>
> $ cat /home/stack/dns.json
> {
> "mesos": [
> {
> "network": "net1",
> "dns": {
> "nameservers": [ "8.8.8.8" ],
> "search": [ "xxx.com" ],
> "options": [ "timeout:4" ]
> }
> }
> ]
> }
> ```
>
> 3. Launch a unified container with `mesos-execute`.
> ```
> $ sudo src/mesos-execute --master=192.168.122.216:5050
> --task=file:///home/stack/task.json
>
> $ cat /home/stack/task.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": {
> "value": "sleep 300"
> },
> "container": {
> "type": "MESOS",
> "mesos": {
> "image": {
> "type": "DOCKER",
> "docker": {
> "name": "busybox"
> }
> }
> },
> "network_infos": [
> {
> "name": "net1"
> },
> {
> "name": "net2"
> }
> ]
> }
> }
> ```
>
> 4. Check the DNS configuration of the unified container.
> ```
> $ ps -ef | grep sleep
> root 20060 20037 2 21:45 ?00:00:00 sh -c sleep 300
> root 20074 20060 0 21:45 ?00:00:00 sleep 300
>
> $ sudo nsenter -t 20060 -m -u -n cat /etc/resolv.conf
> domain net2.com
> search yyy.com xxx.com
> options attempts:3 timeout:4
> nameserver 8.8.4.4
> nameserver 8.8.8.8
> ```
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 60558: Set container DNS with `--default_container_dns` in Docker executor.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60558/#review179994
---
Ship it!
Hi Qian, the changes LGTM but I think we should follow these patches up with a
test for CNI and CNM networks?
- Avinash sridharan
On July 5, 2017, 7:18 a.m., Qian Zhang wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60558/
> ---
>
> (Updated July 5, 2017, 7:18 a.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 8ca0c68836ea5d1a1186e79942f6daccf6c6da46
> src/docker/executor.cpp e03f24461ec7b19cacae79c020406f0a475f2d19
>
>
> Diff: https://reviews.apache.org/r/60558/diff/3/
>
>
> 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": "net1",
> "dns": {
> "nameservers": [ "8.8.8.8", "8.8.4.4" ]
> }
> }
> ],
> "docker": [
> {
> "network": "bridge",
> "dns": {
> "nameservers": [ "8.8.8.8", "8.8.4.4" ],
> "search": [ "xxx.com", "yyy.com" ],
> "options": [ "timeout:3", "attempts:2" ]
> }
> }
> ]
> }
> ```
>
> 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 IDIMAGE COMMAND CREATED
> STATUS PORTS NAMES
> ca642bf31a9fnginx "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
>
>
Re: Review Request 60558: Set container DNS with `--default_container_dns` in Docker executor.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60558/#review179993
---
Ship it!
Ship It!
- Avinash sridharan
On July 5, 2017, 7:18 a.m., Qian Zhang wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60558/
> ---
>
> (Updated July 5, 2017, 7:18 a.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 8ca0c68836ea5d1a1186e79942f6daccf6c6da46
> src/docker/executor.cpp e03f24461ec7b19cacae79c020406f0a475f2d19
>
>
> Diff: https://reviews.apache.org/r/60558/diff/3/
>
>
> 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": "net1",
> "dns": {
> "nameservers": [ "8.8.8.8", "8.8.4.4" ]
> }
> }
> ],
> "docker": [
> {
> "network": "bridge",
> "dns": {
> "nameservers": [ "8.8.8.8", "8.8.4.4" ],
> "search": [ "xxx.com", "yyy.com" ],
> "options": [ "timeout:3", "attempts:2" ]
> }
> }
> ]
> }
> ```
>
> 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 IDIMAGE COMMAND CREATED
> STATUS PORTS NAMES
> ca642bf31a9fnginx "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
>
>
Re: Review Request 60500: Introduced `--default_container_dns` agent flag.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60500/#review179992 --- Ship it! Ship It! - Avinash sridharan On July 5, 2017, 7:03 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60500/ > --- > > (Updated July 5, 2017, 7:03 a.m.) > > > Review request for mesos, Avinash sridharan and Jie Yu. > > > Bugs: MESOS-7709 > https://issues.apache.org/jira/browse/MESOS-7709 > > > Repository: mesos > > > Description > --- > > Introduced `--default_container_dns` agent flag. > > > Diffs > - > > docs/configuration.md 007d0f556375f331148c9ad09be8b301ab31a01c > src/messages/flags.hpp 70ad58c33067a0b058deafdcc4ffc5f554b6ca72 > src/messages/flags.proto e87075f19714739b93d9f4aa33ea8686a1bb5613 > src/slave/flags.hpp c6803eb54e09a5497755e1e5fef2872193eacba6 > src/slave/flags.cpp 398768656b5fa3b7c85474de2b4b008bf7b85cb3 > > > Diff: https://reviews.apache.org/r/60500/diff/2/ > > > Testing > --- > > > Thanks, > > Qian Zhang > >
Re: Review Request 60500: Introduced `--default_container_dns` agent flag.
> On July 3, 2017, 5:22 p.m., Avinash sridharan wrote: > > docs/configuration.md > > Lines 1312 (patched) > > <https://reviews.apache.org/r/60500/diff/1/?file=1766230#file1766230line1312> > > > > `ContainerInfo.docker.parameter` > > Qian Zhang wrote: > The field name is `parameters` rather than `parameter`. Qian I meant put the field in quotest `ContainerInfo.docker.parameters`. - Avinash --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60500/#review179520 --- On July 5, 2017, 7:03 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60500/ > --- > > (Updated July 5, 2017, 7:03 a.m.) > > > Review request for mesos, Avinash sridharan and Jie Yu. > > > Bugs: MESOS-7709 > https://issues.apache.org/jira/browse/MESOS-7709 > > > Repository: mesos > > > Description > --- > > Introduced `--default_container_dns` agent flag. > > > Diffs > - > > docs/configuration.md 007d0f556375f331148c9ad09be8b301ab31a01c > src/messages/flags.hpp 70ad58c33067a0b058deafdcc4ffc5f554b6ca72 > src/messages/flags.proto e87075f19714739b93d9f4aa33ea8686a1bb5613 > src/slave/flags.hpp c6803eb54e09a5497755e1e5fef2872193eacba6 > src/slave/flags.cpp 398768656b5fa3b7c85474de2b4b008bf7b85cb3 > > > Diff: https://reviews.apache.org/r/60500/diff/2/ > > > Testing > --- > > > Thanks, > > Qian Zhang > >
Review Request 60724: Fixed initialization of `LIBPROCESS_IP6`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60724/ --- Review request for mesos, Benjamin Bannier and Benjamin Hindman. Bugs: MESOS-7772 https://issues.apache.org/jira/browse/MESOS-7772 Repository: mesos Description --- Fixed initialization of `LIBPROCESS_IP6`. Diffs - src/slave/main.cpp 358a4394d27d2d123c9cdc9ed3e5295ecbaf9130 Diff: https://reviews.apache.org/r/60724/diff/1/ Testing --- make check Thanks, Avinash sridharan
Review Request 60723: Made intialization for `__address__` consistent.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60723/ --- Review request for mesos and Jiang Yan Xu. Bugs: MESOS-7769 https://issues.apache.org/jira/browse/MESOS-7769 Repository: mesos Description --- Made intialization for `__address__` consistent. Diffs - 3rdparty/libprocess/src/process.cpp 97f737b0b7be23df32f2863d4d6891ec2518529e Diff: https://reviews.apache.org/r/60723/diff/1/ Testing --- sudo ./build/bin/mesos-agent.sh --ip=10.0.2.15 --port=5059 --master=10.0.2.15:5050 --work_dir=/tmp/mesos tcp0 0 10.0.2.15:5059 0.0.0.0:* LISTEN sudo ./build/bin/mesos-agent.sh --port=5059 --master=10.0.2.15:5050 --work_dir=/tmp/mesos tcp0 0 0.0.0.0:50590.0.0.0:* LISTEN sudo ./build/bin/mesos-agent.sh --master=10.0.2.15:5050 --work_dir=/tmp/mesos tcp0 0 0.0.0.0:50510.0.0.0:* LISTEN sudo ./build/bin/mesos-agent.sh --ip=10.0.2.15 --master=10.0.2.15:5050 --work_dir=/tmp/meso tcp0 0 10.0.2.15:5051 0.0.0.0:* LISTEN Thanks, Avinash sridharan
Re: Review Request 60720: Fixed initialization of `__address__` in the abscense of `--ip` flag.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60720/ --- (Updated July 8, 2017, 12:08 a.m.) Review request for mesos and Jiang Yan Xu. Changes --- Added a description about the tests as well. Bugs: MESOS-7769 https://issues.apache.org/jira/browse/MESOS-7769 Repository: mesos Description --- When we introduced the `__address6__ optional IPv6 storage into `libprocess` we also introduced a regression because of which the `port` doesn't get initialized to whatever is specified in the `--port` flag until the `--ip` flag is specified. This change fixes the initialization of the `__address__` in the absence of the `--ip` flag. Diffs - 3rdparty/libprocess/src/process.cpp 264298c16199ded4ccbf948f6893f4209c23aed0 Diff: https://reviews.apache.org/r/60720/diff/1/ Testing (updated) --- sudo ./build/bin/mesos-agent.sh --port=5059 --master=10.0.2.15:5050 --work_dir=/tmp/mesos tcp0 0 0.0.0.0:50590.0.0.0:* LISTEN sudo ./build/bin/mesos-agent.sh --ip=10.0.2.15 --port=5059 --master=10.0.2.15:5050 --work_dir=/tmp/mesos tcp0 0 10.0.2.15:5059 0.0.0.0:* LISTEN Thanks, Avinash sridharan
Re: Review Request 60720: Fixed initialization of `__address__` in the abscense of `--ip` flag.
> On July 7, 2017, 11:18 p.m., Jiang Yan Xu wrote: > > 3rdparty/libprocess/src/process.cpp > > Line 1191 (original), 1192 (patched) > > <https://reviews.apache.org/r/60720/diff/1/?file=1771682#file1771682line1192> > > > > Should `advertise_ip` be treated the same way as `ip`? i.e., flag > > validation and choice between reassignment and modification. Once we move to the pattern of doing `__address__.ip = libprocess_flags->ip.get()` I think handling of `__address__` for `--ip` and `--advertise_ip` will be the same. - Avinash --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60720/#review179958 ------- On July 7, 2017, 10:04 p.m., Avinash sridharan wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60720/ > --- > > (Updated July 7, 2017, 10:04 p.m.) > > > Review request for mesos and Jiang Yan Xu. > > > Bugs: MESOS-7769 > https://issues.apache.org/jira/browse/MESOS-7769 > > > Repository: mesos > > > Description > --- > > When we introduced the `__address6__ optional IPv6 storage into > `libprocess` we also introduced a regression because of which the `port` > doesn't get initialized to whatever is specified in the `--port` flag > until the `--ip` flag is specified. > > This change fixes the initialization of the `__address__` in the > absence of the `--ip` flag. > > > Diffs > - > > 3rdparty/libprocess/src/process.cpp > 264298c16199ded4ccbf948f6893f4209c23aed0 > > > Diff: https://reviews.apache.org/r/60720/diff/1/ > > > Testing > --- > > > Thanks, > > Avinash sridharan > >
Re: Review Request 60720: Fixed initialization of `__address__` in the abscense of `--ip` flag.
> On July 7, 2017, 11:18 p.m., Jiang Yan Xu wrote: > > Could you fill out the 'testing done' section? I know there will be a > > follow up test patch but for this at least `make check` and manual > > verification? > > Jiang Yan Xu wrote: > I committed it but could you respond to the comments afterwards and > follow up on them in another review (if needed)? Thanks Yan !! Will follow it up with a patch. > On July 7, 2017, 11:18 p.m., Jiang Yan Xu wrote: > > 3rdparty/libprocess/src/process.cpp > > Line 1135 (original), 1135 (patched) > > <https://reviews.apache.org/r/60720/diff/1/?file=1771682#file1771682line1135> > > > > You are (re)initalizing/resetting this `__address__` but not > > `__address6__` (to None). Should they be consistent? > > > > Given it's initialized at definition and reset during `finailize` > > perhaps it doesn't need to be done here? > > > > If it needs to be done, can it be moved down closer to where the > > modifications/reassignments are done? `__address6__` is optional hence we don't initialize it (in fact its initalized to `None()`) > On July 7, 2017, 11:18 p.m., Jiang Yan Xu wrote: > > 3rdparty/libprocess/src/process.cpp > > Lines 1151-1157 (original), 1151-1158 (patched) > > <https://reviews.apache.org/r/60720/diff/1/?file=1771682#file1771682line1151> > > > > Why do you need to modify the current instance of `__address__` in the > > case above but but have to create a new instance of `inet::Address` in the > > case below? > > > > Can they be made consistent? yeah I can just set `__address__.ip = libprocess_flags->ip.get()` - Avinash --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60720/#review179958 --- On July 7, 2017, 10:04 p.m., Avinash sridharan wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60720/ > --- > > (Updated July 7, 2017, 10:04 p.m.) > > > Review request for mesos and Jiang Yan Xu. > > > Bugs: MESOS-7769 > https://issues.apache.org/jira/browse/MESOS-7769 > > > Repository: mesos > > > Description > --- > > When we introduced the `__address6__ optional IPv6 storage into > `libprocess` we also introduced a regression because of which the `port` > doesn't get initialized to whatever is specified in the `--port` flag > until the `--ip` flag is specified. > > This change fixes the initialization of the `__address__` in the > absence of the `--ip` flag. > > > Diffs > - > > 3rdparty/libprocess/src/process.cpp > 264298c16199ded4ccbf948f6893f4209c23aed0 > > > Diff: https://reviews.apache.org/r/60720/diff/1/ > > > Testing > --- > > > Thanks, > > Avinash sridharan > >
Review Request 60720: Fixed initialization of `__address__` in the abscense of `--ip` flag.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60720/ --- Review request for mesos and Jiang Yan Xu. Bugs: MESOS-7769 https://issues.apache.org/jira/browse/MESOS-7769 Repository: mesos Description --- When we introduced the `__address6__ optional IPv6 storage into `libprocess` we also introduced a regression because of which the `port` doesn't get initialized to whatever is specified in the `--port` flag until the `--ip` flag is specified. This change fixes the initialization of the `__address__` in the absence of the `--ip` flag. Diffs - 3rdparty/libprocess/src/process.cpp 264298c16199ded4ccbf948f6893f4209c23aed0 Diff: https://reviews.apache.org/r/60720/diff/1/ Testing --- Thanks, Avinash sridharan
Review Request 60713: Modified the access specifier for `net::IP` and `net::IP::Network`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60713/ --- Review request for mesos and Jie Yu. Bugs: MESOS-7767 https://issues.apache.org/jira/browse/MESOS-7767 Repository: mesos Description --- Modified the access specifier for `net::IP` and `net::IP::Network`. Diffs - 3rdparty/stout/include/stout/ip.hpp 6c2f323a4ccb140c54f00236fb50c21180a84ebb Diff: https://reviews.apache.org/r/60713/diff/1/ Testing --- make check Thanks, Avinash sridharan
Re: Review Request 59127: Added IPv6 flags for Mesos agent.
> On July 5, 2017, 7:57 p.m., Benjamin Hindman wrote: > > src/slave/flags.hpp > > Line 173 (original), 173 (patched) > > <https://reviews.apache.org/r/59127/diff/4/?file=1767256#file1767256line173> > > > > Let's keep this generic to IP not IPv4 and add validation like you did > > in libprocess that we currently only support IPv4. I haven't added a validation check here since the `ip` here is just a string. This is anyway passed onto `libprocess` where its converted to `net::IP` and the validation kicks in. > On July 5, 2017, 7:57 p.m., Benjamin Hindman wrote: > > src/slave/flags.hpp > > Lines 187-190 (patched) > > <https://reviews.apache.org/r/59127/diff/4/?file=1767256#file1767256line187> > > > > How about printing this out as a warning as part of some validation > > logic so that users that set this understand that they're not getting what > > they might think they're getting? Added a `LOG(WARNING) in the flag validation. - Avinash --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59127/#review179692 --- On July 7, 2017, 1:02 a.m., Avinash sridharan wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59127/ > --- > > (Updated July 7, 2017, 1:02 a.m.) > > > Review request for mesos, Benjamin Hindman and Jie Yu. > > > Bugs: MESOS-7488 > https://issues.apache.org/jira/browse/MESOS-7488 > > > Repository: mesos > > > Description > --- > > Added IPv6 flags for Mesos agent. > > > Diffs > - > > src/slave/flags.hpp c6803eb54e09a5497755e1e5fef2872193eacba6 > src/slave/flags.cpp 398768656b5fa3b7c85474de2b4b008bf7b85cb3 > > > Diff: https://reviews.apache.org/r/59127/diff/5/ > > > Testing > --- > > make check > > > Thanks, > > Avinash sridharan > >
Re: Review Request 59127: Added IPv6 flags for Mesos agent.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59127/ --- (Updated July 7, 2017, 1:02 a.m.) Review request for mesos, Benjamin Hindman and Jie Yu. Changes --- Addressed BenH's comments. Bugs: MESOS-7488 https://issues.apache.org/jira/browse/MESOS-7488 Repository: mesos Description --- Added IPv6 flags for Mesos agent. Diffs (updated) - src/slave/flags.hpp c6803eb54e09a5497755e1e5fef2872193eacba6 src/slave/flags.cpp 398768656b5fa3b7c85474de2b4b008bf7b85cb3 Diff: https://reviews.apache.org/r/59127/diff/5/ Changes: https://reviews.apache.org/r/59127/diff/4-5/ Testing --- make check Thanks, Avinash sridharan
Re: Review Request 60692: Fixed `net::IPv4` to return `INADDR_ANY` for `ANY()`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60692/ --- (Updated July 6, 2017, 11:46 p.m.) Review request for mesos and Benjamin Hindman. Bugs: MESOS-7765 https://issues.apache.org/jira/browse/MESOS-7765 Repository: mesos Description --- Fixed `net::IPv4` to return `INADDR_ANY` for `ANY()`. Diffs (updated) - 3rdparty/stout/include/stout/ip.hpp 25779fa857d138d78c52d1528c070763eba486a1 Diff: https://reviews.apache.org/r/60692/diff/2/ Changes: https://reviews.apache.org/r/60692/diff/1-2/ Testing --- make check. Also sudo ./build/bin/mesos-tests.sh --gtest_filter=MasterTest.KillUnknownTask ``` [==] Running 1 test from 1 test case. [--] Global test environment set-up. [--] 1 test from MasterTest [ RUN ] MasterTest.KillUnknownTask [ OK ] MasterTest.KillUnknownTask (87 ms) [--] 1 test from MasterTest (88 ms total) [--] Global test environment tear-down [==] 1 test from 1 test case ran. (99 ms total) [ PASSED ] 1 test. ``` Thanks, Avinash sridharan
Re: Review Request 60551: Added a test for docker container running on a v4/6 host network.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60551/ --- (Updated July 6, 2017, 11:45 p.m.) Review request for mesos, Benjamin Hindman and Jie Yu. Changes --- Addressed BenH's comments. Bugs: MESOS-7488 https://issues.apache.org/jira/browse/MESOS-7488 Repository: mesos Description --- Added a test for docker container running on a v4/6 host network. Diffs (updated) - src/tests/containerizer/docker_containerizer_tests.cpp cf9470245ebc81e799fb2b2a67464298564fc56f Diff: https://reviews.apache.org/r/60551/diff/3/ Changes: https://reviews.apache.org/r/60551/diff/2-3/ Testing --- sudo ./bin/mesos-tests.sh Thanks, Avinash sridharan
Re: Review Request 59233: Updated v6 address for containers running on host network.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59233/ --- (Updated July 6, 2017, 11:36 p.m.) Review request for mesos and Jie Yu. Changes --- Addressed BenH's comments. Bugs: MESOS-7488 https://issues.apache.org/jira/browse/MESOS-7488 Repository: mesos Description --- Currently the agent is populating only the v4 address for containers running on the host network. However, clusters running on a dual stack network need to report v4 and v6 address for containers running on the host network. This change allows v6 address specified by operators to be advertised for containers running on the host network. Diffs (updated) - include/mesos/mesos.proto ab2a372184b7cfbaf7a38e90f487cba38c3e80b8 include/mesos/v1/mesos.proto 5e92e5d86023ad6edd94303fbde964bf403abf02 src/slave/slave.cpp 0e24b8cb8d1020af515e3d1862e121e1daf82ce9 Diff: https://reviews.apache.org/r/59233/diff/7/ Changes: https://reviews.apache.org/r/59233/diff/6-7/ Testing --- make check Thanks, Avinash sridharan
Re: Review Request 59130: Added storage for IPv6 in a `libprocess` process.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59130/ --- (Updated July 6, 2017, 11:34 p.m.) Review request for mesos, Benjamin Hindman and Jie Yu. Changes --- Addressed BenH's comments. Bugs: MESOS-7488 https://issues.apache.org/jira/browse/MESOS-7488 Repository: mesos Description --- Added storage for IPv6 in a `libprocess` process. Diffs (updated) - 3rdparty/libprocess/src/process.cpp b3d55390293a11a0ea80f0545a03f8aae04abb81 Diff: https://reviews.apache.org/r/59130/diff/7/ Changes: https://reviews.apache.org/r/59130/diff/6-7/ Testing --- make check. Thanks, Avinash sridharan
Re: Review Request 59131: Added an IPv6 address storage to UPID.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59131/ --- (Updated July 6, 2017, 11:31 p.m.) Review request for mesos and Jie Yu. Changes --- Addressed BenH's comments. Bugs: MESOS-7488 https://issues.apache.org/jira/browse/MESOS-7488 Repository: mesos Description --- Added an IPv6 address storage to UPID. Diffs (updated) - 3rdparty/libprocess/include/process/pid.hpp b0c47ff562c5dfa019f6b392ff269f88a72d40d2 3rdparty/libprocess/src/pid.cpp 5191a9dbf478acfa758c602748cafdb296182953 Diff: https://reviews.apache.org/r/59131/diff/6/ Changes: https://reviews.apache.org/r/59131/diff/5-6/ Testing --- make check. Thanks, Avinash sridharan
Re: Review Request 60692: Fixed `net::IPv4` to return `INADDR_ANY` for `ANY()`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60692/ --- (Updated July 6, 2017, 9:47 p.m.) Review request for mesos and Benjamin Hindman. Bugs: MESOS-7765 https://issues.apache.org/jira/browse/MESOS-7765 Repository: mesos Description --- Fixed `net::IPv4` to return `INADDR_ANY` for `ANY()`. Diffs - 3rdparty/stout/include/stout/ip.hpp 25779fa857d138d78c52d1528c070763eba486a1 Diff: https://reviews.apache.org/r/60692/diff/1/ Testing (updated) --- make check. Also sudo ./build/bin/mesos-tests.sh --gtest_filter=MasterTest.KillUnknownTask ``` [==] Running 1 test from 1 test case. [--] Global test environment set-up. [--] 1 test from MasterTest [ RUN ] MasterTest.KillUnknownTask [ OK ] MasterTest.KillUnknownTask (87 ms) [--] 1 test from MasterTest (88 ms total) [--] Global test environment tear-down [==] 1 test from 1 test case ran. (99 ms total) [ PASSED ] 1 test. ``` Thanks, Avinash sridharan
Review Request 60692: Fixed `net::IPv4` to return `INADDR_ANY` for `ANY()`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60692/ --- Review request for mesos. Repository: mesos Description --- Fixed `net::IPv4` to return `INADDR_ANY` for `ANY()`. Diffs - 3rdparty/stout/include/stout/ip.hpp 25779fa857d138d78c52d1528c070763eba486a1 Diff: https://reviews.apache.org/r/60692/diff/1/ Testing --- make check. Thanks, Avinash sridharan
Re: Review Request 60674: Fixed windows failure due to new `net::IP::Network` class.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60674/ --- (Updated July 6, 2017, 1:55 p.m.) Review request for mesos and Joseph Wu. Changes --- Rebased. Bugs: MESOS-7762 https://issues.apache.org/jira/browse/MESOS-7762 Repository: mesos Description (updated) --- Fixed windows failure due to new `net::IP::Network` class. Diffs (updated) - 3rdparty/stout/include/stout/windows/ip.hpp e2ed99e74bc4338c3945b89666dbd0d0361e80e1 Diff: https://reviews.apache.org/r/60674/diff/2/ Changes: https://reviews.apache.org/r/60674/diff/1-2/ Testing --- make check Thanks, Avinash sridharan
Re: Review Request 60675: Fixed compilation failure in port-mapping network isolator.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60675/ --- (Updated July 6, 2017, 1:54 p.m.) Review request for mesos and Joseph Wu. Changes --- Addressed Jame's comments. Bugs: MESOS-7762 https://issues.apache.org/jira/browse/MESOS-7762 Repository: mesos Description --- Fixed compilation failure in port-mapping network isolator. Diffs (updated) - src/linux/routing/filter/icmp.hpp bc0aed0f4fc7c48f06cece14aa9ec43d7fa36bc4 src/linux/routing/filter/ip.hpp ee416bc0c30dfd2b737d0dc32430d30a6dfdff46 src/linux/routing/route.hpp 01d4e7582b19a38bc39f8c394121fe0bed2c0d87 src/linux/routing/route.cpp ceeaf8e557a88569e1f15b3ffc5f0746b66ea638 src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 9d38289c7161d5e931053b587d115684ccc44c94 src/slave/containerizer/mesos/isolators/network/port_mapping.cpp cd008aaebcd42554a9a81d2b059269546f59c966 src/tests/containerizer/port_mapping_tests.cpp 16e015a8ac53a4aa5336b60c40228720b5f6910a Diff: https://reviews.apache.org/r/60675/diff/3/ Changes: https://reviews.apache.org/r/60675/diff/2-3/ Testing --- make check Thanks, Avinash sridharan
Re: Review Request 60675: Fixed compilation failure in port-mapping network isolator.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60675/ --- (Updated July 6, 2017, 5:03 a.m.) Review request for mesos and Joseph Wu. Changes --- Fixed another missing `IPNetwork` instance. Bugs: MESOS-7762 https://issues.apache.org/jira/browse/MESOS-7762 Repository: mesos Description --- Fixed compilation failure in port-mapping network isolator. Diffs (updated) - src/linux/routing/filter/icmp.hpp bc0aed0f4fc7c48f06cece14aa9ec43d7fa36bc4 src/linux/routing/route.hpp 01d4e7582b19a38bc39f8c394121fe0bed2c0d87 src/linux/routing/route.cpp ceeaf8e557a88569e1f15b3ffc5f0746b66ea638 src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 9d38289c7161d5e931053b587d115684ccc44c94 src/slave/containerizer/mesos/isolators/network/port_mapping.cpp cd008aaebcd42554a9a81d2b059269546f59c966 src/tests/containerizer/port_mapping_tests.cpp 16e015a8ac53a4aa5336b60c40228720b5f6910a Diff: https://reviews.apache.org/r/60675/diff/2/ Changes: https://reviews.apache.org/r/60675/diff/1-2/ Testing --- make check Thanks, Avinash sridharan
Re: Review Request 60674: Fixed windows failure due to new `net::IP::Network` class.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60674/ --- (Updated July 6, 2017, 4:12 a.m.) Review request for mesos and Joseph Wu. Bugs: MESOS-7762 https://issues.apache.org/jira/browse/MESOS-7762 Repository: mesos Description (updated) --- Fixed windows build failure due to new `net::IP::Network` class. Diffs - 3rdparty/stout/include/stout/windows/ip.hpp e2ed99e74bc4338c3945b89666dbd0d0361e80e1 Diff: https://reviews.apache.org/r/60674/diff/1/ Testing --- make check Thanks, Avinash sridharan
Review Request 60674: Fixed windows failure due to new `net::IP::Network` class.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60674/ --- Review request for mesos and Joseph Wu. Bugs: MESOS-7762 https://issues.apache.org/jira/browse/MESOS-7762 Repository: mesos Description --- Fixed windows failure due to new `net::IP::Network` class. Diffs - 3rdparty/stout/include/stout/windows/ip.hpp e2ed99e74bc4338c3945b89666dbd0d0361e80e1 Diff: https://reviews.apache.org/r/60674/diff/1/ Testing --- make check Thanks, Avinash sridharan
Review Request 60675: Fixed compilation failure in port-mapping network isolator.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60675/ --- Review request for mesos and Joseph Wu. Bugs: MESOS-7762 https://issues.apache.org/jira/browse/MESOS-7762 Repository: mesos Description --- Fixed compilation failure in port-mapping network isolator. Diffs - src/linux/routing/filter/icmp.hpp bc0aed0f4fc7c48f06cece14aa9ec43d7fa36bc4 src/linux/routing/route.hpp 01d4e7582b19a38bc39f8c394121fe0bed2c0d87 src/linux/routing/route.cpp ceeaf8e557a88569e1f15b3ffc5f0746b66ea638 src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 9d38289c7161d5e931053b587d115684ccc44c94 src/slave/containerizer/mesos/isolators/network/port_mapping.cpp cd008aaebcd42554a9a81d2b059269546f59c966 src/tests/containerizer/port_mapping_tests.cpp 16e015a8ac53a4aa5336b60c40228720b5f6910a Diff: https://reviews.apache.org/r/60675/diff/1/ Testing --- make check Thanks, Avinash sridharan
Re: Review Request 60558: Set container DNS with `--default_container_dns` in Docker executor.
> On July 3, 2017, 6:35 p.m., Avinash sridharan wrote:
> > src/docker/docker.hpp
> > Lines 167 (patched)
> > <https://reviews.apache.org/r/60558/diff/1/?file=1767463#file1767463line168>
> >
> > For consistency can we use `defautlContainerDNS`?
>
> Qian Zhang wrote:
> Originally I used `defautlContainerDNS`, but that will make this line
> exceeding 80 chars :-( I think it also makes sense to use `defaultDNS` since
> here we know we are creating run options for Docker container, so I think we
> do not have to mention `container` in the parameter name.
You can use this `// NOLINT(whitespace/line_length)` to create exceptions? In
`executor.cpp` you use `defaultContainerDNS` so seemed a bit odd.
- Avinash
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60558/#review179530
---
On June 30, 2017, 7:09 a.m., Qian Zhang wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60558/
> ---
>
> (Updated June 30, 2017, 7:09 a.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 8ca0c68836ea5d1a1186e79942f6daccf6c6da46
> src/docker/executor.cpp e03f24461ec7b19cacae79c020406f0a475f2d19
>
>
> Diff: https://reviews.apache.org/r/60558/diff/1/
>
>
> 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": "net1",
> "dns": {
> "nameservers": [ "8.8.8.8", "8.8.4.4" ]
> }
> }
> ],
> "docker": [
> {
> "network": "bridge",
> "dns": {
> "nameservers": [ "8.8.8.8", "8.8.4.4" ],
> "search": [ "xxx.com", "yyy.com" ],
> "options": [ "timeout:3", "attempts:2" ]
> }
> }
> ]
> }
> ```
>
> 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 IDIMAGE COMMAND CREATED
> STATUS PORTS NAMES
> ca642bf31a9fnginx "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
>
>
Re: Review Request 60500: Introduced `--default_container_dns` agent flag.
> On July 3, 2017, 5:22 p.m., Avinash sridharan wrote: > > docs/configuration.md > > Lines 1329 (patched) > > <https://reviews.apache.org/r/60500/diff/1/?file=1766230#file1766230line1329> > > > > This is interesting. I think this example highlights a subtle > > difference between the behavior of the DNS option for CNI and CNM. For CNI > > networks we don't really have a concept of HOST or BRIDGE networks. > > However, for CNM networks since we support this option for HOST, BRIDGE and > > USER networks, we should be explicit about this information. Also, we need > > to emphasize that the `--dns` options for `HOST` and `BRIDGE` networks are > > supported only for docker 1.13 and above (for `USER` it has been available > > since 1.10). We probably want to enforce these version checks for docker. > > Qian Zhang wrote: > > However, for CNM networks since we support this option for HOST, BRIDGE > and USER networks, we should be explicit about this information. Also, we > need to emphasize that the --dns options for HOST and BRIDGE networks are > supported only for docker 1.13 and above. > > I have a very old version of Docker (1.5.0) and I have checked its > `docker run` command also has the `--dns` option and it can work with > `BRIDGE` network but not with `HOST` network. Do you think we need to put > such info into this help message (I mean emphasizing the minimal Docker > version for `--dns` support with `HOST`, `BRIDGE` and `USER` networks > respectively)? That may make this message too big. > > Qian Zhang wrote: > By checking https://github.com/moby/moby/blob/master/CHANGELOG.md, I got > the following info: > 1. `--dns-opt` was introduced to `docker run` in Docker 1.9.0, see > https://github.com/moby/moby/pull/16031. > 2. `--dns` can be used with `HOST` network in Docker 1.12.0, see > https://github.com/moby/moby/pull/22408. > 3. `--dns-option` was introduced to `docker run` in Docker 1.13.0 and > `--dns-opt` was hidden (but it still can be used), see > https://github.com/moby/moby/pull/28186. > > I will do Docker version check based on the above info in > https://reviews.apache.org/r/60558/. Sounds good. I think given the variations that docker supports with different versions I agree with you that it would be too cryptic to put this message in the help. Lets just add the `Docker version` check and be more explicit in our error messages when the version check fails. > On July 3, 2017, 5:22 p.m., Avinash sridharan wrote: > > src/messages/flags.proto > > Lines 36 (patched) > > <https://reviews.apache.org/r/60500/diff/1/?file=1766232#file1766232line36> > > > > Just a thought, should we be more explicit and keep two different DNS > > structures for Mesos and Docker? For example, Docker has the concept of > > network modes where as Mesos doesn't. With this structure we would have to > > use some kind of pattern matching to identify the network type in case of > > docker, e.g., `network="host"` for host-mode networking, `network="bridge"` > > for bridge-mode networking. This is just a thought, I am fine with these > > validation checks being introduced if this seems cleaner. > > > > At the very least we should definitely have a comment here explaining > > how the network-modes are selected for `docker` based on the network names. > > Qian Zhang wrote: > I think we do not need the network type since I will always use the > network name specified by framework to match `ContainerDNS.docker.network`. > And for sure, I will add some comments to explain how the network will be > matched for both CNM networks and CNI networks. Sounds good. Thanks !! - Avinash --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60500/#review179520 --- On June 28, 2017, 2:57 p.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60500/ > --- > > (Updated June 28, 2017, 2:57 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 > --- > > Introduced `--default_container_dns` agent flag. > > > Diffs > - > > docs/configuration.md 0eb696a949003ff11831aed5e4f4ab384cf9992e > src/messages/flags.hpp 70ad58c33067a0b058deafdcc4ffc5f554b6ca72 > src/messages/flags.proto e87075f19714739b93d9f4aa33ea8686a1bb5613 > src/slave/flags.hpp e75c1b4227b443aedf445921b3f2108d930c112c > src/slave/flags.cpp c84aa6724170bba46bbe8410b71d42a1626e > > > Diff: https://reviews.apache.org/r/60500/diff/1/ > > > Testing > --- > > > Thanks, > > Qian Zhang > >
Re: Review Request 60500: Introduced `--default_container_dns` agent flag.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60500/#review179533 --- src/messages/flags.hpp Lines 58 (patched) <https://reviews.apache.org/r/60500/#comment254294> For CNI, since we are adding these entries to the `resolv.conf` we can't support more than three name servers for a given network. Similarly for CNM we probably need to check if we can support more than 3 nameservers for HOST and BRIDGE networks? - Avinash sridharan On June 28, 2017, 2:57 p.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60500/ > --- > > (Updated June 28, 2017, 2:57 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 > --- > > Introduced `--default_container_dns` agent flag. > > > Diffs > - > > docs/configuration.md 0eb696a949003ff11831aed5e4f4ab384cf9992e > src/messages/flags.hpp 70ad58c33067a0b058deafdcc4ffc5f554b6ca72 > src/messages/flags.proto e87075f19714739b93d9f4aa33ea8686a1bb5613 > src/slave/flags.hpp e75c1b4227b443aedf445921b3f2108d930c112c > src/slave/flags.cpp c84aa6724170bba46bbe8410b71d42a1626e > > > Diff: https://reviews.apache.org/r/60500/diff/1/ > > > Testing > --- > > > Thanks, > > Qian Zhang > >
Re: Review Request 60557: Passed default container DNS info to Docker executor.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60557/#review179532 --- Ship it! Ship It! - Avinash sridharan On June 30, 2017, 6:56 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60557/ > --- > > (Updated June 30, 2017, 6:56 a.m.) > > > Review request for mesos, Avinash sridharan and Jie Yu. > > > Bugs: MESOS-7709 > https://issues.apache.org/jira/browse/MESOS-7709 > > > Repository: mesos > > > Description > --- > > Passed default container DNS info to Docker executor. > > > Diffs > - > > src/docker/executor.hpp a4a8ec9905d3c6b3afcf0e2ba4e174c41fbcd75a > src/docker/executor.cpp e03f24461ec7b19cacae79c020406f0a475f2d19 > src/slave/containerizer/docker.cpp 5cd3b6d95ff78fb114a06d49122b7161a6688646 > > > Diff: https://reviews.apache.org/r/60557/diff/1/ > > > Testing > --- > > > Thanks, > > Qian Zhang > >
Re: Review Request 60558: Set container DNS with `--default_container_dns` in Docker executor.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60558/#review179530
---
src/docker/docker.hpp
Lines 167 (patched)
<https://reviews.apache.org/r/60558/#comment254291>
For consistency can we use `defautlContainerDNS`?
src/docker/docker.cpp
Line 839 (original), 867 (patched)
<https://reviews.apache.org/r/60558/#comment254293>
We would need to perform a version check for supported DNS options here as
well.
- Avinash sridharan
On June 30, 2017, 7:09 a.m., Qian Zhang wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60558/
> ---
>
> (Updated June 30, 2017, 7:09 a.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 8ca0c68836ea5d1a1186e79942f6daccf6c6da46
> src/docker/executor.cpp e03f24461ec7b19cacae79c020406f0a475f2d19
>
>
> Diff: https://reviews.apache.org/r/60558/diff/1/
>
>
> 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": "net1",
> "dns": {
> "nameservers": [ "8.8.8.8", "8.8.4.4" ]
> }
> }
> ],
> "docker": [
> {
> "network": "bridge",
> "dns": {
> "nameservers": [ "8.8.8.8", "8.8.4.4" ],
> "search": [ "xxx.com", "yyy.com" ],
> "options": [ "timeout:3", "attempts:2" ]
> }
> }
> ]
> }
> ```
>
> 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 IDIMAGE COMMAND CREATED
> STATUS PORTS NAMES
> ca642bf31a9fnginx "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
>
>
Re: Review Request 60500: Introduced `--default_container_dns` agent flag.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60500/#review179520 --- docs/configuration.md Lines 1309 (patched) <https://reviews.apache.org/r/60500/#comment254266> s/CNI plugin/ a CNI plugin, docs/configuration.md Lines 1312 (patched) <https://reviews.apache.org/r/60500/#comment254267> `ContainerInfo.docker.parameter` docs/configuration.md Lines 1329 (patched) <https://reviews.apache.org/r/60500/#comment254268> This is interesting. I think this example highlights a subtle difference between the behavior of the DNS option for CNI and CNM. For CNI networks we don't really have a concept of HOST or BRIDGE networks. However, for CNM networks since we support this option for HOST, BRIDGE and USER networks, we should be explicit about this information. Also, we need to emphasize that the `--dns` options for `HOST` and `BRIDGE` networks are supported only for docker 1.13 and above (for `USER` it has been available since 1.10). We probably want to enforce these version checks for docker. src/messages/flags.proto Line 21 (original), 23 (patched) <https://reviews.apache.org/r/60500/#comment254273> Not yours but might as well fix the comment formatting to conform with the rest of the `.proto` files? src/messages/flags.proto Lines 32 (patched) <https://reviews.apache.org/r/60500/#comment254271> 2 lines apart? Also, I think for protobuf definitions we are using the following format for comments: /** * */ src/messages/flags.proto Lines 36 (patched) <https://reviews.apache.org/r/60500/#comment254275> Just a thought, should we be more explicit and keep two different DNS structures for Mesos and Docker? For example, Docker has the concept of network modes where as Mesos doesn't. With this structure we would have to use some kind of pattern matching to identify the network type in case of docker, e.g., `network="host"` for host-mode networking, `network="bridge"` for bridge-mode networking. This is just a thought, I am fine with these validation checks being introduced if this seems cleaner. At the very least we should definitely have a comment here explaining how the network-modes are selected for `docker` based on the network names. src/slave/flags.cpp Lines 766 (patched) <https://reviews.apache.org/r/60500/#comment254277> Ditto for comments on `ocnfigure.md`. - Avinash sridharan On June 28, 2017, 2:57 p.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60500/ > ------- > > (Updated June 28, 2017, 2:57 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 > --- > > Introduced `--default_container_dns` agent flag. > > > Diffs > - > > docs/configuration.md 0eb696a949003ff11831aed5e4f4ab384cf9992e > src/messages/flags.hpp 70ad58c33067a0b058deafdcc4ffc5f554b6ca72 > src/messages/flags.proto e87075f19714739b93d9f4aa33ea8686a1bb5613 > src/slave/flags.hpp e75c1b4227b443aedf445921b3f2108d930c112c > src/slave/flags.cpp c84aa6724170bba46bbe8410b71d42a1626e > > > Diff: https://reviews.apache.org/r/60500/diff/1/ > > > Testing > --- > > > Thanks, > > Qian Zhang > >
Re: Review Request 59128: Added initialization logic in Mesos agent for IPv6 flags.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59128/ --- (Updated June 29, 2017, 11:24 p.m.) Review request for mesos, Benjamin Hindman and Jie Yu. Changes --- Rebased. Bugs: MESOS-7488 https://issues.apache.org/jira/browse/MESOS-7488 Repository: mesos Description --- Operators can now set the IPv6 address for the agent using the `--ip6` and `--ip6_discovery_command` flags. Currently, the only place the agent will use the IPv6 address would be to advertise v6 addresses for containers running on the host network. Diffs (updated) - src/slave/main.cpp 66ae7db4039eedba7ff4eff51495384317c09354 Diff: https://reviews.apache.org/r/59128/diff/3/ Changes: https://reviews.apache.org/r/59128/diff/2-3/ Testing --- make check. Thanks, Avinash sridharan
Re: Review Request 59127: Added IPv6 flags for Mesos agent.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59127/ --- (Updated June 29, 2017, 11:25 p.m.) Review request for mesos, Benjamin Hindman and Jie Yu. Changes --- Rebased. Bugs: MESOS-7488 https://issues.apache.org/jira/browse/MESOS-7488 Repository: mesos Description --- Added IPv6 flags for Mesos agent. Diffs (updated) - src/slave/flags.hpp e75c1b4227b443aedf445921b3f2108d930c112c src/slave/flags.cpp c84aa6724170bba46bbe8410b71d42a1626e Diff: https://reviews.apache.org/r/59127/diff/4/ Changes: https://reviews.apache.org/r/59127/diff/3-4/ Testing --- make check Thanks, Avinash sridharan
Re: Review Request 59688: Moved `net::IPNetwork` to `net::IP:Network`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59688/ --- (Updated June 29, 2017, 11:24 p.m.) Review request for mesos, Benjamin Hindman and Jie Yu. Changes --- Rebased. Bugs: MESOS-7488 https://issues.apache.org/jira/browse/MESOS-7488 Repository: mesos Description --- Moved `net::IPNetwork` to `net::IP:Network`. Diffs (updated) - 3rdparty/stout/include/stout/ip.hpp ee48befac8b9bb4956aec0c5bdebd69347bd5890 3rdparty/stout/tests/ip_tests.cpp 39ee0598e3580510a30894fd3f47d2014f596ef7 Diff: https://reviews.apache.org/r/59688/diff/4/ Changes: https://reviews.apache.org/r/59688/diff/3-4/ Testing --- make check Thanks, Avinash sridharan
