Re: Review Request 62900: Enabled protobuf arenas code generation.

[Gilbert Song]

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



Do we need corresponding changes on v1/***.proto?

- Gilbert Song


On Oct. 11, 2017, 10:34 a.m., Dmitry Zhuk wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62900/
> ---
> 
> (Updated Oct. 11, 2017, 10:34 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-6971
> https://issues.apache.org/jira/browse/MESOS-6971
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This adds `option cc_enable_arenas = true;` to relevant proto files.
> 
> 
> Diffs
> -
> 
>   include/mesos/authentication/authentication.proto 
> 3869d6b117ead633dbc6ae23aa5059d13c9ff70d 
>   include/mesos/master/master.proto 79be497984cdecf91850264734243af29c060d85 
>   include/mesos/mesos.proto 830985a3265b7c104d8fdc50749c395d98f5f3c8 
>   include/mesos/scheduler/scheduler.proto 
> 0528a7ea2df0aa4bf6d4405274943e9858998812 
>   src/messages/log.proto ca740bd011147f8f48cc5dfcacefa5fdbd95ccd0 
>   src/messages/messages.proto afca6d161c2c2049eeffe4189d86b460b345dd51 
> 
> 
> Diff: https://reviews.apache.org/r/62900/diff/1/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Dmitry Zhuk
> 
>

Re: Review Request 61172: Added mesos.http and mesos.exceptions for CLI.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 61172 was successfully built and tested.

Reviews applied: `['61172']`

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

- Mesos Reviewbot Windows


On Jan. 18, 2018, 4:25 a.m., Eric Chung wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61172/
> ---
> 
> (Updated Jan. 18, 2018, 4:25 a.m.)
> 
> 
> Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Part of MESOS-7310, this patch adds the mesos.http and mesos.exceptions
> modules, which provides a Resource class and its descendants for
> abstracting away common operations over http connectioins with JSON
> serialization.
> 
> updated patch according to klueska's recommendations
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/bootstrap c84e549e59bc7a07b5b70c24c63bc0c16881e442 
>   src/python/lib/mesos/exceptions.py PRE-CREATION 
>   src/python/lib/mesos/http.py PRE-CREATION 
>   src/python/lib/requirements.in e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
>   src/python/lib/tests/conftest.py PRE-CREATION 
>   src/python/lib/tests/test_exceptions.py PRE-CREATION 
>   src/python/lib/tests/test_http.py PRE-CREATION 
>   src/python/lib/tox.ini 8ad030d2dbf1fb7a04f1eaadb587462b4ef3f054 
>   support/pylint.config f74f553e238553bd6a6c06f4dd888cc5954a33eb 
> 
> 
> Diff: https://reviews.apache.org/r/61172/diff/9/
> 
> 
> Testing
> ---
> 
> install tox
> cd src/python/lib
> tox
> 
> 
> Thanks,
> 
> Eric Chung
> 
>

Re: Review Request 61172: Added mesos.http and mesos.exceptions for CLI.

[Eric Chung]

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

(Updated Jan. 18, 2018, 4:25 a.m.)


Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.


Repository: mesos


Description (updated)
---

Part of MESOS-7310, this patch adds the mesos.http and mesos.exceptions
modules, which provides a Resource class and its descendants for
abstracting away common operations over http connectioins with JSON
serialization.

updated patch according to klueska's recommendations


Diffs (updated)
-

  src/python/cli_new/bootstrap c84e549e59bc7a07b5b70c24c63bc0c16881e442 
  src/python/lib/mesos/exceptions.py PRE-CREATION 
  src/python/lib/mesos/http.py PRE-CREATION 
  src/python/lib/requirements.in e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
  src/python/lib/tests/conftest.py PRE-CREATION 
  src/python/lib/tests/test_exceptions.py PRE-CREATION 
  src/python/lib/tests/test_http.py PRE-CREATION 
  src/python/lib/tox.ini 8ad030d2dbf1fb7a04f1eaadb587462b4ef3f054 
  support/pylint.config f74f553e238553bd6a6c06f4dd888cc5954a33eb 


Diff: https://reviews.apache.org/r/61172/diff/9/

Changes: https://reviews.apache.org/r/61172/diff/8-9/


Testing
---

install tox
cd src/python/lib
tox


Thanks,

Eric Chung

Re: Review Request 61172: Added mesos.http and mesos.exceptions for CLI.

[Eric Chung]

> On Jan. 5, 2018, 3:23 p.m., Armand Grillet wrote:
> > src/python/cli_new/bootstrap
> > Lines 72 (patched)
> > 
> >
> > This file does not exist, is that normal?

it should...
```
~/mesos/src/python/lib$ git blame requirements-test.in
456a0ac3 (Kevin Klues 2017-07-23 16:21:36 -0700 1) coverage
456a0ac3 (Kevin Klues 2017-07-23 16:21:36 -0700 2) mock
456a0ac3 (Kevin Klues 2017-07-23 16:21:36 -0700 3) pytest
456a0ac3 (Kevin Klues 2017-07-23 16:21:36 -0700 4) pytest-cov
```


> On Jan. 5, 2018, 3:23 p.m., Armand Grillet wrote:
> > src/python/lib/mesos/http.py
> > Lines 72 (patched)
> > 
> >
> > We should do that everywhere in the CLI, this is a useful addition. We 
> > could also use [mypy](http://www.mypy-lang.org/).

that looks pretty cool! we could also migrate to python3 which supports type 
annotations


- Eric


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


On Jan. 1, 2018, 1:52 a.m., Eric Chung wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61172/
> ---
> 
> (Updated Jan. 1, 2018, 1:52 a.m.)
> 
> 
> Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Part of MESOS-7310, this patch adds the mesos.http and mesos.exceptions
> modules, which provides a Resource class and its descendants for
> abstracting away common operations over http connectioins with JSON
> serialization.
> 
> updated patch according to klueska's recommendations
> 
> 
> addressed comments
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/bootstrap c84e549e59bc7a07b5b70c24c63bc0c16881e442 
>   src/python/lib/mesos/exceptions.py PRE-CREATION 
>   src/python/lib/mesos/http.py PRE-CREATION 
>   src/python/lib/requirements.in e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
>   src/python/lib/tests/conftest.py PRE-CREATION 
>   src/python/lib/tests/test_exceptions.py PRE-CREATION 
>   src/python/lib/tests/test_http.py PRE-CREATION 
>   src/python/lib/tox.ini 8ad030d2dbf1fb7a04f1eaadb587462b4ef3f054 
>   support/pylint.config f74f553e238553bd6a6c06f4dd888cc5954a33eb 
> 
> 
> Diff: https://reviews.apache.org/r/61172/diff/8/
> 
> 
> Testing
> ---
> 
> install tox
> cd src/python/lib
> tox
> 
> 
> Thanks,
> 
> Eric Chung
> 
>

Re: Review Request 65203: Updated the CHANGELOG for 1.5.0 release.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65203 was successfully built and tested.

Reviews applied: `['65203']`

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

- Mesos Reviewbot Windows


On Jan. 18, 2018, 12:21 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65203/
> ---
> 
> (Updated Jan. 18, 2018, 12:21 a.m.)
> 
> 
> Review request for mesos, Adam B, Anand Mazumdar, Andrew Schwartzmeyer, 
> Benjamin Mahler, Chun-Hung Hsiao, Gaston Kleiman, Greg Mann, Ilya Pronin, Jie 
> Yu, James Peach, Kapil Arya, Joseph Wu, Kevin Klues, Neil Conway, Qian Zhang, 
> Till Toenshoff, Vinod Kone, Jiang Yan Xu, and Zhitao Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated the CHANGELOG for 1.5.0 release.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 078c5d64100d81ce84dbdff4055cdda465e19011 
> 
> 
> Diff: https://reviews.apache.org/r/65203/diff/1/
> 
> 
> Testing
> ---
> 
> N/A.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 65039: Tested reconciliation when operation is dropped en route to agent.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65039 was successfully built and tested.

Reviews applied: `['64992', '64994', '65039']`

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

- Mesos Reviewbot Windows


On Jan. 18, 2018, 1:57 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65039/
> ---
> 
> (Updated Jan. 18, 2018, 1:57 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, Gaston Kleiman, 
> and Jie Yu.
> 
> 
> Bugs: MESOS-8373
> https://issues.apache.org/jira/browse/MESOS-8373
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds
> StorageLocalResourceProviderTest.ROOT_ReconcileDroppedOperation
> in order to verify that reconciliation is performed correctly
> when an operation is dropped on its way from the master to the
> agent.
> 
> 
> Diffs
> -
> 
>   src/tests/storage_local_resource_provider_tests.cpp 
> bbfe95e9818f25fdd5405db3ad2fe355e023f743 
> 
> 
> Diff: https://reviews.apache.org/r/65039/diff/3/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 64998: Added a SLRP test for CSI plugin restart.

[Chun-Hung Hsiao]

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

(Updated Jan. 18, 2018, 2:53 a.m.)


Review request for mesos, Gaston Kleiman, Greg Mann, and Jie Yu.


Changes
---

Addressed Greg's comments and made some changes for consistency.


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


Repository: mesos


Description
---

The test does the same as the `PublishResources` test, but it kills the
CSI plugin contaier between each operation.


Diffs (updated)
-

  src/Makefile.am 191594b9d67f1aa1342efbaad5e7501004332d74 
  src/slave/container_daemon.cpp d74fa5105c61a6cadfb2610790e559d387ca13a0 
  src/slave/container_daemon_process.hpp PRE-CREATION 
  src/tests/storage_local_resource_provider_tests.cpp 
bbfe95e9818f25fdd5405db3ad2fe355e023f743 


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

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


Testing
---

sudo make check


Thanks,

Chun-Hung Hsiao

Re: Review Request 64998: Added a SLRP test for CSI plugin restart.

[Chun-Hung Hsiao]

> On Jan. 17, 2018, 2:34 a.m., Greg Mann wrote:
> > src/tests/storage_local_resource_provider_tests.cpp
> > Lines 1552-1559 (patched)
> > 
> >
> > Could this be racy? Since we're not awaiting on the UpdateSlaveMessage 
> > containing the RP resources, is it possible that the first offer the 
> > scheduler receives will not contain any raw disk?

Offers that does not match the `OffersHaveAnyResource(...)` criteria will be 
declined immediately by Line 1538. But I should use another set of filters 
which a long `refuse_seconds` for declining such offers.


- Chun-Hung


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


On Jan. 10, 2018, 2:33 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64998/
> ---
> 
> (Updated Jan. 10, 2018, 2:33 a.m.)
> 
> 
> Review request for mesos, Gaston Kleiman, Greg Mann, and Jie Yu.
> 
> 
> Bugs: MESOS-8408
> https://issues.apache.org/jira/browse/MESOS-8408
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The test does the same as the `PublishResources` test, but it kills the
> CSI plugin contaier between each operation.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am bfe9eb1609c71c980c11e7cb0149caf5bdd7ab9b 
>   src/slave/container_daemon.cpp d74fa5105c61a6cadfb2610790e559d387ca13a0 
>   src/slave/container_daemon_process.hpp PRE-CREATION 
>   src/tests/storage_local_resource_provider_tests.cpp 
> bbfe95e9818f25fdd5405db3ad2fe355e023f743 
> 
> 
> Diff: https://reviews.apache.org/r/64998/diff/4/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 64857: Updated example frameworks for mesos-local.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 64857 was successfully built and tested.

Reviews applied: `['64846', '64847', '64848', '64849', '64857']`

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

- Mesos Reviewbot Windows


On Jan. 17, 2018, 11:37 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64857/
> ---
> 
> (Updated Jan. 17, 2018, 11:37 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Armand Grillet, Greg Mann, 
> Kapil Arya, and Vinod Kone.
> 
> 
> Bugs: MESOS-8357 and MESOS-8361
> https://issues.apache.org/jira/browse/MESOS-8357
> https://issues.apache.org/jira/browse/MESOS-8361
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Unifies handling of 'local' when supplied for the '--master' flag.
> 
> 
> Diffs
> -
> 
>   src/examples/balloon_framework.cpp 410966e64aa3f146a88c00babaa51b499dd24d36 
>   src/examples/disk_full_framework.cpp 
> d75f769538d17229bdbf332ea7513b1aa8d7c334 
>   src/examples/docker_no_executor_framework.cpp 
> 1fa408d3847d414c23eed8334db8de02e84cf764 
>   src/examples/dynamic_reservation_framework.cpp 
> 538fbe8847a1b1dcbfe48ad0e3678797801a12f5 
>   src/examples/load_generator_framework.cpp 
> 42867992eba8699cab5b70c62a24b87446139406 
>   src/examples/long_lived_framework.cpp 
> 943160d5a42e02cf05fe92999233ce2e792849fd 
>   src/examples/no_executor_framework.cpp 
> 972ef777cec07d1030af208daef69ebe606d071e 
>   src/examples/test_framework.cpp acf1faf1523c8c03483dfafbc8f8e245322527e4 
>   src/examples/test_http_framework.cpp 
> 482f65efc15a7bac52c33a57b2b876191249410a 
> 
> 
> Diff: https://reviews.apache.org/r/64857/diff/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 65039: Tested reconciliation when operation is dropped en route to agent.

[Greg Mann]

> On Jan. 17, 2018, 8:44 a.m., Benjamin Bannier wrote:
> > src/tests/storage_local_resource_provider_tests.cpp
> > Lines 1656 (patched)
> > 
> >
> > Would it make sense to have the whole test run with paused clock?
> 
> Greg Mann wrote:
> I need to leave a comment here on why I do this. Unfortunately, the 
> standalone container running the CSI plugin complicates things a bit when 
> running with the clock paused. The SLRP runs a `loop()` which looks for the 
> CSI container's domain socket; once the loop finds it, SLRP init continues. 
> Ideally, we would obtain a `Future` which is fulfilled when that container 
> comes online; at that point we could advance the clock enough for the SLRP's 
> loop to execute, and SLRP initialization would continue. However, I don't 
> think there is a way to obtain such a `Future`. I think the easiest way 
> around this is to resume the clock while the CSI container is launched, but I 
> need a nice comment here explaining the need for this.

Another option could be to poll `containerizer->containers()` periodically and 
wait for a container to appear, but resuming/pausing the clock is more concise, 
so I would prefer to leave as-is.


> On Jan. 17, 2018, 8:44 a.m., Benjamin Bannier wrote:
> > src/tests/storage_local_resource_provider_tests.cpp
> > Lines 1797 (patched)
> > 
> >
> > This seems unneeded.

If the clock isn't resumed at the end of the test, teardown will not complete 
successfully. The destruction of containers in `Slave::~Slave()` in 
'cluster.cpp' requires the clock to be running in order to finish.


- Greg


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


On Jan. 18, 2018, 1:57 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65039/
> ---
> 
> (Updated Jan. 18, 2018, 1:57 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, Gaston Kleiman, 
> and Jie Yu.
> 
> 
> Bugs: MESOS-8373
> https://issues.apache.org/jira/browse/MESOS-8373
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds
> StorageLocalResourceProviderTest.ROOT_ReconcileDroppedOperation
> in order to verify that reconciliation is performed correctly
> when an operation is dropped on its way from the master to the
> agent.
> 
> 
> Diffs
> -
> 
>   src/tests/storage_local_resource_provider_tests.cpp 
> bbfe95e9818f25fdd5405db3ad2fe355e023f743 
> 
> 
> Diff: https://reviews.apache.org/r/65039/diff/3/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 65039: Tested reconciliation when operation is dropped en route to agent.

[Greg Mann]

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

(Updated Jan. 18, 2018, 1:57 a.m.)


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


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


Repository: mesos


Description
---

This patch adds
StorageLocalResourceProviderTest.ROOT_ReconcileDroppedOperation
in order to verify that reconciliation is performed correctly
when an operation is dropped on its way from the master to the
agent.


Diffs (updated)
-

  src/tests/storage_local_resource_provider_tests.cpp 
bbfe95e9818f25fdd5405db3ad2fe355e023f743 


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

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


Testing
---

sudo make check


Thanks,

Greg Mann

Re: Review Request 65197: Added some missing email addresses to the contributors list.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65197 was successfully built and tested.

Reviews applied: `['65197']`

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

- Mesos Reviewbot Windows


On Jan. 18, 2018, 12:16 a.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65197/
> ---
> 
> (Updated Jan. 18, 2018, 12:16 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Anand Mazumdar, Armand Grillet, 
> Benjamin Hindman, Greg Mann, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added some missing email addresses to the contributors list.
> 
> 
> Diffs
> -
> 
>   docs/contributors.yaml 8bfeb89a9dea3de424b454dc0958515bb855f455 
> 
> 
> Diff: https://reviews.apache.org/r/65197/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 65204: Eliminated some unnecessary copying in the agent's HTTP operator API.

[Michael Park]

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


Ship it!




Ship It!

- Michael Park


On Jan. 17, 2018, 5:19 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65204/
> ---
> 
> (Updated Jan. 17, 2018, 5:19 p.m.)
> 
> 
> Review request for mesos, Michael Park and Meng Zhu.
> 
> 
> Bugs: MESOS-8455
> https://issues.apache.org/jira/browse/MESOS-8455
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp 94fb72c6e6fbc862314ecf2871ae025e422ac41e 
> 
> 
> Diff: https://reviews.apache.org/r/65204/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Review Request 65204: Eliminated some unnecessary copying in the agent's HTTP operator API.

[Benjamin Mahler]

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

Review request for mesos, Michael Park and Meng Zhu.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  src/slave/http.cpp 94fb72c6e6fbc862314ecf2871ae025e422ac41e 


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


Testing
---

make check


Thanks,

Benjamin Mahler

Re: Review Request 65039: Tested reconciliation when operation is dropped en route to agent.

[Greg Mann]

> On Jan. 17, 2018, 8:44 a.m., Benjamin Bannier wrote:
> > src/tests/storage_local_resource_provider_tests.cpp
> > Lines 1652 (patched)
> > 
> >
> > Did you make sure that the ordering `updateSlave2` will always be ready 
> > before `updateSlave1`?
> > 
> > I see that `FUTURE_PROTOBUF` leverages `EXPECT_CALL` (via 
> > `FutureMessage`), so I wonder if we need something like 
> > `::testing::InSequence` to make this this test safe.
> 
> Greg Mann wrote:
> Yea I think here I am unfortunately relying on an implementation detail 
> of googletest. It does seem to be quite reliable, but it's not great.
> 
> In order to effectively use a `Sequence` to force the order, we would 
> need to pass a `Sequence` object into `FUTURE_PROTOBUF`, so that it can be 
> chained onto `EXPECT_CALL` via `.InSequence()`. A single `InSequence` at the 
> scope of the test wouldn't do the job, since each `EXPECT_CALL` invocation 
> occurs within the scope of the `FutureMessage()` call.
> 
> I would propose the following solution: we add a 
> `SEQUENCED_FUTURE_PROTOBUF` macro which accepts a `Sequence` argument, and we 
> update `FutureProtobuf` and `FutureMessage` to accept an `Option` 
> with a default value of `None()`. This would enable the use of ordered 
> expectations without requiring us to update a ton of callsites. WDYT?

Ah, actually Chun-Hung showed me that this is documented behavior in 
googletest: 
https://github.com/google/googletest/blob/master/googlemock/docs/ForDummies.md#using-multiple-expectations

So, while the code here is a bit awkward, it is correct. I think I'll just add 
a comment here similar to what Chun-Hung has done is his patches, to explain 
the reason for the reverse ordering.


- Greg


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


On Jan. 17, 2018, 10:05 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65039/
> ---
> 
> (Updated Jan. 17, 2018, 10:05 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, Gaston Kleiman, 
> and Jie Yu.
> 
> 
> Bugs: MESOS-8373
> https://issues.apache.org/jira/browse/MESOS-8373
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds
> StorageLocalResourceProviderTest.ROOT_ReconcileDroppedOperation
> in order to verify that reconciliation is performed correctly
> when an operation is dropped on its way from the master to the
> agent.
> 
> 
> Diffs
> -
> 
>   src/tests/storage_local_resource_provider_tests.cpp 
> bbfe95e9818f25fdd5405db3ad2fe355e023f743 
> 
> 
> Diff: https://reviews.apache.org/r/65039/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 65203: Updated the CHANGELOG for 1.5.0 release.

[Jie Yu]

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


Fix it, then Ship it!





CHANGELOG
Line 17 (original), 17 (patched)


Add a blank line above like you did above?



CHANGELOG
Line 60 (original), 130 (patched)


I'd move Feature Graduation up before unresolved critical issues



CHANGELOG
Lines 135 (patched)


I'd combine all capabilities related tickets into one bullet.

This should be similar to the first section that, instead of copying the 
ticket summary, we should write full sentenses to explain that that feature is.



CHANGELOG
Lines 142 (patched)


See my comment above, combine with other capabilities related tickets into 
one bullet.


- Jie Yu


On Jan. 18, 2018, 12:21 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65203/
> ---
> 
> (Updated Jan. 18, 2018, 12:21 a.m.)
> 
> 
> Review request for mesos, Adam B, Anand Mazumdar, Andrew Schwartzmeyer, 
> Benjamin Mahler, Chun-Hung Hsiao, Gaston Kleiman, Greg Mann, Ilya Pronin, Jie 
> Yu, James Peach, Kapil Arya, Joseph Wu, Kevin Klues, Neil Conway, Qian Zhang, 
> Till Toenshoff, Vinod Kone, Jiang Yan Xu, and Zhitao Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated the CHANGELOG for 1.5.0 release.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 078c5d64100d81ce84dbdff4055cdda465e19011 
> 
> 
> Diff: https://reviews.apache.org/r/65203/diff/1/
> 
> 
> Testing
> ---
> 
> N/A.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 65202: Adopted the libprocess `DEFAULT_TEST_TIMEOUT`.

[Mesos Reviewbot Windows]

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



FAIL: Mesos libprocess-tests failed to build

Reviews applied: `['65201', '65202']`

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/65202

Relevant logs:

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

```
  Microsoft (R) C/C++ Optimizing Compiler Version 19.12.25830.2 for x64
  Copyright (C) Microsoft Corporation.  All rights reserved.
  
  cl /c /ID:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests.. 
/ID:\DCOS\mesos\mesos\3rdparty\libprocess\src..\include 
/ID:\DCOS\mesos\mesos\3rdparty\stout\include 
/I"D:\DCOS\mesos\3rdparty\libapr-1.5.2\src\libapr-1.5.2\include" 
/I"D:\DCOS\mesos\3rdparty\libapr-1.5.2\src\libapr-1.5.2-build" 
/I"D:\DCOS\mesos\3rdparty\boost-1.53.0\src\boost-1.53.0" 
/I"D:\DCOS\mesos\3rdparty\curl-7.57.0\src\curl-7.57.0\include" 
/I"D:\DCOS\mesos\3rdparty\elfio-3.2\src\elfio-3.2" 
/I"D:\DCOS\mesos\3rdparty\glog-da816ea70\src\glog-da816ea70\src\windows" 
/I"D:\DCOS\mesos\3rdparty\picojson-1.3.0\src\picojson-1.3.0" 
/I"D:\DCOS\mesos\3rdparty\protobuf-3.5.0\src\protobuf-3.5.0\src" 
/I"D:\DCOS\mesos\3rdparty\zlib-1.2.8\src\zlib-1.2.8" 
/I"D:\DCOS\mesos\3rdparty\zlib-1.2.8\src\zlib-1.2.8-build" 
/I"D:\DCOS\mesos\3rdparty\http_parser-2.6.2\src\http_parser-2.6.2" 
/I"C:\OpenSSL-Win64\include" 
/I"D:\DCOS\mesos\3rdparty\googletest-1.8.0\src\googletest-1.8.0\googlemock\include"
 /I"D:\DCOS\mesos\3rdparty\googletest-1.8.0\sr
 c\googletest-1.8.0\googletest\include" /Zi /W3 /WX- /diagnostics:classic /MP 
/Od /Ob0 /D WIN32 /D _WINDOWS /D UNICODE /D _UNICODE /D __WINDOWS__ /D 
_SCL_SECURE_NO_WARNINGS /D _CRT_SECURE_NO_WARNINGS /D _CRT_NONSTDC_NO_WARNINGS 
/D "PKGLIBEXECDIR=\"WARNINGDONOTUSEME\"" /D "LIBDIR=\"WARNINGDONOTUSEME\"" /D 
"VERSION=\"1.6.0\"" /D "PKGDATADIR=\"\"" /D USE_SSL_SOCKET=1 /D CURL_STATICLIB 
/D NOGDI /D NOMINMAX /D GOOGLE_GLOG_DLL_DECL= /D PICOJSON_USE_INT64 /D 
__STDC_FORMAT_MACROS /D HAVE_LIBZ /D _SILENCE_TR1_NAMESPACE_DEPRECATION_WARNING 
/D "CMAKE_INTDIR=\"Debug\"" /D _UNICODE /D UNICODE /Gm- /EHsc /RTC1 /MTd /GS 
/fp:precise /Zc:wchar_t /Zc:forScope /Zc:inline /GR /Fo"test-linkee.dir\Debug\" 
/Fd"test-linkee.dir\Debug\vc141.pdb" /Gd /TP /errorReport:queue  /bigobj /vd2 
/permissive- D:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\test_linkee.cpp
  test_linkee.cpp
  
  Unknown compiler version - please run the configure tests and report the 
results
Link:
  C:\Program Files (x86)\Microsoft Visual 
Studio\2017\Community\VC\Tools\MSVC\14.12.25827\bin\HostX64\x64\link.exe 
/ERRORREPORT:QUEUE 
/OUT:"D:\DCOS\mesos\3rdparty\libprocess\src\tests\Debug\test-linkee.exe" 
/INCREMENTAL /NOLOGO ..\Debug\process.lib 
"..\libapr-1.5.2\src\libapr-1.5.2-build\Debug\libapr-1.lib" 
"..\curl-7.57.0\src\curl-7.57.0-build\lib\Debug\libcurl-d.lib" crypt32.lib 
"..\glog-da816ea70\src\glog-da816ea70-build\Debug\glog.lib" DbgHelp.lib 
"..\protobuf-3.5.0\src\protobuf-3.5.0-build\Debug\libprotobufd.lib" 
"..\zlib-1.2.8\src\zlib-1.2.8-build\Debug\zlibstaticd.lib" Ws2_32.lib 
IPHlpAPI.lib Mswsock.lib Secur32.lib Userenv.lib 
"..\http_parser-2.6.2\src\http_parser-2.6.2-build\Debug\http_parser.lib" 
"C:\OpenSSL-Win64\lib\VC\ssleay32MTd.lib" 
"C:\OpenSSL-Win64\lib\VC\libeay32MTd.lib" 
"..\libevent-2.1.5-beta\src\libevent-2.1.5-beta-build\lib\Debug\event.lib" 
"..\googletest-1.8.0\src\googletest-1.8.0-build\googlemock\Debug\gmock.lib" 
"..\googlet
 est-1.8.0\src\googletest-1.8.0-build\googlemock\gtest\Debug\gtest.lib" 
kernel32.lib user32.lib gdi32.lib winspool.lib shell32.lib ole32.lib 
oleaut32.lib uuid.lib comdlg32.lib advapi32.lib /MANIFEST 
/MANIFESTUAC:"level='asInvoker' uiAccess='false'" /manifest:embed /DEBUG 
/PDB:"D:/DCOS/mesos/3rdparty/libprocess/src/tests/Debug/test-linkee.pdb" 
/SUBSYSTEM:CONSOLE /TLBID:1 /DYNAMICBASE /NXCOMPAT 
/IMPLIB:"D:/DCOS/mesos/3rdparty/libprocess/src/tests/Debug/test-linkee.lib" 
/MACHINE:X64  /machine:x64 "test-linkee.dir\Debug\test_linkee.obj"
 Creating library 
D:/DCOS/mesos/3rdparty/libprocess/src/tests/Debug/test-linkee.lib and object 
D:/DCOS/mesos/3rdparty/libprocess/src/tests/Debug/test-linkee.exp
  test-linkee.vcxproj -> 
D:\DCOS\mesos\3rdparty\libprocess\src\tests\Debug\test-linkee.exe
FinalizeBuildStatus:
  Deleting file "test-linkee.dir\Debug\test-linkee.tlog\unsuccessfulbuild".
  Touching "test-linkee.dir\Debug\test-linkee.tlog\test-linkee.lastbuildstate".
Done Building Project 
"D:\DCOS\mesos\3rdparty\libprocess\src\tests\test-linkee.vcxproj" (default 
targets).
Done Building Project 

Re: Review Request 65197: Added some missing email addresses to the contributors list.

[Gaston Kleiman]

> On Jan. 17, 2018, 4:23 p.m., Joseph Wu wrote:
> > Hum... so of those email addresses are unfortunate... I suppose it's up to 
> > Andrei and Armand if they want to have those listed.
> > 
> > Everything else looks good though.

Yeah, I am not very sure about those temporary email addreses.

I added them, because I used `contributors.yaml` to generate some stats, and I 
wanted to make them as accurate as possible. But I also think that they add 
noise to the file, so I'd be ok removing them from the patch.


- Gaston


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


On Jan. 17, 2018, 4:16 p.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65197/
> ---
> 
> (Updated Jan. 17, 2018, 4:16 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Anand Mazumdar, Armand Grillet, 
> Benjamin Hindman, Greg Mann, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added some missing email addresses to the contributors list.
> 
> 
> Diffs
> -
> 
>   docs/contributors.yaml 8bfeb89a9dea3de424b454dc0958515bb855f455 
> 
> 
> Diff: https://reviews.apache.org/r/65197/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 65203: Updated the CHANGELOG for 1.5.0 release.

[Vinod Kone]

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


Fix it, then Ship it!




LGTM


CHANGELOG
Line 21 (original), 21 (patched)


s/Support/support for/


- Vinod Kone


On Jan. 18, 2018, 12:21 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65203/
> ---
> 
> (Updated Jan. 18, 2018, 12:21 a.m.)
> 
> 
> Review request for mesos, Adam B, Anand Mazumdar, Andrew Schwartzmeyer, 
> Benjamin Mahler, Chun-Hung Hsiao, Gaston Kleiman, Greg Mann, Ilya Pronin, Jie 
> Yu, James Peach, Kapil Arya, Joseph Wu, Kevin Klues, Neil Conway, Qian Zhang, 
> Till Toenshoff, Vinod Kone, Jiang Yan Xu, and Zhitao Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated the CHANGELOG for 1.5.0 release.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 078c5d64100d81ce84dbdff4055cdda465e19011 
> 
> 
> Diff: https://reviews.apache.org/r/65203/diff/1/
> 
> 
> Testing
> ---
> 
> N/A.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 65203: Updated the CHANGELOG for 1.5.0 release.

[James Peach]

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


Fix it, then Ship it!





CHANGELOG
Line 14 (original), 14 (patched)


"for the Container Storage Interface"



CHANGELOG
Line 23 (original), 23 (patched)


"is built into libprocess"


- James Peach


On Jan. 18, 2018, 12:21 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65203/
> ---
> 
> (Updated Jan. 18, 2018, 12:21 a.m.)
> 
> 
> Review request for mesos, Adam B, Anand Mazumdar, Andrew Schwartzmeyer, 
> Benjamin Mahler, Chun-Hung Hsiao, Gaston Kleiman, Greg Mann, Ilya Pronin, Jie 
> Yu, James Peach, Kapil Arya, Joseph Wu, Kevin Klues, Neil Conway, Qian Zhang, 
> Till Toenshoff, Vinod Kone, Jiang Yan Xu, and Zhitao Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated the CHANGELOG for 1.5.0 release.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 078c5d64100d81ce84dbdff4055cdda465e19011 
> 
> 
> Diff: https://reviews.apache.org/r/65203/diff/1/
> 
> 
> Testing
> ---
> 
> N/A.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 65203: Updated the CHANGELOG for 1.5.0 release.

[Till Toenshoff]

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



lgtm


CHANGELOG
Lines 134 (patched)


This one should be reworded; s/Investigate supporting /Added support for/


- Till Toenshoff


On Jan. 18, 2018, 12:21 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65203/
> ---
> 
> (Updated Jan. 18, 2018, 12:21 a.m.)
> 
> 
> Review request for mesos, Adam B, Anand Mazumdar, Andrew Schwartzmeyer, 
> Benjamin Mahler, Chun-Hung Hsiao, Gaston Kleiman, Greg Mann, Ilya Pronin, Jie 
> Yu, James Peach, Kapil Arya, Joseph Wu, Kevin Klues, Neil Conway, Qian Zhang, 
> Till Toenshoff, Vinod Kone, Jiang Yan Xu, and Zhitao Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated the CHANGELOG for 1.5.0 release.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 078c5d64100d81ce84dbdff4055cdda465e19011 
> 
> 
> Diff: https://reviews.apache.org/r/65203/diff/1/
> 
> 
> Testing
> ---
> 
> N/A.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 65197: Added some missing email addresses to the contributors list.

[Joseph Wu]

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


Ship it!




Hum... so of those email addresses are unfortunate... I suppose it's up to 
Andrei and Armand if they want to have those listed.

Everything else looks good though.


docs/contributors.yaml
Lines 84 (patched)


Looks like there's only one commit with this email:
```
commit 10bdcdc77cad097866055256255d8857dd4fadd3
Author: Budnik Andrei 
Date:   Tue May 2 14:20:57 2017 +0200

Added Andrei Budnik to the contributors list.

This closes #209
```



docs/contributors.yaml
Lines 118 (patched)


Only one here too:
```
commit 30f7f3e1965885406d6d3e49348bee7e7d0df9d5
Author: Armand Grillet 
Date:   Mon Feb 6 22:45:43 2017 +0800

Optimized the size of the favicon.

The favicon now only contains two images: 16x16 and 32x32 (for Retina
displays). The size has reduced from 364 KB to 8 KB.

This closes #202
```

And both are Github PRs :|


- Joseph Wu


On Jan. 17, 2018, 4:16 p.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65197/
> ---
> 
> (Updated Jan. 17, 2018, 4:16 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Anand Mazumdar, Armand Grillet, 
> Benjamin Hindman, Greg Mann, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added some missing email addresses to the contributors list.
> 
> 
> Diffs
> -
> 
>   docs/contributors.yaml 8bfeb89a9dea3de424b454dc0958515bb855f455 
> 
> 
> Diff: https://reviews.apache.org/r/65197/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Review Request 65203: Updated the CHANGELOG for 1.5.0 release.

[Gilbert Song]

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

Review request for mesos, Adam B, Anand Mazumdar, Andrew Schwartzmeyer, 
Benjamin Mahler, Chun-Hung Hsiao, Gaston Kleiman, Greg Mann, Ilya Pronin, Jie 
Yu, James Peach, Kapil Arya, Joseph Wu, Kevin Klues, Neil Conway, Qian Zhang, 
Till Toenshoff, Vinod Kone, Jiang Yan Xu, and Zhitao Li.


Repository: mesos


Description
---

Updated the CHANGELOG for 1.5.0 release.


Diffs
-

  CHANGELOG 078c5d64100d81ce84dbdff4055cdda465e19011 


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


Testing
---

N/A.


Thanks,

Gilbert Song

Re: Review Request 65197: Added some missing email addresses to the contributors list.

[Gaston Kleiman]

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

(Updated Jan. 17, 2018, 4:16 p.m.)


Review request for mesos, Andrei Budnik, Anand Mazumdar, Armand Grillet, 
Benjamin Hindman, Greg Mann, and Joseph Wu.


Changes
---

Added moar email addresses!


Summary (updated)
-

Added some missing email addresses to the contributors list.


Repository: mesos


Description (updated)
---

Added some missing email addresses to the contributors list.


Diffs (updated)
-

  docs/contributors.yaml 8bfeb89a9dea3de424b454dc0958515bb855f455 


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

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


Testing
---


Thanks,

Gaston Kleiman

Re: Review Request 65182: Tested that agent resends unacknowledged op status updates on recovery.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65182 was successfully built and tested.

Reviews applied: `['64992', '64994', '65059', '64998', '65000', '65057', 
'65182']`

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

- Mesos Reviewbot Windows


On Jan. 16, 2018, 1 p.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65182/
> ---
> 
> (Updated Jan. 16, 2018, 1 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Greg Mann.
> 
> 
> Bugs: MESOS-8362
> https://issues.apache.org/jira/browse/MESOS-8362
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a test to verify that the agent resends unacknowledged
> operation status updates after a recovery.
> 
> 
> Diffs
> -
> 
>   src/tests/storage_local_resource_provider_tests.cpp 
> bbfe95e9818f25fdd5405db3ad2fe355e023f743 
> 
> 
> Diff: https://reviews.apache.org/r/65182/diff/3/
> 
> 
> Testing
> ---
> 
> `GLOG_v=1 sudo bin/mesos-tests.sh 
> --gtest_filter='StorageLocalResourceProviderTest.ROOT_RetryOperationStatusUpdateAfterRecovery'
>  --verbose --gtest_repeat=1000 --gtest_break_on_failure` on GNU/Linux
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 65197: Updated Gaston Kleiman's entry in contributors.yaml.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65197 was successfully built and tested.

Reviews applied: `['65197']`

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

- Mesos Reviewbot Windows


On Jan. 17, 2018, 2:28 p.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65197/
> ---
> 
> (Updated Jan. 17, 2018, 2:28 p.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated Gaston Kleiman's entry in contributors.yaml.
> 
> 
> Diffs
> -
> 
>   docs/contributors.yaml 8bfeb89a9dea3de424b454dc0958515bb855f455 
> 
> 
> Diff: https://reviews.apache.org/r/65197/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Review Request 65202: Adopted the libprocess `DEFAULT_TEST_TIMEOUT`.

[James Peach]

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

Review request for mesos, Benjamin Bannier and Gaston Kleiman.


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


Repository: mesos


Description
---

Rather than hard-coding a 15sec test timeout, use the libprocess
`DEFAULT_TEST_TIMEOUT` variable so that this can be globally
tuned. Added a test flag `--default_timeout` that can be used
to set the `DEFAULT_TEST_TIMEOUT` on a test run.


Diffs
-

  src/logging/flags.hpp fe40a11c6a5e3ef44ecf008d1aa03d4e74e59859 
  src/tests/agent_container_api_tests.cpp 
618569277545205017320aaf1f3a70e540d35e30 
  src/tests/containerizer/cgroups_isolator_tests.cpp 
5412a7eccaa836a7f182772f1eb33b10298c96a1 
  src/tests/containerizer/xfs_quota_tests.cpp 
d761ea46fd4f12c6bed74090bf5ead7587f16811 
  src/tests/disk_quota_tests.cpp 4e25e32bf784ea818e18497fe3ebc9f94c5861ff 
  src/tests/fetcher_cache_tests.cpp 7db7b7dcef27b1686ccae5a7408ff2811a3b9255 
  src/tests/flags.hpp df4d4588f405ea884bb89021264e70218e054fae 
  src/tests/flags.cpp 4542f3443830474edd97e8b695af0c61685e947c 
  src/tests/gc_tests.cpp 0e34c017f2c629b527915218f517b88d8392aa4b 
  src/tests/main.cpp f945ac9e8858998174b4c2785f6a5612bae66a41 
  src/tests/master_contender_detector_tests.cpp 
015d310b72a152829641d436f7cb558f72dfd77c 
  src/tests/mesos.hpp 16c75bb0ca570dfb7089743344cf38acdb517705 
  src/tests/registrar_tests.cpp cbb58c1cd72dc13ed16b5465452d17526255c763 
  src/tests/slave_recovery_tests.cpp 641fc6e82be19bdea55d35ec8f34a862de58489e 
  src/tests/slave_tests.cpp 59e3065ad9aadbd90cdfd32e830b433b88d6de86 


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


Testing
---

make check (Fedora 27).


Thanks,

James Peach

Review Request 65201: Added a global DEFAULT_TEST_TIMEOUT variable.

[James Peach]

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

Review request for mesos, Benjamin Bannier and Gaston Kleiman.


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


Repository: mesos


Description
---

Added a global DEFAULT_TEST_TIMEOUT variable that applications can use
to globally tune the default timeout in the `AWAIT_READY` family of
test macros. The default remains 15sec.


Diffs
-

  3rdparty/libprocess/Makefile.am ba9e78148932304ce1961b88e5f97180abd35586 
  3rdparty/libprocess/include/process/gtest.hpp 
eee726653d52af2e4a148819e420ebd22e5123a9 
  3rdparty/libprocess/src/gtest.cpp PRE-CREATION 


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


Testing
---

make check (Fedora 27).


Thanks,

James Peach

Re: Review Request 64849: Added authentication to some example frameworks.

[Till Toenshoff]

> On Jan. 16, 2018, 11:48 p.m., Vinod Kone wrote:
> > src/examples/dynamic_reservation_framework.cpp
> > Line 41 (original)
> > 
> >
> > Any particular reason you deleted these?

Improving consistency as that is the greater goal of this chain. Most other 
examples did not need them - this example also does not benefit. ie. `cerr` is 
being referenced once, adding a `using` for that seemed to add more boilerplate 
than needed.


> On Jan. 16, 2018, 11:48 p.m., Vinod Kone wrote:
> > src/examples/persistent_volume_framework.cpp
> > Line 501 (original)
> > 
> >
> > Why did you revert this change introduced in the previous review?

Initially, I thought I'ld simply leave it as this todo but then decided to go 
the extra mile and covering it as well in this chain.


> On Jan. 16, 2018, 11:48 p.m., Vinod Kone wrote:
> > src/examples/test_http_framework.cpp
> > Line 401 (original), 390 (patched)
> > 
> >
> > aah you fixed in this review!

Yeah, that was a rebase slipup.


- Till


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


On Jan. 17, 2018, 7:01 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64849/
> ---
> 
> (Updated Jan. 17, 2018, 7:01 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Armand Grillet, Greg Mann, 
> Kapil Arya, and Vinod Kone.
> 
> 
> Bugs: MESOS-5362 and MESOS-8357
> https://issues.apache.org/jira/browse/MESOS-5362
> https://issues.apache.org/jira/browse/MESOS-8357
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> All example frameworks now support authenticating when registering
> to the master.
> 
> 
> Diffs
> -
> 
>   src/examples/dynamic_reservation_framework.cpp 
> 538fbe8847a1b1dcbfe48ad0e3678797801a12f5 
>   src/examples/persistent_volume_framework.cpp 
> 71db39d7743d31ea34c2aed579d7a1ef2ed95687 
>   src/examples/test_http_framework.cpp 
> 482f65efc15a7bac52c33a57b2b876191249410a 
>   src/tests/persistent_volume_framework_test.sh 
> 2ab22c03e573d4801c73957f9cad2939b3d3174b 
> 
> 
> Diff: https://reviews.apache.org/r/64849/diff/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 64857: Updated example frameworks for mesos-local.

[Till Toenshoff]

> On Jan. 16, 2018, 11:52 p.m., Vinod Kone wrote:
> > src/examples/dynamic_reservation_framework.cpp
> > Lines 389 (patched)
> > 
> >
> > don't you need to set roles in other example frameworks too for the 
> > local case?

Yes, for the sake of consistency, I should.


- Till


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


On Jan. 17, 2018, 11:37 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64857/
> ---
> 
> (Updated Jan. 17, 2018, 11:37 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Armand Grillet, Greg Mann, 
> Kapil Arya, and Vinod Kone.
> 
> 
> Bugs: MESOS-8357 and MESOS-8361
> https://issues.apache.org/jira/browse/MESOS-8357
> https://issues.apache.org/jira/browse/MESOS-8361
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Unifies handling of 'local' when supplied for the '--master' flag.
> 
> 
> Diffs
> -
> 
>   src/examples/balloon_framework.cpp 410966e64aa3f146a88c00babaa51b499dd24d36 
>   src/examples/disk_full_framework.cpp 
> d75f769538d17229bdbf332ea7513b1aa8d7c334 
>   src/examples/docker_no_executor_framework.cpp 
> 1fa408d3847d414c23eed8334db8de02e84cf764 
>   src/examples/dynamic_reservation_framework.cpp 
> 538fbe8847a1b1dcbfe48ad0e3678797801a12f5 
>   src/examples/load_generator_framework.cpp 
> 42867992eba8699cab5b70c62a24b87446139406 
>   src/examples/long_lived_framework.cpp 
> 943160d5a42e02cf05fe92999233ce2e792849fd 
>   src/examples/no_executor_framework.cpp 
> 972ef777cec07d1030af208daef69ebe606d071e 
>   src/examples/test_framework.cpp acf1faf1523c8c03483dfafbc8f8e245322527e4 
>   src/examples/test_http_framework.cpp 
> 482f65efc15a7bac52c33a57b2b876191249410a 
> 
> 
> Diff: https://reviews.apache.org/r/64857/diff/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 64857: Updated example frameworks for mesos-local.

[Till Toenshoff]

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

(Updated Jan. 17, 2018, 11:37 p.m.)


Review request for mesos, Alexander Rukletsov, Armand Grillet, Greg Mann, Kapil 
Arya, and Vinod Kone.


Changes
---

Limiting roles for all example frameworks when being started in 
"local"-master-mode.


Bugs: MESOS-8357 and MESOS-8361
https://issues.apache.org/jira/browse/MESOS-8357
https://issues.apache.org/jira/browse/MESOS-8361


Repository: mesos


Description
---

Unifies handling of 'local' when supplied for the '--master' flag.


Diffs (updated)
-

  src/examples/balloon_framework.cpp 410966e64aa3f146a88c00babaa51b499dd24d36 
  src/examples/disk_full_framework.cpp d75f769538d17229bdbf332ea7513b1aa8d7c334 
  src/examples/docker_no_executor_framework.cpp 
1fa408d3847d414c23eed8334db8de02e84cf764 
  src/examples/dynamic_reservation_framework.cpp 
538fbe8847a1b1dcbfe48ad0e3678797801a12f5 
  src/examples/load_generator_framework.cpp 
42867992eba8699cab5b70c62a24b87446139406 
  src/examples/long_lived_framework.cpp 
943160d5a42e02cf05fe92999233ce2e792849fd 
  src/examples/no_executor_framework.cpp 
972ef777cec07d1030af208daef69ebe606d071e 
  src/examples/test_framework.cpp acf1faf1523c8c03483dfafbc8f8e245322527e4 
  src/examples/test_http_framework.cpp 482f65efc15a7bac52c33a57b2b876191249410a 


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

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


Testing
---

make check


Thanks,

Till Toenshoff

Re: Review Request 65200: Added a default move constructor for Result.

[Michael Park]

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


Ship it!




Ship It!

- Michael Park


On Jan. 17, 2018, 3:30 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65200/
> ---
> 
> (Updated Jan. 17, 2018, 3:30 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-2921
> https://issues.apache.org/jira/browse/MESOS-2921
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/result.hpp 
> 0ce98fa208e4563036294448e2081b80884b9748 
> 
> 
> Diff: https://reviews.apache.org/r/65200/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Review Request 65200: Added a default move constructor for Result.

[Benjamin Mahler]

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

Review request for mesos and Michael Park.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  3rdparty/stout/include/stout/result.hpp 
0ce98fa208e4563036294448e2081b80884b9748 


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


Testing
---

make check


Thanks,

Benjamin Mahler

Re: Review Request 65039: Tested reconciliation when operation is dropped en route to agent.

[Greg Mann]

> On Jan. 17, 2018, 8:44 a.m., Benjamin Bannier wrote:
> > src/tests/storage_local_resource_provider_tests.cpp
> > Lines 1652 (patched)
> > 
> >
> > Did you make sure that the ordering `updateSlave2` will always be ready 
> > before `updateSlave1`?
> > 
> > I see that `FUTURE_PROTOBUF` leverages `EXPECT_CALL` (via 
> > `FutureMessage`), so I wonder if we need something like 
> > `::testing::InSequence` to make this this test safe.

Yea I think here I am unfortunately relying on an implementation detail of 
googletest. It does seem to be quite reliable, but it's not great.

In order to effectively use a `Sequence` to force the order, we would need to 
pass a `Sequence` object into `FUTURE_PROTOBUF`, so that it can be chained onto 
`EXPECT_CALL` via `.InSequence()`. A single `InSequence` at the scope of the 
test wouldn't do the job, since each `EXPECT_CALL` invocation occurs within the 
scope of the `FutureMessage()` call.

I would propose the following solution: we add a `SEQUENCED_FUTURE_PROTOBUF` 
macro which accepts a `Sequence` argument, and we update `FutureProtobuf` and 
`FutureMessage` to accept an `Option` with a default value of 
`None()`. This would enable the use of ordered expectations without requiring 
us to update a ton of callsites. WDYT?


> On Jan. 17, 2018, 8:44 a.m., Benjamin Bannier wrote:
> > src/tests/storage_local_resource_provider_tests.cpp
> > Lines 1656 (patched)
> > 
> >
> > Would it make sense to have the whole test run with paused clock?

I need to leave a comment here on why I do this. Unfortunately, the standalone 
container running the CSI plugin complicates things a bit when running with the 
clock paused. The SLRP runs a `loop()` which looks for the CSI container's 
domain socket; once the loop finds it, SLRP init continues. Ideally, we would 
obtain a `Future` which is fulfilled when that container comes online; at that 
point we could advance the clock enough for the SLRP's loop to execute, and 
SLRP initialization would continue. However, I don't think there is a way to 
obtain such a `Future`. I think the easiest way around this is to resume the 
clock while the CSI container is launched, but I need a nice comment here 
explaining the need for this.


- Greg


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


On Jan. 17, 2018, 10:05 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65039/
> ---
> 
> (Updated Jan. 17, 2018, 10:05 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, Gaston Kleiman, 
> and Jie Yu.
> 
> 
> Bugs: MESOS-8373
> https://issues.apache.org/jira/browse/MESOS-8373
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds
> StorageLocalResourceProviderTest.ROOT_ReconcileDroppedOperation
> in order to verify that reconciliation is performed correctly
> when an operation is dropped on its way from the master to the
> agent.
> 
> 
> Diffs
> -
> 
>   src/tests/storage_local_resource_provider_tests.cpp 
> bbfe95e9818f25fdd5405db3ad2fe355e023f743 
> 
> 
> Diff: https://reviews.apache.org/r/65039/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 65196: Removed workaround in ZooKeeper test.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65196 was successfully built and tested.

Reviews applied: `['65195', '65196']`

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

- Mesos Reviewbot Windows


On Jan. 17, 2018, 10:05 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65196/
> ---
> 
> (Updated Jan. 17, 2018, 10:05 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Jeff Coffler, and Joseph Wu.
> 
> 
> Bugs: MESOS-7803
> https://issues.apache.org/jira/browse/MESOS-7803
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Now that `fs::list` returns the full path, this code can be fixed.
> 
> 
> Diffs
> -
> 
>   src/tests/zookeeper.cpp 54cf9c63878b5b6d30df27838244eec46312d917 
> 
> 
> Diff: https://reviews.apache.org/r/65196/diff/1/
> 
> 
> Testing
> ---
> 
> This was fixed while enabling recovery for the Windows agent.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 65039: Tested reconciliation when operation is dropped en route to agent.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65039 was successfully built and tested.

Reviews applied: `['64992', '64994', '65039']`

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

- Mesos Reviewbot Windows


On Jan. 17, 2018, 10:05 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65039/
> ---
> 
> (Updated Jan. 17, 2018, 10:05 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, Gaston Kleiman, 
> and Jie Yu.
> 
> 
> Bugs: MESOS-8373
> https://issues.apache.org/jira/browse/MESOS-8373
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds
> StorageLocalResourceProviderTest.ROOT_ReconcileDroppedOperation
> in order to verify that reconciliation is performed correctly
> when an operation is dropped on its way from the master to the
> agent.
> 
> 
> Diffs
> -
> 
>   src/tests/storage_local_resource_provider_tests.cpp 
> bbfe95e9818f25fdd5405db3ad2fe355e023f743 
> 
> 
> Diff: https://reviews.apache.org/r/65039/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Review Request 65197: Updated Gaston Kleiman's entry in contributors.yaml.

[Gaston Kleiman]

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

Review request for mesos and Greg Mann.


Repository: mesos


Description
---

Updated Gaston Kleiman's entry in contributors.yaml.


Diffs
-

  docs/contributors.yaml 8bfeb89a9dea3de424b454dc0958515bb855f455 


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


Testing
---


Thanks,

Gaston Kleiman

Re: Review Request 65166: Added libprocess benchmarks to cmake build setup.

[Andrew Schwartzmeyer]

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


Ship it!




Ship It!

- Andrew Schwartzmeyer


On Jan. 17, 2018, 12:42 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65166/
> ---
> 
> (Updated Jan. 17, 2018, 12:42 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> To make sure that `benchmarks` is built as part of the tests we add a
> dependency of `libprocess-tests` on `benchmarks` even though no actual
> compile- or runtime dependency exists.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/CMakeLists.txt 
> 8b76cab4d549e18e34aef39863188f37cde74223 
> 
> 
> Diff: https://reviews.apache.org/r/65166/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> `ninja benchmarks && ./3rdparty/libprocess/src/tests/benchmarks`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 65166: Added libprocess benchmarks to cmake build setup.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65166 was successfully built and tested.

Reviews applied: `['65166']`

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

- Mesos Reviewbot Windows


On Jan. 17, 2018, 8:42 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65166/
> ---
> 
> (Updated Jan. 17, 2018, 8:42 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> To make sure that `benchmarks` is built as part of the tests we add a
> dependency of `libprocess-tests` on `benchmarks` even though no actual
> compile- or runtime dependency exists.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/CMakeLists.txt 
> 8b76cab4d549e18e34aef39863188f37cde74223 
> 
> 
> Diff: https://reviews.apache.org/r/65166/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> `ninja benchmarks && ./3rdparty/libprocess/src/tests/benchmarks`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 65166: Added libprocess benchmarks to cmake build setup.

[Andrew Schwartzmeyer]

> On Jan. 17, 2018, 2:12 p.m., Andrew Schwartzmeyer wrote:
> > 3rdparty/libprocess/src/tests/CMakeLists.txt
> > Lines 98-101 (patched)
> > 
> >
> > Would you rather it be part of the `tests` target explicitly? You could 
> > add it 
> > [here](https://github.com/apache/mesos/blob/24a746d85780051d862db0697b63c3c52db4362a/CMakeLists.txt#L60).
> > 
> > ```
> > add_custom_target(tests DEPENDS stout-tests libprocess-tests 
> > mesos-tests)
> > ```
> > 
> > I think it probably makes more sense.

Then again, it's explicitly benchmarks for libprocess, so it also makes sense 
as a dependency of `libprocess-tests`. Yeah, nevermind, this is fine I think.


- Andrew


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


On Jan. 17, 2018, 12:42 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65166/
> ---
> 
> (Updated Jan. 17, 2018, 12:42 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> To make sure that `benchmarks` is built as part of the tests we add a
> dependency of `libprocess-tests` on `benchmarks` even though no actual
> compile- or runtime dependency exists.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/CMakeLists.txt 
> 8b76cab4d549e18e34aef39863188f37cde74223 
> 
> 
> Diff: https://reviews.apache.org/r/65166/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> `ninja benchmarks && ./3rdparty/libprocess/src/tests/benchmarks`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 65166: Added libprocess benchmarks to cmake build setup.

[Andrew Schwartzmeyer]

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




3rdparty/libprocess/src/tests/CMakeLists.txt
Lines 98-101 (patched)


Would you rather it be part of the `tests` target explicitly? You could add 
it 
[here](https://github.com/apache/mesos/blob/24a746d85780051d862db0697b63c3c52db4362a/CMakeLists.txt#L60).

```
add_custom_target(tests DEPENDS stout-tests libprocess-tests mesos-tests)
```

I think it probably makes more sense.


- Andrew Schwartzmeyer


On Jan. 17, 2018, 12:42 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65166/
> ---
> 
> (Updated Jan. 17, 2018, 12:42 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> To make sure that `benchmarks` is built as part of the tests we add a
> dependency of `libprocess-tests` on `benchmarks` even though no actual
> compile- or runtime dependency exists.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/CMakeLists.txt 
> 8b76cab4d549e18e34aef39863188f37cde74223 
> 
> 
> Diff: https://reviews.apache.org/r/65166/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> `ninja benchmarks && ./3rdparty/libprocess/src/tests/benchmarks`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Review Request 65196: Removed workaround in ZooKeeper test.

[Andrew Schwartzmeyer]

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

Review request for mesos, Akash Gupta, Jeff Coffler, and Joseph Wu.


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


Repository: mesos


Description
---

Now that `fs::list` returns the full path, this code can be fixed.


Diffs
-

  src/tests/zookeeper.cpp 54cf9c63878b5b6d30df27838244eec46312d917 


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


Testing
---

This was fixed while enabling recovery for the Windows agent.


Thanks,

Andrew Schwartzmeyer

Re: Review Request 65039: Tested reconciliation when operation is dropped en route to agent.

[Greg Mann]

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

(Updated Jan. 17, 2018, 10:05 p.m.)


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


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


Repository: mesos


Description
---

This patch adds
StorageLocalResourceProviderTest.ROOT_ReconcileDroppedOperation
in order to verify that reconciliation is performed correctly
when an operation is dropped on its way from the master to the
agent.


Diffs
-

  src/tests/storage_local_resource_provider_tests.cpp 
bbfe95e9818f25fdd5405db3ad2fe355e023f743 


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


Testing
---

sudo make check


Thanks,

Greg Mann

Review Request 65195: Windows: Fixed `fs::list` to return full paths.

[Andrew Schwartzmeyer]

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

Review request for mesos, Akash Gupta, Jeff Coffler, and Joseph Wu.


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


Repository: mesos


Description
---

This resolves MESOS-7803, where `fs::list` only returned the name of the
found files, dropping the initial path components. That is, for a
directory `foo` with the file `bar` in it, and the pattern
`fs::list("foo/*")`, it was incorrectly returning just `bar`, not
`foo/bar`. This was fixed by getting the `dirname` of the pattern, and
rejoining it with each found file name before it is returned.


Diffs
-

  3rdparty/stout/include/stout/windows/fs.hpp 
bc7cf025c9228a65f2ae0fab9ba27b386d189b46 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

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

[Akash Gupta]

> On Jan. 10, 2018, 1:27 p.m., Alexander Rukletsov wrote:
> > src/tests/containerizer/docker_tests.cpp
> > Lines 139-153 (patched)
> > 
> >
> > Again, this is something we should likely do in a broader scope. 
> > Chances are, someone else in a different place will try to solve the same 
> > problem.
> 
> Andrew Schwartzmeyer wrote:
> Same as above. At the point of someone else solving the same problem, 
> then it would make sense to move the code into a more general purpose 
> location and reuse it. But we shouldn't do it until then, because otherwise 
> we're just guessing at future technical needs.
> 
> Akash Gupta wrote:
> This is specific to Docker and only for test code that checks the 
> container exit value, which is onlt used here. So, I don't think other places 
> will ever use this.
> 
> Andrew Schwartzmeyer wrote:
> I was talking with Alex, and while I don't think `stout` is necessarily 
> the right place for this kind of code, there is potential clean up here:
> 
> ```
> src/tests/containerizer/docker_containerizer_tests.cpp
> 1288:  AWAIT_EXPECT_WEXITSTATUS_EQ(128 + SIGKILL, orphanRun);
> 1425:  AWAIT_EXPECT_WEXITSTATUS_EQ(128 + SIGKILL, orphanRun);
> 
> src/tests/containerizer/ns_tests.cpp
> 101:  AWAIT_EXPECT_WEXITSTATUS_EQ(0, s->status());
> 
> src/tests/containerizer/docker_tests.cpp
> 170:  AWAIT_EXPECT_WEXITSTATUS_EQ(128 + SIGKILL, status);
> 250:  AWAIT_EXPECT_WEXITSTATUS_EQ(128 + SIGKILL, status);
> 318:  AWAIT_EXPECT_WEXITSTATUS_EQ(128 + SIGKILL, run);
> 449:  AWAIT_EXPECT_WEXITSTATUS_EQ(0, run);
> 528:  AWAIT_EXPECT_WEXITSTATUS_EQ(0, run);
> 574:  AWAIT_EXPECT_WEXITSTATUS_EQ(0, run);
> 623:  AWAIT_EXPECT_WEXITSTATUS_EQ(0, run);
> 844:  AWAIT_EXPECT_WEXITSTATUS_EQ(0, status);
> 918:  AWAIT_EXPECT_WEXITSTATUS_EQ(128 + SIGKILL, status);
> 
> src/tests/containerizer/capabilities_tests.cpp
> 98:  AWAIT_EXPECT_WEXITSTATUS_NE(0, s->status());
> 122:  AWAIT_EXPECT_WEXITSTATUS_NE(0, s->status());
> 151:  AWAIT_EXPECT_WEXITSTATUS_EQ(0, s->status());
> 
> 3rdparty/libprocess/src/tests/reap_tests.cpp
> 98:  AWAIT_EXPECT_WEXITSTATUS_EQ(0, status);
> 172:  AWAIT_EXPECT_WEXITSTATUS_EQ(0, status);
> 
> 3rdparty/libprocess/include/process/gtest.hpp
> 623:#define AWAIT_EXPECT_WEXITSTATUS_EQ_FOR(expected, actual, duration)   
>   \
> 627:#define AWAIT_EXPECT_WEXITSTATUS_EQ(expected, actual) 
>   \
> 628:  AWAIT_EXPECT_WEXITSTATUS_EQ_FOR(expected, actual, Seconds(15))
> 664:#define AWAIT_EXPECT_WEXITSTATUS_NE_FOR(expected, actual, duration)   
>   \
> 668:#define AWAIT_EXPECT_WEXITSTATUS_NE(expected, actual) 
>   \
> 669:  AWAIT_EXPECT_WEXITSTATUS_NE_FOR(expected, actual, Seconds(15))
> 
> 3rdparty/libprocess/src/tests/subprocess_tests.cpp
> 72:  AWAIT_EXPECT_WEXITSTATUS_EQ(0, s.get().status());
> 249:  AWAIT_EXPECT_WEXITSTATUS_EQ(0, s.get().status());
> 264:  AWAIT_EXPECT_WEXITSTATUS_EQ(1, s.get().status());
> 323:  AWAIT_EXPECT_WEXITSTATUS_EQ(0, s.get().status());
> 344:  AWAIT_EXPECT_WEXITSTATUS_EQ(0, s.get().status());
> 371:  AWAIT_EXPECT_WEXITSTATUS_EQ(0, s.get().status());
> 411:  AWAIT_EXPECT_WEXITSTATUS_EQ(0, s.get().status());
> 442:  AWAIT_EXPECT_WEXITSTATUS_EQ(0, s.get().status());
> 465:  AWAIT_EXPECT_WEXITSTATUS_EQ(0, s.get().status());
> 497:  AWAIT_EXPECT_WEXITSTATUS_EQ(0, s.get().status());
> 531:  AWAIT_EXPECT_WEXITSTATUS_EQ(0, s.get().status());
> 562:  AWAIT_EXPECT_WEXITSTATUS_EQ(0, s.get().status());
> 599:  AWAIT_EXPECT_WEXITSTATUS_EQ(0, s.get().status());
> 617:  AWAIT_EXPECT_WEXITSTATUS_EQ(0, s.get().status());
> 700:  AWAIT_EXPECT_WEXITSTATUS_EQ(0, s.get().status());
> 763:  AWAIT_EXPECT_WEXITSTATUS_EQ(0, s.get().status());
> 789:  AWAIT_EXPECT_WEXITSTATUS_EQ(0, s.get().status());
> 818:  AWAIT_EXPECT_WEXITSTATUS_EQ(0, s.get().status());
> 847:  AWAIT_EXPECT_WEXITSTATUS_EQ(0, s.get().status());
> 879:  AWAIT_EXPECT_WEXITSTATUS_EQ(0, s.get().status());
> ```
> 
> Especially the ones that do `128 + SIGKILL`; just WTF is that even doing? 
> No one should have to go to [Stack 
> Overflow](https://unix.stackexchange.com/a/99134) to understand what an 
> assertion in a test is doing.
> 
> It may be reasonable to go to `include/process/gtest.hpp` and make this 
> easier, with `AWAIT_EXPECT_EXIT_SIGKILL(status)` (and like 
> `AWAIT_EXPECT_EXIT_SUCCESS(status)` for `== 0`).

The weird exit code only happens to docker containers getting stopped/killed, 
so I think only the `128+SIGKILL` ones need to be cleaned up in 
`docker_tests.cpp` and `docker_containerizer_tests.cpp`


- Akash


---
This is an automatically generated e-mail. To reply, visit:

Re: Review Request 63829: Modified Containerizer::remove to allow top-level containers.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 63829 was successfully built and tested.

Reviews applied: `['63829']`

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

- Mesos Reviewbot Windows


On Jan. 17, 2018, 11:24 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63829/
> ---
> 
> (Updated Jan. 17, 2018, 11:24 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Zhitao Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This includes two changes:
> 1) Containerizer::remove no longer CHECKs for a nested container as
>input and accepts standalone containers too.
> 2) Standalone containers no longer have their runtime directory
>deleted upon exit (which funnily, would prevent the containerizer
>from figuring out that the ContainerID is a standalone).
> 
> This change means that, for standalone containers, the caller will
> need to explicitly remove the standalone container with the
> REMOVE_CONTAINER API.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> ec39d04e7129a4f8b87166f18d3203dc280c373b 
>   src/tests/agent_container_api_tests.cpp 
> 618569277545205017320aaf1f3a70e540d35e30 
> 
> 
> Diff: https://reviews.apache.org/r/63829/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

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

[Akash Gupta]

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

(Updated Jan. 17, 2018, 8:44 p.m.)


Review request for mesos, Alexander Rukletsov, Andrew Schwartzmeyer, Gaston 
Kleiman, Jie Yu, John Kordich, Joseph Wu, and Michael Park.


Changes
---

fix nolint


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


Repository: mesos


Description
---

The Network enum in DockerInfo is specific to Linux containers. `HOST`
doesn't exist on Windows and `BRIDGE` is `NAT` on Windows. The current
default docker network setting was always `HOST`, which broke the
Windows docker executor. Now, if a specific network isn't given, the
network mode will default to `HOST` on Linux agents and `NAT` on Windows
agents. Also, `BRIDGE` mode will be translated to `NAT` on Windows.


Diffs (updated)
-

  src/docker/docker.cpp 722a54ad113fc4e2bb22a8f08e307ab38d5fbfed 


Diff: https://reviews.apache.org/r/63860/diff/10/

Changes: https://reviews.apache.org/r/63860/diff/9-10/


Testing
---

See https://reviews.apache.org/r/63862/ for test results.


Thanks,

Akash Gupta

Re: Review Request 65166: Added libprocess benchmarks to cmake build setup.

[Benjamin Bannier]

> On Jan. 17, 2018, 7:58 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/src/tests/CMakeLists.txt
> > Lines 91-93 (patched)
> > 
> >
> > We don't want to build this executable by default, so drop a 
> > `EXCLUDE_FROM_ALL` in here.
> > 
> > ```
> > add_executable(
> >   benchmarks EXCLUDE_FROM_ALL
> >   benchmarks.cpp
> >   benchmarks.pb.cc)
> > ```

Like discussed offline, we would still want to make sure that this target is 
built at all. I added a dummy dependency of `libprocess-tests` on `benchmarks` 
to make sure it is built as part of e.g., `check`.


- Benjamin


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


On Jan. 17, 2018, 9:42 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65166/
> ---
> 
> (Updated Jan. 17, 2018, 9:42 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> To make sure that `benchmarks` is built as part of the tests we add a
> dependency of `libprocess-tests` on `benchmarks` even though no actual
> compile- or runtime dependency exists.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/CMakeLists.txt 
> 8b76cab4d549e18e34aef39863188f37cde74223 
> 
> 
> Diff: https://reviews.apache.org/r/65166/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> `ninja benchmarks && ./3rdparty/libprocess/src/tests/benchmarks`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 65166: Added libprocess benchmarks to cmake build setup.

[Benjamin Bannier]

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

(Updated Jan. 17, 2018, 9:42 p.m.)


Review request for mesos, Andrew Schwartzmeyer and Joseph Wu.


Changes
---

Addressed comments from Joseph.


Repository: mesos


Description (updated)
---

To make sure that `benchmarks` is built as part of the tests we add a
dependency of `libprocess-tests` on `benchmarks` even though no actual
compile- or runtime dependency exists.


Diffs (updated)
-

  3rdparty/libprocess/src/tests/CMakeLists.txt 
8b76cab4d549e18e34aef39863188f37cde74223 


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

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


Testing
---

`make check`
`ninja benchmarks && ./3rdparty/libprocess/src/tests/benchmarks`


Thanks,

Benjamin Bannier

Re: Review Request 63828: Added authorization tests for standalone container APIs.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 63828 was successfully built and tested.

Reviews applied: `['63828']`

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

- Mesos Reviewbot Windows


On Jan. 17, 2018, 7:21 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63828/
> ---
> 
> (Updated Jan. 17, 2018, 7:21 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas, Gilbert Song, and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The usage of the standalone container APIs is rather coarse-grained.
> A principal can either use the APIs, or it cannot.  This is the case
> for all four actions: Launch, Wait, Kill, and Remove.
> 
> 
> Diffs
> -
> 
>   src/tests/authorization_tests.cpp a76ad18a54ec232e6d6c92ec6a45b445a83f174c 
> 
> 
> Diff: https://reviews.apache.org/r/63828/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 63848: Added test to check standalone containers in GET_CONTAINERS.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 63848 was successfully built and tested.

Reviews applied: `['63848']`

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

- Mesos Reviewbot Windows


On Jan. 17, 2018, 11:21 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63848/
> ---
> 
> (Updated Jan. 17, 2018, 11:21 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Standalone containers are *NOT* expected to show up in the output
> of the agent GET_CONTAINERS call.  This is because the standalone
> containers lack some of the required metadata, like a FrameworkID,
> ExecutorID, and Executor name.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp fb45879310faf7edc7d101be206d49de89819c18 
> 
> 
> Diff: https://reviews.apache.org/r/63848/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 65000: Added an SLRP test for agent being registered with a new ID.

[Greg Mann]

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




src/tests/storage_local_resource_provider_tests.cpp
Lines 981 (patched)


s/becomes a pre-existing volume/become pre-existing volumes/



src/tests/storage_local_resource_provider_tests.cpp
Lines 1003-1011 (patched)


Can you do something like:

```
agentFlags.agent_features = SlaveCapabilities();
agentFlags.agent_features->add_capabilities()->set_type(
SlaveInfo::Capability::RESOURCE_PROVIDER);
```



src/tests/storage_local_resource_provider_tests.cpp
Lines 1039-1040 (patched)


Same question here as in a previous review, regarding potential raciness of 
the first offer. Have you tested this under load?



src/tests/storage_local_resource_provider_tests.cpp
Lines 1051 (patched)


s/volume/volumes/



src/tests/storage_local_resource_provider_tests.cpp
Lines 1151-1158 (patched)


There doesn't seem to be anything here which verifies that the agent has 
registered with a new ID?

You could await on a `FUTURE_PROTOBUF(SlaveRegisteredMessage(), _, _)` to 
ensure this.



src/tests/storage_local_resource_provider_tests.cpp
Lines 1041-1043 (original), 1240-1242 (patched)


Are these changes, and the rest below, meant to go in this RR?


- Greg Mann


On Jan. 10, 2018, 2:34 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65000/
> ---
> 
> (Updated Jan. 10, 2018, 2:34 a.m.)
> 
> 
> Review request for mesos, Gaston Kleiman, Greg Mann, and Jie Yu.
> 
> 
> Bugs: MESOS-8409
> https://issues.apache.org/jira/browse/MESOS-8409
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added an SLRP test for agent being registered with a new ID.
> 
> 
> Diffs
> -
> 
>   src/tests/storage_local_resource_provider_tests.cpp 
> bbfe95e9818f25fdd5405db3ad2fe355e023f743 
> 
> 
> Diff: https://reviews.apache.org/r/65000/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 65059: Speeded up SLRP unit tests.

[Greg Mann]

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




src/tests/storage_local_resource_provider_tests.cpp
Line 604 (original), 607 (patched)


Ah, I'm realizing it would be good to be more consistent with naming here. 
Other variables use "slave" instead of "agent", so probably better to just do 
`slaveFlags` for the flags?


- Greg Mann


On Jan. 10, 2018, 2:32 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65059/
> ---
> 
> (Updated Jan. 10, 2018, 2:32 a.m.)
> 
> 
> Review request for mesos, Gaston Kleiman, Greg Mann, and Jie Yu.
> 
> 
> Bugs: MESOS-8426
> https://issues.apache.org/jira/browse/MESOS-8426
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch makes all SLRP unit tests to use an allocation interval of
> 50ms and decline offers with filters.
> 
> 
> Diffs
> -
> 
>   src/tests/storage_local_resource_provider_tests.cpp 
> bbfe95e9818f25fdd5405db3ad2fe355e023f743 
> 
> 
> Diff: https://reviews.apache.org/r/65059/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 63829: Modified Containerizer::remove to allow top-level containers.

[Joseph Wu]

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

(Updated Jan. 17, 2018, 11:24 a.m.)


Review request for mesos, Gilbert Song, Jie Yu, and Zhitao Li.


Changes
---

Rebased.


Repository: mesos


Description
---

This includes two changes:
1) Containerizer::remove no longer CHECKs for a nested container as
   input and accepts standalone containers too.
2) Standalone containers no longer have their runtime directory
   deleted upon exit (which funnily, would prevent the containerizer
   from figuring out that the ContainerID is a standalone).

This change means that, for standalone containers, the caller will
need to explicitly remove the standalone container with the
REMOVE_CONTAINER API.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.cpp 
ec39d04e7129a4f8b87166f18d3203dc280c373b 
  src/tests/agent_container_api_tests.cpp 
618569277545205017320aaf1f3a70e540d35e30 


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

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


Testing
---

Doesn't build at the moment, as the dependent review needs a rebase (due to my 
earlier changes :) )


Thanks,

Joseph Wu

Re: Review Request 63861: Windows: Updated networking doc.

[Andrew Schwartzmeyer]

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


Ship it!




Ship It!

- Andrew Schwartzmeyer


On Jan. 12, 2018, 6:35 a.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63861/
> ---
> 
> (Updated Jan. 12, 2018, 6:35 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Andrew Schwartzmeyer, Gaston 
> Kleiman, Jie Yu, John Kordich, and Michael Park.
> 
> 
> Bugs: MESOS-7342
> https://issues.apache.org/jira/browse/MESOS-7342
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The networking docs now describe how the Docker network modes in the
> `Network` enum work on Windows, since the enum only has Linux network
> modes.
> 
> 
> Diffs
> -
> 
>   docs/networking.md bdd3a762435aae163fc659ccfea7da825983 
> 
> 
> Diff: https://reviews.apache.org/r/63861/diff/9/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 63848: Added test to check standalone containers in GET_CONTAINERS.

[Joseph Wu]

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

(Updated Jan. 17, 2018, 11:21 a.m.)


Review request for mesos, Gilbert Song and Jie Yu.


Changes
---

Rebased


Repository: mesos


Description
---

Standalone containers are *NOT* expected to show up in the output
of the agent GET_CONTAINERS call.  This is because the standalone
containers lack some of the required metadata, like a FrameworkID,
ExecutorID, and Executor name.


Diffs (updated)
-

  src/tests/api_tests.cpp fb45879310faf7edc7d101be206d49de89819c18 


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

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


Testing
---

make check


Thanks,

Joseph Wu

Re: Review Request 63828: Added authorization tests for standalone container APIs.

[Joseph Wu]

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

(Updated Jan. 17, 2018, 11:21 a.m.)


Review request for mesos, Alexander Rojas, Gilbert Song, and Jie Yu.


Changes
---

Rebased and updated symbols to include the plural-ized protobuf fields.


Repository: mesos


Description
---

The usage of the standalone container APIs is rather coarse-grained.
A principal can either use the APIs, or it cannot.  This is the case
for all four actions: Launch, Wait, Kill, and Remove.


Diffs (updated)
-

  src/tests/authorization_tests.cpp a76ad18a54ec232e6d6c92ec6a45b445a83f174c 


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

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


Testing
---

make check


Thanks,

Joseph Wu

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

[Andrew Schwartzmeyer]

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


Fix it, then Ship it!





src/docker/docker.cpp
Lines 752 (patched)


Nit: You want `// NOLINT(whitespace/line_length)` here.


- Andrew Schwartzmeyer


On Jan. 12, 2018, 6:34 a.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63860/
> ---
> 
> (Updated Jan. 12, 2018, 6:34 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Andrew Schwartzmeyer, Gaston 
> Kleiman, Jie Yu, John Kordich, Joseph Wu, and Michael Park.
> 
> 
> Bugs: MESOS-7342
> https://issues.apache.org/jira/browse/MESOS-7342
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The Network enum in DockerInfo is specific to Linux containers. `HOST`
> doesn't exist on Windows and `BRIDGE` is `NAT` on Windows. The current
> default docker network setting was always `HOST`, which broke the
> Windows docker executor. Now, if a specific network isn't given, the
> network mode will default to `HOST` on Linux agents and `NAT` on Windows
> agents. Also, `BRIDGE` mode will be translated to `NAT` on Windows.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 722a54ad113fc4e2bb22a8f08e307ab38d5fbfed 
> 
> 
> Diff: https://reviews.apache.org/r/63860/diff/9/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/63862/ for test results.
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

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

[Joseph Wu]

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


Ship it!




Ship It!

- Joseph Wu


On Jan. 16, 2018, 8:54 a.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63859/
> ---
> 
> (Updated Jan. 16, 2018, 8:54 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer and John Kordich.
> 
> 
> Bugs: MESOS-7342
> https://issues.apache.org/jira/browse/MESOS-7342
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed `SIGSTOP` and `SIGCONT` on Windows, since they are meaningless,
> and never unused. 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/os/windows/kill.hpp 
> b2a36b5c07df6642bb9b08b61ab3b678bf8a6b36 
>   3rdparty/stout/include/stout/windows.hpp 
> 7aa0ba72c4cc3b688ce6374b3308945ea8cb7572 
> 
> 
> Diff: https://reviews.apache.org/r/63859/diff/7/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/63862/ for test results.
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 64849: Added authentication to some example frameworks.

[Till Toenshoff]

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

(Updated Jan. 17, 2018, 7:01 p.m.)


Review request for mesos, Alexander Rukletsov, Armand Grillet, Greg Mann, Kapil 
Arya, and Vinod Kone.


Changes
---

Rebase on rebase fixup.


Bugs: MESOS-5362 and MESOS-8357
https://issues.apache.org/jira/browse/MESOS-5362
https://issues.apache.org/jira/browse/MESOS-8357


Repository: mesos


Description
---

All example frameworks now support authenticating when registering
to the master.


Diffs (updated)
-

  src/examples/dynamic_reservation_framework.cpp 
538fbe8847a1b1dcbfe48ad0e3678797801a12f5 
  src/examples/persistent_volume_framework.cpp 
71db39d7743d31ea34c2aed579d7a1ef2ed95687 
  src/examples/test_http_framework.cpp 482f65efc15a7bac52c33a57b2b876191249410a 
  src/tests/persistent_volume_framework_test.sh 
2ab22c03e573d4801c73957f9cad2939b3d3174b 


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

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


Testing
---

make check


Thanks,

Till Toenshoff

Re: Review Request 64848: Updated example frameworks to make use of added flags.

[Till Toenshoff]

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

(Updated Jan. 17, 2018, 6:59 p.m.)


Review request for mesos, Alexander Rukletsov, Armand Grillet, Greg Mann, Kapil 
Arya, and Vinod Kone.


Changes
---

Removed rebasing artefact - sorry for that slipup.


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


Repository: mesos


Description
---

The example frameworks now consistently rely on a set of common flags
for parameterizing. This unifies parameter names, descriptions and
validation for those examples.


Diffs (updated)
-

  src/examples/balloon_framework.cpp 410966e64aa3f146a88c00babaa51b499dd24d36 
  src/examples/disk_full_framework.cpp d75f769538d17229bdbf332ea7513b1aa8d7c334 
  src/examples/docker_no_executor_framework.cpp 
1fa408d3847d414c23eed8334db8de02e84cf764 
  src/examples/dynamic_reservation_framework.cpp 
538fbe8847a1b1dcbfe48ad0e3678797801a12f5 
  src/examples/load_generator_framework.cpp 
42867992eba8699cab5b70c62a24b87446139406 
  src/examples/long_lived_framework.cpp 
943160d5a42e02cf05fe92999233ce2e792849fd 
  src/examples/no_executor_framework.cpp 
972ef777cec07d1030af208daef69ebe606d071e 
  src/examples/persistent_volume_framework.cpp 
71db39d7743d31ea34c2aed579d7a1ef2ed95687 
  src/examples/test_framework.cpp acf1faf1523c8c03483dfafbc8f8e245322527e4 


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

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


Testing
---

make check


Thanks,

Till Toenshoff

Re: Review Request 65166: Added libprocess benchmarks to cmake build setup.

[Joseph Wu]

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


Fix it, then Ship it!





3rdparty/libprocess/src/tests/CMakeLists.txt
Lines 81-82 (patched)


Stylistically, we've cut back a bit on these header sections (we could 
probably revise the text in the stout/tests CMakeLists).  

It is nice to call out since benchmarks are not normally built.  You could 
change it to something like:
```
# LIBPROCESS BENCHMARK TESTS

```



3rdparty/libprocess/src/tests/CMakeLists.txt
Lines 91-93 (patched)


We don't want to build this executable by default, so drop a 
`EXCLUDE_FROM_ALL` in here.

```
add_executable(
  benchmarks EXCLUDE_FROM_ALL
  benchmarks.cpp
  benchmarks.pb.cc)
```


- Joseph Wu


On Jan. 15, 2018, 3:48 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65166/
> ---
> 
> (Updated Jan. 15, 2018, 3:48 a.m.)
> 
> 
> Review request for mesos and Andrew Schwartzmeyer.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added libprocess benchmarks to cmake build setup.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/CMakeLists.txt 
> 8b76cab4d549e18e34aef39863188f37cde74223 
> 
> 
> Diff: https://reviews.apache.org/r/65166/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> `ninja benchmarks && ./3rdparty/libprocess/src/tests/benchmarks`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 65090: Added specific flag loader for zookeeper urls preventing password leaks.

[Greg Mann]

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




include/mesos/zookeeper/url.hpp
Lines 32-36 (patched)


Code looks good to me, but I wonder if we should add this as a more generic 
abstraction? It’s possible that other people may want to use this in the 
future; placing it in a common location and giving it a more generic name may 
help people recognize that this could be useful to them.

The functionality which the new struct adds is the ability to have a flag 
which can be set to _either_ a path _or_ a value, without displaying the file’s 
contents in logs or API requests when a path is chosen.

So, perhaps a more generic name like `SecurePathOrValue` would improve 
readability? If we went this route, I think we could place the object and its 
related function overloads in stout.

WDYT?



src/slave/flags.hpp
Line 195 (original), 197 (patched)


Indentation.


- Greg Mann


On Jan. 11, 2018, 10:52 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65090/
> ---
> 
> (Updated Jan. 11, 2018, 10:52 a.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Bugs: MESOS-8413
> https://issues.apache.org/jira/browse/MESOS-8413
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds the struct `zookeeper::SafeUrlLoader` which is used to load
> zookeeper url's from flags.
> 
> This new struct will not shown the contents of a file in logs or the
> `/flags` endpoint if the urls are given using a file (with the
> `file://` prefix). If the url's are given in the command line, not
> changes are noticed.
> 
> 
> Diffs
> -
> 
>   include/mesos/zookeeper/url.hpp 72287d6d811de571c7a14d09ffc2f57e1b3f4446 
>   src/master/flags.hpp dabb414560f506787b6e821a27af623c8da44b11 
>   src/master/main.cpp f65ce637d77ce183f83b70dce6da8d0b4b8b8e71 
>   src/slave/flags.hpp 42c4861b5ecdc808d04bfe8f5c35572074fd2bdc 
>   src/slave/main.cpp f38fec6028ade0e0a51fd2cce7470c5c36e66396 
> 
> 
> Diff: https://reviews.apache.org/r/65090/diff/1/
> 
> 
> Testing
> ---
> 
> ```sh
> make -j12 check
> 
> # We don't seem to test flags in unit tests anywhere,
> # so additionally I ran:
> 
> docker pull zookeeper
> 
> cat < /tmp/$USER/zk-stack.yml
> version: '3.1'
> services:
>   zoo1:
> image: zookeeper
> restart: always
> hostname: zoo1
> ports:
>   - 2181:2181
> environment:
>   ZOO_MY_ID: 1
>   ZOO_SERVERS: server.1=0.0.0.0:2888:3888 server.2=zoo2:2888:3888 
> server.3=zoo3:2888:3888
>   zoo2:
> image: zookeeper
> restart: always
> hostname: zoo2
> ports:
>   - 2182:2181
> environment:
>   ZOO_MY_ID: 2
>   ZOO_SERVERS: server.1=zoo1:2888:3888 server.2=0.0.0.0:2888:3888 
> server.3=zoo3:2888:3888
>   zoo3:
> image: zookeeper
> restart: always
> hostname: zoo3
> ports:
>   - 2183:2181
> environment:
>   ZOO_MY_ID: 2
>   ZOO_SERVERS: server.1=zoo1:2888:3888 server.2=zoo2:2888:3888 
> server.3=0.0.0.0:2888:3888
> EOF
> 
> docker-compose -f /tmp/$USER/zk-stack.yml up
> 
> cd ${MESOS_BUILD_DIR}
> 
> # This command should fail to launch because there is no file zk.conf
> ./bin/mesos-master.sh \
> --work_dir=/tmp/$USER/mesos/master \
> --log_dir=/tmp/$USER/mesos/master/log \
> --ip=$PUBLIC_IP \
> --quorum=1 \
> --zk=file:///tmp/$USER/zk/zk.conf
> 
> cat < /tmp/$USER/zk/zk.conf
> zk://$PUBLIC_IP:2181,$PUBLIC_IP:2182,$PUBLIC_IP:2183/mesos
> EOF
> 
> ./bin/mesos-master.sh \
> --work_dir=/tmp/$USER/mesos/master \
> --log_dir=/tmp/$USER/mesos/master/log \
> --ip=$PUBLIC_IP \
> --quorum=1 \
> --zk=`cat /tmp/$USER/zk/zk.conf`  &
> 
> [[ $(http -b $PUBLIC_IP:5050/flags | jq -r '.flags.zk') == `cat 
> /tmp/$USER/zk/zk.conf` ]]
> 
> kill %1
> 
> 
> ./bin/mesos-master.sh \
> --work_dir=/tmp/$USER/mesos/master \
> --log_dir=/tmp/$USER/mesos/master/log \
> --ip=$PUBLIC_IP \
> --quorum=1 \
> --zk=file:///tmp/$USER/zk/zk.conf &
> 
> [[ $(http -b $PUBLIC_IP:5050/flags | jq -r '.flags.zk') == 
> "/tmp/$USER/zk/zk.conf" ]]
> 
> ./bin/mesos-agent.sh \
> --work_dir=/tmp/$USER/mesos/agent \
> --log_dir=/tmp/$USER/mesos/agent/log \
> --master=file:///tmp/$USER/zk/zk.conf &
> 
> [[ $(http -b $PUBLIC_IP:5051/flags | jq -r '.flags.master') == 
> "/tmp/$USER/zk/zk.conf" ]]
> 
> kill %2
> 
> ./bin/mesos-agent.sh \
> --work_dir=/tmp/$USER/mesos/agent \
> --log_dir=/tmp/$USER/mesos/agent/log \
> --zk=`cat /tmp/$USER/zk/zk.conf`  &
> 
> [[ $(http -b 

Re: Review Request 64848: Updated example frameworks to make use of added flags.

[Till Toenshoff]

> On Jan. 16, 2018, 11:19 p.m., Vinod Kone wrote:
> > src/examples/test_http_framework.cpp
> > Lines 399-401 (original), 401-403 (patched)
> > 
> >
> > hmm. this seems incorrect? how did this compile!?

Dang - that slipped in while rebasing.


- Till


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


On Jan. 15, 2018, 12:29 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64848/
> ---
> 
> (Updated Jan. 15, 2018, 12:29 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Armand Grillet, Greg Mann, 
> Kapil Arya, and Vinod Kone.
> 
> 
> Bugs: MESOS-8357
> https://issues.apache.org/jira/browse/MESOS-8357
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The example frameworks now consistently rely on a set of common flags
> for parameterizing. This unifies parameter names, descriptions and
> validation for those examples.
> 
> 
> Diffs
> -
> 
>   src/examples/balloon_framework.cpp 410966e64aa3f146a88c00babaa51b499dd24d36 
>   src/examples/disk_full_framework.cpp 
> d75f769538d17229bdbf332ea7513b1aa8d7c334 
>   src/examples/docker_no_executor_framework.cpp 
> 1fa408d3847d414c23eed8334db8de02e84cf764 
>   src/examples/dynamic_reservation_framework.cpp 
> 538fbe8847a1b1dcbfe48ad0e3678797801a12f5 
>   src/examples/load_generator_framework.cpp 
> 42867992eba8699cab5b70c62a24b87446139406 
>   src/examples/long_lived_framework.cpp 
> 943160d5a42e02cf05fe92999233ce2e792849fd 
>   src/examples/no_executor_framework.cpp 
> 972ef777cec07d1030af208daef69ebe606d071e 
>   src/examples/persistent_volume_framework.cpp 
> 71db39d7743d31ea34c2aed579d7a1ef2ed95687 
>   src/examples/test_framework.cpp acf1faf1523c8c03483dfafbc8f8e245322527e4 
>   src/examples/test_http_framework.cpp 
> 482f65efc15a7bac52c33a57b2b876191249410a 
> 
> 
> Diff: https://reviews.apache.org/r/64848/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 63661: Updated tests to use `createCallSubscribe`.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 63661 was successfully built and tested.

Reviews applied: `['63661']`

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

- Mesos Reviewbot Windows


On Jan. 17, 2018, 4:29 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63661/
> ---
> 
> (Updated Jan. 17, 2018, 4:29 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-8094
> https://issues.apache.org/jira/browse/MESOS-8094
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated all the tests that send v1 API SUBSCRIBE calls to use the
> `createCallSubscribe` test helper.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp fb45879310faf7edc7d101be206d49de89819c18 
>   src/tests/mesos.hpp 16c75bb0ca570dfb7089743344cf38acdb517705 
>   src/tests/slave_authorization_tests.cpp 
> 4ba0b8e96614a2df0daec576c08fe02462ccaa27 
>   src/tests/slave_recovery_tests.cpp 641fc6e82be19bdea55d35ec8f34a862de58489e 
>   src/tests/slave_tests.cpp 59e3065ad9aadbd90cdfd32e830b433b88d6de86 
> 
> 
> Diff: https://reviews.apache.org/r/63661/diff/4/
> 
> 
> Testing
> ---
> 
> ```
> $ make check
> ```
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 65179: Fixed flaky EOFBeforeRecv test.

[Alexander Rukletsov]

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


Ship it!




Ship It!

- Alexander Rukletsov


On Jan. 16, 2018, 6:10 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65179/
> ---
> 
> (Updated Jan. 16, 2018, 6:10 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Greg Mann, and Joseph Wu.
> 
> 
> Bugs: MESOS-7028
> https://issues.apache.org/jira/browse/MESOS-7028
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch is a small correction of commit d4185d4a11.
> Before this patch, if EOF event for a ssl socket is received before
> a client called `recv`, then this EOF event is lost. This leads to
> hanging `recv` on the ssl socket after calling `shutdown`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/libevent_ssl_socket.cpp 
> 1c95ebabfefd07aaeb053b965ab8e4550dfccaef 
> 
> 
> Diff: https://reviews.apache.org/r/65179/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check (Fedora 25)
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 65179: Fixed flaky EOFBeforeRecv test.

[Greg Mann]

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


Ship it!




Many thanks for this fix, Andrei!!

- Greg Mann


On Jan. 16, 2018, 6:10 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65179/
> ---
> 
> (Updated Jan. 16, 2018, 6:10 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Greg Mann, and Joseph Wu.
> 
> 
> Bugs: MESOS-7028
> https://issues.apache.org/jira/browse/MESOS-7028
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch is a small correction of commit d4185d4a11.
> Before this patch, if EOF event for a ssl socket is received before
> a client called `recv`, then this EOF event is lost. This leads to
> hanging `recv` on the ssl socket after calling `shutdown`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/libevent_ssl_socket.cpp 
> 1c95ebabfefd07aaeb053b965ab8e4550dfccaef 
> 
> 
> Diff: https://reviews.apache.org/r/65179/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check (Fedora 25)
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 63661: Updated tests to use `createCallSubscribe`.

[Armand Grillet]

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

(Updated Jan. 17, 2018, 4:29 p.m.)


Review request for mesos and Alexander Rukletsov.


Changes
---

Fixed issue.


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


Repository: mesos


Description
---

Updated all the tests that send v1 API SUBSCRIBE calls to use the
`createCallSubscribe` test helper.


Diffs (updated)
-

  src/tests/api_tests.cpp fb45879310faf7edc7d101be206d49de89819c18 
  src/tests/mesos.hpp 16c75bb0ca570dfb7089743344cf38acdb517705 
  src/tests/slave_authorization_tests.cpp 
4ba0b8e96614a2df0daec576c08fe02462ccaa27 
  src/tests/slave_recovery_tests.cpp 641fc6e82be19bdea55d35ec8f34a862de58489e 
  src/tests/slave_tests.cpp 59e3065ad9aadbd90cdfd32e830b433b88d6de86 


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

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


Testing
---

```
$ make check
```


Thanks,

Armand Grillet

Re: Review Request 62447: Reverted usage of `-isystem` flag.

[Mesos Reviewbot Windows]

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



FAIL: Failed to apply the dependent review: 62161.

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

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

Relevant logs:

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

```
error: missing binary patch data for '3rdparty/boost-1.65.0.tar.gz'
error: binary patch does not apply to '3rdparty/boost-1.65.0.tar.gz'
error: 3rdparty/boost-1.65.0.tar.gz: patch does not apply
```

- Mesos Reviewbot Windows


On Jan. 17, 2018, 4:10 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62447/
> ---
> 
> (Updated Jan. 17, 2018, 4:10 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This flag breaks the build of mesos against system libraries
> installed under /usr, because it generates a command line
> of `-isystem /usr/include`, which is explicitly not supported
> by gcc. See also https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70129
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 579aed83fc5c9cee17cf9b4b289a91705f64ceda 
>   configure.ac 59292885b506e9b6fab14107174db11739a587b8 
> 
> 
> Diff: https://reviews.apache.org/r/62447/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 65190: Updated comments in io switchboard.

[Alexander Rukletsov]

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


Ship it!




Ship It!

- Alexander Rukletsov


On Jan. 17, 2018, 4:11 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65190/
> ---
> 
> (Updated Jan. 17, 2018, 4:11 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Kevin Klues.
> 
> 
> Bugs: MESOS-7742
> https://issues.apache.org/jira/browse/MESOS-7742
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is addendum to review https://reviews.apache.org/r/65122
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/io/switchboard.cpp 
> a1a50ce5671685012d015845c2f6d07749e393eb 
> 
> 
> Diff: https://reviews.apache.org/r/65190/diff/1/
> 
> 
> Testing
> ---
> 
> None: not a functional change.
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 65122: Fixed race condition in io switchboard for ATTACH_CONTAINER_INPUT call.

[Alexander Rukletsov]

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


Ship it!




Ship It!

- Alexander Rukletsov


On Jan. 17, 2018, 2:14 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65122/
> ---
> 
> (Updated Jan. 17, 2018, 2:14 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Greg Mann, and Kevin Klues.
> 
> 
> Bugs: MESOS-7742
> https://issues.apache.org/jira/browse/MESOS-7742
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, io switchboard terminated itself after io redirect had
> been finished and before http response had been sent to the agent for
> `ATTACH_CONTAINER_INPUT` request. This patch adds a check that an
> input connection exists in io redirect completion callback to postpone
> termination until `ATTACH_CONTAINER_INPUT` is completely processed.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/io/switchboard.cpp 
> 4ef640616d69b91c225206444737a40f9571da55 
> 
> 
> Diff: https://reviews.apache.org/r/65122/diff/3/
> 
> 
> Testing
> ---
> 
> sudo make check (Fedora 25)
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 65190: Updated comments in io switchboard.

[Alexander Rukletsov]

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


Ship it!




Ship It!

- Alexander Rukletsov


On Jan. 17, 2018, 4:11 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65190/
> ---
> 
> (Updated Jan. 17, 2018, 4:11 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Kevin Klues.
> 
> 
> Bugs: MESOS-7742
> https://issues.apache.org/jira/browse/MESOS-7742
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is addendum to review https://reviews.apache.org/r/65122
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/io/switchboard.cpp 
> a1a50ce5671685012d015845c2f6d07749e393eb 
> 
> 
> Diff: https://reviews.apache.org/r/65190/diff/1/
> 
> 
> Testing
> ---
> 
> None: not a functional change.
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 65190: Updated comments in io switchboard.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65190 was successfully built and tested.

Reviews applied: `['65190']`

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

- Mesos Reviewbot Windows


On Jan. 17, 2018, 4:11 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65190/
> ---
> 
> (Updated Jan. 17, 2018, 4:11 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Kevin Klues.
> 
> 
> Bugs: MESOS-7742
> https://issues.apache.org/jira/browse/MESOS-7742
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is addendum to review https://reviews.apache.org/r/65122
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/io/switchboard.cpp 
> a1a50ce5671685012d015845c2f6d07749e393eb 
> 
> 
> Diff: https://reviews.apache.org/r/65190/diff/1/
> 
> 
> Testing
> ---
> 
> None: not a functional change.
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 65122: Fixed race condition in io switchboard for ATTACH_CONTAINER_INPUT call.

[Alexander Rukletsov]

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



Follow up review: https://reviews.apache.org/r/65190/

- Alexander Rukletsov


On Jan. 17, 2018, 2:14 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65122/
> ---
> 
> (Updated Jan. 17, 2018, 2:14 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Greg Mann, and Kevin Klues.
> 
> 
> Bugs: MESOS-7742
> https://issues.apache.org/jira/browse/MESOS-7742
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, io switchboard terminated itself after io redirect had
> been finished and before http response had been sent to the agent for
> `ATTACH_CONTAINER_INPUT` request. This patch adds a check that an
> input connection exists in io redirect completion callback to postpone
> termination until `ATTACH_CONTAINER_INPUT` is completely processed.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/io/switchboard.cpp 
> 4ef640616d69b91c225206444737a40f9571da55 
> 
> 
> Diff: https://reviews.apache.org/r/65122/diff/3/
> 
> 
> Testing
> ---
> 
> sudo make check (Fedora 25)
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 65190: Updated comments in io switchboard.

[Andrei Budnik]

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

(Updated Jan. 17, 2018, 4:11 p.m.)


Review request for mesos, Alexander Rukletsov and Kevin Klues.


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


Repository: mesos


Description
---

This is addendum to review https://reviews.apache.org/r/65122


Diffs
-

  src/slave/containerizer/mesos/io/switchboard.cpp 
a1a50ce5671685012d015845c2f6d07749e393eb 


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


Testing
---

None: not a functional change.


Thanks,

Andrei Budnik

Re: Review Request 62447: Reverted usage of `-isystem` flag.

[Benno Evers]

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

(Updated Jan. 17, 2018, 4:10 p.m.)


Review request for mesos, Benjamin Bannier and Till Toenshoff.


Changes
---

Rebased onto latest master.


Summary (updated)
-

Reverted usage of `-isystem` flag.


Repository: mesos


Description (updated)
---

This flag breaks the build of mesos against system libraries
installed under /usr, because it generates a command line
of `-isystem /usr/include`, which is explicitly not supported
by gcc. See also https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70129


Diffs (updated)
-

  3rdparty/CMakeLists.txt 579aed83fc5c9cee17cf9b4b289a91705f64ceda 
  configure.ac 59292885b506e9b6fab14107174db11739a587b8 


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

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


Testing
---


Thanks,

Benno Evers

Re: Review Request 62445: Removed duplicate block in configure.ac.

[Benno Evers]

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

(Updated Jan. 17, 2018, 4:10 p.m.)


Review request for mesos, Benjamin Bannier and Till Toenshoff.


Changes
---

Rebased onto latest master.


Summary (updated)
-

Removed duplicate block in configure.ac.


Repository: mesos


Description
---

This blocks seems to have been copy/pasted from another place.


Diffs (updated)
-

  configure.ac 59292885b506e9b6fab14107174db11739a587b8 


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

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


Testing
---


Thanks,

Benno Evers

Re: Review Request 62161: Updated boost version.

[Benno Evers]

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

(Updated Jan. 17, 2018, 4:09 p.m.)


Review request for mesos, Benjamin Bannier and Till Toenshoff.


Changes
---

Rebased onto latest master; fixed order of commits.


Summary (updated)
-

Updated boost version.


Repository: mesos


Description (updated)
---

Updated boost version.


Diffs (updated)
-

  3rdparty/boost-1.53.0.tar.gz 175f373c330ff69bdb0d44fc75a29982e68554ab 
  3rdparty/boost-1.65.0.tar.gz PRE-CREATION 
  3rdparty/cmake/Versions.cmake ce6ecf5f37613be464153f89f1b6e77155673855 
  3rdparty/versions.am bd3553396334a1f1ac3a6958088fbf9988f915f3 


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

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


Testing
---


File Attachments


0001-Update-boost-version.patch
  
https://reviews.apache.org/media/uploaded/files/2017/10/06/6101ab66-98c3-45f3-a91b-dc046e24dca5__0001-Update-boost-version.patch


Thanks,

Benno Evers

Re: Review Request 62444: Added UNREACHABLE() macro to __cxa_pure_virtual.

[Benno Evers]

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

(Updated Jan. 17, 2018, 4:07 p.m.)


Review request for mesos, Benjamin Bannier and Till Toenshoff.


Changes
---

Rebased onto latest master.


Summary (updated)
-

Added UNREACHABLE() macro to __cxa_pure_virtual.


Repository: mesos


Description
---

The function __cxa_pure_virtual must not return,
but newer versions of clang detect that the expansion
of the RAW_LOG() macro contains returning code paths
for arguments other than FATAL.


Diffs (updated)
-

  src/logging/logging.cpp 19dba7faac7f2307e5dfdf15f64849f643b19129 


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

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


Testing
---


Thanks,

Benno Evers

Re: Review Request 63661: Updated tests to use `createCallSubscribe`.

[Alexander Rukletsov]

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


Fix it, then Ship it!





src/tests/mesos.hpp
Lines 2144-2145 (patched)


Why not a on-liner?
```
call.mutable_subscribe()->mutable_framework_info()->CopyFrom(frameworkInfo);
```


- Alexander Rukletsov


On Nov. 8, 2017, 10:46 a.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63661/
> ---
> 
> (Updated Nov. 8, 2017, 10:46 a.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-8094
> https://issues.apache.org/jira/browse/MESOS-8094
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated all the tests that send v1 API SUBSCRIBE calls to use the
> `createCallSubscribe` test helper.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp fb45879310faf7edc7d101be206d49de89819c18 
>   src/tests/mesos.hpp 16c75bb0ca570dfb7089743344cf38acdb517705 
>   src/tests/slave_authorization_tests.cpp 
> 4ba0b8e96614a2df0daec576c08fe02462ccaa27 
>   src/tests/slave_recovery_tests.cpp 641fc6e82be19bdea55d35ec8f34a862de58489e 
>   src/tests/slave_tests.cpp 59e3065ad9aadbd90cdfd32e830b433b88d6de86 
> 
> 
> Diff: https://reviews.apache.org/r/63661/diff/3/
> 
> 
> Testing
> ---
> 
> ```
> $ make check
> ```
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Review Request 65190: Updated comments in io switchboard.

[Andrei Budnik]

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

Review request for mesos, Alexander Rukletsov and Kevin Klues.


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


Repository: mesos


Description
---

This is addendum to review https://reviews.apache.org/r/65122


Diffs
-

  src/slave/containerizer/mesos/io/switchboard.cpp 
a1a50ce5671685012d015845c2f6d07749e393eb 


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


Testing
---

None: not a functional change.


Thanks,

Andrei Budnik

Re: Review Request 65126: Added a resource provider test case.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65126 was successfully built and tested.

Reviews applied: `['65125', '65126']`

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

- Mesos Reviewbot Windows


On Jan. 17, 2018, 1:05 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65126/
> ---
> 
> (Updated Jan. 17, 2018, 1:05 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-8445
> https://issues.apache.org/jira/browse/MESOS-8445
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a resource provider test case.
> 
> 
> Diffs
> -
> 
>   src/tests/master_tests.cpp 0edf22431aad85945aeb808a05f11e0bd832bccf 
> 
> 
> Diff: https://reviews.apache.org/r/65126/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 65122: Fixed race condition in io switchboard for ATTACH_CONTAINER_INPUT call.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65122 was successfully built and tested.

Reviews applied: `['65122']`

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

- Mesos Reviewbot Windows


On Jan. 17, 2018, 2:14 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65122/
> ---
> 
> (Updated Jan. 17, 2018, 2:14 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Greg Mann, and Kevin Klues.
> 
> 
> Bugs: MESOS-7742
> https://issues.apache.org/jira/browse/MESOS-7742
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, io switchboard terminated itself after io redirect had
> been finished and before http response had been sent to the agent for
> `ATTACH_CONTAINER_INPUT` request. This patch adds a check that an
> input connection exists in io redirect completion callback to postpone
> termination until `ATTACH_CONTAINER_INPUT` is completely processed.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/io/switchboard.cpp 
> 4ef640616d69b91c225206444737a40f9571da55 
> 
> 
> Diff: https://reviews.apache.org/r/65122/diff/3/
> 
> 
> Testing
> ---
> 
> sudo make check (Fedora 25)
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 65122: Fixed race condition in io switchboard for ATTACH_CONTAINER_INPUT call.

[Andrei Budnik]

> On Jan. 16, 2018, 7:21 p.m., Kevin Klues wrote:
> > src/slave/containerizer/mesos/io/switchboard.cpp
> > Lines 1223 (patched)
> > 
> >
> > If I understand this correctly, this is solving for the case where we 
> > currently have input connected but our output IO has completed. In such a 
> > case, we don't want to terminate immediately, but instead hold off  
> > terminating until the input connection has been closed.
> > 
> > I think the code you have is correct, but we should have a more 
> > detailed comment here about when the process will be terminated in the case 
> > where `inputConnected` is true.
> > 
> > Likewise on line 1673, we should have a more detailed comment as to why 
> > we need to handle this case here.

Updated comments.


> On Jan. 16, 2018, 7:21 p.m., Kevin Klues wrote:
> > src/slave/containerizer/mesos/io/switchboard.cpp
> > Lines 1673 (patched)
> > 
> >
> > We shoudl reverse the order of these to fist check `redirectFinished` 
> > before checking `failure.isSome()`. This will make writing the comment I 
> > mentioned in my other review flow more easily.

Updated.


- Andrei


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


On Jan. 17, 2018, 2:14 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65122/
> ---
> 
> (Updated Jan. 17, 2018, 2:14 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Greg Mann, and Kevin Klues.
> 
> 
> Bugs: MESOS-7742
> https://issues.apache.org/jira/browse/MESOS-7742
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, io switchboard terminated itself after io redirect had
> been finished and before http response had been sent to the agent for
> `ATTACH_CONTAINER_INPUT` request. This patch adds a check that an
> input connection exists in io redirect completion callback to postpone
> termination until `ATTACH_CONTAINER_INPUT` is completely processed.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/io/switchboard.cpp 
> 4ef640616d69b91c225206444737a40f9571da55 
> 
> 
> Diff: https://reviews.apache.org/r/65122/diff/3/
> 
> 
> Testing
> ---
> 
> sudo make check (Fedora 25)
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 65122: Fixed race condition in io switchboard for ATTACH_CONTAINER_INPUT call.

[Andrei Budnik]

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

(Updated Jan. 17, 2018, 2:14 p.m.)


Review request for mesos, Alexander Rukletsov, Greg Mann, and Kevin Klues.


Changes
---

Updated comments.


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


Repository: mesos


Description (updated)
---

Previously, io switchboard terminated itself after io redirect is
finished and before http response was sent to the agent for
`ATTACH_CONTAINER_INPUT` request. This patch adds a check that an
input connection exists in io redirect completion callback to postpone
termination until `ATTACH_CONTAINER_INPUT` is completely processed.


Diffs (updated)
-

  src/slave/containerizer/mesos/io/switchboard.cpp 
4ef640616d69b91c225206444737a40f9571da55 


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

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


Testing
---

sudo make check (Fedora 25)
internal CI


Thanks,

Andrei Budnik

Re: Review Request 65189: Displayed all needed commands in apply-reviews script dry-run mode.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65189 was successfully built and tested.

Reviews applied: `['65189']`

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

- Mesos Reviewbot Windows


On Jan. 17, 2018, 12:22 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65189/
> ---
> 
> (Updated Jan. 17, 2018, 12:22 p.m.)
> 
> 
> Review request for mesos, Armand Grillet and Kevin Klues.
> 
> 
> Bugs: MESOS-8447
> https://issues.apache.org/jira/browse/MESOS-8447
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a shell command to create a file containing the commit
> message for dry-run mode. To minimize platform differences we use
> `printf` instead of `echo`. We output the message on a single line
> for compactness.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py 0ef28cb02bc65acfeb7ea6808f74e1620a8a85c4 
> 
> 
> Diff: https://reviews.apache.org/r/65189/diff/1/
> 
> 
> Testing
> ---
> 
> Redirected output of `apply-reviews.py --dry-run` to a file and successfully 
> applied its output with `sh`.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 65125: Added a helper function for resource provider tests.

[Jan Schlicht]

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

(Updated Jan. 17, 2018, 2:43 p.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


Changes
---

Rebased and addressed issues.


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


Repository: mesos


Description
---

Added a helper function for resource provider tests.


Diffs (updated)
-

  src/tests/api_tests.cpp 6faefc92318c2db0e611d60f187738d0cbb269ff 
  src/tests/mesos.hpp 16c75bb0ca570dfb7089743344cf38acdb517705 
  src/tests/resource_provider_manager_tests.cpp 
d80823c47963e969113dae3623b18b7639c890fc 
  src/tests/slave_tests.cpp 59e3065ad9aadbd90cdfd32e830b433b88d6de86 


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

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


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 64604: Windows: Updated heath-checks.md with Windows implementation.

[Mesos Reviewbot Windows]

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



FAIL: Some Mesos tests failed.

Reviews applied: `['63859', '63860', '63861', '64386', '65127', '64387', 
'64604']`

Failed command: `D:\DCOS\mesos\src\mesos-tests.exe --verbose`

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

Relevant logs:

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

```
[   OK ] Endpoint/SlaveEndpointTest.NoAuthorizer/2 (103 ms)
[--] 9 tests from Endpoint/SlaveEndpointTest (940 ms total)

[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0 (35 
ms)
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1 (44 
ms)
[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest (80 ms 
total)

[--] 1 test from IsolationFlag/CpuIsolatorTest
[ RUN  ] IsolationFlag/CpuIsolatorTest.ROOT_UserCpuUsage/0
[   OK ] IsolationFlag/CpuIsolatorTest.ROOT_UserCpuUsage/0 (2451 ms)
[--] 1 test from IsolationFlag/CpuIsolatorTest (2477 ms total)

[--] 1 test from IsolationFlag/MemoryIsolatorTest
[ RUN  ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0
[   OK ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0 (2370 ms)
[--] 1 test from IsolationFlag/MemoryIsolatorTest (2394 ms total)

[--] Global test environment tear-down
[==] 849 tests from 85 test cases ran. (310470 ms total)
[  PASSED  ] 849 tests.
[  FAILED  ] 0 tests, listed below:

 0 FAILED TESTS

  YOU HAVE 213 DISABLED TESTS

```

- 
[mesos-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64604/logs/mesos-tests-stderr.log):

```
I0117 13:21:08.486173  7516 slave.cpp:931] Agent terminating
I0117 13:21:08.486173  9416 hierarchical.cpp:405] Deactivated framework 
ec134c73-a9f3-41e7-a12e-0f24a2ef25d4-
I0117 13:21:08.486173  7516 slave.cpp:3388] Shutting down framework 
ec134c73-a9f3-41e7-a12e-0f24a2ef25d4-
I0117 13:21:08.486173  7516 slave.cpp:6081] Shutting down executor 
'927c87fa-6f2e-496c-824a-77eca8ae33a0' of framework 
ec134c73-a9f3-41e7-a12e-0f24a2ef25d4- at executor(1)@10.3.1.11:51813
I0117 13:21:08.4871I0117 13:21:07.821267 11840 exec.cpp:162] Version: 1.6.0
I0117 13:21:07.845260 15324 exec.cpp:236] Executor registered on agent 
ec134c73-a9f3-41e7-a12e-0f24a2ef25d4-S0
I0117 13:21:07.848260 10676 executor.cpp:171] Received SUBSCRIBED event
I0117 13:21:07.853286 10676 executor.cpp:175] Subscribed executor on 
build-srv-03.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net
I0117 13:21:07.853286 10676 executor.cpp:171] Received LAUNCH event
I0117 13:21:07.857283 10676 executor.cpp:638] Starting task 
927c87fa-6f2e-496c-824a-77eca8ae33a0
I0117 13:21:07.938252 10676 executor.cpp:478] Running 
'D:\DCOS\mesos\src\mesos-containerizer.exe launch '
I0117 13:21:08.460178 10676 executor.cpp:651] Forked command at 13496
I0117 13:21:08.488173 14384 exec.cpp:445] Executor asked to shutdown
I0117 13:21:08.488173 10976 executor.cpp:171] Received SHUTDOWN event
I0117 13:21:08.489173 10976 executor.cpp:756] Shutting down
I0117 13:21:08.489173 10976 executor.cpp:871] Sending SIGTERM to process tree 
at pid 74 10828 master.cpp:10161] Updating the state of task 
927c87fa-6f2e-496c-824a-77eca8ae33a0 of framework 
ec134c73-a9f3-41e7-a12e-0f24a2ef25d4- (latest state: TASK_KILLED, status 
update state: TASK_KILLED)
I0117 13:21:08.489173 13192 containerizer.cpp:2347] Destroying container 
332962cf-22e3-4a41-b49b-f6da7e0f03ab in RUNNING state
I0117 13:21:08.490172 13192 containerizer.cpp:2961] Transitioning the state of 
container 332962cf-22e3-4a41-b49b-f6da7e0f03ab from RUNNING to DESTROYING
I0117 13:21:08.489173 10828 master.cpp:10265] Removing task 
927c87fa-6f2e-496c-824a-77eca8ae33a0 with resources cpus(allocated: *):4; 
mem(allocated: *):2048; disk(allocated: *):1024; ports(allocated: 
*):[31000-32000] of framework ec134c73-a9f3-41e7-a12e-0f24a2ef25d4- on 
agent ec134c73-a9f3-41e7-a12e-0f24a2ef25d4-S0 at slave(329)@10.3.1.11:51792 
(build-srv-03.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0117 13:21:08.490172 13192 launcher.cpp:156] Asked to destroy container 
332962cf-22e3-4a41-b49b-f6da7e0f03ab
I0117 13:21:08.491173 10828 master.cpp:1306] Agent 
ec134c73-a9f3-41e7-a12e-0f24a2ef25d4-S0 at slave(329)@10.3.1.11:51792 
(build-srv-03.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net) disconnected
I0117 13:21:08.492173 10828 master.cpp:3287] Disconnecting agent 
ec134c73-a9f3-41e7-a12e-0f24a2ef25d4-S0 

Re: Review Request 65180: Removed a comment in io switchboard.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65180 was successfully built and tested.

Reviews applied: `['65180']`

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

- Mesos Reviewbot Windows


On Jan. 17, 2018, 11:36 a.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65180/
> ---
> 
> (Updated Jan. 17, 2018, 11:36 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The comment is incorrect since io switchboard allows reconnects
> after agent restarts.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/io/switchboard.cpp 
> 4ef640616d69b91c225206444737a40f9571da55 
> 
> 
> Diff: https://reviews.apache.org/r/65180/diff/2/
> 
> 
> Testing
> ---
> 
> None: not a functional change.
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 65126: Added a resource provider test case.

[Jan Schlicht]

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

(Updated Jan. 17, 2018, 2:05 p.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


Changes
---

Addressed issues.


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


Repository: mesos


Description
---

Added a resource provider test case.


Diffs (updated)
-

  src/tests/master_tests.cpp 0edf22431aad85945aeb808a05f11e0bd832bccf 


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

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


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 64823: Documented the change in the Protobuf requirement.

[Mesos Reviewbot Windows]

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



FAIL: Failed to apply the current review.

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

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

Relevant logs:

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

```
error: patch failed: CHANGELOG:11
error: CHANGELOG: patch does not apply
error: patch failed: docs/upgrades.md:51
error: docs/upgrades.md: patch does not apply
```

- Mesos Reviewbot Windows


On Dec. 23, 2017, 12:21 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64823/
> ---
> 
> (Updated Dec. 23, 2017, 12:21 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, and Qian Zhang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documented the change in the Protobuf requirement.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 179b53b0568e4d993553f5cdb13668d96aa400d0 
>   docs/upgrades.md f2eb5a94d67b44a02d355981a673bd4ef5129653 
> 
> 
> Diff: https://reviews.apache.org/r/64823/diff/1/
> 
> 
> Testing
> ---
> 
> N/A
> 
> The requirement is checked in https://reviews.apache.org/r/64822/.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 65162: Removed some redundant `get` calls.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65162 was successfully built and tested.

Reviews applied: `['65160', '65161', '65162']`

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

- Mesos Reviewbot Windows


On Jan. 15, 2018, 10:09 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65162/
> ---
> 
> (Updated Jan. 15, 2018, 10:09 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed some redundant `get` calls.
> 
> 
> Diffs
> -
> 
>   src/appc/spec.cpp fc5e24ee5a459522a7a2dc52b4f168c4386b0642 
>   src/authentication/cram_md5/auxprop.cpp 
> 8a20d5bd2d943faa10d2920f76756fb9527075d8 
>   src/cli/execute.cpp 6e626893a64dc07ac9a5f9a5c31e680cb80938fe 
>   src/common/command_utils.cpp c50be7608df3a7e0a6a9d0bb46525a3cbd025bee 
>   src/common/http.cpp 728fc554917ed031f9cb3d811fbbc064307b3e32 
>   src/common/parse.hpp 212d6406a16f5ffd0494197dc44271629dc5ba3b 
>   src/common/protobuf_utils.cpp fda13072743d57d29cdf7ac3c76c0d42a8d36116 
>   src/common/recordio.hpp c58bb14866a3ec3841b09d2e96bb16af20a2e6fd 
>   src/credentials/credentials.hpp c790793c7ea5ed384bdb397bfc1592b8fd1ff327 
>   src/docker/docker.cpp 722a54ad113fc4e2bb22a8f08e307ab38d5fbfed 
>   src/docker/spec.cpp 538cf1883d0dbf953ab5017d9b7d54a68ee72c73 
>   src/executor/executor.cpp 1c972d92984a15d12466e3dd229b16fa60e21ebb 
>   src/files/files.cpp 324b4bcf0ef53931fa7ef9b7a891b160564705ff 
>   src/hook/manager.cpp c659de5418881a77d89e6c579152614c2b47592b 
>   src/internal/evolve.cpp 7758c9b93ad41b45b941c0de8c2b1008fbc4e50d 
>   src/launcher/executor.cpp 050f5a057f360873e2b4738b126289bcd1bd0c7f 
>   src/launcher/fetcher.cpp 7fc69fe22b362bf0d13d629829a2c6a9de4f1579 
>   src/linux/cgroups.cpp cdd0f40ec5748b909e529f21b15523122bf90d41 
>   src/linux/fs.cpp 94e78df0336f63c58eb3d4da0d7fe45dfcb493c1 
>   src/linux/ns.cpp 446640705b7c060776ed87655fb17509865cd335 
>   src/local/local.cpp ba1bcf21df70bba520f3f9b671bb57d498335c44 
>   src/log/catchup.cpp 79cc18c4311d14c82801d856681171c5f1ce95c8 
>   src/log/consensus.cpp f766015fac44cce173ec7ef3f5dd3f0ac9e03ef5 
>   src/log/replica.cpp c86a609f7fc7fcfd42761180c464c383961a986e 
>   src/log/tool/benchmark.cpp b939e26aceafc47015f3f0a93c5ce6a1f8aca605 
>   src/log/tool/initialize.cpp c0f85439a87f939bad076733fbc9e12b60f5b8f9 
>   src/log/tool/read.cpp 50fb0209a67251eb6ae320f3cd6dcd7ba11bdd99 
>   src/master/allocator/mesos/hierarchical.cpp 
> f33ff767dcb93556beb696c96f8cfc17baccb05e 
>   src/master/contender/contender.cpp ea7453d1d4753a77c6161ec0b812bb0bf38483dd 
>   src/master/contender/zookeeper.cpp 64fbb3adbb0812f430fe152550a8a8707c0fed8f 
>   src/master/detector/detector.cpp 9d2e8c4d3fb342fe4ada01628b33b7c470ba63b6 
>   src/master/detector/zookeeper.cpp 1cab567615d756ebd1a40549718c47b642cf0eb8 
>   src/master/http.cpp cae67b8e905a11b0825399aeb540ca259b8938ed 
>   src/master/main.cpp f65ce637d77ce183f83b70dce6da8d0b4b8b8e71 
>   src/master/master.hpp 3d5180b8028b2a15ad111f91863e77747b362593 
>   src/master/master.cpp 465336d33008a848df063d4295416eb91f7bb44f 
>   src/master/quota_handler.cpp 37d43e1d9473b750ffd76701472e7448aa61c8c3 
>   src/master/registrar.cpp aabce22e4b06826e4452c61c27ebdd702b1e47bc 
>   src/master/validation.cpp f0b86775b7919ba6aa4a73038edb78a0adca68f4 
>   src/master/weights_handler.cpp 59104a85d2b74cf42461b02d85101c7c751f706c 
>   src/resource_provider/http_connection.hpp 
> add5acc47aaf79b50ef9a7bd1e17ff0db5b95439 
>   src/resource_provider/registrar.cpp 
> 3e6b64bae1c56d3ca27edd0debac428b47f17d04 
>   src/sched/sched.cpp 613a33b7d3150ad851af674e989a84b3899f2d69 
>   src/scheduler/scheduler.cpp 07d2b37e0874edd27365719705272568d386323b 
>   src/slave/container_loggers/lib_logrotate.cpp 
> bc13e6a52460d4607873c4b0022165038051bc51 
>   src/slave/containerizer/containerizer.cpp 
> fa1d24a56380dd34ed15d3d12e610dfcfe4c56d6 
>   src/slave/containerizer/docker.cpp 6d9c598a5e60a01dbb3e10108aeed6c650338664 
>   src/slave/containerizer/fetcher.cpp 
> 8b26e882eced9478ffdc6b2dcbc0a5796c5c3ce2 
>   src/slave/containerizer/mesos/containerizer.cpp 
> f69f65a5033ce0c1fa2fe9591d8c6bbada5e09fb 
>   src/slave/containerizer/mesos/io/switchboard.cpp 
> 4ef640616d69b91c225206444737a40f9571da55 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpu.cpp 
> fc763bd7834567882146ad25e0266b1183154dc3 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpuacct.cpp 
> ffa0d3b0e6f3d8a1f55c88d61f20cff426119e20 
>   

Review Request 65189: Displayed all needed commands in apply-reviews script dry-run mode.

[Benjamin Bannier]

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

Review request for mesos, Armand Grillet and Kevin Klues.


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


Repository: mesos


Description
---

This patch adds a shell command to create a file containing the commit
message for dry-run mode. To minimize platform differences we use
`printf` instead of `echo`. We output the message on a single line
for compactness.


Diffs
-

  support/apply-reviews.py 0ef28cb02bc65acfeb7ea6808f74e1620a8a85c4 


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


Testing
---

Redirected output of `apply-reviews.py --dry-run` to a file and successfully 
applied its output with `sh`.


Thanks,

Benjamin Bannier

Re: Review Request 64387: Windows: Ported docker health check tests.

[Akash Gupta]

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

(Updated Jan. 17, 2018, 12:12 p.m.)


Review request for mesos, Alexander Rukletsov, Andrew Schwartzmeyer, Gaston 
Kleiman, Jie Yu, Joseph Wu, and Michael Park.


Changes
---

Removed ifdef on other pre-pull


Repository: mesos


Description
---

The `HealthCheckTest.ROOT_DOCKER_*` and
`DockerContainerizerHealthCheckTest.*` tests now work on Windows.


Diffs (updated)
-

  src/tests/environment.cpp 72bd621f02f97ea5fd553f3dc0bd52adb8ddee8f 
  src/tests/health_check_tests.cpp 1893c85169f5e94e164434b93e6a24268224225d 
  src/tests/mesos.hpp 16c75bb0ca570dfb7089743344cf38acdb517705 


Diff: https://reviews.apache.org/r/64387/diff/8/

Changes: https://reviews.apache.org/r/64387/diff/7-8/


Testing
---

Windows Server:
[==] Running 5 tests from 2 test cases.
[--] Global test environment set-up.
[--] 2 tests from HealthCheckTest
[ RUN  ] HealthCheckTest.ROOT_DOCKER_DockerHealthyTask
[   OK ] HealthCheckTest.ROOT_DOCKER_DockerHealthyTask (21263 ms)
[ RUN  ] HealthCheckTest.ROOT_DOCKER_DockerHealthStatusChange
[   OK ] HealthCheckTest.ROOT_DOCKER_DockerHealthStatusChange (23512 ms)
[--] 2 tests from HealthCheckTest (44835 ms total)

[--] 3 tests from NetworkProtocol/DockerContainerizerHealthCheckTest
[ RUN  ] 
NetworkProtocol/DockerContainerizerHealthCheckTest.ROOT_DOCKER_USERNETWORK_NETNAMESPACE_HealthyTaskViaHTTP/0
[   OK ] 
NetworkProtocol/DockerContainerizerHealthCheckTest.ROOT_DOCKER_USERNETWORK_NETNAMESPACE_HealthyTaskViaHTTP/0
 (28487 ms)
[ RUN  ] 
NetworkProtocol/DockerContainerizerHealthCheckTest.ROOT_DOCKER_USERNETWORK_NETNAMESPACE_HealthyTaskViaHTTPS/0
[   OK ] 
NetworkProtocol/DockerContainerizerHealthCheckTest.ROOT_DOCKER_USERNETWORK_NETNAMESPACE_HealthyTaskViaHTTPS/0
 (26447 ms)
[ RUN  ] 
NetworkProtocol/DockerContainerizerHealthCheckTest.ROOT_DOCKER_USERNETWORK_NETNAMESPACE_HealthyTaskViaTCP/0
[   OK ] 
NetworkProtocol/DockerContainerizerHealthCheckTest.ROOT_DOCKER_USERNETWORK_NETNAMESPACE_HealthyTaskViaTCP/0
 (26264 ms)
[--] 3 tests from NetworkProtocol/DockerContainerizerHealthCheckTest 
(81268 ms total)

[--] Global test environment tear-down
[==] 5 tests from 2 test cases ran. (126559 ms total)
[  PASSED  ] 5 tests

Rest of tests pass.

Windows Client (Disabled network health checks):
Proof that network health checks are skipped on Windows Client.
C:\Program Files\Docker\Docker\Resources\bin\docker.exe: Error response from 
daemon: sharing of hyperv containers network is not supported.
356b087e7fa640f83fe27ebeb3396bfc7b2bbebd917aeaec0508b887b41d31f4
-
We cannot run any Docker health checks tests because:
Running in another container's namespace is not supported on this version of 
Windows.

Rest rests pass.

Linux:
make check passes


Thanks,

Akash Gupta

Re: Review Request 64386: Refactored health checks to take in executor information.

[Akash Gupta]

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

(Updated Jan. 17, 2018, 12:10 p.m.)


Review request for mesos, Alexander Rukletsov, Andrew Schwartzmeyer, Gaston 
Kleiman, Jie Yu, Joseph Wu, and Michael Park.


Changes
---

Fixed linux test break on Docker `COMMNAD` health check


Repository: mesos


Description
---

Added information about the calling executor to the health check
library. After the refactoring, the command health check code originally
in the docker executor is now in the health check library.


Diffs (updated)
-

  src/checks/checker.hpp 93502270f31e80c5f7c94b5b456625e9cdea1837 
  src/checks/checker.cpp fff0aac504b4283a210f936e00c977fa60d88b3d 
  src/checks/checker_process.hpp 510f3b2e6e689faaf26595214ce377c2b5518f28 
  src/checks/checker_process.cpp ddb197b8cc2c503fef5ae20af32f5881fff9833f 
  src/checks/health_checker.hpp 019fbd791f250ecc28ff59d779f90e7ccbf0c685 
  src/checks/health_checker.cpp eaf9a18817eeeff7c29c7a4b9d1b183f398760a3 
  src/docker/docker.hpp d9e71f8841a868082170d28fc4f3d495e2eb1e61 
  src/docker/executor.cpp e4c53d558e414e50b1c429fba8e31e504c63744a 
  src/launcher/default_executor.cpp 4a619859095cc2d30f4806813f64a2e48c83b3ea 
  src/launcher/executor.cpp 050f5a057f360873e2b4738b126289bcd1bd0c7f 


Diff: https://reviews.apache.org/r/64386/diff/8/

Changes: https://reviews.apache.org/r/64386/diff/7-8/


Testing
---

See https://reviews.apache.org/r/64387/


Thanks,

Akash Gupta

Re: Review Request 65180: Removed a comment in io switchboard.

[Andrei Budnik]

> On Jan. 16, 2018, 7:24 p.m., Kevin Klues wrote:
> > src/slave/containerizer/mesos/io/switchboard.cpp
> > Lines 666-669 (original)
> > 
> >
> > Does 4069e1c424 address making sure the IO-Switchboard allows 
> > reconnects after agent restarts?
> 
> Andrei Budnik wrote:
> I should probably mention both `4069e1c424` and `9da19f5cc7` as they have 
> changed the logic described in this comment.

Description updated.


- Andrei


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


On Jan. 17, 2018, 11:36 a.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65180/
> ---
> 
> (Updated Jan. 17, 2018, 11:36 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The comment is incorrect since io switchboard allows reconnects
> after agent restarts.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/io/switchboard.cpp 
> 4ef640616d69b91c225206444737a40f9571da55 
> 
> 
> Diff: https://reviews.apache.org/r/65180/diff/2/
> 
> 
> Testing
> ---
> 
> None: not a functional change.
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 65180: Removed a comment in io switchboard.

[Andrei Budnik]

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

(Updated Jan. 17, 2018, 11:36 a.m.)


Review request for mesos, Alexander Rukletsov and Kevin Klues.


Repository: mesos


Description (updated)
---

The comment is incorrect since io switchboard allows reconnects
after agent restarts.


Diffs (updated)
-

  src/slave/containerizer/mesos/io/switchboard.cpp 
4ef640616d69b91c225206444737a40f9571da55 


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

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


Testing
---

None: not a functional change.


Thanks,

Andrei Budnik

Re: Review Request 64823: Documented the change in the Protobuf requirement.

[Benjamin Bannier]

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


Fix it, then Ship it!




Let's make sure to land this in `1.5.0-rc1` in addition to `master`.


CHANGELOG
Lines 14 (patched)


I'd move this section to the bottom. Maybe we could use a more generic 
title, e.g., `Changes to dependencies`.


- Benjamin Bannier


On Dec. 23, 2017, 1:21 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64823/
> ---
> 
> (Updated Dec. 23, 2017, 1:21 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, and Qian Zhang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documented the change in the Protobuf requirement.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 179b53b0568e4d993553f5cdb13668d96aa400d0 
>   docs/upgrades.md f2eb5a94d67b44a02d355981a673bd4ef5129653 
> 
> 
> Diff: https://reviews.apache.org/r/64823/diff/1/
> 
> 
> Testing
> ---
> 
> N/A
> 
> The requirement is checked in https://reviews.apache.org/r/64822/.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 65125: Added a helper function for resource provider tests.

[Benjamin Bannier]

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


Fix it, then Ship it!





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


Let's return an `Owned` here.


- Benjamin Bannier


On Jan. 15, 2018, 3:50 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65125/
> ---
> 
> (Updated Jan. 15, 2018, 3:50 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-8445
> https://issues.apache.org/jira/browse/MESOS-8445
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a helper function for resource provider tests.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp fb45879310faf7edc7d101be206d49de89819c18 
>   src/tests/mesos.hpp 16c75bb0ca570dfb7089743344cf38acdb517705 
>   src/tests/resource_provider_manager_tests.cpp 
> d80823c47963e969113dae3623b18b7639c890fc 
>   src/tests/slave_tests.cpp 59e3065ad9aadbd90cdfd32e830b433b88d6de86 
> 
> 
> Diff: https://reviews.apache.org/r/65125/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 65126: Added a resource provider test case.

[Benjamin Bannier]

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




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


This variable does not seem too useful.


- Benjamin Bannier


On Jan. 16, 2018, 12:04 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65126/
> ---
> 
> (Updated Jan. 16, 2018, 12:04 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-8445
> https://issues.apache.org/jira/browse/MESOS-8445
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a resource provider test case.
> 
> 
> Diffs
> -
> 
>   src/tests/master_tests.cpp 0edf22431aad85945aeb808a05f11e0bd832bccf 
> 
> 
> Diff: https://reviews.apache.org/r/65126/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>