Re: Review Request 64018: Added tests for the d_type support validation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64018/ --- (Updated Nov. 28, 2017, 8:24 p.m.) Review request for mesos, Gilbert Song and

Re: Review Request 63731: Reconciled pending resource provider operations in agent.

> On Nov. 29, 2017, 3:13 a.m., Jie Yu wrote: > > src/slave/slave.cpp > > Lines 6900 (patched) > > > > > > See my comments in https://reviews.apache.org/r/63843/ > > > > What if both the RP and the agent

Re: Review Request 63731: Reconciled pending resource provider operations in agent.

> On Nov. 29, 2017, 3:13 a.m., Jie Yu wrote: > > src/slave/slave.cpp > > Lines 6900 (patched) > > > > > > See my comments in https://reviews.apache.org/r/63843/ > > > > What if both the RP and the agent

Re: Review Request 63377: Added filesystem layout for CSI plugins.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63377/ --- (Updated Nov. 29, 2017, 3:15 a.m.) Review request for mesos, Jie Yu, Joseph

Re: Review Request 63731: Reconciled pending resource provider operations in agent.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63731/#review192104 --- src/slave/slave.cpp Lines 6900 (patched)

Re: Review Request 63843: Implemented a test of offer operation reconcilation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63843/#review192103 --- src/tests/slave_tests.cpp Lines 8831-8839 (patched)

Re: Review Request 64096: Implemented the `OfferOperationStatusUpdateManager`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64096/ --- (Updated Nov. 28, 2017, 6:46 p.m.) Review request for mesos and Greg Mann.

Re: Review Request 63213: Added mount propagation support for HOST_PATH volumes.

> On Nov. 29, 2017, 1:39 a.m., Gilbert Song wrote: > > Should we update the upgrades.md in a followup patch for semantic changes? Yes, and I'll add doc too. - Jie --- This is an automatically generated e-mail. To reply, visit:

Re: Review Request 63200: Added ContainerMountInfo to avoid pre_exec_commands for mounts.

> On Nov. 28, 2017, 8:15 p.m., Gilbert Song wrote: > > src/slave/containerizer/mesos/isolators/filesystem/linux.cpp > > Lines 391-394 (original) > > > > > > Should we just remove MesosContainerizerMount subcommand

Re: Review Request 63732: Reconciled offer operations between agent and master.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63732/#review192090 --- Fix it, then Ship it! src/master/master.cpp Lines 7108-7111

Re: Review Request 64109: Renamed secret key to JWT secret key in the code base.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64109/#review192096 --- Ship it! Ship It! - Chun-Hung Hsiao On Nov. 28, 2017, 2:46

Re: Review Request 64139: Refactored MockSlave creation to use cluster::Slave interfaces.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64139/#review192095 --- Fix it, then Ship it! src/tests/cluster.hpp Line 159

Re: Review Request 63213: Added mount propagation support for HOST_PATH volumes.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63213/#review192093 --- Ship it! Should we update the upgrades.md in a followup patch

Re: Review Request 64132: Allowed the injection of secret generator into the Slave.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64132/#review192092 --- Ship it! Ship It! - Chun-Hung Hsiao On Nov. 28, 2017, 8:33

Re: Review Request 64132: Allowed the injection of secret generator into the Slave.

> On Nov. 29, 2017, 1:11 a.m., Chun-Hung Hsiao wrote: > > src/slave/main.cpp > > Lines 566 (patched) > > > > > > One line apart here. I think i did that in the next patch > On Nov. 29, 2017, 1:11 a.m., Chun-Hung

Re: Review Request 64132: Allowed the injection of secret generator into the Slave.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64132/#review192084 --- src/slave/main.cpp Lines 566 (patched)

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

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61473/ --- (Updated Nov. 29, 2017, 12:59 a.m.) Review request for mesos, James Peach,

