You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Qian Zhang <zh...@gmail.com> on 2017/07/12 06:58:28 UTC

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

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


Testing
-------


Thanks,

Qian Zhang


Re: Review Request 60793: Added a test `CniIsolatorTest.ROOT_VerifyDefaultDNS`.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60793/#review180286
-----------------------------------------------------------



Patch looks great!

Reviews applied: [60500, 60557, 60558, 60600, 60760, 60761, 60793]

Passed command: support\windows-build.bat

- Mesos Reviewbot Windows


On July 12, 2017, 6:58 a.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60793/
> -----------------------------------------------------------
> 
> (Updated July 12, 2017, 6:58 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/1/
> 
> 
> Testing
> -------
> 
> sudo make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>


Re: Review Request 60793: Added a test `CniIsolatorTest.ROOT_VerifyDefaultDNS`.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60793/#review181260
-----------------------------------------------------------



Patch looks great!

Reviews applied: [60500, 60557, 60558, 61075, 60600, 60760, 60761, 60793]

Passed command: support\windows-build.bat

- Mesos Reviewbot Windows


On July 24, 2017, 8:03 a.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60793/
> -----------------------------------------------------------
> 
> (Updated July 24, 2017, 8: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
> -------
> 
> Added a test `CniIsolatorTest.ROOT_VerifyDefaultDNS`.
> 
> 
> Diffs
> -----
> 
>   src/tests/containerizer/cni_isolator_tests.cpp ae0980bd671849fcd3e19941b33c7d3b09fdae7c 
> 
> 
> Diff: https://reviews.apache.org/r/60793/diff/2/
> 
> 
> Testing
> -------
> 
> sudo make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>


Re: Review Request 60793: Added a test `CniIsolatorTest.ROOT_VerifyDefaultDNS`.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60793/#review181321
-----------------------------------------------------------



Patch looks great!

Reviews applied: [60500, 60557, 60558, 61075, 60600, 60760, 60761, 60793]

Passed command: support\windows-build.bat

- Mesos Reviewbot Windows


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 60793: Added a test `CniIsolatorTest.ROOT_VerifyDefaultDNS`.

Posted by Qian Zhang <zh...@gmail.com>.

> On July 28, 2017, 8:41 a.m., Avinash sridharan wrote:
> > src/tests/containerizer/cni_isolator_tests.cpp
> > Lines 1226 (patched)
> > <https://reviews.apache.org/r/60793/diff/3/?file=1782045#file1782045line1226>
> >
> >     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.
> 
> Qian Zhang wrote:
>     I added a test `https://reviews.apache.org/r/61219/` for the wildcard match.

And I added a test https://reviews.apache.org/r/61245/ for the flag validation.


- Qian


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


On July 25, 2017, 2:08 p.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60793/
> -----------------------------------------------------------
> 
> (Updated July 25, 2017, 2:08 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_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 60793: Added a test `CniIsolatorTest.ROOT_VerifyDefaultDNS`.

Posted by Qian Zhang <zh...@gmail.com>.

> On July 28, 2017, 8:41 a.m., Avinash sridharan wrote:
> > src/tests/containerizer/cni_isolator_tests.cpp
> > Lines 1226 (patched)
> > <https://reviews.apache.org/r/60793/diff/3/?file=1782045#file1782045line1226>
> >
> >     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.

I added a test `https://reviews.apache.org/r/61219/` for the wildcard match.


- Qian


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


On July 25, 2017, 2:08 p.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60793/
> -----------------------------------------------------------
> 
> (Updated July 25, 2017, 2:08 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_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 60793: Added a test `CniIsolatorTest.ROOT_VerifyDefaultDNS`.

Posted by Avinash sridharan <av...@mesosphere.io>.
-----------------------------------------------------------
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 60793: Added a test `CniIsolatorTest.ROOT_VerifyDefaultDNS`.

Posted by Avinash sridharan <av...@mesosphere.io>.
-----------------------------------------------------------
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 60793: Added a test `DefaultContainerDNSCniTest.ROOT_VerifyDefaultDNS`.

Posted by Avinash sridharan <av...@mesosphere.io>.
-----------------------------------------------------------
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 60793: Added a test `DefaultContainerDNSCniTest.ROOT_VerifyDefaultDNS`.

Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60793/
-----------------------------------------------------------

(Updated Aug. 3, 2017, 1:56 p.m.)


Review request for mesos, Avinash sridharan and Jie Yu.


Changes
-------

Addressed comments.


Bugs: MESOS-7709
    https://issues.apache.org/jira/browse/MESOS-7709


Repository: mesos


Description
-------

Added a test `DefaultContainerDNSCniTest.ROOT_VerifyDefaultDNS`.


Diffs (updated)
-----

  src/tests/containerizer/cni_isolator_tests.cpp ae0980bd671849fcd3e19941b33c7d3b09fdae7c 


Diff: https://reviews.apache.org/r/60793/diff/5/

Changes: https://reviews.apache.org/r/60793/diff/4-5/


Testing
-------

sudo make check


Thanks,

Qian Zhang


Re: Review Request 60793: Added a test `DefaultContainerDNSCniTest.ROOT_VerifyDefaultDNS`.

Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60793/
-----------------------------------------------------------

(Updated Aug. 2, 2017, 4:13 p.m.)


Review request for mesos, Avinash sridharan and Jie Yu.


Changes
-------

Parameterized the tests.


Summary (updated)
-----------------

Added a test `DefaultContainerDNSCniTest.ROOT_VerifyDefaultDNS`.


Bugs: MESOS-7709
    https://issues.apache.org/jira/browse/MESOS-7709


Repository: mesos


Description (updated)
-------

Added a test `DefaultContainerDNSCniTest.ROOT_VerifyDefaultDNS`.


Diffs (updated)
-----

  src/tests/containerizer/cni_isolator_tests.cpp ae0980bd671849fcd3e19941b33c7d3b09fdae7c 


Diff: https://reviews.apache.org/r/60793/diff/4/

Changes: https://reviews.apache.org/r/60793/diff/3-4/


Testing
-------

sudo make check


Thanks,

Qian Zhang


Re: Review Request 60793: Added a test `CniIsolatorTest.ROOT_VerifyDefaultDNS`.

Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60793/
-----------------------------------------------------------

(Updated July 25, 2017, 2:08 p.m.)


Review request for mesos, Avinash sridharan and Jie Yu.


Changes
-------

Rebased.


Bugs: MESOS-7709
    https://issues.apache.org/jira/browse/MESOS-7709


Repository: mesos


Description
-------

Added a test `CniIsolatorTest.ROOT_VerifyDefaultDNS`.


Diffs (updated)
-----

  src/tests/containerizer/cni_isolator_tests.cpp ae0980bd671849fcd3e19941b33c7d3b09fdae7c 


Diff: https://reviews.apache.org/r/60793/diff/3/

Changes: https://reviews.apache.org/r/60793/diff/2-3/


Testing
-------

sudo make check


Thanks,

Qian Zhang


Re: Review Request 60793: Added a test `CniIsolatorTest.ROOT_VerifyDefaultDNS`.

Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60793/
-----------------------------------------------------------

(Updated July 24, 2017, 4:03 p.m.)


Review request for mesos, Avinash sridharan and Jie Yu.


Changes
-------

Rebased.


Bugs: MESOS-7709
    https://issues.apache.org/jira/browse/MESOS-7709


Repository: mesos


Description
-------

Added a test `CniIsolatorTest.ROOT_VerifyDefaultDNS`.


Diffs (updated)
-----

  src/tests/containerizer/cni_isolator_tests.cpp ae0980bd671849fcd3e19941b33c7d3b09fdae7c 


Diff: https://reviews.apache.org/r/60793/diff/2/

Changes: https://reviews.apache.org/r/60793/diff/1-2/


Testing
-------

sudo make check


Thanks,

Qian Zhang


Re: Review Request 60793: Added a test `CniIsolatorTest.ROOT_VerifyDefaultDNS`.

Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60793/
-----------------------------------------------------------

(Updated July 12, 2017, 2:58 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_VerifyDefaultDNS`.


Diffs
-----

  src/tests/containerizer/cni_isolator_tests.cpp ae0980bd671849fcd3e19941b33c7d3b09fdae7c 


Diff: https://reviews.apache.org/r/60793/diff/1/


Testing (updated)
-------

sudo make check


Thanks,

Qian Zhang