Re: Review Request 70802: Added `Metrics::updateQuota` for quota metrics.

[Benjamin Mahler]

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




src/master/allocator/mesos/metrics.hpp
Lines 51 (patched)


// TODO(mzhu): Remove these ...

We don't want to merely deprecate them (i.e. leave them present), we want 
to remove them right?



src/master/allocator/mesos/metrics.cpp
Lines 196-198 (patched)


Hm.. this doesn't seem to fit with the new approach, where we want to 
always track consumption, allocation, etc. even if quota is the default


- Benjamin Mahler


On June 7, 2019, 5:24 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70802/
> ---
> 
> (Updated June 7, 2019, 5:24 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9807
> https://issues.apache.org/jira/browse/MESOS-9807
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This intends to replace the existing ``Metrics::setQuota` and
> `Metrics::remove` calls.
> 
> Currently, it only tracks guarantees. Need to add limits metrics.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/metrics.hpp 
> a26278ab95aae1fc99ab0afbcfb6dd40651f3a33 
>   src/master/allocator/mesos/metrics.cpp 
> 5533eb9910daf90484c255b75b1bcea54b23ba9f 
> 
> 
> Diff: https://reviews.apache.org/r/70802/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 70801: Refactored allocator with the new quota wrapper struct.

[Benjamin Mahler]

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


Ship it!





src/master/allocator/mesos/hierarchical.cpp
Line 1413 (original), 1413 (patched)


We should probably think through the setting semantics when we do the 
wiring, because this seems to suggest that something is stopping the caller 
from setting to the default? (otherwise the remove / set semantics here won't 
quite hold)



src/master/allocator/mesos/hierarchical.cpp
Line 1450 (original), 1441 (patched)


Ditto here, seems to imply that something is stopping the caller from 
setting to default, perhaps a note about this (i.e. it's impossible to set to 
default using the old API).



src/master/constants.hpp
Lines 176 (patched)


How about: "quota: no guarantees ..."



src/master/allocator/mesos/hierarchical.cpp
Line 2571 (original), 2551 (patched)


Is this done frequently? If so, consider avoiding the double lookup


- Benjamin Mahler


On June 11, 2019, 8:15 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70801/
> ---
> 
> (Updated June 11, 2019, 8:15 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9807
> https://issues.apache.org/jira/browse/MESOS-9807
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch also introduces a constant `DEFAULT_QUOTA`.
> By default, a role has no guarantees and no limits.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 1a3329d0e50ad2adb6aeadcd5d315eb572a85704 
>   src/master/allocator/mesos/hierarchical.cpp 
> ad0bcca90dcce4c9e8384293181c4f4af3d274aa 
>   src/master/constants.hpp 8f729d18ec3678d97efbb74a8e816348c6acbeb4 
> 
> 
> Diff: https://reviews.apache.org/r/70801/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 70836: Added test for DrainInfo in agent API outputs.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [70822, 70834, 70835, 70836]

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

- Mesos Reviewbot


On June 11, 2019, 8:52 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70836/
> ---
> 
> (Updated June 11, 2019, 8:52 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benno Evers, Benjamin Mahler, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-9818
> https://issues.apache.org/jira/browse/MESOS-9818
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test for DrainInfo in agent API outputs.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp c2035976713abb31b3646c0d23771fa40df93271 
> 
> 
> Diff: https://reviews.apache.org/r/70836/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> `bin/mesos-tests.sh --gtest_filter="*DrainInfoInAPIOutputs*" 
> --gtest_repeat=-1 --gtest_break_on_failure`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 70804: Used the new quota struct for the allocator recover call.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [70799, 70800, 70801, 70802, 70803, 70804]

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

- Mesos Reviewbot


On June 11, 2019, 8:16 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70804/
> ---
> 
> (Updated June 11, 2019, 8:16 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9807
> https://issues.apache.org/jira/browse/MESOS-9807
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used the new quota struct for the allocator recover call.
> 
> 
> Diffs
> -
> 
>   include/mesos/allocator/allocator.hpp 
> eafcdfdab7f0fc9570e65db15274b52495aa6de3 
>   src/master/allocator/mesos/allocator.hpp 
> 2d83f382eed9d9dca218adf84bc03883f7486349 
>   src/master/allocator/mesos/hierarchical.hpp 
> 1a3329d0e50ad2adb6aeadcd5d315eb572a85704 
>   src/master/allocator/mesos/hierarchical.cpp 
> ad0bcca90dcce4c9e8384293181c4f4af3d274aa 
>   src/master/master.cpp 2b4e51d2702328268c5cb815c10f6f2b1aa9f5d2 
>   src/tests/allocator.hpp 0718bef4de39a82dbcadebb9967d2ff1cceb1cae 
> 
> 
> Diff: https://reviews.apache.org/r/70804/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 70827: Improved container-specific cgroups test by checking `cpu.shares`.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On June 10, 2019, 7:48 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70827/
> ---
> 
> (Updated June 10, 2019, 7:48 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gilbert Song, and James Peach.
> 
> 
> Bugs: MESOS-9769
> https://issues.apache.org/jira/browse/MESOS-9769
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is to ensure the symbolic links (see below as an example) we
> create for the container exist.
>   ln -s /sys/fs/cgroup/cpu,cpuacct /sys/fs/cgroup/cpu
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/cgroups_isolator_tests.cpp 
> 957f72d78f9ab0bf2775687915099c0109dac6e1 
> 
> 
> Diff: https://reviews.apache.org/r/70827/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> The tests updated in this patch would fail without the previous patch.
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 70826: Supported file operations for command tasks.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On June 10, 2019, 7:46 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70826/
> ---
> 
> (Updated June 10, 2019, 7:46 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gilbert Song, and James Peach.
> 
> 
> Bugs: MESOS-9769
> https://issues.apache.org/jira/browse/MESOS-9769
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported file operations for command tasks.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp fa4bcaad9ac36bf380484dadb14d0b0a86a30aae 
> 
> 
> Diff: https://reviews.apache.org/r/70826/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Review Request 70836: Added test for DrainInfo in agent API outputs.

[Greg Mann]

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

Review request for mesos, Benjamin Bannier, Benno Evers, Benjamin Mahler, and 
Joseph Wu.


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


Repository: mesos


Description
---

Added test for DrainInfo in agent API outputs.


Diffs
-

  src/tests/slave_tests.cpp c2035976713abb31b3646c0d23771fa40df93271 


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


Testing
---

`make check`
`bin/mesos-tests.sh --gtest_filter="*DrainInfoInAPIOutputs*" --gtest_repeat=-1 
--gtest_break_on_failure`


Thanks,

Greg Mann

Review Request 70835: Added the DrainInfo to agent API outputs.

[Greg Mann]

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

Review request for mesos, Benjamin Bannier, Benno Evers, Benjamin Mahler, and 
Joseph Wu.


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


Repository: mesos


Description
---

Added the DrainInfo to agent API outputs.


Diffs
-

  include/mesos/agent/agent.proto 83eb7bbe071fe4f118c38252bd1759ac90fc9c33 
  include/mesos/v1/agent/agent.proto f6574cbb899f3ffca9f5f07f1a235c01cf97912e 
  src/slave/http.cpp 69e6d74e8b113cc6c937f47df8984ff9a63e5bb4 


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


Testing
---

Testing details at the end of this chain.


Thanks,

Greg Mann

Review Request 70834: Added minimal agent handler for 'DrainSlaveMessage'.

[Greg Mann]

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

Review request for mesos, Benjamin Bannier, Benno Evers, Benjamin Mahler, and 
Joseph Wu.


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


Repository: mesos


Description
---

This patch adds a minimal handler to the agent for the
`DrainSlaveMessage`. This handler will later be extended
to implement the full functionality.


Diffs
-

  src/slave/paths.hpp ad768267e6a3c105eab4b36b05351d175d720eaf 
  src/slave/paths.cpp 1163c88eefd2a9e99b56817a3f515a23ee6e3903 
  src/slave/slave.hpp 6954f53ff1531b9fcb688ef76acddf6a3d849a41 
  src/slave/slave.cpp 30039b0857a4d85b4b96fa95d7f8724d57cdec6e 
  src/slave/state.proto 2bdf691026f1ee9f341411b4b2942368212ccfd3 


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


Testing
---

Testing details at the end of this chain.


Thanks,

Greg Mann

Re: Review Request 70804: Used the new quota struct for the allocator recover call.

[Meng Zhu]

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

(Updated June 11, 2019, 1:16 p.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Used the new quota struct for the allocator recover call.


Diffs (updated)
-

  include/mesos/allocator/allocator.hpp 
eafcdfdab7f0fc9570e65db15274b52495aa6de3 
  src/master/allocator/mesos/allocator.hpp 
2d83f382eed9d9dca218adf84bc03883f7486349 
  src/master/allocator/mesos/hierarchical.hpp 
1a3329d0e50ad2adb6aeadcd5d315eb572a85704 
  src/master/allocator/mesos/hierarchical.cpp 
ad0bcca90dcce4c9e8384293181c4f4af3d274aa 
  src/master/master.cpp 2b4e51d2702328268c5cb815c10f6f2b1aa9f5d2 
  src/tests/allocator.hpp 0718bef4de39a82dbcadebb9967d2ff1cceb1cae 


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

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


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 70801: Refactored allocator with the new quota wrapper struct.

[Meng Zhu]

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

(Updated June 11, 2019, 1:15 p.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Rebased.


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


Repository: mesos


Description
---

This patch also introduces a constant `DEFAULT_QUOTA`.
By default, a role has no guarantees and no limits.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.hpp 
1a3329d0e50ad2adb6aeadcd5d315eb572a85704 
  src/master/allocator/mesos/hierarchical.cpp 
ad0bcca90dcce4c9e8384293181c4f4af3d274aa 
  src/master/constants.hpp 8f729d18ec3678d97efbb74a8e816348c6acbeb4 


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

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


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 70800: Refactored quota overcommit check.

[Benjamin Mahler]

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


Fix it, then Ship it!





src/master/quota_handler.cpp
Lines 27 (patched)


no newline here but newline between authorizer and quota includes?



src/master/quota_handler.cpp
Lines 76-78 (original), 77-79 (patched)


Shall we clarify here that it's guarantees as well?



src/master/quota_handler.cpp
Lines 120-121 (original), 124-126 (patched)


```
// Check whether the tree satisfies:
//
//   parent guarantees >= sum(children guarantees)
//
// TODO(mzhu): Add limit check.
```



src/master/quota_handler.cpp
Lines 145-147 (patched)


This TODO seems to belong somewhere else (I assume this is talking about a 
caller? or is this talking about the checks within the quota tree code?)



src/master/quota_handler.cpp
Line 173 (original), 182 (patched)


space after guarantees

also, not yours but one is using parens and the other is not?


- Benjamin Mahler


On June 7, 2019, 5:22 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70800/
> ---
> 
> (Updated June 7, 2019, 5:22 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9807
> https://issues.apache.org/jira/browse/MESOS-9807
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This refactor makes the `QuotaTree` to use the new
> quota wrapper struct.
> 
> Also refactor the check to reflect that it is currently
> only checking guarantees.
> 
> 
> Diffs
> -
> 
>   src/master/quota_handler.cpp 21176dd8a76fda621f0f98878efa67ecf862368c 
> 
> 
> Diff: https://reviews.apache.org/r/70800/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 70799: Added a wrapper struct for quota guarantees and limits.

[Benjamin Mahler]

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


Ship it!




- Benjamin Mahler


On June 7, 2019, 5:22 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70799/
> ---
> 
> (Updated June 7, 2019, 5:22 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9807
> https://issues.apache.org/jira/browse/MESOS-9807
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This struct is temporarily named to `Quota2` to differentiate
> with the existing `Quota` struct. It will replace all `Quota`
> and rename to `Quota`.
> 
> 
> Diffs
> -
> 
>   include/mesos/quota/quota.hpp b028e1286920d43fddfc2234b03290c98a51b5b7 
>   src/master/quota.cpp 001c240772e72975468c930d30a90aaa62cdce53 
> 
> 
> Diff: https://reviews.apache.org/r/70799/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 70822: Added common protobufs for agent draining.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [70822]

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

- Mesos Reviewbot


On June 11, 2019, 5:43 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70822/
> ---
> 
> (Updated June 11, 2019, 5:43 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benno Evers, Benjamin Mahler, 
> Joseph Wu, and Vinod Kone.
> 
> 
> Bugs: MESOS-9753
> https://issues.apache.org/jira/browse/MESOS-9753
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch makes protobuf message updates which will be used
> by both the master and the agent to facilitate automatic
> draining of agents.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 2b4f350815935220c2d2b0dd0e52346bc74c91d9 
>   include/mesos/type_utils.hpp 57b1893160dbe874aa9fec00a3d1b640b9c54906 
>   include/mesos/v1/mesos.proto bafc27499f810791700c4a30dcb1da33b6f31d2e 
>   src/internal/devolve.hpp fefe86e450fa5083b9ff50e92f4594ffb30a54c8 
>   src/internal/devolve.cpp 1d300b49d5cc3de4b8ed409902eb881c7afc07ea 
>   src/internal/evolve.hpp 1044d9df75b6fc1f60d3704be9cb5751e6d4321d 
>   src/internal/evolve.cpp 19c155967bf090fb2ec39211805ff1385787ab59 
>   src/messages/messages.proto e30ad34cc9212b05f85ba5e1d4fcfc9e49ae92c0 
> 
> 
> Diff: https://reviews.apache.org/r/70822/diff/3/
> 
> 
> Testing
> ---
> 
> `make`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 70755: Added tests for the V0 UPDATE_FRAMEWORK scheduler call.

[Benjamin Mahler]

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


Fix it, then Ship it!





src/tests/master/update_framework_tests.cpp
Lines 627 (patched)


.WillOnce on the next line



src/tests/master/update_framework_tests.cpp
Lines 654-655 (patched)


No need to create and pass in the agent flags? They're not being used below?



src/tests/master/update_framework_tests.cpp
Lines 664-666 (patched)


Same race as the v1 tests, this needs to be before StartSlave, otherwise 
the agent might show up in the initial SUBSCRIBE state



src/tests/master/update_framework_tests.cpp
Lines 690-691 (patched)


V0 scheduler event? I guess you mean the master acknowledging the update? 
We may never add that, so probably we should just remove the TODO



src/tests/master/update_framework_tests.cpp
Lines 696 (patched)


This comment seems a little confusing, it made me think that we're about to 
test that the ID set gets rejected, but instead it's just adding it for 
comparision purposes.



src/tests/master/update_framework_tests.cpp
Lines 714 (patched)


newline

and maybe a comment that we sanity check that it's different from the 
default? seems a bit odd to check here as opposed to earlier when calling 
changeAllMutableFields?



src/tests/master/update_framework_tests.cpp
Lines 749-750 (patched)


Ditto here.



src/tests/master/update_framework_tests.cpp
Lines 780 (patched)


do you need to resume here for the test to pass?



src/tests/master/update_framework_tests.cpp
Lines 795 (patched)


does s/1ul/1u/ not work? I think we do this in other tests



src/tests/master/update_framework_tests.cpp
Lines 809-810 (patched)


Ditto here.



src/tests/master/update_framework_tests.cpp
Lines 813-818 (patched)


```
  MockScheduler sched;
  
  // Expect an offer exactly once (after subscribing).
  Future> offers;
```



src/tests/master/update_framework_tests.cpp
Lines 824 (patched)


newline



src/tests/master/update_framework_tests.cpp
Lines 825 (patched)


Ditto here.



src/tests/master/update_framework_tests.cpp
Lines 836-837 (patched)


// TODO(asekretenko): Add a more in-depth check that
  // the allocator does what it should.


- Benjamin Mahler


On June 3, 2019, 8:16 p.m., Andrei Sekretenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70755/
> ---
> 
> (Updated June 3, 2019, 8:16 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Meng Zhu.
> 
> 
> Bugs: MESOS-9793
> https://issues.apache.org/jira/browse/MESOS-9793
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added tests for the V0 UPDATE_FRAMEWORK scheduler call.
> 
> 
> Diffs
> -
> 
>   src/tests/master/update_framework_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/70755/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>

Re: Review Request 70754: Moved UPDATE_FRAMEWORK test helpers to appropriate namespaces.

[Benjamin Mahler]

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


Ship it!




Looks good, but needs a rebase (see comment on previous review)

- Benjamin Mahler


On May 29, 2019, 6:22 p.m., Andrei Sekretenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70754/
> ---
> 
> (Updated May 29, 2019, 6:22 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Meng Zhu.
> 
> 
> Bugs: MESOS-9793
> https://issues.apache.org/jira/browse/MESOS-9793
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Moved UPDATE_FRAMEWORK test helpers to appropriate namespaces.
> 
> 
> Diffs
> -
> 
>   src/tests/master/update_framework_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/70754/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>

Re: Review Request 70753: Templatized UPDATE_FRAMEWORK test helpers over type of FrameworkInfo.

[Benjamin Mahler]

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


Fix it, then Ship it!




Can you rebase this patch? I can't apply it to make the edits, even with a 3 
way merge.


src/tests/master/update_framework_tests.cpp
Lines 104 (patched)


The next patch doesn't move it out of v1 and removes the TODO? Why is it 
here?



src/tests/master/update_framework_tests.cpp
Lines 146 (patched)


Probably we should stick to the existing naming convention of 
`TFrameworkInfo` , e.g.

https://github.com/apache/mesos/blob/1.8.0/src/tests/mesos.hpp#L508



src/tests/master/update_framework_tests.cpp
Line 170 (original), 174 (patched)


newline



src/tests/master/update_framework_tests.cpp
Lines 179 (patched)


ditto here


- Benjamin Mahler


On May 29, 2019, 6:30 p.m., Andrei Sekretenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70753/
> ---
> 
> (Updated May 29, 2019, 6:30 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Meng Zhu.
> 
> 
> Bugs: MESOS-9793
> https://issues.apache.org/jira/browse/MESOS-9793
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch makes possible to use the same helper functions in tests of
> both V1 and V0 UPDATE_FRAMEWORK calls by templatizing them over type of
> FrameworkInfo.
> 
> 
> Diffs
> -
> 
>   src/tests/master/update_framework_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/70753/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>

Re: Review Request 70752: Supported updating framework in MesosSchedulerDriver.

[Benjamin Mahler]

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


Fix it, then Ship it!




Looks good, the main observation I have is that the current approach will 
unnecessarily send the UPDATE_FRAMEWORK message in the case that there is not a 
SUBSCRIBE in progress. In that case, the next SUBSCRIBE would pick up the 
updated FrameworkInfo anyway. It seems pretty easy to resolve that, but for now 
I've left it as is (with the extra UPDATE_FRAMEWORK call), since that seems 
like a significant adjustment for me to make prior to committing.


include/mesos/scheduler.hpp
Lines 327-330 (patched)


Hm.. perhaps just say:

```
// NOTE: If the supplied info is invalid or fails authorization,
// the `error()` callback will be invoked asynchronously (after
// the master replies with a FrameworkErrorMessage).
```



src/sched/sched.cpp
Lines 1603 (patched)


camelCase in mesos code



src/sched/sched.cpp
Lines 1605 (patched)


assignment syntax?

```
*framework.mutable_id() = std::move(framework_id);
```



src/sched/sched.cpp
Lines 1610-1611 (patched)


How about:

"Postponing UPDATE_FRAMEWORK call: not registered with master"



src/sched/sched.cpp
Lines 1666-1667 (patched)


let's use the assignment syntax:

```
*call.mutable_update_framework()->mutable_framework_info() =
  framework;
```



src/sched/sched.cpp
Lines 1689 (patched)


camelCase up in mesos code (whereas libprocess and stout tries to use 
snake_case but is inconsistent about it =/)



src/sched/sched.cpp
Lines 2336-2337 (patched)


We don't use periods in the log messages, so:

```
  LOG(ERROR) << "MesosSchedulerDriver::updateFramework should not be 
called"
 << " with 'FrameworkInfo.id' set, aborting driver";
```



src/sched/sched.cpp
Lines 2344 (patched)


assignment syntax and std::move?



include/mesos/scheduler.hpp
Lines 324 (patched)


"will be"



src/sched/sched.cpp
Lines 1607-1613 (patched)


Hm.. this approach will send an UPDATE_FRAMEWORK unnecessarily when no 
subscribe is in progress. Perhaps the following comment:

```
// If not `connected`, we defer the sending of the UPDATE_FRAMEWORK
// message. Note that this approach may unnecessarily send an
// UPDATE_FRAMEWORK message in the case that a (re)registration is
// not currently in progress (because the next (re)registration
// will have already used the updated `framework`).
//
// TODO(asekretenko): Consider tracking additional state to avoid
// sending an unnecessary UPDATE_FRAMEWORK message.
```

However, we can probably easily fix this by setting 
send_update_framework_on_connect to false when sending a SUBSCRIBE call?



src/sched/sched.cpp
Lines 1610-1611 (patched)


Ditto here about VLOG(1)



src/sched/sched.cpp
Lines 1670 (patched)


VLOG(1) 

This is an embedded library so the approach we took was to use VLOG(1) for 
what might normally be INFO messages. Over time, some INFO level logging has 
been added, so it's becoming less clear, but let's stick with VLOG(1) here for 
now



src/sched/sched.cpp
Lines 2352 (patched)


Let's keep the whitespace consistent across these functions, so newline here


- Benjamin Mahler


On June 7, 2019, 4:26 p.m., Andrei Sekretenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70752/
> ---
> 
> (Updated June 7, 2019, 4:26 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Meng Zhu.
> 
> 
> Bugs: MESOS-9793
> https://issues.apache.org/jira/browse/MESOS-9793
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a method to the MesosSchedulerDriver which updates the
> FrameworkInfo (by making an UPDATE_FRAMEWORK call to the master and also
> updating the FrameworkInfo stored in the driver for the purposes of
> (re-)subscribing).
> 
> 
> Diffs
> -
> 
>   

Re: Review Request 70822: Added common protobufs for agent draining.

[Greg Mann]

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

(Updated June 11, 2019, 5:43 p.m.)


Review request for mesos, Benjamin Bannier, Benno Evers, Benjamin Mahler, 
Joseph Wu, and Vinod Kone.


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


Repository: mesos


Description
---

This patch makes protobuf message updates which will be used
by both the master and the agent to facilitate automatic
draining of agents.


Diffs (updated)
-

  include/mesos/mesos.proto 2b4f350815935220c2d2b0dd0e52346bc74c91d9 
  include/mesos/type_utils.hpp 57b1893160dbe874aa9fec00a3d1b640b9c54906 
  include/mesos/v1/mesos.proto bafc27499f810791700c4a30dcb1da33b6f31d2e 
  src/internal/devolve.hpp fefe86e450fa5083b9ff50e92f4594ffb30a54c8 
  src/internal/devolve.cpp 1d300b49d5cc3de4b8ed409902eb881c7afc07ea 
  src/internal/evolve.hpp 1044d9df75b6fc1f60d3704be9cb5751e6d4321d 
  src/internal/evolve.cpp 19c155967bf090fb2ec39211805ff1385787ab59 
  src/messages/messages.proto e30ad34cc9212b05f85ba5e1d4fcfc9e49ae92c0 


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

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


Testing
---

`make`


Thanks,

Greg Mann

Re: Review Request 70820: Updated `filesystem/linux` isolator for configurable IPC support.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [70773, 70774, 70775, 70798, 70820]

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

- Mesos Reviewbot


On June 10, 2019, 1:20 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70820/
> ---
> 
> (Updated June 10, 2019, 1:20 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Gilbert Song.
> 
> 
> Bugs: MESOS-9826
> https://issues.apache.org/jira/browse/MESOS-9826
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If `namespaces/ipc` isolator is not enabled, for backward
> compatibility /dev/shm will still be handled in `filesystem/linux`
> isolator as before. Otherwise, both /dev/shm and IPC namespace
> will be handled by `namespaces/ipc` isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
> 3cfb6e97a565420c8be2a0e31b481b39cd09d9da 
> 
> 
> Diff: https://reviews.apache.org/r/70820/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 70824: Fixed test `QuotaRoleAllocateNonQuotaResource`.

[Benjamin Mahler]

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


Fix it, then Ship it!




Thanks for the quick fix!


src/tests/hierarchical_allocator_tests.cpp
Lines 4095-4096 (patched)


Non quota seems a little hard to understand, are these the resources that 
don't have guarantees and therefore there's no headroom requirements to check?



src/tests/hierarchical_allocator_tests.cpp
Lines 4108 (patched)


`*allocations` ?


- Benjamin Mahler


On June 11, 2019, 12:53 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70824/
> ---
> 
> (Updated June 11, 2019, 12:53 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9835
> https://issues.apache.org/jira/browse/MESOS-9835
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The test was failing because:
> 
> After `agent3` is added, it misses a settle call where the allocation
> of `agent3` is racy.
> 
> In addition, after MESOS-8456, the allocator now offers non-quota
> resources on an agent (even that means "chopping") on top of a role's
> satisfied guarantees instead of skipping the agent all-together.
> 
> This patch fixes the test by expecting the right amount of resources
> to be allocated from `agent3`.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> bfea34e17151327e8c9590ecb8da74968d086ca0 
> 
> 
> Diff: https://reviews.apache.org/r/70824/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 70811: Fixed compilation error on Mac OS.

[Benjamin Bannier]

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


Ship it!




Ship It!

- Benjamin Bannier


On June 7, 2019, 7:21 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70811/
> ---
> 
> (Updated June 7, 2019, 7:21 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Gilbert Song, James Peach, and 
> Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds missing switch case to fix compilation error introduced
> in `bc5a57122635`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.cpp 
> a69a68823842171bb15ddf34a504e5ce4af232b0 
> 
> 
> Diff: https://reviews.apache.org/r/70811/diff/2/
> 
> 
> Testing
> ---
> 
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 70798: Improved `namespaces/ipc` isolator for configurable IPC support.

[Qian Zhang]

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

(Updated June 11, 2019, 10:33 p.m.)


Review request for mesos, Andrei Budnik and Gilbert Song.


Changes
---

Added comments in the code.


Summary (updated)
-

Improved `namespaces/ipc` isolator for configurable IPC support.


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


Repository: mesos


Description (updated)
---

Improved `namespaces/ipc` isolator for configurable IPC support.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/namespaces/ipc.hpp 
32c888309ca536d944e4d73641aed214805ccce2 
  src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp 
6c8e8eed69ed355094f7d9b54bde25a8a9bc63f7 
  src/slave/containerizer/mesos/paths.hpp 
a5e0920bd4d753f6ba6f3b092c92cb405e9edf35 
  src/slave/containerizer/mesos/paths.cpp 
4281abc522c942c87fcfd811af26f95cbd6f734f 
  src/tests/containerizer/isolator_tests.cpp 
9c14f3acbc19631b2f5cac4dc7cd9caba8527712 


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

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


Testing
---


Thanks,

Qian Zhang

Re: Review Request 70829: Added logrotate option to force Jenkins into removing older artefacts.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [70829]

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

- Mesos Reviewbot


On June 11, 2019, 5:12 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70829/
> ---
> 
> (Updated June 11, 2019, 5:12 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   support/jenkins/Jenkinsfile-packaging-centos 
> de688f45f6530c48f13930cbd78f772873a1bc84 
> 
> 
> Diff: https://reviews.apache.org/r/70829/diff/1/
> 
> 
> Testing
> ---
> 
> Untested but based on documentation and hope: 
> https://support.cloudbees.com/hc/en-us/articles/115000237071-How-do-I-set-discard-old-builds-for-a-Multi-Branch-Pipeline-Job-
> 
> This should remove old logfiles and artefacts after 30 days.
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 70811: Fixed compilation error on Mac OS.

[Till Toenshoff via Review Board]

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


Ship it!




Ship It!

- Till Toenshoff


On June 7, 2019, 5:21 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70811/
> ---
> 
> (Updated June 7, 2019, 5:21 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Gilbert Song, James Peach, and 
> Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds missing switch case to fix compilation error introduced
> in `bc5a57122635`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.cpp 
> a69a68823842171bb15ddf34a504e5ce4af232b0 
> 
> 
> Diff: https://reviews.apache.org/r/70811/diff/2/
> 
> 
> Testing
> ---
> 
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Review Request 70831: Fixed synchronization in MasterTest.UpdateSlaveMessageWithPendingOffers.

[Benjamin Bannier]

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

Review request for mesos, Chun-Hung Hsiao and Jan Schlicht.


Repository: mesos


Description
---

In order for a resource provider to have been assigned and stored an
assigned resource provider ID it needs to receive and process the
corresponding resource provider `SUBSCRIBED` event.

This patch fixes `MasterTest.UpdateSlaveMessageWithPendingOffers` so we
await the correct event. The test was previously awaiting an
`UpdateSlaveMessage` which is only tangentially related, but does not
guarantee correct event ordering.


Diffs
-

  src/tests/master_tests.cpp 097f1b77a59e29c6690210773d1556ebf2bb701e 


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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 70728: Backed `MockResourceProvider` by a process.

[Benjamin Bannier]

> On June 6, 2019, 12:53 a.m., Chun-Hung Hsiao wrote:
> > src/tests/master_tests.cpp
> > Line 9017 (original), 9017 (patched)
> > 
> >
> > Not yours, but it seems to me that the dequeueing of the 
> > `UpdateSlaveMessage` in the master does not causally happen before the 
> > `subscribedDefault` call (which sets up the resource provider ID).

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


- Benjamin


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


On May 27, 2019, 6:32 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70728/
> ---
> 
> (Updated May 27, 2019, 6:32 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We were passing callbacks into `MockResourceProvider` to the HTTP
> driver. Since the lifecycle of the callbacks and the mock provider were
> decoupled and these callbacks were binding the mock provider instance
> the code was not safe as written as the driver could invoke the callback
> after the provider had been destructed.
> 
> This patch makes sure that the callbacks are defered to a process. We
> also dispatch a number of other functions which are strongly coupled to
> the lifecycle of the provider. We still do not hide the provider away
> completelt so the provider can be mocked in tests.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp 2220cecc22778a86f0c29317adf495927e1a900d 
>   src/tests/master_slave_reconciliation_tests.cpp 
> 7b6ac50adacc8416b91c0dde55ff7ba46a20515c 
>   src/tests/master_tests.cpp 097f1b77a59e29c6690210773d1556ebf2bb701e 
>   src/tests/mesos.hpp d0c82fa4c1800ed2a1825d563a9462ecd41b77fd 
>   src/tests/operation_reconciliation_tests.cpp 
> eae318da2273faae904f0155e49bb23cdb24f007 
>   src/tests/resource_provider_manager_tests.cpp 
> 7d48f18e89f046df6c92e52edeef592acfb13b10 
>   src/tests/slave_tests.cpp c2035976713abb31b3646c0d23771fa40df93271 
> 
> 
> Diff: https://reviews.apache.org/r/70728/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 70811: Fixed compilation error on Mac OS.

[Till Toenshoff via Review Board]

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




src/slave/containerizer/mesos/launch.cpp
Line 508 (original), 509 (patched)


I am 100% with BenB here - silently ignoring seems a ticket for hard to 
explain / debug problems. Let's please return an `Error` suggesting that this 
setting was not supported on macOS instead.


- Till Toenshoff


On June 7, 2019, 5:21 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70811/
> ---
> 
> (Updated June 7, 2019, 5:21 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Gilbert Song, James Peach, and 
> Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds missing switch case to fix compilation error introduced
> in `bc5a57122635`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.cpp 
> a69a68823842171bb15ddf34a504e5ce4af232b0 
> 
> 
> Diff: https://reviews.apache.org/r/70811/diff/1/
> 
> 
> Testing
> ---
> 
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 70811: Fixed compilation error on Mac OS.

[Benjamin Bannier]

> On June 7, 2019, 8:55 p.m., Benjamin Bannier wrote:
> > src/slave/containerizer/mesos/launch.cpp
> > Line 508 (original), 509 (patched)
> > 
> >
> > For non-`__linux-_` cases this silently drops any configuration. Why 
> > wouldn't we return an error in that case instead?
> 
> Andrei Budnik wrote:
> This cpp-module has inconsistencies with handling of Linux/non-Linux 
> cases. For example, `prepareMounts` and `maskPath` are simply no-ops on 
> non-Linux platforms.
> This patch fixes compilation error. Fixing the whole inconsistency issue 
> in a single place is not a goal here.

Since we are already not consistent, let's return a failure here then instead.


- Benjamin


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


On June 7, 2019, 7:21 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70811/
> ---
> 
> (Updated June 7, 2019, 7:21 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Gilbert Song, James Peach, and 
> Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds missing switch case to fix compilation error introduced
> in `bc5a57122635`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.cpp 
> a69a68823842171bb15ddf34a504e5ce4af232b0 
> 
> 
> Diff: https://reviews.apache.org/r/70811/diff/1/
> 
> 
> Testing
> ---
> 
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 70829: Added logrotate option to force Jenkins into removing older artefacts.

[Benjamin Bannier]

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


Ship it!




Ship It!

- Benjamin Bannier


On June 11, 2019, 1:42 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70829/
> ---
> 
> (Updated June 11, 2019, 1:42 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   support/jenkins/Jenkinsfile-packaging-centos 
> de688f45f6530c48f13930cbd78f772873a1bc84 
> 
> 
> Diff: https://reviews.apache.org/r/70829/diff/1/
> 
> 
> Testing
> ---
> 
> Untested but based on documentation and hope: 
> https://support.cloudbees.com/hc/en-us/articles/115000237071-How-do-I-set-discard-old-builds-for-a-Multi-Branch-Pipeline-Job-
> 
> This should remove old logfiles and artefacts after 30 days.
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Review Request 70829: Added logrotate option to force Jenkins into removing older artefacts.

[Till Toenshoff via Review Board]

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

Review request for mesos, Benjamin Bannier and Vinod Kone.


Repository: mesos


Description
---

See summary.


Diffs
-

  support/jenkins/Jenkinsfile-packaging-centos 
de688f45f6530c48f13930cbd78f772873a1bc84 


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


Testing
---

Untested but based on documentation and hope: 
https://support.cloudbees.com/hc/en-us/articles/115000237071-How-do-I-set-discard-old-builds-for-a-Multi-Branch-Pipeline-Job-

This should remove old logfiles and artefacts after 30 days.


Thanks,

Till Toenshoff

Re: Review Request 70811: Fixed compilation error on Mac OS.

[Andrei Budnik]

> On Июнь 7, 2019, 6:55 п.п., Benjamin Bannier wrote:
> > src/slave/containerizer/mesos/launch.cpp
> > Line 508 (original), 509 (patched)
> > 
> >
> > For non-`__linux-_` cases this silently drops any configuration. Why 
> > wouldn't we return an error in that case instead?

This cpp-module has inconsistencies with handling of Linux/non-Linux cases. For 
example, `prepareMounts` and `maskPath` are simply no-ops on non-Linux 
platforms.
This patch fixes compilation error. Fixing the whole inconsistency issue in a 
single place is not a goal here.


- Andrei


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


On Июнь 7, 2019, 5:21 п.п., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70811/
> ---
> 
> (Updated Июнь 7, 2019, 5:21 п.п.)
> 
> 
> Review request for mesos, Benjamin Bannier, Gilbert Song, James Peach, and 
> Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds missing switch case to fix compilation error introduced
> in `bc5a57122635`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.cpp 
> a69a68823842171bb15ddf34a504e5ce4af232b0 
> 
> 
> Diff: https://reviews.apache.org/r/70811/diff/1/
> 
> 
> Testing
> ---
> 
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 70827: Improved container-specific cgroups test by checking `cpu.shares`.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [70826, 70827]

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

- Mesos Reviewbot


On June 11, 2019, 2:48 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70827/
> ---
> 
> (Updated June 11, 2019, 2:48 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gilbert Song, and James Peach.
> 
> 
> Bugs: MESOS-9769
> https://issues.apache.org/jira/browse/MESOS-9769
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is to ensure the symbolic links (see below as an example) we
> create for the container exist.
>   ln -s /sys/fs/cgroup/cpu,cpuacct /sys/fs/cgroup/cpu
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/cgroups_isolator_tests.cpp 
> 957f72d78f9ab0bf2775687915099c0109dac6e1 
> 
> 
> Diff: https://reviews.apache.org/r/70827/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> The tests updated in this patch would fail without the previous patch.
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>