Re: Review Request 60562: Updated `accept` to perform operation adjustment in one place.

[Benjamin Mahler]

> On July 1, 2017, 2:13 a.m., Vinod Kone wrote:
> > src/master/master.cpp
> > Lines 3997 (patched)
> > 
> >
> > Can you log a warning here?

I wouldn't log a warning here since it's the responsibility of the processing 
of operations that occurs later to log accordingly when dealing with unknown 
operations.


- Benjamin


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


On June 30, 2017, 9:01 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60562/
> ---
> 
> (Updated June 30, 2017, 9:01 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7735
> https://issues.apache.org/jira/browse/MESOS-7735
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It used to be that the minor adjustments that were made to operations
> were done in various places across `accept` and `_accept`.
> 
> The "executor-injection" for LAUNCH_GROUP was at the beginning of
> `accept`, "allocation-info-injection" for MULTI_ROLE was after offer
> validation, and "health-check-injection" for LAUNCH was in `_accept`.
> 
> The `Master::accept` function is now broken down into distinct
> "metrics accounting", "offer validation", "operation-adjustments", and
> "authorization" stages.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 276b18e4d1bab7e9b28c1c93fe13c93a38abc5cd 
> 
> 
> Diff: https://reviews.apache.org/r/60562/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 60562: Updated `accept` to perform operation adjustment in one place.

[Benjamin Mahler]

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


Fix it, then Ship it!




Made some suggestions for cleanups, feel free to add TODOs and tackle them 
later in this chain. Looks good from a correctness perspective.


src/master/master.cpp
Lines 3883-3886 (original), 3870-3873 (patched)


I was about to make this exact suggestion to simplify this function, then 
studying the code more carefully I noticed we already added a TODO :)



src/master/master.cpp
Lines 3929-3930 (patched)


Any reason not to pull out the master specific normalization / upgrade of 
operations into a utility function that calls out to the various pieces? Seems 
to be a logical normalization / upgrade on Operations that the master needs to 
perform without consulting any of the master state?



src/master/master.cpp
Lines 3942-3947 (original), 4001-4005 (patched)


I would suggest putting the generalized normalization at the top before the 
switch, so that it's clear to the reader that there are general normalizations 
we apply before dealing with the operation specific logic here. At a first read 
of the code, it looks like it's all going to be within the switch.

Ideally, all the normalization could be factored together, I left another 
comment about that.



src/master/master.cpp
Lines 4738-4739 (original), 4764-4765 (patched)


Any reason these validateAndUpgradeResources in `_accept()` are not part of 
the normalization?

It's not obvious to me, maybe add a NOTE about how the normalization of 
operations isn't doing this, above the normalization block you added in 
accept()?


- Benjamin Mahler


