Re: Review Request 64232: Add a temporary filter for overlay backend related tests.

[Meng Zhu]

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

(Updated Dec. 1, 2017, 10:17 p.m.)


Review request for mesos and Gilbert Song.


Repository: mesos


Description
---

This is a temporary filter to disable tests that use
overlay as backend on filesystems where `d_type` support is
missing. In particular, many XFS nodes are known to have this
issue.


Diffs (updated)
-

  src/tests/containerizer/provisioner_docker_tests.cpp 
832c81fe88d753b0f00dfab870d7725cf556fcef 
  src/tests/environment.cpp 607ac6cf903c71bdf8e2f4e08581ef2a352db7cd 


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

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


Testing
---

Checked affected tests.


Thanks,

Meng Zhu

Re: Review Request 64264: Added excluded image parameter to containerizer pruneImages().

[Qian Zhang]

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


Fix it, then Ship it!





src/slave/containerizer/mesos/containerizer.hpp
Lines 182-184 (original), 183-185 (patched)


Not yours. Can we swap these two lines to make it consistent with the 
declaration in `MesosContainerizer`.



src/slave/containerizer/mesos/containerizer.cpp
Lines 2839-2841 (patched)


Just a nit. I think we'd better to handle `containers_` first and then 
`excludedImages`, so what about moving these code at line 2864?



src/tests/containerizer.hpp
Line 127 (original), 127 (patched)


Kill ` excludedImages`.


- Qian Zhang


