Re: Review Request 71024: Supported insecure registry when provisioning docker images.

[Jie Yu]

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



Can you add some unit test for this? For instance, toggle the new agent flag, 
test with some docker registry (ideally, an unsecure registry, but I think it's 
ok to test with a secure one too just to excersize the code path).


src/slave/flags.hpp
Lines 61 (patched)


I would rather make this a boolean


- Jie Yu


On July 9, 2019, 3:24 a.m., fei long wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71024/
> ---
> 
> (Updated July 9, 2019, 3:24 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6183
> https://issues.apache.org/jira/browse/MESOS-6183
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported insecure registry when provisioning docker images.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
> d013c9d71c39c09e600f181aba31b8037aa9226a 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 35b6afbb6b22575b90963927352443a8ddaf9885 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> 286ee5f00933b4c86a96dc4e10e42f9e7eac0ce2 
>   src/slave/flags.hpp a10bf698dc447b1411c06083c2ba7585d7a78389 
>   src/slave/flags.cpp b4e3eb99221a09404dbbf813da33607867a78691 
>   src/slave/http.cpp 69e6d74e8b113cc6c937f47df8984ff9a63e5bb4 
>   src/uri/fetchers/docker.hpp 2bb921474f3a8147d8cbf54579452f8df216d2f1 
>   src/uri/fetchers/docker.cpp 8f5fc964f056b349ce57ced139e07f538cb1cfd2 
> 
> 
> Diff: https://reviews.apache.org/r/71024/diff/1/
> 
> 
> Testing
> ---
> 
> All testing cases passed.
> 
> 
> Thanks,
> 
> fei long
> 
>

Re: Review Request 71005: Used euid to determine subprocess' user when launching tasks.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On July 3, 2019, 11:12 a.m., fei long wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71005/
> ---
> 
> (Updated July 3, 2019, 11:12 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: mesos-9876
> https://issues.apache.org/jira/browse/mesos-9876
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used euid to determine subprocess' user when launching tasks.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.cpp 
> b29ec556399a40aa662987a11a2b64e6a16de889 
> 
> 
> Diff: https://reviews.apache.org/r/71005/diff/1/
> 
> 
> Testing
> ---
> 
> All testing cases passed.
> 
> 
> Thanks,
> 
> fei long
> 
>

Re: Review Request 71022: Added tests for `UPDATE_QUOTA`.

[Mesos Reviewbot]

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



Bad patch!

Reviews applied: [71019, 71020, 71021, 71022]

Failed command: ['bash', '-c', "set -o pipefail; 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 2>&1 | tee 
build_71022"]

Error:
..
4073b43 on agent 4dac6b5f-2b59-45cf-9202-06d12f77be62-S0
I0709 03:35:01.168401 18254 status_update_manager_process.hpp:528] Forwarding 
operation status update OPERATION_FINISHED (Status UUID: 
a96cd6c2-c340-4b72-ad92-80df0992c953) for operation UUID 
b955f716-c2c3-4506-a00d-0e19f4073b43 on agent 
4dac6b5f-2b59-45cf-9202-06d12f77be62-S0
I0709 03:35:01.169265 18277 http_connection.hpp:131] Sending 2 call to 
http://172.17.0.3:42622/slave(1196)/api/v1/resource_provider
I0709 03:35:01.170203 18255 process.cpp:3671] Handling HTTP event for process 
'slave(1196)' with path: '/slave(1196)/api/v1/resource_provider'
I0709 03:35:01.173105 18275 process.cpp:3671] Handling HTTP event for process 
'master' with path: '/master/api/v1'
I0709 03:35:01.174614 18276 http.cpp:1115] HTTP POST for /master/api/v1 from 
172.17.0.3:60644
I0709 03:35:01.174844 18276 http.cpp:263] Processing call UNRESERVE_RESOURCES
I0709 03:35:01.175524 18276 master.cpp:3785] Authorizing principal 
'test-principal' to unreserve resources 
'[{"disk":{"source":{"id":"/tmp/CSIVersion_StorageLocalResourceProviderTest_OperatorOperationsWithResourceProviderResources_v0_CidaWX/2GB-7f5f815d-7d9f-4de7-86e5-80d2a26ce62b","mount":{"root":"./csi/org.apache.mesos.csi.test/local/mounts"},"profile":"test","type":"MOUNT","vendor":"org.apache.mesos.csi.test.local"}},"name":"disk","provider_id":{"value":"6c76a1b0-a9eb-4a97-b9db-4ba08b6011e3"},"reservations":[{"role":"storage","type":"DYNAMIC"},{"principal":"test-principal","role":"storage/default-role","type":"DYNAMIC"}],"scalar":{"value":2048.0},"type":"SCALAR"}]'
I0709 03:35:01.177569 18271 master.cpp:12470] Removing offer 
4dac6b5f-2b59-45cf-9202-06d12f77be62-O5
I0709 03:35:01.177723 18261 sched.cpp:960] Rescinded offer 
4dac6b5f-2b59-45cf-9202-06d12f77be62-O5
I0709 03:35:01.177804 18261 sched.cpp:971] Scheduler::offerRescinded took 
21895ns
I0709 03:35:01.178423 18268 hierarchical.cpp:1218] Recovered disk(allocated: 
storage/default-role)(reservations: 
[(DYNAMIC,storage),(DYNAMIC,storage/default-role,test-principal)])[MOUNT(org.apache.mesos.csi.test.local,/tmp/CSIVersion_StorageLocalResourceProviderTest_OperatorOperationsWithResourceProviderResources_v0_CidaWX/2GB-7f5f815d-7d9f-4de7-86e5-80d2a26ce62b,test)]:2048;
 cpus(allocated: storage/default-role):2; mem(allocated: 
storage/default-role):1024; disk(allocated: storage/default-role):1024; 
ports(allocated: storage/default-role):[31000-32000] (total: cpus:2; mem:1024; 
disk:1024; ports:[31000-32000]; disk(reservations: 
[(DYNAMIC,storage),(DYNAMIC,storage/default-role,test-principal)])[MOUNT(org.apache.mesos.csi.test.local,/tmp/CSIVersion_StorageLocalResourceProviderTest_OperatorOperationsWithResourceProviderResources_v0_CidaWX/2GB-7f5f815d-7d9f-4de7-86e5-80d2a26ce62b,test)]:2048,
 allocated: {}) on agent 4dac6b5f-2b59-45cf-9202-06d12f77be62-S0 from framework 