Re: Review Request 64135: Removed stringification for hashmaps of strings.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64135/#review192087 --- Ship it! Ship It! - Jie Yu On Nov. 28, 2017, 9:31 p.m.,

Re: Review Request 64000: Added generic stringification of hashmap of stringifiable types.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64000/#review192086 --- Ship it! Ship It! - Jie Yu On Nov. 28, 2017, 9:32 p.m.,

Review Request 64139: Refactored MockSlave creation to use cluster::Slave interfaces.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64139/ --- Review request for mesos, Chun-Hung Hsiao and Greg Mann. Repository: mesos

Re: Review Request 63817: Windows: Enabled more agent tests.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63817/ --- (Updated Nov. 28, 2017, 4:10 p.m.) Review request for mesos, Akash Gupta, Jie

Re: Review Request 63816: Windows: Fixed MESOS-6816 to enable `ExecutorEnvironmentVariables`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63816/ --- (Updated Nov. 28, 2017, 4:10 p.m.) Review request for mesos, Akash Gupta, Jie

Re: Review Request 63814: Windows: Fixed `os::host_default_path()`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63814/ --- (Updated Nov. 28, 2017, 4:09 p.m.) Review request for mesos, Akash Gupta, Jie

Re: Review Request 63812: Windows: Fixed `os::realpath` to behave like POSIX version.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63812/ --- (Updated Nov. 28, 2017, 4:08 p.m.) Review request for mesos, Akash Gupta, Jie

Re: Review Request 63212: Added a findByTarget method for fs::MountInfoTable.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63212/#review192080 --- Ship it! Ship It! - Gilbert Song On Nov. 27, 2017, 4:28

Re: Review Request 63211: Moved the logic of setting '/' as rslave into the launch helper.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63211/#review192079 --- Ship it! Ship It! - Gilbert Song On Nov. 26, 2017, 8:49

Re: Review Request 64136: Introduced an allocator helper function to track used resources.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64136/#review192078 --- src/master/allocator/mesos/hierarchical.cpp Lines 284-304

Re: Review Request 63022: Imported resources from CSI plugins in storage local resource provider.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63022/ --- (Updated Nov. 28, 2017, 11:39 p.m.) Review request for mesos, James DeFelice,

Re: Review Request 64018: Added tests for the d_type support validation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64018/#review192074 --- src/tests/containerizer/xfs_quota_tests.cpp Lines 53 (patched)

Re: Review Request 63823: Initialized and subscribed storage local resource provider.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63823/ --- (Updated Nov. 28, 2017, 11:14 p.m.) Review request for mesos, Greg Mann, Jie

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

> On Nov. 27, 2017, 4:25 p.m., Ilya Pronin wrote: > > src/master/http.cpp > > Lines 343-345 (patched) > > > > > > I may be ignorant of the discussion behind this, but since we don't > > treat these tasks as

Re: Review Request 64098: Send status updates when agent re-registers.

> On Nov. 28, 2017, 11:22 a.m., Jiang Yan Xu wrote: > > src/master/master.cpp > > Lines 6788 (patched) > > > > > > I think this type of status updates could benefit from a distinct > > reason for to make it more

Re: Review Request 63732: Reconciled offer operations between agent and master.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63732/ --- (Updated Nov. 28, 2017, 10:32 p.m.) Review request for mesos, Jie Yu and Jan

Re: Review Request 63731: Reconciled pending resource provider operations in agent.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63731/ --- (Updated Nov. 28, 2017, 10:32 p.m.) Review request for mesos, Greg Mann, Jie

Re: Review Request 63842: Allowed removing non-terminal offer operations.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63842/ --- (Updated Nov. 28, 2017, 10:32 p.m.) Review request for mesos, Greg Mann, Jie

Review Request 64135: Removed stringification for hashmaps of strings.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64135/ --- Review request for mesos and Jie Yu. Repository: mesos Description ---

Re: Review Request 63997: Added a new allocator method to add resources to agents.

