Re: Review Request 66644: Remove unknown unreachable tasks when agent reregisters.

[Jiang Yan Xu]

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


Fix it, then Ship it!




I'll commit with these minor fixes.


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


I'll quote the `unreachable` as it is code.



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


Space in between `foreachkey (`



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


Space in between `foreach (`



src/tests/partition_tests.cpp
Lines 168-169 (patched)


This clearly can fit in the previous line which would be less jagged.



src/tests/partition_tests.cpp
Lines 396 (patched)


Period to end the sentence.


- Jiang Yan Xu


On May 1, 2018, 10:18 p.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66644/
> ---
> 
> (Updated May 1, 2018, 10:18 p.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: 8750
> https://issues.apache.org/jira/browse/8750
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A RunTask messsage could get dropped for an agent while its
> disconnected from the master and when such an agent goes unreachable
> then this dropped task message gets added to the unreachable tasks.
> When the agent reregisters, the master sends status updates for the
> tasks that the agent reported when re-registering and these tasks are
> also removed from the unreachableTasks on the framework but since the
> agent doesn't know about the dropped task so it doesn't get removed
> from the unreachableTasks leading to a check failure when
> this inconsistency is detected during framework removal.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 0d9620dd0c232dc1df83477e838eeb7313bf8828 
>   src/master/master.cpp 767ad8cfe142b47ef07172bcb2a4fb49fc3e833a 
>   src/tests/partition_tests.cpp 9138e5c745cf354a3573e1ab0b251d46702833cc 
> 
> 
> Diff: https://reviews.apache.org/r/66644/diff/5/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Megha Sharma
> 
>

Re: Review Request 66924: Made the master include the operation ID in OPERATION_DROPPED updates.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 66924 was successfully built and tested.

Reviews applied: `['66908', '66924']`

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

- Mesos Reviewbot Windows


On May 4, 2018, midnight, Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66924/
> ---
> 
> (Updated May 4, 2018, midnight)
> 
> 
> Review request for mesos, Benjamin Bannier and Greg Mann.
> 
> 
> Bugs: MESOS-8784
> https://issues.apache.org/jira/browse/MESOS-8784
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made the master include the operation ID in OPERATION_DROPPED updates.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp c30cf082d9ddf2e5dd97e0d887d6d07d8e03cbd4 
>   src/master/master.cpp c723a291ed8d219ced4492bc905ac6b52683ae47 
>   src/tests/storage_local_resource_provider_tests.cpp 
> ccb114aacc904998dfeef4128f6d38dfb3c865c7 
> 
> 
> Diff: https://reviews.apache.org/r/66924/diff/1/
> 
> 
> Testing
> ---
> 
> `make check` on GNU/Linux.
> 
> 
> The test in this patch fails without the corresponding master change, but 
> succeeds with it applied.
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 66749: Added more logging to agent recovery path.

[Meng Zhu]

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

(Updated May 3, 2018, 8:52 p.m.)


Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.


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


Repository: mesos


Description
---

Added logging in some agent recovery continuations to
make analyzing agent recovery related issue less painful.


Diffs (updated)
-

  src/slave/containerizer/composing.cpp 
186102c66d373dcd799cadd9fed7d1c8cb894971 
  src/slave/containerizer/docker.cpp a4c9c10e91e75f406329cbb2086f39b549cbeed0 
  src/slave/containerizer/mesos/containerizer.cpp 
01386ac3d36ec7a401b8d1be7834bc1f3fce55ef 
  src/slave/containerizer/mesos/linux_launcher.cpp 
af34a856e092a880a0809da34ead9d8588b0ac8f 
  src/slave/slave.cpp 6ca3d79fd38c800f258c571bb58164427db2ac7c 


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

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


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 66938: Made the 'SchedulerDriver' abort when operation's 'id' field is set.

[Gaston Kleiman]

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


Ship it!




Ship It!

- Gaston Kleiman


On May 3, 2018, 4:14 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66938/
> ---
> 
> (Updated May 3, 2018, 4:14 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Gaston Kleiman, Jan Schlicht, and 
> Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since the 'SchedulerDriver' does not support operation status updates,
> this patch adds a check to the driver which will abort the scheduler
> if the 'id' field is set in an offer operation.
> 
> 
> Diffs
> -
> 
>   src/sched/sched.cpp 620a3b26d8bf3487b6ce922b2280be7da291df06 
> 
> 
> Diff: https://reviews.apache.org/r/66938/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 66938: Made the 'SchedulerDriver' abort when operation's 'id' field is set.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [66938]

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

- Mesos Reviewbot


On May 3, 2018, 11:14 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66938/
> ---
> 
> (Updated May 3, 2018, 11:14 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Gaston Kleiman, Jan Schlicht, and 
> Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since the 'SchedulerDriver' does not support operation status updates,
> this patch adds a check to the driver which will abort the scheduler
> if the 'id' field is set in an offer operation.
> 
> 
> Diffs
> -
> 
>   src/sched/sched.cpp 620a3b26d8bf3487b6ce922b2280be7da291df06 
> 
> 
> Diff: https://reviews.apache.org/r/66938/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 66940: Documented the changes in the agent flags and modules.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 66940 was successfully built and tested.

Reviews applied: `['66940']`

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

- Mesos Reviewbot Windows


On May 3, 2018, 10:38 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66940/
> ---
> 
> (Updated May 3, 2018, 10:38 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Greg Mann, and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documented the changes in the agent flags and modules.
> 
> 
> Diffs
> -
> 
>   docs/upgrades.md 2de8900d4a7d192fad959da8a1b6929067a6bfc9 
> 
> 
> Diff: https://reviews.apache.org/r/66940/diff/1/
> 
> 
> Testing
> ---
> 
> Manually tested if the hyperlinks work ;) No test is needed.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66749: Added more logging to agent recovery path.

[Chun-Hung Hsiao]

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


Fix it, then Ship it!





src/slave/containerizer/docker.cpp
Lines 913 (patched)


Got the list of Docker containers


- Chun-Hung Hsiao


On April 23, 2018, 10:30 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66749/
> ---
> 
> (Updated April 23, 2018, 10:30 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.
> 
> 
> Bugs: MESOS-8793
> https://issues.apache.org/jira/browse/MESOS-8793
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added logging in some agent recovery continuations to
> make analyzing agent recovery related issue less painful.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/composing.cpp 
> 186102c66d373dcd799cadd9fed7d1c8cb894971 
>   src/slave/containerizer/docker.cpp a4c9c10e91e75f406329cbb2086f39b549cbeed0 
>   src/slave/containerizer/mesos/containerizer.cpp 
> def09f1104213bf73d9f95cb5ad2fb80e3bdb04a 
>   src/slave/containerizer/mesos/linux_launcher.cpp 
> af34a856e092a880a0809da34ead9d8588b0ac8f 
>   src/slave/slave.cpp 9d2d1928b231044988f1855eb518448db38ff04f 
> 
> 
> Diff: https://reviews.apache.org/r/66749/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 66931: Fixed a race in resource provider resubscription test.

[Chun-Hung Hsiao]

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




src/tests/resource_provider_manager_tests.cpp
Lines 1132 (patched)


This action will be invoked in the Driver's context. Could you explain why 
this could happen?



src/tests/resource_provider_manager_tests.cpp
Line 1140 (original), 1146 (patched)


Instead of creating a new mock resource provider, could we restart the 
agent with the same pid, and let the same mock resource provider to subscribe 
to the new agent?

Feel free to drop this if you have concerns with this approach.


- Chun-Hung Hsiao


On May 3, 2018, 11:46 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66931/
> ---
> 
> (Updated May 3, 2018, 11:46 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Jan Schlicht.
> 
> 
> Bugs: MESOS-8874
> https://issues.apache.org/jira/browse/MESOS-8874
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We previously did not make ensure that after the simulated agent
> failover in
> `ResourceProviderManagerHttpApiTest.ResubscribeResourceProvider` the
> mock resource provider created as part of the test did not reconnect
> to the restarted agent (as opposed to the newly initialized resource
> provider). This lead to unmet test expectations.
> 
> With this patch we now explicitly tear down the mock resource provider
> after we have detected that the agent went away to prevent the race.
> 
> 
> Diffs
> -
> 
>   src/tests/resource_provider_manager_tests.cpp 
> e8ca377fd0a927b99fdaf6a8ee0139025a41298e 
> 
> 
> Diff: https://reviews.apache.org/r/66931/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> Ran the test repeatedly under high system load without triggering the issue 
> again with this patch.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 66931: Fixed a race in resource provider resubscription test.

[Chun-Hung Hsiao]

> On May 3, 2018, 1:52 p.m., Jan Schlicht wrote:
> > src/tests/resource_provider_manager_tests.cpp
> > Lines 1130-1131 (patched)
> > 
> >
> > Nit: `disconnected` is called asynchronously. The `Clock::settle()` 
> > below will make sure that it is called. Not having the settle would 
> > introduce another flakyness though. To be extra sure, we could set a future 
> > and await that one before we reset `resourceProvider` with a new instance.

+1 for awaiting a future instead of using `Clock::settle()`. This would make 
the intention more clear.


- Chun-Hung


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


On May 3, 2018, 11:46 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66931/
> ---
> 
> (Updated May 3, 2018, 11:46 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Jan Schlicht.
> 
> 
> Bugs: MESOS-8874
> https://issues.apache.org/jira/browse/MESOS-8874
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We previously did not make ensure that after the simulated agent
> failover in
> `ResourceProviderManagerHttpApiTest.ResubscribeResourceProvider` the
> mock resource provider created as part of the test did not reconnect
> to the restarted agent (as opposed to the newly initialized resource
> provider). This lead to unmet test expectations.
> 
> With this patch we now explicitly tear down the mock resource provider
> after we have detected that the agent went away to prevent the race.
> 
> 
> Diffs
> -
> 
>   src/tests/resource_provider_manager_tests.cpp 
> e8ca377fd0a927b99fdaf6a8ee0139025a41298e 
> 
> 
> Diff: https://reviews.apache.org/r/66931/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> Ran the test repeatedly under high system load without triggering the issue 
> again with this patch.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 66900: Avoid copying of re-register framework messages in the master.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 66900 was successfully built and tested.

Reviews applied: `['66860', '66900']`

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

- Mesos Reviewbot Windows


On May 4, 2018, 12:58 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66900/
> ---
> 
> (Updated May 4, 2018, 12:58 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-8628
> https://issues.apache.org/jira/browse/MESOS-8628
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Prior to this patch, the re-register framework message was copied
> into a subscribe call. This updates the handler to perform moves
> instead.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp c30cf082d9ddf2e5dd97e0d887d6d07d8e03cbd4 
>   src/master/master.cpp c723a291ed8d219ced4492bc905ac6b52683ae47 
> 
> 
> Diff: https://reviews.apache.org/r/66900/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 66920: Improved tests for resizing persistent volumes.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 66920 was successfully built and tested.

Reviews applied: `['66218', '66049', '66733', '66050', '66219', '66858', 
'66220', '66531', '66532', '66052', '66051', '66227', '66920']`

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

- Mesos Reviewbot Windows


On May 4, 2018, 8:02 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66920/
> ---
> 
> (Updated May 4, 2018, 8:02 a.m.)
> 
> 
> Review request for mesos, Greg Mann and Zhitao Li.
> 
> 
> Bugs: MESOS-4965
> https://issues.apache.org/jira/browse/MESOS-4965
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Now the `GrowVolume` and `ShrinkVolume` tests launch tasks after
> resizing the volumes to ensure that the operations take effect on
> agents. The `NonSpeculativeGrowAndLaunch` and
> `NonSpeculativeShrinkAndLaunch` tests launch an additional task to
> verify that the original volume consumed by the operations cannot be
> used by subsequent tasks.
> 
> This patch also adjusted when the clock is resumed so schedulers will
> not receive unexpected offers.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_tests.cpp 
> 4edf781711d9efdb994114aeb6289b6af750b87a 
> 
> 
> Diff: https://reviews.apache.org/r/66920/diff/4/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66940: Documented the changes in the agent flags and modules.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [66893, 66894, 66896, 66940]

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

- Mesos Reviewbot


On May 3, 2018, 10:38 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66940/
> ---
> 
> (Updated May 3, 2018, 10:38 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Greg Mann, and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documented the changes in the agent flags and modules.
> 
> 
> Diffs
> -
> 
>   docs/upgrades.md 2de8900d4a7d192fad959da8a1b6929067a6bfc9 
> 
> 
> Diff: https://reviews.apache.org/r/66940/diff/1/
> 
> 
> Testing
> ---
> 
> Manually tested if the hyperlinks work ;) No test is needed.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66939: Improved validation messages for some operations.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 66939 was successfully built and tested.

Reviews applied: `['66939']`

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

- Mesos Reviewbot Windows


On May 3, 2018, 11:54 p.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66939/
> ---
> 
> (Updated May 3, 2018, 11:54 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improved validation messages for some operations.
> 
> 
> Diffs
> -
> 
>   src/master/validation.cpp ac2e1bb8771841ec59b3bdcdeffb6c6230680d4d 
> 
> 
> Diff: https://reviews.apache.org/r/66939/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 66860: Avoid copying of register framework messages in the master.

[Meng Zhu]

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

(Updated May 3, 2018, 5:58 p.m.)


Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
---

Prior to this patch, the register framework message was copied
into a subscribe call. This updates the handler to perform moves
instead.


Diffs (updated)
-

  src/master/master.hpp c30cf082d9ddf2e5dd97e0d887d6d07d8e03cbd4 
  src/master/master.cpp c723a291ed8d219ced4492bc905ac6b52683ae47 


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

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


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 66920: Improved tests for resizing persistent volumes.

[Chun-Hung Hsiao]

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

(Updated May 4, 2018, 12:02 a.m.)


Review request for mesos, Greg Mann and Zhitao Li.


Changes
---

Addressed Greg's comments.


Repository: mesos


Description
---

Now the `GrowVolume` and `ShrinkVolume` tests launch tasks after
resizing the volumes to ensure that the operations take effect on
agents. The `NonSpeculativeGrowAndLaunch` and
`NonSpeculativeShrinkAndLaunch` tests launch an additional task to
verify that the original volume consumed by the operations cannot be
used by subsequent tasks.

This patch also adjusted when the clock is resumed so schedulers will
not receive unexpected offers.


Diffs (updated)
-

  src/tests/persistent_volume_tests.cpp 
4edf781711d9efdb994114aeb6289b6af750b87a 


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

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


Testing
---

sudo make check


Thanks,

Chun-Hung Hsiao

Review Request 66924: Made the master include the operation ID in OPERATION_DROPPED updates.

[Gaston Kleiman]

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

Review request for mesos, Benjamin Bannier and Greg Mann.


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


Repository: mesos


Description
---

Made the master include the operation ID in OPERATION_DROPPED updates.


Diffs
-

  src/master/master.hpp c30cf082d9ddf2e5dd97e0d887d6d07d8e03cbd4 
  src/master/master.cpp c723a291ed8d219ced4492bc905ac6b52683ae47 
  src/tests/storage_local_resource_provider_tests.cpp 
ccb114aacc904998dfeef4128f6d38dfb3c865c7 


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


Testing
---

`make check` on GNU/Linux.


The test in this patch fails without the corresponding master change, but 
succeeds with it applied.


Thanks,

Gaston Kleiman

Re: Review Request 66920: Improved tests for resizing persistent volumes.

[Greg Mann]

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


Fix it, then Ship it!





src/tests/persistent_volume_tests.cpp
Lines 845 (patched)


s/keep/make/



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


s/keep/make/


- Greg Mann


On May 3, 2018, 8:55 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66920/
> ---
> 
> (Updated May 3, 2018, 8:55 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Zhitao Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Now the `GrowVolume` and `ShrinkVolume` tests launch tasks after
> resizing the volumes to ensure that the operations take effect on
> agents. The `NonSpeculativeGrowAndLaunch` and
> `NonSpeculativeShrinkAndLaunch` tests launch an additional task to
> verify that the original volume consumed by the operations cannot be
> used by subsequent tasks.
> 
> This patch also adjusted when the clock is resumed so schedulers will
> not receive unexpected offers.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_tests.cpp 
> 4edf781711d9efdb994114aeb6289b6af750b87a 
> 
> 
> Diff: https://reviews.apache.org/r/66920/diff/3/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66939: Improved validation messages for some operations.

[Gaston Kleiman]

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

(Updated May 3, 2018, 4:54 p.m.)


Review request for mesos, Greg Mann and Jan Schlicht.


Changes
---

Addressed Greg's feedback. I guess I need new glasses ¯\_(?)_/¯.


Repository: mesos


Description
---

Improved validation messages for some operations.


Diffs (updated)
-

  src/master/validation.cpp ac2e1bb8771841ec59b3bdcdeffb6c6230680d4d 


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

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


Testing
---


Thanks,

Gaston Kleiman

Re: Review Request 66938: Made the 'SchedulerDriver' abort when operation's 'id' field is set.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 66938 was successfully built and tested.

Reviews applied: `['66938']`

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

- Mesos Reviewbot Windows


On May 3, 2018, 11:14 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66938/
> ---
> 
> (Updated May 3, 2018, 11:14 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Gaston Kleiman, Jan Schlicht, and 
> Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since the 'SchedulerDriver' does not support operation status updates,
> this patch adds a check to the driver which will abort the scheduler
> if the 'id' field is set in an offer operation.
> 
> 
> Diffs
> -
> 
>   src/sched/sched.cpp 620a3b26d8bf3487b6ce922b2280be7da291df06 
> 
> 
> Diff: https://reviews.apache.org/r/66938/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 66920: Improved tests for resizing persistent volumes.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [66218, 66049, 66733, 66050, 66219, 66858, 66220, 66531, 
66532, 66052, 66051, 66227, 66920]

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

- Mesos Reviewbot


On May 3, 2018, 8:55 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66920/
> ---
> 
> (Updated May 3, 2018, 8:55 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Zhitao Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Now the `GrowVolume` and `ShrinkVolume` tests launch tasks after
> resizing the volumes to ensure that the operations take effect on
> agents. The `NonSpeculativeGrowAndLaunch` and
> `NonSpeculativeShrinkAndLaunch` tests launch an additional task to
> verify that the original volume consumed by the operations cannot be
> used by subsequent tasks.
> 
> This patch also adjusted when the clock is resumed so schedulers will
> not receive unexpected offers.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_tests.cpp 
> 4edf781711d9efdb994114aeb6289b6af750b87a 
> 
> 
> Diff: https://reviews.apache.org/r/66920/diff/3/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66938: Made the 'SchedulerDriver' abort when operation's 'id' field is set.

[Greg Mann]

> On May 3, 2018, 10:25 p.m., Gaston Kleiman wrote:
> > src/sched/sched.cpp
> > Lines 1349-1352 (patched)
> > 
> >
> > We should call `error()` here so that the framework is gracefully 
> > teared down instead of triggering a segfault.

Good call, thanks! This also allows us to keep the existing test.


- Greg


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


On May 3, 2018, 11:14 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66938/
> ---
> 
> (Updated May 3, 2018, 11:14 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Gaston Kleiman, Jan Schlicht, and 
> Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since the 'SchedulerDriver' does not support operation status updates,
> this patch adds a check to the driver which will abort the scheduler
> if the 'id' field is set in an offer operation.
> 
> 
> Diffs
> -
> 
>   src/sched/sched.cpp 620a3b26d8bf3487b6ce922b2280be7da291df06 
> 
> 
> Diff: https://reviews.apache.org/r/66938/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 66938: Made the 'SchedulerDriver' abort when operation's 'id' field is set.

[Greg Mann]

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

(Updated May 3, 2018, 11:14 p.m.)


Review request for mesos, Benjamin Bannier, Gaston Kleiman, Jan Schlicht, and 
Vinod Kone.


Repository: mesos


Description (updated)
---

Since the 'SchedulerDriver' does not support operation status updates,
this patch adds a check to the driver which will abort the scheduler
if the 'id' field is set in an offer operation.


Diffs (updated)
-

  src/sched/sched.cpp 620a3b26d8bf3487b6ce922b2280be7da291df06 


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

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


Testing
---

make check


Thanks,

Greg Mann

Re: Review Request 66939: Improved validation messages for some operations.

[Greg Mann]

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


Fix it, then Ship it!





src/master/validation.cpp
Line 2368 (original), 2368 (patched)


Nit: let's be consistent with backticks or quotes. Looks like quotes may be 
more consistent with existing messages?


- Greg Mann


On May 3, 2018, 10:36 p.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66939/
> ---
> 
> (Updated May 3, 2018, 10:36 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improved validation messages for some operations.
> 
> 
> Diffs
> -
> 
>   src/master/validation.cpp ac2e1bb8771841ec59b3bdcdeffb6c6230680d4d 
> 
> 
> Diff: https://reviews.apache.org/r/66939/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Review Request 66940: Documented the changes in the agent flags and modules.

[Chun-Hung Hsiao]

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

Review request for mesos, Gilbert Song, Greg Mann, and Jie Yu.


Repository: mesos


Description
---

Documented the changes in the agent flags and modules.


Diffs
-

  docs/upgrades.md 2de8900d4a7d192fad959da8a1b6929067a6bfc9 


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


Testing
---

Manually tested if the hyperlinks work ;) No test is needed.


Thanks,

Chun-Hung Hsiao

Review Request 66939: Improved validation messages for some operations.

[Gaston Kleiman]

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

Review request for mesos, Greg Mann and Jan Schlicht.


Repository: mesos


Description
---

Improved validation messages for some operations.


Diffs
-

  src/master/validation.cpp ac2e1bb8771841ec59b3bdcdeffb6c6230680d4d 


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


Testing
---


Thanks,

Gaston Kleiman

Re: Review Request 66938: Made the 'SchedulerDriver' abort when operation's 'id' field is set.

[Gaston Kleiman]

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




src/sched/sched.cpp
Lines 1349-1352 (patched)


We should call `error()` here so that the framework is gracefully teared 
down instead of triggering a segfault.


- Gaston Kleiman


On May 3, 2018, 3:07 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66938/
> ---
> 
> (Updated May 3, 2018, 3:07 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Gaston Kleiman, Jan Schlicht, and 
> Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since the 'SchedulerDriver' does not support operation status updates,
> this patch adds a CHECK to the driver which will abort the scheduler
> if the 'id' field is set in an offer operation.
> 
> This patch also removes the test
> 'SchedulerTest.OperationFeedbackValidationSchedulerDriverFramework'
> because with these new changes, it causes the test harness to abort.
> 
> 
> Diffs
> -
> 
>   src/sched/sched.cpp 620a3b26d8bf3487b6ce922b2280be7da291df06 
>   src/tests/scheduler_tests.cpp 749420a23bc1a3873bd4d5aee56e78cff79bb1af 
> 
> 
> Diff: https://reviews.apache.org/r/66938/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 66920: Improved tests for resizing persistent volumes.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 66920 was successfully built and tested.

Reviews applied: `['66218', '66049', '66733', '66050', '66219', '66858', 
'66220', '66531', '66532', '66052', '66051', '66227', '66920']`

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

- Mesos Reviewbot Windows


On May 3, 2018, 1:55 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66920/
> ---
> 
> (Updated May 3, 2018, 1:55 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Zhitao Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Now the `GrowVolume` and `ShrinkVolume` tests launch tasks after
> resizing the volumes to ensure that the operations take effect on
> agents. The `NonSpeculativeGrowAndLaunch` and
> `NonSpeculativeShrinkAndLaunch` tests launch an additional task to
> verify that the original volume consumed by the operations cannot be
> used by subsequent tasks.
> 
> This patch also adjusted when the clock is resumed so schedulers will
> not receive unexpected offers.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_tests.cpp 
> 4edf781711d9efdb994114aeb6289b6af750b87a 
> 
> 
> Diff: https://reviews.apache.org/r/66920/diff/3/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Review Request 66938: Made the 'SchedulerDriver' abort when operation's 'id' field is set.

[Greg Mann]

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

Review request for mesos, Benjamin Bannier, Gaston Kleiman, Jan Schlicht, and 
Vinod Kone.


Repository: mesos


Description
---

Since the 'SchedulerDriver' does not support operation status updates,
this patch adds a CHECK to the driver which will abort the scheduler
if the 'id' field is set in an offer operation.

This patch also removes the test
'SchedulerTest.OperationFeedbackValidationSchedulerDriverFramework'
because with these new changes, it causes the test harness to abort.


Diffs
-

  src/sched/sched.cpp 620a3b26d8bf3487b6ce922b2280be7da291df06 
  src/tests/scheduler_tests.cpp 749420a23bc1a3873bd4d5aee56e78cff79bb1af 


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


Testing
---

make check


Thanks,

Greg Mann

Re: Review Request 66532: Added test for authorization actions for `RESIZE_VOLUME`.

[Zhitao Li]

> On May 2, 2018, 11:51 a.m., Chun-Hung Hsiao wrote:
> > src/tests/persistent_volume_tests.cpp
> > Lines 1289 (patched)
> > 
> >
> > I don't think we need this but it doesn't hurt. I'll leave it up to you 
> > to decide if you want to keep this. Please feel free to drop it if you 
> > prefer keeping it.
> > 
> > This is more of a personal preference ;) I prefer always introducing 
> > synchronizations for a reason but some others prefer making tests as 
> > deterministic as possible.

I think I prefer keeping it for now.


- Zhitao


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


On May 1, 2018, 3:55 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66532/
> ---
> 
> (Updated May 1, 2018, 3:55 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Greg Mann.
> 
> 
> Bugs: MESOS-8748
> https://issues.apache.org/jira/browse/MESOS-8748
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test for authorization actions for `RESIZE_VOLUME`.
> 
> 
> Diffs
> -
> 
>   src/tests/authorization_tests.cpp a76ad18a54ec232e6d6c92ec6a45b445a83f174c 
>   src/tests/persistent_volume_tests.cpp 
> 4edf781711d9efdb994114aeb6289b6af750b87a 
> 
> 
> Diff: https://reviews.apache.org/r/66532/diff/8/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 63367: Added overloads for strings::startsWith and strings::endsWith().

[Benjamin Mahler]

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


Ship it!





3rdparty/stout/include/stout/strings.hpp
Lines 390 (patched)


Can you mention the max length optimization you're doing?



3rdparty/stout/include/stout/strings.hpp
Lines 411-413 (patched)


Any reason not to do the same max len optimization here?


- Benjamin Mahler


On May 2, 2018, 6:30 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63367/
> ---
> 
> (Updated May 2, 2018, 6:30 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This saves an unnecessary memory allocation when
> testing if a string starts or ends with a string literal,
> which accounts for almost all usages of these functions
> in Mesos and in libprocess.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/strings.hpp 
> 067a7923c02342bccd9be1136a981fd6b0e0e9b4 
>   3rdparty/stout/tests/strings_tests.cpp 
> 395540aad88c76a66a43a54edfe9ca1a2d46d3b4 
> 
> 
> Diff: https://reviews.apache.org/r/63367/diff/6/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 65572: Fixed two slave recovery tests.

[Chun-Hung Hsiao]

> On May 3, 2018, 8:56 p.m., Benjamin Mahler wrote:
> > Should this be marked as committed?

Let me do it since I just saw this lol


- Chun-Hung


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


On Feb. 9, 2018, 1:12 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65572/
> ---
> 
> (Updated Feb. 9, 2018, 1:12 a.m.)
> 
> 
> Review request for mesos, Greg Mann and Vinod Kone.
> 
> 
> Bugs: MESOS-8552
> https://issues.apache.org/jira/browse/MESOS-8552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These two tests are affected by the lingering executor fix
> (#65109). We should wait to make sure tasks are actually launched
> on the executor before trigger the agent failover. Otherwise, the
> container will be shutdown upon re-registration because the executor
> has never received any task.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_recovery_tests.cpp 77aa60c953bd0769eaba05f001755e4cec9ba028 
> 
> 
> Diff: https://reviews.apache.org/r/65572/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 66896: Updated `csi.md`.

[Greg Mann]

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


Ship it!




Ship It!

- Greg Mann


On May 1, 2018, 9:45 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66896/
> ---
> 
> (Updated May 1, 2018, 9:45 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Greg Mann, and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated `csi.md`.
> 
> 
> Diffs
> -
> 
>   docs/csi.md 77a5edbfdc69b451f37f2d5ca7f21bafe19d7872 
> 
> 
> Diff: https://reviews.apache.org/r/66896/diff/1/
> 
> 
> Testing
> ---
> 
> This is a documentation change so no tests are needed.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66894: Fixed typos in `upgrades.md`.

[Greg Mann]

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


Ship it!




Thanks for the cleanup!! :)

- Greg Mann


On May 1, 2018, 9:33 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66894/
> ---
> 
> (Updated May 1, 2018, 9:33 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Greg Mann, and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed typos in `upgrades.md`.
> 
> 
> Diffs
> -
> 
>   docs/upgrades.md 2de8900d4a7d192fad959da8a1b6929067a6bfc9 
> 
> 
> Diff: https://reviews.apache.org/r/66894/diff/1/
> 
> 
> Testing
> ---
> 
> This is a documentation change so no tests are needed.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 65572: Fixed two slave recovery tests.

[Benjamin Mahler]

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



Should this be marked as committed?

- Benjamin Mahler


On Feb. 9, 2018, 1:12 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65572/
> ---
> 
> (Updated Feb. 9, 2018, 1:12 a.m.)
> 
> 
> Review request for mesos, Greg Mann and Vinod Kone.
> 
> 
> Bugs: MESOS-8552
> https://issues.apache.org/jira/browse/MESOS-8552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These two tests are affected by the lingering executor fix
> (#65109). We should wait to make sure tasks are actually launched
> on the executor before trigger the agent failover. Otherwise, the
> container will be shutdown upon re-registration because the executor
> has never received any task.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_recovery_tests.cpp 77aa60c953bd0769eaba05f001755e4cec9ba028 
> 
> 
> Diff: https://reviews.apache.org/r/65572/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 66920: Improved tests for resizing persistent volumes.

[Chun-Hung Hsiao]

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

(Updated May 3, 2018, 8:55 p.m.)


Review request for mesos, Greg Mann and Zhitao Li.


Changes
---

Addressed Zhitao's comments.


Repository: mesos


Description
---

Now the `GrowVolume` and `ShrinkVolume` tests launch tasks after
resizing the volumes to ensure that the operations take effect on
agents. The `NonSpeculativeGrowAndLaunch` and
`NonSpeculativeShrinkAndLaunch` tests launch an additional task to
verify that the original volume consumed by the operations cannot be
used by subsequent tasks.

This patch also adjusted when the clock is resumed so schedulers will
not receive unexpected offers.


Diffs (updated)
-

  src/tests/persistent_volume_tests.cpp 
4edf781711d9efdb994114aeb6289b6af750b87a 


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

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


Testing
---

sudo make check


Thanks,

Chun-Hung Hsiao

Re: Review Request 66893: Documented the changes in gRPC and CSI supports.

[Greg Mann]

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


Ship it!




Ship It!

- Greg Mann


On May 1, 2018, 9:47 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66893/
> ---
> 
> (Updated May 1, 2018, 9:47 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Greg Mann, and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documented the changes in gRPC and CSI supports.
> 
> 
> Diffs
> -
> 
>   docs/upgrades.md 2de8900d4a7d192fad959da8a1b6929067a6bfc9 
> 
> 
> Diff: https://reviews.apache.org/r/66893/diff/2/
> 
> 
> Testing
> ---
> 
> This is a documentation change so no tests needed.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66923: Added documentation on volume resize support.

[Greg Mann]

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




docs/operator-http-api.md
Lines 1987-1990 (patched)


Could you provide links to the persistent volume docs here and below for 
SHRINK_VOLUME? Since those docs provide lots of specific information about the 
API I think it would be helpful.



docs/persistent-volume.md
Lines 59-60 (original), 62-63 (patched)


Looks like this should also be updated?



docs/persistent-volume.md
Lines 271 (patched)


s/with same/with the same/



docs/persistent-volume.md
Lines 309 (patched)


s/sending a/sending an/



docs/persistent-volume.md
Lines 310 (patched)


If you're going to mention `acceptOffers` explicitly here, then we should 
also mention the ACCEPT call.

Something like:
"We can grow the persistent volume by including an `Offer::Operation` 
message when accepting an offer (this can be done via the `SchedulerDriver`'s 
`acceptOffers` method, or via the v1 scheduler API's `ACCEPT` call)."



docs/persistent-volume.md
Lines 311 (patched)


s/specified/specifies/



docs/persistent-volume.md
Lines 312 (patched)


s/specified/specifies/



docs/persistent-volume.md
Lines 317 (patched)


Nit: missing a space before the `{` on this line. Here and elsewhere.



docs/persistent-volume.md
Lines 347 (patched)


s/new size/the new size/



docs/persistent-volume.md
Lines 384 (patched)


s/IN/In/



docs/persistent-volume.md
Lines 415 (patched)


s/sending a/sending an/



docs/persistent-volume.md
Lines 416 (patched)


Ditto regarding mentioning `acceptOffers` here.



docs/persistent-volume.md
Lines 417-418 (patched)


s/specified/specifies/



docs/persistent-volume.md
Lines 447 (patched)


s/new size/the new size/



docs/persistent-volume.md
Lines 489-497 (patched)


Could you also include these restrictions in the docs for the new operator 
API calls that you're adding?



docs/persistent-volume.md
Lines 496-497 (patched)


Suggestion:
"Volume resize operations cannot be included in an ACCEPT call with other 
operations which make use of the resized volume."



docs/persistent-volume.md
Lines 500 (patched)


To better distinguish this heading and the next, I would recommend:
"Versioned HTTP Operator API"



docs/persistent-volume.md
Lines 508 (patched)


s/volume/volumes/



docs/persistent-volume.md
Lines 511-512 (patched)


I would recommend removing this statement, since it is already mentioned in 
the next section.



docs/persistent-volume.md
Lines 515 (patched)


To better distinguish this heading from the preceding one, I would 
recommend:
"Unversioned Operator HTTP Endpoints"



docs/persistent-volume.md
Lines 520 (patched)


s/encourage to use/encourage operators to use the/



docs/persistent-volume.md
Lines 521 (patched)


s/and new/as new/


- Greg Mann


On May 3, 2018, 12:01 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66923/
> ---
> 
> (Updated May 3, 2018, 12:01 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Greg Mann.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation on volume resize support.
> 
> 
> Diffs
> -
> 
>   docs/authorization.md fdbef770c38ab38ab748b4f943b331d4442a2ce1 
>   docs/operator-http-api.md 9be1e2db60562b369bdc6ee2c732a74fad049580 
>   docs/persistent-volume.md 1a5799b8d8e5302e0b01da6b9a16a3b9fb75c898 
> 
> 
> Diff: https://reviews.apache.org/r/66923/diff/1/
> 
> 
> Testing
> ---
> 
> 

Re: Review Request 66849: Added missing test expectation.

[Gaston Kleiman]

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


Ship it!




Ship It!

- Gaston Kleiman


On April 27, 2018, 3:15 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66849/
> ---
> 
> (Updated April 27, 2018, 3:15 a.m.)
> 
> 
> Review request for mesos, Gaston Kleiman and Greg Mann.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> On master failover the scheduler will get disconnected. Add a test
> expectation for that. This silences a gmock warning.
> 
> 
> Diffs
> -
> 
>   src/tests/operation_reconciliation_tests.cpp 
> 76c169567de6e0ef99c984a673cd69ce7725d6b6 
> 
> 
> Diff: https://reviews.apache.org/r/66849/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> Before this patch the fixed tests emits a gmock warning about an unexpected 
> function call, with this patch the warning disappears.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 66849: Added missing test expectation.

[Greg Mann]

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


Ship it!




Ship It!

- Greg Mann


On April 27, 2018, 10:15 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66849/
> ---
> 
> (Updated April 27, 2018, 10:15 a.m.)
> 
> 
> Review request for mesos, Gaston Kleiman and Greg Mann.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> On master failover the scheduler will get disconnected. Add a test
> expectation for that. This silences a gmock warning.
> 
> 
> Diffs
> -
> 
>   src/tests/operation_reconciliation_tests.cpp 
> 76c169567de6e0ef99c984a673cd69ce7725d6b6 
> 
> 
> Diff: https://reviews.apache.org/r/66849/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> Before this patch the fixed tests emits a gmock warning about an unexpected 
> function call, with this patch the warning disappears.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 66908: Correctly reconciled dropped operation after agent failover.

[Greg Mann]

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


Fix it, then Ship it!




LGTM! Benjamin, do you think you could update this during your day tomorrow so 
that we can land it before the 1.6 RC cut?


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


I think the following is appropriate?

s/after a master failover/after a master or agent failover/


- Greg Mann


On May 2, 2018, 2:35 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66908/
> ---
> 
> (Updated May 2, 2018, 2:35 p.m.)
> 
> 
> Review request for mesos, Gaston Kleiman and Greg Mann.
> 
> 
> Bugs: MESOS-8870
> https://issues.apache.org/jira/browse/MESOS-8870
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When the master receives an `UpdateSlaveMessage` after agent failover
> it previously did not correctly detect dropped operations (operations
> known to the master, but unknown to the agent) and did not trigger
> reconciliation for such operations.
> 
> This patch fixes the handler in the master so that such dropped
> operations are reconciled.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp c723a291ed8d219ced4492bc905ac6b52683ae47 
>   src/tests/master_slave_reconciliation_tests.cpp 
> 6bb4263323bcfd191c8e3c1ccba10a240e9ddd83 
> 
> 
> Diff: https://reviews.apache.org/r/66908/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> The test in this patch fails without the corresponding master change, but 
> succeeds with it applied.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 66920: Improved tests for resizing persistent volumes.

[Chun-Hung Hsiao]

> On May 3, 2018, 3:51 p.m., Zhitao Li wrote:
> > src/tests/persistent_volume_tests.cpp
> > Lines 604 (patched)
> > 
> >
> > Technically we only tested that persistent volume and its data can 
> > still be accessed.
> > 
> > I do not know a reliable and efficient way to test the volume size 
> > actually changed after resizing.
> > 
> > Maybe clarify the comment?

Hmm I thought the slave will check the consumed task resources but apparently 
it only checks checkpointed resources:
https://github.com/apache/mesos/blob/master/src/slave/slave.cpp#L2743
For `GROW_VOLUME`, this check is sufficient to ensure that the volume is 
resized.
However, for `SHRINK_VOLUME`, I'll make a dynamic reservation on the disk 
resources first so when the volume is shrunk, the freed space is considered as 
a checkpointed resource:
https://github.com/apache/mesos/blob/master/src/common/resources_utils.cpp#L30


> On May 3, 2018, 3:51 p.m., Zhitao Li wrote:
> > src/tests/persistent_volume_tests.cpp
> > Line 751 (original), 807 (patched)
> > 
> >
> > ```
> > This test verifies that launching any task depending either origial or 
> > grown volume of a `GROW_VOLUME` call in the same `acceptOffers` will be 
> > dropped...
> > ```

Thanks. I'll say the following:
```
// This test verifies that any task to launch after a `GROW_VOLUME` in the same
// `ACCEPT` call is dropped if the task consumes the original or grown volume
```


- Chun-Hung


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


On May 3, 2018, 4:54 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66920/
> ---
> 
> (Updated May 3, 2018, 4:54 a.m.)
> 
> 
> Review request for mesos, Greg Mann and Zhitao Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Now the `GrowVolume` and `ShrinkVolume` tests launch tasks after
> resizing the volumes to ensure that the operations take effect on
> agents. The `NonSpeculativeGrowAndLaunch` and
> `NonSpeculativeShrinkAndLaunch` tests launch an additional task to
> verify that the original volume consumed by the operations cannot be
> used by subsequent tasks.
> 
> This patch also adjusted when the clock is resumed so schedulers will
> not receive unexpected offers.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_tests.cpp 
> 4edf781711d9efdb994114aeb6289b6af750b87a 
> 
> 
> Diff: https://reviews.apache.org/r/66920/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66934: Removed credential-based authentication of 'MockResourceProvider'.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [66932, 66933, 66934]

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

- Mesos Reviewbot


On May 3, 2018, 12:57 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66934/
> ---
> 
> (Updated May 3, 2018, 12:57 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Chun-Hung Hsiao.
> 
> 
> Bugs: MESOS-8774
> https://issues.apache.org/jira/browse/MESOS-8774
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Resource provider authentication is token-based and using a separate
> realm. As a result, credential parameters have been removed from
> 'MockResourceProvider'. Also, authentication for the resource provider
> realm is disabled by default and only enabled in tests that instantiate
> a Storage Local Resource Provider which support JWT.
> 
> 
> Diffs
> -
> 
>   src/tests/agent_resource_provider_config_api_tests.cpp 
> 6ced582e14966eea3bb109e2e2de0e554acc2968 
>   src/tests/api_tests.cpp dd8e221d8fd1b2a241505345337897e4ee4a6347 
>   src/tests/executor_http_api_tests.cpp 
> 51cca7a90b3c5c5380ede0f17d66b9fdfeb88c27 
>   src/tests/master_tests.cpp d5ce52c0eb1ba333d1b3dd35518545c13d828ce6 
>   src/tests/mesos.hpp 756a52189ea3336b203ea1d606e2ba17762d57aa 
>   src/tests/operation_reconciliation_tests.cpp 
> 76c169567de6e0ef99c984a673cd69ce7725d6b6 
>   src/tests/resource_provider_manager_tests.cpp 
> e8ca377fd0a927b99fdaf6a8ee0139025a41298e 
>   src/tests/slave_tests.cpp ae82438a4f2f79820fe6d476e25b4915aa5f212c 
>   src/tests/storage_local_resource_provider_tests.cpp 
> ccb114aacc904998dfeef4128f6d38dfb3c865c7 
> 
> 
> Diff: https://reviews.apache.org/r/66934/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 66920: Improved tests for resizing persistent volumes.

[Zhitao Li]

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




src/tests/persistent_volume_tests.cpp
Lines 604 (patched)


Technically we only tested that persistent volume and its data can still be 
accessed.

I do not know a reliable and efficient way to test the volume size actually 
changed after resizing.

Maybe clarify the comment?



src/tests/persistent_volume_tests.cpp
Line 751 (original), 807 (patched)


```
This test verifies that launching any task depending either origial or 
grown volume of a `GROW_VOLUME` call in the same `acceptOffers` will be 
dropped...
```



src/tests/persistent_volume_tests.cpp
Line 874 (original), 942 (patched)


```
This test verifies that launching any task depending either origial or 
shrunk volume of a `SHRINK_VOLUME` call in the same `acceptOffers` will be 
dropped...
```


- Zhitao Li


On May 2, 2018, 9:54 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66920/
> ---
> 
> (Updated May 2, 2018, 9:54 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Zhitao Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Now the `GrowVolume` and `ShrinkVolume` tests launch tasks after
> resizing the volumes to ensure that the operations take effect on
> agents. The `NonSpeculativeGrowAndLaunch` and
> `NonSpeculativeShrinkAndLaunch` tests launch an additional task to
> verify that the original volume consumed by the operations cannot be
> used by subsequent tasks.
> 
> This patch also adjusted when the clock is resumed so schedulers will
> not receive unexpected offers.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_tests.cpp 
> 4edf781711d9efdb994114aeb6289b6af750b87a 
> 
> 
> Diff: https://reviews.apache.org/r/66920/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66934: Removed credential-based authentication of 'MockResourceProvider'.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 66934 was successfully built and tested.

Reviews applied: `['66932', '66933', '66934']`

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

- Mesos Reviewbot Windows


On May 3, 2018, 12:57 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66934/
> ---
> 
> (Updated May 3, 2018, 12:57 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Chun-Hung Hsiao.
> 
> 
> Bugs: MESOS-8774
> https://issues.apache.org/jira/browse/MESOS-8774
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Resource provider authentication is token-based and using a separate
> realm. As a result, credential parameters have been removed from
> 'MockResourceProvider'. Also, authentication for the resource provider
> realm is disabled by default and only enabled in tests that instantiate
> a Storage Local Resource Provider which support JWT.
> 
> 
> Diffs
> -
> 
>   src/tests/agent_resource_provider_config_api_tests.cpp 
> 6ced582e14966eea3bb109e2e2de0e554acc2968 
>   src/tests/api_tests.cpp dd8e221d8fd1b2a241505345337897e4ee4a6347 
>   src/tests/executor_http_api_tests.cpp 
> 51cca7a90b3c5c5380ede0f17d66b9fdfeb88c27 
>   src/tests/master_tests.cpp d5ce52c0eb1ba333d1b3dd35518545c13d828ce6 
>   src/tests/mesos.hpp 756a52189ea3336b203ea1d606e2ba17762d57aa 
>   src/tests/operation_reconciliation_tests.cpp 
> 76c169567de6e0ef99c984a673cd69ce7725d6b6 
>   src/tests/resource_provider_manager_tests.cpp 
> e8ca377fd0a927b99fdaf6a8ee0139025a41298e 
>   src/tests/slave_tests.cpp ae82438a4f2f79820fe6d476e25b4915aa5f212c 
>   src/tests/storage_local_resource_provider_tests.cpp 
> ccb114aacc904998dfeef4128f6d38dfb3c865c7 
> 
> 
> Diff: https://reviews.apache.org/r/66934/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 66931: Fixed a race in resource provider resubscription test.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [66931]

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

- Mesos Reviewbot


On May 3, 2018, 11:46 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66931/
> ---
> 
> (Updated May 3, 2018, 11:46 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Jan Schlicht.
> 
> 
> Bugs: MESOS-8874
> https://issues.apache.org/jira/browse/MESOS-8874
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We previously did not make ensure that after the simulated agent
> failover in
> `ResourceProviderManagerHttpApiTest.ResubscribeResourceProvider` the
> mock resource provider created as part of the test did not reconnect
> to the restarted agent (as opposed to the newly initialized resource
> provider). This lead to unmet test expectations.
> 
> With this patch we now explicitly tear down the mock resource provider
> after we have detected that the agent went away to prevent the race.
> 
> 
> Diffs
> -
> 
>   src/tests/resource_provider_manager_tests.cpp 
> e8ca377fd0a927b99fdaf6a8ee0139025a41298e 
> 
> 
> Diff: https://reviews.apache.org/r/66931/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> Ran the test repeatedly under high system load without triggering the issue 
> again with this patch.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 66931: Fixed a race in resource provider resubscription test.

[Jan Schlicht]

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


Ship it!





src/tests/resource_provider_manager_tests.cpp
Lines 1130-1131 (patched)


Nit: `disconnected` is called asynchronously. The `Clock::settle()` below 
will make sure that it is called. Not having the settle would introduce another 
flakyness though. To be extra sure, we could set a future and await that one 
before we reset `resourceProvider` with a new instance.


- Jan Schlicht


On May 3, 2018, 1:46 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66931/
> ---
> 
> (Updated May 3, 2018, 1:46 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Jan Schlicht.
> 
> 
> Bugs: MESOS-8874
> https://issues.apache.org/jira/browse/MESOS-8874
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We previously did not make ensure that after the simulated agent
> failover in
> `ResourceProviderManagerHttpApiTest.ResubscribeResourceProvider` the
> mock resource provider created as part of the test did not reconnect
> to the restarted agent (as opposed to the newly initialized resource
> provider). This lead to unmet test expectations.
> 
> With this patch we now explicitly tear down the mock resource provider
> after we have detected that the agent went away to prevent the race.
> 
> 
> Diffs
> -
> 
>   src/tests/resource_provider_manager_tests.cpp 
> e8ca377fd0a927b99fdaf6a8ee0139025a41298e 
> 
> 
> Diff: https://reviews.apache.org/r/66931/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> Ran the test repeatedly under high system load without triggering the issue 
> again with this patch.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 66931: Fixed a race in resource provider resubscription test.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 66931 was successfully built and tested.

Reviews applied: `['66931']`

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

- Mesos Reviewbot Windows


On May 3, 2018, 1:46 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66931/
> ---
> 
> (Updated May 3, 2018, 1:46 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Jan Schlicht.
> 
> 
> Bugs: MESOS-8874
> https://issues.apache.org/jira/browse/MESOS-8874
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We previously did not make ensure that after the simulated agent
> failover in
> `ResourceProviderManagerHttpApiTest.ResubscribeResourceProvider` the
> mock resource provider created as part of the test did not reconnect
> to the restarted agent (as opposed to the newly initialized resource
> provider). This lead to unmet test expectations.
> 
> With this patch we now explicitly tear down the mock resource provider
> after we have detected that the agent went away to prevent the race.
> 
> 
> Diffs
> -
> 
>   src/tests/resource_provider_manager_tests.cpp 
> e8ca377fd0a927b99fdaf6a8ee0139025a41298e 
> 
> 
> Diff: https://reviews.apache.org/r/66931/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> Ran the test repeatedly under high system load without triggering the issue 
> again with this patch.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Review Request 66934: Removed credential-based authentication of 'MockResourceProvider'.

[Jan Schlicht]

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

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


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


Repository: mesos


Description
---

Resource provider authentication is token-based and using a separate
realm. As a result, credential parameters have been removed from
'MockResourceProvider'. Also, authentication for the resource provider
realm is disabled by default and only enabled in tests that instantiate
a Storage Local Resource Provider which support JWT.


Diffs
-

  src/tests/agent_resource_provider_config_api_tests.cpp 
6ced582e14966eea3bb109e2e2de0e554acc2968 
  src/tests/api_tests.cpp dd8e221d8fd1b2a241505345337897e4ee4a6347 
  src/tests/executor_http_api_tests.cpp 
51cca7a90b3c5c5380ede0f17d66b9fdfeb88c27 
  src/tests/master_tests.cpp d5ce52c0eb1ba333d1b3dd35518545c13d828ce6 
  src/tests/mesos.hpp 756a52189ea3336b203ea1d606e2ba17762d57aa 
  src/tests/operation_reconciliation_tests.cpp 
76c169567de6e0ef99c984a673cd69ce7725d6b6 
  src/tests/resource_provider_manager_tests.cpp 
e8ca377fd0a927b99fdaf6a8ee0139025a41298e 
  src/tests/slave_tests.cpp ae82438a4f2f79820fe6d476e25b4915aa5f212c 
  src/tests/storage_local_resource_provider_tests.cpp 
ccb114aacc904998dfeef4128f6d38dfb3c865c7 


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


Testing
---

make check


Thanks,

Jan Schlicht

Review Request 66933: Added a flag for resource provider authentication.

[Jan Schlicht]

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

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


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


Repository: mesos


Description
---

If this flag is set to true, HTTP requests to the resource provider API
have to be authenticated using JWT. A token is already provided to
Storage Local Resource Providers by agents for standalone container
authorization and will be used here as well.


Diffs
-

  src/slave/constants.hpp d1d15c39c87b9712c96b9f7516a7a0a3e56514cf 
  src/slave/flags.hpp a839591a2b66444ad97fced0620201dde656352d 
  src/slave/flags.cpp 02e1a8b0aa4215c7d07cae24afffdaa248efd8da 
  src/slave/slave.cpp 6ca3d79fd38c800f258c571bb58164427db2ac7c 
  src/tests/cluster.cpp c071da69500e1d8a223f255904acf7e28100e774 


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


Testing
---

make check


Thanks,

Jan Schlicht

Review Request 66932: Adden token based authentication for resource providers.

[Jan Schlicht]

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

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


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


Repository: mesos


Description
---

If a token is provided, it will be used in HTTP requests to the resource
provider manager. This allows JWT based authentication and authorization
for resource providers.
The (unimplemented) credential support in `resource_provider::Driver`
has been removed in favor of the token-based approach.


Diffs
-

  include/mesos/v1/resource_provider.hpp 
787c619cb6d7574c3ffcd22c5ad2f3a6a3b56694 
  src/resource_provider/driver.cpp 70b7e3244a18dd854a85c65853fe32d82a5ec7d4 
  src/resource_provider/http_connection.hpp 
0ae8124150dff96296a4a2c358338fe196717b1a 
  src/resource_provider/storage/provider.cpp 
d1267cf8df06cc758c47d47535b868a3ac741a0b 
  src/tests/mesos.hpp 756a52189ea3336b203ea1d606e2ba17762d57aa 


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


Testing
---

make check


Thanks,

Jan Schlicht

Review Request 66931: Fixed a race in resource provider resubscription test.

[Benjamin Bannier]

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

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


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


Repository: mesos


Description
---

We previously did not make ensure that after the simulated agent
failover in
`ResourceProviderManagerHttpApiTest.ResubscribeResourceProvider` the
mock resource provider created as part of the test did not reconnect
to the restarted agent (as opposed to the newly initialized resource
provider). This lead to unmet test expectations.

With this patch we now explicitly tear down the mock resource provider
after we have detected that the agent went away to prevent the race.


Diffs
-

  src/tests/resource_provider_manager_tests.cpp 
e8ca377fd0a927b99fdaf6a8ee0139025a41298e 


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


Testing
---

`make check`

Ran the test repeatedly under high system load without triggering the issue 
again with this patch.


Thanks,

Benjamin Bannier

Re: Review Request 66920: Improved tests for resizing persistent volumes.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [66218, 66049, 66733, 66050, 66219, 66858, 66220, 66531, 
66532, 66052, 66051, 66227, 66920]

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

- Mesos Reviewbot


On May 3, 2018, 4:54 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66920/
> ---
> 
> (Updated May 3, 2018, 4:54 a.m.)
> 
> 
> Review request for mesos, Greg Mann and Zhitao Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Now the `GrowVolume` and `ShrinkVolume` tests launch tasks after
> resizing the volumes to ensure that the operations take effect on
> agents. The `NonSpeculativeGrowAndLaunch` and
> `NonSpeculativeShrinkAndLaunch` tests launch an additional task to
> verify that the original volume consumed by the operations cannot be
> used by subsequent tasks.
> 
> This patch also adjusted when the clock is resumed so schedulers will
> not receive unexpected offers.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_tests.cpp 
> 4edf781711d9efdb994114aeb6289b6af750b87a 
> 
> 
> Diff: https://reviews.apache.org/r/66920/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66920: Improved tests for resizing persistent volumes.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 66920 was successfully built and tested.

Reviews applied: `['66218', '66049', '66733', '66050', '66219', '66858', 
'66220', '66531', '66532', '66052', '66051', '66227', '66920']`

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

- Mesos Reviewbot Windows


On May 3, 2018, 4:54 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66920/
> ---
> 
> (Updated May 3, 2018, 4:54 a.m.)
> 
> 
> Review request for mesos, Greg Mann and Zhitao Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Now the `GrowVolume` and `ShrinkVolume` tests launch tasks after
> resizing the volumes to ensure that the operations take effect on
> agents. The `NonSpeculativeGrowAndLaunch` and
> `NonSpeculativeShrinkAndLaunch` tests launch an additional task to
> verify that the original volume consumed by the operations cannot be
> used by subsequent tasks.
> 
> This patch also adjusted when the clock is resumed so schedulers will
> not receive unexpected offers.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_tests.cpp 
> 4edf781711d9efdb994114aeb6289b6af750b87a 
> 
> 
> Diff: https://reviews.apache.org/r/66920/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66923: Added documentation on volume resize support.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [66218, 66049, 66733, 66050, 66219, 66858, 66220, 66531, 
66532, 66052, 66051, 66227, 66923]

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

- Mesos Reviewbot


On May 3, 2018, 12:01 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66923/
> ---
> 
> (Updated May 3, 2018, 12:01 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Greg Mann.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation on volume resize support.
> 
> 
> Diffs
> -
> 
>   docs/authorization.md fdbef770c38ab38ab748b4f943b331d4442a2ce1 
>   docs/operator-http-api.md 9be1e2db60562b369bdc6ee2c732a74fad049580 
>   docs/persistent-volume.md 1a5799b8d8e5302e0b01da6b9a16a3b9fb75c898 
> 
> 
> Diff: https://reviews.apache.org/r/66923/diff/1/
> 
> 
> Testing
> ---
> 
> https://github.com/zhitaoli/mesos/blob/zhitao/public/volume_resize_mesos_4965_speculative/docs/persistent-volume.md
> https://github.com/zhitaoli/mesos/blob/zhitao/public/volume_resize_mesos_4965_speculative/docs/operator-http-api.md
> https://github.com/zhitaoli/mesos/blob/zhitao/public/volume_resize_mesos_4965_speculative/docs/authorization.md
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>