Re: Review Request 61901: Added common validation for Volume.

[Joseph Wu]

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


Ship it!




Ship It!

- Joseph Wu


On Aug. 28, 2017, 8:28 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61901/
> ---
> 
> (Updated Aug. 28, 2017, 8:28 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Joseph Wu.
> 
> 
> Bugs: MESOS-7306
> https://issues.apache.org/jira/browse/MESOS-7306
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added common validation for Volume.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 5157b2baac4ff6c4493faaeb7a311381e8e76fe7 
>   src/common/validation.hpp 8c92436eb6c8aa87ffd6d6cf3f42079d6c90e525 
>   src/common/validation.cpp 33b501bfa6613dab70a663f552a424ffa8a15b0a 
>   src/tests/CMakeLists.txt 6c05ea93c3288fb9608c191234f95e270390d89e 
>   src/tests/common_validation_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/61901/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 61189: Added authorization for V1 events.

[Mesos Reviewbot Windows]

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



Failed to apply patch!

Reviews applied: [61262, 61189]

Logs available here: http://104.210.40.105/logs/master/61189

- Mesos Reviewbot Windows


On Aug. 19, 2017, 3:33 a.m., Quinn Leng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61189/
> ---
> 
> (Updated Aug. 19, 2017, 3:33 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Alexander Rojas, and Greg Mann.
> 
> 
> Bugs: MESOS-7785
> https://issues.apache.org/jira/browse/MESOS-7785
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added authorization filtering for the V1 operator events on the
> master, the subscriber should only receive events that are
> authorized based on their principal and ACLs. Since the authorizer
> is called for every event emitted on the stream, the change of
> authorization rule will affect events that the subscribers can
> receive.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp b09a9d0ab5fab179d0e400932dce7d3ca958d7a8 
>   src/master/master.hpp d9cfc42646c874661e4ec79322126d93a2caf604 
>   src/master/master.cpp 7f38a5e21884546d4b4c866ca5918db779af8f99 
>   src/tests/api_tests.cpp 34480ea6818c904c64cb678562866f0b43dae0d6 
> 
> 
> Diff: https://reviews.apache.org/r/61189/diff/6/
> 
> 
> Testing
> ---
> 
> make check
> GLOG_v=2 ./bin/mesos-tests.sh 
> --gtest_filter="ContentType/MasterAPITest.EventsAuthorizationFiltering*" 
> --verbose --gtest_repeat=100 --gtest_break_on_failure
> 
> 
> Thanks,
> 
> Quinn Leng
> 
>

Re: Review Request 61189: Added authorization for V1 events.

[Quinn Leng]

> On 八月 23, 2017, 12:29 a.m., Greg Mann wrote:
> > src/tests/api_tests.cpp
> > Lines 2214 (patched)
> > 
> >
> > Since you don't modify these flags at all, you can get rid of this. If 
> > you start the master without a flags parameter, it will use the flags 
> > produced by `CreateMasterFlags()` by default.

This masterFlags because we are using the "masterFlags.allocation_interval" in 
later section.


> On 八月 23, 2017, 12:29 a.m., Greg Mann wrote:
> > src/tests/api_tests.cpp
> > Lines  (patched)
> > 
> >
> > Can you move this down where you register the `connected` expectation 
> > on the scheduler?

I don't quite understand it, this is pretty close to the connected expectation, 
why we need to move it?


- Quinn


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


On 八月 19, 2017, 3:33 a.m., Quinn Leng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61189/
> ---
> 
> (Updated 八月 19, 2017, 3:33 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Alexander Rojas, and Greg Mann.
> 
> 
> Bugs: MESOS-7785
> https://issues.apache.org/jira/browse/MESOS-7785
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added authorization filtering for the V1 operator events on the
> master, the subscriber should only receive events that are
> authorized based on their principal and ACLs. Since the authorizer
> is called for every event emitted on the stream, the change of
> authorization rule will affect events that the subscribers can
> receive.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp b09a9d0ab5fab179d0e400932dce7d3ca958d7a8 
>   src/master/master.hpp d9cfc42646c874661e4ec79322126d93a2caf604 
>   src/master/master.cpp 7f38a5e21884546d4b4c866ca5918db779af8f99 
>   src/tests/api_tests.cpp 34480ea6818c904c64cb678562866f0b43dae0d6 
> 
> 
> Diff: https://reviews.apache.org/r/61189/diff/5/
> 
> 
> Testing
> ---
> 
> make check
> GLOG_v=2 ./bin/mesos-tests.sh 
> --gtest_filter="ContentType/MasterAPITest.EventsAuthorizationFiltering*" 
> --verbose --gtest_repeat=100 --gtest_break_on_failure
> 
> 
> Thanks,
> 
> Quinn Leng
> 
>

Re: Review Request 61189: Added authorization for V1 events.

[Quinn Leng]

> On 八月 16, 2017, 9:21 a.m., Alexander Rojas wrote:
> > Can you also update the summary to mention that it only affects the 
> > `SUBSCRIBE` event.

The authorization filtering does affect othre events. And I have updated the 
summary to make it more specific, thanks.


- Quinn


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


On 八月 19, 2017, 3:33 a.m., Quinn Leng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61189/
> ---
> 
> (Updated 八月 19, 2017, 3:33 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Alexander Rojas, and Greg Mann.
> 
> 
> Bugs: MESOS-7785
> https://issues.apache.org/jira/browse/MESOS-7785
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added authorization filtering for the V1 operator events on the
> master, the subscriber should only receive events that are
> authorized based on their principal and ACLs. Since the authorizer
> is called for every event emitted on the stream, the change of
> authorization rule will affect events that the subscribers can
> receive.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp b09a9d0ab5fab179d0e400932dce7d3ca958d7a8 
>   src/master/master.hpp d9cfc42646c874661e4ec79322126d93a2caf604 
>   src/master/master.cpp 7f38a5e21884546d4b4c866ca5918db779af8f99 
>   src/tests/api_tests.cpp 34480ea6818c904c64cb678562866f0b43dae0d6 
> 
> 
> Diff: https://reviews.apache.org/r/61189/diff/5/
> 
> 
> Testing
> ---
> 
> make check
> GLOG_v=2 ./bin/mesos-tests.sh 
> --gtest_filter="ContentType/MasterAPITest.EventsAuthorizationFiltering*" 
> --verbose --gtest_repeat=100 --gtest_break_on_failure
> 
> 
> Thanks,
> 
> Quinn Leng
> 
>

Re: Review Request 61901: Added common validation for Volume.

[Jie Yu]

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

(Updated Aug. 29, 2017, 3:28 a.m.)


Review request for mesos, Gilbert Song and Joseph Wu.


Changes
---

addressed review comments.


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


Repository: mesos


Description
---

Added common validation for Volume.


Diffs (updated)
-

  src/Makefile.am 5157b2baac4ff6c4493faaeb7a311381e8e76fe7 
  src/common/validation.hpp 8c92436eb6c8aa87ffd6d6cf3f42079d6c90e525 
  src/common/validation.cpp 33b501bfa6613dab70a663f552a424ffa8a15b0a 
  src/tests/CMakeLists.txt 6c05ea93c3288fb9608c191234f95e270390d89e 
  src/tests/common_validation_tests.cpp PRE-CREATION 


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

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


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 60890: WIP: Defined API for launching standalone containers.

[Jie Yu]

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




include/mesos/agent/agent.proto
Line 65 (original), 67 (patched)


We should deprecate this as well. `REMOVE_CONTAINER` will be useful for 
standalone container as well (to remove the runtime dir which won't be deleted 
until this call).


- Jie Yu


On Aug. 29, 2017, 12:26 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60890/
> ---
> 
> (Updated Aug. 29, 2017, 12:26 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-7305
> https://issues.apache.org/jira/browse/MESOS-7305
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Launching a standalone container is very similar to launching a
> nested container, except that the caller must specify some Resources.
> As such, this patch deprecates some nested container APIs
> and replaces them with more generically named APIs.
> 
> This applies to the Launch, Wait, and Kill (nested) container APIs.
> 
> 
> Diffs
> -
> 
>   include/mesos/agent/agent.proto 7c8c8a7d8298e91e4e002327b3b27d4c74b5cbae 
>   include/mesos/v1/agent/agent.proto 3e199124b23fa027232790d99370fe2f33660096 
> 
> 
> Diff: https://reviews.apache.org/r/60890/diff/2/
> 
> 
> Testing
> ---
> 
> make
> 
> See later patches in chain.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 61901: Added common validation for Volume.

[Jie Yu]

> On Aug. 28, 2017, 7:11 p.m., Joseph Wu wrote:
> > src/common/validation.cpp
> > Lines 206 (patched)
> > 
> >
> > Seems like you could start with adding this:
> > ```
> > if (!path::absolute(volume.host_path()) &&
> > !path::absolute(volume.container_path())) {
> >   ...
> > }
> > ```
> > 
> > (Some form of which exists in the filesystem/linux isolator and Docker 
> > containerizer)

I'll do that later.


> On Aug. 28, 2017, 7:11 p.m., Joseph Wu wrote:
> > src/common/validation.cpp
> > Lines 212-217 (patched)
> > 
> >
> > Might be clearer to count the number of occurrences rather than 
> > bit-mask them.
> > 
> > Then error if the number of occurrences is not equal to one.
> > 
> > ---
> > 
> > Also, I'm surprised that we don't have this validation already.  
> > 
> > It seems like, in the existing code, if you:
> > 1) Specify some value for all three fields.
> > 2) Enable the appropriate isolators (filesystem/linux, docker/volume, 
> > volume/sandbox_path).
> > 3) All three isolators will do their thing, possibly running over each 
> > other in the process.

Used occurrences! thanks for the tip.


- Jie


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


On Aug. 24, 2017, 11:58 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61901/
> ---
> 
> (Updated Aug. 24, 2017, 11:58 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Joseph Wu.
> 
> 
> Bugs: MESOS-7306
> https://issues.apache.org/jira/browse/MESOS-7306
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added common validation for Volume.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 5157b2baac4ff6c4493faaeb7a311381e8e76fe7 
>   src/common/validation.hpp 8c92436eb6c8aa87ffd6d6cf3f42079d6c90e525 
>   src/common/validation.cpp 33b501bfa6613dab70a663f552a424ffa8a15b0a 
>   src/tests/CMakeLists.txt 6c05ea93c3288fb9608c191234f95e270390d89e 
>   src/tests/common_validation_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/61901/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 60890: WIP: Defined API for launching standalone containers.

[Joseph Wu]

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

(Updated Aug. 28, 2017, 5:26 p.m.)


Review request for mesos, Gilbert Song and Jie Yu.


Changes
---

Standalone container API and (more importantly) implementation is almost 
identical to the nested container API.  Hence, we can feasibly combine parts of 
their APIs.


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


Repository: mesos


Description (updated)
---

Launching a standalone container is very similar to launching a
nested container, except that the caller must specify some Resources.
As such, this patch deprecates some nested container APIs
and replaces them with more generically named APIs.

This applies to the Launch, Wait, and Kill (nested) container APIs.


Diffs (updated)
-

  include/mesos/agent/agent.proto 7c8c8a7d8298e91e4e002327b3b27d4c74b5cbae 
  include/mesos/v1/agent/agent.proto 3e199124b23fa027232790d99370fe2f33660096 


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

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


Testing
---

make

See later patches in chain.


Thanks,

Joseph Wu

Re: Review Request 61927: MESOS-7912 In strict mode code, functions can only be declared at top level

[Benjamin Mahler]

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


Ship it!




Ship It!

- Benjamin Mahler


On Aug. 26, 2017, 1:14 p.m., Alastair Montgomery wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61927/
> ---
> 
> (Updated Aug. 26, 2017, 1:14 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-7912
> https://issues.apache.org/jira/browse/MESOS-7912
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix for https://issues.apache.org/jira/browse/MESOS-7912
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/js/controllers.js 27456144c 
> 
> 
> Diff: https://reviews.apache.org/r/61927/diff/1/
> 
> 
> Testing
> ---
> 
> Tested in Chrome Windows Version Version 37.0.2062.102 m and Mesos UI is now 
> displaying correctly where before it was throwing the strict error.
> 
> 
> Thanks,
> 
> Alastair Montgomery
> 
>

Re: Review Request 61899: Added HOST_PATH volume source protobuf.

[Joseph Wu]

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


Ship it!




Ship It!

- Joseph Wu


On Aug. 24, 2017, 4:57 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61899/
> ---
> 
> (Updated Aug. 24, 2017, 4:57 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Joseph Wu.
> 
> 
> Bugs: MESOS-7306
> https://issues.apache.org/jira/browse/MESOS-7306
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added HOST_PATH volume source protobuf.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 86936cf9ad9406fa4c3358ccb3c58e2f3259d65f 
>   include/mesos/v1/mesos.proto 20d9ecff68465c6517a5a239a4bd0f42f82d0cc6 
> 
> 
> Diff: https://reviews.apache.org/r/61899/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 61900: Added equal operator for Error.

[Joseph Wu]

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


Ship it!




Ship It!

- Joseph Wu


On Aug. 24, 2017, 4:58 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61900/
> ---
> 
> (Updated Aug. 24, 2017, 4:58 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Joseph Wu.
> 
> 
> Bugs: MESOS-7306
> https://issues.apache.org/jira/browse/MESOS-7306
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added equal operator for Error.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/errorbase.hpp 
> 2118b8ed7005f34f23e90d05a7245c2786e37893 
> 
> 
> Diff: https://reviews.apache.org/r/61900/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 61473: Do not kill non partition aware tasks.

[Mesos Reviewbot Windows]

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



Failed to apply patch!

Reviews applied: [61473]

Logs available here: http://104.210.40.105/logs/master/61473

- Mesos Reviewbot Windows


On Aug. 28, 2017, 8:31 a.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61473/
> ---
> 
> (Updated Aug. 28, 2017, 8:31 a.m.)
> 
> 
> Review request for mesos, Vinod Kone and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7215
> https://issues.apache.org/jira/browse/MESOS-7215
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Master will not kill the tasks for non-Partition aware frameworks
> when an unreachable agent re-registers with the master.
> Master used to send a ShutdownFrameworkMessages to the agent
> to kill the tasks from non partition aware frameworks including the
> ones that are still registered which was problematic because the offer
> from this agent could still go to the same framework which could then
> launch new tasks. The agent would then receive tasks of the same
> framework and ignore them because it thinks the framework is shutting
> down. The framework is not shutting down of course, so from the master
> and the scheduler’s perspective the task is pending in STAGING forever
> until the next agent reregistration, which could happen much later.
> This commit fixes the problem by not shutting down the non-partition
> aware frameworks on such an agent.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp ef4decc40062458dd3783eb9f16abec53a7af79f 
>   src/master/master.hpp d7c67d06ada4f77fb424ea06bb6e91893a6632ce 
>   src/master/master.cpp a9d2191d92d0a56c8a137cb28f41a3632db54fe9 
>   src/tests/partition_tests.cpp 0886f4890ac3fec6f38146946892769a99c3e68f 
> 
> 
> Diff: https://reviews.apache.org/r/61473/diff/6/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Megha Sharma
> 
>

Re: Review Request 61473: Do not kill non partition aware tasks.

[Mesos Reviewbot]

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



Bad patch!

Reviews applied: [61473]

Failed command: python support/apply-reviews.py -n -r 61473

Error:
2017-08-28 23:34:26 URL:https://reviews.apache.org/r/61473/diff/raw/ 
[15375/15375] -> "61473.patch" [1]
Traceback (most recent call last):
  File "support/apply-reviews.py", line 417, in 
main()
  File "support/apply-reviews.py", line 412, in main
reviewboard(options)
  File "support/apply-reviews.py", line 402, in reviewboard
apply_review(options)
  File "support/apply-reviews.py", line 160, in apply_review
commit_patch(options)
  File "support/apply-reviews.py", line 261, in commit_patch
message.write(data['message'])
UnicodeEncodeError: 'ascii' codec can't encode character u'\u2019' in position 
655: ordinal not in range(128)

Full log: https://builds.apache.org/job/Mesos-Reviewbot/19104/console

- Mesos Reviewbot


On Aug. 28, 2017, 11:31 a.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61473/
> ---
> 
> (Updated Aug. 28, 2017, 11:31 a.m.)
> 
> 
> Review request for mesos, Vinod Kone and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7215
> https://issues.apache.org/jira/browse/MESOS-7215
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Master will not kill the tasks for non-Partition aware frameworks
> when an unreachable agent re-registers with the master.
> Master used to send a ShutdownFrameworkMessages to the agent
> to kill the tasks from non partition aware frameworks including the
> ones that are still registered which was problematic because the offer
> from this agent could still go to the same framework which could then
> launch new tasks. The agent would then receive tasks of the same
> framework and ignore them because it thinks the framework is shutting
> down. The framework is not shutting down of course, so from the master
> and the scheduler’s perspective the task is pending in STAGING forever
> until the next agent reregistration, which could happen much later.
> This commit fixes the problem by not shutting down the non-partition
> aware frameworks on such an agent.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp ef4decc40062458dd3783eb9f16abec53a7af79f 
>   src/master/master.hpp d7c67d06ada4f77fb424ea06bb6e91893a6632ce 
>   src/master/master.cpp a9d2191d92d0a56c8a137cb28f41a3632db54fe9 
>   src/tests/partition_tests.cpp 0886f4890ac3fec6f38146946892769a99c3e68f 
> 
> 
> Diff: https://reviews.apache.org/r/61473/diff/6/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Megha Sharma
> 
>

Re: Review Request 61929: Parameterized the volume host path tests on executor type.

[Joseph Wu]

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


Ship it!




None of our other test parameterization goes this far, but this certainly looks 
more neat than parameterizing on a `bool` or `string`.

- Joseph Wu


On Aug. 26, 2017, 1:21 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61929/
> ---
> 
> (Updated Aug. 26, 2017, 1:21 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Joseph Wu.
> 
> 
> Bugs: MESOS-7306
> https://issues.apache.org/jira/browse/MESOS-7306
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This allows us to test the isolator when the container is a normal
> container (command executor) or a nested container (default executor).
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/volume_host_path_isolator_tests.cpp PRE-CREATION 
>   src/tests/mesos.hpp f80e5fb0b76146e4748192361e8d1feff1ed687e 
> 
> 
> Diff: https://reviews.apache.org/r/61929/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 61666: Added test to verify filtering of resource reservations & allocations.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [60915, 61666]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


On Aug. 15, 2017, 5:59 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61666/
> ---
> 
> (Updated Aug. 15, 2017, 5:59 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Greg Mann.
> 
> 
> Bugs: MESOS-7892
> https://issues.apache.org/jira/browse/MESOS-7892
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch extends `SlaveAuthorizerTest.FilterStateEndpoint` test to
> check that agent's `/state` endpoint shows resource reservations and
> allocations based on the 'VIEW_ROLE' permissions of the principal
> doing the request.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_authorization_tests.cpp 
> 536a8efda0cc1ba08285c379b01af6ec64a82117 
> 
> 
> Diff: https://reviews.apache.org/r/61666/diff/3/
> 
> 
> Testing
> ---
> 
> make check (mac os x, fedora 25)
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 60622: Add new stout function: path::uri (convert filename to valid URI).

[Jeff Coffler]

> On Aug. 15, 2017, 6:04 p.m., Andrew Schwartzmeyer wrote:
> > 3rdparty/stout/include/stout/path.hpp
> > Lines 77 (patched)
> > 
> >
> > If this is the case, what are we doing on Linux if the path has 
> > backslashes in it?
> > 
> > I recognize this wasn't already previously handled, but it's a good 
> > question. Should this function be normalizing as well?

On Linux, backslashes are a valid part of the filename. We can't just 
"normalize" this was that makes the filename fundamentally different. You can 
percent-encode a URI to contain a '\' character, but then this would imply that 
we have a URI decode function, and today we don't have that in any common form.

This was always a problem on Linux - my changes don't affect that one way or 
the other. If this is a problem for customers, we should create a separate bug 
for this and fix it separately from these changes.


- Jeff


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


On July 3, 2017, 7:30 p.m., Jeff Coffler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60622/
> ---
> 
> (Updated July 3, 2017, 7:30 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and 
> Li Li.
> 
> 
> Bugs: MESOS-6705
> https://issues.apache.org/jira/browse/MESOS-6705
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add new stout function: path::uri (convert filename to valid URI).
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/path.hpp 
> 6ee3a44cd6a878fe383aa68df40b82857b93d0b4 
>   3rdparty/stout/tests/path_tests.cpp 
> f8c14d5aefe0b49adb778da784143a328c96183d 
> 
> 
> Diff: https://reviews.apache.org/r/60622/diff/1/
> 
> 
> Testing
> ---
> 
> See upstream
> 
> 
> Thanks,
> 
> Jeff Coffler
> 
>

Re: Review Request 61919: Adjusted the test helpers for creating host and sandbox path volumes.

[Joseph Wu]

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


Ship it!




Ship It!

- Joseph Wu


On Aug. 25, 2017, 3:29 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61919/
> ---
> 
> (Updated Aug. 25, 2017, 3:29 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Joseph Wu.
> 
> 
> Bugs: MESOS-7306
> https://issues.apache.org/jira/browse/MESOS-7306
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, we used the same helper for creating both host and sandbox
> path volumes. This patch split that into two separate helpers.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/linux_filesystem_isolator_tests.cpp 
> 77131870ccfec0ec426962e56dc4f2cd35433946 
>   src/tests/containerizer/volume_host_path_isolator_tests.cpp PRE-CREATION 
>   src/tests/mesos.hpp f80e5fb0b76146e4748192361e8d1feff1ed687e 
> 
> 
> Diff: https://reviews.apache.org/r/61919/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 61915: Enabled `DockerContainerizerProcess::usage` for all platforms.

[Andrew Schwartzmeyer]

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




src/slave/containerizer/docker.cpp
Lines 1869-1870 (patched)


Ah, sorry, went to update it and saw it as submitted. Thanks for fixing :)


- Andrew Schwartzmeyer


On Aug. 25, 2017, 1:32 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61915/
> ---
> 
> (Updated Aug. 25, 2017, 1:32 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Joseph Wu, Kevin Klues, and Li Li.
> 
> 
> Bugs: MESOS-7917
> https://issues.apache.org/jira/browse/MESOS-7917
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When originally implemented this code was enabled only for Linux.
> However, this code should work anywhere Docker is being used, as it
> obtains the information by shelling out to the `docker` executable.
> 
> This patch fixes the resources information provided by the `mesos-agent`
> on Windows platforms for Docker containers by making the `statistics`
> JSON object available.
> 
> Note that the `cgroupsStatistics` is Linux specific, and so the
> pre-processor guard was not removed but reduced in scope for just it.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp cbcff39a5d8398f71570849615aeb4368130b433 
> 
> 
> Diff: https://reviews.apache.org/r/61915/diff/1/
> 
> 
> Testing
> ---
> 
> Built on Windows, started a Windows Docker container, observed the 
> `statistics` field from `/container` being populated where it was 
> not previously included.
> 
> `make check` passed on CentOS 7.
> 
> Diagnosing the Windows tests, have a machine configuration with Docker 
> containers right now.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 61908: Used path::absolute to replace some startsWith checks.

[Joseph Wu]

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


Ship it!




Oh, didn't notice this one when I commented on an earlier patch.
I'd be OK with keeping this separate.  Squashing would be fine too.

- Joseph Wu


On Aug. 24, 2017, 5:01 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61908/
> ---
> 
> (Updated Aug. 24, 2017, 5:01 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Joseph Wu.
> 
> 
> Bugs: MESOS-7306
> https://issues.apache.org/jira/browse/MESOS-7306
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used path::absolute to replace some startsWith checks.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
> d7fe9a8f43ab6436fc1e202443e71cf6b7e2d0d3 
> 
> 
> Diff: https://reviews.apache.org/r/61908/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 61907: Moved host volume related tests to a dedicated file.

[Joseph Wu]

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


Ship it!




Ship It!

- Joseph Wu


On Aug. 24, 2017, 5 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61907/
> ---
> 
> (Updated Aug. 24, 2017, 5 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Joseph Wu.
> 
> 
> Bugs: MESOS-7306
> https://issues.apache.org/jira/browse/MESOS-7306
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Moved host volume related tests to a dedicated file.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 5157b2baac4ff6c4493faaeb7a311381e8e76fe7 
>   src/tests/CMakeLists.txt 6c05ea93c3288fb9608c191234f95e270390d89e 
>   src/tests/containerizer/linux_filesystem_isolator_tests.cpp 
> 77131870ccfec0ec426962e56dc4f2cd35433946 
>   src/tests/containerizer/volume_host_path_isolator_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/61907/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 61952: Fixed the communication between old masters and new agents.

[Greg Mann]

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



Should we also update the upgrade guide in the documentation with information 
about reservation refinement in the context of upgrades?

- Greg Mann


On Aug. 28, 2017, 8:46 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61952/
> ---
> 
> (Updated Aug. 28, 2017, 8:46 p.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Bugs: MESOS-7922
> https://issues.apache.org/jira/browse/MESOS-7922
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> For re-registration, 1.4 agents used to send the resources in tasks
> and executors to the master in the "post-reservation-refinement" format,
> which is incompatible for pre-1.4 masters. This patch changes the agent
> such that it always downgrades the resources to
> the "pre-reservation-refinement" format, and the master unconditionally
> upgrades the resources to "post-reservation-refinement" format.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp a9d2191d92d0a56c8a137cb28f41a3632db54fe9 
>   src/slave/slave.cpp 3e880b3ad54dbad46f71c335c2a93edb9a546176 
> 
> 
> Diff: https://reviews.apache.org/r/61952/diff/3/
> 
> 
> Testing
> ---
> 
> - Started 1.3.1 master, agent.
> - Ran a persistent volume framework with the task commands changed to have 
> `sleep 100`.
> - Upgraded the agent from 1.3.1 to `master`.
> - Observed the following message:
> > W0828 12:48:22.066509 40596 master.cpp:5702] Dropping re-registration of 
> > agent
> at slave(1)@10.0.49.2:5051 because it sent an invalid re-registration: Task 
> uses
> invalid resources: Invalid DiskInfo: Persistent volumes cannot be created from
> unreserved resources
> 
> ---
> 
> - Started 1.3.1 master, agent.
> - Ran a persistent volume framework with the task commands changed to have 
> `sleep 100`.
> - Upgraded the agent from 1.3.1 to `master` + this patch.
> - The agent was able to register successfully.
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 61952: Fixed the communication between old masters and new agents.

[Greg Mann]

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


Ship it!




Ship It!

- Greg Mann


On Aug. 28, 2017, 8:46 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61952/
> ---
> 
> (Updated Aug. 28, 2017, 8:46 p.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Bugs: MESOS-7922
> https://issues.apache.org/jira/browse/MESOS-7922
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> For re-registration, 1.4 agents used to send the resources in tasks
> and executors to the master in the "post-reservation-refinement" format,
> which is incompatible for pre-1.4 masters. This patch changes the agent
> such that it always downgrades the resources to
> the "pre-reservation-refinement" format, and the master unconditionally
> upgrades the resources to "post-reservation-refinement" format.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp a9d2191d92d0a56c8a137cb28f41a3632db54fe9 
>   src/slave/slave.cpp 3e880b3ad54dbad46f71c335c2a93edb9a546176 
> 
> 
> Diff: https://reviews.apache.org/r/61952/diff/3/
> 
> 
> Testing
> ---
> 
> - Started 1.3.1 master, agent.
> - Ran a persistent volume framework with the task commands changed to have 
> `sleep 100`.
> - Upgraded the agent from 1.3.1 to `master`.
> - Observed the following message:
> > W0828 12:48:22.066509 40596 master.cpp:5702] Dropping re-registration of 
> > agent
> at slave(1)@10.0.49.2:5051 because it sent an invalid re-registration: Task 
> uses
> invalid resources: Invalid DiskInfo: Persistent volumes cannot be created from
> unreserved resources
> 
> ---
> 
> - Started 1.3.1 master, agent.
> - Ran a persistent volume framework with the task commands changed to have 
> `sleep 100`.
> - Upgraded the agent from 1.3.1 to `master` + this patch.
> - The agent was able to register successfully.
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 61906: Added a dependency check 'filesystem/linux' isolator.

[Joseph Wu]

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


Ship it!




Ship It!

- Joseph Wu


On Aug. 24, 2017, 5 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61906/
> ---
> 
> (Updated Aug. 24, 2017, 5 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Joseph Wu.
> 
> 
> Bugs: MESOS-7306
> https://issues.apache.org/jira/browse/MESOS-7306
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The 'filesystem/linux' isolator depends on 'linux' launcher to create
> the mount namespace. Therefore, we add this dependency check and try
> to fail early.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
> d7fe9a8f43ab6436fc1e202443e71cf6b7e2d0d3 
> 
> 
> Diff: https://reviews.apache.org/r/61906/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 61905: Integrated 'volume/host_path' into MesosContainerizer.

[Joseph Wu]

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


Ship it!





src/slave/containerizer/mesos/isolators/filesystem/linux.cpp
Line 518 (original), 514 (patched)


Since this line is being changed, might as well use `path::absolute` too.


- Joseph Wu


On Aug. 24, 2017, 5 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61905/
> ---
> 
> (Updated Aug. 24, 2017, 5 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Joseph Wu.
> 
> 
> Bugs: MESOS-7306
> https://issues.apache.org/jira/browse/MESOS-7306
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch removed the host volume logics from the 'filesystem/linux'
> isolator, and integrated the new 'volume/host_path' isolator. For
> backward compatibility, we always enable 'volume/host_path' isolator
> if 'filesystem/linux' isolator is enabled.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> f91eef22dbaf040cb0026862cd8922d84c090b73 
>   src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
> d7fe9a8f43ab6436fc1e202443e71cf6b7e2d0d3 
> 
> 
> Diff: https://reviews.apache.org/r/61905/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 61904: Added volume/host_path isolator.

[Joseph Wu]

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


Fix it, then Ship it!





src/slave/containerizer/mesos/isolators/volume/host_path.cpp
Lines 100-101 (patched)


Perhaps expand on this note?

i.e. If an old master (no validation code) is used to launch a task with a 
volume.



src/slave/containerizer/mesos/isolators/volume/host_path.cpp
Lines 108-111 (patched)


Consider adding a NOTE here about why it is *not* an error for 
`volume.host_path` to be relative (even though it is an error in 
`volume.source().host_path()`).


- Joseph Wu


On Aug. 28, 2017, 1:26 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61904/
> ---
> 
> (Updated Aug. 28, 2017, 1:26 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Joseph Wu.
> 
> 
> Bugs: MESOS-7223 and MESOS-7306
> https://issues.apache.org/jira/browse/MESOS-7223
> https://issues.apache.org/jira/browse/MESOS-7306
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This isolator will be used to handle HOST_PATH volumes. The goal is to
> move the logics of handling host volumes from the linux filesystem
> isolator to this dedicated isolator to make the code more modular.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 0816c6ebaa9caaa5b6a606e601427ed17112a300 
>   src/Makefile.am 5157b2baac4ff6c4493faaeb7a311381e8e76fe7 
>   src/slave/containerizer/mesos/isolators/volume/host_path.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/volume/host_path.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/61904/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 61946: Added validation of resource provider operations.

[Jie Yu]

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




src/master/validation.cpp
Lines 2205 (patched)


I think `checkpointedResources` should not be used for Resource Provider 
provided resources. It should only apply to agent default resources. The 
checkpointing should be done by the corresponding resource provider, not the 
agent for RP provided resources.

As a result, for operations like RESERVE/UNRESERVE/CREATE/DESTROY, we need 
to send operation to the corresponding resource provider as well. This does 
make sense. If we ask agent to persist those information, what will be the 
semantics if the resource provider is marked as gone?

However, this does get complicated if we want to guarantee ordering for 
operations in one `acceptOffers` call (for backwards compatibility), and we do 
want to allow frameworks to launch a task right after reserve operation (the 
current semantics).

To support that, I think we need to speculatively assume the operation will 
be sucessful (thus allow a subsequent launch immediately at the master side). 
However, when the checkpointing fails, we need a way to abort the subsequent 
launch at the agent side. This is essentially why we CHECK fail if the 
checkpointing fails at the agent previously for `checkpointedResources`.

For the resource provider case, we should do the same thing. We can abort 
the agent if a checkpointing fails. However, this only applies to the local 
resource provider that lives in the agent process. If a LRP is outside of the 
agent process, how to abort the subsequent task launch if a previous operation 
fails is something we should think about. For instance, always reject 
operations from the agent's RP manager if the operation is for a stale stream 
ID?


- Jie Yu


On Aug. 28, 2017, 3:28 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61946/
> ---
> 
> (Updated Aug. 28, 2017, 3:28 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added validation of resource provider operations.
> 
> 
> Diffs
> -
> 
>   src/master/validation.hpp f4925752f20ae8ca4de1d9b4a3d5ffc394db9585 
>   src/master/validation.cpp 7c3247d407c9e6aa8cce457d6c6be0c39f4b532f 
> 
> 
> Diff: https://reviews.apache.org/r/61946/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 61952: Fixed the communication between old masters and new agents.

[Michael Park]

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

(Updated Aug. 28, 2017, 1:46 p.m.)


Review request for mesos and Greg Mann.


Changes
---

Added manual testing.


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


Repository: mesos


Description
---

For re-registration, 1.4 agents used to send the resources in tasks
and executors to the master in the "post-reservation-refinement" format,
which is incompatible for pre-1.4 masters. This patch changes the agent
such that it always downgrades the resources to
the "pre-reservation-refinement" format, and the master unconditionally
upgrades the resources to "post-reservation-refinement" format.


Diffs
-

  src/master/master.cpp a9d2191d92d0a56c8a137cb28f41a3632db54fe9 
  src/slave/slave.cpp 3e880b3ad54dbad46f71c335c2a93edb9a546176 


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


Testing (updated)
---

- Started 1.3.1 master, agent.
- Ran a persistent volume framework with the task commands changed to have 
`sleep 100`.
- Upgraded the agent from 1.3.1 to `master`.
- Observed the following message:
> W0828 12:48:22.066509 40596 master.cpp:5702] Dropping re-registration of agent
at slave(1)@10.0.49.2:5051 because it sent an invalid re-registration: Task uses
invalid resources: Invalid DiskInfo: Persistent volumes cannot be created from
unreserved resources

---

- Started 1.3.1 master, agent.
- Ran a persistent volume framework with the task commands changed to have 
`sleep 100`.
- Upgraded the agent from 1.3.1 to `master` + this patch.
- The agent was able to register successfully.


Thanks,

Michael Park

Re: Review Request 61952: Fixed the communication between old masters and new agents.

[Mesos Reviewbot Windows]

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



Patch looks great!

Reviews applied: [61952]

Logs available here: http://104.210.40.105/logs/master/61952

- Mesos Reviewbot Windows


On Aug. 28, 2017, 6:31 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61952/
> ---
> 
> (Updated Aug. 28, 2017, 6:31 p.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Bugs: MESOS-7922
> https://issues.apache.org/jira/browse/MESOS-7922
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> For re-registration, 1.4 agents used to send the resources in tasks
> and executors to the master in the "post-reservation-refinement" format,
> which is incompatible for pre-1.4 masters. This patch changes the agent
> such that it always downgrades the resources to
> the "pre-reservation-refinement" format, and the master unconditionally
> upgrades the resources to "post-reservation-refinement" format.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp a9d2191d92d0a56c8a137cb28f41a3632db54fe9 
>   src/slave/slave.cpp 3e880b3ad54dbad46f71c335c2a93edb9a546176 
> 
> 
> Diff: https://reviews.apache.org/r/61952/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael Park
> 
>

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

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [61172]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


On Aug. 9, 2017, 5:54 p.m., Eric Chung wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61172/
> ---
> 
> (Updated Aug. 9, 2017, 5:54 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.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/bootstrap c84e549e59bc7a07b5b70c24c63bc0c16881e442 
>   src/python/cli_new/pylint.config 63fbb94fbf47cccd1053b4c1eafac18b88dbc818 
>   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 
> 
> 
> Diff: https://reviews.apache.org/r/61172/diff/3/
> 
> 
> Testing
> ---
> 
> install tox
> cd src/python/lib
> tox
> 
> 
> Thanks,
> 
> Eric Chung
> 
>

Re: Review Request 61904: Added volume/host_path isolator.

[Jie Yu]

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

(Updated Aug. 28, 2017, 8:26 p.m.)


Review request for mesos, Gilbert Song and Joseph Wu.


Bugs: MESOS-7223 and MESOS-7306
https://issues.apache.org/jira/browse/MESOS-7223
https://issues.apache.org/jira/browse/MESOS-7306


Repository: mesos


Description
---

This isolator will be used to handle HOST_PATH volumes. The goal is to
move the logics of handling host volumes from the linux filesystem
isolator to this dedicated isolator to make the code more modular.


Diffs
-

  src/CMakeLists.txt 0816c6ebaa9caaa5b6a606e601427ed17112a300 
  src/Makefile.am 5157b2baac4ff6c4493faaeb7a311381e8e76fe7 
  src/slave/containerizer/mesos/isolators/volume/host_path.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/volume/host_path.cpp PRE-CREATION 


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


Testing
---

sudo make check


Thanks,

Jie Yu

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

[Mesos Reviewbot Windows]

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



Bad patch!

Reviews applied: [61172]

Logs available here: http://104.210.40.105/logs/master/61172

- Mesos Reviewbot Windows


On Aug. 9, 2017, 10:54 a.m., Eric Chung wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61172/
> ---
> 
> (Updated Aug. 9, 2017, 10:54 a.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.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/bootstrap c84e549e59bc7a07b5b70c24c63bc0c16881e442 
>   src/python/cli_new/pylint.config 63fbb94fbf47cccd1053b4c1eafac18b88dbc818 
>   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 
> 
> 
> Diff: https://reviews.apache.org/r/61172/diff/3/
> 
> 
> Testing
> ---
> 
> install tox
> cd src/python/lib
> tox
> 
> 
> Thanks,
> 
> Eric Chung
> 
>

Re: Review Request 61915: Enabled `DockerContainerizerProcess::usage` for all platforms.

[Alexander Rukletsov]

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


Fix it, then Ship it!





src/slave/containerizer/docker.cpp
Lines 1869-1870 (patched)


Let's insert a blank line here.


- Alexander Rukletsov


On Aug. 25, 2017, 8:32 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61915/
> ---
> 
> (Updated Aug. 25, 2017, 8:32 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Joseph Wu, Kevin Klues, and Li Li.
> 
> 
> Bugs: MESOS-7917
> https://issues.apache.org/jira/browse/MESOS-7917
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When originally implemented this code was enabled only for Linux.
> However, this code should work anywhere Docker is being used, as it
> obtains the information by shelling out to the `docker` executable.
> 
> This patch fixes the resources information provided by the `mesos-agent`
> on Windows platforms for Docker containers by making the `statistics`
> JSON object available.
> 
> Note that the `cgroupsStatistics` is Linux specific, and so the
> pre-processor guard was not removed but reduced in scope for just it.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp cbcff39a5d8398f71570849615aeb4368130b433 
> 
> 
> Diff: https://reviews.apache.org/r/61915/diff/1/
> 
> 
> Testing
> ---
> 
> Built on Windows, started a Windows Docker container, observed the 
> `statistics` field from `/container` being populated where it was 
> not previously included.
> 
> `make check` passed on CentOS 7.
> 
> Diagnosing the Windows tests, have a machine configuration with Docker 
> containers right now.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 61899: Added HOST_PATH volume source protobuf.

[Jie Yu]

> On Aug. 28, 2017, 6:58 p.m., Joseph Wu wrote:
> > include/mesos/mesos.proto
> > Lines 2516-2518 (original), 2516-2518 (patched)
> > 
> >
> > Does this new enum deprecate this `host_path` field?  
> > 
> > The only difference is that the new enum only takes absolute paths 
> > whereas the existing `host_path` can be relative (according to the 
> > comments).

We cannot deprecate it yet because SANDBOX_PATH (SELF TYPE) still use this 
field. i'll mark it as deprecated later once I refactor the sandbox_path volume 
as well (soon).


- Jie


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


On Aug. 24, 2017, 11:57 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61899/
> ---
> 
> (Updated Aug. 24, 2017, 11:57 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Joseph Wu.
> 
> 
> Bugs: MESOS-7306
> https://issues.apache.org/jira/browse/MESOS-7306
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added HOST_PATH volume source protobuf.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 86936cf9ad9406fa4c3358ccb3c58e2f3259d65f 
>   include/mesos/v1/mesos.proto 20d9ecff68465c6517a5a239a4bd0f42f82d0cc6 
> 
> 
> Diff: https://reviews.apache.org/r/61899/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 61662: Added SlaveRecoveryTest.RebootWithSlaveInfoMismatchAndRestart test.

[Jiang Yan Xu]

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


Ship it!




Will fix the nit when committing.


src/tests/slave_recovery_tests.cpp
Lines 2852-2853 (patched)


Nit: "before" sounds like we are merely racing against it when we are 
completely preventing it.

Here I think it sufficies to say:

```
// Simulate an agent restart before reregistration completes.
```


- Jiang Yan Xu


On Aug. 15, 2017, 7:48 a.m., Ilya Pronin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61662/
> ---
> 
> (Updated Aug. 15, 2017, 7:48 a.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7795
> https://issues.apache.org/jira/browse/MESOS-7795
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The new test verifies that the agent removes the 'latest' symlink when it 
> decides to recover as a new one after reboot. And that it can successfully 
> recover again if restarted before checkpointing new agent info.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_recovery_tests.cpp 9ba6f6005c2edce67e1f63005518ab71ac981423 
> 
> 
> Diff: https://reviews.apache.org/r/61662/diff/1/
> 
> 
> Testing
> ---
> 
> Ran `make check`. Verified that this test fails without the bugfix patch.
> 
> 
> Thanks,
> 
> Ilya Pronin
> 
>

Re: Review Request 61661: Added 'latest' symlink removal.

[Jiang Yan Xu]

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


Ship it!




Ship It!

- Jiang Yan Xu


On Aug. 15, 2017, 7:48 a.m., Ilya Pronin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61661/
> ---
> 
> (Updated Aug. 15, 2017, 7:48 a.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7795
> https://issues.apache.org/jira/browse/MESOS-7795
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> After a reboot the agent checkpoints new boot ID after recovering and
> agent info after registering with the master. If new agent info is
> incompatible with the checkpointed one, the agent will recover as a new
> one. But if the agent is restarted before registering it will fail to
> start again, because new boot ID will already be checkpointed.
> 
> This patch fixes it by checkpointing the agent's decision to recover as
> a new one by removing the "latest" meta symlink.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 7381530515f86faf4c3e8f82bcd9483f6cf0498b 
> 
> 
> Diff: https://reviews.apache.org/r/61661/diff/1/
> 
> 
> Testing
> ---
> 
> Ran `make check`. A new test is added in the subsequent patch.
> 
> 
> Thanks,
> 
> Ilya Pronin
> 
>

Re: Review Request 61902: Added master and agent validation for ContainerInfo.

[Joseph Wu]

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


Ship it!




LGTM (assuming the upstream patches don't change much).

- Joseph Wu


On Aug. 24, 2017, 4:58 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61902/
> ---
> 
> (Updated Aug. 24, 2017, 4:58 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Joseph Wu.
> 
> 
> Bugs: MESOS-7306
> https://issues.apache.org/jira/browse/MESOS-7306
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added master and agent validation for ContainerInfo.
> 
> 
> Diffs
> -
> 
>   src/master/validation.cpp 7c3247d407c9e6aa8cce457d6c6be0c39f4b532f 
>   src/slave/validation.cpp 85c43cacf1361d269c28a7fe8bd1da8615949ec8 
> 
> 
> Diff: https://reviews.apache.org/r/61902/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 61903: Fixed a indentation issue in linux filesystem isolator.

[Joseph Wu]

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


Ship it!




Ship It!

- Joseph Wu


On Aug. 24, 2017, 4:59 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61903/
> ---
> 
> (Updated Aug. 24, 2017, 4:59 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Joseph Wu.
> 
> 
> Bugs: MESOS-7306
> https://issues.apache.org/jira/browse/MESOS-7306
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a indentation issue in linux filesystem isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
> d7fe9a8f43ab6436fc1e202443e71cf6b7e2d0d3 
> 
> 
> Diff: https://reviews.apache.org/r/61903/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 61901: Added common validation for Volume.

[Joseph Wu]

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




src/common/validation.cpp
Lines 206 (patched)


Seems like you could start with adding this:
```
if (!path::absolute(volume.host_path()) &&
!path::absolute(volume.container_path())) {
  ...
}
```

(Some form of which exists in the filesystem/linux isolator and Docker 
containerizer)



src/common/validation.cpp
Lines 212-217 (patched)


Might be clearer to count the number of occurrences rather than bit-mask 
them.

Then error if the number of occurrences is not equal to one.

---

Also, I'm surprised that we don't have this validation already.  

It seems like, in the existing code, if you:
1) Specify some value for all three fields.
2) Enable the appropriate isolators (filesystem/linux, docker/volume, 
volume/sandbox_path).
3) All three isolators will do their thing, possibly running over each 
other in the process.



src/common/validation.cpp
Lines 249-250 (patched)


The default case should be an error on its own.  You could leave out the 
default case so that this code won't compile when a new Source type is defined.


- Joseph Wu


On Aug. 24, 2017, 4:58 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61901/
> ---
> 
> (Updated Aug. 24, 2017, 4:58 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Joseph Wu.
> 
> 
> Bugs: MESOS-7306
> https://issues.apache.org/jira/browse/MESOS-7306
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added common validation for Volume.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 5157b2baac4ff6c4493faaeb7a311381e8e76fe7 
>   src/common/validation.hpp 8c92436eb6c8aa87ffd6d6cf3f42079d6c90e525 
>   src/common/validation.cpp 33b501bfa6613dab70a663f552a424ffa8a15b0a 
>   src/tests/CMakeLists.txt 6c05ea93c3288fb9608c191234f95e270390d89e 
>   src/tests/common_validation_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/61901/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 61899: Added HOST_PATH volume source protobuf.

[Joseph Wu]

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




include/mesos/mesos.proto
Lines 2516-2518 (original), 2516-2518 (patched)


Does this new enum deprecate this `host_path` field?  

The only difference is that the new enum only takes absolute paths whereas 
the existing `host_path` can be relative (according to the comments).


- Joseph Wu


On Aug. 24, 2017, 4:57 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61899/
> ---
> 
> (Updated Aug. 24, 2017, 4:57 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Joseph Wu.
> 
> 
> Bugs: MESOS-7306
> https://issues.apache.org/jira/browse/MESOS-7306
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added HOST_PATH volume source protobuf.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 86936cf9ad9406fa4c3358ccb3c58e2f3259d65f 
>   include/mesos/v1/mesos.proto 20d9ecff68465c6517a5a239a4bd0f42f82d0cc6 
> 
> 
> Diff: https://reviews.apache.org/r/61899/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 58021: Added storage-related offer operations.

[Jie Yu]

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




src/common/protobuf_utils.cpp
Line 442 (original), 442 (patched)


I'd see if we can avoid changing `inject` to different methods based on the 
type. See if something like the following will work or not:

```
struct Injector
{
  void operator()(
  Resource& resource,
  const Reosurce::AllocationInfo& allocationInfo)
  { ... }
  
  void operator()(
  RepeatedPtrField* resources,
  const Reosurce::AllocationInfo& allocationInfo)
  { ... }
} inject;
```


- Jie Yu


On Aug. 8, 2017, 8:21 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58021/
> ---
> 
> (Updated Aug. 8, 2017, 8:21 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-7314
> https://issues.apache.org/jira/browse/MESOS-7314
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added storage-related offer operations.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto f31f5bdc2ace2b261885b252e7c01ceb9b76c461 
>   include/mesos/v1/mesos.proto 66386a84bc21989d1c1237e629d5d04662a368fa 
>   src/common/protobuf_utils.cpp 3ae68e93a985a4cfe23be9c9bd8f92e418102a39 
>   src/common/resources.cpp 8d4388935ada6be60448e6f6a88db0e5fc4ad4a1 
>   src/common/resources_utils.cpp 821bd0967d55f7fbcb57a4efae5fc390af53ca79 
>   src/master/master.cpp 7f38a5e21884546d4b4c866ca5918db779af8f99 
>   src/v1/resources.cpp 508f3f85c8388a41f57f964b0f6df3b4708d3442 
> 
> 
> Diff: https://reviews.apache.org/r/58021/diff/5/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 61952: Fixed the communication between old masters and new agents.

[Greg Mann]

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



This change LGTM, one small nit below. Could you also verify this fix with a 
manual upgrade test - maybe our 'support/test-upgrade.py' script could be used?


src/slave/slave.cpp
Lines 1591-1592 (patched)


Looks like your linebreak is a bit early here on L1591?


- Greg Mann


On Aug. 28, 2017, 6:31 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61952/
> ---
> 
> (Updated Aug. 28, 2017, 6:31 p.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Bugs: MESOS-7922
> https://issues.apache.org/jira/browse/MESOS-7922
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> For re-registration, 1.4 agents used to send the resources in tasks
> and executors to the master in the "post-reservation-refinement" format,
> which is incompatible for pre-1.4 masters. This patch changes the agent
> such that it always downgrades the resources to
> the "pre-reservation-refinement" format, and the master unconditionally
> upgrades the resources to "post-reservation-refinement" format.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp a9d2191d92d0a56c8a137cb28f41a3632db54fe9 
>   src/slave/slave.cpp 3e880b3ad54dbad46f71c335c2a93edb9a546176 
> 
> 
> Diff: https://reviews.apache.org/r/61952/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 61849: Improved consistency of cout/cerr and glog usage in main functions.

[Mesos Reviewbot Windows]

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



Patch looks great!

Reviews applied: [61849]

Logs available here: http://104.210.40.105/logs/master/61849

- Mesos Reviewbot Windows


On Aug. 25, 2017, 10:34 a.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61849/
> ---
> 
> (Updated Aug. 25, 2017, 10:34 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-7586
> https://issues.apache.org/jira/browse/MESOS-7586
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The main functions now have a common initialization pattern:
>   1. Load flags.
>   2. Check if the --help flag has been passed.
>   3. Check if the flags are correct.
>   4. Initialize logging.
>   5. Parse and setup the environment variables.
>   6. Initialize libprocess.
> 
> This change reduces the number of messages "WARNING: Logging before
> InitGoogleLogging() is written to STDERR" as we now use glog only after
> the fourth step. This forces all glog messages to end up in a logdir.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp aff97939b088e3747111ca6ec00fe76c59e9b396 
>   src/launcher/default_executor.cpp 30bae5c931c81b81ff9590eea8051b2232388813 
>   src/launcher/executor.cpp 65b9a2c52ba5a5a514fff992018e356f388468fc 
>   src/local/main.cpp f0d7e8ca1a64b5409632ea0c0894a1dc84f9796e 
>   src/master/main.cpp 1a78a55fd52c5eebad689c2a7f16c95979260d79 
>   src/slave/main.cpp a4a8ced6825ece1b25003824d987ff83fb799ad7 
> 
> 
> Diff: https://reviews.apache.org/r/61849/diff/4/
> 
> 
> Testing
> ---
> 
> ```
> $ make check
> ```
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 61952: Fixed the communication between old masters and new agents.

[Michael Park]

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

(Updated Aug. 28, 2017, 11:31 a.m.)


Review request for mesos and Greg Mann.


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


Repository: mesos


Description
---

For re-registration, 1.4 agents used to send the resources in tasks
and executors to the master in the "post-reservation-refinement" format,
which is incompatible for pre-1.4 masters. This patch changes the agent
such that it always downgrades the resources to
the "pre-reservation-refinement" format, and the master unconditionally
upgrades the resources to "post-reservation-refinement" format.


Diffs
-

  src/master/master.cpp a9d2191d92d0a56c8a137cb28f41a3632db54fe9 
  src/slave/slave.cpp 3e880b3ad54dbad46f71c335c2a93edb9a546176 


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


Testing
---


Thanks,

Michael Park

Re: Review Request 61947: Implemented handling of resource provider offer operations.

[Mesos Reviewbot]

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



Bad patch!

Reviews applied: [61947, 61946, 61810, 58021, 58047, 58048, 57999, 57998, 
57997, 57911]

Failed command: python support/apply-reviews.py -n -r 58048

Error:
2017-08-28 18:30:30 URL:https://reviews.apache.org/r/58048/diff/raw/ 
[11585/11585] -> "58048.patch" [1]
error: patch failed: src/tests/mesos.hpp:875
error: src/tests/mesos.hpp: patch does not apply

Full log: https://builds.apache.org/job/Mesos-Reviewbot/19101/console

- Mesos Reviewbot


On Aug. 28, 2017, 8:30 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61947/
> ---
> 
> (Updated Aug. 28, 2017, 8:30 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> While the resource provider manager is responsible to apply offer
> operations by sending events to the respective resource providers,
> the master takes care of accepting these operations. Hence, for local
> resource providers the master sends an `ApplyResourceOperationMessage`
> to the agent where the resource provider is running on. The agent
> then relays the operation contained in the message to the resource
> provider manager.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp a9d2191d92d0a56c8a137cb28f41a3632db54fe9 
>   src/messages/messages.proto dc4e19c08d671743f08223dcdf1dbc336b3940bd 
>   src/slave/slave.hpp 7d07868451e93d34ba694d40216c1e4036fd4094 
>   src/slave/slave.cpp 3e880b3ad54dbad46f71c335c2a93edb9a546176 
>   src/tests/resource_provider_manager_tests.cpp 
> 83a1340fa16b19e3297a8c0ca413afc312de00ec 
> 
> 
> Diff: https://reviews.apache.org/r/61947/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 58048: Added id to Resource.DiskInfo.

[Jie Yu]

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




src/common/resources.cpp
Line 187 (original), 187 (patched)


We also need to update `addable` and `subtractable` similar to what we did 
for persistent volumes.

Depending on the "disk" source type, the semantics is different:
1. RAW w/ id:
   - Cannot be split into smaller pieces
   - Cannot be merged even if they are equal
2. RAW w/o id:
   - Can be split into smaller pieces
   - Can be merged if RAW parts equal
3. PATH w/ id:
   - Can be split into smaller pieces
   - Can be merged if PATH parts equal
4. PATH w/o id:
   - Same as PATH w/ id
5. MOUNT w/ id:
   - Cannot be split into smaller pieces
   - Cannot be merged even if they are equal
6. MOUNT w/o id:
   - Same as MOUNT w/ id


- Jie Yu


On May 19, 2017, 9:16 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58048/
> ---
> 
> (Updated May 19, 2017, 9:16 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Bugs: MESOS-7312
> https://issues.apache.org/jira/browse/MESOS-7312
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ids will allow to create distinguishable resources, e.g., of RAW or
> BLOCK type.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto e138e2aedb1369acdde82facf9dfea9780411154 
>   include/mesos/v1/mesos.proto ab617bfd2bc46e44f7d71cefecf5eac3630d22b3 
>   src/common/resources.cpp 1d07f1a049ba3bb3f5d78f05031f33ba97e07e8c 
>   src/tests/mesos.hpp 3c57f25ff0bec55582b8a7decef53be61a9ec6f4 
>   src/tests/resources_tests.cpp 5dcbce2dd4944194b59790551929d6d75b805ba3 
>   src/v1/resources.cpp 1bb5d0741c9f9ea59f34e92159a335661019444d 
> 
> 
> Diff: https://reviews.apache.org/r/58048/diff/4/
> 
> 
> Testing
> ---
> 
> make check (OS X, Fedora25)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Review Request 61952: Fixed the communication between old masters and new agents.

[Michael Park]

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

Review request for mesos and Greg Mann.


Repository: mesos


Description
---

A 1.4 agent used to send the resources to the master in
the "post-reservation-refinement" format, which is incompatible
for pre-1.4 masters. The new agent now always downgrades
the resources to the "pre-reservation-refinement" format, and
the 1.4 master unconditionally upgrades the resources to
"post-reservation-refinement" format.


Diffs
-

  src/master/master.cpp a9d2191d92d0a56c8a137cb28f41a3632db54fe9 
  src/slave/slave.cpp 3e880b3ad54dbad46f71c335c2a93edb9a546176 


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


Testing
---


Thanks,

Michael Park

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

[Eric Chung]

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



bump?

- Eric Chung


On Aug. 9, 2017, 5:54 p.m., Eric Chung wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61172/
> ---
> 
> (Updated Aug. 9, 2017, 5:54 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.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/bootstrap c84e549e59bc7a07b5b70c24c63bc0c16881e442 
>   src/python/cli_new/pylint.config 63fbb94fbf47cccd1053b4c1eafac18b88dbc818 
>   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 
> 
> 
> Diff: https://reviews.apache.org/r/61172/diff/3/
> 
> 
> Testing
> ---
> 
> install tox
> cd src/python/lib
> tox
> 
> 
> Thanks,
> 
> Eric Chung
> 
>

Re: Review Request 61473: Do not kill non partition aware tasks.

[Mesos Reviewbot]

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



Bad patch!

Reviews applied: [61473]

Failed command: python support/apply-reviews.py -n -r 61473

Error:
2017-08-28 17:26:22 URL:https://reviews.apache.org/r/61473/diff/raw/ 
[15375/15375] -> "61473.patch" [1]
Traceback (most recent call last):
  File "support/apply-reviews.py", line 417, in 
main()
  File "support/apply-reviews.py", line 412, in main
reviewboard(options)
  File "support/apply-reviews.py", line 402, in reviewboard
apply_review(options)
  File "support/apply-reviews.py", line 160, in apply_review
commit_patch(options)
  File "support/apply-reviews.py", line 261, in commit_patch
message.write(data['message'])
UnicodeEncodeError: 'ascii' codec can't encode character u'\u2019' in position 
655: ordinal not in range(128)

Full log: https://builds.apache.org/job/Mesos-Reviewbot/19100/console

- Mesos Reviewbot


On Aug. 28, 2017, 3:31 p.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61473/
> ---
> 
> (Updated Aug. 28, 2017, 3:31 p.m.)
> 
> 
> Review request for mesos, Vinod Kone and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7215
> https://issues.apache.org/jira/browse/MESOS-7215
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Master will not kill the tasks for non-Partition aware frameworks
> when an unreachable agent re-registers with the master.
> Master used to send a ShutdownFrameworkMessages to the agent
> to kill the tasks from non partition aware frameworks including the
> ones that are still registered which was problematic because the offer
> from this agent could still go to the same framework which could then
> launch new tasks. The agent would then receive tasks of the same
> framework and ignore them because it thinks the framework is shutting
> down. The framework is not shutting down of course, so from the master
> and the scheduler’s perspective the task is pending in STAGING forever
> until the next agent reregistration, which could happen much later.
> This commit fixes the problem by not shutting down the non-partition
> aware frameworks on such an agent.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 959091c8ec03b6ac7bcb5d21b04d2f7d5aff7d54 
>   src/master/master.hpp b802fd153a10f6012cea381f153c28cc78cae995 
>   src/master/master.cpp 7f38a5e21884546d4b4c866ca5918db779af8f99 
>   src/tests/partition_tests.cpp 62a84f797201ccd18b71490949e3130d2b9c3668 
> 
> 
> Diff: https://reviews.apache.org/r/61473/diff/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Megha Sharma
> 
>

Re: Review Request 61947: Implemented handling of resource provider offer operations.

[Mesos Reviewbot Windows]

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



Failed to apply patch!

Reviews applied: [61275, 61654, 61271, 61272, 61179, 61180, 61181, 61278, 
61182, 57911, 57997, 57998, 57999, 58048, 58047, 58021, 61810, 61946, 61947]

Logs available here: http://104.210.40.105/logs/master/61947

- Mesos Reviewbot Windows


On Aug. 28, 2017, 3:30 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61947/
> ---
> 
> (Updated Aug. 28, 2017, 3:30 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> While the resource provider manager is responsible to apply offer
> operations by sending events to the respective resource providers,
> the master takes care of accepting these operations. Hence, for local
> resource providers the master sends an `ApplyResourceOperationMessage`
> to the agent where the resource provider is running on. The agent
> then relays the operation contained in the message to the resource
> provider manager.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp a9d2191d92d0a56c8a137cb28f41a3632db54fe9 
>   src/messages/messages.proto dc4e19c08d671743f08223dcdf1dbc336b3940bd 
>   src/slave/slave.hpp 7d07868451e93d34ba694d40216c1e4036fd4094 
>   src/slave/slave.cpp 3e880b3ad54dbad46f71c335c2a93edb9a546176 
>   src/tests/resource_provider_manager_tests.cpp 
> 83a1340fa16b19e3297a8c0ca413afc312de00ec 
> 
> 
> Diff: https://reviews.apache.org/r/61947/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 61473: Do not kill non partition aware tasks.

[Mesos Reviewbot Windows]

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



Failed to apply patch!

Reviews applied: [61473]

Logs available here: http://104.210.40.105/logs/master/61473

- Mesos Reviewbot Windows


On Aug. 28, 2017, 3:31 p.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61473/
> ---
> 
> (Updated Aug. 28, 2017, 3:31 p.m.)
> 
> 
> Review request for mesos, Vinod Kone and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7215
> https://issues.apache.org/jira/browse/MESOS-7215
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Master will not kill the tasks for non-Partition aware frameworks
> when an unreachable agent re-registers with the master.
> Master used to send a ShutdownFrameworkMessages to the agent
> to kill the tasks from non partition aware frameworks including the
> ones that are still registered which was problematic because the offer
> from this agent could still go to the same framework which could then
> launch new tasks. The agent would then receive tasks of the same
> framework and ignore them because it thinks the framework is shutting
> down. The framework is not shutting down of course, so from the master
> and the scheduler’s perspective the task is pending in STAGING forever
> until the next agent reregistration, which could happen much later.
> This commit fixes the problem by not shutting down the non-partition
> aware frameworks on such an agent.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 959091c8ec03b6ac7bcb5d21b04d2f7d5aff7d54 
>   src/master/master.hpp b802fd153a10f6012cea381f153c28cc78cae995 
>   src/master/master.cpp 7f38a5e21884546d4b4c866ca5918db779af8f99 
>   src/tests/partition_tests.cpp 62a84f797201ccd18b71490949e3130d2b9c3668 
> 
> 
> Diff: https://reviews.apache.org/r/61473/diff/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Megha Sharma
> 
>

Re: Review Request 61924: Moved the 'RejectingObjectApprover' to a public header.

[Mesos Reviewbot Windows]

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



Bad patch!

Reviews applied: [61922, 61923, 61924]

Logs available here: http://104.210.40.105/logs/master/61924

- Mesos Reviewbot Windows


On Aug. 28, 2017, 4:17 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61924/
> ---
> 
> (Updated Aug. 28, 2017, 4:17 p.m.)
> 
> 
> Review request for Greg Mann.
> 
> 
> Bugs: MESOS-7914
> https://issues.apache.org/jira/browse/MESOS-7914
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Moved the 'RejectingObjectApprover' to a public header.
> 
> 
> Diffs
> -
> 
>   src/authorizer/local/authorizer.cpp 
> 82ae846fd39de64704f0e8bd0fe2972f3750d2e6 
>   src/common/http.hpp 0e6b1c59860e75c04e2e3be2520ed2b39c84ac90 
> 
> 
> Diff: https://reviews.apache.org/r/61924/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 61849: Improved consistency of cout/cerr and glog usage in main functions.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [61849]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


On Aug. 25, 2017, 5:34 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61849/
> ---
> 
> (Updated Aug. 25, 2017, 5:34 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-7586
> https://issues.apache.org/jira/browse/MESOS-7586
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The main functions now have a common initialization pattern:
>   1. Load flags.
>   2. Check if the --help flag has been passed.
>   3. Check if the flags are correct.
>   4. Initialize logging.
>   5. Parse and setup the environment variables.
>   6. Initialize libprocess.
> 
> This change reduces the number of messages "WARNING: Logging before
> InitGoogleLogging() is written to STDERR" as we now use glog only after
> the fourth step. This forces all glog messages to end up in a logdir.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp aff97939b088e3747111ca6ec00fe76c59e9b396 
>   src/launcher/default_executor.cpp 30bae5c931c81b81ff9590eea8051b2232388813 
>   src/launcher/executor.cpp 65b9a2c52ba5a5a514fff992018e356f388468fc 
>   src/local/main.cpp f0d7e8ca1a64b5409632ea0c0894a1dc84f9796e 
>   src/master/main.cpp 1a78a55fd52c5eebad689c2a7f16c95979260d79 
>   src/slave/main.cpp a4a8ced6825ece1b25003824d987ff83fb799ad7 
> 
> 
> Diff: https://reviews.apache.org/r/61849/diff/4/
> 
> 
> Testing
> ---
> 
> ```
> $ make check
> ```
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 61473: Do not kill non partition aware tasks.

[Megha Sharma]

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

(Updated Aug. 28, 2017, 3:31 p.m.)


Review request for mesos, Vinod Kone and Jiang Yan Xu.


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


Repository: mesos


Description
---

Master will not kill the tasks for non-Partition aware frameworks
when an unreachable agent re-registers with the master.
Master used to send a ShutdownFrameworkMessages to the agent
to kill the tasks from non partition aware frameworks including the
ones that are still registered which was problematic because the offer
from this agent could still go to the same framework which could then
launch new tasks. The agent would then receive tasks of the same
framework and ignore them because it thinks the framework is shutting
down. The framework is not shutting down of course, so from the master
and the scheduler’s perspective the task is pending in STAGING forever
until the next agent reregistration, which could happen much later.
This commit fixes the problem by not shutting down the non-partition
aware frameworks on such an agent.


Diffs (updated)
-

  src/master/http.cpp 959091c8ec03b6ac7bcb5d21b04d2f7d5aff7d54 
  src/master/master.hpp b802fd153a10f6012cea381f153c28cc78cae995 
  src/master/master.cpp 7f38a5e21884546d4b4c866ca5918db779af8f99 
  src/tests/partition_tests.cpp 62a84f797201ccd18b71490949e3130d2b9c3668 


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

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


Testing
---

make check


Thanks,

Megha Sharma

Re: Review Request 61473: Do not kill non partition aware tasks.

[Megha Sharma]

> On Aug. 10, 2017, 9 p.m., Jiang Yan Xu wrote:
> > src/master/http.cpp
> > Lines 342 (patched)
> > 
> >
> > One empty line above.

Fixed


> On Aug. 10, 2017, 9 p.m., Jiang Yan Xu wrote:
> > src/master/http.cpp
> > Lines 3203-3206 (original), 3215-3227 (patched)
> > 
> >
> > Should we only loop through the once? And in order to try not to 
> > duplicate similar lines, consider the following?
> > 
> > ```
> > foreachvalue (const Owned& _task, framework->unreachableTasks) {
> >   Owned task = _task;
> > 
> >   if (framework->capabilities.partitionAware) {
> > task = ...
> >   }
> >   
> >   frameworkTaskSummaries[frameworkId].count(*task.get());
> >   slaveTaskSummaries[task->slave_id()].count(*task.get());
> > }
> > ```

Fixed


> On Aug. 10, 2017, 9 p.m., Jiang Yan Xu wrote:
> > src/master/http.cpp
> > Lines 4017-4027 (original), 4038-4063 (patched)
> > 
> >
> > Similar to above, don't duplicate lines.

Fixed


> On Aug. 10, 2017, 9 p.m., Jiang Yan Xu wrote:
> > src/master/http.cpp
> > Lines 4167-4175 (original), 4203-4221 (patched)
> > 
> >
> > Ditto.

Fixed


> On Aug. 10, 2017, 9 p.m., Jiang Yan Xu wrote:
> > src/master/master.hpp
> > Lines 2403-2408 (patched)
> > 
> >
> > Can we hold off creating a helper for this? IMO this helper is doing 
> > too little and not so much of an "abstraction", i.e., what the method does 
> > it not perfectly captured by the name of the mehod. Inlining 1) making a 
> > copy & 2) setting the state is not too much redudancy than calling this 
> > method.

Code changed, N/A anymore


> On Aug. 10, 2017, 9 p.m., Jiang Yan Xu wrote:
> > src/master/master.hpp
> > Lines 2769-2771 (original), 2773-2775 (patched)
> > 
> >
> > Fix the comment.

Fixed


> On Aug. 10, 2017, 9 p.m., Jiang Yan Xu wrote:
> > src/master/master.cpp
> > Lines 6398-6414 (original), 6389-6405 (patched)
> > 
> >
> > Fix the comments.

Fixed


> On Aug. 10, 2017, 9 p.m., Jiang Yan Xu wrote:
> > src/master/master.cpp
> > Lines 6422 (patched)
> > 
> >
> > `erase` can handle the `!contains` case.

Fixed


> On Aug. 10, 2017, 9 p.m., Jiang Yan Xu wrote:
> > src/master/master.cpp
> > Lines 6477-6480 (original), 6464-6467 (patched)
> > 
> >
> > Fix the comment.

Fixed


> On Aug. 10, 2017, 9 p.m., Jiang Yan Xu wrote:
> > src/master/master.cpp
> > Line 7137 (original), 7112 (patched)
> > 
> >
> > If our code stops making such distinction, I don't think the comment 
> > should continue to make such distinction.

Fixed


> On Aug. 10, 2017, 9 p.m., Jiang Yan Xu wrote:
> > src/master/master.cpp
> > Lines 7132-7137 (patched)
> > 
> >
> > Do this only when we actually send an update i.e., 
> > `framework->connected()`?

Code changed, not applicable any more.


> On Aug. 10, 2017, 9 p.m., Jiang Yan Xu wrote:
> > src/master/master.cpp
> > Lines 8928-8931 (original), 8907-8910 (patched)
> > 
> >
> > So for this update event we are going to send TASK_UNREACHABLE instead 
> > TASK_LOST, which is unintended?

Code changed, not applicable any more.


- Megha


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


On Aug. 10, 2017, 4:07 p.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61473/
> ---
> 
> (Updated Aug. 10, 2017, 4:07 p.m.)
> 
> 
> Review request for mesos, Vinod Kone and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7215
> https://issues.apache.org/jira/browse/MESOS-7215
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Master will not kill the tasks for non-Partition aware frameworks
> when an unreachable agent re-registers with the master.
> Master used to send a ShutdownFrameworkMessages to the agent
> to kill the tasks from non partition 

Review Request 61947: Implemented handling of resource provider offer operations.

[Jan Schlicht]

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

Review request for mesos, Benjamin Bannier and Jie Yu.


Repository: mesos


Description
---

While the resource provider manager is responsible to apply offer
operations by sending events to the respective resource providers,
the master takes care of accepting these operations. Hence, for local
resource providers the master sends an `ApplyResourceOperationMessage`
to the agent where the resource provider is running on. The agent
then relays the operation contained in the message to the resource
provider manager.


Diffs
-

  src/master/master.cpp a9d2191d92d0a56c8a137cb28f41a3632db54fe9 
  src/messages/messages.proto dc4e19c08d671743f08223dcdf1dbc336b3940bd 
  src/slave/slave.hpp 7d07868451e93d34ba694d40216c1e4036fd4094 
  src/slave/slave.cpp 3e880b3ad54dbad46f71c335c2a93edb9a546176 
  src/tests/resource_provider_manager_tests.cpp 
83a1340fa16b19e3297a8c0ca413afc312de00ec 


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


Testing
---

make check


Thanks,

Jan Schlicht

Review Request 61946: Added validation of resource provider operations.

[Jan Schlicht]

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

Review request for mesos, Benjamin Bannier and Jie Yu.


Repository: mesos


Description
---

Added validation of resource provider operations.


Diffs
-

  src/master/validation.hpp f4925752f20ae8ca4de1d9b4a3d5ffc394db9585 
  src/master/validation.cpp 7c3247d407c9e6aa8cce457d6c6be0c39f4b532f 


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


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 61915: Enabled `DockerContainerizerProcess::usage` for all platforms.

[Mesos Reviewbot Windows]

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



Patch looks great!

Reviews applied: [61915]

Logs available here: http://104.210.40.105/logs/master/61915

- Mesos Reviewbot Windows


On Aug. 25, 2017, 8:32 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61915/
> ---
> 
> (Updated Aug. 25, 2017, 8:32 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Joseph Wu, Kevin Klues, and Li Li.
> 
> 
> Bugs: MESOS-7917
> https://issues.apache.org/jira/browse/MESOS-7917
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When originally implemented this code was enabled only for Linux.
> However, this code should work anywhere Docker is being used, as it
> obtains the information by shelling out to the `docker` executable.
> 
> This patch fixes the resources information provided by the `mesos-agent`
> on Windows platforms for Docker containers by making the `statistics`
> JSON object available.
> 
> Note that the `cgroupsStatistics` is Linux specific, and so the
> pre-processor guard was not removed but reduced in scope for just it.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp cbcff39a5d8398f71570849615aeb4368130b433 
> 
> 
> Diff: https://reviews.apache.org/r/61915/diff/1/
> 
> 
> Testing
> ---
> 
> Built on Windows, started a Windows Docker container, observed the 
> `statistics` field from `/container` being populated where it was 
> not previously included.
> 
> `make check` passed on CentOS 7.
> 
> Diagnosing the Windows tests, have a machine configuration with Docker 
> containers right now.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 61849: Improved consistency of cout/cerr and glog usage in main functions.

[Andrei Budnik]

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


Ship it!




- Andrei Budnik


On Aug. 25, 2017, 5:34 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61849/
> ---
> 
> (Updated Aug. 25, 2017, 5:34 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-7586
> https://issues.apache.org/jira/browse/MESOS-7586
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The main functions now have a common initialization pattern:
>   1. Load flags.
>   2. Check if the --help flag has been passed.
>   3. Check if the flags are correct.
>   4. Initialize logging.
>   5. Parse and setup the environment variables.
>   6. Initialize libprocess.
> 
> This change reduces the number of messages "WARNING: Logging before
> InitGoogleLogging() is written to STDERR" as we now use glog only after
> the fourth step. This forces all glog messages to end up in a logdir.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp aff97939b088e3747111ca6ec00fe76c59e9b396 
>   src/launcher/default_executor.cpp 30bae5c931c81b81ff9590eea8051b2232388813 
>   src/launcher/executor.cpp 65b9a2c52ba5a5a514fff992018e356f388468fc 
>   src/local/main.cpp f0d7e8ca1a64b5409632ea0c0894a1dc84f9796e 
>   src/master/main.cpp 1a78a55fd52c5eebad689c2a7f16c95979260d79 
>   src/slave/main.cpp a4a8ced6825ece1b25003824d987ff83fb799ad7 
> 
> 
> Diff: https://reviews.apache.org/r/61849/diff/4/
> 
> 
> Testing
> ---
> 
> ```
> $ make check
> ```
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 61849: Improved consistency of cout/cerr and glog usage in main functions.

[Mesos Reviewbot Windows]

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



Patch looks great!

Reviews applied: [61849]

Logs available here: http://104.210.40.105/logs/master/61849

- Mesos Reviewbot Windows


On Aug. 25, 2017, 5:34 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61849/
> ---
> 
> (Updated Aug. 25, 2017, 5:34 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-7586
> https://issues.apache.org/jira/browse/MESOS-7586
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The main functions now have a common initialization pattern:
>   1. Load flags.
>   2. Check if the --help flag has been passed.
>   3. Check if the flags are correct.
>   4. Initialize logging.
>   5. Parse and setup the environment variables.
>   6. Initialize libprocess.
> 
> This change reduces the number of messages "WARNING: Logging
> before InitGoogleLogging() is written to STDERR" as we use
> glog only after the fourth step in the modified files.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp aff97939b088e3747111ca6ec00fe76c59e9b396 
>   src/launcher/default_executor.cpp 30bae5c931c81b81ff9590eea8051b2232388813 
>   src/launcher/executor.cpp 65b9a2c52ba5a5a514fff992018e356f388468fc 
>   src/local/main.cpp f0d7e8ca1a64b5409632ea0c0894a1dc84f9796e 
>   src/master/main.cpp 1a78a55fd52c5eebad689c2a7f16c95979260d79 
>   src/slave/main.cpp a4a8ced6825ece1b25003824d987ff83fb799ad7 
> 
> 
> Diff: https://reviews.apache.org/r/61849/diff/3/
> 
> 
> Testing
> ---
> 
> ```
> $ make check
> ```
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 60915: Enabled filtering of resource allocations and reservations in agent.

[Alexander Rojas]

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


Ship it!




Ship It!

- Alexander Rojas


On July 21, 2017, 2:24 a.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60915/
> ---
> 
> (Updated July 21, 2017, 2:24 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Alexander Rojas, Benjamin 
> Mahler, Greg Mann, and haosdent huang.
> 
> 
> Bugs: MESOS-7892
> https://issues.apache.org/jira/browse/MESOS-7892
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds support of the `VIEW_ROLE` ACL to the results generated by the
> `/state` endpoint in the agent for fields `reserved_resources_full`,
> `reserved_resources` and `reserved_resources_allocated`.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp a828c5ff0654aea7be0ec47caac742a55fdfce56 
> 
> 
> Diff: https://reviews.apache.org/r/60915/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 61801: Used _EXIT macro in `CgroupsAnyHierarchyTest.ROOT_CGROUPS_Write` test.

[Mesos Reviewbot Windows]

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



Patch looks great!

Reviews applied: [61797, 61798, 61799, 61800, 61801]

Logs available here: http://104.210.40.105/logs/master/61801

- Mesos Reviewbot Windows


On Aug. 21, 2017, 8:54 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61801/
> ---
> 
> (Updated Aug. 21, 2017, 8:54 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Alexander Rojas, Benjamin 
> Bannier, Benjamin Mahler, and James Peach.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used _EXIT macro in `CgroupsAnyHierarchyTest.ROOT_CGROUPS_Write` test.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/cgroups_tests.cpp 
> c5d83e667da0b2c8c03937c66e2a973d547cd3d2 
> 
> 
> Diff: https://reviews.apache.org/r/61801/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check (mac os x, fedora 25)
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 61929: Parameterized the volume host path tests on executor type.

[Mesos Reviewbot Windows]

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



Patch looks great!

Reviews applied: [61899, 61900, 61901, 61902, 61903, 61904, 61905, 61906, 
61907, 61908, 61919, 61929]

Logs available here: http://104.210.40.105/logs/master/61929

- Mesos Reviewbot Windows


On Aug. 26, 2017, 8:21 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61929/
> ---
> 
> (Updated Aug. 26, 2017, 8:21 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Joseph Wu.
> 
> 
> Bugs: MESOS-7306
> https://issues.apache.org/jira/browse/MESOS-7306
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This allows us to test the isolator when the container is a normal
> container (command executor) or a nested container (default executor).
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/volume_host_path_isolator_tests.cpp PRE-CREATION 
>   src/tests/mesos.hpp f80e5fb0b76146e4748192361e8d1feff1ed687e 
> 
> 
> Diff: https://reviews.apache.org/r/61929/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 61921: Added tests to ensure that tasks can access their parent's volumes.

[Mesos Reviewbot Windows]

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



Failed to apply patch!

Reviews applied: [61921]

Logs available here: http://104.210.40.105/logs/master/61921

- Mesos Reviewbot Windows


On Aug. 25, 2017, 11:45 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61921/
> ---
> 
> (Updated Aug. 25, 2017, 11:45 p.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Bugs: MESOS-7916
> https://issues.apache.org/jira/browse/MESOS-7916
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These tests verifies that sibling tasks can share a Volume owned by
> their parent executor using 'sandbox_path' volumes.
> 
> 
> Diffs
> -
> 
>   src/tests/default_executor_tests.cpp 
> afe0afabf784fb65eb833beadd3c584722c321e1 
> 
> 
> Diff: https://reviews.apache.org/r/61921/diff/1/
> 
> 
> Testing
> ---
> 
> `sudo bin/mesos-tests.sh --gtest_filter="*TasksSharingViaSandboxVolumes*" 
> --gtest_repeat=500 --gtest_break_on_failure` on GNU/Linux.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 61920: Added a test that uses environment secrets and the DefaultExecutor.

[Mesos Reviewbot Windows]

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



Failed to apply patch!

Reviews applied: [61920]

Logs available here: http://104.210.40.105/logs/master/61920

- Mesos Reviewbot Windows


On Aug. 25, 2017, 11:45 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61920/
> ---
> 
> (Updated Aug. 25, 2017, 11:45 p.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Bugs: MESOS-7916
> https://issues.apache.org/jira/browse/MESOS-7916
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test checks that environment secrets are properly resolved and
> exposed to tasks started by the DefaultExecutor.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/environment_secret_isolator_tests.cpp 
> 2ba156f081c3a7c47d746f4750dbd74771b341c1 
> 
> 
> Diff: https://reviews.apache.org/r/61920/diff/1/
> 
> 
> Testing
> ---
> 
> The test succeeds on GNU/Linux.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 61927: MESOS-7912 In strict mode code, functions can only be declared at top level

[Mesos Reviewbot Windows]

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



Bad review!

Error:
No reviewers specified. Please find a reviewer by asking on JIRA or the mailing 
list.

- Mesos Reviewbot Windows


On Aug. 26, 2017, 9:14 p.m., Alastair Montgomery wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61927/
> ---
> 
> (Updated Aug. 26, 2017, 9:14 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-7912
> https://issues.apache.org/jira/browse/MESOS-7912
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix for https://issues.apache.org/jira/browse/MESOS-7912
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/js/controllers.js 27456144c 
> 
> 
> Diff: https://reviews.apache.org/r/61927/diff/1/
> 
> 
> Testing
> ---
> 
> Tested in Chrome Windows Version Version 37.0.2062.102 m and Mesos UI is now 
> displaying correctly where before it was throwing the strict error.
> 
> 
> Thanks,
> 
> Alastair Montgomery
> 
>