Re: Review Request 65252: Updated the v1/mesos.proto to keep consistancy with general mesos.proto.

2018-01-19 Thread Mesos Reviewbot Windows 

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



PASS: Mesos patch 65252 was successfully built and tested.

Reviews applied: `['65203', '65252']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65252

- Mesos Reviewbot Windows


On Jan. 20, 2018, 2:05 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65252/
> ---
> 
> (Updated Jan. 20, 2018, 2:05 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Chun-Hung Hsiao, Gaston Kleiman, 
> Greg Mann, Jie Yu, Michael Park, and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated the v1/mesos.proto to keep consistancy with general mesos.proto.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/mesos.proto b8e016e60322879e53ffa3bef23481015b0a6d2d 
> 
> 
> Diff: https://reviews.apache.org/r/65252/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Review Request 65252: Updated the v1/mesos.proto to keep consistancy with general mesos.proto.

2018-01-19 Thread Gilbert Song 

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

Review request for mesos, Benjamin Mahler, Chun-Hung Hsiao, Gaston Kleiman, 
Greg Mann, Jie Yu, Michael Park, and Neil Conway.


Repository: mesos


Description
---

Updated the v1/mesos.proto to keep consistancy with general mesos.proto.


Diffs
-

  include/mesos/v1/mesos.proto b8e016e60322879e53ffa3bef23481015b0a6d2d 


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


Testing
---


Thanks,

Gilbert Song

Re: Review Request 65203: Updated the CHANGELOG for 1.5.0 release.

2018-01-19 Thread Gilbert Song 

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

(Updated Jan. 19, 2018, 6:01 p.m.)


Review request for mesos, Adam B, Anand Mazumdar, Andrew Schwartzmeyer, 
Benjamin Mahler, Chun-Hung Hsiao, Gaston Kleiman, Greg Mann, Ilya Pronin, Jie 
Yu, James Peach, Kapil Arya, Joseph Wu, Kevin Klues, Neil Conway, Qian Zhang, 
Till Toenshoff, Vinod Kone, Jiang Yan Xu, and Zhitao Li.


Repository: mesos


Description
---

Updated the CHANGELOG for 1.5.0 release.


Diffs (updated)
-

  CHANGELOG 078c5d64100d81ce84dbdff4055cdda465e19011 


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

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


Testing
---

N/A.


Thanks,

Gilbert Song

Re: Review Request 65109: Fixed a bug relating to lingering executors.

2018-01-19 Thread Meng Zhu 

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

(Updated Jan. 19, 2018, 5:39 p.m.)


Review request for mesos, Benjamin Mahler and Vinod Kone.


Changes
---

Patch updated per our review meeting.


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


Repository: mesos


Description
---

An executor should be shutdown if it has never got any tasks
i.e. all of its initial tasks are killed before launching.
See MESOS-8411.

This patch ensures this by checking an executor's various
tasks queues during task kill and executor registration,
and shutting down executors that had never received any tasks.


Diffs (updated)
-

  src/slave/constants.hpp e6cb7cc0ccdaaf981eb66defa21b38720f4e1de9 
  src/slave/slave.hpp 09c01ebd1b5e8008ba9e7d412042f1db76a2c5a5 
  src/slave/slave.cpp 43c7955237655accdf869db1b6494a35f77acdb3 


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

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


Testing
---

make check
new tests in #65111


Thanks,

Meng Zhu

Re: Review Request 65182: Tested that agent resends unacknowledged op status updates on recovery.

2018-01-19 Thread Mesos Reviewbot Windows 

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



PASS: Mesos patch 65182 was successfully built and tested.

Reviews applied: `['64992', '64994', '65059', '64998', '65232', '65057', 
'65182']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65182

- Mesos Reviewbot Windows