4dac6b5f-2b59-45
 cf-9202-06d12f77be62-
I0709 03:35:01.178540 18268 hierarchical.cpp:1264] Framework 
4dac6b5f-2b59-45cf-9202-06d12f77be62- filtered agent 
4dac6b5f-2b59-45cf-9202-06d12f77be62-S0 for 5secs
I0709 03:35:01.180860 18263 master.cpp:12361] Sending operation '' (uuid: 
db54fe76-3dda-41ef-9384-44f211cf8517) to agent 
4dac6b5f-2b59-45cf-9202-06d12f77be62-S0 at slave(1196)@172.17.0.3:42622 
(fc165f183aad)
I0709 03:35:01.181380 18277 slave.cpp:4198] Ignoring new checkpointed resources 
and operations identical to the current version
I0709 03:35:01.183845 18260 hierarchical.cpp:1508] Performed allocation for 1 
agents in 1.163109ms
I0709 03:35:01.184108 18273 provider.cpp:481] Received APPLY_OPERATION event
I0709 03:35:01.184156 18273 provider.cpp:1295] Received UNRESERVE operation '' 
(uuid: db54fe76-3dda-41ef-9384-44f211cf8517)
I0709 03:35:01.184527 18266 master.cpp:10195] Sending offers [ 
4dac6b5f-2b59-45cf-9202-06d12f77be62-O6 ] to framework 
4dac6b5f-2b59-45cf-9202-06d12f77be62- (default) at 
scheduler-68435923-60d8-4dc3-a43a-03309815a5d5@172.17.0.3:42622
I0709 03:35:01.185142 18276 sched.cpp:934] Scheduler::resourceOffers took 
69570ns
I0709 03:35:01.209959 18265 http.cpp:1115] HTTP POST for 
/slave(1196)/api/v1/resource_provider from 172.17.0.3:60590
I0709 03:35:01.210876 18256 slave.cpp:8217] Handling resource provider message 
'UPDATE_OPERATION_STATUS: (uuid: b955f716-c2c3-4506-a00d-0e19f4073b43) for 
framework  (latest state: OPERATION_FINISHED, status update state: 

Re: Review Request 70757: Added a NNP isolator.

[Jacob Janco]

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

(Updated July 9, 2019, 12:55 a.m.)


Review request for mesos, Andrei Budnik, Gilbert Song, Jie Yu, and James Peach.


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


Repository: mesos


Description
---

This patch adds the isolation capability of flipping the
NO_NEW_PRIVILEGES bit for process control.


Diffs (updated)
-

  include/mesos/slave/containerizer.proto 
2d04f3c182a4274dda527a3da56c894c3c892a12 
  src/CMakeLists.txt 1d4f541b73c07a307a8b61f217e0cfad5dc095e4 
  src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153 
  src/slave/containerizer/mesos/containerizer.cpp 
043244841a73fa3f5f7119bc38f6d3a04be8990b 
  src/slave/containerizer/mesos/isolators/linux/nnp.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/linux/nnp.cpp PRE-CREATION 
  src/slave/containerizer/mesos/launch.cpp 
b29ec556399a40aa662987a11a2b64e6a16de889 
  src/tests/CMakeLists.txt 3ea8b4400ff3b00470aa147cb8f39f62802727e3 
  src/tests/containerizer/linux_nnp_isolator_tests.cpp PRE-CREATION 


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

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


Testing
---


Thanks,

Jacob Janco

Re: Review Request 71030: Added quota_consumption field to /roles endpoint.

[Meng Zhu]

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



As mentioned in previous patch, add composition? A TODO is fine.


src/master/readonly_handler.cpp
Lines 738 (patched)


I am thinking we should just call it `consumption`, `quota_consumption` 
might throw some people off guard if a role has default quota (where user did 
not set any quota on). It is also more clean to me, as we say allocation, not 
quota_allocation.

If we change this, should also change the accessor function name.


- Meng Zhu


On July 8, 2019, 11:07 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71030/
> ---
> 
> (Updated July 8, 2019, 11:07 a.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Meng Zhu.
> 
> 
> Bugs: MESOS-9871
> https://issues.apache.org/jira/browse/MESOS-9871
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Per the previous commit that adds the quota consumption computation,
> quota consumption is:
> 
>   Allocation + Unallocated Reservation ==
>   Reservations + Unreserved Allocation
> 
> That is, reservations count towards quota regardless of whether
> they're allocated. Allocation counts towards quota. Offered resources
> *do not* count towards quota, this is to (1) provide stability of the
> quota consumption metrics in the face of offers flowing in and out,
> and (2) to ensure that we treat offers as rescindable and therefore
> not yet "counting" towards quota. Also, if in the future schedulers
> are offered more than their quota to improve their choices, counting
> offered resources as quota consumption will be problematic.
> 
> 
> Diffs
> -
> 
>   src/master/readonly_handler.cpp 0d1e3dc19352863263c3e8992a63852c7f27225b 
>   src/tests/role_tests.cpp bf5b3cba59d38eee41ed222d14bc8c928fcf2a79 
> 
> 
> Diff: https://reviews.apache.org/r/71030/diff/1/
> 
> 
> Testing
> ---
> 
> Added a test in a subsequent patch.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 71029: Added Role::quotaConsumption to the master.

[Meng Zhu]

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




src/master/master.hpp
Lines 2714-2715 (patched)


If we want to expose the composition, reserved unused, unreserved used and 
etc. We probably want to split this into several functions? e.g. allocation(), 
reservation(), used_reservaiton() and then combine as needed in the endpoint 
handler.


- Meng Zhu


On July 8, 2019, 11:07 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71029/
> ---
> 
> (Updated July 8, 2019, 11:07 a.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Meng Zhu.
> 
> 
> Bugs: MESOS-9871
> https://issues.apache.org/jira/browse/MESOS-9871
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Quota consumption is computed as follows:
> 
>   Allocation + Unallocated Reservation ==
>   Reservations + Unreserved Allocation
> 
> That is, reservations count towards quota regardless of whether
> they're allocated. Allocation counts towards quota. Offered resources
> *do not* count towards quota, this is to (1) provide stability of the
> quota consumption metrics in the face of offers flowing in and out,
> and (2) to ensure that we treat offers as rescindable and therefore
> not yet "counting" towards quota. Also, if in the future schedulers
> are offered more than their quota to improve their choices, counting
> offered resources as quota consumption will be problematic.
> 
> 
> Diffs
> -
> 
>   src/master/framework.cpp c35de2574ef683a9f397a957422fdbf8fa751313 
>   src/master/master.hpp 23dafe746b6f9b3d70ad7220f54c4d49068b8af8 
> 
> 
> Diff: https://reviews.apache.org/r/71029/diff/1/
> 
> 
> Testing
> ---
> 
> Added a test in a subsequent patch.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 71029: Added Role::quotaConsumption to the master.

[Meng Zhu]

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


Fix it, then Ship it!





src/master/master.hpp
Lines 2730 (patched)


`totalUsedResources` here also includes the executor resources which are 
not managed by the allocator. Do we want to include that in the consumption?

If no, then the consumption is not going to be consistent with framework 
stats. But if yes, since the consumption will be compared with quota (both from 
the user perspective as well as internally in the future), that means when 
operator configures the quota, they need to take executor resources into 
account. I am not sure if that's good user experience.

For simplicity, I am OK with leaving it as is now. But let's document the 
rationale and/or any todos.

Also please update the description for this.



src/master/master.hpp
Lines 2743-2752 (patched)


how about total_reservaiton - used_reservation:

```
ResourceQuantities unallocatedReservation;

 foreachvalue (Slave* slave, master->slaves.registered) {
   ResourceQuantities totalReservation =
 ResourceQuantities::fromScalarResources(
   slave->totalResources.filter(reservedToRoleSubtree).scalars());
   
   ResourceQuantities usedReservation =
 ResourceQuantities::fromScalarResources(
   slave->usedResources.filter(reservedToRoleSubtree).scalars());
   
   unallocatedReservation += totalReservation - usedReservation;
 }

```

Should also be more performant to use ResourceQuantities and no need to 
clear the allocation info.



src/master/master.hpp
Lines 2758 (patched)


Nits, how about avoiding the addition and the two variables by just having 
a `consumption`:

```
// Quota consumption = allocation + unallocated reservation

ResourceQuantities consumption;

// Add allocation



// Add unallocated reservation
...

return consumption;

```


- Meng Zhu


On July 8, 2019, 11:07 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71029/
> ---
> 
> (Updated July 8, 2019, 11:07 a.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Meng Zhu.
> 
> 
> Bugs: MESOS-9871
> https://issues.apache.org/jira/browse/MESOS-9871
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Quota consumption is computed as follows:
> 
>   Allocation + Unallocated Reservation ==
>   Reservations + Unreserved Allocation
> 
> That is, reservations count towards quota regardless of whether
> they're allocated. Allocation counts towards quota. Offered resources
> *do not* count towards quota, this is to (1) provide stability of the
> quota consumption metrics in the face of offers flowing in and out,
> and (2) to ensure that we treat offers as rescindable and therefore
> not yet "counting" towards quota. Also, if in the future schedulers
> are offered more than their quota to improve their choices, counting
> offered resources as quota consumption will be problematic.
> 
> 
> Diffs
> -
> 
>   src/master/framework.cpp c35de2574ef683a9f397a957422fdbf8fa751313 
>   src/master/master.hpp 23dafe746b6f9b3d70ad7220f54c4d49068b8af8 
> 
> 
> Diff: https://reviews.apache.org/r/71029/diff/1/
> 
> 
> Testing
> ---
> 
> Added a test in a subsequent patch.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 71008: Implemented transition from DRAINING to DRAINED in master.

[Greg Mann]

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




src/master/master.cpp
Lines 6238-6240 (patched)


Nit: I think we usually place the logical operators at the end of the 
preceding line, rather than the beginning of the next?



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


Newline before `.then`



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


Can we eliminate this temporary variable?


- Greg Mann


On July 3, 2019, 5:23 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71008/
> ---
> 
> (Updated July 3, 2019, 5:23 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Greg Mann, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-9814
> https://issues.apache.org/jira/browse/MESOS-9814
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This adds logic in the master to detect when a DRAINING agent can
> be transitioned into a DRAINED state.  When this happens, the new
> state is checkpointed into the registry and, if the agent is to be
> marked "gone", the master will remove the agent.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 23dafe746b6f9b3d70ad7220f54c4d49068b8af8 
>   src/master/master.cpp 5247377c2e7e92b9843dd4c9d28f92ba679ad742 
> 
> 
> Diff: https://reviews.apache.org/r/71008/diff/2/
> 
> 
> Testing
> ---
> 
> TODO: Need to write some unit tests.  I'll want to rebase onto the agent 
> changes so that there is more detectable stuff in the tests.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 70960: Added test for agent to leave draining state on its own.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [70822, 70839, 70834, 70835, 70836, 70899, 70900, 70901, 
70903, 70904, 70906, 70907, 70936, 70958, 70959, 70960]

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 28, 2019, 2:27 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70960/
> ---
> 
> (Updated June 28, 2019, 2:27 a.m.)
> 
> 
> Review request for mesos, Greg Mann and Joseph Wu.
> 
> 
> Bugs: MESOS-9860
> https://issues.apache.org/jira/browse/MESOS-9860
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a test which confirms that the agent leaves a draining
> state on its own once all frameworks on the agent have no more pending
> tasks and all their executors have neither launched or queued tasks.
> 
> The test uses the fact that the agent rejects task launches while
> draining.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 7ba2df9fef9966cd09e0af3f4ef3bb260b6167f6 
> 
> 
> Diff: https://reviews.apache.org/r/70960/diff/4/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 70757: Added a NNP isolator.

[James Peach]

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




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


This check should move to the isolator creation factory function. It's 
typical for isolators to enforce prerequisites there.



src/tests/containerizer/linux_nnp_isolator_tests.cpp
Lines 44 (patched)


`cout` isn't used.



src/tests/containerizer/linux_nnp_isolator_tests.cpp
Lines 85 (patched)


Add a newline.



src/tests/containerizer/linux_nnp_isolator_tests.cpp
Lines 94 (patched)


You can just use `ASSERT_SOME` on the `version` here.



src/tests/containerizer/linux_nnp_isolator_tests.cpp
Lines 99 (patched)


Here, and in the isolator check, I think that we should phrase this as:
```
kernel version >= xx.yy.zz
```

I think that the `+` could be misread as part of the version name.



src/tests/containerizer/linux_nnp_isolator_tests.cpp
Lines 100 (patched)


Maybe `LOG(INFO)`, and make the message say that you are skipping the test.



src/tests/containerizer/linux_nnp_isolator_tests.cpp
Lines 129 (patched)


I think this would be more readable with a raw string, i.e:

```
R"~(
...
)~"
```

There's a few examples of tests that use this for scripts.


- James Peach


On July 4, 2019, 12:26 a.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70757/
> ---
> 
> (Updated July 4, 2019, 12:26 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gilbert Song, Jie Yu, and James 
> Peach.
> 
> 
> Bugs: MESOS-9770
> https://issues.apache.org/jira/browse/MESOS-9770
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds the isolation capability of flipping the
> NO_NEW_PRIVILEGES bit for process control.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/containerizer.proto 
> 2d04f3c182a4274dda527a3da56c894c3c892a12 
>   src/CMakeLists.txt 1d4f541b73c07a307a8b61f217e0cfad5dc095e4 
>   src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 043244841a73fa3f5f7119bc38f6d3a04be8990b 
>   src/slave/containerizer/mesos/isolators/linux/nnp.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/linux/nnp.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/launch.cpp 
> b29ec556399a40aa662987a11a2b64e6a16de889 
>   src/tests/CMakeLists.txt 3ea8b4400ff3b00470aa147cb8f39f62802727e3 
>   src/tests/containerizer/linux_nnp_isolator_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/70757/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Re: Review Request 71034: Enabled the Docker executor to accept kill policy overrides.

[Joseph Wu]

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




src/docker/executor.hpp
Lines 157 (patched)


Missing after the type `const Option`, an `&`



src/docker/executor.cpp
Line 932 (original), 938-950 (patched)


Can you try disambiguating like this:
```
// Need to disambiguate overloaded function.
void (DockerExecutorProcess::*killTask)(
ExecutorDriver*, const TaskID&, const Option&)
  = ::killTask;

return process::dispatch(
  process.get(), killTask, driver, taskId, None());
```



src/docker/executor.cpp
Lines 980 (patched)


Missing `&` after the Option<>.


- Joseph Wu


On July 8, 2019, 11:25 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71034/
> ---
> 
> (Updated July 8, 2019, 11:25 a.m.)
> 
> 
> Review request for mesos, Benno Evers, Benjamin Mahler, and Joseph Wu.
> 
> 
> Bugs: MESOS-9853
> https://issues.apache.org/jira/browse/MESOS-9853
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This adds a new `killTask()` overload to the Docker executor
> and updates the Mesos executor driver to call into that
> overload when the loaded executor is the Docker executor.
> 
> This allows the executor driver to pass the kill policy
> override, when present, into the Docker executor.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.hpp f21e84c71f646e84404c65fc2ded64bcaff482ef 
>   src/docker/executor.cpp f638e4b65155bcca1be36424b7061ea26a3d6ca3 
>   src/exec/exec.cpp c0fa3b61667da96bc4395bae9956c54446268122 
> 
> 
> Diff: https://reviews.apache.org/r/71034/diff/1/
> 
> 
> Testing
> ---
> 
> Details at the end of this chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 71028: Renamed the roles ui column from 'Allocation' to 'Allocation + Offered'.

[Meng Zhu]

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


Ship it!




Ship It!

- Meng Zhu


On July 8, 2019, 10:46 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71028/
> ---
> 
> (Updated July 8, 2019, 10:46 a.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Meng Zhu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This column actually displays the allocation + offered amount, so this
> renames it accordingly to avoid confusion. Later, we will split these
> apart into separate selectable columns.
> 
> 
> Diffs
> -
> 
>   src/webui/app/roles/roles.html 02cb7ba09845d4f9b51630d9c59c1bbd533ac478 
> 
> 
> Diff: https://reviews.apache.org/r/71028/diff/1/
> 
> 
> Testing
> ---
> 
> make check and rendered the UI locally
> 
> 
> File Attachments
> 
> 
> Roles Table
>   
> https://reviews.apache.org/media/uploaded/files/2019/07/08/41785a71-2676-4ff6-8653-2d10442605e1__Screen_Shot_2019-07-08_at_1.43.37_PM.png
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 71027: Renamed Role::allocatedResources to Role::allocatedAndOfferedResources.

[Meng Zhu]

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


Ship it!





src/master/master.hpp
Lines 2702-2716 (original), 2702-2716 (patched)


Looking at the resource json writer, we should be able to just use resource 
quantities. The only obstacle seems to be the unfortunately named `_revocable`:

https://github.com/apache/mesos/blob/master/src/common/http.cpp#L720



src/master/master.hpp
Lines 2714-2715 (original), 2714-2715 (patched)


we are adding ranges here? another reason to use quantities.


- Meng Zhu


On July 8, 2019, 10:45 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71027/
> ---
> 
> (Updated July 8, 2019, 10:45 a.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Meng Zhu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This exposes allocated + offered, so it has been renamed accordingly.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp b42ebb953e0510e83ec6bd041cbddbeb8f60067c 
>   src/master/master.hpp 23dafe746b6f9b3d70ad7220f54c4d49068b8af8 
>   src/master/readonly_handler.cpp 0d1e3dc19352863263c3e8992a63852c7f27225b 
> 
> 
> Diff: https://reviews.apache.org/r/71027/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 70996: Implemented master endpoints for agent draining.

[Greg Mann]

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




src/master/http.cpp
Lines 3879-3881 (patched)


Nit: I think we usually place the `&&` at the end of the preceding line 
rather than the beginning of the next.

Ditto below.

Also, I wonder if we should factor this block out into a helper like `bool 
isKnownSlave(const SlaveID&)`? Then we have a single location to update if/when 
we add more containers of known slaves?



src/master/master.hpp
Lines 678 (patched)


Provide a comment documenting this new function? It may not be consistent, 
but let's start setting a good example now :)



src/master/master.hpp
Lines 2109-2110 (patched)


s/both admitted and unreachable/admitted, unreachable, and recovered/

Ditto below.



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


Having this conditional within `Master::reactivate()` looks a bit weird to 
me. What do you think about moving it to the callsite in 
`Master::___reregisterSlave()`, and instead placing a 
`CHECK(!slaves.deactivated.contains(slave->id));` within this function?


- Greg Mann


On July 2, 2019, 7:57 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70996/
> ---
> 
> (Updated July 2, 2019, 7:57 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Greg Mann, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-9814
> https://issues.apache.org/jira/browse/MESOS-9814
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fleshes out three master calls:
>   * DRAIN_AGENT
>   * DEACTIVATE_AGENT
>   * REACTIVATE_AGENT
> 
> The master now stores a mapping of agent draining or deactivation
> information.  When an agent reconnects, this information is checked
> before informing the allocator about the agent.
> 
> This commit leaves out certain aspects of each endpoint's validation,
> such as checking if draining agents are not in maintenance schedules.
> 
> The DRAIN_AGENT call is not completely implemented here, because the
> transition from DRAINING to DRAINED will be added in a separate commit.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp b42ebb953e0510e83ec6bd041cbddbeb8f60067c 
>   src/master/master.hpp 23dafe746b6f9b3d70ad7220f54c4d49068b8af8 
>   src/master/master.cpp 5247377c2e7e92b9843dd4c9d28f92ba679ad742 
> 
> 
> Diff: https://reviews.apache.org/r/70996/diff/1/
> 
> 
> Testing
> ---
> 
> TODO: At this point, there isn't anything exposed by the master which can be 
> used to check the results of these APIs.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 71020: Added overcommit and hierarchical inclusion check for `UPDATE_QUOTA`.

[Meng Zhu]

> On July 7, 2019, 3:50 p.m., Benjamin Mahler wrote:
> > I assume we'll track the missing pieces with additional tickets or leave 
> > the existing ticket open until we have them?

Yes, let's keep the ticket open.


> On July 7, 2019, 3:50 p.m., Benjamin Mahler wrote:
> > src/master/quota_handler.cpp
> > Lines 94 (patched)
> > 
> >
> > This seems like a transitive property that doesn't need to be spelled 
> > out?
> > 
> > sum of children's guarantees <=
> > parent's guarantee <=
> > parent's limit
> > 
> > It seems somewhat unnatural to directly check:
> > 
> > sum of children's guarantees <=
> > parent's limit
> > 
> > rather than check:
> > 
> > sum of children's guarantees <=
> > parent's limit
> > 
> > &&
> > 
> > guarantee <= limit

Good point! Updated the comment, commit description and related tickets.


> On July 7, 2019, 3:50 p.m., Benjamin Mahler wrote:
> > src/master/quota_handler.cpp
> > Lines 467-515 (patched)
> > 
> >
> > Hm.. is this code copied from the original quota handler? Is your plan 
> > to have this be the single path and the old api goes through this?
> > 
> > if so, can you clarify with a TODO that this is duplicated from the 
> > original quota handler and the plan is to have that go through this path?

The logic here is a bit different (supposedly more performant). Added a TODO to 
pull this out as a standalone function to be shared by the old and the new.


> On July 7, 2019, 3:50 p.m., Benjamin Mahler wrote:
> > src/master/quota_handler.cpp
> > Lines 485 (patched)
> > 
> >
> > BadRequest vs Conflict below seems a bit inconsitent? They're both 
> > violations of the constraints?

Changed to return `BadRequest` in both cases.


- Meng


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


On July 7, 2019, 3:12 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71020/
> ---
> 
> (Updated July 7, 2019, 3:12 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9812
> https://issues.apache.org/jira/browse/MESOS-9812
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The overcommit check validates that the total quota guarantees in
> the cluster is contained by the cluster capacity.
> 
> The hierarchical inclusion check validates that the sum of
> children's  guarantees is contained by the parent guarantee.
> 
> Further validation is needed for:
> 
> - Check a role's limit is less than its current consumption.
> - Check a role's limit is less than its parent's limit.
> - Check the sum of children's guarantees are less than its
> parent's limit.
> 
> 
> Diffs
> -
> 
>   src/master/quota_handler.cpp f9c743170461d8d83180db20c917d3842241d4df 
> 
> 
> Diff: https://reviews.apache.org/r/71020/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> tests in r/71022/
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 70906: Fixed pid checkpointing for `TestContainerizer`.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On July 8, 2019, 2:20 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70906/
> ---
> 
> (Updated July 8, 2019, 2:20 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Greg Mann, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In order for a `MockExecutor` to be able to reregister after agent
> restart a persisted pid is required. This patch adds checkpointing of
> the pid.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer.cpp fab7e81339f3b7b3fec2a4f6cb7e3a52df2607ed 
> 
> 
> Diff: https://reviews.apache.org/r/70906/diff/3/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 70906: Fixed pid checkpointing for `TestContainerizer`.

[Benjamin Bannier]

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

(Updated July 8, 2019, 9:20 p.m.)


Review request for mesos, Gilbert Song, Greg Mann, and Joseph Wu.


Repository: mesos


Description
---

In order for a `MockExecutor` to be able to reregister after agent
restart a persisted pid is required. This patch adds checkpointing of
the pid.


Diffs
-

  src/tests/containerizer.cpp fab7e81339f3b7b3fec2a4f6cb7e3a52df2607ed 


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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 71032: Added a test for /roles quota_consumption field.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [71031, 71027, 71028, 71029, 71030, 71032]

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 July 8, 2019, 6:07 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71032/
> ---
> 
> (Updated July 8, 2019, 6:07 p.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Meng Zhu.
> 
> 
> Bugs: MESOS-9871
> https://issues.apache.org/jira/browse/MESOS-9871
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This ensures that the various edge different states of resources
> are correctly handled, see test comment for more details.
> 
> 
> Diffs
> -
> 
>   src/tests/role_tests.cpp bf5b3cba59d38eee41ed222d14bc8c928fcf2a79 
> 
> 
> Diff: https://reviews.apache.org/r/71032/diff/1/
> 
> 
> Testing
> ---
> 
> Ran 1000 times in repetition.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 71029: Added Role::quotaConsumption to the master.

[Benjamin Mahler]

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




src/master/master.hpp
Lines 2728 (patched)


If we want this to accurately capture the hierarchical consumption, this 
needs to loop over all frameworks (i.e. `master->frameworks`) instead.


- Benjamin Mahler


On July 8, 2019, 6:07 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71029/
> ---
> 
> (Updated July 8, 2019, 6:07 p.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Meng Zhu.
> 
> 
> Bugs: MESOS-9871
> https://issues.apache.org/jira/browse/MESOS-9871
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Quota consumption is computed as follows:
> 
>   Allocation + Unallocated Reservation ==
>   Reservations + Unreserved Allocation
> 
> That is, reservations count towards quota regardless of whether
> they're allocated. Allocation counts towards quota. Offered resources
> *do not* count towards quota, this is to (1) provide stability of the
> quota consumption metrics in the face of offers flowing in and out,
> and (2) to ensure that we treat offers as rescindable and therefore
> not yet "counting" towards quota. Also, if in the future schedulers
> are offered more than their quota to improve their choices, counting
> offered resources as quota consumption will be problematic.
> 
> 
> Diffs
> -
> 
>   src/master/framework.cpp c35de2574ef683a9f397a957422fdbf8fa751313 
>   src/master/master.hpp 23dafe746b6f9b3d70ad7220f54c4d49068b8af8 
> 
> 
> Diff: https://reviews.apache.org/r/71029/diff/1/
> 
> 
> Testing
> ---
> 
> Added a test in a subsequent patch.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 70924: Added registry operations for DE/RE-ACTIVATE_AGENT calls.

[Joseph Wu]

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

(Updated July 8, 2019, 11:39 a.m.)


Review request for mesos, Benjamin Bannier, Benjamin Mahler, Greg Mann, and 
Vinod Kone.


Changes
---

Updated to use minimum capability helper.
Added extra conditionals to stop looping through agents earlier.


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


Repository: mesos


Description
---

This adds the associated registry operation and fields for the
DEACTIVATE_AGENT and REACTIVATE_AGENT master calls.

Like the DRAIN_AGENT call, the deactivation state also persists when
agents become unreachable/reachable, which is already handled by the
DRAIN_AGENT operation implementation.

Likewise, this feature is not downgrade compatible, and a minimum
capability is added when the deactivation feature is used.
If all draining or deactivated agents are reactivated, the minimum
capability is removed.


Diffs (updated)
-

  src/master/registry_operations.hpp 5a3010d46c4eddb0d660eb7368ec836903477a9b 
  src/master/registry_operations.cpp c417c4d6c4272eeab15f5313ca5160d4a0aa4da4 
  src/tests/registrar_tests.cpp 5f0de523705d124ddd2d21ad355f06633d68c141 


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

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


Testing
---

make check


Thanks,

Joseph Wu

Re: Review Request 70923: Added a registry operation for the DRAIN_AGENT call.

[Joseph Wu]

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

(Updated July 8, 2019, 11:38 a.m.)


Review request for mesos, Benjamin Bannier, Benjamin Mahler, Greg Mann, and 
Vinod Kone.


Changes
---

Updated to use the minimum capability helper.


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


Repository: mesos


Description
---

This adds the associated registry operation and fields for the
DRAIN_AGENT master call.  This call affects admitted or unreachable
agents, but this commit only deals with admitted agents.

Because this feature is not downgrade compatible, a minimum capability
is added when the draining feature is used.


Diffs (updated)
-

  src/master/registry.proto 67904edf73235a4007f7c59c05dc4f1abab1cd08 
  src/master/registry_operations.hpp 5a3010d46c4eddb0d660eb7368ec836903477a9b 
  src/master/registry_operations.cpp c417c4d6c4272eeab15f5313ca5160d4a0aa4da4 
  src/tests/registrar_tests.cpp 5f0de523705d124ddd2d21ad355f06633d68c141 


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

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


Testing
---

make check


Thanks,

Joseph Wu

Review Request 71035: Added test to verify that Docker executor can override kill policy.

[Greg Mann]

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

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


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


Repository: mesos


Description
---

This adds a test which verifies that when a scheduler attemps to
override a task's default kill policy, the Docker executor will
honor that override.


Diffs
-

  src/internal/evolve.hpp ffbb342a42ec3501383500dee55dff3588725478 
  src/internal/evolve.cpp 81de15e192580d2f35237e124c11902e1fc67a1d 
  src/tests/containerizer/docker_containerizer_tests.cpp 
a6217581e20168c5114f733323e927a83ac59fd0 


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


Testing
---

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

Verified that this new test fails on current master, and passes when the 
preceding patches in this chain are applied.


Thanks,

Greg Mann

Review Request 71034: Enabled the Docker executor to accept kill policy overrides.

[Greg Mann]

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

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


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


Repository: mesos


Description
---

This adds a new `killTask()` overload to the Docker executor
and updates the Mesos executor driver to call into that
overload when the loaded executor is the Docker executor.

This allows the executor driver to pass the kill policy
override, when present, into the Docker executor.


Diffs
-

  src/docker/executor.hpp f21e84c71f646e84404c65fc2ded64bcaff482ef 
  src/docker/executor.cpp f638e4b65155bcca1be36424b7061ea26a3d6ca3 
  src/exec/exec.cpp c0fa3b61667da96bc4395bae9956c54446268122 


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


Testing
---

Details at the end of this chain.


Thanks,

Greg Mann

Review Request 71033: Moved the Docker executor declaration into a header.

[Greg Mann]

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

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


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


Repository: mesos


Description
---

This moves the declaration of the Docker executor into the
Docker executor header file and moves the code for the Docker
executor binary into a new launcher implementation file.

This change will enable the Mesos executor driver
implementation to make use of the `DockerExecutor` symbol.


Diffs
-

  src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153 
  src/docker/executor.hpp f21e84c71f646e84404c65fc2ded64bcaff482ef 
  src/docker/executor.cpp f638e4b65155bcca1be36424b7061ea26a3d6ca3 
  src/launcher/docker_executor.cpp PRE-CREATION 


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


Testing
---

Details at the end of this chain.


Thanks,

Greg Mann

Review Request 71029: Added Role::quotaConsumption to the master.

[Benjamin Mahler]

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

Review request for mesos, Andrei Sekretenko and Meng Zhu.


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


Repository: mesos


Description
---

Quota consumption is computed as follows:

  Allocation + Unallocated Reservation ==
  Reservations + Unreserved Allocation

That is, reservations count towards quota regardless of whether
they're allocated. Allocation counts towards quota. Offered resources
*do not* count towards quota, this is to (1) provide stability of the
quota consumption metrics in the face of offers flowing in and out,
and (2) to ensure that we treat offers as rescindable and therefore
not yet "counting" towards quota. Also, if in the future schedulers
are offered more than their quota to improve their choices, counting
offered resources as quota consumption will be problematic.


Diffs
-

  src/master/framework.cpp c35de2574ef683a9f397a957422fdbf8fa751313 
  src/master/master.hpp 23dafe746b6f9b3d70ad7220f54c4d49068b8af8 


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


Testing
---

Added a test in a subsequent patch.


Thanks,

Benjamin Mahler

Review Request 71032: Added a test for /roles quota_consumption field.

[Benjamin Mahler]

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

Review request for mesos, Andrei Sekretenko and Meng Zhu.


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


Repository: mesos


Description
---

This ensures that the various edge different states of resources
are correctly handled, see test comment for more details.


Diffs
-

  src/tests/role_tests.cpp bf5b3cba59d38eee41ed222d14bc8c928fcf2a79 


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


Testing
---

Ran 1000 times in repetition.


Thanks,

Benjamin Mahler

Review Request 71030: Added quota_consumption field to /roles endpoint.

[Benjamin Mahler]

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

Review request for mesos, Andrei Sekretenko and Meng Zhu.


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


Repository: mesos


Description
---

Per the previous commit that adds the quota consumption computation,
quota consumption is:

  Allocation + Unallocated Reservation ==
  Reservations + Unreserved Allocation

That is, reservations count towards quota regardless of whether
they're allocated. Allocation counts towards quota. Offered resources
*do not* count towards quota, this is to (1) provide stability of the
quota consumption metrics in the face of offers flowing in and out,
and (2) to ensure that we treat offers as rescindable and therefore
not yet "counting" towards quota. Also, if in the future schedulers
are offered more than their quota to improve their choices, counting
offered resources as quota consumption will be problematic.


Diffs
-

  src/master/readonly_handler.cpp 0d1e3dc19352863263c3e8992a63852c7f27225b 
  src/tests/role_tests.cpp bf5b3cba59d38eee41ed222d14bc8c928fcf2a79 


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


Testing
---

Added a test in a subsequent patch.


Thanks,

Benjamin Mahler

Review Request 71031: Updated LaunchTask test action to use a unique TaskID across runs.

[Benjamin Mahler]

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

Review request for mesos, Andrei Sekretenko and Meng Zhu.


Repository: mesos


Description
---

The existing incremented counter is problematic if the action is
used multiple times in a test, as the task id will clash across
usages.

No tests appear to rely on the existing task id assignment scheme,
so this updates it to be randomly unique via UUID.


Diffs
-

  src/tests/mesos.hpp d0c82fa4c1800ed2a1825d563a9462ecd41b77fd 


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


Testing
---

make check


Thanks,

Benjamin Mahler

Review Request 71028: Renamed the roles ui column from 'Allocation' to 'Allocation + Offered'.

[Benjamin Mahler]

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

Review request for mesos, Andrei Sekretenko and Meng Zhu.


Repository: mesos


Description
---

This column actually displays the allocation + offered amount, so this
renames it accordingly to avoid confusion. Later, we will split these
apart into separate selectable columns.


Diffs
-

  src/webui/app/roles/roles.html 02cb7ba09845d4f9b51630d9c59c1bbd533ac478 


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


Testing
---

make check and rendered the UI locally


File Attachments


Roles Table
  
https://reviews.apache.org/media/uploaded/files/2019/07/08/41785a71-2676-4ff6-8653-2d10442605e1__Screen_Shot_2019-07-08_at_1.43.37_PM.png


Thanks,

Benjamin Mahler

Review Request 71027: Renamed Role::allocatedResources to Role::allocatedAndOfferedResources.

[Benjamin Mahler]

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

Review request for mesos, Andrei Sekretenko and Meng Zhu.


Repository: mesos


Description
---

This exposes allocated + offered, so it has been renamed accordingly.


Diffs
-

  src/master/http.cpp b42ebb953e0510e83ec6bd041cbddbeb8f60067c 
  src/master/master.hpp 23dafe746b6f9b3d70ad7220f54c4d49068b8af8 
  src/master/readonly_handler.cpp 0d1e3dc19352863263c3e8992a63852c7f27225b 


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


Testing
---

make check


Thanks,

Benjamin Mahler

Re: Review Request 71024: Supported insecure registry when provisioning docker images.

[Mesos Reviewbot]

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



Bad review!

Reviews applied: []

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

- Mesos Reviewbot


On July 8, 2019, 2:27 p.m., fei long wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71024/
> ---
> 
> (Updated July 8, 2019, 2:27 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-6183
> https://issues.apache.org/jira/browse/MESOS-6183
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported insecure registry when provisioning docker images.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
> d013c9d71c39c09e600f181aba31b8037aa9226a 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 35b6afbb6b22575b90963927352443a8ddaf9885 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> 286ee5f00933b4c86a96dc4e10e42f9e7eac0ce2 
>   src/slave/flags.hpp a10bf698dc447b1411c06083c2ba7585d7a78389 
>   src/slave/flags.cpp b4e3eb99221a09404dbbf813da33607867a78691 
>   src/slave/http.cpp 69e6d74e8b113cc6c937f47df8984ff9a63e5bb4 
>   src/uri/fetchers/docker.hpp 2bb921474f3a8147d8cbf54579452f8df216d2f1 
>   src/uri/fetchers/docker.cpp 8f5fc964f056b349ce57ced139e07f538cb1cfd2 
> 
> 
> Diff: https://reviews.apache.org/r/71024/diff/1/
> 
> 
> Testing
> ---
> 
> All testing cases passed.
> 
> 
> Thanks,
> 
> fei long
> 
>

Review Request 71024: Supported insecure registry when provisioning docker images.

[fei long]

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

Review request for mesos.


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


Repository: mesos


Description
---

Supported insecure registry when provisioning docker images.


Diffs
-

  src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
d013c9d71c39c09e600f181aba31b8037aa9226a 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
35b6afbb6b22575b90963927352443a8ddaf9885 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
286ee5f00933b4c86a96dc4e10e42f9e7eac0ce2 
  src/slave/flags.hpp a10bf698dc447b1411c06083c2ba7585d7a78389 
  src/slave/flags.cpp b4e3eb99221a09404dbbf813da33607867a78691 
  src/slave/http.cpp 69e6d74e8b113cc6c937f47df8984ff9a63e5bb4 
  src/uri/fetchers/docker.hpp 2bb921474f3a8147d8cbf54579452f8df216d2f1 
  src/uri/fetchers/docker.cpp 8f5fc964f056b349ce57ced139e07f538cb1cfd2 


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


Testing
---

All testing cases passed.


Thanks,

fei long