> On Nov. 28, 2017, 12:26 a.m., Benjamin Mahler wrote: > > include/mesos/allocator/allocator.hpp > > Lines 220 (patched) > > > > > > Hm.. why is this called out? I don't think a caller could depend on > > this for

Re: Review Request 64000: Added generic stringification of hashmap of stringifiable types.

> On Nov. 27, 2017, 7:24 p.m., Jie Yu wrote: > > include/mesos/type_utils.hpp > > Line 467 (original), 467 (patched) > > > > > > Do you also need to update v1/mesos.cpp? > > Benjamin Bannier wrote: > This makes

Re: Review Request 63732: Reconciled offer operations between agent and master.

> On Nov. 27, 2017, 9:03 p.m., Jie Yu wrote: > > src/master/master.cpp > > Lines 7211-7218 (patched) > > > > > > i feel this is unnecessary. It should be the responsibility of the > > agent to convert the

Review Request 64136: Introduced an allocator helper function to track used resources.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64136/ --- Review request for mesos, Benjamin Mahler and Jie Yu. Repository: mesos

Re: Review Request 63997: Added a new allocator method to add resources to agents.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63997/ --- (Updated Nov. 28, 2017, 10:32 p.m.) Review request for mesos, Benjamin Mahler

Re: Review Request 64086: Removed currently unneeded 'AWAIT_READY's in 'MockResourceProvider'.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64086/ --- (Updated Nov. 28, 2017, 10:32 p.m.) Review request for mesos, Jie Yu and Jan

Re: Review Request 64000: Added generic stringification of hashmap of stringifiable types.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64000/ --- (Updated Nov. 28, 2017, 10:32 p.m.) Review request for mesos, Alexander

Re: Review Request 63843: Implemented a test of offer operation reconcilation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63843/ --- (Updated Nov. 28, 2017, 10:32 p.m.) Review request for mesos, Greg Mann, Jie

Review Request 64132: Allowed the injection of secret generator into the Slave.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64132/ --- Review request for mesos, Chun-Hung Hsiao and Greg Mann. Repository: mesos

Re: Review Request 63210: Moved mounts prepare to a helper in launch.cpp.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63210/#review192061 --- Ship it! Ship It! - Gilbert Song On Nov. 26, 2017, 8:49

Re: Review Request 63200: Added ContainerMountInfo to avoid pre_exec_commands for mounts.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63200/#review192057 --- Fix it, then Ship it!

Re: Review Request 63377: Added filesystem layout for CSI plugins.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63377/ --- (Updated Nov. 28, 2017, 8:06 p.m.) Review request for mesos, Jie Yu, Joseph

Re: Review Request 64107: Renamed agent flag '--executor_secret_key' to '--jwt_secret_key'.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64107/#review192055 --- Ship it! Ship It! - Greg Mann On Nov. 28, 2017, 2:46 a.m.,

Re: Review Request 64098: Send status updates when agent re-registers.

> On Nov. 28, 2017, 11:22 a.m., Jiang Yan Xu wrote: > > src/master/master.cpp > > Lines 6788 (patched) > > > > > > I think this type of status updates could benefit from a distinct > > reason for to make it more

Re: Review Request 64109: Renamed secret key to JWT secret key in the code base.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64109/#review192053 --- Ship it! Ship It! - Greg Mann On Nov. 28, 2017, 2:46 a.m.,

Re: Review Request 64098: Send status updates when agent re-registers.

> On Nov. 28, 2017, 11:22 a.m., Jiang Yan Xu wrote: > > src/master/master.cpp > > Lines 6788 (patched) > > > > > > I think this type of status updates could benefit from a distinct > > reason for to make it more

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

> On Nov. 28, 2017, 12:46 a.m., Jiang Yan Xu wrote: > > src/master/master.cpp > > Line 9370 (original), 9322 (patched) > > > > > > I have the similar feeling as Ilya, without context it's hard to > > understand