On Jan. 19, 2018, 3:07 p.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65182/
> ---
> 
> (Updated Jan. 19, 2018, 3:07 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Greg Mann.
> 
> 
> Bugs: MESOS-8362
> https://issues.apache.org/jira/browse/MESOS-8362
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a test to verify that the agent resends unacknowledged
> operation status updates after a recovery.
> 
> 
> Diffs
> -
> 
>   src/tests/storage_local_resource_provider_tests.cpp 
> bbfe95e9818f25fdd5405db3ad2fe355e023f743 
> 
> 
> Diff: https://reviews.apache.org/r/65182/diff/5/
> 
> 
> Testing
> ---
> 
> `GLOG_v=1 sudo bin/mesos-tests.sh 
> --gtest_filter='StorageLocalResourceProviderTest.ROOT_RetryOperationStatusUpdateAfterRecovery'
>  --verbose --gtest_repeat=1000 --gtest_break_on_failure` on GNU/Linux
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 65057: Tested that op status updates dropped en route to master are resent.

2018-01-19 Thread Greg Mann 

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


Ship it!




Ship It!

- Greg Mann


On Jan. 19, 2018, 11:07 p.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65057/
> ---
> 
> (Updated Jan. 19, 2018, 11:07 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Greg Mann, and Jie Yu.
> 
> 
> Bugs: MESOS-8363 and MESOS-8420
> https://issues.apache.org/jira/browse/MESOS-8363
> https://issues.apache.org/jira/browse/MESOS-8420
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds
> `StorageLocalResourceProviderTest.ROOT_RetryOperationStatusUpdate` that
> verifies that operation status updates are resent to the master after
> being dropped en route to it.
> 
> 
> Diffs
> -
> 
>   src/tests/storage_local_resource_provider_tests.cpp 
> bbfe95e9818f25fdd5405db3ad2fe355e023f743 
> 
> 
> Diff: https://reviews.apache.org/r/65057/diff/6/
> 
> 
> Testing
> ---
> 
> `sudo bin/mesos-tests.sh 
> --gtest_filter='StorageLocalResourceProviderTest.ROOT_RetryOperationStatusUpdate'
>  --gtest_repeat=100 --gtest_break_on_failure` on GNU/Linux
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 65060: Cleaned up endpoint directories after SLRP tests.

2018-01-19 Thread Greg Mann 

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




src/tests/storage_local_resource_provider_tests.cpp
Lines 210-233 (patched)


As we discussed offline, it would be great if we can move this into the 
test fixture's `TearDown()` method.


- Greg Mann


On Jan. 10, 2018, 2:37 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65060/
> ---
> 
> (Updated Jan. 10, 2018, 2:37 a.m.)
> 
> 
> Review request for mesos, Gaston Kleiman, Greg Mann, and Jie Yu.
> 
> 
> Bugs: MESOS-8427
> https://issues.apache.org/jira/browse/MESOS-8427
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Cleaned up endpoint directories after SLRP tests.
> 
> 
> Diffs
> -
> 
>   src/tests/storage_local_resource_provider_tests.cpp 
> bbfe95e9818f25fdd5405db3ad2fe355e023f743 
> 
> 
> Diff: https://reviews.apache.org/r/65060/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 65182: Tested that agent resends unacknowledged op status updates on recovery.

2018-01-19 Thread Gaston Kleiman 

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

(Updated Jan. 19, 2018, 3:07 p.m.)


Review request for mesos, Chun-Hung Hsiao and Greg Mann.


Changes
---

Addressed feedback.


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


Repository: mesos


Description
---

This patch adds a test to verify that the agent resends unacknowledged
operation status updates after a recovery.


Diffs (updated)
-

  src/tests/storage_local_resource_provider_tests.cpp 
bbfe95e9818f25fdd5405db3ad2fe355e023f743 


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

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


Testing
---

`GLOG_v=1 sudo bin/mesos-tests.sh 
--gtest_filter='StorageLocalResourceProviderTest.ROOT_RetryOperationStatusUpdateAfterRecovery'
 --verbose --gtest_repeat=1000 --gtest_break_on_failure` on GNU/Linux


Thanks,

Gaston Kleiman

Re: Review Request 65057: Tested that op status updates dropped en route to master are resent.

2018-01-19 Thread Gaston Kleiman 

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

(Updated Jan. 19, 2018, 3:07 p.m.)


Review request for mesos, Chun-Hung Hsiao, Greg Mann, and Jie Yu.


Changes
---

Removed unnecessary decline.


Bugs: MESOS-8363 and MESOS-8420
https://issues.apache.org/jira/browse/MESOS-8363
https://issues.apache.org/jira/browse/MESOS-8420


Repository: mesos


Description
---

This patch adds
`StorageLocalResourceProviderTest.ROOT_RetryOperationStatusUpdate` that
verifies that operation status updates are resent to the master after
being dropped en route to it.


Diffs (updated)
-

  src/tests/storage_local_resource_provider_tests.cpp 
bbfe95e9818f25fdd5405db3ad2fe355e023f743 


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

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


Testing
---

`sudo bin/mesos-tests.sh 
--gtest_filter='StorageLocalResourceProviderTest.ROOT_RetryOperationStatusUpdate'
 --gtest_repeat=100 --gtest_break_on_failure` on GNU/Linux


Thanks,

Gaston Kleiman

Re: Review Request 65182: Tested that agent resends unacknowledged op status updates on recovery.

2018-01-19 Thread Greg Mann 

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


Fix it, then Ship it!





src/tests/storage_local_resource_provider_tests.cpp
Lines 1847 (patched)


Ditto here regarding the decline, as in parent review.



src/tests/storage_local_resource_provider_tests.cpp
Lines 1921-1923 (patched)


Hmm this seems potentially flaky. What if this expectation is registered 
before the update acknowledgement is processed? To avoid flakiness, we should 
probably capture the ACK and await on it before we make this call.


- Greg Mann


On Jan. 16, 2018, 9 p.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65182/
> ---
> 
> (Updated Jan. 16, 2018, 9 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Greg Mann.
> 
> 
> Bugs: MESOS-8362
> https://issues.apache.org/jira/browse/MESOS-8362
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a test to verify that the agent resends unacknowledged
> operation status updates after a recovery.
> 
> 
> Diffs
> -
> 
>   src/tests/storage_local_resource_provider_tests.cpp 
> bbfe95e9818f25fdd5405db3ad2fe355e023f743 
> 
> 
> Diff: https://reviews.apache.org/r/65182/diff/4/
> 
> 
> Testing
> ---
> 
> `GLOG_v=1 sudo bin/mesos-tests.sh 
> --gtest_filter='StorageLocalResourceProviderTest.ROOT_RetryOperationStatusUpdateAfterRecovery'
>  --verbose --gtest_repeat=1000 --gtest_break_on_failure` on GNU/Linux
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 61172: Added mesos.http and mesos.exceptions for CLI.

2018-01-19 Thread Mesos Reviewbot Windows 

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



PASS: Mesos patch 61172 was successfully built and tested.

Reviews applied: `['61172']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/61172

- Mesos Reviewbot Windows


On Jan. 19, 2018, 8:48 p.m., Eric Chung wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61172/
> ---
> 
> (Updated Jan. 19, 2018, 8:48 p.m.)
> 
> 
> Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added mesos.http and mesos.exceptions for CLI.
> 
> Part of MESOS-7310, this patch adds the mesos.http and mesos.exceptions
> modules, which provides a Resource class and its descendants for
> abstracting away common operations over http connectioins with JSON
> serialization.
> 
> updated patch according to klueska's recommendations
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/bootstrap c84e549e59bc7a07b5b70c24c63bc0c16881e442 
>   src/python/lib/mesos/exceptions.py PRE-CREATION 
>   src/python/lib/mesos/http.py PRE-CREATION 
>   src/python/lib/requirements.in e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
>   src/python/lib/tests/conftest.py PRE-CREATION 
>   src/python/lib/tests/test_exceptions.py PRE-CREATION 
>   src/python/lib/tests/test_http.py PRE-CREATION 
>   src/python/lib/tox.ini 8ad030d2dbf1fb7a04f1eaadb587462b4ef3f054 
>   support/pylint.config f74f553e238553bd6a6c06f4dd888cc5954a33eb 
> 
> 
> Diff: https://reviews.apache.org/r/61172/diff/10/
> 
> 
> Testing
> ---
> 
> install tox
> cd src/python/lib
> tox
> 
> 
> Thanks,
> 
> Eric Chung
> 
>

Re: Review Request 61172: Added mesos.http and mesos.exceptions for CLI.

2018-01-19 Thread Eric Chung 


> On Jan. 18, 2018, 1:59 p.m., Kevin Klues wrote:
> > src/python/lib/mesos/exceptions.py
> > Lines 27-33 (patched)
> > 
> >
> > I would prefer to do this at each call site instead of wrapping it up 
> > and hiding it in here. This will keep it consistent with the rest of the 
> > code base.
> > 
> > That said, I do like keeping track of the original exception. So 
> > something like the following would be better in my opinion:
> > 
> > ```
> > def __init__(self, message=None, original_exception=None):
> > self.original_exception = original_exception
> > super(MesosException, self).__init__(message)
> > ```
> > 
> > Is the name `original_exception` the best name here? Is there a more 
> > standard way of wrapping exceptions like this in python and keeping track 
> > of the original ones?

it seems there is no good solution for this use case in python 2, however it is 
supported natively in python 3 via exception chaining: 
https://www.python.org/dev/peps/pep-3134/ maybe another incentive for us to 
migrate. :)

just curious though, are you saying i shouldn't append the exception to the 
message?


- Eric


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


On Jan. 19, 2018, 8:48 p.m., Eric Chung wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61172/
> ---
> 
> (Updated Jan. 19, 2018, 8:48 p.m.)
> 
> 
> Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added mesos.http and mesos.exceptions for CLI.
> 
> Part of MESOS-7310, this patch adds the mesos.http and mesos.exceptions
> modules, which provides a Resource class and its descendants for
> abstracting away common operations over http connectioins with JSON
> serialization.
> 
> updated patch according to klueska's recommendations
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/bootstrap c84e549e59bc7a07b5b70c24c63bc0c16881e442 
>   src/python/lib/mesos/exceptions.py PRE-CREATION 
>   src/python/lib/mesos/http.py PRE-CREATION 
>   src/python/lib/requirements.in e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
>   src/python/lib/tests/conftest.py PRE-CREATION 
>   src/python/lib/tests/test_exceptions.py PRE-CREATION 
>   src/python/lib/tests/test_http.py PRE-CREATION 
>   src/python/lib/tox.ini 8ad030d2dbf1fb7a04f1eaadb587462b4ef3f054 
>   support/pylint.config f74f553e238553bd6a6c06f4dd888cc5954a33eb 
> 
> 
> Diff: https://reviews.apache.org/r/61172/diff/10/
> 
> 
> Testing
> ---
> 
> install tox
> cd src/python/lib
> tox
> 
> 
> Thanks,
> 
> Eric Chung
> 
>

Re: Review Request 61172: Added mesos.http and mesos.exceptions for CLI.

2018-01-19 Thread Eric Chung 

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

(Updated Jan. 19, 2018, 8:48 p.m.)


Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.


Repository: mesos


Description (updated)
---

Added mesos.http and mesos.exceptions for CLI.

Part of MESOS-7310, this patch adds the mesos.http and mesos.exceptions
modules, which provides a Resource class and its descendants for
abstracting away common operations over http connectioins with JSON
serialization.

updated patch according to klueska's recommendations


Diffs (updated)
-

  src/python/cli_new/bootstrap c84e549e59bc7a07b5b70c24c63bc0c16881e442 
  src/python/lib/mesos/exceptions.py PRE-CREATION 
  src/python/lib/mesos/http.py PRE-CREATION 
  src/python/lib/requirements.in e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
  src/python/lib/tests/conftest.py PRE-CREATION 
  src/python/lib/tests/test_exceptions.py PRE-CREATION 
  src/python/lib/tests/test_http.py PRE-CREATION 
  src/python/lib/tox.ini 8ad030d2dbf1fb7a04f1eaadb587462b4ef3f054 
  support/pylint.config f74f553e238553bd6a6c06f4dd888cc5954a33eb 


Diff: https://reviews.apache.org/r/61172/diff/10/

Changes: https://reviews.apache.org/r/61172/diff/9-10/


Testing
---

install tox
cd src/python/lib
tox


Thanks,

Eric Chung

Re: Review Request 65232: Resumed the clock if necessary when destroying test agent.

2018-01-19 Thread Greg Mann 


> On Jan. 19, 2018, 5:03 p.m., Benjamin Bannier wrote:
> > Thanks for this fix Greg. This will help remove some unsafe boilerplate 
> > from tests. We should probably follow up with a tech-debt ticket removing 
> > unnecessary clock resumes in tests.

Created https://issues.apache.org/jira/browse/MESOS-8466 to look for 
unnecessary calls to `Clock::resume()` in the tests.


- Greg


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


On Jan. 19, 2018, 5:36 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65232/
> ---
> 
> (Updated Jan. 19, 2018, 5:36 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Gaston Kleiman.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Because the `cgroups::destroy()` code path makes use of `delay()`,
> the clock must not be paused in order for the destructor of the
> test `Slave` to reliably destroy all remaining containers.
> 
> This patch updates the destructor to check if the clock is paused
> and, if so, resume it before destroying containers.
> 
> 
> Diffs
> -
> 
>   src/tests/cluster.cpp 568c9c7eda9d69e85bc32ce3259548b348ed9f25 
> 
> 
> Diff: https://reviews.apache.org/r/65232/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 65232: Resumed the clock if necessary when destroying test agent.

2018-01-19 Thread Greg Mann 

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

(Updated Jan. 19, 2018, 5:36 p.m.)


Review request for mesos, Benjamin Bannier and Gaston Kleiman.


Repository: mesos


Description
---

Because the `cgroups::destroy()` code path makes use of `delay()`,
the clock must not be paused in order for the destructor of the
test `Slave` to reliably destroy all remaining containers.

This patch updates the destructor to check if the clock is paused
and, if so, resume it before destroying containers.


Diffs (updated)
-

  src/tests/cluster.cpp 568c9c7eda9d69e85bc32ce3259548b348ed9f25 


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

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


Testing
---

make check


Thanks,

Greg Mann

Re: Review Request 64998: Added a SLRP test for CSI plugin restart.

2018-01-19 Thread Greg Mann 

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


Fix it, then Ship it!





src/slave/container_daemon_process.hpp
Lines 76-79 (patched)


Nit: one fewer newline here. I can fix this while committing.


- Greg Mann


On Jan. 18, 2018, 2:53 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64998/
> ---
> 
> (Updated Jan. 18, 2018, 2:53 a.m.)
> 
> 
> Review request for mesos, Gaston Kleiman, Greg Mann, and Jie Yu.
> 
> 
> Bugs: MESOS-8408
> https://issues.apache.org/jira/browse/MESOS-8408
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The test does the same as the `PublishResources` test, but it kills the
> CSI plugin contaier between each operation.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 191594b9d67f1aa1342efbaad5e7501004332d74 
>   src/slave/container_daemon.cpp d74fa5105c61a6cadfb2610790e559d387ca13a0 
>   src/slave/container_daemon_process.hpp PRE-CREATION 
>   src/tests/storage_local_resource_provider_tests.cpp 
> bbfe95e9818f25fdd5405db3ad2fe355e023f743 
> 
> 
> Diff: https://reviews.apache.org/r/64998/diff/6/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 65232: Resumed the clock if necessary when destroying test agent.

2018-01-19 Thread Benjamin Bannier 

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


Fix it, then Ship it!




Thanks for this fix Greg. This will help remove some unsafe boilerplate from 
tests. We should probably follow up with a tech-debt ticket removing 
unnecessary clock resumes in tests.


src/tests/cluster.cpp
Lines 117 (patched)


This would be the only symbol from `process` we pull into this scope. Let's 
stick to the current pattern of full names here for local consistency.



src/tests/cluster.cpp
Line 342 (original), 344 (patched)


Suggest to not touch this now, see above.

Here and below in `Master::start`.


- Benjamin Bannier


On Jan. 19, 2018, 1:40 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65232/
> ---
> 
> (Updated Jan. 19, 2018, 1:40 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Gaston Kleiman.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Because the `cgroups::destroy()` code path makes use of `delay()`,
> the clock must not be paused in order for the destructor of the
> test `Slave` to reliably destroy all remaining containers.
> 
> This patch updates the destructor to check if the clock is paused
> and, if so, resume it before destroying containers.
> 
> 
> Diffs
> -
> 
>   src/tests/cluster.cpp 568c9c7eda9d69e85bc32ce3259548b348ed9f25 
> 
> 
> Diff: https://reviews.apache.org/r/65232/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 65208: Prevented redundant descriptions when applying reviewboard reviews.

2018-01-19 Thread Armand Grillet 

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


Ship it!




Ship It!

- Armand Grillet


On Jan. 18, 2018, 10:53 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65208/
> ---
> 
> (Updated Jan. 18, 2018, 10:53 a.m.)
> 
> 
> Review request for mesos, Armand Grillet and Till Toenshoff.
> 
> 
> Bugs: MESOS-7979
> https://issues.apache.org/jira/browse/MESOS-7979
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Reviewboard's `rbt` can be configured to guess field when creating
> review requests. With that setting it e.g., uses the first line of a
> commit as description, and the remainder as description. If the commit
> message has no body it will use the summary as description as well,
> leading to redundant information in commit messages which committers
> usually remove again manually before committing.
> 
> This patch prevents reusing a reviewboard description which is
> identical to the summary when synthesising commit message with
> `apply_reviews.py`.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py 0ef28cb02bc65acfeb7ea6808f74e1620a8a85c4 
> 
> 
> Diff: https://reviews.apache.org/r/65208/diff/1/
> 
> 
> Testing
> ---
> 
> Applied a commit with redundant description; the commit message only contains 
> the reviewboard URL as body.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 65044: Added the v1 API 'GET_OPERATIONS' call for master and agent.

2018-01-19 Thread Jan Schlicht 

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

(Updated Jan. 19, 2018, 2:37 p.m.)


Review request for mesos, Benjamin Bannier, Gaston Kleiman, Greg Mann, and Jie 
Yu.


Changes
---

Addressed issues.


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


Repository: mesos


Description
---

The 'GET_OPERATIONS' call lists all operations known to master or agent.


Diffs (updated)
-

  include/mesos/agent/agent.proto 315820041990d28e5cdae71c8385d4551d56558c 
  include/mesos/master/master.proto 3e34634f3b864222d07374936c0d9a18269c2fbd 
  include/mesos/v1/agent/agent.proto 9e8b49defb83ffd4dd240ebb3a09b2dd3610ab2a 
  include/mesos/v1/master/master.proto 6759c3004e7e04c0044360b0c54f4438c2f6ec8a 
  src/master/http.cpp c489b6f525f157811549b2cc84addd9d85e87990 
  src/master/master.hpp 05136781b6b1539f37c283e8127e4bafb187a0d1 
  src/master/validation.cpp f0b86775b7919ba6aa4a73038edb78a0adca68f4 
  src/slave/http.hpp 1619bb77e3e7d5a6d387e4d4bb071d83ce914967 
  src/slave/http.cpp 77e711ceeb0e2613d629b5e21fd686f85dfad11a 
  src/slave/validation.cpp 0c2ccda177734cf3c47c0346ed34d20d58e7d932 
  src/tests/api_tests.cpp bdacc30be4dc8656a41a0c47d0e350d48e59ad94 


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

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


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 65090: Mesos flags related to ZooKeeper use SecurePathOrValue.

2018-01-19 Thread Mesos Reviewbot Windows 

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



FAIL: Failed to apply the dependent review: 65226.

Failed command: `python.exe .\support\apply-reviews.py -n -r 65226`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65090

Relevant logs:

- 
[apply-review-65226-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65090/logs/apply-review-65226-stdout.log):

```
error: patch failed: 3rdparty/stout/include/stout/flags/fetch.hpp:64
error: 3rdparty/stout/include/stout/flags/fetch.hpp: patch does not apply
error: patch failed: 3rdparty/stout/include/stout/flags/flag.hpp:100
error: 3rdparty/stout/include/stout/flags/flag.hpp: patch does not apply
error: patch failed: 3rdparty/stout/include/stout/flags/parse.hpp:25
error: 3rdparty/stout/include/stout/flags/parse.hpp: patch does not apply
```

- Mesos Reviewbot Windows


On Jan. 19, 2018, 2:05 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65090/
> ---
> 
> (Updated Jan. 19, 2018, 2:05 p.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Bugs: MESOS-8413
> https://issues.apache.org/jira/browse/MESOS-8413
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Up until now the Mesos master flag `--zk` as well as the Mesos agent
> flag `--master` would leak ZooKeeper authentication credentials in
> both logs and results for the `/flags` endpoint, if the credentials
> were part of the configuration url.
> 
> This patch prevents this leakage if a user decides to store the
> ZooKeeper url in a file and pass the file as a value to the flags
> mentioned above (using the preffix `file://`).
> 
> 
> Diffs
> -
> 
>   src/master/flags.hpp dabb414560f506787b6e821a27af623c8da44b11 
>   src/master/main.cpp f65ce637d77ce183f83b70dce6da8d0b4b8b8e71 
>   src/slave/flags.hpp 42c4861b5ecdc808d04bfe8f5c35572074fd2bdc 
>   src/slave/main.cpp f38fec6028ade0e0a51fd2cce7470c5c36e66396 
> 
> 
> Diff: https://reviews.apache.org/r/65090/diff/3/
> 
> 
> Testing
> ---
> 
> ```sh
> make -j12 check
> 
> # We don't seem to test flags in unit tests anywhere,
> # so additionally I ran:
> 
> docker pull zookeeper
> 
> cat < /tmp/$USER/zk-stack.yml
> version: '3.1'
> services:
>   zoo1:
> image: zookeeper
> restart: always
> hostname: zoo1
> ports:
>   - 2181:2181
> environment:
>   ZOO_MY_ID: 1
>   ZOO_SERVERS: server.1=0.0.0.0:2888:3888 server.2=zoo2:2888:3888 
> server.3=zoo3:2888:3888
>   zoo2:
> image: zookeeper
> restart: always
> hostname: zoo2
> ports:
>   - 2182:2181
> environment:
>   ZOO_MY_ID: 2
>   ZOO_SERVERS: server.1=zoo1:2888:3888 server.2=0.0.0.0:2888:3888 
> server.3=zoo3:2888:3888
>   zoo3:
> image: zookeeper
> restart: always
> hostname: zoo3
> ports:
>   - 2183:2181
> environment:
>   ZOO_MY_ID: 2
>   ZOO_SERVERS: server.1=zoo1:2888:3888 server.2=zoo2:2888:3888 
> server.3=0.0.0.0:2888:3888
> EOF
> 
> docker-compose -f /tmp/$USER/zk-stack.yml up
> 
> cd ${MESOS_BUILD_DIR}
> 
> # This command should fail to launch because there is no file zk.conf
> ./bin/mesos-master.sh \
> --work_dir=/tmp/$USER/mesos/master \
> --log_dir=/tmp/$USER/mesos/master/log \
> --ip=$PUBLIC_IP \
> --quorum=1 \
> --zk=file:///tmp/$USER/zk/zk.conf
> 
> cat < /tmp/$USER/zk/zk.conf
> zk://$PUBLIC_IP:2181,$PUBLIC_IP:2182,$PUBLIC_IP:2183/mesos
> EOF
> 
> ./bin/mesos-master.sh \
> --work_dir=/tmp/$USER/mesos/master \
> --log_dir=/tmp/$USER/mesos/master/log \
> --ip=$PUBLIC_IP \
> --quorum=1 \
> --zk=`cat /tmp/$USER/zk/zk.conf`  &
> 
> [[ $(http -b $PUBLIC_IP:5050/flags | jq -r '.flags.zk') == `cat 
> /tmp/$USER/zk/zk.conf` ]]
> 
> kill %1
> 
> 
> ./bin/mesos-master.sh \
> --work_dir=/tmp/$USER/mesos/master \
> --log_dir=/tmp/$USER/mesos/master/log \
> --ip=$PUBLIC_IP \
> --quorum=1 \
> --zk=file:///tmp/$USER/zk/zk.conf &
> 
> [[ $(http -b $PUBLIC_IP:5050/flags | jq -r '.flags.zk') == 
> "/tmp/$USER/zk/zk.conf" ]]
> 
> ./bin/mesos-agent.sh \
> --work_dir=/tmp/$USER/mesos/agent \
> --log_dir=/tmp/$USER/mesos/agent/log \
> --master=file:///tmp/$USER/zk/zk.conf &
> 
> [[ $(http -b $PUBLIC_IP:5051/flags | jq -r '.flags.master') == 
> "/tmp/$USER/zk/zk.conf" ]]
> 
> kill %2
> 
> ./bin/mesos-agent.sh \
> --work_dir=/tmp/$USER/mesos/agent \
> --log_dir=/tmp/$USER/mesos/agent/log \
> --zk=`cat /tmp/$USER/zk/zk.conf`  &
> 
> [[ $(http -b $PUBLIC_IP:5051/flags | jq -r '.flags.master') == `cat 
> /tmp/$USER/zk/zk.conf` ]]
> 
> kill %2
> kill %1
> ```
> 
>

Re: Review Request 65226: Added SecurePathOrValue for file flags which need not to leak contents.

2018-01-19 Thread Alexander Rojas 

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

(Updated Jan. 19, 2018, 2:06 p.m.)


Review request for mesos and Greg Mann.


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


Repository: mesos


Description
---

The default behavior when serializing a flag, if its value comes from
a file (by using the prefix `file://`) is to show the contents of
the file instead of the path to the file.

A common pattern for flags containing sensitive information is to
store the values in a secure file, however the default behavior
of serializing flags causes these values to be leaked when
serializing a subclass of `FlagsBase`.

This patch introduces the structure `SecurePathOrValue` so that
the default behavior is overriden and the path of the flag will
be shown instead of the flag value itself.


Diffs (updated)
-

  3rdparty/stout/include/stout/flags/fetch.hpp 
788d09e0a2083ead7d6ccb0c233cfdfd5d1ddb81 
  3rdparty/stout/include/stout/flags/flag.hpp 
6580c294fc5d06854eadc19b1515172a4a700e89 
  3rdparty/stout/include/stout/flags/parse.hpp 
6112d699be4cbdcb7a7872da8f29d41aab0939a3 


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

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


Testing
---

```
make check
```

Full testing in follow up patch.


Thanks,

Alexander Rojas

Re: Review Request 65090: Mesos flags related to ZooKeeper use SecurePathOrValue.

2018-01-19 Thread Alexander Rojas 

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

(Updated Jan. 19, 2018, 2:05 p.m.)


Review request for mesos and Greg Mann.


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


Repository: mesos


Description (updated)
---

Up until now the Mesos master flag `--zk` as well as the Mesos agent
flag `--master` would leak ZooKeeper authentication credentials in
both logs and results for the `/flags` endpoint, if the credentials
were part of the configuration url.

This patch prevents this leakage if a user decides to store the
ZooKeeper url in a file and pass the file as a value to the flags
mentioned above (using the preffix `file://`).


Diffs (updated)
-

  src/master/flags.hpp dabb414560f506787b6e821a27af623c8da44b11 
  src/master/main.cpp f65ce637d77ce183f83b70dce6da8d0b4b8b8e71 
  src/slave/flags.hpp 42c4861b5ecdc808d04bfe8f5c35572074fd2bdc 
  src/slave/main.cpp f38fec6028ade0e0a51fd2cce7470c5c36e66396 


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

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


Testing
---

```sh
make -j12 check

# We don't seem to test flags in unit tests anywhere,
# so additionally I ran:

docker pull zookeeper

cat < /tmp/$USER/zk-stack.yml
version: '3.1'
services:
  zoo1:
image: zookeeper
restart: always
hostname: zoo1
ports:
  - 2181:2181
environment:
  ZOO_MY_ID: 1
  ZOO_SERVERS: server.1=0.0.0.0:2888:3888 server.2=zoo2:2888:3888 
server.3=zoo3:2888:3888
  zoo2:
image: zookeeper
restart: always
hostname: zoo2
ports:
  - 2182:2181
environment:
  ZOO_MY_ID: 2
  ZOO_SERVERS: server.1=zoo1:2888:3888 server.2=0.0.0.0:2888:3888 
server.3=zoo3:2888:3888
  zoo3:
image: zookeeper
restart: always
hostname: zoo3
ports:
  - 2183:2181
environment:
  ZOO_MY_ID: 2
  ZOO_SERVERS: server.1=zoo1:2888:3888 server.2=zoo2:2888:3888 
server.3=0.0.0.0:2888:3888
EOF

docker-compose -f /tmp/$USER/zk-stack.yml up

cd ${MESOS_BUILD_DIR}

# This command should fail to launch because there is no file zk.conf
./bin/mesos-master.sh \
--work_dir=/tmp/$USER/mesos/master \
--log_dir=/tmp/$USER/mesos/master/log \
--ip=$PUBLIC_IP \
--quorum=1 \
--zk=file:///tmp/$USER/zk/zk.conf

cat < /tmp/$USER/zk/zk.conf
zk://$PUBLIC_IP:2181,$PUBLIC_IP:2182,$PUBLIC_IP:2183/mesos
EOF

./bin/mesos-master.sh \
--work_dir=/tmp/$USER/mesos/master \
--log_dir=/tmp/$USER/mesos/master/log \
--ip=$PUBLIC_IP \
--quorum=1 \
--zk=`cat /tmp/$USER/zk/zk.conf`  &

[[ $(http -b $PUBLIC_IP:5050/flags | jq -r '.flags.zk') == `cat 
/tmp/$USER/zk/zk.conf` ]]

kill %1


./bin/mesos-master.sh \
--work_dir=/tmp/$USER/mesos/master \
--log_dir=/tmp/$USER/mesos/master/log \
--ip=$PUBLIC_IP \
--quorum=1 \
--zk=file:///tmp/$USER/zk/zk.conf &

[[ $(http -b $PUBLIC_IP:5050/flags | jq -r '.flags.zk') == 
"/tmp/$USER/zk/zk.conf" ]]

./bin/mesos-agent.sh \
--work_dir=/tmp/$USER/mesos/agent \
--log_dir=/tmp/$USER/mesos/agent/log \
--master=file:///tmp/$USER/zk/zk.conf &

[[ $(http -b $PUBLIC_IP:5051/flags | jq -r '.flags.master') == 
"/tmp/$USER/zk/zk.conf" ]]

kill %2

./bin/mesos-agent.sh \
--work_dir=/tmp/$USER/mesos/agent \
--log_dir=/tmp/$USER/mesos/agent/log \
--zk=`cat /tmp/$USER/zk/zk.conf`  &

[[ $(http -b $PUBLIC_IP:5051/flags | jq -r '.flags.master') == `cat 
/tmp/$USER/zk/zk.conf` ]]

kill %2
kill %1
```


Thanks,

Alexander Rojas