Re: Review Request 71742: Mesos agent shouldn't respond pings if no master is registered

[Xudong Ni via Review Board]

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

(Updated Nov. 22, 2019, 8 p.m.)


Review request for mesos, Benjamin Mahler and Jiang Yan Xu.


Changes
---

remove the test no longer applicable in the new scenario


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


Repository: mesos


Description
---

In the case agents lost ZooKeeper connections and resetting its
master to none and beginning to dropping control messages from the
master, agent should not respond pings from master.


Diffs (updated)
-

  src/slave/slave.cpp 3839a120446339fea8aa857f431a2dba28ed4002 
  src/tests/slave_recovery_tests.cpp 0efd3a6ac09ad06d9365b7bb2295157b5175e6b8 


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

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


Testing (updated)
---

make check


Thanks,

Xudong Ni

Re: Review Request 71742: Mesos agent shouldn't respond pings if no master is registered

[Xudong Ni via Review Board]

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

(Updated Nov. 19, 2019, 10:39 p.m.)


Review request for mesos, Benjamin Mahler and Jiang Yan Xu.


Changes
---

Addressed the comments except the test which will be updated later


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


Repository: mesos


Description
---

In the case agents lost ZooKeeper connections and resetting its
master to none and beginning to dropping control messages from the
master, agent should not respond pings from master.


Diffs (updated)
-

  src/slave/slave.cpp 3839a120446339fea8aa857f431a2dba28ed4002 


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

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


Testing
---

==] 2322 tests from 222 test cases ran. (1038166 ms total)
[  PASSED  ] 2321 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] SlaveRecoveryTest/0.PingTimeoutDuringRecovery, where TypeParam = 
mesos::internal::slave::MesosContainerizer

This failed test verifies that the agent responds to pings from the master 
while the agent is performing recovery, this PR will break this scenario.


Thanks,

Xudong Ni

Re: Review Request 71742: Mesos agent shouldn't respond pings if no master is registered

[Xudong Ni via Review Board]

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

(Updated Nov. 13, 2019, 10:39 p.m.)


Review request for mesos and Jiang Yan Xu.


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


Repository: mesos


Description
---

In the case agents lost ZooKeeper connections and resetting its
master to none and beginning to dropping control messages from the
master, agent should not respond pings from master.


Diffs
-

  src/slave/slave.cpp 3839a120446339fea8aa857f431a2dba28ed4002 


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


Testing (updated)
---

==] 2322 tests from 222 test cases ran. (1038166 ms total)
[  PASSED  ] 2321 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] SlaveRecoveryTest/0.PingTimeoutDuringRecovery, where TypeParam = 
mesos::internal::slave::MesosContainerizer

This failed test verifies that the agent responds to pings from the master 
while the agent is performing recovery, this PR will break this scenario.


Thanks,

Xudong Ni

Re: Review Request 71742: Mesos agent shouldn't respond pings if no master is registered

[Xudong Ni via Review Board]

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

(Updated Nov. 13, 2019, 12:18 a.m.)


Review request for mesos and Jiang Yan Xu.


Summary (updated)
-

Mesos agent shouldn't respond pings if no master is registered


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


Repository: mesos


Description (updated)
---

In the case agents lost ZooKeeper connections and resetting its
master to none and beginning to dropping control messages from the
master, agent should not respond pings from master.


Diffs (updated)
-

  src/slave/slave.cpp 3839a120446339fea8aa857f431a2dba28ed4002 


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

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


Testing (updated)
---

WIP


Thanks,

Xudong Ni

Review Request 71742: Mesos agent responds pings only from currently registered master

[Xudong Ni via Review Board]

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

Review request for mesos.


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


Repository: mesos


Description
---

In the case agents lost ZooKeeper connections and resetting its
master to None and beginning to dropping control messages from the
master, agent should not respond pings from that master.


Diffs
-

  src/slave/slave.cpp 3839a120446339fea8aa857f431a2dba28ed4002 


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


Testing
---


Thanks,

Xudong Ni

Re: Review Request 71080: Master should store the list of completed framework ids for lifecycle.

[Xudong Ni via Review Board]

> On July 24, 2019, 9:34 p.m., Jiang Yan Xu wrote:
> > include/mesos/allocator/allocator.hpp
> > Lines 66 (patched)
> > 
> >
> > `maxCompletedFrameworks` is used for the metrics to keep for completed 
> > frameworks in the allocator.
> > 
> > `maxCompletedFramework_ids` (we should name it 
> > `maxCompletedFrameworkIDs` if it were useful) is not used and we don't need 
> > to add this field.

maxCompletedFramework_ids can be removed since we don't need it in metrics


- Xudong


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