On June 30, 2017, 9:01 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60562/
> ---
> 
> (Updated June 30, 2017, 9:01 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7735
> https://issues.apache.org/jira/browse/MESOS-7735
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It used to be that the minor adjustments that were made to operations
> were done in various places across `accept` and `_accept`.
> 
> The "executor-injection" for LAUNCH_GROUP was at the beginning of
> `accept`, "allocation-info-injection" for MULTI_ROLE was after offer
> validation, and "health-check-injection" for LAUNCH was in `_accept`.
> 
> The `Master::accept` function is now broken down into distinct
> "metrics accounting", "offer validation", "operation-adjustments", and
> "authorization" stages.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 276b18e4d1bab7e9b28c1c93fe13c93a38abc5cd 
> 
> 
> Diff: https://reviews.apache.org/r/60562/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 60564: Performed validation/upgrade of `Resource` objects before authorization.

[Vinod Kone]

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




src/master/master.cpp
Lines 3845-3856 (original), 3845-3856 (patched)


don't we need to do this (recovering of resources when offer is in error) 
in your below block as well?



src/master/master.cpp
Lines 3999 (patched)


see my comment above.  you need to recover resources for dropped 
operations. can you add a test?


- Vinod Kone


On June 30, 2017, 9:02 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60564/
> ---
> 
> (Updated June 30, 2017, 9:02 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7735
> https://issues.apache.org/jira/browse/MESOS-7735
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Performed validation/upgrade of `Resource` objects before authorization.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 95e9691b3e3ef101e3e7ddc7a498d5c5bf2276e3 
>   src/tests/master_tests.cpp cfb799fd105e9880cd56415b2a84e604c8f62703 
> 
> 
> Diff: https://reviews.apache.org/r/60564/diff/2/
> 
> 
> Testing
> ---
> 
> Added a new test + `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 60563: Updated `validateAndUpgradeResources` to operate on `Operation`s.

[Vinod Kone]

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


Ship it!





src/common/resources_utils.cpp
Line 199 (original), 199 (patched)


I feel like validate and upgrade could be split into 2 independent helpers, 
but if there is no use case yet, that's fine.


- Vinod Kone


On July 1, 2017, 12:56 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60563/
> ---
> 
> (Updated July 1, 2017, 12:56 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7735
> https://issues.apache.org/jira/browse/MESOS-7735
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated `validateAndUpgradeResources` to operate on `Operation`s.
> 
> 
> Diffs
> -
> 
>   src/common/resources_utils.hpp 7128297c45fbf94af9384b678bf201f3d9c48b65 
>   src/common/resources_utils.cpp 3a6a57817d4b0c4f4133b0a8d2b6f4d9ea31ea6b 
> 
> 
> Diff: https://reviews.apache.org/r/60563/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 60562: Updated `accept` to perform operation adjustment in one place.

[Vinod Kone]

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


Fix it, then Ship it!




This is great. Thanks for the cleanup.


src/master/master.cpp
Line 3819 (original)


Can you add a TODO to add metrics for LAUNCH_GROUP launch and decline as 
well?



src/master/master.cpp
Line 3940 (original), 3927 (patched)


move this to #4005?



src/master/master.cpp
Lines 3997 (patched)


Can you log a warning here?


- Vinod Kone


On June 30, 2017, 9:01 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60562/
> ---
> 
> (Updated June 30, 2017, 9:01 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7735
> https://issues.apache.org/jira/browse/MESOS-7735
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It used to be that the minor adjustments that were made to operations
> were done in various places across `accept` and `_accept`.
> 
> The "executor-injection" for LAUNCH_GROUP was at the beginning of
> `accept`, "allocation-info-injection" for MULTI_ROLE was after offer
> validation, and "health-check-injection" for LAUNCH was in `_accept`.
> 
> The `Master::accept` function is now broken down into distinct
> "metrics accounting", "offer validation", "operation-adjustments", and
> "authorization" stages.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 276b18e4d1bab7e9b28c1c93fe13c93a38abc5cd 
> 
> 
> Diff: https://reviews.apache.org/r/60562/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 60563: Updated `validateAndUpgradeResources` to operate on `Operation`s.

[Michael Park]

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

(Updated June 30, 2017, 5:56 p.m.)


Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description (updated)
---

Updated `validateAndUpgradeResources` to operate on `Operation`s.


Diffs (updated)
-

  src/common/resources_utils.hpp 7128297c45fbf94af9384b678bf201f3d9c48b65 
  src/common/resources_utils.cpp 3a6a57817d4b0c4f4133b0a8d2b6f4d9ea31ea6b 


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

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


Testing
---


Thanks,

Michael Park

Re: Review Request 60563: Updated `validateAndUpgradeResources` to operate on `Operation`s.

[Michael Park]

> On June 30, 2017, 12:44 p.m., Benjamin Bannier wrote:
> > src/common/resources_utils.cpp
> > Line 199 (original), 199 (patched)
> > 
> >
> > It seems by unconditionally calling `mutable_xyz` for `repeated` fields 
> > `xyz` below we might add default entries to them if they were empty before. 
> > AFAICTt this might not be what callers of this function expect.

Hm... do you mean the `mutable_resources()` calls below? I don't understand how 
we would be adding default entries. Could you be more specific?


> On June 30, 2017, 12:44 p.m., Benjamin Bannier wrote:
> > src/common/resources_utils.cpp
> > Line 201 (original), 201 (patched)
> > 
> >
> > Should we `CHECK_NOTNULL` somewhere here?

Sure.


- Michael


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


On June 30, 2017, 2:01 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60563/
> ---
> 
> (Updated June 30, 2017, 2:01 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7735
> https://issues.apache.org/jira/browse/MESOS-7735
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated `validateAndUpgradeResources` to operate on `Operation`s.
> 
> 
> Diffs
> -
> 
>   src/common/resources_utils.hpp 7128297c45fbf94af9384b678bf201f3d9c48b65 
>   src/common/resources_utils.cpp 3a6a57817d4b0c4f4133b0a8d2b6f4d9ea31ea6b 
> 
> 
> Diff: https://reviews.apache.org/r/60563/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 60414: Update documentation for framework bounding capabilities.

[Jie Yu]

> On July 1, 2017, 12:22 a.m., Jie Yu wrote:
> > Can you update the CHANGELOG as well?

And also upgrades.md.


- Jie


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


On June 24, 2017, 6:11 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60414/
> ---
> 
> (Updated June 24, 2017, 6:11 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-7671
> https://issues.apache.org/jira/browse/MESOS-7671
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update documentation for framework bounding capabilities.
> 
> 
> Diffs
> -
> 
>   docs/linux_capabilities.md 30b9f0c67ef3004b814d931eebca935f8c82db94 
> 
> 
> Diff: https://reviews.apache.org/r/60414/diff/2/
> 
> 
> Testing
> ---
> 
> None.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 60414: Update documentation for framework bounding capabilities.

[Jie Yu]

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



Can you update the CHANGELOG as well?

- Jie Yu


On June 24, 2017, 6:11 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60414/
> ---
> 
> (Updated June 24, 2017, 6:11 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-7671
> https://issues.apache.org/jira/browse/MESOS-7671
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update documentation for framework bounding capabilities.
> 
> 
> Diffs
> -
> 
>   docs/linux_capabilities.md 30b9f0c67ef3004b814d931eebca935f8c82db94 
> 
> 
> Diff: https://reviews.apache.org/r/60414/diff/2/
> 
> 
> Testing
> ---
> 
> None.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 60411: Allow frameworks to specify the capabilities bounding set.

[James Peach]

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

(Updated July 1, 2017, 12:12 a.m.)


Review request for mesos and Jie Yu.


Changes
---

Fix typo in description.


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


Repository: mesos


Description (updated)
---

Allow frameworks to specify the capabilities bounding set in the
LinuxInfo message. We need to explicitly make sure that this does
not exceed and bounding set specified by the operator, since that
is the outer limit of allowed privilege.


Diffs
-

  src/slave/containerizer/mesos/isolators/linux/capabilities.cpp 
ff7b9f19e69f51bcf80ab4f4920604f2002242f1 


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


Testing
---

make check (Fedora 25)


Thanks,

James Peach

Re: Review Request 60497: Update mesos-execute capabilities options.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On June 28, 2017, 7:54 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60497/
> ---
> 
> (Updated June 28, 2017, 7:54 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-7671
> https://issues.apache.org/jira/browse/MESOS-7671
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update mesos-execute capabilities options to match the
> framework capabilities fields. Rename `--capabilities`
> to `--effective_capabilities`, and add a new
> `--bounding_capabilities` option.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp 6e153446efe63700606ff33df1fb681438673084 
> 
> 
> Diff: https://reviews.apache.org/r/60497/diff/1/
> 
> 
> Testing
> ---
> 
> make check (Fedora 26)
> light manual testing
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 60414: Update documentation for framework bounding capabilities.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On June 24, 2017, 6:11 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60414/
> ---
> 
> (Updated June 24, 2017, 6:11 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-7671
> https://issues.apache.org/jira/browse/MESOS-7671
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update documentation for framework bounding capabilities.
> 
> 
> Diffs
> -
> 
>   docs/linux_capabilities.md 30b9f0c67ef3004b814d931eebca935f8c82db94 
> 
> 
> Diff: https://reviews.apache.org/r/60414/diff/2/
> 
> 
> Testing
> ---
> 
> None.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 60413: Rename and deprecate the LinuxInfo capability_info field.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On June 24, 2017, 6:11 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60413/
> ---
> 
> (Updated June 24, 2017, 6:11 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-7671
> https://issues.apache.org/jira/browse/MESOS-7671
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> For clarity, deprecate the LinuxInfo `capability_info`` field in
> favor of a new `effective_capabilities` field.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 3338349c399c9fb5b7fb2fc5886fb492ffb944c4 
>   include/mesos/v1/mesos.proto 6f0ad71260c36a9cb401c716e02c561580fb433b 
>   src/cli/execute.cpp 6e153446efe63700606ff33df1fb681438673084 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.cpp 
> ff7b9f19e69f51bcf80ab4f4920604f2002242f1 
>   src/tests/containerizer/linux_capabilities_isolator_tests.cpp 
> 5982c33a7493832c41cf89f9a78375167c1064bd 
> 
> 
> Diff: https://reviews.apache.org/r/60413/diff/3/
> 
> 
> Testing
> ---
> 
> make check (Fedora 25)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 60412: Add more linux/capabilities isolator test cases.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On June 24, 2017, 6:11 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60412/
> ---
> 
> (Updated June 24, 2017, 6:11 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-7671
> https://issues.apache.org/jira/browse/MESOS-7671
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add a case to verify that the effective framework capabilities
> must be within the bounding framework capabilities.
> 
> Check that setting the framework capabilities to something that
> is insufficient to execute ping overrides the operator flags and
> fails.
> 
> Check that setting the framework bounding capabilities to allow
> ping overrides the operator flags and succeeds.
> 
> Check that setting the framework effective and bounding
> capabilities to allow ping overrides the operator flags and
> succeeds.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/linux_capabilities_isolator_tests.cpp 
> 5982c33a7493832c41cf89f9a78375167c1064bd 
> 
> 
> Diff: https://reviews.apache.org/r/60412/diff/2/
> 
> 
> Testing
> ---
> 
> make check (Fedora 25)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 60411: Allow frameworks to specify the capabilities bounding set.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On June 24, 2017, 6:11 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60411/
> ---
> 
> (Updated June 24, 2017, 6:11 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-7671
> https://issues.apache.org/jira/browse/MESOS-7671
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow frameworks to specifc the capabilities bounding set in the
> LinuxInfo message. We need to explicitly make sure that this does
> not exceed and bounding set specified by the operator, since that
> is the outer limit of allowed privilege.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.cpp 
> ff7b9f19e69f51bcf80ab4f4920604f2002242f1 
> 
> 
> Diff: https://reviews.apache.org/r/60411/diff/2/
> 
> 
> Testing
> ---
> 
> make check (Fedora 25)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 60107: Added filtering to the '/tasks' endpoint.

[Quinn Leng]

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

(Updated June 30, 2017, 11:57 p.m.)


Review request for mesos, Anand Mazumdar, Alexander Rojas, Greg Mann, and Vinod 
Kone.


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


Repository: mesos


Description
---

Added filtering to the '/tasks' endpoint.


Diffs (updated)
-

  src/common/http.hpp 93d6088e97c2384f9f6d26e010a501abf2deb43e 
  src/common/http.cpp 2f7718cbc2e449b4f7c89754e8f84eac2f3c35b6 
  src/master/http.cpp 4dd43fd7c3fb986f4eed78bce574b6d3af156b67 
  src/tests/master_tests.cpp 6cd4be7e5ba48c6562ce91b28e76b065522cfbea 


Diff: https://reviews.apache.org/r/60107/diff/15/

Changes: https://reviews.apache.org/r/60107/diff/14-15/


Testing
---

Passed "make check"
Passed "GTEST_FILTER="MasterTest.TasksEndpoint" make check -j48"
Passed "GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="MasterTest.TasksEndpoint" 
--gtest_repeat=1000 --gtest_break_on_failure"


Thanks,

Quinn Leng

Re: Review Request 60410: Add bounding set support to linux/capabilities tests.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On June 24, 2017, 6:11 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60410/
> ---
> 
> (Updated June 24, 2017, 6:11 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-7671
> https://issues.apache.org/jira/browse/MESOS-7671
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add a framework bounding set parameter to the `linux/capabilities`
> isolator tests so that we can add parameterized test cases where
> the framework specified a bounding capabilities set.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/linux_capabilities_isolator_tests.cpp 
> 5982c33a7493832c41cf89f9a78375167c1064bd 
> 
> 
> Diff: https://reviews.apache.org/r/60410/diff/2/
> 
> 
> Testing
> ---
> 
> make check (Fedora 25)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 60409: Add bounding_capabilities to LinuxInfo.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On June 24, 2017, 6:11 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60409/
> ---
> 
> (Updated June 24, 2017, 6:11 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-7671
> https://issues.apache.org/jira/browse/MESOS-7671
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add a bounding_capabilities CapabilityInfo field to the LinuxInfo
> to carry the framework-specified capabilities bounding set.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 3338349c399c9fb5b7fb2fc5886fb492ffb944c4 
>   include/mesos/v1/mesos.proto 6f0ad71260c36a9cb401c716e02c561580fb433b 
> 
> 
> Diff: https://reviews.apache.org/r/60409/diff/2/
> 
> 
> Testing
> ---
> 
> make check (Fedora 25)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 60280: Provide full path to the custom executor.

[Jie Yu]

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




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


What if `launchInfo.working_directory()` is not set? Maybe use os::realpath 
here to get the absolute path?


- Jie Yu


On June 22, 2017, 3:20 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60280/
> ---
> 
> (Updated June 22, 2017, 3:20 p.m.)
> 
> 
> Review request for mesos, Jie Yu, James Peach, and Zhitao Li.
> 
> 
> Bugs: MESOS-7703
> https://issues.apache.org/jira/browse/MESOS-7703
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If a framework specifies use of its own executor and sets shell to false the 
> executor is never found. This provides the full path to the executor so that 
> the `execvp` or `execvpe` is successful.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.cpp 162ca1c2e 
> 
> 
> Diff: https://reviews.apache.org/r/60280/diff/3/
> 
> 
> Testing
> ---
> 
> `cd build && cmake .. -DCMAKE_BUILD_TYPE=Release && make -j4`
> Also spun up a master and agent, connected and sent a task using the UCR 
> (both with and without the use of an OCI image) via our own framework, and 
> checked the sandbox to verify that things went accordingly.
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>

Re: Review Request 60107: Added filtering to the '/tasks' endpoint.

[Quinn Leng]

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

(Updated June 30, 2017, 11:46 p.m.)


Review request for mesos, Anand Mazumdar, Alexander Rojas, Greg Mann, and Vinod 
Kone.


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


Repository: mesos


Description
---

Added filtering to the '/tasks' endpoint.


Diffs (updated)
-

  src/common/http.hpp 93d6088e97c2384f9f6d26e010a501abf2deb43e 
  src/common/http.cpp 2f7718cbc2e449b4f7c89754e8f84eac2f3c35b6 
  src/master/http.cpp 4dd43fd7c3fb986f4eed78bce574b6d3af156b67 
  src/tests/master_tests.cpp 6cd4be7e5ba48c6562ce91b28e76b065522cfbea 


Diff: https://reviews.apache.org/r/60107/diff/14/

Changes: https://reviews.apache.org/r/60107/diff/13-14/


Testing
---

Passed "make check"
Passed "GTEST_FILTER="MasterTest.TasksEndpoint" make check -j48"
Passed "GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="MasterTest.TasksEndpoint" 
--gtest_repeat=1000 --gtest_break_on_failure"


Thanks,

Quinn Leng

Re: Review Request 60107: Added filtering to the '/tasks' endpoint.

[Greg Mann]

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




src/master/http.cpp
Lines 3758 (patched)


You need a space at the end of this line, so it merges well with the 
subsequent line.



src/master/http.cpp
Lines 3760 (patched)


You need a space at the end of this line, so it merges well with the 
subsequent line.


- Greg Mann


On June 30, 2017, 11:37 p.m., Quinn Leng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60107/
> ---
> 
> (Updated June 30, 2017, 11:37 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Alexander Rojas, Greg Mann, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-7630
> https://issues.apache.org/jira/browse/MESOS-7630
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added filtering to the '/tasks' endpoint.
> 
> 
> Diffs
> -
> 
>   src/common/http.hpp 93d6088e97c2384f9f6d26e010a501abf2deb43e 
>   src/common/http.cpp 2f7718cbc2e449b4f7c89754e8f84eac2f3c35b6 
>   src/master/http.cpp 4dd43fd7c3fb986f4eed78bce574b6d3af156b67 
>   src/tests/master_tests.cpp 6cd4be7e5ba48c6562ce91b28e76b065522cfbea 
> 
> 
> Diff: https://reviews.apache.org/r/60107/diff/13/
> 
> 
> Testing
> ---
> 
> Passed "make check"
> Passed "GTEST_FILTER="MasterTest.TasksEndpoint" make check -j48"
> Passed "GLOG_v=1 ./bin/mesos-tests.sh 
> --gtest_filter="MasterTest.TasksEndpoint" --gtest_repeat=1000 
> --gtest_break_on_failure"
> 
> 
> Thanks,
> 
> Quinn Leng
> 
>

Re: Review Request 60107: Added filtering to the '/tasks' endpoint.

[Greg Mann]

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




src/master/http.cpp
Lines 3758-3762 (original), 3758-3767 (patched)


s/The id/The ID/



src/master/http.cpp
Lines 3758-3759 (patched)


Could you put the period at the end, after the parentheses (the same as the 
existing cases here).



src/master/http.cpp
Lines 3760 (patched)


s/The id/The ID/


- Greg Mann


On June 30, 2017, 11:37 p.m., Quinn Leng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60107/
> ---
> 
> (Updated June 30, 2017, 11:37 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Alexander Rojas, Greg Mann, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-7630
> https://issues.apache.org/jira/browse/MESOS-7630
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added filtering to the '/tasks' endpoint.
> 
> 
> Diffs
> -
> 
>   src/common/http.hpp 93d6088e97c2384f9f6d26e010a501abf2deb43e 
>   src/common/http.cpp 2f7718cbc2e449b4f7c89754e8f84eac2f3c35b6 
>   src/master/http.cpp 4dd43fd7c3fb986f4eed78bce574b6d3af156b67 
>   src/tests/master_tests.cpp 6cd4be7e5ba48c6562ce91b28e76b065522cfbea 
> 
> 
> Diff: https://reviews.apache.org/r/60107/diff/13/
> 
> 
> Testing
> ---
> 
> Passed "make check"
> Passed "GTEST_FILTER="MasterTest.TasksEndpoint" make check -j48"
> Passed "GLOG_v=1 ./bin/mesos-tests.sh 
> --gtest_filter="MasterTest.TasksEndpoint" --gtest_repeat=1000 
> --gtest_break_on_failure"
> 
> 
> Thanks,
> 
> Quinn Leng
> 
>

Re: Review Request 60107: Added filtering to the '/tasks' endpoint.

[Greg Mann]

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




src/master/http.cpp
Lines 3758-3762 (original), 3758-3767 (patched)


Let's list these query parameters alphabetically.


- Greg Mann


On June 30, 2017, 11:37 p.m., Quinn Leng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60107/
> ---
> 
> (Updated June 30, 2017, 11:37 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Alexander Rojas, Greg Mann, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-7630
> https://issues.apache.org/jira/browse/MESOS-7630
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added filtering to the '/tasks' endpoint.
> 
> 
> Diffs
> -
> 
>   src/common/http.hpp 93d6088e97c2384f9f6d26e010a501abf2deb43e 
>   src/common/http.cpp 2f7718cbc2e449b4f7c89754e8f84eac2f3c35b6 
>   src/master/http.cpp 4dd43fd7c3fb986f4eed78bce574b6d3af156b67 
>   src/tests/master_tests.cpp 6cd4be7e5ba48c6562ce91b28e76b065522cfbea 
> 
> 
> Diff: https://reviews.apache.org/r/60107/diff/13/
> 
> 
> Testing
> ---
> 
> Passed "make check"
> Passed "GTEST_FILTER="MasterTest.TasksEndpoint" make check -j48"
> Passed "GLOG_v=1 ./bin/mesos-tests.sh 
> --gtest_filter="MasterTest.TasksEndpoint" --gtest_repeat=1000 
> --gtest_break_on_failure"
> 
> 
> Thanks,
> 
> Quinn Leng
> 
>

Re: Review Request 60107: Added filtering to the '/tasks' endpoint.

[Quinn Leng]

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

(Updated June 30, 2017, 11:37 p.m.)


Review request for mesos, Anand Mazumdar, Alexander Rojas, Greg Mann, and Vinod 
Kone.


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


Repository: mesos


Description
---

Added filtering to the '/tasks' endpoint.


Diffs (updated)
-

  src/common/http.hpp 93d6088e97c2384f9f6d26e010a501abf2deb43e 
  src/common/http.cpp 2f7718cbc2e449b4f7c89754e8f84eac2f3c35b6 
  src/master/http.cpp 4dd43fd7c3fb986f4eed78bce574b6d3af156b67 
  src/tests/master_tests.cpp 6cd4be7e5ba48c6562ce91b28e76b065522cfbea 


Diff: https://reviews.apache.org/r/60107/diff/13/

Changes: https://reviews.apache.org/r/60107/diff/12-13/


Testing
---

Passed "make check"
Passed "GTEST_FILTER="MasterTest.TasksEndpoint" make check -j48"
Passed "GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="MasterTest.TasksEndpoint" 
--gtest_repeat=1000 --gtest_break_on_failure"


Thanks,

Quinn Leng

Re: Review Request 60203: Introduce HTB class.

[Jie Yu]

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


Fix it, then Ship it!





src/linux/routing/queueing/class.hpp
Lines 31-35 (patched)


I'd move this to internal.hpp to be consistent with others.



src/linux/routing/queueing/htb.hpp
Lines 69 (patched)


i like the namespace idea. Can we put qdisc related functions under `qdisc` 
namespace?



src/linux/routing/queueing/htb.cpp
Line 41 (original), 45 (patched)


This can be qdisk::Config (similar to cls::Config)!



src/linux/routing/queueing/htb.cpp
Lines 195 (patched)


2 lines apart



src/linux/routing/queueing/internal.hpp
Lines 127 (patched)


one line above


- Jie Yu


On June 30, 2017, 11:01 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60203/
> ---
> 
> (Updated June 30, 2017, 11:01 p.m.)
> 
> 
> Review request for mesos, Ilya Pronin, Jie Yu, Santhosh Kumar Shanmugham, and 
> Cong Wang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Based heavily on Cong Wang's original review 45605.
> 
> Introduce support for HTB class and expose configuration.
> 
> 
> Diffs
> -
> 
>   src/linux/routing/queueing/class.hpp PRE-CREATION 
>   src/linux/routing/queueing/htb.hpp 857646190d21387f98832f5094128505a52a0776 
>   src/linux/routing/queueing/htb.cpp 464120214b75e2e422c8ea6a57c5654ba77d669f 
>   src/linux/routing/queueing/internal.hpp 
> 9fe522ee017c86af8c7b2e518cd0957af08750e4 
> 
> 
> Diff: https://reviews.apache.org/r/60203/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 60107: Added filtering to the '/tasks' endpoint.

[Greg Mann]

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



Also note: we need to follow up these patches with updates to the 
documentation/endpoint help strings which explain the new query parameters.

- Greg Mann


On June 30, 2017, 11:26 p.m., Quinn Leng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60107/
> ---
> 
> (Updated June 30, 2017, 11:26 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Alexander Rojas, Greg Mann, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-7630
> https://issues.apache.org/jira/browse/MESOS-7630
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added filtering to the '/tasks' endpoint.
> 
> 
> Diffs
> -
> 
>   src/common/http.hpp 93d6088e97c2384f9f6d26e010a501abf2deb43e 
>   src/common/http.cpp 2f7718cbc2e449b4f7c89754e8f84eac2f3c35b6 
>   src/master/http.cpp 4dd43fd7c3fb986f4eed78bce574b6d3af156b67 
>   src/tests/master_tests.cpp 6cd4be7e5ba48c6562ce91b28e76b065522cfbea 
> 
> 
> Diff: https://reviews.apache.org/r/60107/diff/12/
> 
> 
> Testing
> ---
> 
> Passed "make check"
> Passed "GTEST_FILTER="MasterTest.TasksEndpoint" make check -j48"
> Passed "GLOG_v=1 ./bin/mesos-tests.sh 
> --gtest_filter="MasterTest.TasksEndpoint" --gtest_repeat=1000 
> --gtest_break_on_failure"
> 
> 
> Thanks,
> 
> Quinn Leng
> 
>

Re: Review Request 60107: Added filtering to the '/tasks' endpoint.

[Quinn Leng]

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

(Updated June 30, 2017, 11:26 p.m.)


Review request for mesos, Anand Mazumdar, Alexander Rojas, Greg Mann, and Vinod 
Kone.


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


Repository: mesos


Description (updated)
---

Added filtering to the '/tasks' endpoint.


Diffs (updated)
-

  src/common/http.hpp 93d6088e97c2384f9f6d26e010a501abf2deb43e 
  src/common/http.cpp 2f7718cbc2e449b4f7c89754e8f84eac2f3c35b6 
  src/master/http.cpp 4dd43fd7c3fb986f4eed78bce574b6d3af156b67 
  src/tests/master_tests.cpp 6cd4be7e5ba48c6562ce91b28e76b065522cfbea 


Diff: https://reviews.apache.org/r/60107/diff/12/

Changes: https://reviews.apache.org/r/60107/diff/11-12/


Testing
---

Passed "make check"
Passed "GTEST_FILTER="MasterTest.TasksEndpoint" make check -j48"
Passed "GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="MasterTest.TasksEndpoint" 
--gtest_repeat=1000 --gtest_break_on_failure"


Thanks,

Quinn Leng

Re: Review Request 60107: Added filtering to the '/tasks' endpoint.

[Greg Mann]

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




src/tests/master_tests.cpp
Lines 3510-3543 (patched)


It looks like we now have the framework ID in one of these expected results 
but not the other - could we be consistent here?


- Greg Mann


On June 30, 2017, 1:37 a.m., Quinn Leng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60107/
> ---
> 
> (Updated June 30, 2017, 1:37 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Alexander Rojas, Greg Mann, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-7630
> https://issues.apache.org/jira/browse/MESOS-7630
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added filtering to the '/tasks' endpoint.
> 
> Added query parameter support for '/tasks' endpoint.
> To query a specific task, you can specify 'task_id'
> and 'framework_id' in the query parameter. Thus the
> format would look like 
> {{'/tasks?framework_id=[framework id]_id=[task id]'}}
> 
> If no task id and framework id specified, it will return
> the whole list of tasks just like previous version.
> 
> 
> Diffs
> -
> 
>   src/common/http.hpp 93d6088e97c2384f9f6d26e010a501abf2deb43e 
>   src/common/http.cpp 2f7718cbc2e449b4f7c89754e8f84eac2f3c35b6 
>   src/master/http.cpp 4dd43fd7c3fb986f4eed78bce574b6d3af156b67 
>   src/tests/master_tests.cpp 6cd4be7e5ba48c6562ce91b28e76b065522cfbea 
> 
> 
> Diff: https://reviews.apache.org/r/60107/diff/11/
> 
> 
> Testing
> ---
> 
> Passed "make check"
> Passed "GTEST_FILTER="MasterTest.TasksEndpoint" make check -j48"
> Passed "GLOG_v=1 ./bin/mesos-tests.sh 
> --gtest_filter="MasterTest.TasksEndpoint" --gtest_repeat=1000 
> --gtest_break_on_failure"
> 
> 
> Thanks,
> 
> Quinn Leng
> 
>

Re: Review Request 60203: Introduce HTB class.

[Ian Downes]

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

(Updated June 30, 2017, 4:01 p.m.)


Review request for mesos, Ilya Pronin, Jie Yu, Santhosh Kumar Shanmugham, and 
Cong Wang.


Changes
---

Addressed comments. Shuffled some code for cleaner naming.


Repository: mesos


Description
---

Based heavily on Cong Wang's original review 45605.

Introduce support for HTB class and expose configuration.


Diffs (updated)
-

  src/linux/routing/queueing/class.hpp PRE-CREATION 
  src/linux/routing/queueing/htb.hpp 857646190d21387f98832f5094128505a52a0776 
  src/linux/routing/queueing/htb.cpp 464120214b75e2e422c8ea6a57c5654ba77d669f 
  src/linux/routing/queueing/internal.hpp 
9fe522ee017c86af8c7b2e518cd0957af08750e4 


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

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


Testing
---

make check


Thanks,

Ian Downes

Re: Review Request 59294: Optionally scale egress bandwidth with CPU.

[Ian Downes]

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

(Updated June 30, 2017, 4 p.m.)


Review request for mesos, Dmitry Zhuk, Ilya Pronin, and Jie Yu.


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

Add support to isolators/port_mapping for optionally scaling egress bandwidth 
with CPU and with minimum and maximum limits.


Diffs (updated)
-

  include/mesos/mesos.proto e2502acc3542d64c2b2b13f8c0363a3c8372e20e 
  src/slave/containerizer/mesos/isolators/network/helper.cpp 
4ed879dca42f85fc9dd7638e763822cf10fa8405 
  src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
9d38289c7161d5e931053b587d115684ccc44c94 
  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
cd008aaebcd42554a9a81d2b059269546f59c966 
  src/slave/flags.hpp e75c1b4227b443aedf445921b3f2108d930c112c 
  src/slave/flags.cpp c84aa6724170bba46bbe8410b71d42a1626e 
  src/tests/containerizer/port_mapping_tests.cpp 
16e015a8ac53a4aa5336b60c40228720b5f6910a 


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

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


Testing
---

# added a new test 
$ make check


Thanks,

Ian Downes

Re: Review Request 60203: Introduce HTB class.

[Ian Downes]

- Ian


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


On June 19, 2017, 12:36 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60203/
> ---
> 
> (Updated June 19, 2017, 12:36 p.m.)
> 
> 
> Review request for mesos, Ilya Pronin, Jie Yu, Santhosh Kumar Shanmugham, and 
> Cong Wang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Based heavily on Cong Wang's original review 45605.
> 
> Introduce support for HTB class and expose configuration.
> 
> 
> Diffs
> -
> 
>   src/linux/routing/queueing/class.hpp PRE-CREATION 
>   src/linux/routing/queueing/htb.hpp 857646190d21387f98832f5094128505a52a0776 
>   src/linux/routing/queueing/htb.cpp 464120214b75e2e422c8ea6a57c5654ba77d669f 
>   src/linux/routing/queueing/internal.hpp 
> 9fe522ee017c86af8c7b2e518cd0957af08750e4 
> 
> 
> Diff: https://reviews.apache.org/r/60203/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 60203: Introduce HTB class.

[Ian Downes]

> On June 23, 2017, 6:02 a.m., Ilya Pronin wrote:
> > src/linux/routing/queueing/internal.hpp
> > Lines 595-597 (patched)
> > 
> >
> > Why don't we handle this here to be consistent with other `*Class()` 
> > functions?

I read through the error handling chain and for rtnl_class_delete it doesn't 
appear to check for class existence and return an interpretable error code. 
I've left the current generic error handling and removed the erroneous comment.


- Ian


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


On June 19, 2017, 12:36 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60203/
> ---
> 
> (Updated June 19, 2017, 12:36 p.m.)
> 
> 
> Review request for mesos, Ilya Pronin, Jie Yu, Santhosh Kumar Shanmugham, and 
> Cong Wang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Based heavily on Cong Wang's original review 45605.
> 
> Introduce support for HTB class and expose configuration.
> 
> 
> Diffs
> -
> 
>   src/linux/routing/queueing/class.hpp PRE-CREATION 
>   src/linux/routing/queueing/htb.hpp 857646190d21387f98832f5094128505a52a0776 
>   src/linux/routing/queueing/htb.cpp 464120214b75e2e422c8ea6a57c5654ba77d669f 
>   src/linux/routing/queueing/internal.hpp 
> 9fe522ee017c86af8c7b2e518cd0957af08750e4 
> 
> 
> Diff: https://reviews.apache.org/r/60203/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 59294: Optionally scale egress bandwidth with CPU.

[Ian Downes]

> On June 26, 2017, 6:08 a.m., Ilya Pronin wrote:
> > src/slave/containerizer/mesos/isolators/network/port_mapping.cpp
> > Lines 2209-2210 (original), 2301-2302 (patched)
> > 
> >
> > Do we expect the helper to hang? Is this a safeguard for the new code 
> > deployment?

Nope, it's not expected to hang but the 10 second timeout is indeed a safety 
check in case something is amiss. This method is only used during recovery and 
in tests.


> On June 26, 2017, 6:08 a.m., Ilya Pronin wrote:
> > src/slave/containerizer/mesos/isolators/network/port_mapping.cpp
> > Lines 4315-4317 (patched)
> > 
> >
> > If we set `ceil` low, it will prevent heavy CPU users' from bursting. 
> > Wouldn't it be more flexible to make `egress_ceil_limit` flag additive 
> > instead of absolute?

Additive burst makes it more complicated as you need to at least min(rate + 
ceil, max) or scale ceil with cpu as well. Instead, I think it's simpler to 
scale rate by cpu and burst naturally becomes progressively less useful for 
higher cpu.


- Ian


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


On June 21, 2017, 1:48 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59294/
> ---
> 
> (Updated June 21, 2017, 1:48 p.m.)
> 
> 
> Review request for mesos, Dmitry Zhuk, Ilya Pronin, and Jie Yu.
> 
> 
> Bugs: MESOS-7508
> https://issues.apache.org/jira/browse/MESOS-7508
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add support to isolators/port_mapping for optionally scaling egress bandwidth 
> with CPU and with minimum and maximum limits.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto e2502acc3542d64c2b2b13f8c0363a3c8372e20e 
>   src/slave/containerizer/mesos/isolators/network/helper.cpp 
> 4ed879dca42f85fc9dd7638e763822cf10fa8405 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
> 9d38289c7161d5e931053b587d115684ccc44c94 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> cd008aaebcd42554a9a81d2b059269546f59c966 
>   src/slave/flags.hpp e75c1b4227b443aedf445921b3f2108d930c112c 
>   src/slave/flags.cpp c84aa6724170bba46bbe8410b71d42a1626e 
>   src/tests/containerizer/port_mapping_tests.cpp 
> 16e015a8ac53a4aa5336b60c40228720b5f6910a 
> 
> 
> Diff: https://reviews.apache.org/r/59294/diff/4/
> 
> 
> Testing
> ---
> 
> # added a new test 
> $ make check
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 60581: Added filtering to the '/slaves' endpoint.

[Quinn Leng]

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

(Updated June 30, 2017, 10:44 p.m.)


Review request for mesos, Anand Mazumdar, Alexander Rojas, Greg Mann, and Vinod 
Kone.


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


Repository: mesos


Description (updated)
---

Added filtering to the '/slaves' endpoint.

Added query parameter support for 'slaves'
endpoint. We can use 'slave_id' to specify
which slave information to query. Thus it
looks like '/slaves?slave_id=[slave id]'

If not slave id is specified, it will return
information of all slaves, just like before

The structure for response is the same for both
specifying or not specifying slave id. Thus even
for request with slave id specified, the response
will still contain both fields 'slaves', 
'recovered_slaves' etc..


Diffs (updated)
-

  src/common/http.hpp 93d6088e97c2384f9f6d26e010a501abf2deb43e 
  src/common/http.cpp 2f7718cbc2e449b4f7c89754e8f84eac2f3c35b6 
  src/master/http.cpp 4dd43fd7c3fb986f4eed78bce574b6d3af156b67 
  src/tests/master_tests.cpp 6cd4be7e5ba48c6562ce91b28e76b065522cfbea 


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

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


Testing
---

Passed 'make check -j48'
Passed 'GTEST_FILTER="MasterTest.SlavesEndpointQuerySlave" make check -j48'
Passed 'GLOG_v=1 ./bin/mesos-tests.sh 
--gtest_filter="MasterTest.SlavesEndpointQuerySlave" --gtest_repeat=1000 
--gtest_break_on_failure'


Thanks,

Quinn Leng

Review Request 60581: Added filtering to the '/slaves' endpoint.

[Quinn Leng]

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

Review request for mesos, Anand Mazumdar, Alexander Rojas, Greg Mann, and Vinod 
Kone.


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


Repository: mesos


Description
---

Added filtering to the '/slaves' endpoint.

Add query parameter support for that endpoint,
to get the information of one slave, we can use
'slave_id' as the query parameter. Thus it looks 
like '/slaves?slave_id=[slave id]'

When no slave_id is specified, it will return
information of all slaves.

The structure for the response is reserved, thus
even if slave_id is specified, the structure for
response still contains field 'slaves' and
'recovered_slaves' etc..


Diffs
-

  src/common/http.hpp 93d6088e97c2384f9f6d26e010a501abf2deb43e 
  src/common/http.cpp 2f7718cbc2e449b4f7c89754e8f84eac2f3c35b6 
  src/master/http.cpp 4dd43fd7c3fb986f4eed78bce574b6d3af156b67 
  src/tests/.master_tests.cpp.swp PRE-CREATION 
  src/tests/master_tests.cpp 6cd4be7e5ba48c6562ce91b28e76b065522cfbea 


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


Testing
---

Passed 'make check -j48'
Passed 'GTEST_FILTER="MasterTest.SlavesEndpointQuerySlave" make check -j48'
Passed 'GLOG_v=1 ./bin/mesos-tests.sh 
--gtest_filter="MasterTest.SlavesEndpointQuerySlave" --gtest_repeat=1000 
--gtest_break_on_failure'


Thanks,

Quinn Leng

Re: Review Request 60520: Remove posix/subprocess.hpp dependencies.

[James Peach]

> On June 30, 2017, 10:24 p.m., Gastón Kleiman wrote:
> > docs/modules.md
> > Line 91 (original), 91 (patched)
> > 
> >
> > Is the trailing comma here valid?

Oops, looke like I picked up an unrelated change. I'll rebase this away.


- James


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


On June 28, 2017, 10:49 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60520/
> ---
> 
> (Updated June 28, 2017, 10:49 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, and Jiang Yan 
> Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix a number of places where we were depending on symbols made
> visible by . In most cases, this is
> just tweaking the includes, but in the Checker we just duplicate
> the trivial fork(2) helper since that is easer than trying to
> export it as a libprocess API.
> 
> 
> Diffs
> -
> 
>   docs/modules.md c5231e242cfe42834beeb98846c9308e0fd8dff3 
>   src/checks/checker_process.cpp 66f5452bb5f9c7f45819c221be7b098e5999b7a7 
>   src/slave/containerizer/mesos/isolators/volume/image.cpp 
> c89af3e0a16f769c45f2846d1e76baedb8248d4d 
>   src/uri/fetchers/docker.cpp 7f3e4e73d1dc5d15f875e2890acbe5369fb59d78 
> 
> 
> Diff: https://reviews.apache.org/r/60520/diff/1/
> 
> 
> Testing
> ---
> 
> make check (Fedora 26).
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 60520: Remove posix/subprocess.hpp dependencies.

[Gastón Kleiman]

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




docs/modules.md
Line 91 (original), 91 (patched)


Is the trailing comma here valid?



src/checks/checker_process.cpp
Lines 101 (patched)


We might want to add a comment here explaining why we need to duplicate the 
helper.

I'll leave this up to the shepherd, but the method is trivial enough that 
we might want to just expand it in `cloneWithSetns`.


- Gastón Kleiman


On June 28, 2017, 10:49 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60520/
> ---
> 
> (Updated June 28, 2017, 10:49 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, and Jiang Yan 
> Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix a number of places where we were depending on symbols made
> visible by . In most cases, this is
> just tweaking the includes, but in the Checker we just duplicate
> the trivial fork(2) helper since that is easer than trying to
> export it as a libprocess API.
> 
> 
> Diffs
> -
> 
>   docs/modules.md c5231e242cfe42834beeb98846c9308e0fd8dff3 
>   src/checks/checker_process.cpp 66f5452bb5f9c7f45819c221be7b098e5999b7a7 
>   src/slave/containerizer/mesos/isolators/volume/image.cpp 
> c89af3e0a16f769c45f2846d1e76baedb8248d4d 
>   src/uri/fetchers/docker.cpp 7f3e4e73d1dc5d15f875e2890acbe5369fb59d78 
> 
> 
> Diff: https://reviews.apache.org/r/60520/diff/1/
> 
> 
> Testing
> ---
> 
> make check (Fedora 26).
> 
> 
> Thanks,
> 
> James Peach
> 
>

Review Request 60580: Added filtering to the '/frameworks' endpoint.

[Quinn Leng]

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

Review request for mesos, Anand Mazumdar, Alexander Rojas, Greg Mann, and Vinod 
Kone.


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


Repository: mesos


Description
---

Added filtering to the '/frameworks' endpoint.

Add query parameter support for '/frameworks' endpoint.
To query the information about a specific framework, we
can use 'framework_id' to specify the framework, thus it
looks like'/frameworks?framework_id=[framework id]'

To maintain consistency, the structure for response is 
reserved, thus the response still contains field 'frameworks'
and 'completed_frameworks' etc..

Since code for '/state' endpoint and '/frameworks' are coupled
(they share 'FullFrameworkWriter' class), thus the code for '/state'
endpoint is also updated.


Diffs
-

  src/common/http.hpp 93d6088e97c2384f9f6d26e010a501abf2deb43e 
  src/common/http.cpp 2f7718cbc2e449b4f7c89754e8f84eac2f3c35b6 
  src/master/http.cpp 4dd43fd7c3fb986f4eed78bce574b6d3af156b67 
  src/tests/master_tests.cpp 6cd4be7e5ba48c6562ce91b28e76b065522cfbea 


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


Testing
---

Passed 'make check -j48'
Passed 'GTEST_FILTER="MasterTest.FrameworksEndpointQueryFramework" make check 
-j48'
Passed 'GLOG_v=1 ./bin/mesos-tests.sh 
--gtest_filter="MasterTest.FrameworksEndpointQueryFramework" 
--gtest_repeat=1000 --gtest_break_on_failure'


Thanks,

Quinn Leng

Re: Review Request 60566: Validated and upgraded resources on the V1 operator API path.

[Benjamin Bannier]

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




src/tests/master_tests.cpp
Lines 7695 (patched)


Let's not depend on master implementation details here just to get the 
`SlaveID`, instead just intercept the `SlaveRegistrationMessage` like below.



src/tests/master_tests.cpp
Lines 7797 (patched)


Ditto.


- Benjamin Bannier


On June 30, 2017, 2:56 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60566/
> ---
> 
> (Updated June 30, 2017, 2:56 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 4dd43fd7c3fb986f4eed78bce574b6d3af156b67 
>   src/master/master.hpp 9dd6a530c373516dc81bfd51ee6e95f25588148f 
>   src/tests/master_tests.cpp cfb799fd105e9880cd56415b2a84e604c8f62703 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 518bdf858096ec9bcfa4f899ead5a6c3d103c521 
>   src/tests/reservation_endpoints_tests.cpp 
> f710a188a7875c1cb847e39276b4b65332703ca5 
> 
> 
> Diff: https://reviews.apache.org/r/60566/diff/1/
> 
> 
> Testing
> ---
> 
> Added new tests + `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 60564: Performed validation/upgrade of `Resource` objects before authorization.

[Benjamin Bannier]

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


Fix it, then Ship it!





src/master/master.cpp
Lines 3955-3978 (patched)


I find these nested `switch` statements (with `default` cases) pretty hard 
to read. What about pulling below `for` loop into a lambda, and calling it from 
explicit cases for `LAUNCH` or `LAUNCHGROUP` instead.


- Benjamin Bannier


On June 30, 2017, 11:02 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60564/
> ---
> 
> (Updated June 30, 2017, 11:02 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7735
> https://issues.apache.org/jira/browse/MESOS-7735
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 276b18e4d1bab7e9b28c1c93fe13c93a38abc5cd 
>   src/tests/master_tests.cpp cfb799fd105e9880cd56415b2a84e604c8f62703 
> 
> 
> Diff: https://reviews.apache.org/r/60564/diff/1/
> 
> 
> Testing
> ---
> 
> Added a new test + `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 60563: Updated `validateAndUpgradeResources` to operate on `Operation`s.

[Benjamin Bannier]

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




src/common/resources_utils.cpp
Line 199 (original), 199 (patched)


It seems by unconditionally calling `mutable_xyz` for `repeated` fields 
`xyz` below we might add default entries to them if they were empty before. 
AFAICTt this might not be what callers of this function expect.



src/common/resources_utils.cpp
Line 201 (original), 201 (patched)


Should we `CHECK_NOTNULL` somewhere here?


- Benjamin Bannier


On June 30, 2017, 11:01 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60563/
> ---
> 
> (Updated June 30, 2017, 11:01 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7735
> https://issues.apache.org/jira/browse/MESOS-7735
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/common/resources_utils.hpp 7128297c45fbf94af9384b678bf201f3d9c48b65 
>   src/common/resources_utils.cpp 3a6a57817d4b0c4f4133b0a8d2b6f4d9ea31ea6b 
> 
> 
> Diff: https://reviews.apache.org/r/60563/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 60562: Updated `accept` to perform operation adjustment in one place.

[Benjamin Bannier]

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




src/master/master.cpp
Lines 3929 (patched)


Just for clarification, is the plan for this block to ultimately absorb the 
mutating part of `validateAndUpgrade`? Looking at your patch 
https://reviews.apache.org/r/60563/ makes this look pretty (validation and 
update order inverted there wrt. here).



src/master/master.cpp
Lines 3932-3936 (patched)


Nice to see these finally handled by a `switch` instead of a couple of `if` 
statements!


- Benjamin Bannier


On June 30, 2017, 11:01 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60562/
> ---
> 
> (Updated June 30, 2017, 11:01 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7735
> https://issues.apache.org/jira/browse/MESOS-7735
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It used to be that the minor adjustments that were made to operations
> were done in various places across `accept` and `_accept`.
> 
> The "executor-injection" for LAUNCH_GROUP was at the beginning of
> `accept`, "allocation-info-injection" for MULTI_ROLE was after offer
> validation, and "health-check-injection" for LAUNCH was in `_accept`.
> 
> The `Master::accept` function is now broken down into distinct
> "metrics accounting", "offer validation", "operation-adjustments", and
> "authorization" stages.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 276b18e4d1bab7e9b28c1c93fe13c93a38abc5cd 
> 
> 
> Diff: https://reviews.apache.org/r/60562/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 60397: Check perf version compatibility in tests with disabled coredumps.

[Andrei Budnik]

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

(Updated June 30, 2017, 5:41 p.m.)


Review request for mesos, Benjamin Bannier, James Peach, and Neil Conway.


Summary (updated)
-

Check perf version compatibility in tests with disabled coredumps.


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


Repository: mesos


Description
---

For autotools build, the docker-build script performs a 'distcheck'
build. This type of build warns if any unexpected files are left in
the build directory after an uninstall, mainly to detect broken
uninstall Makefile rules. The return status of the build container is
the result of the distcheck.
This fixes an issue where in some dockerized configurations
invocations of 'perf' segfaulted (producing a core file as a
side-effect), where the failure case was already anticipated and
handled by the caller.


Diffs (updated)
-

  src/tests/containerizer/perf_tests.cpp 
d8aab08eb131f974821fb85662cbc6cc685d2f3e 
  src/tests/environment.cpp a7262cd97361b55ff31238341657e764df6c9ea5 


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

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


Testing
---

1. make check (mac os x 10.12, fedora 25)
2. internal CI

Needs to be confirmed by Apache CI, e.g., reviewbot.


Thanks,

Andrei Budnik

Re: Review Request 59599: Added 'type' and 'name' fields to ResourceProviderInfo.

[Benjamin Bannier]

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


Ship it!




Ship It!

- Benjamin Bannier


On June 26, 2017, 6:30 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59599/
> ---
> 
> (Updated June 26, 2017, 6:30 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Chun-Hung Hsiao, 
> Joseph Wu, and Jan Schlicht.
> 
> 
> Bugs: MESOS-7560
> https://issues.apache.org/jira/browse/MESOS-7560
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added 'type' and 'name' fields to ResourceProviderInfo.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 38449da44a5288afc7e2ef17ef5e1380ecabd5d2 
>   include/mesos/v1/mesos.proto 81eca47ce1074f5e522a0410f01ab2b2cfc76aa2 
> 
> 
> Diff: https://reviews.apache.org/r/59599/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 56895: Added tests to ensure slave recovery post reboot.

[Megha Sharma]

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

(Updated June 30, 2017, 3:18 p.m.)


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


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


Repository: mesos


Description
---

Added tests to verify that the state is recovered post reboot and
agentId is retained given the recovery finishes without errors and
if the recovery fails due to slaveInfo mismatch then agent no
state is recoverd making the agent register as a new agent.


Diffs (updated)
-

  src/tests/slave_recovery_tests.cpp f6eafcbe3d89c7a69c03db0fd7bc10ae73d06584 


Diff: https://reviews.apache.org/r/56895/diff/13/

Changes: https://reviews.apache.org/r/56895/diff/12-13/


Testing
---

make check done together with 60104 and 60103


Thanks,

Megha Sharma

Re: Review Request 59936: Added streaming function for ResourceProviderInfo.

[Benjamin Bannier]

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


Ship it!




Ship It!

- Benjamin Bannier


On June 30, 2017, 6:02 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59936/
> ---
> 
> (Updated June 30, 2017, 6:02 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added streaming function for ResourceProviderInfo.
> 
> 
> Diffs
> -
> 
>   include/mesos/type_utils.hpp c7872c0cd7f19d923e8be1f33a775d5eb17a0cf7 
>   include/mesos/v1/mesos.hpp c2e6ec5c4fa06d0179d6aaf3371ace4b6e6dbdaf 
>   src/common/type_utils.cpp 5f8e72b97d6766f9729079f6c6c013bc117cedb9 
>   src/v1/mesos.cpp 3b9f9b43e748159ae753880bbe4a5975814073ab 
> 
> 
> Diff: https://reviews.apache.org/r/59936/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 59937: Added agent `--resource_provider_config_dir` flag.

[Benjamin Bannier]

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


Ship it!




Ship It!

- Benjamin Bannier


On June 30, 2017, 6:10 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59937/
> ---
> 
> (Updated June 30, 2017, 6:10 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Jan Schlicht.
> 
> 
> Bugs: MESOS-7571
> https://issues.apache.org/jira/browse/MESOS-7571
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added agent `--resource_provider_config_dir` flag.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 0eb696a949003ff11831aed5e4f4ab384cf9992e 
>   src/slave/flags.hpp e75c1b4227b443aedf445921b3f2108d930c112c 
>   src/slave/flags.cpp c84aa6724170bba46bbe8410b71d42a1626e 
> 
> 
> Diff: https://reviews.apache.org/r/59937/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 59937: Added agent `--resource_provider_config_dir` flag.

[Jan Schlicht]

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


Ship it!




Ship It!

- Jan Schlicht


On June 30, 2017, 6:10 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59937/
> ---
> 
> (Updated June 30, 2017, 6:10 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Jan Schlicht.
> 
> 
> Bugs: MESOS-7571
> https://issues.apache.org/jira/browse/MESOS-7571
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added agent `--resource_provider_config_dir` flag.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 0eb696a949003ff11831aed5e4f4ab384cf9992e 
>   src/slave/flags.hpp e75c1b4227b443aedf445921b3f2108d930c112c 
>   src/slave/flags.cpp c84aa6724170bba46bbe8410b71d42a1626e 
> 
> 
> Diff: https://reviews.apache.org/r/59937/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Review Request 60566: Validated and upgraded resources on the V1 operator API path.

[Michael Park]

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

Review request for mesos and Benjamin Mahler.


Repository: mesos


Description
---

See summary.


Diffs
-

  src/master/http.cpp 4dd43fd7c3fb986f4eed78bce574b6d3af156b67 
  src/master/master.hpp 9dd6a530c373516dc81bfd51ee6e95f25588148f 
  src/tests/master_tests.cpp cfb799fd105e9880cd56415b2a84e604c8f62703 
  src/tests/persistent_volume_endpoints_tests.cpp 
518bdf858096ec9bcfa4f899ead5a6c3d103c521 
  src/tests/reservation_endpoints_tests.cpp 
f710a188a7875c1cb847e39276b4b65332703ca5 


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


Testing
---

Added new tests + `make check`


Thanks,

Michael Park

Re: Review Request 59936: Added streaming function for ResourceProviderInfo.

[Jan Schlicht]

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


Ship it!




Ship It!

- Jan Schlicht


On June 30, 2017, 6:02 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59936/
> ---
> 
> (Updated June 30, 2017, 6:02 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added streaming function for ResourceProviderInfo.
> 
> 
> Diffs
> -
> 
>   include/mesos/type_utils.hpp c7872c0cd7f19d923e8be1f33a775d5eb17a0cf7 
>   include/mesos/v1/mesos.hpp c2e6ec5c4fa06d0179d6aaf3371ace4b6e6dbdaf 
>   src/common/type_utils.cpp 5f8e72b97d6766f9729079f6c6c013bc117cedb9 
>   src/v1/mesos.cpp 3b9f9b43e748159ae753880bbe4a5975814073ab 
> 
> 
> Diff: https://reviews.apache.org/r/59936/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 59935: Introduced a streaming function for a vector.

[Jan Schlicht]

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


Ship it!




Ship It!

- Jan Schlicht


On June 30, 2017, 5:54 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59935/
> ---
> 
> (Updated June 30, 2017, 5:54 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduced a streaming function for a vector.
> 
> 
> Diffs
> -
> 
>   include/mesos/type_utils.hpp c7872c0cd7f19d923e8be1f33a775d5eb17a0cf7 
>   include/mesos/v1/mesos.hpp c2e6ec5c4fa06d0179d6aaf3371ace4b6e6dbdaf 
>   src/common/type_utils.cpp 5f8e72b97d6766f9729079f6c6c013bc117cedb9 
>   src/v1/mesos.cpp 3b9f9b43e748159ae753880bbe4a5975814073ab 
> 
> 
> Diff: https://reviews.apache.org/r/59935/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 60561: Replaced a few raw `for` loops with `foreach`.

[Benjamin Mahler]

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


Ship it!




Ship It!

- Benjamin Mahler


On June 30, 2017, 9 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60561/
> ---
> 
> (Updated June 30, 2017, 9 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 276b18e4d1bab7e9b28c1c93fe13c93a38abc5cd 
> 
> 
> Diff: https://reviews.apache.org/r/60561/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Review Request 60564: Performed validation/upgrade of `Resource` objects before authorization.

[Michael Park]

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

Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  src/master/master.cpp 276b18e4d1bab7e9b28c1c93fe13c93a38abc5cd 
  src/tests/master_tests.cpp cfb799fd105e9880cd56415b2a84e604c8f62703 


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


Testing
---

Added a new test + `make check`


Thanks,

Michael Park

Review Request 60563: Updated `validateAndUpgradeResources` to operate on `Operation`s.

[Michael Park]

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

Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  src/common/resources_utils.hpp 7128297c45fbf94af9384b678bf201f3d9c48b65 
  src/common/resources_utils.cpp 3a6a57817d4b0c4f4133b0a8d2b6f4d9ea31ea6b 


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


Testing
---


Thanks,

Michael Park

Review Request 60562: Updated `accept` to perform operation adjustment in one place.

[Michael Park]

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

Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
---

It used to be that the minor adjustments that were made to operations
were done in various places across `accept` and `_accept`.

The "executor-injection" for LAUNCH_GROUP was at the beginning of
`accept`, "allocation-info-injection" for MULTI_ROLE was after offer
validation, and "health-check-injection" for LAUNCH was in `_accept`.

The `Master::accept` function is now broken down into distinct
"metrics accounting", "offer validation", "operation-adjustments", and
"authorization" stages.


Diffs
-

  src/master/master.cpp 276b18e4d1bab7e9b28c1c93fe13c93a38abc5cd 


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


Testing
---

`make check`


Thanks,

Michael Park

Review Request 60561: Replaced a few raw `for` loops with `foreach`.

[Michael Park]

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

Review request for mesos and Benjamin Mahler.


Repository: mesos


Description
---

See summary.


Diffs
-

  src/master/master.cpp 276b18e4d1bab7e9b28c1c93fe13c93a38abc5cd 


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


Testing
---

`make check`


Thanks,

Michael Park

Re: Review Request 60558: Set container DNS with `--default_container_dns` in Docker executor.

[Qian Zhang]

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

(Updated June 30, 2017, 3:09 p.m.)


Review request for mesos, Avinash sridharan and Jie Yu.


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


Repository: mesos


Description
---

Set container DNS with `--default_container_dns` in Docker executor.


Diffs
-

  src/docker/docker.hpp 5593cb635e073334c6c2566be3d803cd7febb1c3 
  src/docker/docker.cpp 8ca0c68836ea5d1a1186e79942f6daccf6c6da46 
  src/docker/executor.cpp e03f24461ec7b19cacae79c020406f0a475f2d19 


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


Testing (updated)
---

sudo make check

1. Start Mesos master.
```
$ sudo ./bin/mesos-master.sh --work_dir=/opt/mesos
```

2. Start Mesos agent.
```
$ sudo ./bin/mesos-slave.sh --master=192.168.122.216:5050 
--containerizers=mesos,docker --image_providers=docker 
--image_provisioner_backend=aufs 
--isolation=filesystem/linux,docker/runtime,network/cni,cgroups/cpu,cgroups/mem 
--network_cni_config_dir=/opt/cni/net_configs 
--network_cni_plugins_dir=/opt/cni/plugins --work_dir=/opt/mesos 
--docker_store_dir=/opt/mesos/store/docker 
--executor_registration_timeout=60mins 
--default_container_dns=file:///home/stack/dns.json

$ cat /home/stack/dns.json
{
  "mesos": [
{
  "network": "net1",
  "dns": {
"nameservers": [ "8.8.8.8", "8.8.4.4" ]
  }
}
  ],
  "docker": [
{
  "network": "bridge",
  "dns": {
"nameservers": [ "8.8.8.8", "8.8.4.4" ],
"search": [ "xxx.com", "yyy.com" ],
"options": [ "timeout:3", "attempts:2" ]
  }
}
  ]
}
```

3. Launch a Docker container with `mesos-execute`.
```
$ sudo src/mesos-execute --master=192.168.122.216:5050 
--task=file:///home/stack/task-docker.json

$cat /home/stack/task-docker.json 
{
  "name": "test",
  "task_id": {"value" : "test"},
  "agent_id": {"value" : ""},
  "resources": [
{
  "name": "cpus",
  "type": "SCALAR",
  "scalar": {
"value": 0.1
  }
},
{
  "name": "mem",
  "type": "SCALAR",
  "scalar": {
"value": 32
  }
}
  ],
  "command": {
"shell": false
  },
  "container": {
"type": "DOCKER",
"docker": {
  "image": "nginx",
  "network": "BRIDGE"
}
  }
}
```

4. Check the DNS configuration of the Docker container.
```
$ docker ps 
CONTAINER IDIMAGE   COMMAND  CREATED
  STATUS  PORTS   NAMES
ca642bf31a9fnginx   "nginx -g 'daemon off"   About a minute 
ago   Up About a minute   80/tcp, 443/tcp 
mesos-1d48fc32-a138-4c31-a5a9-fd7d279231da

$ docker exec ca642bf31a9f cat /etc/resolv.conf 
search xxx.com yyy.com
nameserver 8.8.8.8
nameserver 8.8.4.4
options timeout:3 attempts:2
```


Thanks,

Qian Zhang

Review Request 60558: Set container DNS with `--default_container_dns` in Docker executor.

[Qian Zhang]

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

Review request for mesos, Avinash sridharan and Jie Yu.


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


Repository: mesos


Description
---

Set container DNS with `--default_container_dns` in Docker executor.


Diffs
-

  src/docker/docker.hpp 5593cb635e073334c6c2566be3d803cd7febb1c3 
  src/docker/docker.cpp 8ca0c68836ea5d1a1186e79942f6daccf6c6da46 
  src/docker/executor.cpp e03f24461ec7b19cacae79c020406f0a475f2d19 


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


Testing
---


Thanks,

Qian Zhang

Review Request 60557: Passed default container DNS info to Docker executor.

[Qian Zhang]

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

Review request for mesos, Avinash sridharan and Jie Yu.


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


Repository: mesos


Description
---

Passed default container DNS info to Docker executor.


Diffs
-

  src/docker/executor.hpp a4a8ec9905d3c6b3afcf0e2ba4e174c41fbcd75a 
  src/docker/executor.cpp e03f24461ec7b19cacae79c020406f0a475f2d19 
  src/slave/containerizer/docker.cpp 5cd3b6d95ff78fb114a06d49122b7161a6688646 


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


Testing
---


Thanks,

Qian Zhang