Re: Review Request 64098: Send status updates when agent re-registers.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64098/#review192041 --- Reviewed the non-testing portion. Will continue to review the

Re: Review Request 64098: Send status updates when agent re-registers.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64098/#review191979 --- Looks good to me overall. Great to see this patch coming. Do we

Re: Review Request 64028: Removed duplicate resources conversions.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64028/ --- (Updated Nov. 28, 2017, 5:41 p.m.) Review request for mesos, Benjamin Mahler

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

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61473/ --- (Updated Nov. 28, 2017, 5:28 p.m.) Review request for mesos, James Peach,

Re: Review Request 64054: Refactored offer operation handling for speculative operations.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64054/#review192038 --- Fix it, then Ship it! src/master/master.cpp Lines 9929

Re: Review Request 64054: Refactored offer operation handling for speculative operations.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64054/ --- (Updated Nov. 28, 2017, 3:25 p.m.) Review request for mesos, Benjamin Bannier

Re: Review Request 64065: Allowed resubscription of resource providers.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64065/ --- (Updated Nov. 28, 2017, 3:20 p.m.) Review request for mesos, Benjamin Bannier,

Re: Review Request 64054: Refactored offer operation handling for speculative operations.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64054/#review192026 --- src/master/master.cpp Line 10867 (original), 10879 (patched)

Re: Review Request 63910: Added 3 tests for TCP/HTTP(S) health check support for Docker container.

> On Nov. 28, 2017, 7:44 p.m., Alexander Rukletsov wrote: > > Surprisingly hard to review the diff Yeah, In `src/tests/health_check_tests.cpp`, I only removed 3 existing tests, and added 3 new tests, but did not change any other tests. I am not sure why the code diff of this patch is

Re: Review Request 63910: Added 3 tests for TCP/HTTP(S) health check support for Docker container.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63910/#review192015 --- Ship it! Surprisingly hard to review the diff - Alexander

Re: Review Request 64065: Allowed resubscription of resource providers.

> On Nov. 28, 2017, 3:28 a.m., Jie Yu wrote: > > src/resource_provider/manager.cpp > > Lines 389 (patched) > > > > > > I don't like the pointer way neither. Especially the fact that it might > > point to a stack

Re: Review Request 63910: Added 3 tests for TCP/HTTP(S) health check support for Docker container.

> On Nov. 28, 2017, 6 p.m., Alexander Rukletsov wrote: > > src/tests/health_check_tests.cpp > > Lines 2227 (patched) > > > > > > This is invoked regardless of the fixture parameter. Is this on > > purpose? Is it

Re: Review Request 64065: Allowed resubscription of resource providers.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64065/ --- (Updated Nov. 28, 2017, 12:17 p.m.) Review request for mesos, Benjamin

Re: Review Request 63910: Added 3 tests for TCP/HTTP(S) health check support for Docker container.

> On Nov. 27, 2017, 6:03 p.m., Alexander Rukletsov wrote: > > src/tests/containerizer/docker_containerizer_tests.cpp > > Lines 4963-4964 (original), 4936-4937 (patched) > > > > > > Swap these lines We need to

Re: Review Request 63910: Added 3 tests for TCP/HTTP(S) health check support for Docker container.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63910/ --- (Updated Nov. 28, 2017, 7:12 p.m.) Review request for mesos, Alexander

Re: Review Request 64054: Refactored offer operation handling for speculative operations.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64054/ --- (Updated Nov. 28, 2017, 11:42 a.m.) Review request for mesos, Benjamin Bannier

Re: Review Request 63910: Added 3 tests for TCP/HTTP(S) health check support for Docker container.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63910/#review192007 --- src/tests/health_check_tests.cpp Lines 2227 (patched)

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

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61473/#review191976 --- Almost there! include/mesos/mesos.proto Lines 344-346