On July 24, 2019, 11:10 p.m., Xudong Ni wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71080/
> ---
> 
> (Updated July 24, 2019, 11:10 p.m.)
> 
> 
> Review request for mesos, James Peach and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-8636
> https://issues.apache.org/jira/browse/MESOS-8636
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It should be separately from that for webUI and endpoints.
> Currently the master stores the history of completed frameworks in
> a map with the full historical data of the framework, it is
> prohibitively expensive to keep a long history; In order to reject
> frameworks from reregistering if they have previously marked as
> completed, we only need to persist the framework IDs and are able
> to keep long history.
> 
> 
> Diffs
> -
> 
>   docs/configuration/master.md c56ac8510ea968f9587e23e81ed310caa968ee9e 
>   docs/operator-http-api.md 1167838928e0fcb774cf904fa6ef5e6847134e15 
>   src/master/constants.hpp 26afa356b7844b4ec6c2caeef33bd39c51148d5f 
>   src/master/flags.hpp 78623d68bf428cd3f52684303d98a525d42eb878 
>   src/master/flags.cpp 74f4daadd48e8e691be43759b88dc8b3c2df489a 
>   src/master/http.cpp 765d5052ab6a8d731113f8922d20fb280b843003 
>   src/master/master.hpp 5c229c53f96dda6b30731017b8adbf57c9fc9f95 
>   src/master/master.cpp fbde112eabf2c6da363ab313cf3f4c54f581eaa7 
> 
> 
> Diff: https://reviews.apache.org/r/71080/diff/8/
> 
> 
> Testing
> ---
> 
> make check
> 
> sudo GLOG_v=1 ./bin/mesos-tests.sh --verbose 
> --gtest_filter=MasterTest.MaxCompletedFrameworksFlag --gtest_break_on_failure 
> --gtest_repeat=1000
> [   OK ] MasterTest.MaxCompletedFrameworksFlag (230 ms)
> [--] 1 test from MasterTest (235 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (281 ms total)
> [  PASSED  ] 1 test.
> 
> 
> Thanks,
> 
> Xudong Ni
> 
>

Re: Review Request 71080: Master should store the list of completed framework ids for lifecycle.

[Xudong Ni via Review Board]

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

(Updated July 24, 2019, 11:10 p.m.)


Review request for mesos, James Peach and Jiang Yan Xu.


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


Repository: mesos


Description
---

It should be separately from that for webUI and endpoints.
Currently the master stores the history of completed frameworks in
a map with the full historical data of the framework, it is
prohibitively expensive to keep a long history; In order to reject
frameworks from reregistering if they have previously marked as
completed, we only need to persist the framework IDs and are able
to keep long history.


Diffs (updated)
-

  docs/configuration/master.md c56ac8510ea968f9587e23e81ed310caa968ee9e 
  docs/operator-http-api.md 1167838928e0fcb774cf904fa6ef5e6847134e15 
  src/master/constants.hpp 26afa356b7844b4ec6c2caeef33bd39c51148d5f 
  src/master/flags.hpp 78623d68bf428cd3f52684303d98a525d42eb878 
  src/master/flags.cpp 74f4daadd48e8e691be43759b88dc8b3c2df489a 
  src/master/http.cpp 765d5052ab6a8d731113f8922d20fb280b843003 
  src/master/master.hpp 5c229c53f96dda6b30731017b8adbf57c9fc9f95 
  src/master/master.cpp fbde112eabf2c6da363ab313cf3f4c54f581eaa7 


Diff: https://reviews.apache.org/r/71080/diff/8/

Changes: https://reviews.apache.org/r/71080/diff/7-8/


Testing
---

make check

sudo GLOG_v=1 ./bin/mesos-tests.sh --verbose 
--gtest_filter=MasterTest.MaxCompletedFrameworksFlag --gtest_break_on_failure 
--gtest_repeat=1000
[   OK ] MasterTest.MaxCompletedFrameworksFlag (230 ms)
[--] 1 test from MasterTest (235 ms total)

[--] Global test environment tear-down
[==] 1 test from 1 test case ran. (281 ms total)
[  PASSED  ] 1 test.


Thanks,

Xudong Ni

Re: Review Request 71080: Master should store the list of completed framework ids for lifecycle.

[Xudong Ni via Review Board]

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

(Updated July 19, 2019, 3:10 a.m.)


Review request for mesos, James Peach and Jiang Yan Xu.


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


Repository: mesos


Description
---

It should be separately from that for webUI and endpoints.
Currently the master stores the history of completed frameworks in
a map with the full historical data of the framework, it is
prohibitively expensive to keep a long history; In order to reject
frameworks from reregistering if they have previously marked as
completed, we only need to persist the framework ids and are able
to keep long history.


Diffs (updated)
-

  docs/configuration/master.md c56ac8510ea968f9587e23e81ed310caa968ee9e 
  docs/operator-http-api.md 2d4a9b66e20cf19eceec718b7de3d812ab285772 
  include/mesos/allocator/allocator.hpp 
2bab53ab5fb25931a724c20a039e1301983ba574 
  src/master/constants.hpp 26afa356b7844b4ec6c2caeef33bd39c51148d5f 
  src/master/flags.hpp 78623d68bf428cd3f52684303d98a525d42eb878 
  src/master/flags.cpp 74f4daadd48e8e691be43759b88dc8b3c2df489a 
  src/master/http.cpp 765d5052ab6a8d731113f8922d20fb280b843003 
  src/master/master.hpp ffa7423ba533725f7c1123d9aa507b1348e7f281 
  src/master/master.cpp f1ca637b4cb0382caec53b5a81f6a4eb46f4dd2d 


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

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


Testing
---

make check

sudo GLOG_v=1 ./bin/mesos-tests.sh --verbose 
--gtest_filter=MasterTest.MaxCompletedFrameworksFlag --gtest_break_on_failure 
--gtest_repeat=1000
[   OK ] MasterTest.MaxCompletedFrameworksFlag (230 ms)
[--] 1 test from MasterTest (235 ms total)

[--] Global test environment tear-down
[==] 1 test from 1 test case ran. (281 ms total)
[  PASSED  ] 1 test.


Thanks,

Xudong Ni

Re: Review Request 71080: Master should store the list of completed framework ids for lifecycle.

[Xudong Ni via Review Board]

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

(Updated July 19, 2019, 12:09 a.m.)


Review request for mesos, James Peach and Jiang Yan Xu.


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


Repository: mesos


Description
---

It should be separately from that for webUI and endpoints.
Currently the master stores the history of completed frameworks in
a map with the full historical data of the framework, it is
prohibitively expensive to keep a long history; In order to reject
frameworks from reregistering if they have previously marked as
completed, we only need to persist the framework ids and are able
to keep long history.


Diffs (updated)
-

  docs/configuration/master.md c56ac8510ea968f9587e23e81ed310caa968ee9e 
  docs/operator-http-api.md 2d4a9b66e20cf19eceec718b7de3d812ab285772 
  include/mesos/allocator/allocator.hpp 
2bab53ab5fb25931a724c20a039e1301983ba574 
  src/master/flags.hpp 78623d68bf428cd3f52684303d98a525d42eb878 
  src/master/flags.cpp 74f4daadd48e8e691be43759b88dc8b3c2df489a 
  src/master/http.cpp 765d5052ab6a8d731113f8922d20fb280b843003 
  src/master/master.hpp ffa7423ba533725f7c1123d9aa507b1348e7f281 
  src/master/master.cpp f1ca637b4cb0382caec53b5a81f6a4eb46f4dd2d 


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

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


Testing
---

make check

sudo GLOG_v=1 ./bin/mesos-tests.sh --verbose 
--gtest_filter=MasterTest.MaxCompletedFrameworksFlag --gtest_break_on_failure 
--gtest_repeat=1000
[   OK ] MasterTest.MaxCompletedFrameworksFlag (230 ms)
[--] 1 test from MasterTest (235 ms total)

[--] Global test environment tear-down
[==] 1 test from 1 test case ran. (281 ms total)
[  PASSED  ] 1 test.


Thanks,

Xudong Ni

Re: Review Request 71080: Master should store the list of completed framework ids for lifecycle.

[Xudong Ni via Review Board]

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

(Updated July 17, 2019, 6:58 p.m.)


Review request for mesos, James Peach and Jiang Yan Xu.


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


Repository: mesos


Description
---

It should be separately from that for webUI and endpoints.
Currently the master stores the history of completed frameworks in
a map with the full historical data of the framework, it is
prohibitively expensive to keep a long history; In order to reject
frameworks from reregistering if they have previously marked as
completed, we only need to persist the framework ids and are able
to keep long history.


Diffs (updated)
-

  docs/configuration/master.md c56ac8510ea968f9587e23e81ed310caa968ee9e 
  docs/operator-http-api.md 2d4a9b66e20cf19eceec718b7de3d812ab285772 
  include/mesos/allocator/allocator.hpp 
2bab53ab5fb25931a724c20a039e1301983ba574 
  src/master/flags.hpp 78623d68bf428cd3f52684303d98a525d42eb878 
  src/master/flags.cpp 74f4daadd48e8e691be43759b88dc8b3c2df489a 
  src/master/http.cpp 765d5052ab6a8d731113f8922d20fb280b843003 
  src/master/master.hpp ffa7423ba533725f7c1123d9aa507b1348e7f281 
  src/master/master.cpp f1ca637b4cb0382caec53b5a81f6a4eb46f4dd2d 


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

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


Testing
---

make check


Thanks,

Xudong Ni

Review Request 71080: Master should store the list of completed framework ids for lifecycle.

[Xudong Ni via Review Board]

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

Review request for mesos, James Peach and Jiang Yan Xu.


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


Repository: mesos


Description
---

It should be separately from that for webUI and endpoints.
Currently the master stores the history of completed frameworks in
a map with the full historical data of the framework, it is
prohibitively expensive to keep a long history; In order to reject
frameworks from reregistering if they have previously marked as
completed, we only need to persist the framework ids and are able
to keep long history.


Diffs
-

  docs/configuration/master.md c56ac8510ea968f9587e23e81ed310caa968ee9e 
  docs/operator-http-api.md 2d4a9b66e20cf19eceec718b7de3d812ab285772 
  include/mesos/allocator/allocator.hpp 
2bab53ab5fb25931a724c20a039e1301983ba574 
  src/master/flags.hpp 78623d68bf428cd3f52684303d98a525d42eb878 
  src/master/flags.cpp 74f4daadd48e8e691be43759b88dc8b3c2df489a 
  src/master/http.cpp cd0f40cb7b966d6620e3fb49d4c08807185c9101 
  src/master/master.hpp e8def83fe9bcee19772df9a9764852bc694c5247 
  src/master/master.cpp 5247377c2e7e92b9843dd4c9d28f92ba679ad742 


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


Testing
---

make check


Thanks,

Xudong Ni

Re: Review Request 70660: Fix the XFS build for recent Fedora versions.

[Xudong Ni via Review Board]

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


Ship it!




Ship It!

- Xudong Ni


On May 17, 2019, 2:24 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70660/
> ---
> 
> (Updated May 17, 2019, 2:24 a.m.)
> 
> 
> Review request for mesos, Xudong Ni, Gilbert Song, Jie Yu, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> On Fedora 30, we need to include  to get the right
> types, since  is no longer self-contained. For
> earlier versions, it is still safe to include both headers if
> they are available, though h> may need to be
> obtained via the xfsprogs-qa-devel package.
> 
> 
> Diffs
> -
> 
>   configure.ac b4bad5716986e2f7c132c6515179a65ccbfdaeac 
>   src/slave/containerizer/mesos/isolators/xfs/utils.hpp 
> db1829abbaac6113d39e71673403afe75b5ee738 
> 
> 
> Diff: https://reviews.apache.org/r/70660/diff/1/
> 
> 
> Testing
> ---
> 
> make check (Fedota 30)
> make (CentOS 6)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 69615: Disable containerizer ptrace attach.

[Xudong Ni via Review Board]

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


Ship it!




Ship It!

- Xudong Ni


On Jan. 2, 2019, 5:15 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69615/
> ---
> 
> (Updated Jan. 2, 2019, 5:15 p.m.)
> 
> 
> Review request for mesos, Xudong Ni, Gilbert Song, Jie Yu, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-9349
> https://issues.apache.org/jira/browse/MESOS-9349
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Use `prctl(PR_SET_DUMPABLE)` to disable the ability to attach to
> the containerizer process(es) on Linux systems. This prevents
> unprivileged containerized processes from reading information
> about the containerizer process(es) from `/proc`. This gives an
> additional layer of protection against leaking information to
> untrusted container processes.
> 
> 
> Diffs
> -
> 
>   docs/configuration/agent.md 330283f4e3957075dd4310de4a841feac23de36c 
>   src/launcher/executor.cpp f962e800f23d5582b1bc04a263253893492a5054 
>   src/slave/containerizer/mesos/containerizer.cpp 
> a5cf2da55c046c5c45e0c2ca3400f64de12de62b 
>   src/slave/containerizer/mesos/launch.hpp 
> 0a6394d56321948ad760ac69c05456319a254842 
>   src/slave/containerizer/mesos/launch.cpp 
> 2f1c9e7a8748c9d7eab25bc8567ca68308e680f9 
>   src/slave/flags.hpp 494ae02ab5eb365e2cda5017be573691107c3f28 
>   src/slave/flags.cpp 6bac8e1409f04d639204c45eda8a90c098e3dbd0 
>   src/slave/slave.cpp ad3b693a716cf6103345a157bf28dd60a7b07d32 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 449928c10b897061642af8ad267f8b70695940e6 
>   src/tests/slave_tests.cpp 4aed5d68e9a408821880ffaede482937be1999f4 
> 
> 
> Diff: https://reviews.apache.org/r/69615/diff/2/
> 
> 
> Testing
> ---
> 
> make check (Fedora 29)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 69615: Disable containerizer ptrace attach.

[Xudong Ni via Review Board]

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




src/slave/containerizer/mesos/launch.cpp
Lines 142 (patched)


Is this redudent empty line comparing to the rest of style?



src/tests/containerizer/mesos_containerizer_tests.cpp
Lines 304 (patched)


Is this redudent empty line comparing to the rest of style?



src/tests/containerizer/mesos_containerizer_tests.cpp
Lines 423 (patched)


Is this redudent empty line comparing to the rest of style?



src/tests/containerizer/mesos_containerizer_tests.cpp
Lines 425 (patched)


Shall we have two empty lines before the next test?


- Xudong Ni


On Dec. 21, 2018, 5:20 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69615/
> ---
> 
> (Updated Dec. 21, 2018, 5:20 a.m.)
> 
> 
> Review request for mesos, Xudong Ni, Gilbert Song, Jie Yu, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-9349
> https://issues.apache.org/jira/browse/MESOS-9349
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Use `prctl(PR_SET_DUMPABLE)` to disable the ability to attach to
> the containerizer process(es) on Linux systems. This prevents
> unprivileged containerized processes from reading information
> about the containerizer process(es) from `/proc`. This gives an
> additional layer of protection against leaking information to
> untrusted container processes.
> 
> 
> Diffs
> -
> 
>   docs/configuration/agent.md 7a8df6852dc2af174a6c5a552dca88fa1b1c29f3 
>   src/launcher/executor.cpp f962e800f23d5582b1bc04a263253893492a5054 
>   src/slave/containerizer/mesos/containerizer.cpp 
> a5cf2da55c046c5c45e0c2ca3400f64de12de62b 
>   src/slave/containerizer/mesos/launch.hpp 
> 0a6394d56321948ad760ac69c05456319a254842 
>   src/slave/containerizer/mesos/launch.cpp 
> 2f1c9e7a8748c9d7eab25bc8567ca68308e680f9 
>   src/slave/flags.hpp 29d8b7985ffde57da02b5fe0d3a524e98acc27c8 
>   src/slave/flags.cpp ccaf65029ec2d0e78041fc3992a0bf5ca0798686 
>   src/slave/slave.cpp ad3b693a716cf6103345a157bf28dd60a7b07d32 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 449928c10b897061642af8ad267f8b70695940e6 
>   src/tests/slave_tests.cpp 4aed5d68e9a408821880ffaede482937be1999f4 
> 
> 
> Diff: https://reviews.apache.org/r/69615/diff/1/
> 
> 
> Testing
> ---
> 
> make check (Fedora 29)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 68706: Added master failover reregistration progress metrics.

[Xudong Ni via Review Board]

> On Oct. 15, 2018, 4:45 p.m., James Peach wrote:
> > I think that we need test for this as well. At minimum, we ought to update 
> > `MasterTest.MetricsInMetricsEndpoint`. Best would be a test that registers 
> > a number of agents, then restarts the master and validates the metrics.

Basic test was added; And we validated the actual metrics in the real 
environment


> On Oct. 15, 2018, 4:45 p.m., James Peach wrote:
> > src/master/master.cpp
> > Lines 1850 (patched)
> > 
> >
> > I found the arithmetic here pretty confusing. How about simplifying 
> > this to:
> > ```
> > 
> > double percentRegistered = metrics->slave_reregistrations.value().get() 
> > / expectedAgentCount;
> > 
> > if (slave25PercentageRegistered.value().get() == 0) {
> >   if (percentRegistered > 0.25) {
> > slaves_25_percent_reregistered_secs = t;
> >   }
> > }
> > ```

This is actually fixed for another comments to use the ceil:

  if((recovered_agents_25_percent_reregistered_secs.value().get() == 0.0)
  && (reregisteredAgentCount == ceil(recoveredAgentCount.get() * 0.25)))
  {
recovered_agents_25_percent_reregistered_secs = t;
  }


- Xudong


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


On Oct. 19, 2018, 11:56 p.m., Xudong Ni wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68706/
> ---
> 
> (Updated Oct. 19, 2018, 11:56 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, James Peach, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-9178
> https://issues.apache.org/jira/browse/MESOS-9178
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> During the master failover, the time that the master elected is
> considered as the start of failover. In the progress of
> reregistration, the percentile represents the time when such
> percentile of agents finished registration again; The percentile of
> these data as in this metrics can represent overall reregistration
> progress; In case of degradation towards to the end of reregistration,
> the high percentile can reflect it; In the case there are unreachable
> agents in the failover, if certain percentile recovery couldn't be
> reached, the intiail value of that percentile will not be updated.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 868787bb2f9d879531402f83507b322462322efc 
>   src/master/metrics.hpp e1da18e6ba2737f729e1e30653020538150ae898 
> 
> 
> Diff: https://reviews.apache.org/r/68706/diff/7/
> 
> 
> Testing
> ---
> 
> Automation:
> [ RUN  ] MasterTest.MetricsInMetricsEndpoint
> [   OK ] MasterTest.MetricsInMetricsEndpoint (42 ms)
> 
> Real world cases:
> 
> While the master is not elected or there is no agents recovered yet
> "master/recovered_agents_100_percent_reregistered_secs": 0.0,
> "master/recovered_agents_25_percent_reregistered_secs": 0.0,
> "master/recovered_agents_50_percent_reregistered_secs": 0.0,
> "master/recovered_agents_75_percent_reregistered_secs": 0.0,
> "master/recovered_agents_90_percent_reregistered_secs": 0.0,
> "master/recovered_agents_99_percent_reregistered_secs": 0.0,
> "master/slave_reregistrations": 0.0,
> 
> While reregistrations is in progress: 5 out of 6 completed:
> "master/recovered_agents_100_percent_reregistered_secs": 0.0,
> "master/recovered_agents_25_percent_reregistered_secs": 2.0,
> "master/recovered_agents_50_percent_reregistered_secs": 3.0,
> "master/recovered_agents_75_percent_reregistered_secs": 6.0,
> "master/recovered_agents_90_percent_reregistered_secs": 0.0,
> "master/recovered_agents_99_percent_reregistered_secs": 0.0,
> "master/slave_reregistrations": 5.0,
> 
> 
> While 6 reregistrations were all completed:
> "master/recovered_agents_100_percent_reregistered_secs": 22.0,
> "master/recovered_agents_25_percent_reregistered_secs": 2.0,
> "master/recovered_agents_50_percent_reregistered_secs": 3.0,
> "master/recovered_agents_75_percent_reregistered_secs": 6.0,
> "master/recovered_agents_90_percent_reregistered_secs": 22.0,
> "master/recovered_agents_99_percent_reregistered_secs": 22.0,
> "master/slave_reregistrations": 6.0,
> 
> 
> Thanks,
> 
> Xudong Ni
> 
>

Re: Review Request 68706: Added master failover reregistration progress metrics.

[Xudong Ni via Review Board]

> On Oct. 18, 2018, 6:14 p.m., Jiang Yan Xu wrote:
> > src/master/master.cpp
> > Lines 1874-1875 (patched)
> > 
> >
> > If we use equality operator and only set the timer when such a number 
> > of reregistered agent is reached we are guaranteed to only set each timer 
> > once (but we may need to set multiple timers in one call) right? This 
> > alleviates the need to check if the timer is already set!
> > 
> > This should also work with boundary cases like the total recovered 
> > agents count being 0 or 1 (overlapping percentiles) etc. Right?
> 
> Jiang Yan Xu wrote:
> We should comment on the reasons for dropping issues.

This is actaully fixed and marked as dropped somehow. We used equality operator 
to 0.0 to check whether the percentile was reached or not; The reason we used 
push gauge not timer is explained in push gauge vs timer comments section


> On Oct. 18, 2018, 6:14 p.m., Jiang Yan Xu wrote:
> > src/master/metrics.hpp
> > Lines 51 (patched)
> > 
> >
> > On using Timer (e.g., like 
> > [state_fetch](https://github.com/apache/mesos/blob/7f36ebc1775398a43b2aa3a81bb647fb296b8313/src/master/registrar.cpp#L172))
> >  vs. PushGauge, after looking at how it'll be used I think the main 
> > advantage of Timer is that it doesn't export any value if you haven't set 
> > it.
> > 
> > Consider the two cases:
> > 
> > 1. There are 1000 recovered agents and 0 have reregsitered, should all 
> > the timers have zero values or should they be absent?
> > 
> > 2. There are 0 recovered agents (e.g., brand new cluster), should all 
> > of the metrics be zero or non-existent? I feel like they should be zero, as 
> > in, e.g., 100% of all 0 agents are reregistered within 0 secs.
> > 
> > So timer handles this natrually. Also it sets the `_secs` name for you 
> > but that's a minor conveninence.
> 
> Jiang Yan Xu wrote:
> We should comment on the reasons for dropping issues.

Sorry about this, I did make the comments but it must be in one of draft which 
was not saved. 

I agree that metrics should be zero but not absent when certain percentige were 
not reached. I did tried both PushGauge and Timer implementation and tested in 
our clusters.

If we used the timer, when the value was not set, that metric is actually 
missing. PushGauge is set with the initial value 0.0 and we can tell whether 
it's set yet, the metric will always exist no matter that percentile reached or 
not, and it has better performance.

The brand new cluster case was tested and the results were included in the test 
results.


- Xudong


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


On Oct. 19, 2018, 11:56 p.m., Xudong Ni wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68706/
> ---
> 
> (Updated Oct. 19, 2018, 11:56 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, James Peach, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-9178
> https://issues.apache.org/jira/browse/MESOS-9178
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> During the master failover, the time that the master elected is
> considered as the start of failover. In the progress of
> reregistration, the percentile represents the time when such
> percentile of agents finished registration again; The percentile of
> these data as in this metrics can represent overall reregistration
> progress; In case of degradation towards to the end of reregistration,
> the high percentile can reflect it; In the case there are unreachable
> agents in the failover, if certain percentile recovery couldn't be
> reached, the intiail value of that percentile will not be updated.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 868787bb2f9d879531402f83507b322462322efc 
>   src/master/metrics.hpp e1da18e6ba2737f729e1e30653020538150ae898 
> 
> 
> Diff: https://reviews.apache.org/r/68706/diff/7/
> 
> 
> Testing
> ---
> 
> Automation:
> [ RUN  ] MasterTest.MetricsInMetricsEndpoint
> [   OK ] MasterTest.MetricsInMetricsEndpoint (42 ms)
> 
> Real world cases:
> 
> While the master is not elected or there is no agents recovered yet
> "master/recovered_agents_100_percent_reregistered_secs": 0.0,
> "master/recovered_agents_25_percent_reregistered_secs": 0.0,
> "master/recovered_agents_50_percent_reregistered_secs": 0.0,
> "master/recovered_agents_75_percent_reregistered_secs": 0.0,
> "master/recovered_agents_90_percent_reregistered_secs": 0.0,
> "master/recovered_agents_99_percent_reregistered_secs": 0.0,
> 

Re: Review Request 68706: Added master failover reregistration progress metrics.

[Xudong Ni via Review Board]

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

(Updated Oct. 19, 2018, 11:56 p.m.)


Review request for mesos, Benjamin Mahler, James Peach, and Jiang Yan Xu.


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


Repository: mesos


Description
---

During the master failover, the time that the master elected is
considered as the start of failover. In the progress of
reregistration, the percentile represents the time when such
percentile of agents finished registration again; The percentile of
these data as in this metrics can represent overall reregistration
progress; In case of degradation towards to the end of reregistration,
the high percentile can reflect it; In the case there are unreachable
agents in the failover, if certain percentile recovery couldn't be
reached, the intiail value of that percentile will not be updated.


Diffs (updated)
-

  src/master/master.cpp 868787bb2f9d879531402f83507b322462322efc 
  src/master/metrics.hpp e1da18e6ba2737f729e1e30653020538150ae898 


Diff: https://reviews.apache.org/r/68706/diff/7/

Changes: https://reviews.apache.org/r/68706/diff/6-7/


Testing (updated)
---

Automation:
[ RUN  ] MasterTest.MetricsInMetricsEndpoint
[   OK ] MasterTest.MetricsInMetricsEndpoint (42 ms)

Real world cases:

While the master is not elected or there is no agents recovered yet
"master/recovered_agents_100_percent_reregistered_secs": 0.0,
"master/recovered_agents_25_percent_reregistered_secs": 0.0,
"master/recovered_agents_50_percent_reregistered_secs": 0.0,
"master/recovered_agents_75_percent_reregistered_secs": 0.0,
"master/recovered_agents_90_percent_reregistered_secs": 0.0,
"master/recovered_agents_99_percent_reregistered_secs": 0.0,
"master/slave_reregistrations": 0.0,

While reregistrations is in progress: 5 out of 6 completed:
"master/recovered_agents_100_percent_reregistered_secs": 0.0,
"master/recovered_agents_25_percent_reregistered_secs": 2.0,
"master/recovered_agents_50_percent_reregistered_secs": 3.0,
"master/recovered_agents_75_percent_reregistered_secs": 6.0,
"master/recovered_agents_90_percent_reregistered_secs": 0.0,
"master/recovered_agents_99_percent_reregistered_secs": 0.0,
"master/slave_reregistrations": 5.0,


While 6 reregistrations were all completed:
"master/recovered_agents_100_percent_reregistered_secs": 22.0,
"master/recovered_agents_25_percent_reregistered_secs": 2.0,
"master/recovered_agents_50_percent_reregistered_secs": 3.0,
"master/recovered_agents_75_percent_reregistered_secs": 6.0,
"master/recovered_agents_90_percent_reregistered_secs": 22.0,
"master/recovered_agents_99_percent_reregistered_secs": 22.0,
"master/slave_reregistrations": 6.0,


Thanks,

Xudong Ni

Re: Review Request 68706: Added master failover reregistration progress metrics.

[Xudong Ni via Review Board]

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

(Updated Oct. 19, 2018, 11:14 p.m.)


Review request for mesos, Benjamin Mahler, James Peach, and Jiang Yan Xu.


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


Repository: mesos


Description
---

During the master failover, the time that the master elected is
considered as the start of failover. In the progress of
reregistration, the percentile represents the time when such
percentile of agents finished registration again; The percentile of
these data as in this metrics can represent overall reregistration
progress; In case of degradation towards to the end of reregistration,
the high percentile can reflect it; In the case there are unreachable
agents in the failover, if certain percentile recovery couldn't be
reached, the intiail value of that percentile will not be updated.


Diffs (updated)
-

  src/master/master.cpp 868787bb2f9d879531402f83507b322462322efc 
  src/master/metrics.hpp e1da18e6ba2737f729e1e30653020538150ae898 


Diff: https://reviews.apache.org/r/68706/diff/6/

Changes: https://reviews.apache.org/r/68706/diff/5-6/


Testing (updated)
---

Automation:
[ RUN  ] MasterTest.MetricsInMetricsEndpoint
[   OK ] MasterTest.MetricsInMetricsEndpoint (42 ms)

Real world cases:

While the master is not elected or there is no agents to recover
"master/recovered_agents_100_percent_reregistered_secs": 0.0,
"master/recovered_agents_25_percent_reregistered_secs": 0.0,
"master/recovered_agents_50_percent_reregistered_secs": 0.0,
"master/recovered_agents_75_percent_reregistered_secs": 0.0,
"master/recovered_agents_90_percent_reregistered_secs": 0.0,
"master/recovered_agents_99_percent_reregistered_secs": 0.0,


While reregistrations is in progress: 4 out of 6 completed:
"master/recovered_agents_100_percent_reregistered_secs": 0.0,
"master/recovered_agents_25_percent_reregistered_secs": 1.0,
"master/recovered_agents_50_percent_reregistered_secs": 8.0,
"master/recovered_agents_75_percent_reregistered_secs": 16.0,
"master/recovered_agents_90_percent_reregistered_secs": 0.0,
"master/recovered_agents_99_percent_reregistered_secs": 0.0,
"master/slave_reregistrations": 4.0,


While 6 reregistrations were all completed:
"master/recovered_agents_100_percent_reregistered_secs": 39.0,
"master/recovered_agents_25_percent_reregistered_secs": 1.0,
"master/recovered_agents_50_percent_reregistered_secs": 8.0,
"master/recovered_agents_75_percent_reregistered_secs": 16.0,
"master/recovered_agents_90_percent_reregistered_secs": 31.0,
"master/recovered_agents_99_percent_reregistered_secs": 31.0,
"master/slave_reregistrations": 6.0,


Thanks,

Xudong Ni

Re: Review Request 68706: Added master failover reregistration progress metrics.

[Xudong Ni via Review Board]

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

(Updated Oct. 16, 2018, 4:49 p.m.)


Review request for mesos, Benjamin Mahler, James Peach, and Jiang Yan Xu.


Changes
---

Sync master


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


Repository: mesos


Description
---

During the master failover, the time that the master elected is
considered as the start of failover. In the progress of
reregistration, the percentile represents the time when such
percentile of agents finished registration again; The percentile of
these data as in this metrics can represent overall reregistration
progress; In case of degradation towards to the end of reregistration,
the high percentile can reflect it; In the case there are unreachable
agents in the failover, if certain percentile recovery couldn't be
reached, the intiail value of that percentile will not be updated.


Diffs (updated)
-

  docs/monitoring.md 00c6ea94bcb73746aef740236632ede123f5b534 
  src/master/master.hpp ea7e9242b62fe6c2cc0e717f9a9f2f0c1cc0a390 
  src/master/master.cpp 868787bb2f9d879531402f83507b322462322efc 
  src/master/metrics.hpp e1da18e6ba2737f729e1e30653020538150ae898 
  src/master/metrics.cpp 56a7eef2d279ad3248092d37d19013d3ac110757 
  src/tests/master_tests.cpp 1db8ed7d81acbcd8bad4b7ca77c501d1d99cc135 


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

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


Testing
---

Automation:
[ RUN  ] MasterTest.MetricsInMetricsEndpoint
[   OK ] MasterTest.MetricsInMetricsEndpoint (42 ms)

Real world cases:
While reregistrations is in progress: 3277 out of 3582 completed:
"master/slave_reregistrations": 3277.0,
"master/slaves_100_percent_reregistered_secs": 0.0,
"master/slaves_25_percent_reregistered_secs": 5.0,
"master/slaves_50_percent_reregistered_secs": 11.0,
"master/slaves_75_percent_reregistered_secs": 20.0,
"master/slaves_90_percent_reregistered_secs": 30.0,
"master/slaves_99_percent_reregistered_secs": 0.0,


While 3582 reregistrations were all completed:
"master/slave_reregistrations": 3582.0,
"master/slaves_100_percent_reregistered_secs": 54.0,
"master/slaves_25_percent_reregistered_secs": 5.0,
"master/slaves_50_percent_reregistered_secs": 11.0,
"master/slaves_75_percent_reregistered_secs": 20.0,
"master/slaves_90_percent_reregistered_secs": 30.0,
"master/slaves_99_percent_reregistered_secs": 39.0,


Thanks,

Xudong Ni

Re: Review Request 68706: Added master failover reregistration progress metrics.

[Xudong Ni via Review Board]

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

(Updated Oct. 16, 2018, 4:19 a.m.)


Review request for mesos, Benjamin Mahler, James Peach, and Jiang Yan Xu.


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


Repository: mesos


Description
---

During the master failover, the time that the master elected is
considered as the start of failover. In the progress of
reregistration, the percentile represents the time when such
percentile of agents finished registration again; The percentile of
these data as in this metrics can represent overall reregistration
progress; In case of degradation towards to the end of reregistration,
the high percentile can reflect it; In the case there are unreachable
agents in the failover, if certain percentile recovery couldn't be
reached, the intiail value of that percentile will not be updated.


Diffs (updated)
-

  docs/monitoring.md 00c6ea94bcb73746aef740236632ede123f5b534 
  src/master/master.hpp ea7e9242b62fe6c2cc0e717f9a9f2f0c1cc0a390 
  src/master/master.cpp 06d769aeba16586a020729d454f4d00688b78c78 
  src/master/metrics.hpp e1da18e6ba2737f729e1e30653020538150ae898 
  src/master/metrics.cpp 56a7eef2d279ad3248092d37d19013d3ac110757 
  src/tests/master_tests.cpp 1db8ed7d81acbcd8bad4b7ca77c501d1d99cc135 


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

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


Testing (updated)
---

Automation:
[ RUN  ] MasterTest.MetricsInMetricsEndpoint
[   OK ] MasterTest.MetricsInMetricsEndpoint (42 ms)

Real world cases:
While reregistrations is in progress: 3277 out of 3582 completed:
"master/slave_reregistrations": 3277.0,
"master/slaves_100_percent_reregistered_secs": 0.0,
"master/slaves_25_percent_reregistered_secs": 5.0,
"master/slaves_50_percent_reregistered_secs": 11.0,
"master/slaves_75_percent_reregistered_secs": 20.0,
"master/slaves_90_percent_reregistered_secs": 30.0,
"master/slaves_99_percent_reregistered_secs": 0.0,


While 3582 reregistrations were all completed:
"master/slave_reregistrations": 3582.0,
"master/slaves_100_percent_reregistered_secs": 54.0,
"master/slaves_25_percent_reregistered_secs": 5.0,
"master/slaves_50_percent_reregistered_secs": 11.0,
"master/slaves_75_percent_reregistered_secs": 20.0,
"master/slaves_90_percent_reregistered_secs": 30.0,
"master/slaves_99_percent_reregistered_secs": 39.0,


Thanks,

Xudong Ni

Re: Review Request 68706: Added master failover reregistration progress metrics.

[Xudong Ni via Review Board]

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

(Updated Oct. 10, 2018, 5:22 p.m.)


Review request for mesos, Benjamin Mahler, James Peach, and Jiang Yan Xu.


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


Repository: mesos


Description
---

During the master failover, the time that the master elected is
considered as the start of failover. In the progress of
reregistration, the percentile represents the time when such
percentile of agents finished registration again; The percentile of
these data as in this metrics can represent overall reregistration
progress; In case of degradation towards to the end of reregistration,
the high percentile can reflect it; In the case there are unreachable
agents in the failover, if certain percentile recovery couldn't be
reached, the intiail value of that percentile will not be updated.


Diffs
-

  docs/monitoring.md 00c6ea94bcb73746aef740236632ede123f5b534 
  src/master/master.hpp ea7e9242b62fe6c2cc0e717f9a9f2f0c1cc0a390 
  src/master/master.cpp 06d769aeba16586a020729d454f4d00688b78c78 
  src/master/metrics.hpp e1da18e6ba2737f729e1e30653020538150ae898 
  src/master/metrics.cpp 56a7eef2d279ad3248092d37d19013d3ac110757 


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


Testing (updated)
---

Tested in mmaster with 6 reregistration agents:
"master/slave_reregistrations": 6,

In the middle of reregistration process:
"master/slaves_100_percent_reregistered_secs": 0,
"master/slaves_25_percent_reregistered_secs": 2.244662016,
"master/slaves_50_percent_reregistered_secs": 3.599491072,
"master/slaves_75_percent_reregistered_secs": 9.53919616,
"master/slaves_90_percent_reregistered_secs": 0,
"master/slaves_99_percent_reregistered_secs": 0,

When all registrations finished:
"master/slaves_100_percent_reregistered_secs": 29.697210112,
"master/slaves_25_percent_reregistered_secs": 2.244662016,
"master/slaves_50_percent_reregistered_secs": 3.599491072,
"master/slaves_75_percent_reregistered_secs": 9.53919616,
"master/slaves_90_percent_reregistered_secs": 29.697210112,
"master/slaves_99_percent_reregistered_secs": 29.697210112,

With 3606 agents, the last 1% take significant time
"master/slave_reregistrations": 3606,
"master/slave_shutdowns_canceled": 0,
"master/slave_shutdowns_completed": 0,
"master/slave_shutdowns_scheduled": 0,
"master/slave_unreachable_canceled": 0,
"master/slave_unreachable_completed": 0,
"master/slave_unreachable_scheduled": 0,
"master/slaves_100_percent_reregistered_secs": 58.585202944,
"master/slaves_25_percent_reregistered_secs": 9.966434048,
"master/slaves_50_percent_reregistered_secs": 20.259571968,
"master/slaves_75_percent_reregistered_secs": 30.598885888,
"master/slaves_90_percent_reregistered_secs": 36.396082944,
"master/slaves_99_percent_reregistered_secs": 39.811022848,


Thanks,

Xudong Ni

Re: Review Request 68706: Added master failover reregistration progress metrics.

[Xudong Ni via Review Board]

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

(Updated Oct. 9, 2018, 11:24 p.m.)


Review request for mesos, Benjamin Mahler, James Peach, and Jiang Yan Xu.


Changes
---

fixed the style


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


Repository: mesos


Description
---

During the master failover, the time that the master elected is
considered as the start of failover. In the progress of
reregistration, the percentile represents the time when such
percentile of agents finished registration again; The percentile of
these data as in this metrics can represent overall reregistration
progress; In case of degradation towards to the end of reregistration,
the high percentile can reflect it; In the case there are unreachable
agents in the failover, if certain percentile recovery couldn't be
reached, the intiail value of that percentile will not be updated.


Diffs (updated)
-

  docs/monitoring.md 00c6ea94bcb73746aef740236632ede123f5b534 
  src/master/master.hpp ea7e9242b62fe6c2cc0e717f9a9f2f0c1cc0a390 
  src/master/master.cpp 06d769aeba16586a020729d454f4d00688b78c78 
  src/master/metrics.hpp e1da18e6ba2737f729e1e30653020538150ae898 
  src/master/metrics.cpp 56a7eef2d279ad3248092d37d19013d3ac110757 


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

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


Testing
---

Tested in mmaster with 6 reregistration agents:
"master/slave_reregistrations": 6,

In the middle of reregistration process:
"master/slaves_100_percent_reregistered_secs": 0,
"master/slaves_25_percent_reregistered_secs": 2.244662016,
"master/slaves_50_percent_reregistered_secs": 3.599491072,
"master/slaves_75_percent_reregistered_secs": 9.53919616,
"master/slaves_90_percent_reregistered_secs": 0,
"master/slaves_99_percent_reregistered_secs": 0,

When all registrations finished:
"master/slaves_100_percent_reregistered_secs": 29.697210112,
"master/slaves_25_percent_reregistered_secs": 2.244662016,
"master/slaves_50_percent_reregistered_secs": 3.599491072,
"master/slaves_75_percent_reregistered_secs": 9.53919616,
"master/slaves_90_percent_reregistered_secs": 29.697210112,
"master/slaves_99_percent_reregistered_secs": 29.697210112,


Thanks,

Xudong Ni

Re: Review Request 68706: Added master failover reregistration progress metrics.

[Xudong Ni via Review Board]

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

(Updated Oct. 9, 2018, 8:45 p.m.)


Review request for mesos, Benjamin Mahler, James Peach, and Jiang Yan Xu.


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


Repository: mesos


Description (updated)
---

During the master failover, the time that the master elected is
considered as the start of failover. In the progress of
reregistration, the percentile represents the time when such
percentile of agents finished registration again; The percentile of
these data as in this metrics can represent overall reregistration
progress; In case of degradation towards to the end of reregistration,
the high percentile can reflect it; In the case there are unreachable
agents in the failover, if certain percentile recovery couldn't be
reached, the intiail value of that percentile will not be updated.


Diffs (updated)
-

  docs/monitoring.md 00c6ea94bcb73746aef740236632ede123f5b534 
  src/master/master.hpp ea7e9242b62fe6c2cc0e717f9a9f2f0c1cc0a390 
  src/master/master.cpp 06d769aeba16586a020729d454f4d00688b78c78 
  src/master/metrics.hpp e1da18e6ba2737f729e1e30653020538150ae898 
  src/master/metrics.cpp 56a7eef2d279ad3248092d37d19013d3ac110757 


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

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


Testing (updated)
---

Tested in mmaster with 6 reregistration agents:
"master/slave_reregistrations": 6,

In the middle of reregistration process:
"master/slaves_100_percent_reregistered_secs": 0,
"master/slaves_25_percent_reregistered_secs": 2.244662016,
"master/slaves_50_percent_reregistered_secs": 3.599491072,
"master/slaves_75_percent_reregistered_secs": 9.53919616,
"master/slaves_90_percent_reregistered_secs": 0,
"master/slaves_99_percent_reregistered_secs": 0,

When all registrations finished:
"master/slaves_100_percent_reregistered_secs": 29.697210112,
"master/slaves_25_percent_reregistered_secs": 2.244662016,
"master/slaves_50_percent_reregistered_secs": 3.599491072,
"master/slaves_75_percent_reregistered_secs": 9.53919616,
"master/slaves_90_percent_reregistered_secs": 29.697210112,
"master/slaves_99_percent_reregistered_secs": 29.697210112,


Thanks,

Xudong Ni

Re: Review Request 68706: Added master failover reregistration progress metrics.

[Xudong Ni via Review Board]

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

(Updated Sept. 12, 2018, 9:42 p.m.)


Review request for mesos, Benjamin Mahler, James Peach, and Jiang Yan Xu.


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


Repository: mesos


Description
---

When an agent is reregistrated, the time delta from that moment to
the master elected time was saved; In the progress of reregistration,
each data entry represents the registration time delta from master
elected time; The percentile of these data as in this metrics can
represent overall reregistration progress; In case of degradation
towards to the end of reregistration, the high percentile will
reflect it.

Note: These metrics only represent the completed reregistration; It
does not monitor agents were finally marked as unreachable that the
reregistration didn't actually happen, the unreachable agents were
already monitored by existing metrics.


Diffs
-

  docs/monitoring.md 00c6ea94bcb73746aef740236632ede123f5b534 
  src/master/master.cpp 06d769aeba16586a020729d454f4d00688b78c78 
  src/master/metrics.hpp e1da18e6ba2737f729e1e30653020538150ae898 
  src/master/metrics.cpp 56a7eef2d279ad3248092d37d19013d3ac110757 


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


Testing (updated)
---

Tested in mmaster with seven reregistration agents:
"master/slaves_reregistration_secs":32.321583104,
"master/slaves_reregistration_secs/count":7,
"master/slaves_reregistration_secs/max":32.321583104,
"master/slaves_reregistration_secs/min":3.35373696,
"master/slaves_reregistration_secs/p50":8.774915072,
"master/slaves_reregistration_secs/p90":30.8676036608,
"master/slaves_reregistration_secs/p95":31.59459338236,
"master/slaves_reregistration_secs/p99":32.176185159679996,
"master/slaves_reregistration_secs/p999":32.307043309567995,
"master/slaves_reregistration_secs/p":32.3201291245568,


Thanks,

Xudong Ni

Review Request 68706: Added master failover reregistration progress metrics.

[Xudong Ni via Review Board]

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

Review request for mesos.


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


Repository: mesos


Description
---

When an agent is reregistrated, the time delta from that moment to
the master elected time was saved; In the progress of reregistration,
each data entry represents the registration time delta from master
elected time; The percentile of these data as in this metrics can
represent overall reregistration progress; In case of degradation
towards to the end of reregistration, the high percentile will
reflect it.

Note: These metrics only represent the completed reregistration; It
does not monitor agents were finally marked as unreachable that the
reregistration didn't actually happen, the unreachable agents were
already monitored by existing metrics.


Diffs
-

  docs/monitoring.md 00c6ea94bcb73746aef740236632ede123f5b534 
  src/master/master.cpp 06d769aeba16586a020729d454f4d00688b78c78 
  src/master/metrics.hpp e1da18e6ba2737f729e1e30653020538150ae898 
  src/master/metrics.cpp 56a7eef2d279ad3248092d37d19013d3ac110757 


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


Testing
---


Thanks,

Xudong Ni

Re: Review Request 68366: Added a custom port range option to the `network/ports` isolator.

[Xudong Ni via Review Board]

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

(Updated Aug. 24, 2018, 10:54 p.m.)


Review request for mesos and James Peach.


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


Repository: mesos


Description
---

Added the `--container_ports_isolated_range` flag to the
`network/ports` isolator. This allows the operator to specify a custom
port range to be protected by the isolator. If a task listens on a port
that it isn't holding resources for, the isolator will not raise a
limitation unless the port is within this range. We can represent the
`--check_agent_port_range_only` as a special case of a protected range.


Diffs (updated)
-

  docs/configuration/agent.md e98a9786aa2d1f5c87aec4db8b65457c3293156e 
  docs/isolators/network-ports.md 5d14fc2985e099783b09e2a19f99641b4ddbd768 
  src/slave/containerizer/mesos/isolators/network/ports.hpp 
6944d01e0f8a11eda381ef1754f19ee0cf9359c8 
  src/slave/containerizer/mesos/isolators/network/ports.cpp 
2a7ff2530f898cf892739c715b07b3387b423ed9 
  src/slave/flags.hpp bff194fef98f38a8b91d86ef4ec99889d0cfe31f 
  src/slave/flags.cpp e017f3921a0bccc03f6ef639a04163bf7fc4e79b 
  src/tests/containerizer/ports_isolator_tests.cpp 
db080c4e9c8b0c036294a8f7a42617ca1231f884 


Diff: https://reviews.apache.org/r/68366/diff/9/

Changes: https://reviews.apache.org/r/68366/diff/8-9/


Testing (updated)
---

sudo GLOG_v=1 ./bin/mesos-tests.sh --verbose 
--gtest_filter="NetworkPortsIsolatorTest.*"

[--] 13 tests from NetworkPortsIsolatorTest (26387 ms total)

[--] Global test environment tear-down
[==] 13 tests from 1 test case ran. (26399 ms total)
[  PASSED  ] 13 tests.


Thanks,

Xudong Ni

Re: Review Request 68366: Added a custom port range option to the `network/ports` isolator.

[Xudong Ni via Review Board]

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

(Updated Aug. 23, 2018, 10:16 p.m.)


Review request for mesos and James Peach.


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


Repository: mesos


Description
---

Added the `--container_ports_isolated_range` flag to the
`network/ports` isolator. This allows the operator to specify a custom
port range to be protected by the isolator. If a task listens on a port
that it isn't holding resources for, the isolator will not raise a
limitation unless the port is within this range. We can represent the
`--check_agent_port_range_only` as a special case of a protected range.


Diffs (updated)
-

  docs/configuration/agent.md e98a9786aa2d1f5c87aec4db8b65457c3293156e 
  docs/isolators/network-ports.md 5d14fc2985e099783b09e2a19f99641b4ddbd768 
  src/slave/containerizer/mesos/isolators/network/ports.hpp 
6944d01e0f8a11eda381ef1754f19ee0cf9359c8 
  src/slave/containerizer/mesos/isolators/network/ports.cpp 
2a7ff2530f898cf892739c715b07b3387b423ed9 
  src/slave/flags.hpp bff194fef98f38a8b91d86ef4ec99889d0cfe31f 
  src/slave/flags.cpp e017f3921a0bccc03f6ef639a04163bf7fc4e79b 
  src/tests/containerizer/ports_isolator_tests.cpp 
db080c4e9c8b0c036294a8f7a42617ca1231f884 


Diff: https://reviews.apache.org/r/68366/diff/8/

Changes: https://reviews.apache.org/r/68366/diff/7-8/


Testing (updated)
---

New test added to test feature:

[   OK ] NetworkPortsIsolatorTest.ROOT_NC_PortEnforcementIsolatedPort (1794 
ms)
[--] 1 test from NetworkPortsIsolatorTest (1794 ms total)

[--] Global test environment tear-down
[==] 1 test from 1 test case ran. (1806 ms total)
[  PASSED  ] 1 test.

Existing test updated to test the negative cases:

[   OK ] NetworkPortsIsolatorTest.ROOT_IsolatorFlags (78 ms)
[--] 1 test from NetworkPortsIsolatorTest (78 ms total)

[--] Global test environment tear-down
[==] 1 test from 1 test case ran. (91 ms total)
[  PASSED  ] 1 test

Existing test for isolator feature:

[   OK ] NetworkPortsIsolatorTest.ROOT_NC_NoPortEnforcement (1798 ms)
[--] 1 test from NetworkPortsIsolatorTest (1798 ms total)

[--] Global test environment tear-down
[==] 1 test from 1 test case ran. (1810 ms total)
[  PASSED  ] 1 test.

[   OK ] NetworkPortsIsolatorTest.ROOT_NC_NoPortsResource (1772 ms)
[--] 1 test from NetworkPortsIsolatorTest (1773 ms total)

[--] Global test environment tear-down
[==] 1 test from 1 test case ran. (1784 ms total)
[  PASSED  ] 1 test.

[   OK ] NetworkPortsIsolatorTest.ROOT_NC_AllocatedPorts (1827 ms)
[--] 1 test from NetworkPortsIsolatorTest (1828 ms total)

[--] Global test environment tear-down
[==] 1 test from 1 test case ran. (1841 ms total)
[  PASSED  ] 1 test.


Thanks,

Xudong Ni

Re: Review Request 68366: Added a custom port range option to the `network/ports` isolator.

[Xudong Ni via Review Board]

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

(Updated Aug. 23, 2018, 2:56 a.m.)


Review request for mesos and James Peach.


Summary (updated)
-

Added a custom port range option to the `network/ports` isolator.


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


Repository: mesos


Description (updated)
---

Added the `--container_ports_isolated_range` flag to the
`network/ports` isolator. This allows the operator to specify a custom
port range to be protected by the isolator. If a task listens on a port
that it isn't holding resources for, the isolator will not raise a
limitation unless the port is within this range. We can represent the
`--check_agent_port_range_only` as a special case of a protected range.


Diffs (updated)
-

  docs/configuration/agent.md e98a9786aa2d1f5c87aec4db8b65457c3293156e 
  docs/isolators/network-ports.md 5d14fc2985e099783b09e2a19f99641b4ddbd768 
  src/slave/containerizer/mesos/isolators/network/ports.hpp 
6944d01e0f8a11eda381ef1754f19ee0cf9359c8 
  src/slave/containerizer/mesos/isolators/network/ports.cpp 
2a7ff2530f898cf892739c715b07b3387b423ed9 
  src/slave/flags.hpp bff194fef98f38a8b91d86ef4ec99889d0cfe31f 
  src/slave/flags.cpp e017f3921a0bccc03f6ef639a04163bf7fc4e79b 
  src/tests/containerizer/ports_isolator_tests.cpp 
db080c4e9c8b0c036294a8f7a42617ca1231f884 


Diff: https://reviews.apache.org/r/68366/diff/7/

Changes: https://reviews.apache.org/r/68366/diff/6-7/


Testing (updated)
---

New test added to test feature:

[   OK ] NetworkPortsIsolatorTest.ROOT_NC_PortEnforcementIsolatedPort (1678 
ms)
[--] 1 test from NetworkPortsIsolatorTest (1679 ms total)

[--] Global test environment tear-down
[==] 1 test from 1 test case ran. (1691 ms total)
[  PASSED  ] 1 test.

Existing test updated to test the negative cases:

[   OK ] NetworkPortsIsolatorTest.ROOT_IsolatorFlags (73 ms)
[--] 1 test from NetworkPortsIsolatorTest (73 ms total)

[--] Global test environment tear-down
[==] 1 test from 1 test case ran. (85 ms total)
[  PASSED  ] 1 test.

Existing test for isolator feature:

[   OK ] NetworkPortsIsolatorTest.ROOT_NC_NoPortEnforcement (1789 ms)
[--] 1 test from NetworkPortsIsolatorTest (1790 ms total)

[--] Global test environment tear-down
[==] 1 test from 1 test case ran. (1801 ms total)
[  PASSED  ] 1 test.

[   OK ] NetworkPortsIsolatorTest.ROOT_NC_NoPortsResource (1795 ms)
[--] 1 test from NetworkPortsIsolatorTest (1795 ms total)

[--] Global test environment tear-down
[==] 1 test from 1 test case ran. (1806 ms total)
[  PASSED  ] 1 test.

[   OK ] NetworkPortsIsolatorTest.ROOT_NC_AllocatedPorts (1782 ms)
[--] 1 test from NetworkPortsIsolatorTest (1783 ms total)

[--] Global test environment tear-down
[==] 1 test from 1 test case ran. (1794 ms total)
[  PASSED  ] 1 test.


Thanks,

Xudong Ni

Re: Review Request 68366: Added agent protected port range option in network isolator.

[Xudong Ni via Review Board]

> On Aug. 22, 2018, 7:27 p.m., James Peach wrote:
> > Can you please update the commit comment to better describe the specific 
> > changes?
> > 
> > Maybe something along these lines:
> > 
> > ```
> > Added a custom port range option to the `network/ports` isolator.
> > 
> > Added the `--foo-bar` flag to the `network/ports` isolator. This allows
> > the operator to specify a custom port range to be protected by the 
> > isolator. If a task
> > listens on a port that it isn't holding resources for, the isolator will
> > not raise a limitation unless the port is within this range. We can
> > represent the `--check_agent_port_range_only` as a special case of a
> > protected range.
> > 
> > etc ...
> > ```

commit comment is updated


- Xudong


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


On Aug. 22, 2018, 5:35 p.m., Xudong Ni wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68366/
> ---
> 
> (Updated Aug. 22, 2018, 5:35 p.m.)
> 
> 
> Review request for mesos and James Peach.
> 
> 
> Bugs: MESOS-9133
> https://issues.apache.org/jira/browse/MESOS-9133
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> For a network isolator disabled environment, in practice, there could
> be a lot of users already binding to ephemeral ports; It would take
> a lot of efforts to find/notify/modify those apps; In order to take
> advantage of network isolator and enable it in such system, it would
> be useful to add mesos-agent configuration option to allow enforce
> port isolation in only the specified certain port range
> 
> 
> Diffs
> -
> 
>   docs/configuration/agent.md e98a9786aa2d1f5c87aec4db8b65457c3293156e 
>   docs/isolators/network-ports.md 5d14fc2985e099783b09e2a19f99641b4ddbd768 
>   src/slave/containerizer/mesos/isolators/network/ports.hpp 
> 6944d01e0f8a11eda381ef1754f19ee0cf9359c8 
>   src/slave/containerizer/mesos/isolators/network/ports.cpp 
> 2a7ff2530f898cf892739c715b07b3387b423ed9 
>   src/slave/flags.hpp bff194fef98f38a8b91d86ef4ec99889d0cfe31f 
>   src/slave/flags.cpp e017f3921a0bccc03f6ef639a04163bf7fc4e79b 
>   src/tests/containerizer/ports_isolator_tests.cpp 
> db080c4e9c8b0c036294a8f7a42617ca1231f884 
> 
> 
> Diff: https://reviews.apache.org/r/68366/diff/6/
> 
> 
> Testing
> ---
> 
> New test added to test feature:
> 
> [   OK ] NetworkPortsIsolatorTest.ROOT_NC_PortEnforcementProtectedPort 
> (1886 ms)
> [--] 1 test from NetworkPortsIsolatorTest (1887 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (1900 ms total)
> [  PASSED  ] 1 test.
> 
> Existing test updated to test the negative cases:
> 
> [   OK ] NetworkPortsIsolatorTest.ROOT_IsolatorFlags (58 ms)
> [--] 1 test from NetworkPortsIsolatorTest (58 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (69 ms total)
> [  PASSED  ] 1 test.
> 
> Existing test for isolator feature:
> 
> [   OK ] NetworkPortsIsolatorTest.ROOT_NC_AllocatedPorts (1992 ms)
> [--] 1 test from NetworkPortsIsolatorTest (1993 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (2004 ms total)
> [  PASSED  ] 1 test.
> 
> 
> Thanks,
> 
> Xudong Ni
> 
>

Re: Review Request 68366: Added agent protected port range option in network isolator.

[Xudong Ni via Review Board]

> On Aug. 22, 2018, 11:50 p.m., James Peach wrote:
> > I think this change broke the 
> > `NetworkPortsIsolatorTest.ROOT_NC_NoPortsResource` test, just regressing 
> > against master.

Fixed it; There was an empty ports condition removed in one of iteration but 
didn't add back in the later iterations


- Xudong


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


On Aug. 22, 2018, 5:35 p.m., Xudong Ni wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68366/
> ---
> 
> (Updated Aug. 22, 2018, 5:35 p.m.)
> 
> 
> Review request for mesos and James Peach.
> 
> 
> Bugs: MESOS-9133
> https://issues.apache.org/jira/browse/MESOS-9133
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> For a network isolator disabled environment, in practice, there could
> be a lot of users already binding to ephemeral ports; It would take
> a lot of efforts to find/notify/modify those apps; In order to take
> advantage of network isolator and enable it in such system, it would
> be useful to add mesos-agent configuration option to allow enforce
> port isolation in only the specified certain port range
> 
> 
> Diffs
> -
> 
>   docs/configuration/agent.md e98a9786aa2d1f5c87aec4db8b65457c3293156e 
>   docs/isolators/network-ports.md 5d14fc2985e099783b09e2a19f99641b4ddbd768 
>   src/slave/containerizer/mesos/isolators/network/ports.hpp 
> 6944d01e0f8a11eda381ef1754f19ee0cf9359c8 
>   src/slave/containerizer/mesos/isolators/network/ports.cpp 
> 2a7ff2530f898cf892739c715b07b3387b423ed9 
>   src/slave/flags.hpp bff194fef98f38a8b91d86ef4ec99889d0cfe31f 
>   src/slave/flags.cpp e017f3921a0bccc03f6ef639a04163bf7fc4e79b 
>   src/tests/containerizer/ports_isolator_tests.cpp 
> db080c4e9c8b0c036294a8f7a42617ca1231f884 
> 
> 
> Diff: https://reviews.apache.org/r/68366/diff/6/
> 
> 
> Testing
> ---
> 
> New test added to test feature:
> 
> [   OK ] NetworkPortsIsolatorTest.ROOT_NC_PortEnforcementProtectedPort 
> (1886 ms)
> [--] 1 test from NetworkPortsIsolatorTest (1887 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (1900 ms total)
> [  PASSED  ] 1 test.
> 
> Existing test updated to test the negative cases:
> 
> [   OK ] NetworkPortsIsolatorTest.ROOT_IsolatorFlags (58 ms)
> [--] 1 test from NetworkPortsIsolatorTest (58 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (69 ms total)
> [  PASSED  ] 1 test.
> 
> Existing test for isolator feature:
> 
> [   OK ] NetworkPortsIsolatorTest.ROOT_NC_AllocatedPorts (1992 ms)
> [--] 1 test from NetworkPortsIsolatorTest (1993 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (2004 ms total)
> [  PASSED  ] 1 test.
> 
> 
> Thanks,
> 
> Xudong Ni
> 
>

Re: Review Request 68366: Added agent protected port range option in network isolator.

[Xudong Ni via Review Board]

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

(Updated Aug. 22, 2018, 5:35 p.m.)


Review request for mesos and James Peach.


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


Repository: mesos


Description
---

For a network isolator disabled environment, in practice, there could
be a lot of users already binding to ephemeral ports; It would take
a lot of efforts to find/notify/modify those apps; In order to take
advantage of network isolator and enable it in such system, it would
be useful to add mesos-agent configuration option to allow enforce
port isolation in only the specified certain port range


Diffs (updated)
-

  docs/configuration/agent.md e98a9786aa2d1f5c87aec4db8b65457c3293156e 
  docs/isolators/network-ports.md 5d14fc2985e099783b09e2a19f99641b4ddbd768 
  src/slave/containerizer/mesos/isolators/network/ports.hpp 
6944d01e0f8a11eda381ef1754f19ee0cf9359c8 
  src/slave/containerizer/mesos/isolators/network/ports.cpp 
2a7ff2530f898cf892739c715b07b3387b423ed9 
  src/slave/flags.hpp bff194fef98f38a8b91d86ef4ec99889d0cfe31f 
  src/slave/flags.cpp e017f3921a0bccc03f6ef639a04163bf7fc4e79b 
  src/tests/containerizer/ports_isolator_tests.cpp 
db080c4e9c8b0c036294a8f7a42617ca1231f884 


Diff: https://reviews.apache.org/r/68366/diff/6/

Changes: https://reviews.apache.org/r/68366/diff/5-6/


Testing
---

New test added to test feature:

[   OK ] NetworkPortsIsolatorTest.ROOT_NC_PortEnforcementProtectedPort 
(1886 ms)
[--] 1 test from NetworkPortsIsolatorTest (1887 ms total)

[--] Global test environment tear-down
[==] 1 test from 1 test case ran. (1900 ms total)
[  PASSED  ] 1 test.

Existing test updated to test the negative cases:

[   OK ] NetworkPortsIsolatorTest.ROOT_IsolatorFlags (58 ms)
[--] 1 test from NetworkPortsIsolatorTest (58 ms total)

[--] Global test environment tear-down
[==] 1 test from 1 test case ran. (69 ms total)
[  PASSED  ] 1 test.

Existing test for isolator feature:

[   OK ] NetworkPortsIsolatorTest.ROOT_NC_AllocatedPorts (1992 ms)
[--] 1 test from NetworkPortsIsolatorTest (1993 ms total)

[--] Global test environment tear-down
[==] 1 test from 1 test case ran. (2004 ms total)
[  PASSED  ] 1 test.


Thanks,

Xudong Ni

Re: Review Request 68366: Added agent protected port range option in network isolator.

[Xudong Ni via Review Board]

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

(Updated Aug. 20, 2018, 9:53 p.m.)


Review request for mesos and James Peach.


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


Repository: mesos


Description
---

For a network isolator disabled environment, in practice, there could
be a lot of users already binding to ephemeral ports; It would take
a lot of efforts to find/notify/modify those apps; In order to take
advantage of network isolator and enable it in such system, it would
be useful to add mesos-agent configuration option to allow enforce
port isolation in only the specified certain port range


Diffs (updated)
-

  docs/configuration/agent.md 4e50b681bb956d559da6bf1d2c504099aae3cafb 
  docs/isolators/network-ports.md 5d14fc2985e099783b09e2a19f99641b4ddbd768 
  src/slave/containerizer/mesos/isolators/network/ports.hpp 
6944d01e0f8a11eda381ef1754f19ee0cf9359c8 
  src/slave/containerizer/mesos/isolators/network/ports.cpp 
2a7ff2530f898cf892739c715b07b3387b423ed9 
  src/slave/flags.hpp 88c35da5fd754abbd4bd316e1fa9efa4a70a6b8c 
  src/slave/flags.cpp 54d9acc8693f53294bdc2a88183cac84a8dfbfd9 
  src/tests/containerizer/ports_isolator_tests.cpp 
db080c4e9c8b0c036294a8f7a42617ca1231f884 


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

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


Testing (updated)
---

New test added to test feature:

[   OK ] NetworkPortsIsolatorTest.ROOT_NC_PortEnforcementProtectedPort 
(1886 ms)
[--] 1 test from NetworkPortsIsolatorTest (1887 ms total)

[--] Global test environment tear-down
[==] 1 test from 1 test case ran. (1900 ms total)
[  PASSED  ] 1 test.

Existing test updated to test the negative cases:

[   OK ] NetworkPortsIsolatorTest.ROOT_IsolatorFlags (58 ms)
[--] 1 test from NetworkPortsIsolatorTest (58 ms total)

[--] Global test environment tear-down
[==] 1 test from 1 test case ran. (69 ms total)
[  PASSED  ] 1 test.

Existing test for isolator feature:

[   OK ] NetworkPortsIsolatorTest.ROOT_NC_AllocatedPorts (1992 ms)
[--] 1 test from NetworkPortsIsolatorTest (1993 ms total)

[--] Global test environment tear-down
[==] 1 test from 1 test case ran. (2004 ms total)
[  PASSED  ] 1 test.


Thanks,

Xudong Ni

Re: Review Request 68366: Added agent protected port range option in network isolator.

[Xudong Ni via Review Board]

> On Aug. 17, 2018, 10:02 p.m., James Peach wrote:
> > src/slave/containerizer/mesos/isolators/network/ports.cpp
> > Lines 343 (patched)
> > 
> >
> > `flags.container_ports_protected_range` is already an interval set. You 
> > don't need to convert it to resources and back again.

flags.container_ports_protected_range is a string, we do need to convert it 
into range. Is there better way to do it?


- Xudong


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


On Aug. 17, 2018, 6:27 p.m., Xudong Ni wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68366/
> ---
> 
> (Updated Aug. 17, 2018, 6:27 p.m.)
> 
> 
> Review request for mesos and James Peach.
> 
> 
> Bugs: MESOS-9133
> https://issues.apache.org/jira/browse/MESOS-9133
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> For a network isolator disabled environment, in practice, there could
> be a lot of users already binding to ephemeral ports; It would take
> a lot of efforts to find/notify/modify those apps; In order to take
> advantage of network isolator and enable it in such system, it would
> be useful to add mesos-agent configuration option to allow enforce
> port isolation in only the specified certain port range
> 
> 
> Diffs
> -
> 
>   docs/configuration/agent.md 4e50b681bb956d559da6bf1d2c504099aae3cafb 
>   docs/isolators/network-ports.md 5d14fc2985e099783b09e2a19f99641b4ddbd768 
>   src/slave/containerizer/mesos/isolators/network/ports.hpp 
> 6944d01e0f8a11eda381ef1754f19ee0cf9359c8 
>   src/slave/containerizer/mesos/isolators/network/ports.cpp 
> 2a7ff2530f898cf892739c715b07b3387b423ed9 
>   src/slave/flags.hpp 88c35da5fd754abbd4bd316e1fa9efa4a70a6b8c 
>   src/slave/flags.cpp 54d9acc8693f53294bdc2a88183cac84a8dfbfd9 
>   src/slave/main.cpp 489e87522588be259d382f588b66907ba29f1788 
>   src/tests/containerizer/ports_isolator_tests.cpp 
> db080c4e9c8b0c036294a8f7a42617ca1231f884 
> 
> 
> Diff: https://reviews.apache.org/r/68366/diff/4/
> 
> 
> Testing
> ---
> 
> New test added to test feature:
> [   OK ] NetworkPortsIsolatorTest.ROOT_NC_PortEnforcementProtectedPort 
> (1812 ms)
> [--] 1 test from NetworkPortsIsolatorTest (1813 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (1826 ms total)
> [  PASSED  ] 1 test.
> 
> Existing test updated to test the negative cases:
> 
> [ RUN  ] NetworkPortsIsolatorTest.ROOT_IsolatorFlags
> [   OK ] NetworkPortsIsolatorTest.ROOT_IsolatorFlags (69 ms)
> [--] 1 test from NetworkPortsIsolatorTest (70 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (82 ms total)
> [  PASSED  ] 1 test.
> 
> Existing test for isolator feature:
> 
> [   OK ] NetworkPortsIsolatorTest.ROOT_NC_AllocatedPorts (1821 ms)
> [--] 1 test from NetworkPortsIsolatorTest (1822 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (1836 ms total)
> [  PASSED  ] 1 test.
> 
> 
> Thanks,
> 
> Xudong Ni
> 
>

Re: Review Request 68366: Added agent protected port range option in network isolator.

[Xudong Ni via Review Board]

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

(Updated Aug. 17, 2018, 6:27 p.m.)


Review request for mesos and James Peach.


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


Repository: mesos


Description
---

For a network isolator disabled environment, in practice, there could
be a lot of users already binding to ephemeral ports; It would take
a lot of efforts to find/notify/modify those apps; In order to take
advantage of network isolator and enable it in such system, it would
be useful to add mesos-agent configuration option to allow enforce
port isolation in only the specified certain port range


Diffs (updated)
-

  docs/configuration/agent.md 4e50b681bb956d559da6bf1d2c504099aae3cafb 
  docs/isolators/network-ports.md 5d14fc2985e099783b09e2a19f99641b4ddbd768 
  src/slave/containerizer/mesos/isolators/network/ports.hpp 
6944d01e0f8a11eda381ef1754f19ee0cf9359c8 
  src/slave/containerizer/mesos/isolators/network/ports.cpp 
2a7ff2530f898cf892739c715b07b3387b423ed9 
  src/slave/flags.hpp 88c35da5fd754abbd4bd316e1fa9efa4a70a6b8c 
  src/slave/flags.cpp 54d9acc8693f53294bdc2a88183cac84a8dfbfd9 
  src/slave/main.cpp 489e87522588be259d382f588b66907ba29f1788 
  src/tests/containerizer/ports_isolator_tests.cpp 
db080c4e9c8b0c036294a8f7a42617ca1231f884 


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

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


Testing
---

New test added to test feature:
[   OK ] NetworkPortsIsolatorTest.ROOT_NC_PortEnforcementProtectedPort 
(1812 ms)
[--] 1 test from NetworkPortsIsolatorTest (1813 ms total)

[--] Global test environment tear-down
[==] 1 test from 1 test case ran. (1826 ms total)
[  PASSED  ] 1 test.

Existing test updated to test the negative cases:

[ RUN  ] NetworkPortsIsolatorTest.ROOT_IsolatorFlags
[   OK ] NetworkPortsIsolatorTest.ROOT_IsolatorFlags (69 ms)
[--] 1 test from NetworkPortsIsolatorTest (70 ms total)

[--] Global test environment tear-down
[==] 1 test from 1 test case ran. (82 ms total)
[  PASSED  ] 1 test.

Existing test for isolator feature:

[   OK ] NetworkPortsIsolatorTest.ROOT_NC_AllocatedPorts (1821 ms)
[--] 1 test from NetworkPortsIsolatorTest (1822 ms total)

[--] Global test environment tear-down
[==] 1 test from 1 test case ran. (1836 ms total)
[  PASSED  ] 1 test.


Thanks,

Xudong Ni

Re: Review Request 68366: Added agent protected port range option in network isolator.

[Xudong Ni via Review Board]

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

(Updated Aug. 17, 2018, 5:21 p.m.)


Review request for mesos and James Peach.


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


Repository: mesos


Description
---

For a network isolator disabled environment, in practice, there could
be a lot of users already binding to ephemeral ports; It would take
a lot of efforts to find/notify/modify those apps; In order to take
advantage of network isolator and enable it in such system, it would
be useful to add mesos-agent configuration option to allow enforce
port isolation in only the specified certain port range


Diffs (updated)
-

  docs/configuration/agent.md 4e50b681bb956d559da6bf1d2c504099aae3cafb 
  docs/isolators/network-ports.md 5d14fc2985e099783b09e2a19f99641b4ddbd768 
  src/slave/containerizer/mesos/isolators/network/ports.hpp 
6944d01e0f8a11eda381ef1754f19ee0cf9359c8 
  src/slave/containerizer/mesos/isolators/network/ports.cpp 
2a7ff2530f898cf892739c715b07b3387b423ed9 
  src/slave/flags.hpp 88c35da5fd754abbd4bd316e1fa9efa4a70a6b8c 
  src/slave/flags.cpp 54d9acc8693f53294bdc2a88183cac84a8dfbfd9 
  src/slave/main.cpp 489e87522588be259d382f588b66907ba29f1788 
  src/tests/containerizer/ports_isolator_tests.cpp 
db080c4e9c8b0c036294a8f7a42617ca1231f884 


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

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


Testing (updated)
---

New test added to test feature:
[   OK ] NetworkPortsIsolatorTest.ROOT_NC_PortEnforcementProtectedPort 
(1812 ms)
[--] 1 test from NetworkPortsIsolatorTest (1813 ms total)

[--] Global test environment tear-down
[==] 1 test from 1 test case ran. (1826 ms total)
[  PASSED  ] 1 test.

Existing test updated to test the negative cases:

[ RUN  ] NetworkPortsIsolatorTest.ROOT_IsolatorFlags
[   OK ] NetworkPortsIsolatorTest.ROOT_IsolatorFlags (69 ms)
[--] 1 test from NetworkPortsIsolatorTest (70 ms total)

[--] Global test environment tear-down
[==] 1 test from 1 test case ran. (82 ms total)
[  PASSED  ] 1 test.

Existing test for isolator feature:

[   OK ] NetworkPortsIsolatorTest.ROOT_NC_AllocatedPorts (1821 ms)
[--] 1 test from NetworkPortsIsolatorTest (1822 ms total)

[--] Global test environment tear-down
[==] 1 test from 1 test case ran. (1836 ms total)
[  PASSED  ] 1 test.


Thanks,

Xudong Ni

Re: Review Request 68366: Added agent protected port range option in network isolator.

[Xudong Ni via Review Board]

> On Aug. 16, 2018, 5:22 p.m., James Peach wrote:
> > As per our offline discussion, I think that we can generalize this to make 
> > it consistent with the existing options. If we have the concept of a 
> > protected port range, then the existing features map to "protect all ports" 
> > and "protect agent ports". This option will end up being "protect a custom 
> > port range". I think that this concept makes the options easier to explain 
> > and easier for operators to reason about.

Updated the review as suggested, make the option more generic


- Xudong


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


On Aug. 16, 2018, 9:28 p.m., Xudong Ni wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68366/
> ---
> 
> (Updated Aug. 16, 2018, 9:28 p.m.)
> 
> 
> Review request for mesos and James Peach.
> 
> 
> Bugs: MESOS-9133
> https://issues.apache.org/jira/browse/MESOS-9133
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> For a network isolator disabled environment, in practice, there could
> be a lot of users already binding to ephemeral ports; It would take
> a lot of efforts to find/notify/modify those apps; In order to take
> advantage of network isolator and enable it in such system, it would
> be useful to add mesos-agent configuration option to allow enforce
> port isolation in only the specified certain port range
> 
> 
> Diffs
> -
> 
>   docs/configuration/agent.md 4e50b681bb956d559da6bf1d2c504099aae3cafb 
>   docs/isolators/network-ports.md 5d14fc2985e099783b09e2a19f99641b4ddbd768 
>   src/slave/containerizer/mesos/isolators/network/ports.hpp 
> 6944d01e0f8a11eda381ef1754f19ee0cf9359c8 
>   src/slave/containerizer/mesos/isolators/network/ports.cpp 
> 2a7ff2530f898cf892739c715b07b3387b423ed9 
>   src/slave/flags.hpp 88c35da5fd754abbd4bd316e1fa9efa4a70a6b8c 
>   src/slave/flags.cpp 54d9acc8693f53294bdc2a88183cac84a8dfbfd9 
>   src/tests/containerizer/ports_isolator_tests.cpp 
> db080c4e9c8b0c036294a8f7a42617ca1231f884 
> 
> 
> Diff: https://reviews.apache.org/r/68366/diff/2/
> 
> 
> Testing
> ---
> 
> New test added to test feature:
> 
> [ RUN  ] NetworkPortsIsolatorTest.ROOT_NC_PortEnforcementProtectedPort
> [   OK ] NetworkPortsIsolatorTest.ROOT_NC_PortEnforcementProtectedPort 
> (786 ms)
> [--] 1 test from NetworkPortsIsolatorTest (787 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (799 ms total)
> [  PASSED  ] 1 test.
> 
> Existing test updated to test the negative cases:
> 
> [ RUN  ] NetworkPortsIsolatorTest.ROOT_IsolatorFlags
> [   OK ] NetworkPortsIsolatorTest.ROOT_IsolatorFlags (70 ms)
> [--] 1 test from NetworkPortsIsolatorTest (71 ms total)
> 
> Existing test for isolator feature:
> 
> [   OK ] NetworkPortsIsolatorTest.ROOT_NC_AllocatedPorts (1895 ms)
> [--] 1 test from NetworkPortsIsolatorTest (1896 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (1909 ms total)
> [  PASSED  ] 1 test.
> 
> 
> Thanks,
> 
> Xudong Ni
> 
>

Re: Review Request 68366: Added agent protected port range option in network isolator.

[Xudong Ni via Review Board]

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

(Updated Aug. 16, 2018, 9:28 p.m.)


Review request for mesos and James Peach.


Summary (updated)
-

Added agent protected port range option in network isolator.


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


Repository: mesos


Description (updated)
---

For a network isolator disabled environment, in practice, there could
be a lot of users already binding to ephemeral ports; It would take
a lot of efforts to find/notify/modify those apps; In order to take
advantage of network isolator and enable it in such system, it would
be useful to add mesos-agent configuration option to allow enforce
port isolation in only the specified certain port range


Diffs (updated)
-

  docs/configuration/agent.md 4e50b681bb956d559da6bf1d2c504099aae3cafb 
  docs/isolators/network-ports.md 5d14fc2985e099783b09e2a19f99641b4ddbd768 
  src/slave/containerizer/mesos/isolators/network/ports.hpp 
6944d01e0f8a11eda381ef1754f19ee0cf9359c8 
  src/slave/containerizer/mesos/isolators/network/ports.cpp 
2a7ff2530f898cf892739c715b07b3387b423ed9 
  src/slave/flags.hpp 88c35da5fd754abbd4bd316e1fa9efa4a70a6b8c 
  src/slave/flags.cpp 54d9acc8693f53294bdc2a88183cac84a8dfbfd9 
  src/tests/containerizer/ports_isolator_tests.cpp 
db080c4e9c8b0c036294a8f7a42617ca1231f884 


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

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


Testing (updated)
---

New test added to test feature:

[ RUN  ] NetworkPortsIsolatorTest.ROOT_NC_PortEnforcementProtectedPort
[   OK ] NetworkPortsIsolatorTest.ROOT_NC_PortEnforcementProtectedPort (786 
ms)
[--] 1 test from NetworkPortsIsolatorTest (787 ms total)

[--] Global test environment tear-down
[==] 1 test from 1 test case ran. (799 ms total)
[  PASSED  ] 1 test.

Existing test updated to test the negative cases:

[ RUN  ] NetworkPortsIsolatorTest.ROOT_IsolatorFlags
[   OK ] NetworkPortsIsolatorTest.ROOT_IsolatorFlags (70 ms)
[--] 1 test from NetworkPortsIsolatorTest (71 ms total)

Existing test for isolator feature:

[   OK ] NetworkPortsIsolatorTest.ROOT_NC_AllocatedPorts (1895 ms)
[--] 1 test from NetworkPortsIsolatorTest (1896 ms total)

[--] Global test environment tear-down
[==] 1 test from 1 test case ran. (1909 ms total)
[  PASSED  ] 1 test.


Thanks,

Xudong Ni

Review Request 68366: Added agent config option to allow ignoring ephemeral port range.

[Xudong Ni via Review Board]

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

Review request for mesos and James Peach.


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


Repository: mesos


Description
---

For a network isolator disabled environment, in practice, there could
be a lot of users already binding to ephemeral ports; It would take
a lot of efforts to find/notify/modify those apps; In order to take
advantage of network isolator and enable it in such system, it would
be useful to add mesos-agent configuration option to allow ignoring
ports bound within the ephemeral port range


Diffs
-

  docs/configuration/agent.md 4e50b681bb956d559da6bf1d2c504099aae3cafb 
  docs/isolators/network-ports.md 5d14fc2985e099783b09e2a19f99641b4ddbd768 
  src/slave/containerizer/mesos/isolators/network/ports.hpp 
6944d01e0f8a11eda381ef1754f19ee0cf9359c8 
  src/slave/containerizer/mesos/isolators/network/ports.cpp 
2a7ff2530f898cf892739c715b07b3387b423ed9 
  src/slave/flags.hpp 88c35da5fd754abbd4bd316e1fa9efa4a70a6b8c 
  src/slave/flags.cpp 54d9acc8693f53294bdc2a88183cac84a8dfbfd9 
  src/tests/containerizer/ports_isolator_tests.cpp 
db080c4e9c8b0c036294a8f7a42617ca1231f884 


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


Testing
---

New test added to test feature:

[ RUN  ] NetworkPortsIsolatorTest.ROOT_IsolatorFlags
[   OK ] NetworkPortsIsolatorTest.ROOT_IsolatorFlags (70 ms)
[--] 1 test from NetworkPortsIsolatorTest (71 ms total)

[--] Global test environment tear-down
[==] 1 test from 1 test case ran. (82 ms total)
[  PASSED  ] 1 test.

Existing test updated to test the negative cases:

[ RUN  ] NetworkPortsIsolatorTest.ROOT_IsolatorFlags
[   OK ] NetworkPortsIsolatorTest.ROOT_IsolatorFlags (70 ms)
[--] 1 test from NetworkPortsIsolatorTest (71 ms total)

Existing test for isolator feature:

[   OK ] NetworkPortsIsolatorTest.ROOT_NC_AllocatedPorts (1895 ms)
[--] 1 test from NetworkPortsIsolatorTest (1896 ms total)

[--] Global test environment tear-down
[==] 1 test from 1 test case ran. (1909 ms total)
[  PASSED  ] 1 test.


Thanks,

Xudong Ni

Re: Review Request 67365: Added MESOS-8340 to the 1.7.x CHANGELOG.

[Xudong Ni via Review Board]

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


Ship it!




Ship It!

- Xudong Ni


On May 30, 2018, 6:57 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67365/
> ---
> 
> (Updated May 30, 2018, 6:57 p.m.)
> 
> 
> Review request for mesos, Xudong Ni and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-8340
> https://issues.apache.org/jira/browse/MESOS-8340
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added MESOS-8340 to the 1.7.x CHANGELOG.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 52c7fc3bcd73a0eead2934644f636b85c60e0061 
>   docs/upgrades.md 1a1705cd9c38583c591392dd154d136fce04129d 
> 
> 
> Diff: https://reviews.apache.org/r/67365/diff/2/
> 
> 
> Testing
> ---
> 
> Manual inspection of the dev website.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 67365: Added MESOS-8340 to the 1.7.x CHANGELOG.

[Xudong Ni via Review Board]

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




CHANGELOG
Lines 8 (patched)


Need empty line to separate new features


- Xudong Ni


On May 29, 2018, 10:46 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67365/
> ---
> 
> (Updated May 29, 2018, 10:46 p.m.)
> 
> 
> Review request for mesos, Xudong Ni and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-8340
> https://issues.apache.org/jira/browse/MESOS-8340
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added MESOS-8340 to the 1.7.x CHANGELOG.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 52c7fc3bcd73a0eead2934644f636b85c60e0061 
>   docs/upgrades.md 1a1705cd9c38583c591392dd154d136fce04129d 
> 
> 
> Diff: https://reviews.apache.org/r/67365/diff/1/
> 
> 
> Testing
> ---
> 
> Manual inspection of the dev website.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 67271: Renamed an ambiguous variable in network port isolator.

[Xudong Ni via Review Board]

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

(Updated May 24, 2018, 9:57 p.m.)


Review request for mesos and James Peach.


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


Repository: mesos


Description
---

Renamed an ambiguous variable in network port isolator.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/network/ports.hpp 
ba71087194a3ae74c7e40dffa9c108b02ffa10ad 
  src/slave/containerizer/mesos/isolators/network/ports.cpp 
1f84ed4fb2a30fd095e2faec1038de1fa19a15c5 


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

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


Testing
---


Thanks,

Xudong Ni

Re: Review Request 67195: Added enforce_container_ports flag for network ports isolation.

[Xudong Ni via Review Board]

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

(Updated May 24, 2018, 9:56 p.m.)


Review request for mesos, James Peach and Jiang Yan Xu.


Summary (updated)
-

Added enforce_container_ports flag for network ports isolation.


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


Repository: mesos


Description (updated)
---

To reduce deployment risk, a nonenforce mode is added for network
port isolator. When this flag is set as false(default is false),
even task uses ports not in the container resources, the container
will not raise any limitation.

Added new test for this flag and updated the existing tests


Diffs (updated)
-

  docs/configuration/agent.md 962211a54177a54b3e38a93aad9af3c7a0f94ecb 
  docs/isolators/network-ports.md ea63968481ce52c46e0a98e242da49baf6962009 
  src/slave/containerizer/mesos/isolators/network/ports.hpp 
ba71087194a3ae74c7e40dffa9c108b02ffa10ad 
  src/slave/containerizer/mesos/isolators/network/ports.cpp 
1f84ed4fb2a30fd095e2faec1038de1fa19a15c5 
  src/slave/flags.hpp a839591a2b66444ad97fced0620201dde656352d 
  src/slave/flags.cpp a319b5ea633c41fd8a252c5e1617ac52d1480ba5 
  src/tests/containerizer/ports_isolator_tests.cpp 
c5b9f926047792e7f9d1f0937fa5355b1dd77965 


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

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


Testing
---

New test added for the flag; Related unit tests passed.

[ OK ] NetworkPortsIsolatorTest.ROOT_NC_AllocatedPorts (1906 ms)
[ OK ] NetworkPortsIsolatorTest.ROOT_NC_NoPortsResource (1788 ms)
[ OK ] NetworkPortsIsolatorTest.ROOT_NC_NoPortEnforement (17001 ms)


Thanks,

Xudong Ni

Re: Review Request 67195: Adding enforce_container_ports flag for network ports isolation.

[Xudong Ni via Review Board]

> On May 24, 2018, 8:30 p.m., James Peach wrote:
> > Commit subject should be "Added ..."

subject changed


- Xudong


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


On May 23, 2018, 6:33 p.m., Xudong Ni wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67195/
> ---
> 
> (Updated May 23, 2018, 6:33 p.m.)
> 
> 
> Review request for mesos, James Peach and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-8340
> https://issues.apache.org/jira/browse/MESOS-8340
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> To reduce deployment risk, a nonenforce mode is added for network
> port isolator. When this flag is set as false(default is false),
> even task uses ports not in the container resources, the container
> will not raise any limitation.
> 
> Add new test for this flag and update the existing tests
> 
> 
> Diffs
> -
> 
>   docs/isolators/network-ports.md ea63968481ce52c46e0a98e242da49baf6962009 
>   src/slave/containerizer/mesos/isolators/network/ports.hpp 
> ba71087194a3ae74c7e40dffa9c108b02ffa10ad 
>   src/slave/containerizer/mesos/isolators/network/ports.cpp 
> 1f84ed4fb2a30fd095e2faec1038de1fa19a15c5 
>   src/slave/flags.hpp a839591a2b66444ad97fced0620201dde656352d 
>   src/slave/flags.cpp a319b5ea633c41fd8a252c5e1617ac52d1480ba5 
>   src/tests/containerizer/ports_isolator_tests.cpp 
> c5b9f926047792e7f9d1f0937fa5355b1dd77965 
> 
> 
> Diff: https://reviews.apache.org/r/67195/diff/3/
> 
> 
> Testing
> ---
> 
> New test added for the flag; Related unit tests passed.
> 
> [ OK ] NetworkPortsIsolatorTest.ROOT_NC_AllocatedPorts (1906 ms)
> [ OK ] NetworkPortsIsolatorTest.ROOT_NC_NoPortsResource (1788 ms)
> [ OK ] NetworkPortsIsolatorTest.ROOT_NC_NoPortEnforement (17001 ms)
> 
> 
> Thanks,
> 
> Xudong Ni
> 
>

Re: Review Request 67195: Adding enforce_container_ports flag for network ports isolation.

[Xudong Ni via Review Board]

> On May 24, 2018, 5:58 p.m., Gilbert Song wrote:
> > we need to update `configure.md` correspondingly.

Updated configuration/agent.md


- Xudong


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


On May 23, 2018, 6:33 p.m., Xudong Ni wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67195/
> ---
> 
> (Updated May 23, 2018, 6:33 p.m.)
> 
> 
> Review request for mesos, James Peach and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-8340
> https://issues.apache.org/jira/browse/MESOS-8340
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> To reduce deployment risk, a nonenforce mode is added for network
> port isolator. When this flag is set as false(default is false),
> even task uses ports not in the container resources, the container
> will not raise any limitation.
> 
> Add new test for this flag and update the existing tests
> 
> 
> Diffs
> -
> 
>   docs/isolators/network-ports.md ea63968481ce52c46e0a98e242da49baf6962009 
>   src/slave/containerizer/mesos/isolators/network/ports.hpp 
> ba71087194a3ae74c7e40dffa9c108b02ffa10ad 
>   src/slave/containerizer/mesos/isolators/network/ports.cpp 
> 1f84ed4fb2a30fd095e2faec1038de1fa19a15c5 
>   src/slave/flags.hpp a839591a2b66444ad97fced0620201dde656352d 
>   src/slave/flags.cpp a319b5ea633c41fd8a252c5e1617ac52d1480ba5 
>   src/tests/containerizer/ports_isolator_tests.cpp 
> c5b9f926047792e7f9d1f0937fa5355b1dd77965 
> 
> 
> Diff: https://reviews.apache.org/r/67195/diff/3/
> 
> 
> Testing
> ---
> 
> New test added for the flag; Related unit tests passed.
> 
> [ OK ] NetworkPortsIsolatorTest.ROOT_NC_AllocatedPorts (1906 ms)
> [ OK ] NetworkPortsIsolatorTest.ROOT_NC_NoPortsResource (1788 ms)
> [ OK ] NetworkPortsIsolatorTest.ROOT_NC_NoPortEnforement (17001 ms)
> 
> 
> Thanks,
> 
> Xudong Ni
> 
>

Re: Review Request 67271: Renamed an ambiguous variable in network port isolator.

[Xudong Ni via Review Board]

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

(Updated May 24, 2018, 6:17 p.m.)


Review request for mesos and James Peach.


Summary (updated)
-

Renamed an ambiguous variable in network port isolator.


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


Repository: mesos


Description (updated)
---

Renamed an ambiguous variable in network port isolator.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/network/ports.hpp 
ba71087194a3ae74c7e40dffa9c108b02ffa10ad 
  src/slave/containerizer/mesos/isolators/network/ports.cpp 
1f84ed4fb2a30fd095e2faec1038de1fa19a15c5 


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

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


Testing
---


Thanks,

Xudong Ni

Re: Review Request 67271: Change ambiguous parameter name.

[Xudong Ni via Review Board]

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

(Updated May 23, 2018, 6:42 p.m.)


Review request for mesos and James Peach.


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


Repository: mesos


Description
---

Change ambiguous parameter name.


Diffs
-

  src/slave/containerizer/mesos/isolators/network/ports.hpp 
ba71087194a3ae74c7e40dffa9c108b02ffa10ad 
  src/slave/containerizer/mesos/isolators/network/ports.cpp 
1f84ed4fb2a30fd095e2faec1038de1fa19a15c5 


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


Testing
---


Thanks,

Xudong Ni

Re: Review Request 67195: Adding enforce_container_ports flag for network ports isolation.

[Xudong Ni via Review Board]

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

(Updated May 23, 2018, 6:33 p.m.)


Review request for mesos, James Peach and Jiang Yan Xu.


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


Repository: mesos


Description
---

To reduce deployment risk, a nonenforce mode is added for network
port isolator. When this flag is set as false(default is false),
even task uses ports not in the container resources, the container
will not raise any limitation.

Add new test for this flag and update the existing tests


Diffs (updated)
-

  docs/isolators/network-ports.md ea63968481ce52c46e0a98e242da49baf6962009 
  src/slave/containerizer/mesos/isolators/network/ports.hpp 
ba71087194a3ae74c7e40dffa9c108b02ffa10ad 
  src/slave/containerizer/mesos/isolators/network/ports.cpp 
1f84ed4fb2a30fd095e2faec1038de1fa19a15c5 
  src/slave/flags.hpp a839591a2b66444ad97fced0620201dde656352d 
  src/slave/flags.cpp a319b5ea633c41fd8a252c5e1617ac52d1480ba5 
  src/tests/containerizer/ports_isolator_tests.cpp 
c5b9f926047792e7f9d1f0937fa5355b1dd77965 


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

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


Testing
---

New test added for the flag; Related unit tests passed.

[ OK ] NetworkPortsIsolatorTest.ROOT_NC_AllocatedPorts (1906 ms)
[ OK ] NetworkPortsIsolatorTest.ROOT_NC_NoPortsResource (1788 ms)
[ OK ] NetworkPortsIsolatorTest.ROOT_NC_NoPortEnforement (17001 ms)


Thanks,

Xudong Ni

Re: Review Request 67271: Change ambiguous parameter name.

[Xudong Ni via Review Board]

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

(Updated May 23, 2018, 6:32 p.m.)


Review request for mesos and James Peach.


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


Repository: mesos


Description
---

Change ambiguous parameter name.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/network/ports.hpp 
ba71087194a3ae74c7e40dffa9c108b02ffa10ad 
  src/slave/containerizer/mesos/isolators/network/ports.cpp 
1f84ed4fb2a30fd095e2faec1038de1fa19a15c5 


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

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


Testing
---


Thanks,

Xudong Ni

Review Request 67271: Change ambiguous parameter name.

[Xudong Ni via Review Board]

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

Review request for mesos and James Peach.


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


Repository: mesos


Description
---

Change ambiguous parameter name.


Diffs
-

  src/slave/containerizer/mesos/isolators/network/ports.hpp 
ba71087194a3ae74c7e40dffa9c108b02ffa10ad 
  src/slave/containerizer/mesos/isolators/network/ports.cpp 
1f84ed4fb2a30fd095e2faec1038de1fa19a15c5 


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


Testing
---


Thanks,

Xudong Ni

Re: Review Request 67195: Adding enforce_container_ports flag for network ports isolation.

[Xudong Ni via Review Board]

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

(Updated May 23, 2018, 5:57 p.m.)


Review request for mesos, James Peach and Jiang Yan Xu.


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


Repository: mesos


Description (updated)
---

To reduce deployment risk, a nonenforce mode is added for network
port isolator. When this flag is set as false(default is false),
even task uses ports not in the container resources, the container
will not raise any limitation.

Add new test for this flag and update the existing tests


Diffs (updated)
-

  docs/isolators/network-ports.md ea63968481ce52c46e0a98e242da49baf6962009 
  src/slave/containerizer/mesos/isolators/network/ports.hpp 
ba71087194a3ae74c7e40dffa9c108b02ffa10ad 
  src/slave/containerizer/mesos/isolators/network/ports.cpp 
1f84ed4fb2a30fd095e2faec1038de1fa19a15c5 
  src/slave/flags.hpp a839591a2b66444ad97fced0620201dde656352d 
  src/slave/flags.cpp a319b5ea633c41fd8a252c5e1617ac52d1480ba5 
  src/tests/containerizer/ports_isolator_tests.cpp 
c5b9f926047792e7f9d1f0937fa5355b1dd77965 


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

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


Testing
---

New test added for the flag; Related unit tests passed.

[ OK ] NetworkPortsIsolatorTest.ROOT_NC_AllocatedPorts (1906 ms)
[ OK ] NetworkPortsIsolatorTest.ROOT_NC_NoPortsResource (1788 ms)
[ OK ] NetworkPortsIsolatorTest.ROOT_NC_NoPortEnforement (17001 ms)


Thanks,

Xudong Ni

Review Request 67244: Adding enforce_container_ports flag for network ports isolation.

[Xudong Ni via Review Board]

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

Review request for mesos.


Repository: mesos


Description
---

To reduce deployment risk, a nonenforce mode is added for network
port isolator. When this flag is set as false(default is false),
even task uses ports not in the container resources, the container
will not raise any limitation.

Add new test for this flag and update the existing tests


Diffs (updated)
-

  docs/isolators/network-ports.md ea63968481ce52c46e0a98e242da49baf6962009 
  src/slave/containerizer/mesos/isolators/network/ports.hpp 
ba71087194a3ae74c7e40dffa9c108b02ffa10ad 
  src/slave/containerizer/mesos/isolators/network/ports.cpp 
1f84ed4fb2a30fd095e2faec1038de1fa19a15c5 
  src/slave/flags.hpp a839591a2b66444ad97fced0620201dde656352d 
  src/slave/flags.cpp a319b5ea633c41fd8a252c5e1617ac52d1480ba5 
  src/tests/containerizer/ports_isolator_tests.cpp 
c5b9f926047792e7f9d1f0937fa5355b1dd77965 


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


Testing
---


Thanks,

Xudong Ni

Re: Review Request 67244: Adding enforce_container_ports flag for network ports isolation.

[Xudong Ni via Review Board]

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

(Updated May 22, 2018, 4:02 a.m.)


Review request for mesos, James Peach and Jiang Yan Xu.


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


Repository: mesos


Description
---

To reduce deployment risk, a nonenforce mode is added for network
port isolator. When this flag is set as false(default is false),
even task uses ports not in the container resources, the container
will not raise any limitation.

Add new test for this flag and update the existing tests


Diffs
-

  docs/isolators/network-ports.md ea63968481ce52c46e0a98e242da49baf6962009 
  src/slave/containerizer/mesos/isolators/network/ports.hpp 
ba71087194a3ae74c7e40dffa9c108b02ffa10ad 
  src/slave/containerizer/mesos/isolators/network/ports.cpp 
1f84ed4fb2a30fd095e2faec1038de1fa19a15c5 
  src/slave/flags.hpp a839591a2b66444ad97fced0620201dde656352d 
  src/slave/flags.cpp a319b5ea633c41fd8a252c5e1617ac52d1480ba5 
  src/tests/containerizer/ports_isolator_tests.cpp 
c5b9f926047792e7f9d1f0937fa5355b1dd77965 


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


Testing
---


Thanks,

Xudong Ni

Re: Review Request 67195: Adding enforce_container_ports flag for network ports isolation.

[Xudong Ni via Review Board]

> On May 21, 2018, 8:56 p.m., James Peach wrote:
> > src/slave/containerizer/mesos/isolators/network/ports.cpp
> > Lines 577 (patched)
> > 
> >
> > This is more complicated than it needs to be. You can simply do this:
> > 
> > ```
> > if (!enforceContainerPorts) {
> >   if (info->activePorts.isSome() &&
> >   info->activePorts == listeners) {
> > VLOG(2) << "Skipping container ports violation log";
> > continue;
> >   }
> > 
> >   // Cache the last listeners sample so that we will
> >   // only log new ports resource violations.
> >   info->activePorts = listeners; 
> > }
> > 
> > 
> > ```

Not sure I fully understand the idea, correct me if I am wrong; In the hashmap, 
the listener ports is a set of ports which may have both allocated and 
unallocated ports(the same applied to loggedPorts and unloggedPorts, we can not 
either use "==" or contains() set function to compare the set. We need set 
operation substraction to get either unallocated or unlogged set.


- Xudong


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


On May 21, 2018, 4:14 p.m., Xudong Ni wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67195/
> ---
> 
> (Updated May 21, 2018, 4:14 p.m.)
> 
> 
> Review request for mesos, James Peach and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-8340
> https://issues.apache.org/jira/browse/MESOS-8340
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> To reduce deployment risk, a nonenforce mode is added for network
> port isolator. When this flag is set as false(default is false),
> even task uses ports not in the container resources, the container
> will not raise any limitation.
> 
> Add new test for this flag and update the existing tests
> 
> https://reviews.apache.org/r/67195/
> 
> 
> Diffs
> -
> 
>   docs/isolators/network-ports.md ea63968481ce52c46e0a98e242da49baf6962009 
>   src/slave/containerizer/mesos/isolators/network/ports.hpp 
> ba71087194a3ae74c7e40dffa9c108b02ffa10ad 
>   src/slave/containerizer/mesos/isolators/network/ports.cpp 
> 1f84ed4fb2a30fd095e2faec1038de1fa19a15c5 
>   src/slave/flags.hpp a839591a2b66444ad97fced0620201dde656352d 
>   src/slave/flags.cpp a319b5ea633c41fd8a252c5e1617ac52d1480ba5 
>   src/tests/containerizer/ports_isolator_tests.cpp 
> c5b9f926047792e7f9d1f0937fa5355b1dd77965 
> 
> 
> Diff: https://reviews.apache.org/r/67195/diff/1/
> 
> 
> Testing
> ---
> 
> New test added for the flag; Related unit tests passed.
> 
> [ OK ] NetworkPortsIsolatorTest.ROOT_NC_AllocatedPorts (1906 ms)
> [ OK ] NetworkPortsIsolatorTest.ROOT_NC_NoPortsResource (1788 ms)
> [ OK ] NetworkPortsIsolatorTest.ROOT_NC_NoPortEnforement (17001 ms)
> 
> 
> Thanks,
> 
> Xudong Ni
> 
>

Re: Review Request 67195: Adding enforce_container_ports flag for network ports isolation.

[Xudong Ni via Review Board]

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

(Updated May 21, 2018, 4:14 p.m.)


Review request for mesos, James Peach and Jiang Yan Xu.


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


Repository: mesos


Description
---

To reduce deployment risk, a nonenforce mode is added for network
port isolator. When this flag is set as false(default is false),
even task uses ports not in the container resources, the container
will not raise any limitation.

Add new test for this flag and update the existing tests

https://reviews.apache.org/r/67195/


Diffs
-

  docs/isolators/network-ports.md ea63968481ce52c46e0a98e242da49baf6962009 
  src/slave/containerizer/mesos/isolators/network/ports.hpp 
ba71087194a3ae74c7e40dffa9c108b02ffa10ad 
  src/slave/containerizer/mesos/isolators/network/ports.cpp 
1f84ed4fb2a30fd095e2faec1038de1fa19a15c5 
  src/slave/flags.hpp a839591a2b66444ad97fced0620201dde656352d 
  src/slave/flags.cpp a319b5ea633c41fd8a252c5e1617ac52d1480ba5 
  src/tests/containerizer/ports_isolator_tests.cpp 
c5b9f926047792e7f9d1f0937fa5355b1dd77965 


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


Testing
---

New test added for the flag; Related unit tests passed.

[ OK ] NetworkPortsIsolatorTest.ROOT_NC_AllocatedPorts (1906 ms)
[ OK ] NetworkPortsIsolatorTest.ROOT_NC_NoPortsResource (1788 ms)
[ OK ] NetworkPortsIsolatorTest.ROOT_NC_NoPortEnforement (17001 ms)


Thanks,

Xudong Ni

Review Request 67195: Adding enforce_container_ports flag for network ports isolation.

[Xudong Ni via Review Board]

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

Review request for mesos, James Peach and Jiang Yan Xu.


Bugs: mesos-8340
https://issues.apache.org/jira/browse/mesos-8340


Repository: mesos


Description
---

To reduce deployment risk, a nonenforce mode is added for network
port isolator. When this flag is set as false(default is false),
even task uses ports not in the container resources, the container
will not raise any limitation.

Add new test for this flag and update the existing tests

https://reviews.apache.org/r/67195/


Diffs
-

  docs/isolators/network-ports.md ea63968481ce52c46e0a98e242da49baf6962009 
  src/slave/containerizer/mesos/isolators/network/ports.hpp 
ba71087194a3ae74c7e40dffa9c108b02ffa10ad 
  src/slave/containerizer/mesos/isolators/network/ports.cpp 
1f84ed4fb2a30fd095e2faec1038de1fa19a15c5 
  src/slave/flags.hpp a839591a2b66444ad97fced0620201dde656352d 
  src/slave/flags.cpp a319b5ea633c41fd8a252c5e1617ac52d1480ba5 
  src/tests/containerizer/ports_isolator_tests.cpp 
c5b9f926047792e7f9d1f0937fa5355b1dd77965 


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


Testing
---

New test added for the flag; Related unit tests passed.

[ OK ] NetworkPortsIsolatorTest.ROOT_NC_AllocatedPorts (1906 ms)
[ OK ] NetworkPortsIsolatorTest.ROOT_NC_NoPortsResource (1788 ms)
[ OK ] NetworkPortsIsolatorTest.ROOT_NC_NoPortEnforement (17001 ms)


Thanks,

Xudong Ni

Re: Review Request 66919: Failure to update registry should abort the master process.

[Xudong Ni via Review Board]

> On May 8, 2018, 12:08 a.m., Jiang Yan Xu wrote:
> > src/master/http.cpp
> > Lines 4163 (patched)
> > 
> >
> > You missed a space between `result)` and `{` which I didn't catch 
> > initially but fixed up in a subsequent commit.

Thanks for the follow up commit


- Xudong


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


On May 7, 2018, 6:10 p.m., Xudong Ni wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66919/
> ---
> 
> (Updated May 7, 2018, 6:10 p.m.)
> 
> 
> Review request for mesos, Joseph Wu and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-8630
> https://issues.apache.org/jira/browse/MESOS-8630
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When the registrar fails to update the registry it would abort the
> actor and fail all future operations,However when the registrar
> updates is requested by an operator API such as maintenance update,
> the master process doesn't shut down (a 500 error is returned to the
> client instead)and all subsequent operations will fail.
> 
> Review: https://reviews.apache.org/r/66919
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 135ae4389623385a6638158f5f23d6daca14a0ad 
> 
> 
> Diff: https://reviews.apache.org/r/66919/diff/3/
> 
> 
> Testing
> ---
> 
> xujyan made a local test change to verify this patch:
> https://github.com/xujyan/mesos/commit/68051320a87431f6d2f3fbad6b0b97814200a731
> 
> Test output:
> F0504 20:43:36.530680 1858991 http.cpp:4304] CHECK_READY(result): is FAILED: 
> Failed to update registry: Failed to perform store within 5secs
> 
> 
> Thanks,
> 
> Xudong Ni
> 
>

Re: Review Request 66919: Failure to update registry should abort the master process.

[Xudong Ni via Review Board]

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

(Updated May 7, 2018, 6:10 p.m.)


Review request for mesos, Joseph Wu and Jiang Yan Xu.


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


Repository: mesos


Description
---

When the registrar fails to update the registry it would abort the
actor and fail all future operations,However when the registrar
updates is requested by an operator API such as maintenance update,
the master process doesn't shut down (a 500 error is returned to the
client instead)and all subsequent operations will fail.

Review: https://reviews.apache.org/r/66919


Diffs
-

  src/master/http.cpp 135ae4389623385a6638158f5f23d6daca14a0ad 


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


Testing (updated)
---

xujyan made a local test change to verify this patch:
https://github.com/xujyan/mesos/commit/68051320a87431f6d2f3fbad6b0b97814200a731

Test output:
F0504 20:43:36.530680 1858991 http.cpp:4304] CHECK_READY(result): is FAILED: 
Failed to update registry: Failed to perform store within 5secs


Thanks,

Xudong Ni

Re: Review Request 66919: Failure to update registry should abort the master process.

[Xudong Ni via Review Board]

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

(Updated May 7, 2018, 5:20 p.m.)


Review request for mesos, Joseph Wu and Jiang Yan Xu.


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


Repository: mesos


Description (updated)
---

When the registrar fails to update the registry it would abort the
actor and fail all future operations,However when the registrar
updates is requested by an operator API such as maintenance update,
the master process doesn't shut down (a 500 error is returned to the
client instead)and all subsequent operations will fail.

Review: https://reviews.apache.org/r/66919


Diffs (updated)
-

  src/master/http.cpp 135ae4389623385a6638158f5f23d6daca14a0ad 


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

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


Testing
---

No error found by running all unit tests(make check)


Thanks,

Xudong Ni

Re: Review Request 66919: Failure to update registry should abort the master process.

[Xudong Ni via Review Board]

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

(Updated May 7, 2018, 4:54 p.m.)


Review request for mesos, Joseph Wu and Jiang Yan Xu.


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


Repository: mesos


Description (updated)
---

When the registrar fails to update the registry it would abort the
actor and fail all future operations,However when the registrar
updates is requested by an operator API such as maintenance update,
the master process doesn't shut down (a 500 error is returned to the
client instead)and all subsequent operations will fail.


Diffs (updated)
-

  src/master/http.cpp 135ae4389623385a6638158f5f23d6daca14a0ad 


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

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


Testing
---

No error found by running all unit tests(make check)


Thanks,

Xudong Ni

Re: Review Request 66919: Failure to update registry should abort the master process.

[Xudong Ni via Review Board]

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

(Updated May 2, 2018, 9:38 p.m.)


Review request for mesos.


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


Repository: mesos


Description (updated)
---

When the registrar fails to update the registry it would abort the
actor and fail all future operations,However when the registrar
updates is requested by an operator API such as maintenance update,
the master process doesn't shut down (a 500 error is returned to the
client instead)and all subsequent operations will fail.

Review: https://reviews.apache.org/r/66919


Diffs
-

  src/master/http.cpp 135ae4389623385a6638158f5f23d6daca14a0ad 


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


Testing
---

No error found by running all unit tests(make check)


Thanks,

Xudong Ni

Review Request 66919: Failure to update registry should abort the master process.

[Xudong Ni via Review Board]

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

Review request for mesos.


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


Repository: mesos


Description
---

When the registrar fails to update the registry it would abort the
actor and fail all future operations,However when the registrar
updates is requested by an operator API such as maintenance update,
the master process doesn't shut down (a 500 error is returned to the
client instead)and all subsequent operations will fail.


Diffs
-

  src/master/http.cpp 135ae4389623385a6638158f5f23d6daca14a0ad 


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


Testing
---

No error found by running all unit tests(make check)


Thanks,

Xudong Ni