On Dec. 2, 2017, 9:10 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64264/
> ---
> 
> (Updated Dec. 2, 2017, 9:10 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Qian Zhang, Vinod Kone, and Zhitao Li.
> 
> 
> Bugs: MESOS-8295
> https://issues.apache.org/jira/browse/MESOS-8295
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added excluded image parameter to containerizer pruneImages().
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/composing.hpp 
> 3d006093f484308311e054fdd1d3d27bab3f37cd 
>   src/slave/containerizer/composing.cpp 
> 83880962c004e259619b123db8196ee8293683a8 
>   src/slave/containerizer/containerizer.hpp 
> 7a0c6fc6452325a318d9e1c91874098e401bc1ea 
>   src/slave/containerizer/docker.hpp 9df98499bb7f334d8595e22ac93126453296a0f2 
>   src/slave/containerizer/docker.cpp d837442ca073d6872c94a9332599423c005a8874 
>   src/slave/containerizer/mesos/containerizer.hpp 
> e2739e017cb8dda37d94ad809ca1bd461f308bfb 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 44e8f7acc0302f68cd237bc27fe01131a1475711 
>   src/tests/containerizer.hpp 3d162fecf03baf4747fa8b833fb0292d55195003 
>   src/tests/containerizer.cpp 665bd5d3968e47c243b5a47589715d1649066183 
>   src/tests/containerizer/mock_containerizer.hpp 
> bbfa7681a874158caf085d132b16b59b74a0c4ab 
> 
> 
> Diff: https://reviews.apache.org/r/64264/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 64266: Added an optional agent flag '--image_gc_config'.

[Qian Zhang]

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


Fix it, then Ship it!





docs/configuration/agent.md
Lines 882 (patched)


Since it can be a file or a string, I would suggest to change it from 
`JSON-formatted config file` to `JSON-formatted configuration`.

And do we want to mention it can only work with Mesos containerizer?



docs/configuration/agent.md
Lines 886 (patched)


s/image auto gc/auto image gc/



docs/configuration/agent.md
Lines 889 (patched)


s/details/the expected format/



src/slave/flags.cpp
Lines 159-166 (patched)


Ditto for my comments in agent.md.


- Qian Zhang


On Dec. 2, 2017, 9:10 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64266/
> ---
> 
> (Updated Dec. 2, 2017, 9:10 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Qian Zhang, Vinod Kone, and Zhitao Li.
> 
> 
> Bugs: MESOS-8294
> https://issues.apache.org/jira/browse/MESOS-8294
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is an optional agent flag. If it is not set, it means
> the automatic container image gc is not enabled. Users have
> to trigger image gc manually via the operator API. If it is
> set, the image auto gc is enabled.
> 
> 
> Diffs
> -
> 
>   docs/configuration/agent.md f1e0681be34d194d66e2b2ecd3f76653a54068e2 
>   src/messages/flags.hpp 00e26cd2699a3da9722170324b5c0190850405da 
>   src/slave/flags.hpp 0c02b49c054ef2597298aefe0997beacfc9efeb8 
>   src/slave/flags.cpp 0eeecdc6023443a99c8e8fe29c2f7bf791c0a36e 
> 
> 
> Diff: https://reviews.apache.org/r/64266/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 64265: Added a flag conversion protobuf message 'ImageGcConfig'.

[Qian Zhang]

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


Fix it, then Ship it!





src/messages/flags.proto
Lines 112-114 (patched)


Can we add comments to describe each of the fields?


- Qian Zhang


On Dec. 2, 2017, 9:10 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64265/
> ---
> 
> (Updated Dec. 2, 2017, 9:10 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Qian Zhang, Vinod Kone, and Zhitao Li.
> 
> 
> Bugs: MESOS-8294
> https://issues.apache.org/jira/browse/MESOS-8294
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a flag conversion protobuf message 'ImageGcConfig'.
> 
> 
> Diffs
> -
> 
>   src/messages/flags.proto 7ae9ef82cf9e918cac1eadc9f3ec0534ad4922b2 
> 
> 
> Diff: https://reviews.apache.org/r/64265/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

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

[Gaston Kleiman]

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

(Updated Dec. 1, 2017, 6:46 p.m.)


Review request for mesos and Greg Mann.


Changes
---

Addressed some more comments.


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


Repository: mesos


Description
---

This class will handle the offer operation status updates generated by
the agent and by resource providers.


Diffs (updated)
-

  src/CMakeLists.txt 592489df070a0c4fdee814a4a7b613e62a544f88 
  src/Makefile.am 08d29ab58fe5c7f5e0c9173b504787ec254ed99b 
  src/status_update_manager/offer_operation_status_update_manager.hpp 
PRE-CREATION 
  src/status_update_manager/offer_operation_status_update_manager.cpp 
PRE-CREATION 
  src/tests/CMakeLists.txt 5e5202079c9798b91028e845490b747d4d1d0a66 
  src/tests/offer_operation_status_update_manager_tests.cpp PRE-CREATION 


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

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


Testing
---

This patch addes new tests that passed 5000 times on GNU/Linux.


Thanks,

Gaston Kleiman

Re: Review Request 64095: Added a generic actor to be used by status update managers.

[Gaston Kleiman]

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

(Updated Dec. 1, 2017, 6:44 p.m.)


Review request for mesos and Greg Mann.


Changes
---

Changed some indentation to reduce "jaggedness".


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


Repository: mesos


Description
---

This actor handles the checkpointing, recovery, and retry of status
updates.

It will initially be used by the offer operations status update
manager, but it was designed and implemented so that it can replace
the current implementation of the task status update manager.


Diffs (updated)
-

  src/status_update_manager/status_update_manager_process.hpp PRE-CREATION 


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

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


Testing
---

Added new tests at the end of the chain. They passed 5000 times on GNU/Linux.


Thanks,

Gaston Kleiman

Re: Review Request 63765: Fixed an issue with the scheduler driver subscribe backoff time.

[Meng Zhu]

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

(Updated Dec. 1, 2017, 8:47 p.m.)


Review request for mesos, Gilbert Song and Vinod Kone.


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


Repository: mesos


Description
---

When framework failover time is set to zero (which is the
default value), the scheduler driver subscribe backoff time
will also become zero. Ignore failover time if it is zero
when deciding the subscribe backoff time.
Also added a dedicated test.


Diffs (updated)
-

  src/sched/sched.cpp 6028499285ad092ffd252e842c5d9835dd4442f8 
  src/tests/scheduler_driver_tests.cpp 14d872b8fadfd4ef16d8923fb0df924331534bc3 


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

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


Testing
---

make check, and the new dedicated test.


Thanks,

Meng Zhu

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

[Jan Schlicht]

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

(Updated Dec. 1, 2017, 9:45 a.m.)


Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, and Jie Yu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

A resource provider can resubscribe by including the resource provider
ID it got assigned as part of its 'ResourceProviderInfo' in a
'SUBSCRIBE' call. A resubscription is necessary if either the resource
provider or the resource provider manager (i.e. an agent) failed over.


Diffs (updated)
-

  src/resource_provider/manager.cpp 5fdce7f1777c48029a979f3c77933e1753d6ba00 
  src/tests/mesos.hpp 99542c599e7d0f5027e9358081822d91b526590e 
  src/tests/resource_provider_manager_tests.cpp 
0b7c4ad6bb0052847b884959e3171cd7ab382b45 
  src/tests/slave_tests.cpp 1344e0a5376137b5b933edc9ea4a83be58542654 


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

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


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 60891: Added ACLs and AuthZ for standalone containers.

[Alexander Rojas]

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



This patch didn't add tests in 
[authorization_tests.cpp](https://github.com/apache/mesos/blob/master/src/tests/authorization_tests.cpp)

- Alexander Rojas


On Nov. 14, 2017, 2:24 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60891/
> ---
> 
> (Updated Nov. 14, 2017, 2:24 a.m.)
> 
> 
> Review request for mesos, Alexander Rojas, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: MESOS-7305
> https://issues.apache.org/jira/browse/MESOS-7305
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This defines some coarse-grained AuthZ for launching and managing
> standalone containers.  Each HTTP principal can be given the right
> to Launch, Wait upon, Kill, or Remove standalone containers under
> a given (posix) user.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/acls.proto 
> 587b71489730f9a1252c73c0239e3d9892b3ae8e 
>   include/mesos/authorizer/authorizer.proto 
> 87a805794f430fc8b2e47de6d624b95deef162b4 
>   src/authorizer/local/authorizer.cpp 
> 2fe7b879e649b13322cfcb300c21ef1ed0fea410 
> 
> 
> Diff: https://reviews.apache.org/r/60891/diff/5/
> 
> 
> Testing
> ---
> 
> See later in chain.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 64232: Add a temporary filter for overlay backend related tests.

[Gilbert Song]

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




src/tests/environment.cpp
Lines 599 (patched)


wrong location?


- Gilbert Song


On Nov. 30, 2017, 3:17 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64232/
> ---
> 
> (Updated Nov. 30, 2017, 3:17 p.m.)
> 
> 
> Review request for mesos and Gilbert Song.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is a temporary filter to disable tests that use
> overlay as backend on filesystems where `d_type` support is
> missing. In particular, many XFS nodes are known to have this
> issue.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 832c81fe88d753b0f00dfab870d7725cf556fcef 
>   src/tests/environment.cpp 607ac6cf903c71bdf8e2f4e08581ef2a352db7cd 
> 
> 
> Diff: https://reviews.apache.org/r/64232/diff/1/
> 
> 
> Testing
> ---
> 
> Checked affected tests.
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 64010: Added additional parameters to Allocator::updateSlave().

[Benno Evers]

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

(Updated Dec. 1, 2017, 6:31 p.m.)


Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

The current `SlaveInfo` of a slave is now passed as an additional
parameter to `updateSlave()`, to account for the possibility of
it changing during an agent restart.

While the existing HierarchicalDRFAllocator only cares about the domain
and hostname fields, the full SlaveInfo is passed since custom
allocators might base their scheduling decisions on other fields, for
example attributes.

Additionally, this also provides callers with an option to reset
existing offer filters for a given slave id when the update is
applied.


Diffs (updated)
-

  include/mesos/allocator/allocator.hpp 
acb9e4f6a843e64c915c43c218d8a533ca6b 
  src/master/allocator/mesos/allocator.hpp 
48254b6e0974bdc16ffda04d3d271538048d3206 
  src/master/allocator/mesos/hierarchical.hpp 
3c87dc797cf70f3aa48b1ed9f86d673d4ea2fe76 
  src/master/allocator/mesos/hierarchical.cpp 
ab2abf868f9252154d934243521622c5cb107182 
  src/tests/allocator.hpp fc5d9efc8dab3bd971fa8938e3f82e8291c4ab9d 
  src/tests/hierarchical_allocator_tests.cpp 
0309074bab180be122c9b0074981e6f69c97feee 


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

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


Testing
---


Thanks,

Benno Evers

Re: Review Request 64011: Updated master behaviour to update agent state on reregistration.

[Benno Evers]

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

(Updated Dec. 1, 2017, 6:31 p.m.)


Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

When an agent reregisters, the master will now always update
the agent information it holds in memory, and will write any
changes back to the registry if necessary.


Diffs (updated)
-

  src/master/master.hpp 5d2ae658070d9c5a0bc630c15ff89dc449857f46 
  src/master/master.cpp dfe60ef670edcaefa0c1241df2e2870f650fcf9e 


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

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


Testing
---


Thanks,

Benno Evers

Re: Review Request 64252: Convert resource format of messages entering master.

[Vinod Kone]

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


Fix it, then Ship it!





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


s/format/format./

TODO for moving this stuff (and for task and executor resources) including 
validation into the installer handler.



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


ditto. see above.


- Vinod Kone


On Dec. 1, 2017, 6:30 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64252/
> ---
> 
> (Updated Dec. 1, 2017, 6:30 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Convert resource format of messages entering master.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp dfe60ef670edcaefa0c1241df2e2870f650fcf9e 
>   src/master/registry_operations.cpp 1e1eadb539482113e8ca8cb0542e5aaf34a38b41 
> 
> 
> Diff: https://reviews.apache.org/r/64252/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 64012: Added new --reconfiguration_compatibility slave flag and implementation.

[Vinod Kone]

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




src/tests/master_tests.cpp
Line 2758 (original), 2785 (patched)


re-registration



src/tests/slave_tests.cpp
Lines 8828 (patched)


you should expand the comment on what you are testing here.



src/tests/slave_tests.cpp
Line 8817 (original), 8831 (patched)


Can you start a scheduler in this test and make sure it gets offers with 
new domains and resources after the restart?



src/tests/slave_recovery_tests.cpp
Lines 4774 (patched)


Can you add a test that checks that any attribute changes will result in 
filtered offers being resent?


- Vinod Kone


On Dec. 1, 2017, 6:32 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64012/
> ---
> 
> (Updated Dec. 1, 2017, 6:32 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This flag allows operators to weaken the checks performed by the agent
> when recovering state, in particular it allows to recover running tasks
> even when parts of the recovered SlaveInfo don't match the current
> state.
> 
> The set of possible changes is quite restricted for now,
> to avoid accidenetally introducing semantic bugs.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 15cda101099ce819b72d09bece37ce106b900bb0 
>   src/Makefile.am 3444388e1799a6de774c41c65d273084706ed6ca 
>   src/slave/compatibility.hpp PRE-CREATION 
>   src/slave/compatibility.cpp PRE-CREATION 
>   src/slave/flags.hpp f25d8aff29d0698eaf06dd40a86a35e43bc9e094 
>   src/slave/flags.cpp 34edf4cb4a1afc366e6b6b4ddf78e90e49853fdf 
>   src/slave/slave.hpp 643d8559244f0842e82b293c1ef99cb26e111059 
>   src/slave/slave.cpp 3896e432fe4b40a458e75537ccfcdd689cdf3f43 
>   src/tests/CMakeLists.txt 5e5202079c9798b91028e845490b747d4d1d0a66 
>   src/tests/master_tests.cpp 57eae320a7a398527cd3623c89bf67f319a8e955 
>   src/tests/slave_compatibility_tests.cpp PRE-CREATION 
>   src/tests/slave_recovery_tests.cpp f14c6ef69eb20a03454c8197df79b572a3c6d050 
>   src/tests/slave_tests.cpp 8ab63ac70083f1acaf1d253f9818efd11add6e94 
> 
> 
> Diff: https://reviews.apache.org/r/64012/diff/5/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 64095: Added a generic actor to be used by status update managers.

[Greg Mann]

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




src/status_update_manager/status_update_manager_process.hpp
Lines 588 (patched)


Could we switch to returning an `Owned` here instead of a raw pointer?



src/status_update_manager/status_update_manager_process.hpp
Lines 593-596 (patched)


What are the implications of this for the empty file case? It seems the 
caller must be aware that they should delete an empty file, but I'm not sure 
that they receive specific enough feedback for this.

Perhaps `recover` should remove the file if it's empty?



src/status_update_manager/status_update_manager_process.hpp
Lines 609-610 (patched)


Is there a reason you're not using `O_APPEND` here?



src/status_update_manager/status_update_manager_process.hpp
Lines 650 (patched)


You can probably remove this comment.



src/status_update_manager/status_update_manager_process.hpp
Lines 651 (patched)


s/update stream/updates/



src/status_update_manager/status_update_manager_process.hpp
Lines 654 (patched)


Given the log line above, I think this comment can be removed as well.


- Greg Mann


On Dec. 1, 2017, 1:42 a.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64095/
> ---
> 
> (Updated Dec. 1, 2017, 1:42 a.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Bugs: MESOS-8197
> https://issues.apache.org/jira/browse/MESOS-8197
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This actor handles the checkpointing, recovery, and retry of status
> updates.
> 
> It will initially be used by the offer operations status update
> manager, but it was designed and implemented so that it can replace
> the current implementation of the task status update manager.
> 
> 
> Diffs
> -
> 
>   src/status_update_manager/status_update_manager_process.hpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/64095/diff/3/
> 
> 
> Testing
> ---
> 
> Added new tests at the end of the chain. They passed 5000 times on GNU/Linux.
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Review Request 64255: Removed `os/realpath.hpp` from `posix/os.hpp`.

[Andrew Schwartzmeyer]

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

Review request for mesos and Jie Yu.


Repository: mesos


Description
---

This is part of a clean-up chain to fix the problem where `stout/os.hpp`
previously made `os::realpath()` available, but it was instead moved to
its own header. Instead, files must include what is used, and so include
the `realpath.hpp` header.

Since `posix/os.hpp` doesn't use `os::realpath()`, its inclusion of
`os/realpath.hpp` was removed, and instead code using the function was
fixed to include the header directly.

Note that this now matches `windows/os.hpp`, which also does not include
`realpath.hpp`. The same pattern should be followed for other APIs as
needed.


Diffs
-

  3rdparty/stout/include/stout/posix/os.hpp 
8511dfd419a646df17eb687d732bb975f4c23d53 
  3rdparty/stout/tests/os/filesystem_tests.cpp 
3946885b12a78c765e506c855a12aed1efbeafad 
  3rdparty/stout/tests/os_tests.cpp 26514f3604fcc81effbfcb6845116a62021272ca 


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


Testing
---

See end of chain.


Thanks,

Andrew Schwartzmeyer

Review Request 64257: Included `stout/os/realpath.hpp` where `os::realpath()` is used.

[Andrew Schwartzmeyer]

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

Review request for mesos and Jie Yu.


Repository: mesos


Description
---

Included `stout/os/realpath.hpp` where `os::realpath()` is used.


Diffs
-

  src/cli/mesos.cpp b347f395712da15729563e23b4a05c26434895ac 
  src/csi/paths.cpp 6caefc5af8ed94a9d4ea4a325e94b71883ac622d 
  src/examples/balloon_framework.cpp 0136344c160ea6aeacc57e61382640689a77ee1a 
  src/examples/long_lived_framework.cpp 
e6742159108f9eae4f2548bb84d6495309737d9c 
  src/examples/test_framework.cpp c6a293ed5f88bf4d73f23d97cbf9ffdd8b941f1a 
  src/examples/test_http_framework.cpp 373035837f51e818629b1588d2ab8803c656f527 
  src/files/files.cpp fbff2078312ea31bb120f8ba7584b53b4a607609 
  src/linux/cgroups.cpp 0a7bb89567d4b3ecee7c4f9a259368fb4eae7f34 
  src/linux/systemd.cpp 6318f48fc3c56d2414a8a50fd2d9debe74fddeb9 
  src/slave/containerizer/fetcher.cpp 8c4b7e6baefba116cd63c2275c90793a31ae12ec 
  src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
134ae5d545ba9fa49f47e3f1afbe6cce3580c1c8 
  src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
f40ded73824d42117951d05067ec89131f4e9b34 
  src/slave/containerizer/mesos/isolators/filesystem/posix.cpp 
270d2aa6e06f323bfb6eee3b703a24a600a55871 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
1c207009b9d718093eae03b411813104ea34a9d7 
  src/slave/containerizer/mesos/isolators/volume/host_path.cpp 
409fc148dcb89dfec18d77c5381ad547337eec8a 
  src/slave/containerizer/mesos/launch.cpp 
b1065be0d12934e9b10feadb7d1b82af8ad7cebc 
  src/slave/containerizer/mesos/provisioner/appc/store.cpp 
9e65990c71e991247c35a9b84b69274e113fad6a 
  src/slave/containerizer/mesos/provisioner/backends/overlay.cpp 
3149220316b901c0986d8546504d8ee78bec7181 
  src/slave/containerizer/mesos/provisioner/provisioner.cpp 
81d2f93673e890f135e87ad86f30dfa16ea683ae 
  src/slave/slave.cpp 3896e432fe4b40a458e75537ccfcdd689cdf3f43 
  src/slave/state.cpp d49c38eada220c379f88acadb3522b1a112ab051 
  src/tests/containerizer/provisioner_appc_tests.cpp 
9c4b685654acbdece730d1663aa8d54706763a21 
  src/tests/containerizer/rootfs.cpp fe9252dff859c2b08f609140ead2ef123844fbed 
  src/tests/gc_tests.cpp 6f055b5eb2a201401ca7dde93275b2d28a886a3c 
  src/tests/script.cpp 8d40e01da005cb05e7804f0b3975e3e0edb8f3bd 


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


Testing
---

See end of chain.

This is the `mesos` portion.


Thanks,

Andrew Schwartzmeyer

Review Request 64256: Included `stout/os/realpath.hpp` where `os::realpath()` is used.

[Andrew Schwartzmeyer]

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

Review request for mesos and Jie Yu.


Repository: mesos


Description
---

Included `stout/os/realpath.hpp` where `os::realpath()` is used.


Diffs
-

  3rdparty/libprocess/include/process/ssl/gtest.hpp 
b368eda1edd588475f4f4838a26ba211a0850b9a 


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


Testing
---

See end of chain.

This is the `libprocess` portion.


Thanks,

Andrew Schwartzmeyer

Review Request 64258: Included `stout/os/permissions.hpp` for the secret generator.

[Andrew Schwartzmeyer]

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

Review request for mesos and Jie Yu.


Repository: mesos


Description
---

The `os::Permissions` started being used in these files in `e1207f9d`,
but not included. On Windows, no other header included it either, and so
the SSL configuration of the build broke.


Diffs
-

  src/slave/main.cpp abe48fdf56ba356d757d8c6e97d01ae3b6cfa0a3 
  src/tests/cluster.cpp b530133fa77572f00ef8a945d3466a27eb95ceab 


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


Testing
---

`make check` on CentOS 7

Built and ran tests on Windows with `-DENABLE_SSL=ON`. Verified 
`libprocess-tests` SSLTest suite passed.

Note that without this chain, the CI builds are currently broken.


Thanks,

Andrew Schwartzmeyer

Review Request 64252: Convert resource format of messages entering master.

[Benno Evers]

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

Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

Convert resource format of messages entering master.


Diffs
-

  src/master/master.cpp dfe60ef670edcaefa0c1241df2e2870f650fcf9e 
  src/master/registry_operations.cpp 1e1eadb539482113e8ca8cb0542e5aaf34a38b41 


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


Testing
---


Thanks,

Benno Evers

Re: Review Request 64012: Added new --reconfiguration_compatibility slave flag and implementation.

[Benno Evers]

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

(Updated Dec. 1, 2017, 6:32 p.m.)


Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

This flag allows operators to weaken the checks performed by the agent
when recovering state, in particular it allows to recover running tasks
even when parts of the recovered SlaveInfo don't match the current
state.

The set of possible changes is quite restricted for now,
to avoid accidenetally introducing semantic bugs.


Diffs (updated)
-

  src/CMakeLists.txt 15cda101099ce819b72d09bece37ce106b900bb0 
  src/Makefile.am 3444388e1799a6de774c41c65d273084706ed6ca 
  src/slave/compatibility.hpp PRE-CREATION 
  src/slave/compatibility.cpp PRE-CREATION 
  src/slave/flags.hpp f25d8aff29d0698eaf06dd40a86a35e43bc9e094 
  src/slave/flags.cpp 34edf4cb4a1afc366e6b6b4ddf78e90e49853fdf 
  src/slave/slave.hpp 643d8559244f0842e82b293c1ef99cb26e111059 
  src/slave/slave.cpp 3896e432fe4b40a458e75537ccfcdd689cdf3f43 
  src/tests/CMakeLists.txt 5e5202079c9798b91028e845490b747d4d1d0a66 
  src/tests/master_tests.cpp 57eae320a7a398527cd3623c89bf67f319a8e955 
  src/tests/slave_compatibility_tests.cpp PRE-CREATION 
  src/tests/slave_recovery_tests.cpp f14c6ef69eb20a03454c8197df79b572a3c6d050 
  src/tests/slave_tests.cpp 8ab63ac70083f1acaf1d253f9818efd11add6e94 


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

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


Testing
---


Thanks,

Benno Evers

Re: Review Request 64008: Activated AGENT_UPDATE master capability.

[Benno Evers]

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

(Updated Dec. 1, 2017, 6:32 p.m.)


Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

Activated AGENT_UPDATE master capability.


Diffs (updated)
-

  src/master/constants.cpp 55eecfbe5e48515767eadece54c8d0488dfaa830 
  src/tests/master_tests.cpp 57eae320a7a398527cd3623c89bf67f319a8e955 


Diff: https://reviews.apache.org/r/64008/diff/6/

Changes: https://reviews.apache.org/r/64008/diff/5-6/


Testing
---


Thanks,

Benno Evers

Re: Review Request 64009: Added new UpdateSlave registry operation.

[Benno Evers]

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

(Updated Dec. 1, 2017, 6:37 p.m.)


Review request for mesos, Benjamin Mahler, Michael Park, and Vinod Kone.


Repository: mesos


Description
---

This operation can be used to update the stored state
of an existing, admitted slave.


Diffs (updated)
-

  src/master/registry_operations.hpp 5b78306fb2e2bd7e30200f112f0381a595c7b25d 
  src/master/registry_operations.cpp 1e1eadb539482113e8ca8cb0542e5aaf34a38b41 
  src/tests/registrar_tests.cpp 21d78e90e57c145aa9f4b26f1a2f0278390b98d4 


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

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


Testing
---


Thanks,

Benno Evers

Re: Review Request 64009: Added new UpdateSlave registry operation.

[Vinod Kone]

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


Fix it, then Ship it!





src/master/registry_operations.cpp
Line 64 (original), 70 (patched)


Worth adding a comment here on why you are doing this.


- Vinod Kone


On Dec. 1, 2017, 6:37 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64009/
> ---
> 
> (Updated Dec. 1, 2017, 6:37 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Michael Park, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This operation can be used to update the stored state
> of an existing, admitted slave.
> 
> 
> Diffs
> -
> 
>   src/master/registry_operations.hpp 5b78306fb2e2bd7e30200f112f0381a595c7b25d 
>   src/master/registry_operations.cpp 1e1eadb539482113e8ca8cb0542e5aaf34a38b41 
>   src/tests/registrar_tests.cpp 21d78e90e57c145aa9f4b26f1a2f0278390b98d4 
> 
> 
> Diff: https://reviews.apache.org/r/64009/diff/5/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 64010: Added additional parameters to Allocator::updateSlave().

[Vinod Kone]

> On Dec. 1, 2017, 12:24 p.m., Benjamin Bannier wrote:
> > include/mesos/allocator/allocator.hpp
> > Lines 211-212 (patched)
> > 
> >
> > This doesn't look what was in the original design where a framework 
> > could receive an update for agent updates and remove offer filters itself 
> > (we already independently plan to extend the scheduler interfaces to allow 
> > reviving a single agent for offer operation feedback).
> > 
> > I feel having a master remove unexpired offer filters by itself 
> > violates responsibilities. It would seem much more straight-forward to 
> > reason about lifecycles would the framework which created the filter also 
> > be the sole on responsible for cleaning it up before expiry. The originally 
> > designed agent update channel to the framework would permit this, and I 
> > hope this is what we still plan to implement.
> > 
> > This makes me wonder whether we should we mark this parameter as 
> > transitional if we cannot get rid of it completely now. We are working on a 
> > public interface here so this is not ideal either.

We don't have an agent update message yet. That's a bigger project that needs 
to be done.

For now, instead of changing the interface, we will remove the filters inside 
the allocator itself on attribute changes only.


- Vinod


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


On Dec. 1, 2017, 6:31 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64010/
> ---
> 
> (Updated Dec. 1, 2017, 6:31 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The current `SlaveInfo` of a slave is now passed as an additional
> parameter to `updateSlave()`, to account for the possibility of
> it changing during an agent restart.
> 
> While the existing HierarchicalDRFAllocator only cares about the domain
> and hostname fields, the full SlaveInfo is passed since custom
> allocators might base their scheduling decisions on other fields, for
> example attributes.
> 
> Additionally, this also provides callers with an option to reset
> existing offer filters for a given slave id when the update is
> applied.
> 
> 
> Diffs
> -
> 
>   include/mesos/allocator/allocator.hpp 
> acb9e4f6a843e64c915c43c218d8a533ca6b 
>   src/master/allocator/mesos/allocator.hpp 
> 48254b6e0974bdc16ffda04d3d271538048d3206 
>   src/master/allocator/mesos/hierarchical.hpp 
> 3c87dc797cf70f3aa48b1ed9f86d673d4ea2fe76 
>   src/master/allocator/mesos/hierarchical.cpp 
> ab2abf868f9252154d934243521622c5cb107182 
>   src/tests/allocator.hpp fc5d9efc8dab3bd971fa8938e3f82e8291c4ab9d 
>   src/tests/hierarchical_allocator_tests.cpp 
> 0309074bab180be122c9b0074981e6f69c97feee 
> 
> 
> Diff: https://reviews.apache.org/r/64010/diff/5/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 64232: Add a temporary filter for overlay backend related tests.

[Mesos Reviewbot Windows]

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



FAIL: Mesos libprocess-tests failed to build

Reviews applied: `['64232']`

Failed command: `cmake.exe --build . --target libprocess-tests --config Debug`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64232

Relevant logs:

- 
[libprocess-tests-build-cmake-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64232/logs/libprocess-tests-build-cmake-stdout.log):

```
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\state_machine_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\state_machine_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(805): warning 
C4244: 'argument': conversion from 'ULONG_PTR' to 'pid_t', possible loss of 
data (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\state_machine_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\subprocess_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\subprocess_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\system_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\system_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(805): warning 
C4244: 'argument': conversion from 'ULONG_PTR' to 'pid_t', possible loss of 
data (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\subprocess_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(805): warning 
C4244: 'argument': conversion from 'ULONG_PTR' to 'pid_t', possible loss of 
data (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\system_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\time_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\time_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(805): warning 
C4244: 'argument': conversion from 'ULONG_PTR' to 'pid_t', possible loss of 
data (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\time_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\ssl_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\ssl_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]

Re: Review Request 63021: Added `getService()` function to launch CSI plugins.

[Jie Yu]

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


Fix it, then Ship it!





src/common/type_utils.cpp
Lines 746 (patched)


Please udpate v1



src/resource_provider/storage/provider.cpp
Lines 154 (patched)


Can we change `getContainerIdPrefix` to not have the tailing `-`?

So here, you basically construct three things:
1) prefix
2) csi plugin type/name
3) services

and join them by `--`



src/resource_provider/storage/provider.cpp
Lines 508-511 (patched)


Instead of nesting, i'd prefer flat them out:

```
return client
  .then(defer(..., [=](const csi::Client& client) {
return client.GetSupportedVersion(...);
  })
  .then(defer(..., [=](const ...) {
...
  });
```

Any reason not using `const ref` for the parameter?



src/resource_provider/storage/provider.cpp
Lines 591 (patched)


You need to return after calling `fatal`


- Jie Yu


On Dec. 1, 2017, 1:36 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63021/
> ---
> 
> (Updated Dec. 1, 2017, 1:36 a.m.)
> 
> 
> Review request for mesos, James DeFelice, Jie Yu, Joseph Wu, and Jan Schlicht.
> 
> 
> Bugs: MESOS-8032
> https://issues.apache.org/jira/browse/MESOS-8032
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `getService()` method first checks if there is already a container
> daemon for the specified plugin component, and creates a new one if not.
> The post-start hook for the container daemon will call `connect()` to
> wait for the endpoint socket file to appear and connect to it, then
> set up the corresponding promise of CSI client. The post-stop hook will
> remove the socket file and create a new promise for the next call to the
> post-start hook to set it up.
> 
> 
> Diffs
> -
> 
>   include/mesos/type_utils.hpp a348c7df083324602d25f40069ad49e29f7918a5 
>   src/common/type_utils.cpp 3657d55ab2a052b5c659022e3fc264e748429d54 
>   src/resource_provider/storage/provider.cpp 
> f586afc256fbcb2f2bf2451ffb7e7ba1d59cfa78 
> 
> 
> Diff: https://reviews.apache.org/r/63021/diff/14/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 64010: Added additional parameters to Allocator::updateSlave().

[Vinod Kone]

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



Lets split this review into 2

1) `updateSlave()` should take SlaveInfo and the allocator should use hostname 
and domain from SlaveInfo instead of storing it separately.
2) Rest of the changes in this review

- Vinod Kone


On Dec. 1, 2017, 6:31 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64010/
> ---
> 
> (Updated Dec. 1, 2017, 6:31 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The current `SlaveInfo` of a slave is now passed as an additional
> parameter to `updateSlave()`, to account for the possibility of
> it changing during an agent restart.
> 
> While the existing HierarchicalDRFAllocator only cares about the domain
> and hostname fields, the full SlaveInfo is passed since custom
> allocators might base their scheduling decisions on other fields, for
> example attributes.
> 
> Additionally, this also provides callers with an option to reset
> existing offer filters for a given slave id when the update is
> applied.
> 
> 
> Diffs
> -
> 
>   include/mesos/allocator/allocator.hpp 
> acb9e4f6a843e64c915c43c218d8a533ca6b 
>   src/master/allocator/mesos/allocator.hpp 
> 48254b6e0974bdc16ffda04d3d271538048d3206 
>   src/master/allocator/mesos/hierarchical.hpp 
> 3c87dc797cf70f3aa48b1ed9f86d673d4ea2fe76 
>   src/master/allocator/mesos/hierarchical.cpp 
> ab2abf868f9252154d934243521622c5cb107182 
>   src/tests/allocator.hpp fc5d9efc8dab3bd971fa8938e3f82e8291c4ab9d 
>   src/tests/hierarchical_allocator_tests.cpp 
> 0309074bab180be122c9b0074981e6f69c97feee 
> 
> 
> Diff: https://reviews.apache.org/r/64010/diff/5/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 64011: Updated master behaviour to update agent state on reregistration.

[Vinod Kone]

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




src/master/master.cpp
Lines 6923-6927 (original), 6894-6898 (patched)


this comment might be stale after your changes in the previous review.

change updateSlave return value to `Try`



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


s/reregistering/re-registering/


- Vinod Kone


On Dec. 1, 2017, 6:31 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64011/
> ---
> 
> (Updated Dec. 1, 2017, 6:31 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When an agent reregisters, the master will now always update
> the agent information it holds in memory, and will write any
> changes back to the registry if necessary.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 5d2ae658070d9c5a0bc630c15ff89dc449857f46 
>   src/master/master.cpp dfe60ef670edcaefa0c1241df2e2870f650fcf9e 
> 
> 
> Diff: https://reviews.apache.org/r/64011/diff/5/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 64008: Activated AGENT_UPDATE master capability.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Dec. 1, 2017, 6:32 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64008/
> ---
> 
> (Updated Dec. 1, 2017, 6:32 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Activated AGENT_UPDATE master capability.
> 
> 
> Diffs
> -
> 
>   src/master/constants.cpp 55eecfbe5e48515767eadece54c8d0488dfaa830 
>   src/tests/master_tests.cpp 57eae320a7a398527cd3623c89bf67f319a8e955 
> 
> 
> Diff: https://reviews.apache.org/r/64008/diff/6/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 64250: Added new reasons task status update.

[Jiang Yan Xu]

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




docs/task-state-reasons.md
Lines 370-372 (patched)


Should it be in the "Reasons for Non-Terminal Status Updates" section and 
is there a reason to refer to TASK_LOST?



docs/task-state-reasons.md
Lines 479 (patched)


"The task was part of an accepted offer": this is irrevelent to either 
TASK_UNREACHABLE or the reason REASON_SLAVE_REREGISTERED.



docs/task-state-reasons.md
Lines 480-482 (patched)


We don't need to put too much details on TASK_UNREACHABLE since it's 
described elsewhere.



docs/task-state-reasons.md
Lines 483-485 (patched)


I think we can just focus on:

- This is sent when an agent previously marked as unreachable re-registers.
- Status updates with this reason are not the original ones, see the 
comments for `REASON_RECONCILIATION`.



include/mesos/mesos.proto
Lines 2406 (patched)


I think has convention used here is to order the reasons alphabetically.



include/mesos/v1/mesos.proto
Lines 2387 (patched)


Ditto.


- Jiang Yan Xu


On Dec. 1, 2017, 8:20 a.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64250/
> ---
> 
> (Updated Dec. 1, 2017, 8:20 a.m.)
> 
> 
> Review request for mesos, Ilya Pronin, James Peach, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added new reasons `REASON_SLAVE_REREGISTERED` and
> `REASON_AGENT_REREGISTERED` for v0 and v1 task status update.
> The new reason will be used when master starts to send status
> update during the re-registeration of an agent which was
> previosuly removed by the master because of being unreachable.
> 
> 
> Diffs
> -
> 
>   docs/task-state-reasons.md 07f7927e28fe5624a6c9b74de731483aaa729ca3 
>   include/mesos/mesos.proto 11089b7d6f48f001ae64e8b0c67a2732684aa2fa 
>   include/mesos/v1/mesos.proto c496da927903f472c78d455b7fdf886d8fb692c4 
> 
> 
> Diff: https://reviews.apache.org/r/64250/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Megha Sharma
> 
>

Re: Review Request 63021: Added `getService()` function to launch CSI plugins.

[Jie Yu]

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




src/resource_provider/storage/provider.cpp
Lines 173 (patched)


I'd rename this to `getCSIPluginContainerInfo`


- Jie Yu


On Dec. 1, 2017, 1:36 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63021/
> ---
> 
> (Updated Dec. 1, 2017, 1:36 a.m.)
> 
> 
> Review request for mesos, James DeFelice, Jie Yu, Joseph Wu, and Jan Schlicht.
> 
> 
> Bugs: MESOS-8032
> https://issues.apache.org/jira/browse/MESOS-8032
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `getService()` method first checks if there is already a container
> daemon for the specified plugin component, and creates a new one if not.
> The post-start hook for the container daemon will call `connect()` to
> wait for the endpoint socket file to appear and connect to it, then
> set up the corresponding promise of CSI client. The post-stop hook will
> remove the socket file and create a new promise for the next call to the
> post-start hook to set it up.
> 
> 
> Diffs
> -
> 
>   include/mesos/type_utils.hpp a348c7df083324602d25f40069ad49e29f7918a5 
>   src/common/type_utils.cpp 3657d55ab2a052b5c659022e3fc264e748429d54 
>   src/resource_provider/storage/provider.cpp 
> f586afc256fbcb2f2bf2451ffb7e7ba1d59cfa78 
> 
> 
> Diff: https://reviews.apache.org/r/63021/diff/14/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 63632: Migrated to event consumer interface.

[Michael Park]

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


Fix it, then Ship it!





src/master/master.hpp
Line 548 (original), 548 (patched)


Can we remove the `virtual`s here similar to the libprocess patch?



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


I think the `TODO` is sufficient as breadcrumb
for us to know that we need to change code here.

No need to make an extra copy of `MessageEvent`,
here and below.


- Michael Park


On Nov. 7, 2017, 8:51 a.m., Dmitry Zhuk wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63632/
> ---
> 
> (Updated Nov. 7, 2017, 8:51 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Michael Park.
> 
> 
> Bugs: MESOS-6972
> https://issues.apache.org/jira/browse/MESOS-6972
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Migrated to event consumer interface.
> 
> 
> Diffs
> -
> 
>   src/common/recordio.hpp 378363492e04c141671fbed0467bf558b61e5dae 
>   src/master/master.hpp 0c1253aa1a88d113d42ce362d6992ad84217728c 
>   src/master/master.cpp ee212c1a21b432d3aa8e90d748d12cb0a754d57c 
>   src/tests/fetcher_cache_tests.cpp e2ecb102e2e7b4c4b3b2b736fdc453feeb15816e 
> 
> 
> Diff: https://reviews.apache.org/r/63632/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Dmitry Zhuk
> 
>

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

[Mesos Reviewbot Windows]

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



FAIL: Mesos libprocess-tests failed to build

Reviews applied: `['64250', '64098']`

Failed command: `cmake.exe --build . --target libprocess-tests --config Debug`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64098

Relevant logs:

- 
[libprocess-tests-build-cmake-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64098/logs/libprocess-tests-build-cmake-stdout.log):

```
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\state_machine_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\state_machine_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(805): warning 
C4244: 'argument': conversion from 'ULONG_PTR' to 'pid_t', possible loss of 
data (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\state_machine_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\subprocess_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\subprocess_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\system_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\system_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(805): warning 
C4244: 'argument': conversion from 'ULONG_PTR' to 'pid_t', possible loss of 
data (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\system_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(805): warning 
C4244: 'argument': conversion from 'ULONG_PTR' to 'pid_t', possible loss of 
data (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\subprocess_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\time_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\time_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(805): warning 
C4244: 'argument': conversion from 'ULONG_PTR' to 'pid_t', possible loss of 
data (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\time_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\ssl_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\ssl_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]

Re: Review Request 64232: Add a temporary filter for overlay backend related tests.

[Meng Zhu]

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

(Updated Dec. 1, 2017, 1:01 p.m.)


Review request for mesos and Gilbert Song.


Repository: mesos


Description
---

This is a temporary filter to disable tests that use
overlay as backend on filesystems where `d_type` support is
missing. In particular, many XFS nodes are known to have this
issue.


Diffs (updated)
-

  src/tests/containerizer/provisioner_docker_tests.cpp 
832c81fe88d753b0f00dfab870d7725cf556fcef 
  src/tests/environment.cpp 607ac6cf903c71bdf8e2f4e08581ef2a352db7cd 


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

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


Testing
---

Checked affected tests.


Thanks,

Meng Zhu

Re: Review Request 64250: Added new reasons task status update.

[Megha Sharma]

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

(Updated Dec. 1, 2017, 9:06 p.m.)


Review request for mesos, Ilya Pronin, James Peach, and Jiang Yan Xu.


Changes
---

done testing


Repository: mesos


Description
---

Added new reasons `REASON_SLAVE_REREGISTERED` and
`REASON_AGENT_REREGISTERED` for v0 and v1 task status update.
The new reason will be used when master starts to send status
update during the re-registeration of an agent which was
previosuly removed by the master because of being unreachable.


Diffs
-

  docs/task-state-reasons.md 07f7927e28fe5624a6c9b74de731483aaa729ca3 
  include/mesos/mesos.proto 11089b7d6f48f001ae64e8b0c67a2732684aa2fa 
  include/mesos/v1/mesos.proto c496da927903f472c78d455b7fdf886d8fb692c4 


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


Testing (updated)
---

with make check


Thanks,

Megha Sharma

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

[Benjamin Bannier]

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


Fix it, then Ship it!




Thanks Jan! Will fix minor style nits while committing.


src/tests/mesos.hpp
Lines 2794 (patched)


nit: Let's put these on their own lines.



src/tests/mesos.hpp
Line 2962 (original), 2968 (patched)


Let's add an assertion here that `info.has_id()`.



src/tests/slave_tests.cpp
Lines 8987-8988 (original), 8994-8995 (patched)


nit: Fits on a single line.



src/tests/slave_tests.cpp
Lines 8999-9000 (original), 9006-9007 (patched)


nit: Fits on a single line.


- Benjamin Bannier


On Dec. 1, 2017, 9:45 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64065/
> ---
> 
> (Updated Dec. 1, 2017, 9:45 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, and Jie Yu.
> 
> 
> Bugs: MESOS-8269
> https://issues.apache.org/jira/browse/MESOS-8269
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A resource provider can resubscribe by including the resource provider
> ID it got assigned as part of its 'ResourceProviderInfo' in a
> 'SUBSCRIBE' call. A resubscription is necessary if either the resource
> provider or the resource provider manager (i.e. an agent) failed over.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/manager.cpp 5fdce7f1777c48029a979f3c77933e1753d6ba00 
>   src/tests/mesos.hpp 99542c599e7d0f5027e9358081822d91b526590e 
>   src/tests/resource_provider_manager_tests.cpp 
> 0b7c4ad6bb0052847b884959e3171cd7ab382b45 
>   src/tests/slave_tests.cpp 1344e0a5376137b5b933edc9ea4a83be58542654 
> 
> 
> Diff: https://reviews.apache.org/r/64065/diff/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 64238: Updated the allocator to track allocations via a single code path.

[Mesos Reviewbot Windows]

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



FAIL: Mesos libprocess-tests failed to build

Reviews applied: `['64236', '64237', '64238']`

Failed command: `cmake.exe --build . --target libprocess-tests --config Debug`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64238

Relevant logs:

- 
[libprocess-tests-build-cmake-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64238/logs/libprocess-tests-build-cmake-stdout.log):

```
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\state_machine_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\state_machine_tests.cpp)system_tests.cpp
 [C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(805): warning 
C4244: 'argument': conversion from 'ULONG_PTR' to 'pid_t', possible loss of 
data (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\state_machine_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\subprocess_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\subprocess_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\system_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\system_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(805): warning 
C4244: 'argument': conversion from 'ULONG_PTR' to 'pid_t', possible loss of 
data (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\system_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(805): warning 
C4244: 'argument': conversion from 'ULONG_PTR' to 'pid_t', possible loss of 
data (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\subprocess_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\time_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\time_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(805): warning 
C4244: 'argument': conversion from 'ULONG_PTR' to 'pid_t', possible loss of 
data (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\time_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\ssl_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\ssl_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]

Re: Review Request 63862: Windows: Ported docker_tests.cpp.

[Mesos Reviewbot Windows]

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



FAIL: Mesos libprocess-tests failed to build

Reviews applied: `['63859', '63860', '63861', '63862']`

Failed command: `cmake.exe --build . --target libprocess-tests --config Debug`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/63862

Relevant logs:

- 
[libprocess-tests-build-cmake-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/63862/logs/libprocess-tests-build-cmake-stdout.log):

```
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\state_machine_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\state_machine_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(805): warning 
C4244: 'argument': conversion from 'ULONG_PTR' to 'pid_t', possible loss of 
data (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\state_machine_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\subprocess_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\subprocess_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\system_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\system_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(805): warning 
C4244: 'argument': conversion from 'ULONG_PTR' to 'pid_t', possible loss of 
data (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\subprocess_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(805): warning 
C4244: 'argument': conversion from 'ULONG_PTR' to 'pid_t', possible loss of 
data (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\system_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\time_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\time_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(805): warning 
C4244: 'argument': conversion from 'ULONG_PTR' to 'pid_t', possible loss of 
data (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\time_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\ssl_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\ssl_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]

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

[Mesos Reviewbot Windows]

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



FAIL: Mesos libprocess-tests failed to build

Failed command: `cmake.exe --build . --target libprocess-tests --config Debug`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64065

Relevant logs:

- 
[libprocess-tests-build-cmake-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64065/logs/libprocess-tests-build-cmake-stdout.log):

```
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\state_machine_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\state_machine_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(805): warning 
C4244: 'argument': conversion from 'ULONG_PTR' to 'pid_t', possible loss of 
data (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\state_machine_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\subprocess_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\subprocess_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\system_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\system_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(805): warning 
C4244: 'argument': conversion from 'ULONG_PTR' to 'pid_t', possible loss of 
data (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\system_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(805): warning 
C4244: 'argument': conversion from 'ULONG_PTR' to 'pid_t', possible loss of 
data (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\subprocess_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\time_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\time_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(805): warning 
C4244: 'argument': conversion from 'ULONG_PTR' to 'pid_t', possible loss of 
data (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\time_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\ssl_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\ssl_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]


"C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj" (default 
target) (1) ->

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

[Mesos Reviewbot Windows]

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



FAIL: Mesos libprocess-tests failed to build

Reviews applied: `['64093', '64094', '64095', '64096']`

Failed command: `cmake.exe --build . --target libprocess-tests --config Debug`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64096

Relevant logs:

- 
[libprocess-tests-build-cmake-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64096/logs/libprocess-tests-build-cmake-stdout.log):

```
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\state_machine_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\state_machine_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(805): warning 
C4244: 'argument': conversion from 'ULONG_PTR' to 'pid_t', possible loss of 
data (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\state_machine_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\subprocess_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\subprocess_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\system_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\system_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(805): warning 
C4244: 'argument': conversion from 'ULONG_PTR' to 'pid_t', possible loss of 
data (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\subprocess_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(805): warning 
C4244: 'argument': conversion from 'ULONG_PTR' to 'pid_t', possible loss of 
data (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\system_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\time_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\time_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(805): warning 
C4244: 'argument': conversion from 'ULONG_PTR' to 'pid_t', possible loss of 
data (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\time_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\ssl_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\ssl_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]

Re: Review Request 64104: Added dependency of curl to agent. Enabled most health check tests.

[Mesos Reviewbot Windows]

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



FAIL: Failed to apply the current review.

Failed command: `python.exe .\support\apply-reviews.py -n -r 64104`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64104

Relevant logs:

- 
[apply-review-64104-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64104/logs/apply-review-64104-stdout.log):

```
error: patch failed: src/tests/health_check_tests.cpp:1944
error: src/tests/health_check_tests.cpp: patch does not apply
```

- Mesos Reviewbot Windows


On Nov. 30, 2017, 12:18 p.m., John Kordich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64104/
> ---
> 
> (Updated Nov. 30, 2017, 12:18 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Jeff Coffler, 
> and Joseph Wu.
> 
> 
> Bugs: MESOS-6709
> https://issues.apache.org/jira/browse/MESOS-6709
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added dependency of curl to agent. Enabled most health check tests.
> 
> 
> Diffs
> -
> 
>   src/slave/CMakeLists.txt 6f08f3dc95f3a1408cbef7b8f0b0cc2522558924 
>   src/tests/health_check_tests.cpp c0dcba265363f2149b217b189ee5a8e925e40ea1 
> 
> 
> Diff: https://reviews.apache.org/r/64104/diff/1/
> 
> 
> Testing
> ---
> 
> There are two health check tests which are not enabled yet, because they 
> require the IOSwitchboard to be ported first. We can't resolve MESOS-6709 
> quite yet until that is addressed. The rest of the tests pass on both 
> Linux/Windows, as mentioned in #64102
> 
> 
> Thanks,
> 
> John Kordich
> 
>

Re: Review Request 64195: Updated the tests to use MULTI_ROLE frameworks by default.

[Mesos Reviewbot Windows]

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



FAIL: Failed to apply the current review.

Failed command: `python.exe .\support\apply-reviews.py -n -r 64195`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64195

Relevant logs:

- 
[apply-review-64195-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64195/logs/apply-review-64195-stdout.log):

```
error: patch failed: src/tests/slave_tests.cpp:850
error: src/tests/slave_tests.cpp: patch does not apply
```

- Mesos Reviewbot Windows


On Nov. 29, 2017, 8:45 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64195/
> ---
> 
> (Updated Nov. 29, 2017, 8:45 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-8237
> https://issues.apache.org/jira/browse/MESOS-8237
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Now that we strip the `Resource.allocation_info` for non-MULTI_ROLE
> schedulers, it's simpler to default the tests to use the MULTI_ROLE
> capability, since we've already updated the majority of the tests
> to be aware of `Resource.allocation_info`.
> 
> 
> Diffs
> -
> 
>   src/examples/balloon_framework.cpp 0136344c160ea6aeacc57e61382640689a77ee1a 
>   src/examples/disk_full_framework.cpp 
> 28f71c35b32b46c7bf2cce9fe971266f7bd73c8a 
>   src/examples/dynamic_reservation_framework.cpp 
> 5ee3867228a70c8fa0b4b5092fb44cb556a670e0 
>   src/examples/long_lived_framework.cpp 
> e6742159108f9eae4f2548bb84d6495309737d9c 
>   src/examples/no_executor_framework.cpp 
> fd920f58f5ccb903127e9c6b112be30f04baa069 
>   src/examples/persistent_volume_framework.cpp 
> 674d58a9e6fa34733f765d2925cedafeaab1eecd 
>   src/examples/test_framework.cpp c6a293ed5f88bf4d73f23d97cbf9ffdd8b941f1a 
>   src/examples/test_http_framework.cpp 
> 373035837f51e818629b1588d2ab8803c656f527 
>   src/tests/api_tests.cpp 66cb059c96ff9ecda594e13de9e5dc3909f3 
>   src/tests/containerizer/cgroups_isolator_tests.cpp 
> 421a72f1694a1a5c5e1bf4c525361dbd670c68af 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 7a42bb93508abf9020d0bcc5b3daa2f6834be2e3 
>   src/tests/containerizer/linux_filesystem_isolator_tests.cpp 
> 655f9f81f4bc55a85d7c5e27609a016254793380 
>   src/tests/default_executor_tests.cpp 
> 04d9e1b9d4682c3bb5355253e3a8e5b06d78e98f 
>   src/tests/disk_quota_tests.cpp fc297995c42821aa4b0a7719a6a19c3bbf65db47 
>   src/tests/fault_tolerance_tests.cpp 
> 33a2220b4b4f019799db7b2482ce73b3454fa58e 
>   src/tests/hook_tests.cpp 2e58d1133343a96b67d0816a54ff8eb874522ba3 
>   src/tests/master_allocator_tests.cpp 
> 3400d70bb0ba564eac43c4639eee0efd4d8059e6 
>   src/tests/master_authorization_tests.cpp 
> eff97f155e7472a0cd5994408ed73474392593ad 
>   src/tests/master_quota_tests.cpp 058f6d24da50cbf3c28b091afa88f634a8102b62 
>   src/tests/master_tests.cpp 01f45a9bb6378c5fded31bf58fbcad93fe7ed719 
>   src/tests/master_validation_tests.cpp 
> 0e1c8b490ebe10bc50b8b6b530fa85128b967584 
>   src/tests/mesos.hpp f02c7c6962c9d0bee57712d8aad9997582c7404b 
>   src/tests/oversubscription_tests.cpp 
> 2f403d3c9d2df7d5425b335781e8b62153854ef3 
>   src/tests/partition_tests.cpp e49c474167076b4136a161ed29b11db9a13455a7 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 883192d2badf8af87c95bc9a5fc3e711cd240193 
>   src/tests/persistent_volume_tests.cpp 
> b11d26e45c184cf3f0623629b898f67dc95c4fa0 
>   src/tests/reservation_endpoints_tests.cpp 
> a96cc714fd40421a7db9a4c0debf71546daf5d4c 
>   src/tests/reservation_tests.cpp fa375cd45e4c68551801521176632a56acd6d0ea 
>   src/tests/resource_provider_manager_tests.cpp 
> 0b7c4ad6bb0052847b884959e3171cd7ab382b45 
>   src/tests/role_tests.cpp 084555ae70b80330cdc8729cd62f8baddf291a01 
>   src/tests/scheduler_http_api_tests.cpp 
> 80e52fb22dbba4a0703f1b192205acaff89f883b 
>   src/tests/scheduler_tests.cpp 45fc9c0cfccdb22c2e3e8d5de30c04575814a0e9 
>   src/tests/slave_authorization_tests.cpp 
> 11fd0d4e35523eca23a9603ecef0c9d0b65dde38 
>   src/tests/slave_recovery_tests.cpp f14c6ef69eb20a03454c8197df79b572a3c6d050 
>   src/tests/slave_tests.cpp a2274b691cf94b003c4bc15450d176a9c73517d5 
>   src/tests/upgrade_tests.cpp 7f434dbba858f636719eec24e92b306b76430c4c 
> 
> 
> Diff: https://reviews.apache.org/r/64195/diff/1/
> 
> 
> Testing
> ---
> 
> Ran the tests, including with root.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 64158: Used helper functions instead of switches for resource extraction.

[Benjamin Bannier]

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


Fix it, then Ship it!




Thanks Jan! Will fix minor style nits while committing.


src/master/master.hpp
Line 2836 (original), 2813 (patched)


We pull `&` to the type.



src/master/master.hpp
Line 2897 (original), 2859 (patched)


We pull `&` to the type.


- Benjamin Bannier


On Nov. 29, 2017, 2:52 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64158/
> ---
> 
> (Updated Nov. 29, 2017, 2:52 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Depeding on the type of an offer operation, different resource need
> to be extracted from an operation. Instead of using a switch, the helper
> functions 'isSpeculativeOperation' and 'getConsumedResources' are used
> instead.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 2a2e830354db4a2191fb8321beb8174b80f7ba7d 
>   src/master/master.cpp 700e12433b0b66efc3f5dd296711c0f203a13144 
>   src/slave/slave.cpp e1566832f90cca372ad2f1cc13d1e7f76fa53285 
> 
> 
> Diff: https://reviews.apache.org/r/64158/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 64226: Added a `ns::supported` convenience API.

[Mesos Reviewbot Windows]

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



FAIL: Mesos libprocess-tests failed to build

Reviews applied: `['64226']`

Failed command: `cmake.exe --build . --target libprocess-tests --config Debug`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64226

Relevant logs:

- 
[libprocess-tests-build-cmake-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64226/logs/libprocess-tests-build-cmake-stdout.log):

```
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\state_machine_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\state_machine_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(805): warning 
C4244: 'argument': conversion from 'ULONG_PTR' to 'pid_t', possible loss of 
data (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\state_machine_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\subprocess_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\subprocess_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\system_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\system_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(805): warning 
C4244: 'argument': conversion from 'ULONG_PTR' to 'pid_t', possible loss of 
data (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\subprocess_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(805): warning 
C4244: 'argument': conversion from 'ULONG_PTR' to 'pid_t', possible loss of 
data (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\system_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\time_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\time_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(805): warning 
C4244: 'argument': conversion from 'ULONG_PTR' to 'pid_t', possible loss of 
data (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\time_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\ssl_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\ssl_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]

Re: Review Request 60891: Added ACLs and AuthZ for standalone containers.

[Joseph Wu]

> On Dec. 1, 2017, 1:30 a.m., Alexander Rojas wrote:
> > This patch didn't add tests in 
> > [authorization_tests.cpp](https://github.com/apache/mesos/blob/master/src/tests/authorization_tests.cpp)

Yeah, I have the tests up for review separately: 
https://reviews.apache.org/r/63828/ (which you've already reviewed ;)


- Joseph


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


On Nov. 13, 2017, 5:24 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60891/
> ---
> 
> (Updated Nov. 13, 2017, 5:24 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: MESOS-7305
> https://issues.apache.org/jira/browse/MESOS-7305
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This defines some coarse-grained AuthZ for launching and managing
> standalone containers.  Each HTTP principal can be given the right
> to Launch, Wait upon, Kill, or Remove standalone containers under
> a given (posix) user.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/acls.proto 
> 587b71489730f9a1252c73c0239e3d9892b3ae8e 
>   include/mesos/authorizer/authorizer.proto 
> 87a805794f430fc8b2e47de6d624b95deef162b4 
>   src/authorizer/local/authorizer.cpp 
> 2fe7b879e649b13322cfcb300c21ef1ed0fea410 
> 
> 
> Diff: https://reviews.apache.org/r/60891/diff/5/
> 
> 
> Testing
> ---
> 
> See later in chain.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 64010: Added additional parameters to Allocator::updateSlave().

[Benjamin Bannier]

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




include/mesos/allocator/allocator.hpp
Lines 211-212 (patched)


This doesn't look what was in the original design where a framework could 
receive an update for agent updates and remove offer filters itself (we already 
independently plan to extend the scheduler interfaces to allow reviving a 
single agent for offer operation feedback).

I feel having a master remove unexpired offer filters by itself violates 
responsibilities. It would seem much more straight-forward to reason about 
lifecycles would the framework which created the filter also be the sole on 
responsible for cleaning it up before expiry. The originally designed agent 
update channel to the framework would permit this, and I hope this is what we 
still plan to implement.

This makes me wonder whether we should we mark this parameter as 
transitional if we cannot get rid of it completely now. We are working on a 
public interface here so this is not ideal either.



include/mesos/allocator/allocator.hpp
Line 215 (original), 219-220 (patched)


I feel it might make sense to remove the default arguments here as they are 
error-prone and could lead to surprising behavior, see 
http://www.gotw.ca/gotw/005.htm.


- Benjamin Bannier


On Nov. 29, 2017, 7:17 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64010/
> ---
> 
> (Updated Nov. 29, 2017, 7:17 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The current `SlaveInfo` of a slave is now passed as an additional
> parameter to `updateSlave()`, to account for the possibility of
> it changing during an agent restart.
> 
> While the existing HierarchicalDRFAllocator only cares about the domain
> and hostname fields, the full SlaveInfo is passed since custom
> allocators might base their scheduling decisions on other fields, for
> example attributes.
> 
> Additionally, this also provides callers with an option to reset
> existing offer filters for a given slave id when the update is
> applied.
> 
> 
> Diffs
> -
> 
>   include/mesos/allocator/allocator.hpp 
> ae122003487ca8956573e993cd3993aa8cc286f1 
>   src/master/allocator/mesos/allocator.hpp 
> 8fa4fdeec4ec64bcd49fc442c230d8684a11cfd9 
>   src/master/allocator/mesos/hierarchical.hpp 
> 2c4832b29842330fa57756cd3d4202f265a820f3 
>   src/master/allocator/mesos/hierarchical.cpp 
> 5ce9ceaa3a5f84a1e076d45448863c418531cc2b 
>   src/tests/allocator.hpp 6a84f1beb86dceb5a5e9bf4615c13a216f3d0905 
>   src/tests/hierarchical_allocator_tests.cpp 
> f0f95ba4f667bf8ea54e985d8cde913a4170d8ff 
> 
> 
> Diff: https://reviews.apache.org/r/64010/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 63830: Fixed 'NoOffersWithAllRolesSuppressed' test.

[Alexander Rukletsov]

> On Nov. 21, 2017, 5:53 p.m., Alexander Rukletsov wrote:
> > src/tests/scheduler_tests.cpp
> > Lines 1515-1516 (original), 1534-1535 (patched)
> > 
> >
> > `.WillRepeatedly(Return());`
> 
> Jiang Yan Xu wrote:
> There's only one agent with no tasks launched so I don't expect 
> additional offers?

I see it in a different way. The default behaviour is ignore subsequent offers 
and only if you need to verify that in some particular case no more offers will 
be made, you should omit `.WillRepeatedly(Return());`. In this case what you 
want to check is that "scheduler starts getting offers" saying nothing about 
its quantity or periodicity.


- Alexander


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


On Nov. 21, 2017, 5:46 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63830/
> ---
> 
> (Updated Nov. 21, 2017, 5:46 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-7996
> https://issues.apache.org/jira/browse/MESOS-7996
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The previous iteration of the test failed to reveal the bug in MESOS-8200 due 
> to a race condition.
> 
> 
> Diffs
> -
> 
>   src/tests/scheduler_tests.cpp 45fc9c0cfccdb22c2e3e8d5de30c04575814a0e9 
> 
> 
> Diff: https://reviews.apache.org/r/63830/diff/1/
> 
> 
> Testing
> ---
> 
> As expected, this test would reliably fail without /r/63741/ with the 
> following
> 
> ```
> I1115 07:48:28.536093 3117031 scheduler.cpp:741] Enqueuing event SUBSCRIBED 
> received from http://17.121.128.13:38423/master/api/v1/scheduler
> I1115 07:48:28.536224 3117031 scheduler.cpp:741] Enqueuing event HEARTBEAT 
> received from http://17.121.128.13:38423/master/api/v1/scheduler
> I1115 07:48:28.536244 3117029 master.cpp:8115] Sending 1 offers to framework 
> 560bd0e1-8ab7-4e7e-be7a-54cb5f399531- (default)
> I1115 07:48:28.536650 3117029 scheduler.cpp:741] Enqueuing event OFFERS 
> received from http://17.121.128.13:38423/master/api/v1/scheduler
> ../../src/tests/scheduler_tests.cpp:1497: Failure
> Mock function called more times than expected - returning directly.
> Function call: offers(0x7fffabd197a0, @0x7f7d8c005460 48-byte object 
> <50-98 0B-B0 7D-7F 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 
> 00-00 00-00 00-00 00-00 01-00 00-00 04-00 00-00 A0-54 00-8C 7D-7F 00-00>)
>  Expected: to be never called
>Actual: called once - over-saturated and active
> ```
> 
> The test passes with /r/63741/.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 63831: Fixed a bug that removed the suppressed framework from sorter.

[Alexander Rukletsov]

> On Nov. 21, 2017, 6:27 p.m., Alexander Rukletsov wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 430-437 (original), 431-438 (patched)
> > 
> >
> > Do we need a lambda here?
> 
> Jiang Yan Xu wrote:
> I kept the original use of lambdas. 
> 
> I think the benefit of labmdas here is more about readability: compared 
> to 
> 
> ```
> set removedRoles = oldRoles;
> foreach (const string& role, newRoles) {
>   result.erase(role);
> }
> ```
> 
> The constness and construction of the variable is clearly isolated into a 
> small block in a long method with many similar variables/code blocks.
> 
> However as pointed out by the TODO, we should probably just implement a 
> set difference operator so all these become one-liners. I'll do it later.

Sounds like a wrong tool for the job and hence confusing. I would have used an 
explicit scope then. But maybe this is just me. I'll ask MPark for the 
reasoning here. Since it has been here before, it's fine to leave it for now. 
Regading introducing the set operator, FYI: 
https://reviews.apache.org/r/57110/#comment239761


- Alexander


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


On Nov. 28, 2017, 12:45 a.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63831/
> ---
> 
> (Updated Nov. 28, 2017, 12:45 a.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-8223
> https://issues.apache.org/jira/browse/MESOS-8223
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In `updateFramework`, we currently treat frameworks suppressing a role the 
> same way as frameworks moving off a role and this could lead to the framework 
> being removed from the framework sorter, which is unexpected. We should only 
> remove the framework from a framework sorter when it's moving away from the 
> corresponding role.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 5ce9ceaa3a5f84a1e076d45448863c418531cc2b 
>   src/tests/scheduler_tests.cpp 45fc9c0cfccdb22c2e3e8d5de30c04575814a0e9 
> 
> 
> Diff: https://reviews.apache.org/r/63831/diff/2/
> 
> 
> Testing
> ---
> 
> make check.
> 
> Added a test.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 64247: Fixed a flaky test case in reservation tests.

[Mesos Reviewbot Windows]

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



FAIL: Mesos libprocess-tests failed to build

Reviews applied: `['64247']`

Failed command: `cmake.exe --build . --target libprocess-tests --config Debug`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64247

Relevant logs:

- 
[libprocess-tests-build-cmake-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64247/logs/libprocess-tests-build-cmake-stdout.log):

```
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\state_machine_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\state_machine_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(805): warning 
C4244: 'argument': conversion from 'ULONG_PTR' to 'pid_t', possible loss of 
data (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\state_machine_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\subprocess_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\subprocess_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\system_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\system_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(805): warning 
C4244: 'argument': conversion from 'ULONG_PTR' to 'pid_t', possible loss of 
data (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\subprocess_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(805): warning 
C4244: 'argument': conversion from 'ULONG_PTR' to 'pid_t', possible loss of 
data (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\system_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\time_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\time_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(805): warning 
C4244: 'argument': conversion from 'ULONG_PTR' to 'pid_t', possible loss of 
data (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\time_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\ssl_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\ssl_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]

Re: Review Request 63831: Fixed a bug that removed the suppressed framework from sorter.

[Mesos Reviewbot Windows]

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



FAIL: Mesos libprocess-tests failed to build

Reviews applied: `['63741', '63830', '63831']`

Failed command: `cmake.exe --build . --target libprocess-tests --config Debug`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/63831

Relevant logs:

- 
[libprocess-tests-build-cmake-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/63831/logs/libprocess-tests-build-cmake-stdout.log):

```
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\state_machine_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\state_machine_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(805): warning 
C4244: 'argument': conversion from 'ULONG_PTR' to 'pid_t', possible loss of 
data (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\state_machine_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\subprocess_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\subprocess_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\system_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\system_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(805): warning 
C4244: 'argument': conversion from 'ULONG_PTR' to 'pid_t', possible loss of 
data (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\subprocess_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(805): warning 
C4244: 'argument': conversion from 'ULONG_PTR' to 'pid_t', possible loss of 
data (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\system_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\time_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\time_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(805): warning 
C4244: 'argument': conversion from 'ULONG_PTR' to 'pid_t', possible loss of 
data (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\time_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\ssl_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\ssl_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]

Re: Review Request 63831: Fixed a bug that removed the suppressed framework from sorter.

[Alexander Rukletsov]

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


Ship it!




Ship It!

- Alexander Rukletsov


On Nov. 28, 2017, 12:45 a.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63831/
> ---
> 
> (Updated Nov. 28, 2017, 12:45 a.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-8223
> https://issues.apache.org/jira/browse/MESOS-8223
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In `updateFramework`, we currently treat frameworks suppressing a role the 
> same way as frameworks moving off a role and this could lead to the framework 
> being removed from the framework sorter, which is unexpected. We should only 
> remove the framework from a framework sorter when it's moving away from the 
> corresponding role.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 5ce9ceaa3a5f84a1e076d45448863c418531cc2b 
>   src/tests/scheduler_tests.cpp 45fc9c0cfccdb22c2e3e8d5de30c04575814a0e9 
> 
> 
> Diff: https://reviews.apache.org/r/63831/diff/2/
> 
> 
> Testing
> ---
> 
> make check.
> 
> Added a test.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 63830: Fixed 'NoOffersWithAllRolesSuppressed' test.

[Alexander Rukletsov]

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


Ship it!




Ship It!

- Alexander Rukletsov


On Nov. 21, 2017, 5:46 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63830/
> ---
> 
> (Updated Nov. 21, 2017, 5:46 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-7996
> https://issues.apache.org/jira/browse/MESOS-7996
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The previous iteration of the test failed to reveal the bug in MESOS-8200 due 
> to a race condition.
> 
> 
> Diffs
> -
> 
>   src/tests/scheduler_tests.cpp 45fc9c0cfccdb22c2e3e8d5de30c04575814a0e9 
> 
> 
> Diff: https://reviews.apache.org/r/63830/diff/1/
> 
> 
> Testing
> ---
> 
> As expected, this test would reliably fail without /r/63741/ with the 
> following
> 
> ```
> I1115 07:48:28.536093 3117031 scheduler.cpp:741] Enqueuing event SUBSCRIBED 
> received from http://17.121.128.13:38423/master/api/v1/scheduler
> I1115 07:48:28.536224 3117031 scheduler.cpp:741] Enqueuing event HEARTBEAT 
> received from http://17.121.128.13:38423/master/api/v1/scheduler
> I1115 07:48:28.536244 3117029 master.cpp:8115] Sending 1 offers to framework 
> 560bd0e1-8ab7-4e7e-be7a-54cb5f399531- (default)
> I1115 07:48:28.536650 3117029 scheduler.cpp:741] Enqueuing event OFFERS 
> received from http://17.121.128.13:38423/master/api/v1/scheduler
> ../../src/tests/scheduler_tests.cpp:1497: Failure
> Mock function called more times than expected - returning directly.
> Function call: offers(0x7fffabd197a0, @0x7f7d8c005460 48-byte object 
> <50-98 0B-B0 7D-7F 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 
> 00-00 00-00 00-00 00-00 01-00 00-00 04-00 00-00 A0-54 00-8C 7D-7F 00-00>)
>  Expected: to be never called
>Actual: called once - over-saturated and active
> ```
> 
> The test passes with /r/63741/.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Review Request 64247: Fixed a flaky test case in reservation tests.

[Jan Schlicht]

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

Review request for mesos, Alexander Rukletsov, Benjamin Bannier, and Jie Yu.


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


Repository: mesos


Description
---

ReservationTest.MasterFailover could fail when parameterized with
the 'RESOURCE_PROVIDER' capability. After the simulated master failover,
the re-registered framework could receive an offer before the
re-registered sends a 'UPDATE_SLAVE' message. On receiving this message,
the master would rescind the offer and send out a new one. The test
expected a single offer to be send. It has been changed to accept the
first offer and ignore subsequent ones.

Also, the test case has been updated to run with paused clocks.


Diffs
-

  src/tests/reservation_tests.cpp f29e9bbb3d3b4dbaf21cd04f5efcf0df7cc34459 


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


Testing
---

src/mesos-tests

`src/mesos-tests 
--gtest_filter=ResourceProviderCapability/ReservationTest.MasterFailover/1 
--gtest_repeat=2000 --gtest_break_on_failure` while running `stress --cpu 8 
--io 8` in the background (on a 4 core machine)


Thanks,

Jan Schlicht

Review Request 64248: Added `cpp17::invoke` in .

[Michael Park]

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

Review request for mesos and Dmitry Zhuk.


Repository: mesos


Description
---

See summary.


Diffs
-

  3rdparty/stout/Makefile.am 4386017acd6ca465be3f735460c11d50b440ccc5 
  3rdparty/stout/include/stout/cpp17.hpp PRE-CREATION 
  3rdparty/stout/tests/CMakeLists.txt 6e5773f1e03671de7ac007caf49edd0f1cd7aedd 
  3rdparty/stout/tests/cpp17_tests.cpp PRE-CREATION 


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


Testing
---

Addded `cpp17_tests.cpp`


Thanks,

Michael Park

Re: Review Request 63741: Fixed a bug in devolving framework subscription with suppressed roles.

[Alexander Rukletsov]

> On Nov. 21, 2017, 6:04 p.m., Alexander Rukletsov wrote:
> > src/internal/devolve.cpp
> > Lines 203-204 (patched)
> > 
> >
> > I believe we prefer writing `CopyFrom()` explicitly, no?
> 
> Jiang Yan Xu wrote:
> I don't think it's estalished as a rule even though we do use CopyFrom 
> most often previously. Now that we have upgraded to protobuf 3.5 with move 
> semantics, I think we can probably standarize on prefering the assignment 
> operator as the choice between copy and move can be determined simply by the 
> rvalue-ness of the operand unless copying is explicitly intended. I 
> understand that this particular case wouldn't be moved but styling wise it's 
> more futre-proof. We should further this discussion. For now I think this is 
> OK as this style is already used in the codebase.

I didn't know `CopyFrom` and `operator=` can behave differently in proto 3.5. 
Could you please point me to that code to broaden my horizons?


- Alexander


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


On Nov. 28, 2017, 12:29 a.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63741/
> ---
> 
> (Updated Nov. 28, 2017, 12:29 a.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-8200
> https://issues.apache.org/jira/browse/MESOS-8200
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Also improved logging to show the suppressed roles.
> 
> 
> Diffs
> -
> 
>   src/internal/devolve.cpp 3a02490d734ef4a558b448871f9a1054d832f457 
>   src/internal/evolve.cpp cb1c0eb0908b05ef4f14462f6cf704a9151ea875 
>   src/master/master.cpp 53263e499d88b906b6406c24c0dfb737e589e813 
> 
> 
> Diff: https://reviews.apache.org/r/63741/diff/2/
> 
> 
> Testing
> ---
> 
> Tested together with /r/63830/.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Review Request 64250: Added new reasons task status update.

[Megha Sharma]

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

Review request for mesos, Ilya Pronin, James Peach, and Jiang Yan Xu.


Repository: mesos


Description
---

Added new reasons `REASON_SLAVE_REREGISTERED` and
`REASON_AGENT_REREGISTERED` for v0 and v1 task status update.
The new reason will be used when master starts to send status
update during the re-registeration of an agent which was
previosuly removed by the master because of being unreachable.


Diffs
-

  docs/task-state-reasons.md 07f7927e28fe5624a6c9b74de731483aaa729ca3 
  include/mesos/mesos.proto 11089b7d6f48f001ae64e8b0c67a2732684aa2fa 
  include/mesos/v1/mesos.proto c496da927903f472c78d455b7fdf886d8fb692c4 


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


Testing
---


Thanks,

Megha Sharma

Re: Review Request 63741: Fixed a bug in devolving framework subscription with suppressed roles.

[Alexander Rukletsov]

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


Ship it!




Ship It!

- Alexander Rukletsov


On Nov. 28, 2017, 12:29 a.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63741/
> ---
> 
> (Updated Nov. 28, 2017, 12:29 a.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-8200
> https://issues.apache.org/jira/browse/MESOS-8200
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Also improved logging to show the suppressed roles.
> 
> 
> Diffs
> -
> 
>   src/internal/devolve.cpp 3a02490d734ef4a558b448871f9a1054d832f457 
>   src/internal/evolve.cpp cb1c0eb0908b05ef4f14462f6cf704a9151ea875 
>   src/master/master.cpp 53263e499d88b906b6406c24c0dfb737e589e813 
> 
> 
> Diff: https://reviews.apache.org/r/63741/diff/2/
> 
> 
> Testing
> ---
> 
> Tested together with /r/63830/.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 63636: Added placeholder implementation.

[Michael Park]

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


Fix it, then Ship it!





3rdparty/stout/include/stout/lambda.hpp
Lines 474-476 (patched)


We should inherit from `lambda::Placeholder` here,
`is_placeholder` is expected to provide `::value` for example.



3rdparty/stout/include/stout/lambda.hpp
Lines 479-481 (patched)


We shouldn't need this one.


- Michael Park


On Nov. 22, 2017, 6:13 a.m., Dmitry Zhuk wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63636/
> ---
> 
> (Updated Nov. 22, 2017, 6:13 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Michael Park.
> 
> 
> Bugs: MESOS-6972
> https://issues.apache.org/jira/browse/MESOS-6972
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added placeholder implementation.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/lambda.hpp 
> a61d97b69e7eebd057c94148d39c6e1fc3066017 
> 
> 
> Diff: https://reviews.apache.org/r/63636/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Dmitry Zhuk
> 
>

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

[Greg Mann]

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




src/CMakeLists.txt
Lines 487 (patched)


Perhaps the filenames 'offer_operation.{cpp,hpp}' would suffice, since they 
sit within the 'status_update_manager' directory. WDYT?



src/status_update_manager/offer_operation_status_update_manager.hpp
Lines 17-18 (patched)


s/PROCESS_//



src/status_update_manager/offer_operation_status_update_manager.hpp
Lines 46-47 (patched)


Let's be a little more explicit here:

"called in order to forward a new status update to its recipient."



src/status_update_manager/offer_operation_status_update_manager.hpp
Lines 79 (patched)


s/exist/exist or is empty/



src/status_update_manager/offer_operation_status_update_manager.hpp
Lines 80-83 (patched)


Need to update this comment and the signature for the `strict` param.



src/status_update_manager/offer_operation_status_update_manager.hpp
Lines 91-97 (patched)


Can be removed.



src/status_update_manager/offer_operation_status_update_manager.hpp
Lines 102 (patched)


Let's use an `Owned` here instead of a raw pointer.



src/status_update_manager/offer_operation_status_update_manager.cpp
Lines 36-38 (patched)


I think it's a bit more consistent with the spirit of our style guide (i.e. 
to reduce "jaggedness") to wrap after the '<' and indent 4 spaces:

```
process = new StatusUpdateManagerProcess<
UUID,
OfferOperationStatusUpdateRecord,
OfferOperationStatusUpdate>();
```


http://mesos.apache.org/documentation/latest/c++-style-guide/#function-definition-invocation

Could you switch to that format throughout this chain?


- Greg Mann


On Dec. 1, 2017, 12:27 a.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64096/
> ---
> 
> (Updated Dec. 1, 2017, 12:27 a.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Bugs: MESOS-8197
> https://issues.apache.org/jira/browse/MESOS-8197
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This class will handle the offer operation status updates generated by
> the agent and by resource providers.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 76ef6cac925e36a31e4f6c347271955411284056 
>   src/Makefile.am 4a3b728dc2cbfaf4b10068562778a87a6b331238 
>   src/status_update_manager/offer_operation_status_update_manager.hpp 
> PRE-CREATION 
>   src/status_update_manager/offer_operation_status_update_manager.cpp 
> PRE-CREATION 
>   src/tests/CMakeLists.txt 65fca826e8b46ad064096e0f02b771985bef95d0 
>   src/tests/offer_operation_status_update_manager_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/64096/diff/3/
> 
> 
> Testing
> ---
> 
> This patch addes new tests that passed 5000 times on GNU/Linux.
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 63630: Added support for callable once functors.

[Michael Park]

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


Fix it, then Ship it!





3rdparty/stout/include/stout/lambda.hpp
Lines 357 (patched)


Let's use `cpp17::invoke` here.



3rdparty/stout/include/stout/lambda.hpp
Lines 375 (patched)


Let's elaborate on this comment a little bit around
the heap allocation involved here due to
the type-erasure and mention that `std::function`
also has this overhead-ish.



3rdparty/stout/include/stout/lambda.hpp
Lines 393 (patched)


Let's decay this earlier.



3rdparty/stout/include/stout/lambda.hpp
Lines 404 (patched)


Can we make this a `unique_ptr` such that we can default some of the 
operations?



3rdparty/stout/include/stout/lambda.hpp
Lines 420-425 (patched)


We recently started to stick to this pattern:
```
tmeplate ::type = 0>
```


- Michael Park


On Nov. 22, 2017, 6:10 a.m., Dmitry Zhuk wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63630/
> ---
> 
> (Updated Nov. 22, 2017, 6:10 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Michael Park.
> 
> 
> Bugs: MESOS-6972
> https://issues.apache.org/jira/browse/MESOS-6972
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `CallableOnce` class is similar to `std::function`, but allows it to be
> called only once. Together with `partial` this provides foundation for
> copy-less `defer`, `dispatch` and `Future`.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/lambda.hpp 
> a61d97b69e7eebd057c94148d39c6e1fc3066017 
> 
> 
> Diff: https://reviews.apache.org/r/63630/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Dmitry Zhuk
> 
>

Re: Review Request 64248: Added `cpp17::invoke` in .

[Mesos Reviewbot Windows]

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



FAIL: Mesos libprocess-tests failed to build

Reviews applied: `['64248']`

Failed command: `cmake.exe --build . --target libprocess-tests --config Debug`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64248

Relevant logs:

- 
[libprocess-tests-build-cmake-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64248/logs/libprocess-tests-build-cmake-stdout.log):

```
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\state_machine_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\state_machine_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(805): warning 
C4244: 'argument': conversion from 'ULONG_PTR' to 'pid_t', possible loss of 
data (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\state_machine_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\subprocess_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\subprocess_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\system_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\system_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(805): warning 
C4244: 'argument': conversion from 'ULONG_PTR' to 'pid_t', possible loss of 
data (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\subprocess_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(805): warning 
C4244: 'argument': conversion from 'ULONG_PTR' to 'pid_t', possible loss of 
data (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\system_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\time_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\time_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(805): warning 
C4244: 'argument': conversion from 'ULONG_PTR' to 'pid_t', possible loss of 
data (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\time_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\ssl_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\ssl_tests.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]

Re: Review Request 63631: Separated event visiting and consumption.

[Michael Park]

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


Fix it, then Ship it!




The `ProcessBase` and `EventVisitor` relationship is a bit weird already,
and I'm a bit concerned about essentially changing the definition of
`ProcessBase` across the entire codebase, but I think overall the patch
looks consistent and correct.


3rdparty/libprocess/src/tests/benchmarks.cpp
Lines 608-609 (original), 608-609 (patched)


Let's remove this since we're constructing new ones within the `for` loop 
now.


- Michael Park


On Nov. 7, 2017, 8:51 a.m., Dmitry Zhuk wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63631/
> ---
> 
> (Updated Nov. 7, 2017, 8:51 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Michael Park.
> 
> 
> Bugs: MESOS-6972
> https://issues.apache.org/jira/browse/MESOS-6972
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This introduces `EventConsumer` interface, which add support of events
> with movable only data. This allows consumers to move data out of
> event, rather than copying it. This is required to implement movable
> only objects support in `defer` to guarantee that deferred functor is
> invoked only once, allowing deferred parameters to be moved into call.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/event.hpp 
> 2e246205020c3c5b8c2eb5187a8eb3643d1e6d4d 
>   3rdparty/libprocess/include/process/process.hpp 
> dc3375ce62556322eb2bc60ade61f313ade123b8 
>   3rdparty/libprocess/include/process/protobuf.hpp 
> fe152f273332470ac50f9715291897bb04cf95b9 
>   3rdparty/libprocess/include/process/run.hpp 
> 1540a78e52a90d4c1d4165c46be353caaad21bce 
>   3rdparty/libprocess/src/process.cpp 
> 71ae7129ffbd0e22eda2863b17bbcf588298c37b 
>   3rdparty/libprocess/src/tests/benchmarks.cpp 
> e6c77d565d5acf72b475a085e9504679253b4b97 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> 952c92c033e2363cff0c2c68610d3820b97d177e 
> 
> 
> Diff: https://reviews.apache.org/r/63631/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Dmitry Zhuk
> 
>

Re: Review Request 63862: Windows: Ported docker_tests.cpp.

[Andrew Schwartzmeyer]

> On Dec. 1, 2017, 3:25 a.m., Mesos Reviewbot Windows wrote:
> > FAIL: Mesos libprocess-tests failed to build
> > 
> > Reviews applied: `['63859', '63860', '63861', '63862']`
> > 
> > Failed command: `cmake.exe --build . --target libprocess-tests --config 
> > Debug`
> > 
> > All the build artifacts available at: 
> > http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/63862
> > 
> > Relevant logs:
> > 
> > - 
> > [libprocess-tests-build-cmake-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/63862/logs/libprocess-tests-build-cmake-stdout.log):
> > 
> > ```
> >   C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): 
> > warning C4996: 'GetVersionExW': was declared deprecated (compiling source 
> > file 
> > C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\state_machine_tests.cpp) 
> > [C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
> >   C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): 
> > warning C4996: 'GetVersionExW': was declared deprecated (compiling source 
> > file 
> > C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\state_machine_tests.cpp) 
> > [C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
> >   C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(805): 
> > warning C4244: 'argument': conversion from 'ULONG_PTR' to 'pid_t', possible 
> > loss of data (compiling source file 
> > C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\state_machine_tests.cpp) 
> > [C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
> >   C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): 
> > warning C4996: 'GetVersionExW': was declared deprecated (compiling source 
> > file 
> > C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\subprocess_tests.cpp) 
> > [C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
> >   C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): 
> > warning C4996: 'GetVersionExW': was declared deprecated (compiling source 
> > file 
> > C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\subprocess_tests.cpp) 
> > [C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
> >   C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): 
> > warning C4996: 'GetVersionExW': was declared deprecated (compiling source 
> > file C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\system_tests.cpp) 
> > [C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
> >   C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): 
> > warning C4996: 'GetVersionExW': was declared deprecated (compiling source 
> > file C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\system_tests.cpp) 
> > [C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
> >   C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(805): 
> > warning C4244: 'argument': conversion from 'ULONG_PTR' to 'pid_t', possible 
> > loss of data (compiling source file 
> > C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\subprocess_tests.cpp) 
> > [C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
> >   C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(805): 
> > warning C4244: 'argument': conversion from 'ULONG_PTR' to 'pid_t', possible 
> > loss of data (compiling source file 
> > C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\system_tests.cpp) 
> > [C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
> >   C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): 
> > warning C4996: 'GetVersionExW': was declared deprecated (compiling source 
> > file C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\time_tests.cpp) 
> > [C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
> >   C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): 
> > warning C4996: 'GetVersionExW': was declared deprecated (compiling source 
> > file C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\time_tests.cpp) 
> > [C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
> >   C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(805): 
> > warning C4244: 'argument': conversion from 'ULONG_PTR' to 'pid_t', possible 
> > loss of data (compiling source file 
> > C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\time_tests.cpp) 
> > [C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
> >   C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): 
> > warning C4996: 'GetVersionExW': was declared deprecated (compiling source 
> > file C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\ssl_tests.cpp) 
> > [C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
> >   C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): 
> > warning C4996: 'GetVersionExW': was declared deprecated (compiling source 
> > file 

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

[Greg Mann]

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




src/status_update_manager/offer_operation_status_update_manager.cpp
Lines 57-59 (patched)


Yesterday we discussed moving the header's `typedef` out to file scope, 
which would let you use it here.


- Greg Mann


On Dec. 1, 2017, 12:27 a.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64096/
> ---
> 
> (Updated Dec. 1, 2017, 12:27 a.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Bugs: MESOS-8197
> https://issues.apache.org/jira/browse/MESOS-8197
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This class will handle the offer operation status updates generated by
> the agent and by resource providers.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 76ef6cac925e36a31e4f6c347271955411284056 
>   src/Makefile.am 4a3b728dc2cbfaf4b10068562778a87a6b331238 
>   src/status_update_manager/offer_operation_status_update_manager.hpp 
> PRE-CREATION 
>   src/status_update_manager/offer_operation_status_update_manager.cpp 
> PRE-CREATION 
>   src/tests/CMakeLists.txt 65fca826e8b46ad064096e0f02b771985bef95d0 
>   src/tests/offer_operation_status_update_manager_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/64096/diff/3/
> 
> 
> Testing
> ---
> 
> This patch addes new tests that passed 5000 times on GNU/Linux.
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 64104: Added dependency of curl to agent. Enabled most health check tests.

[Akash Gupta]

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




src/tests/health_check_tests.cpp
Line 565 (original), 565 (patched)


The docker tests should be disabled for now since they don't work. For 
example, this tests tries running the `alpine` image on Windows. I'm working on 
enabling the docker health checks on Windows.



src/tests/health_check_tests.cpp
Line 2492 (original), 2492 (patched)


Should remove the `DOCKER_` since it uses the mesos containerizer.


- Akash Gupta


On Nov. 30, 2017, 5:18 p.m., John Kordich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64104/
> ---
> 
> (Updated Nov. 30, 2017, 5:18 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Jeff Coffler, 
> and Joseph Wu.
> 
> 
> Bugs: MESOS-6709
> https://issues.apache.org/jira/browse/MESOS-6709
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added dependency of curl to agent. Enabled most health check tests.
> 
> 
> Diffs
> -
> 
>   src/slave/CMakeLists.txt 6f08f3dc95f3a1408cbef7b8f0b0cc2522558924 
>   src/tests/health_check_tests.cpp c0dcba265363f2149b217b189ee5a8e925e40ea1 
> 
> 
> Diff: https://reviews.apache.org/r/64104/diff/1/
> 
> 
> Testing
> ---
> 
> There are two health check tests which are not enabled yet, because they 
> require the IOSwitchboard to be ported first. We can't resolve MESOS-6709 
> quite yet until that is addressed. The rest of the tests pass on both 
> Linux/Windows, as mentioned in #64102
> 
> 
> Thanks,
> 
> John Kordich
> 
>

Re: Review Request 64103: Changed dependency of curl to libcurl for stout.

[Akash Gupta]

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


Ship it!




Ship It!

- Akash Gupta


On Nov. 30, 2017, 5:18 p.m., John Kordich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64103/
> ---
> 
> (Updated Nov. 30, 2017, 5:18 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Jeff Coffler, 
> and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changed dependency of curl to libcurl for stout.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/CMakeLists.txt 145b772a4f5da5653b918f746fbfc44e58321e47 
> 
> 
> Diff: https://reviews.apache.org/r/64103/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> John Kordich
> 
>

Re: Review Request 64102: Renamed curl target to libcurl, and staging of curl.exe on Windows.

[Akash Gupta]

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


Ship it!




Ship It!

- Akash Gupta


On Nov. 30, 2017, 5:18 p.m., John Kordich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64102/
> ---
> 
> (Updated Nov. 30, 2017, 5:18 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Jeff Coffler, 
> and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Renamed curl target to libcurl, and staging of curl.exe on Windows.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt c34b65d5ff306e17c3519667ae303a8678fca43d 
> 
> 
> Diff: https://reviews.apache.org/r/64102/diff/1/
> 
> 
> Testing
> ---
> 
> Ran all tests on Windows/Linux (for this patch plus #64103 and #64104). No 
> related tests failed on either Linux or Windows.
> 
> 
> Thanks,
> 
> John Kordich
> 
>

Re: Review Request 63860: Windows: Mapped the Docker network info types.

[Akash Gupta]

> On Dec. 1, 2017, 1:47 a.m., Jie Yu wrote:
> > src/docker/docker.cpp
> > Lines 742-743 (original), 742-752 (patched)
> > 
> >
> > It's weird that user specifies HOST in the API, but we use "nat" 
> > instead.
> > 
> > Why can't we use transparent? I don't quite get that from the comments.

It's not a default network on Windows like it is on Linux. If you do a `docker 
network ls` on a fresh Windows box, you see a `nat` network and a `none` 
network. Transparent is the network driver type, so to use it, you need to 
create an user defined network by doing `docker network create -d transparent 
` and then to use it, you do `docker run --network= 
...`. I agree that using transparent would make more sense, but we would have 
to make the agent create the network and pass that in to the executor.


- Akash


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


On Nov. 27, 2017, 5:37 p.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63860/
> ---
> 
> (Updated Nov. 27, 2017, 5:37 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer and John Kordich.
> 
> 
> Bugs: MESOS-7342
> https://issues.apache.org/jira/browse/MESOS-7342
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The current Network enum in DockerInfo is specific to Linux containers.
> Instead of supporting {host, bridge, none, user} networks, Windows
> docker supports {nat, none, user} networks. Now, if the host or bridge
> network type is sent to the Windows agent, it will be internally
> converted to nat.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 722a54ad113fc4e2bb22a8f08e307ab38d5fbfed 
> 
> 
> Diff: https://reviews.apache.org/r/63860/diff/3/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/63862/ for test results.
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 63765: Fixed an issue with the scheduler driver subscribe backoff time.

[Vinod Kone]

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




src/tests/scheduler_driver_tests.cpp
Lines 107 (patched)


s/enqueue/enqueued/

sorry my bad for the typo in the comment.


- Vinod Kone


On Nov. 18, 2017, 9:32 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63765/
> ---
> 
> (Updated Nov. 18, 2017, 9:32 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Vinod Kone.
> 
> 
> Bugs: MESOS-8171
> https://issues.apache.org/jira/browse/MESOS-8171
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When framework failover time is set to zero (which is the
> default value), the scheduler driver subscribe backoff time
> will also become zero. Ignore failover time if it is zero
> when deciding the subscribe backoff time.
> Also added a dedicated test.
> 
> 
> Diffs
> -
> 
>   src/sched/sched.cpp 6028499285ad092ffd252e842c5d9835dd4442f8 
>   src/tests/scheduler_driver_tests.cpp 
> 14d872b8fadfd4ef16d8923fb0df924331534bc3 
> 
> 
> Diff: https://reviews.apache.org/r/63765/diff/2/
> 
> 
> Testing
> ---
> 
> make check, and the new dedicated test.
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 64256: Included `stout/os/realpath.hpp` where `os::realpath()` is used.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Dec. 1, 2017, 8:11 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64256/
> ---
> 
> (Updated Dec. 1, 2017, 8:11 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Included `stout/os/realpath.hpp` where `os::realpath()` is used.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/ssl/gtest.hpp 
> b368eda1edd588475f4f4838a26ba211a0850b9a 
> 
> 
> Diff: https://reviews.apache.org/r/64256/diff/1/
> 
> 
> Testing
> ---
> 
> See end of chain.
> 
> This is the `libprocess` portion.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 64255: Removed `os/realpath.hpp` from `posix/os.hpp`.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Dec. 1, 2017, 8:10 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64255/
> ---
> 
> (Updated Dec. 1, 2017, 8:10 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is part of a clean-up chain to fix the problem where `stout/os.hpp`
> previously made `os::realpath()` available, but it was instead moved to
> its own header. Instead, files must include what is used, and so include
> the `realpath.hpp` header.
> 
> Since `posix/os.hpp` doesn't use `os::realpath()`, its inclusion of
> `os/realpath.hpp` was removed, and instead code using the function was
> fixed to include the header directly.
> 
> Note that this now matches `windows/os.hpp`, which also does not include
> `realpath.hpp`. The same pattern should be followed for other APIs as
> needed.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/posix/os.hpp 
> 8511dfd419a646df17eb687d732bb975f4c23d53 
>   3rdparty/stout/tests/os/filesystem_tests.cpp 
> 3946885b12a78c765e506c855a12aed1efbeafad 
>   3rdparty/stout/tests/os_tests.cpp 26514f3604fcc81effbfcb6845116a62021272ca 
> 
> 
> Diff: https://reviews.apache.org/r/64255/diff/1/
> 
> 
> Testing
> ---
> 
> See end of chain.
> 
> Note that I grepped for all files using `os::realpath` and added the header.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 63859: Windows: Fixed mock signal values in stout.

[Akash Gupta]

> On Dec. 1, 2017, 2:14 a.m., Andrew Schwartzmeyer wrote:
> > 3rdparty/stout/include/stout/windows.hpp
> > Lines 343-348 (original), 343-348 (patched)
> > 
> >
> > :/ I'd really like to fix whatever code is using these signals for 
> > logic. I feel like the defining the signals for Windows was originally a 
> > band-aid, and understand this patch didn't add them.
> > 
> > The funny thing is that, since these values aren't defined on Windows, 
> > they could be any number so long as only the symbol is used in the rest of 
> > the code base. I think this is why this worked anyway.
> > 
> > Akash, what bug did you run into that required correcting these?
> > 
> > (And indeed, they now appear correct. SIGKILL is 9, SIGCONT is 18 in 
> > decimal, and SIGSTOP is 19 in decimal).

I think they worked before because they were used only within stout. One of the 
docker executor method signature is `docker->kill(ID, SIGNAL)`, which 
eventually calls `docker kill -s SIGNAL ID`. Docker (and go standard library) 
defines the Linux signal values on Windows, so it's expecting `docker kill -s 
9`. If you want, I can fix the docker executor to ignore the signal field on 
Windows and just send `docker kill` without the `-s`.


- Akash


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


On Nov. 27, 2017, 5:36 p.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63859/
> ---
> 
> (Updated Nov. 27, 2017, 5:36 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer and John Kordich.
> 
> 
> Bugs: MESOS-7342
> https://issues.apache.org/jira/browse/MESOS-7342
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Also fixed the WEXITSTATUS macro to cast the exit code instead of
> bit-masking it, since Windows exit codes are 32 bit unsigned ints.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/windows.hpp 
> 7aa0ba72c4cc3b688ce6374b3308945ea8cb7572 
> 
> 
> Diff: https://reviews.apache.org/r/63859/diff/3/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/63862/ for test results.
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 64095: Added a generic actor to be used by status update managers.

[Greg Mann]

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




src/status_update_manager/status_update_manager_process.hpp
Lines 654 (patched)


Actually, after looking at this again I was thinking: perhaps this comment 
should be changed to tell readers that we are doing two things here: both 
building our in-memory streams and the state object which we return to the 
caller. WDYT?



src/status_update_manager/status_update_manager_process.hpp
Lines 666 (patched)


Could we use a `switch` here instead of `if`?



src/status_update_manager/status_update_manager_process.hpp
Lines 678 (patched)


s/.get()./->/



src/status_update_manager/status_update_manager_process.hpp
Lines 692 (patched)


As written, this is a bit opaque. I had to look up why we're seeking from 
the current position with an offset of zero. I now see that we're doing this in 
an attempt to return the current file position, ensuring that the current 
position is valid.

Could you clarify this? I could imagine doing it by renaming this variable 
from `lseek` to `currentPosition`, or putting a note in the comment above.



src/status_update_manager/status_update_manager_process.hpp
Lines 709-710 (patched)


Fits on one line.



src/status_update_manager/status_update_manager_process.hpp
Lines 717 (patched)


Could you get rid of this `else`?



src/status_update_manager/status_update_manager_process.hpp
Lines 725 (patched)


We could probably use a `std::pair` here instead. And maybe we can use 
`std::make_pair`, as long as it doesn't have issues with type deduction?



src/status_update_manager/status_update_manager_process.hpp
Lines 734 (patched)


s/duplicate/duplicate or has already been acknowledged/



src/status_update_manager/status_update_manager_process.hpp
Lines 747 (patched)


s/uuid/status_uuid/



src/status_update_manager/status_update_manager_process.hpp
Lines 754 (patched)


Could you remove the exclamation point?



src/status_update_manager/status_update_manager_process.hpp
Lines 780-783 (patched)


I know that in this context, using just `uuid` is not as ambiguous, but 
given the presence of multiple UUIDs in general I think it's better to be 
explicit. Could we use `status_uuid` for the function argument?



src/status_update_manager/status_update_manager_process.hpp
Lines 790-791 (patched)


Since we print the status UUID in the stream operator for the update type, 
we should be able to eliminate the explicit UUID in the log line here and not 
lose any information.



src/status_update_manager/status_update_manager_process.hpp
Lines 847-849 (patched)


I think it's more common for us to keep the empty function body on the same 
line:

```
: terminated(false), streamId(_streamId), path(_path), fd(_fd) {}
```

Here and elsewhere.



src/status_update_manager/status_update_manager_process.hpp
Lines 855 (patched)


s/its/it's/



src/status_update_manager/status_update_manager_process.hpp
Lines 873 (patched)


Ditto - use a `switch` here.



src/status_update_manager/status_update_manager_process.hpp
Lines 885-886 (patched)


Hmm this error message seems strange in the ACK case?



src/status_update_manager/status_update_manager_process.hpp
Lines 905 (patched)


Ditto - use a `switch` here.



src/status_update_manager/status_update_manager_process.hpp
Lines 907-908 (patched)


Let's pass framework ID into the creation method to eliminate the NONE case 
here.



src/status_update_manager/status_update_manager_process.hpp
Lines 912 (patched)


I think we can remove this comment.



src/status_update_manager/status_update_manager_process.hpp
Lines 923 (patched)


Can remove this comment.


- Greg Mann


On Dec. 1, 2017, 1:42 a.m., Gaston Kleiman 

Re: Review Request 64250: Added new reasons task status update.

[Jiang Yan Xu]

> On Dec. 1, 2017, 1:55 p.m., Ilya Pronin wrote:
> > docs/task-state-reasons.md
> > Lines 474-477 (patched)
> > 
> >
> > I don't quite follow this note. A modified copy of which update? Should 
> > we just say that these updates reflect the states of the tasks reported by 
> > the agent upon its re-registration?
> 
> Megha Sharma wrote:
> Here, we are saying that master takes the actual state reported by the 
> agent and enriches it with reason and message before sending to the 
> framework. But I am open to changing it if you or Yan feel it doesn't convery 
> the idea.

I was suggesting it because this status is generated by the master the same way 
they are generated upon reconciliation.

For `REASON_RECONCILIATION` in the same doc there is this note:

```
Note: Status updates with this reason are not the original ones, but rather a 
modified copy that is re-sent from the master. In particular, the original data 
and message fields are erased and the original reason field is overwritten by 
REASON_RECONCILIATION .
```

It is the same for `REASON_SLAVE_REREGISTERED`. 


Megha I just noticed that you didn't mention the erasure of `data` and 
`message` fields, but perhaps this is not worth repeating. I was originally 
suggesting refering to `REASON_RECONCILIATION` for details: something like: 
`Status updates with this reason are a modified copies re-sent by the master. 
See comments for REASON_RECONCILIATION.`

Would this be clearer?

"reflect the states of the tasks reported by the agent upon its 
re-registration" this sentence is good too.


- Jiang Yan


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


On Dec. 1, 2017, 1:06 p.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64250/
> ---
> 
> (Updated Dec. 1, 2017, 1:06 p.m.)
> 
> 
> Review request for mesos, Ilya Pronin, James Peach, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added new reasons `REASON_SLAVE_REREGISTERED` and
> `REASON_AGENT_REREGISTERED` for v0 and v1 task status update.
> The new reason will be used when master starts to send status
> update during the re-registration of an agent which was
> previously removed by the master because of being unreachable
> or is unknown to the master due to the garbage collection of
> the unreachable and gone agents in the registry.
> 
> 
> Diffs
> -
> 
>   docs/task-state-reasons.md 07f7927e28fe5624a6c9b74de731483aaa729ca3 
>   include/mesos/mesos.proto 11089b7d6f48f001ae64e8b0c67a2732684aa2fa 
>   include/mesos/v1/mesos.proto c496da927903f472c78d455b7fdf886d8fb692c4 
> 
> 
> Diff: https://reviews.apache.org/r/64250/diff/4/
> 
> 
> Testing
> ---
> 
> code changes verified with make check and the documents changes with markdown 
> viewer
> 
> 
> Thanks,
> 
> Megha Sharma
> 
>

Re: Review Request 64250: Added new reasons task status update.

[Ilya Pronin]

> On Dec. 1, 2017, 1:55 p.m., Ilya Pronin wrote:
> > docs/task-state-reasons.md
> > Lines 474-477 (patched)
> > 
> >
> > I don't quite follow this note. A modified copy of which update? Should 
> > we just say that these updates reflect the states of the tasks reported by 
> > the agent upon its re-registration?
> 
> Megha Sharma wrote:
> Here, we are saying that master takes the actual state reported by the 
> agent and enriches it with reason and message before sending to the 
> framework. But I am open to changing it if you or Yan feel it doesn't convery 
> the idea.
> 
> Jiang Yan Xu wrote:
> I was suggesting it because this status is generated by the master the 
> same way they are generated upon reconciliation.
> 
> For `REASON_RECONCILIATION` in the same doc there is this note:
> 
> ```
> Note: Status updates with this reason are not the original ones, but 
> rather a modified copy that is re-sent from the master. In particular, the 
> original data and message fields are erased and the original reason field is 
> overwritten by REASON_RECONCILIATION .
> ```
> 
> It is the same for `REASON_SLAVE_REREGISTERED`. 
> 
> 
> Megha I just noticed that you didn't mention the erasure of `data` and 
> `message` fields, but perhaps this is not worth repeating. I was originally 
> suggesting refering to `REASON_RECONCILIATION` for details: something like: 
> `Status updates with this reason are a modified copies re-sent by the master. 
> See comments for REASON_RECONCILIATION.`
> 
> Would this be clearer?
> 
> "reflect the states of the tasks reported by the agent upon its 
> re-registration" this sentence is good too.

Oh, now I got where this came from :) Yeah, it would be clearer if there was a 
mention that new states come from the agent re-registration message and a 
reference to `REASON_RECONCILIATION` description. Or it should be enough to 
jsut say that these are produced by the master and reflect the states reported 
by the agent during re-registration.


- Ilya


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


On Dec. 1, 2017, 1:06 p.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64250/
> ---
> 
> (Updated Dec. 1, 2017, 1:06 p.m.)
> 
> 
> Review request for mesos, Ilya Pronin, James Peach, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added new reasons `REASON_SLAVE_REREGISTERED` and
> `REASON_AGENT_REREGISTERED` for v0 and v1 task status update.
> The new reason will be used when master starts to send status
> update during the re-registration of an agent which was
> previously removed by the master because of being unreachable
> or is unknown to the master due to the garbage collection of
> the unreachable and gone agents in the registry.
> 
> 
> Diffs
> -
> 
>   docs/task-state-reasons.md 07f7927e28fe5624a6c9b74de731483aaa729ca3 
>   include/mesos/mesos.proto 11089b7d6f48f001ae64e8b0c67a2732684aa2fa 
>   include/mesos/v1/mesos.proto c496da927903f472c78d455b7fdf886d8fb692c4 
> 
> 
> Diff: https://reviews.apache.org/r/64250/diff/4/
> 
> 
> Testing
> ---
> 
> code changes verified with make check and the documents changes with markdown 
> viewer
> 
> 
> Thanks,
> 
> Megha Sharma
> 
>

Re: Review Request 64093: Added operators for offer operation update protobuf classes.

[Greg Mann]

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




src/messages/messages.cpp
Lines 132 (patched)


s/UUID/status UUID/


- Greg Mann


On Nov. 30, 2017, 11:22 p.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64093/
> ---
> 
> (Updated Nov. 30, 2017, 11:22 p.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Bugs: MESOS-8197
> https://issues.apache.org/jira/browse/MESOS-8197
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added operators for offer operation update protobuf classes.
> 
> 
> Diffs
> -
> 
>   include/mesos/type_utils.hpp a348c7df083324602d25f40069ad49e29f7918a5 
>   src/common/type_utils.cpp 3657d55ab2a052b5c659022e3fc264e748429d54 
>   src/messages/messages.hpp 2756ebae5e2dccce63030eb7cec55d35445fa97b 
>   src/messages/messages.cpp 6029502e8394817a0ab6ee2f63895ad02349bb0d 
> 
> 
> Diff: https://reviews.apache.org/r/64093/diff/3/
> 
> 
> Testing
> ---
> 
> Added new tests at the end of the chain. They passed 5000 times on GNU/Linux.
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

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

[Jiang Yan Xu]

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




src/master/master.cpp
Line 6808 (original), 6808 (patched)


When considering the comment by Ilya in MESOS-6406 (i.e., what if agents 
GCed from the unreachable or gone list reregster?), seems like we can do this:

1. Move down the line `slaves.recovered.erase(slaveInfo.id());` to after we 
process `recoveredTasks`.
2. Instead of checking `slaves.unreachable.contains(slaveInfo.id()` we 
could check `!slaves.recovered.contains(slaveInfo.id()`
3. Now we are sending status updates for two cases: reregistering 
unreachable agents or unknown agents (which could have been marked either 
unreachable or gone but we can't distiguish)
- We can distinguish unreachable and unknown in the task status message.
- We can probably log a warning about tasks from unknown agents.



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


s/REASON_AGENT_REREGISTERED/REASON_SLAVE_REREGISTERED/


- Jiang Yan Xu


On Nov. 27, 2017, 4:55 p.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64098/
> ---
> 
> (Updated Nov. 27, 2017, 4:55 p.m.)
> 
> 
> Review request for mesos, Ilya Pronin, James Peach, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6406
> https://issues.apache.org/jira/browse/MESOS-6406
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Master will send task status updates to frameworks when an agent
> which has been previously removed by the master for being unreachable
> re-registers.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp dfe60ef670edcaefa0c1241df2e2870f650fcf9e 
>   src/tests/master_allocator_tests.cpp 
> 3400d70bb0ba564eac43c4639eee0efd4d8059e6 
>   src/tests/master_tests.cpp 57eae320a7a398527cd3623c89bf67f319a8e955 
>   src/tests/partition_tests.cpp 31ebfe1655438eceae74d72a223df03a9dbd282d 
>   src/tests/persistent_volume_tests.cpp 
> 4aa3c2e8b0f461cd78053707cff8bcb2e6f2b0d7 
>   src/tests/slave_recovery_tests.cpp f14c6ef69eb20a03454c8197df79b572a3c6d050 
>   src/tests/upgrade_tests.cpp 7f434dbba858f636719eec24e92b306b76430c4c 
> 
> 
> Diff: https://reviews.apache.org/r/64098/diff/5/
> 
> 
> Testing
> ---
> 
> with make check
> 
> 
> Thanks,
> 
> Megha Sharma
> 
>

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

[Megha Sharma]

> On Dec. 1, 2017, 9:29 p.m., Jiang Yan Xu wrote:
> > src/master/master.cpp
> > Line 6808 (original), 6808 (patched)
> > 
> >
> > When considering the comment by Ilya in MESOS-6406 (i.e., what if 
> > agents GCed from the unreachable or gone list reregster?), seems like we 
> > can do this:
> > 
> > 1. Move down the line `slaves.recovered.erase(slaveInfo.id());` to 
> > after we process `recoveredTasks`.
> > 2. Instead of checking `slaves.unreachable.contains(slaveInfo.id()` we 
> > could check `!slaves.recovered.contains(slaveInfo.id()`
> > 3. Now we are sending status updates for two cases: reregistering 
> > unreachable agents or unknown agents (which could have been marked either 
> > unreachable or gone but we can't distiguish)
> > - We can distinguish unreachable and unknown in the task status 
> > message.
> > - We can probably log a warning about tasks from unknown agents.

Sounds good, thanks for fomalizing.


> On Dec. 1, 2017, 9:29 p.m., Jiang Yan Xu wrote:
> > src/master/master.cpp
> > Lines 6818 (patched)
> > 
> >
> > s/REASON_AGENT_REREGISTERED/REASON_SLAVE_REREGISTERED/

Thanks!


- Megha


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


On Nov. 28, 2017, 12:55 a.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64098/
> ---
> 
> (Updated Nov. 28, 2017, 12:55 a.m.)
> 
> 
> Review request for mesos, Ilya Pronin, James Peach, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6406
> https://issues.apache.org/jira/browse/MESOS-6406
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Master will send task status updates to frameworks when an agent
> which has been previously removed by the master for being unreachable
> re-registers.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp dfe60ef670edcaefa0c1241df2e2870f650fcf9e 
>   src/tests/master_allocator_tests.cpp 
> 3400d70bb0ba564eac43c4639eee0efd4d8059e6 
>   src/tests/master_tests.cpp 57eae320a7a398527cd3623c89bf67f319a8e955 
>   src/tests/partition_tests.cpp 31ebfe1655438eceae74d72a223df03a9dbd282d 
>   src/tests/persistent_volume_tests.cpp 
> 4aa3c2e8b0f461cd78053707cff8bcb2e6f2b0d7 
>   src/tests/slave_recovery_tests.cpp f14c6ef69eb20a03454c8197df79b572a3c6d050 
>   src/tests/upgrade_tests.cpp 7f434dbba858f636719eec24e92b306b76430c4c 
> 
> 
> Diff: https://reviews.apache.org/r/64098/diff/6/
> 
> 
> Testing
> ---
> 
> with make check
> 
> 
> Thanks,
> 
> Megha Sharma
> 
>

Re: Review Request 63862: Windows: Ported docker_tests.cpp.

[Akash Gupta]

> On Dec. 1, 2017, 2:33 a.m., Andrew Schwartzmeyer wrote:
> > src/tests/containerizer/docker_tests.cpp
> > Lines 62 (patched)
> > 
> >
> > We should probably comment as to why this doesn't use PowerShell 
> > (nanoserver image doesn't have it).
> > 
> > Especially considering that `SLEEP_COMMAND` exists elsewhere, someone 
> > could easily assume they should change it.

There's a nanoserver image at `microsoft/powershell`. I can use that.


> On Dec. 1, 2017, 2:33 a.m., Andrew Schwartzmeyer wrote:
> > src/tests/containerizer/docker_tests.cpp
> > Lines 63 (patched)
> > 
> >
> > Is this the canonical path it should use on Windows? Seems like we 
> > could choose an arbitrary path.

It's an arbitrary path. I just chose it because the Linux test uses 
`/mnt/sandbox/mesos`


- Akash


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


On Dec. 1, 2017, 2:34 a.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63862/
> ---
> 
> (Updated Dec. 1, 2017, 2:34 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Jie Yu, John Kordich, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-7342
> https://issues.apache.org/jira/browse/MESOS-7342
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ported the disabled tests to run on Windows. The following tests
> could not be ported due to Windows platform limitations and remain
> diabled:
>   - ROOT_DOCKER_MountRelativeContainerPath (can't mount volumes inside
> sandbox).
>   - ROOT_DOCKER_NVIDIA_GPU_DeviceAllow (no GPU container support).
>   - ROOT_DOCKER_NVIDIA_GPU_InspectDevices (no GPU container support).
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_tests.cpp 
> 5cabf5a0b0164f9923008677c58806c8931cbc8d 
> 
> 
> Diff: https://reviews.apache.org/r/63862/diff/2/
> 
> 
> Testing
> ---
> 
> Windows mesos-test:
> [==] 754 tests from 77 test cases ran. (270586 ms total)
> [  PASSED  ] 754 tests.
> 
> 
> Windows DockerTest only:
> [==] 11 tests from 1 test case ran. (85617 ms total)
> [  PASSED  ] 11 tests.
> 
> Linux DockerTest (only 12 tests instead of 14, because I don't have Nvidia 
> GPU):
> [==] 12 tests from 1 test case ran. (12413 ms total)
> [  PASSED  ] 12 tests.
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 63860: Windows: Mapped the Docker network info types.

[Akash Gupta]

> On Dec. 1, 2017, 1:47 a.m., Jie Yu wrote:
> > src/docker/docker.cpp
> > Lines 742-743 (original), 742-752 (patched)
> > 
> >
> > It's weird that user specifies HOST in the API, but we use "nat" 
> > instead.
> > 
> > Why can't we use transparent? I don't quite get that from the comments.
> 
> Akash Gupta wrote:
> It's not a default network on Windows like it is on Linux. If you do a 
> `docker network ls` on a fresh Windows box, you see a `nat` network and a 
> `none` network. Transparent is the network driver type, so to use it, you 
> need to create an user defined network by doing `docker network create -d 
> transparent ` and then to use it, you do `docker run 
> --network= ...`. I agree that using transparent would make more 
> sense, but we would have to make the agent create the network and pass that 
> in to the executor.

The real issue is that the API is Linux specific, since `HOST` and `BRIDGE` are 
linux only and `NAT` isn't defined, so a Windows user specifying `HOST` doesn't 
make sense. I think the best solution is to add `NAT` to the protobuf and then 
reword the docs to say that if the network mode is not given, then `HOST` and 
`BRIDGE` will be chosen for Linux and Windows respectively. I'm not sure if you 
can have different default settings for the protobuf, but we could have an 
undocumented `HOST -> NAT` conversion, since `HOST` mode isn't valid on Windows.


- Akash


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


On Nov. 27, 2017, 5:37 p.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63860/
> ---
> 
> (Updated Nov. 27, 2017, 5:37 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer and John Kordich.
> 
> 
> Bugs: MESOS-7342
> https://issues.apache.org/jira/browse/MESOS-7342
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The current Network enum in DockerInfo is specific to Linux containers.
> Instead of supporting {host, bridge, none, user} networks, Windows
> docker supports {nat, none, user} networks. Now, if the host or bridge
> network type is sent to the Windows agent, it will be internally
> converted to nat.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 722a54ad113fc4e2bb22a8f08e307ab38d5fbfed 
> 
> 
> Diff: https://reviews.apache.org/r/63860/diff/3/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/63862/ for test results.
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 63830: Fixed 'NoOffersWithAllRolesSuppressed' test.

[Jiang Yan Xu]

> On Nov. 21, 2017, 9:53 a.m., Alexander Rukletsov wrote:
> > src/tests/scheduler_tests.cpp
> > Lines 1515-1516 (original), 1534-1535 (patched)
> > 
> >
> > `.WillRepeatedly(Return());`
> 
> Jiang Yan Xu wrote:
> There's only one agent with no tasks launched so I don't expect 
> additional offers?
> 
> Alexander Rukletsov wrote:
> I see it in a different way. The default behaviour is ignore subsequent 
> offers and only if you need to verify that in some particular case no more 
> offers will be made, you should omit `.WillRepeatedly(Return());`. In this 
> case what you want to check is that "scheduler starts getting offers" saying 
> nothing about its quantity or periodicity.

Oh ok, but if I am intentionally testing no more offers will be made I'll 
probably need to settle the clock etc to prevent race conditions (and comment 
as such).

Are you suggesting that when writing tests we should by default always add 
`.WillRepeatedly(Return());`? I am not sure if I have seen this being adopted 
and it seems to result in necessary code.

So my approach has been that, if the test is pretty deterministic, write the 
test asserting the precise case. If there are certain unpredicable race 
condidtions (maybe not worth preventing because you don't care), then use these 
`WillRepeatedly // Ignore because don't care` calls.

Reasonable? :)


- Jiang Yan


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


On Nov. 21, 2017, 9:46 a.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63830/
> ---
> 
> (Updated Nov. 21, 2017, 9:46 a.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-7996
> https://issues.apache.org/jira/browse/MESOS-7996
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The previous iteration of the test failed to reveal the bug in MESOS-8200 due 
> to a race condition.
> 
> 
> Diffs
> -
> 
>   src/tests/scheduler_tests.cpp 45fc9c0cfccdb22c2e3e8d5de30c04575814a0e9 
> 
> 
> Diff: https://reviews.apache.org/r/63830/diff/1/
> 
> 
> Testing
> ---
> 
> As expected, this test would reliably fail without /r/63741/ with the 
> following
> 
> ```
> I1115 07:48:28.536093 3117031 scheduler.cpp:741] Enqueuing event SUBSCRIBED 
> received from http://17.121.128.13:38423/master/api/v1/scheduler
> I1115 07:48:28.536224 3117031 scheduler.cpp:741] Enqueuing event HEARTBEAT 
> received from http://17.121.128.13:38423/master/api/v1/scheduler
> I1115 07:48:28.536244 3117029 master.cpp:8115] Sending 1 offers to framework 
> 560bd0e1-8ab7-4e7e-be7a-54cb5f399531- (default)
> I1115 07:48:28.536650 3117029 scheduler.cpp:741] Enqueuing event OFFERS 
> received from http://17.121.128.13:38423/master/api/v1/scheduler
> ../../src/tests/scheduler_tests.cpp:1497: Failure
> Mock function called more times than expected - returning directly.
> Function call: offers(0x7fffabd197a0, @0x7f7d8c005460 48-byte object 
> <50-98 0B-B0 7D-7F 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 
> 00-00 00-00 00-00 00-00 01-00 00-00 04-00 00-00 A0-54 00-8C 7D-7F 00-00>)
>  Expected: to be never called
>Actual: called once - over-saturated and active
> ```
> 
> The test passes with /r/63741/.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 64250: Added new reasons task status update.

[Jiang Yan Xu]

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


Fix it, then Ship it!





docs/task-state-reasons.md
Lines 476 (patched)


What is the `/written` part about? `overwritten` covers all cases?



docs/task-state-reasons.md
Lines 477 (patched)


Given the discussion in /r/64098/, here perhaps add another `Note: Due to 
garbage collection of the unreachable and gone agents in the registry, Mesos 
also sends such status updates for agents unknown to the master`.



docs/task-state-reasons.md
Lines 478 (patched)


Should there be a ``?

Also when it involves doc changes, could you in the testing done section 
mention that you have used a markdown viewer to review the doc change?


- Jiang Yan Xu


On Dec. 1, 2017, 1:06 p.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64250/
> ---
> 
> (Updated Dec. 1, 2017, 1:06 p.m.)
> 
> 
> Review request for mesos, Ilya Pronin, James Peach, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added new reasons `REASON_SLAVE_REREGISTERED` and
> `REASON_AGENT_REREGISTERED` for v0 and v1 task status update.
> The new reason will be used when master starts to send status
> update during the re-registeration of an agent which was
> previosuly removed by the master because of being unreachable.
> 
> 
> Diffs
> -
> 
>   docs/task-state-reasons.md 07f7927e28fe5624a6c9b74de731483aaa729ca3 
>   include/mesos/mesos.proto 11089b7d6f48f001ae64e8b0c67a2732684aa2fa 
>   include/mesos/v1/mesos.proto c496da927903f472c78d455b7fdf886d8fb692c4 
> 
> 
> Diff: https://reviews.apache.org/r/64250/diff/2/
> 
> 
> Testing
> ---
> 
> with make check
> 
> 
> Thanks,
> 
> Megha Sharma
> 
>

Re: Review Request 64250: Added new reasons task status update.

[Ilya Pronin]

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



Looks good. I just have a small suggestion about the doc.


docs/task-state-reasons.md
Lines 474-477 (patched)


I don't quite follow this note. A modified copy of which update? Should we 
just say that these updates reflect the states of the tasks reported by the 
agent upon its re-registration?


- Ilya Pronin


On Dec. 1, 2017, 1:06 p.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64250/
> ---
> 
> (Updated Dec. 1, 2017, 1:06 p.m.)
> 
> 
> Review request for mesos, Ilya Pronin, James Peach, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added new reasons `REASON_SLAVE_REREGISTERED` and
> `REASON_AGENT_REREGISTERED` for v0 and v1 task status update.
> The new reason will be used when master starts to send status
> update during the re-registeration of an agent which was
> previosuly removed by the master because of being unreachable.
> 
> 
> Diffs
> -
> 
>   docs/task-state-reasons.md 07f7927e28fe5624a6c9b74de731483aaa729ca3 
>   include/mesos/mesos.proto 11089b7d6f48f001ae64e8b0c67a2732684aa2fa 
>   include/mesos/v1/mesos.proto c496da927903f472c78d455b7fdf886d8fb692c4 
> 
> 
> Diff: https://reviews.apache.org/r/64250/diff/2/
> 
> 
> Testing
> ---
> 
> with make check
> 
> 
> Thanks,
> 
> Megha Sharma
> 
>

Re: Review Request 64257: Included `stout/os/realpath.hpp` where `os::realpath()` is used.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Dec. 1, 2017, 8:11 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64257/
> ---
> 
> (Updated Dec. 1, 2017, 8:11 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Included `stout/os/realpath.hpp` where `os::realpath()` is used.
> 
> 
> Diffs
> -
> 
>   src/cli/mesos.cpp b347f395712da15729563e23b4a05c26434895ac 
>   src/csi/paths.cpp 6caefc5af8ed94a9d4ea4a325e94b71883ac622d 
>   src/examples/balloon_framework.cpp 0136344c160ea6aeacc57e61382640689a77ee1a 
>   src/examples/long_lived_framework.cpp 
> e6742159108f9eae4f2548bb84d6495309737d9c 
>   src/examples/test_framework.cpp c6a293ed5f88bf4d73f23d97cbf9ffdd8b941f1a 
>   src/examples/test_http_framework.cpp 
> 373035837f51e818629b1588d2ab8803c656f527 
>   src/files/files.cpp fbff2078312ea31bb120f8ba7584b53b4a607609 
>   src/linux/cgroups.cpp 0a7bb89567d4b3ecee7c4f9a259368fb4eae7f34 
>   src/linux/systemd.cpp 6318f48fc3c56d2414a8a50fd2d9debe74fddeb9 
>   src/slave/containerizer/fetcher.cpp 
> 8c4b7e6baefba116cd63c2275c90793a31ae12ec 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
> 134ae5d545ba9fa49f47e3f1afbe6cce3580c1c8 
>   src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
> f40ded73824d42117951d05067ec89131f4e9b34 
>   src/slave/containerizer/mesos/isolators/filesystem/posix.cpp 
> 270d2aa6e06f323bfb6eee3b703a24a600a55871 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> 1c207009b9d718093eae03b411813104ea34a9d7 
>   src/slave/containerizer/mesos/isolators/volume/host_path.cpp 
> 409fc148dcb89dfec18d77c5381ad547337eec8a 
>   src/slave/containerizer/mesos/launch.cpp 
> b1065be0d12934e9b10feadb7d1b82af8ad7cebc 
>   src/slave/containerizer/mesos/provisioner/appc/store.cpp 
> 9e65990c71e991247c35a9b84b69274e113fad6a 
>   src/slave/containerizer/mesos/provisioner/backends/overlay.cpp 
> 3149220316b901c0986d8546504d8ee78bec7181 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> 81d2f93673e890f135e87ad86f30dfa16ea683ae 
>   src/slave/slave.cpp 3896e432fe4b40a458e75537ccfcdd689cdf3f43 
>   src/slave/state.cpp d49c38eada220c379f88acadb3522b1a112ab051 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 9c4b685654acbdece730d1663aa8d54706763a21 
>   src/tests/containerizer/rootfs.cpp fe9252dff859c2b08f609140ead2ef123844fbed 
>   src/tests/gc_tests.cpp 6f055b5eb2a201401ca7dde93275b2d28a886a3c 
>   src/tests/script.cpp 8d40e01da005cb05e7804f0b3975e3e0edb8f3bd 
> 
> 
> Diff: https://reviews.apache.org/r/64257/diff/1/
> 
> 
> Testing
> ---
> 
> See end of chain.
> 
> This is the `mesos` portion.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 64258: Included `stout/os/permissions.hpp` for the secret generator.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Dec. 1, 2017, 8:13 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64258/
> ---
> 
> (Updated Dec. 1, 2017, 8:13 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `os::Permissions` started being used in these files in `e1207f9d`,
> but not included. On Windows, no other header included it either, and so
> the SSL configuration of the build broke.
> 
> 
> Diffs
> -
> 
>   src/slave/main.cpp abe48fdf56ba356d757d8c6e97d01ae3b6cfa0a3 
>   src/tests/cluster.cpp b530133fa77572f00ef8a945d3466a27eb95ceab 
> 
> 
> Diff: https://reviews.apache.org/r/64258/diff/1/
> 
> 
> Testing
> ---
> 
> `make check` on CentOS 7
> 
> Built and ran tests on Windows with `-DENABLE_SSL=ON`. Verified 
> `libprocess-tests` SSLTest suite passed.
> 
> Note that without this chain, the CI builds are currently broken.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 63021: Added `getService()` function to launch CSI plugins.

[Jie Yu]

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




src/resource_provider/storage/provider.cpp
Lines 188 (patched)


Can you add a TODO here? This is a bit hacky way to get the v1 operator API 
endpoint.


- Jie Yu


On Dec. 1, 2017, 1:36 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63021/
> ---
> 
> (Updated Dec. 1, 2017, 1:36 a.m.)
> 
> 
> Review request for mesos, James DeFelice, Jie Yu, Joseph Wu, and Jan Schlicht.
> 
> 
> Bugs: MESOS-8032
> https://issues.apache.org/jira/browse/MESOS-8032
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `getService()` method first checks if there is already a container
> daemon for the specified plugin component, and creates a new one if not.
> The post-start hook for the container daemon will call `connect()` to
> wait for the endpoint socket file to appear and connect to it, then
> set up the corresponding promise of CSI client. The post-stop hook will
> remove the socket file and create a new promise for the next call to the
> post-start hook to set it up.
> 
> 
> Diffs
> -
> 
>   include/mesos/type_utils.hpp a348c7df083324602d25f40069ad49e29f7918a5 
>   src/common/type_utils.cpp 3657d55ab2a052b5c659022e3fc264e748429d54 
>   src/resource_provider/storage/provider.cpp 
> f586afc256fbcb2f2bf2451ffb7e7ba1d59cfa78 
> 
> 
> Diff: https://reviews.apache.org/r/63021/diff/14/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 64044: Recover controller and node services and clean up unused containers.

[Jie Yu]

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


Fix it, then Ship it!





src/resource_provider/storage/provider.cpp
Lines 465 (patched)


Let's use csiPluginContainerInfo


- Jie Yu


On Dec. 1, 2017, 1:38 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64044/
> ---
> 
> (Updated Dec. 1, 2017, 1:38 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, Joseph Wu, and Jan 
> Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The storage local resource provider now scans through the list of plugin
> containers from
> 
>   `/csi///containers/`,
> 
> and kill containers that will no longer in use, then starts up the
> containers for the controller and node services.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/storage/provider.cpp 
> f586afc256fbcb2f2bf2451ffb7e7ba1d59cfa78 
> 
> 
> Diff: https://reviews.apache.org/r/64044/diff/2/
> 
> 
> Testing
> ---
> 
> The `prepareControllerService` and `prepareNodeService` functions are
> moved from r63022.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 64144: Made master acknowledge offer operation updates when 'id' isn't set.

[Greg Mann]

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




src/master/master.cpp
Lines 7500-7502 (patched)


Remove NONE case here.


- Greg Mann


On Nov. 29, 2017, 5:47 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64144/
> ---
> 
> (Updated Nov. 29, 2017, 5:47 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Gaston Kleiman, Jie Yu, and Jan 
> Schlicht.
> 
> 
> Bugs: MESOS-8193
> https://issues.apache.org/jira/browse/MESOS-8193
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When a framework does not request feedback about an operation,
> the master should acknowledge offer operation status updates
> to the agent so that the updates are not retried.
> 
> 
> Diffs
> -
> 
>   src/common/protobuf_utils.hpp 6f991e86e46512d5a2bc4e67e160189fccb77f6a 
>   src/common/protobuf_utils.cpp c0ff306ae6c16cbba6fd08469b639b9f906c672b 
>   src/master/master.cpp 700e12433b0b66efc3f5dd296711c0f203a13144 
> 
> 
> Diff: https://reviews.apache.org/r/64144/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 63831: Fixed a bug that removed the suppressed framework from sorter.

[Jiang Yan Xu]

> On Nov. 21, 2017, 10:27 a.m., Alexander Rukletsov wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 430-437 (original), 431-438 (patched)
> > 
> >
> > Do we need a lambda here?
> 
> Jiang Yan Xu wrote:
> I kept the original use of lambdas. 
> 
> I think the benefit of labmdas here is more about readability: compared 
> to 
> 
> ```
> set removedRoles = oldRoles;
> foreach (const string& role, newRoles) {
>   result.erase(role);
> }
> ```
> 
> The constness and construction of the variable is clearly isolated into a 
> small block in a long method with many similar variables/code blocks.
> 
> However as pointed out by the TODO, we should probably just implement a 
> set difference operator so all these become one-liners. I'll do it later.
> 
> Alexander Rukletsov wrote:
> Sounds like a wrong tool for the job and hence confusing. I would have 
> used an explicit scope then. But maybe this is just me. I'll ask MPark for 
> the reasoning here. Since it has been here before, it's fine to leave it for 
> now. Regading introducing the set operator, FYI: 
> https://reviews.apache.org/r/57110/#comment239761

Oh ok, thanks for the pointer!


- Jiang Yan


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


On Nov. 27, 2017, 4:45 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63831/
> ---
> 
> (Updated Nov. 27, 2017, 4:45 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-8223
> https://issues.apache.org/jira/browse/MESOS-8223
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In `updateFramework`, we currently treat frameworks suppressing a role the 
> same way as frameworks moving off a role and this could lead to the framework 
> being removed from the framework sorter, which is unexpected. We should only 
> remove the framework from a framework sorter when it's moving away from the 
> corresponding role.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 5ce9ceaa3a5f84a1e076d45448863c418531cc2b 
>   src/tests/scheduler_tests.cpp 45fc9c0cfccdb22c2e3e8d5de30c04575814a0e9 
> 
> 
> Diff: https://reviews.apache.org/r/63831/diff/2/
> 
> 
> Testing
> ---
> 
> make check.
> 
> Added a test.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 64272: Improved Windows isolators with `struct Info` abstraction.

[Andrew Schwartzmeyer]

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

(Updated Dec. 1, 2017, 3:37 p.m.)


Review request for mesos, Akash Gupta, Jie Yu, and Joseph Wu.


Changes
---

Added check in update that `pid.isSome()`.


Repository: mesos


Description
---

Replaced the two maps of `ContainerId -> pid` and `ContainerId -> limit`
with a mapping of `ContainerId -> struct Info { pid, limit }`. This
abstraction correctly ties the the `pid` and cpu/mem `limit` together.
Notably this fixes a subtle bug in `prepare` so that it can no longer be
called multiple times if a resource limit hadn't been set.

Furthermore, this patch imports all the types used in the source files
to enchance code conciseness, and then reformatted with `clang-format`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/windows/cpu.hpp 
8672b5943ff305b929dd7514581df4515dff1f50 
  src/slave/containerizer/mesos/isolators/windows/cpu.cpp 
e9f4daccdd507312998984cad231c3c7d310af2b 
  src/slave/containerizer/mesos/isolators/windows/mem.hpp 
24426f7219bc7831c1682153d7c9c1a6e502e3d6 
  src/slave/containerizer/mesos/isolators/windows/mem.cpp 
5abcb4edfa350e1de5b34bfd43d4e961b7208799 


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

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


Testing
---

`make check` on CentOS 7 (probably pointless)

`ctest` on Windows 10. All tests passed. (Building again after rebase.)


Thanks,

Andrew Schwartzmeyer

Re: Review Request 64226: Added a `ns::supported` convenience API.

[James Peach]

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

(Updated Dec. 1, 2017, 11:46 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Address review feedback.


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


Repository: mesos


Description (updated)
---

Added a `ns::supported` convenience API which directly expresses the
intent to probe whether a specific Linux namespace is supported on
the running kernel and takes care of kernel versioning special cases.

This API subsumes the previous usages of `ns::namespaces`, so that API
is now used only retained as an internal helper.


Diffs (updated)
-

  src/linux/ns.hpp e24d79a41eefcade343b2825b5a758d7d30a5f91 
  src/linux/ns.cpp 5e2df1ed56432c6c1bfa04a31dba7f9f547d6139 
  src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp 
2d89d598d24e3bcf01d652ce3f586c9e3ccfc20b 
  src/slave/containerizer/mesos/isolators/namespaces/pid.cpp 
4f8253b58018581e022eb1832b9b07703cbd318d 
  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
d60052e9e1ccdfaaac6c0db6acfdae325cea6a9e 
  src/tests/containerizer/ns_tests.cpp 61adf8525b1df1a70d7fd9c747d2106630a7627d 
  src/tests/containerizer/setns_test_helper.cpp 
045caf684baf6e724fa5f99b6cc3feeb87817ba3 


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

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


Testing
---

make check (Fedora 27)


Thanks,

James Peach

Re: Review Request 64195: Updated the tests to use MULTI_ROLE frameworks by default.

[Benjamin Mahler]

> On Dec. 1, 2017, 1:16 a.m., Michael Park wrote:
> > src/tests/slave_tests.cpp
> > Line 860 (original), 860 (patched)
> > 
> >
> > Huh... so these used to be an empty string?

These used to be `"*"` (the default for the `role` field). Now the default 
framework info has `roles = ["*"]`.


- Benjamin


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


On Nov. 30, 2017, 1:45 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64195/
> ---
> 
> (Updated Nov. 30, 2017, 1:45 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-8237
> https://issues.apache.org/jira/browse/MESOS-8237
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Now that we strip the `Resource.allocation_info` for non-MULTI_ROLE
> schedulers, it's simpler to default the tests to use the MULTI_ROLE
> capability, since we've already updated the majority of the tests
> to be aware of `Resource.allocation_info`.
> 
> 
> Diffs
> -
> 
>   src/examples/balloon_framework.cpp 0136344c160ea6aeacc57e61382640689a77ee1a 
>   src/examples/disk_full_framework.cpp 
> 28f71c35b32b46c7bf2cce9fe971266f7bd73c8a 
>   src/examples/dynamic_reservation_framework.cpp 
> 5ee3867228a70c8fa0b4b5092fb44cb556a670e0 
>   src/examples/long_lived_framework.cpp 
> e6742159108f9eae4f2548bb84d6495309737d9c 
>   src/examples/no_executor_framework.cpp 
> fd920f58f5ccb903127e9c6b112be30f04baa069 
>   src/examples/persistent_volume_framework.cpp 
> 674d58a9e6fa34733f765d2925cedafeaab1eecd 
>   src/examples/test_framework.cpp c6a293ed5f88bf4d73f23d97cbf9ffdd8b941f1a 
>   src/examples/test_http_framework.cpp 
> 373035837f51e818629b1588d2ab8803c656f527 
>   src/tests/api_tests.cpp 66cb059c96ff9ecda594e13de9e5dc3909f3 
>   src/tests/containerizer/cgroups_isolator_tests.cpp 
> 421a72f1694a1a5c5e1bf4c525361dbd670c68af 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 7a42bb93508abf9020d0bcc5b3daa2f6834be2e3 
>   src/tests/containerizer/linux_filesystem_isolator_tests.cpp 
> 655f9f81f4bc55a85d7c5e27609a016254793380 
>   src/tests/default_executor_tests.cpp 
> 04d9e1b9d4682c3bb5355253e3a8e5b06d78e98f 
>   src/tests/disk_quota_tests.cpp fc297995c42821aa4b0a7719a6a19c3bbf65db47 
>   src/tests/fault_tolerance_tests.cpp 
> 33a2220b4b4f019799db7b2482ce73b3454fa58e 
>   src/tests/hook_tests.cpp 2e58d1133343a96b67d0816a54ff8eb874522ba3 
>   src/tests/master_allocator_tests.cpp 
> 3400d70bb0ba564eac43c4639eee0efd4d8059e6 
>   src/tests/master_authorization_tests.cpp 
> eff97f155e7472a0cd5994408ed73474392593ad 
>   src/tests/master_quota_tests.cpp 058f6d24da50cbf3c28b091afa88f634a8102b62 
>   src/tests/master_tests.cpp 01f45a9bb6378c5fded31bf58fbcad93fe7ed719 
>   src/tests/master_validation_tests.cpp 
> 0e1c8b490ebe10bc50b8b6b530fa85128b967584 
>   src/tests/mesos.hpp f02c7c6962c9d0bee57712d8aad9997582c7404b 
>   src/tests/oversubscription_tests.cpp 
> 2f403d3c9d2df7d5425b335781e8b62153854ef3 
>   src/tests/partition_tests.cpp e49c474167076b4136a161ed29b11db9a13455a7 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 883192d2badf8af87c95bc9a5fc3e711cd240193 
>   src/tests/persistent_volume_tests.cpp 
> b11d26e45c184cf3f0623629b898f67dc95c4fa0 
>   src/tests/reservation_endpoints_tests.cpp 
> a96cc714fd40421a7db9a4c0debf71546daf5d4c 
>   src/tests/reservation_tests.cpp fa375cd45e4c68551801521176632a56acd6d0ea 
>   src/tests/resource_provider_manager_tests.cpp 
> 0b7c4ad6bb0052847b884959e3171cd7ab382b45 
>   src/tests/role_tests.cpp 084555ae70b80330cdc8729cd62f8baddf291a01 
>   src/tests/scheduler_http_api_tests.cpp 
> 80e52fb22dbba4a0703f1b192205acaff89f883b 
>   src/tests/scheduler_tests.cpp 45fc9c0cfccdb22c2e3e8d5de30c04575814a0e9 
>   src/tests/slave_authorization_tests.cpp 
> 11fd0d4e35523eca23a9603ecef0c9d0b65dde38 
>   src/tests/slave_recovery_tests.cpp f14c6ef69eb20a03454c8197df79b572a3c6d050 
>   src/tests/slave_tests.cpp a2274b691cf94b003c4bc15450d176a9c73517d5 
>   src/tests/upgrade_tests.cpp 7f434dbba858f636719eec24e92b306b76430c4c 
> 
> 
> Diff: https://reviews.apache.org/r/64195/diff/1/
> 
> 
> Testing
> ---
> 
> Ran the tests, including with root.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 64250: Added new reasons task status update.

[Ilya Pronin]

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


Ship it!




Ship It!

- Ilya Pronin


On Dec. 1, 2017, 1:06 p.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64250/
> ---
> 
> (Updated Dec. 1, 2017, 1:06 p.m.)
> 
> 
> Review request for mesos, Ilya Pronin, James Peach, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added new reasons `REASON_SLAVE_REREGISTERED` and
> `REASON_AGENT_REREGISTERED` for v0 and v1 task status update.
> The new reason will be used when master starts to send status
> update during the re-registration of an agent which was
> previously removed by the master because of being unreachable
> or is unknown to the master due to the garbage collection of
> the unreachable and gone agents in the registry.
> 
> 
> Diffs
> -
> 
>   docs/task-state-reasons.md 07f7927e28fe5624a6c9b74de731483aaa729ca3 
>   include/mesos/mesos.proto 11089b7d6f48f001ae64e8b0c67a2732684aa2fa 
>   include/mesos/v1/mesos.proto c496da927903f472c78d455b7fdf886d8fb692c4 
> 
> 
> Diff: https://reviews.apache.org/r/64250/diff/5/
> 
> 
> Testing
> ---
> 
> code changes verified with make check and the documents changes with markdown 
> viewer
> 
> 
> Thanks,
> 
> Megha Sharma
> 
>

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

[Megha Sharma]

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

(Updated Dec. 2, 2017, 12:12 a.m.)


Review request for mesos, Ilya Pronin, James Peach, and Jiang Yan Xu.


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


Repository: mesos


Description (updated)
---

Master will send task status updates to frameworks when an agent
which has been previously removed by the master for being unreachable
or is unknown to the master due to the garbage collection of
the unreachable and gone agents in the registry re-registers.


Diffs (updated)
-

  src/master/master.cpp dfe60ef670edcaefa0c1241df2e2870f650fcf9e 
  src/tests/master_allocator_tests.cpp 3400d70bb0ba564eac43c4639eee0efd4d8059e6 
  src/tests/master_tests.cpp 57eae320a7a398527cd3623c89bf67f319a8e955 
  src/tests/partition_tests.cpp 31ebfe1655438eceae74d72a223df03a9dbd282d 
  src/tests/persistent_volume_tests.cpp 
4aa3c2e8b0f461cd78053707cff8bcb2e6f2b0d7 
  src/tests/slave_recovery_tests.cpp f14c6ef69eb20a03454c8197df79b572a3c6d050 
  src/tests/upgrade_tests.cpp 7f434dbba858f636719eec24e92b306b76430c4c 


Diff: https://reviews.apache.org/r/64098/diff/7/

Changes: https://reviews.apache.org/r/64098/diff/6-7/


Testing
---

with make check


Thanks,

Megha Sharma

Re: Review Request 63862: Windows: Ported docker_tests.cpp.

[Andrew Schwartzmeyer]

> On Dec. 1, 2017, 3:25 a.m., Mesos Reviewbot Windows wrote:
> > FAIL: Mesos libprocess-tests failed to build
> > 
> > Reviews applied: `['63859', '63860', '63861', '63862']`
> > 
> > Failed command: `cmake.exe --build . --target libprocess-tests --config 
> > Debug`
> > 
> > All the build artifacts available at: 
> > http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/63862
> > 
> > Relevant logs:
> > 
> > - 
> > [libprocess-tests-build-cmake-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/63862/logs/libprocess-tests-build-cmake-stdout.log):
> > 
> > ```
> >   C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): 
> > warning C4996: 'GetVersionExW': was declared deprecated (compiling source 
> > file 
> > C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\state_machine_tests.cpp) 
> > [C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
> >   C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): 
> > warning C4996: 'GetVersionExW': was declared deprecated (compiling source 
> > file 
> > C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\state_machine_tests.cpp) 
> > [C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
> >   C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(805): 
> > warning C4244: 'argument': conversion from 'ULONG_PTR' to 'pid_t', possible 
> > loss of data (compiling source file 
> > C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\state_machine_tests.cpp) 
> > [C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
> >   C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): 
> > warning C4996: 'GetVersionExW': was declared deprecated (compiling source 
> > file 
> > C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\subprocess_tests.cpp) 
> > [C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
> >   C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): 
> > warning C4996: 'GetVersionExW': was declared deprecated (compiling source 
> > file 
> > C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\subprocess_tests.cpp) 
> > [C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
> >   C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): 
> > warning C4996: 'GetVersionExW': was declared deprecated (compiling source 
> > file C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\system_tests.cpp) 
> > [C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
> >   C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): 
> > warning C4996: 'GetVersionExW': was declared deprecated (compiling source 
> > file C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\system_tests.cpp) 
> > [C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
> >   C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(805): 
> > warning C4244: 'argument': conversion from 'ULONG_PTR' to 'pid_t', possible 
> > loss of data (compiling source file 
> > C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\subprocess_tests.cpp) 
> > [C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
> >   C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(805): 
> > warning C4244: 'argument': conversion from 'ULONG_PTR' to 'pid_t', possible 
> > loss of data (compiling source file 
> > C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\system_tests.cpp) 
> > [C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
> >   C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): 
> > warning C4996: 'GetVersionExW': was declared deprecated (compiling source 
> > file C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\time_tests.cpp) 
> > [C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
> >   C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): 
> > warning C4996: 'GetVersionExW': was declared deprecated (compiling source 
> > file C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\time_tests.cpp) 
> > [C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
> >   C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(805): 
> > warning C4244: 'argument': conversion from 'ULONG_PTR' to 'pid_t', possible 
> > loss of data (compiling source file 
> > C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\time_tests.cpp) 
> > [C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
> >   C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): 
> > warning C4996: 'GetVersionExW': was declared deprecated (compiling source 
> > file C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\ssl_tests.cpp) 
> > [C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
> >   C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): 
> > warning C4996: 'GetVersionExW': was declared deprecated (compiling source 
> > file 

Re: Review Request 64075: Added the `ResourceProviderState` protobuf for resource providers.

[Jie Yu]

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


Fix it, then Ship it!





src/resource_provider/storage/provider.cpp
Lines 431-433 (patched)


Let's just return Failure here



src/resource_provider/storage/provider.cpp
Lines 442 (patched)


Please add some comments about why ignoring the NOne() case here. Might 
worth printing a warning or info here.



src/resource_provider/storage/provider.cpp
Line 414 (original), 445 (patched)


Can you handle None() case here? or at least add some comment about why 
it's ok to ignore it?


- Jie Yu


On Dec. 1, 2017, 1:47 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64075/
> ---
> 
> (Updated Dec. 1, 2017, 1:47 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, Joseph Wu, and Jan 
> Schlicht.
> 
> 
> Bugs: MESOS-8265
> https://issues.apache.org/jira/browse/MESOS-8265
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `ResourceProviderState` protobuf includes the list of pending offer
> operations, the total resources, and the current resource version UUID.
> Note that the pending operations do not includes completed operations
> that have not been acknowledged yet.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 4a3b728dc2cbfaf4b10068562778a87a6b331238 
>   src/resource_provider/registry.hpp 048cd6b9327de5396c7a56637f23c08694df1b58 
>   src/resource_provider/state.hpp PRE-CREATION 
>   src/resource_provider/state.proto PRE-CREATION 
>   src/resource_provider/storage/provider.cpp 
> f586afc256fbcb2f2bf2451ffb7e7ba1d59cfa78 
> 
> 
> Diff: https://reviews.apache.org/r/64075/diff/5/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 64075: Added the `ResourceProviderState` protobuf for resource providers.

[Jie Yu]

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


Fix it, then Ship it!





src/Makefile.am
Lines 935 (patched)


We should update CMake for this


- Jie Yu


On Dec. 1, 2017, 1:47 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64075/
> ---
> 
> (Updated Dec. 1, 2017, 1:47 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, Joseph Wu, and Jan 
> Schlicht.
> 
> 
> Bugs: MESOS-8265
> https://issues.apache.org/jira/browse/MESOS-8265
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `ResourceProviderState` protobuf includes the list of pending offer
> operations, the total resources, and the current resource version UUID.
> Note that the pending operations do not includes completed operations
> that have not been acknowledged yet.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 4a3b728dc2cbfaf4b10068562778a87a6b331238 
>   src/resource_provider/registry.hpp 048cd6b9327de5396c7a56637f23c08694df1b58 
>   src/resource_provider/state.hpp PRE-CREATION 
>   src/resource_provider/state.proto PRE-CREATION 
>   src/resource_provider/storage/provider.cpp 
> f586afc256fbcb2f2bf2451ffb7e7ba1d59cfa78 
> 
> 
> Diff: https://reviews.apache.org/r/64075/diff/5/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 64250: Added new reasons task status update.

[Megha Sharma]

> On Dec. 1, 2017, 9:55 p.m., Ilya Pronin wrote:
> > docs/task-state-reasons.md
> > Lines 474-477 (patched)
> > 
> >
> > I don't quite follow this note. A modified copy of which update? Should 
> > we just say that these updates reflect the states of the tasks reported by 
> > the agent upon its re-registration?
> 
> Megha Sharma wrote:
> Here, we are saying that master takes the actual state reported by the 
> agent and enriches it with reason and message before sending to the 
> framework. But I am open to changing it if you or Yan feel it doesn't convery 
> the idea.
> 
> Jiang Yan Xu wrote:
> I was suggesting it because this status is generated by the master the 
> same way they are generated upon reconciliation.
> 
> For `REASON_RECONCILIATION` in the same doc there is this note:
> 
> ```
> Note: Status updates with this reason are not the original ones, but 
> rather a modified copy that is re-sent from the master. In particular, the 
> original data and message fields are erased and the original reason field is 
> overwritten by REASON_RECONCILIATION .
> ```
> 
> It is the same for `REASON_SLAVE_REREGISTERED`. 
> 
> 
> Megha I just noticed that you didn't mention the erasure of `data` and 
> `message` fields, but perhaps this is not worth repeating. I was originally 
> suggesting refering to `REASON_RECONCILIATION` for details: something like: 
> `Status updates with this reason are a modified copies re-sent by the master. 
> See comments for REASON_RECONCILIATION.`
> 
> Would this be clearer?
> 
> "reflect the states of the tasks reported by the agent upon its 
> re-registration" this sentence is good too.
> 
> Ilya Pronin wrote:
> Oh, now I got where this came from :) Yeah, it would be clearer if there 
> was a mention that new states come from the agent re-registration message and 
> a reference to `REASON_RECONCILIATION` description. Or it should be enough to 
> jsut say that these are produced by the master and reflect the states 
> reported by the agent during re-registration.

PTAL, updated the description based on the discussion.


- Megha


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


On Dec. 1, 2017, 9:06 p.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64250/
> ---
> 
> (Updated Dec. 1, 2017, 9:06 p.m.)
> 
> 
> Review request for mesos, Ilya Pronin, James Peach, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added new reasons `REASON_SLAVE_REREGISTERED` and
> `REASON_AGENT_REREGISTERED` for v0 and v1 task status update.
> The new reason will be used when master starts to send status
> update during the re-registration of an agent which was
> previously removed by the master because of being unreachable
> or is unknown to the master due to the garbage collection of
> the unreachable and gone agents in the registry.
> 
> 
> Diffs
> -
> 
>   docs/task-state-reasons.md 07f7927e28fe5624a6c9b74de731483aaa729ca3 
>   include/mesos/mesos.proto 11089b7d6f48f001ae64e8b0c67a2732684aa2fa 
>   include/mesos/v1/mesos.proto c496da927903f472c78d455b7fdf886d8fb692c4 
> 
> 
> Diff: https://reviews.apache.org/r/64250/diff/5/
> 
> 
> Testing
> ---
> 
> code changes verified with make check and the documents changes with markdown 
> viewer
> 
> 
> Thanks,
> 
> Megha Sharma
> 
>

Review Request 64272: Improved Windows isolators with `struct Info` abstraction.

[Andrew Schwartzmeyer]

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

Review request for mesos, Akash Gupta, Jie Yu, and Joseph Wu.


Repository: mesos


Description
---

Replaced the two maps of `ContainerId -> pid` and `ContainerId -> limit`
with a mapping of `ContainerId -> struct Info { pid, limit }`. This
abstraction correctly ties the the `pid` and cpu/mem `limit` together.
Notably this fixes a subtle bug in `prepare` so that it can no longer be
called multiple times if a resource limit hadn't been set.

Furthermore, this patch imports all the types used in the source files
to enchance code conciseness, and then reformatted with `clang-format`.


Diffs
-

  src/slave/containerizer/mesos/isolators/windows/cpu.hpp 
8672b5943ff305b929dd7514581df4515dff1f50 
  src/slave/containerizer/mesos/isolators/windows/cpu.cpp 
e9f4daccdd507312998984cad231c3c7d310af2b 
  src/slave/containerizer/mesos/isolators/windows/mem.hpp 
24426f7219bc7831c1682153d7c9c1a6e502e3d6 
  src/slave/containerizer/mesos/isolators/windows/mem.cpp 
5abcb4edfa350e1de5b34bfd43d4e961b7208799 


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


Testing
---

`make check` on CentOS 7 (probably pointless)

`ctest` on Windows 10. All tests passed. (Building again after rebase.)


Thanks,

Andrew Schwartzmeyer