Review Request 63321: Added protobuf messages for V1 scheduler operation feedback.

2017-10-25 Thread Greg Mann

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

Review request for mesos, Gaston Kleiman, Jie Yu, and Vinod Kone.


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


Repository: mesos


Description
---

This patch adds new and updated protobuf messages to facilitate
offer operation status updates, as well as acknowledgement of
those updates and operation status reconciliation.


Diffs
-

  include/mesos/scheduler/scheduler.proto 
f82f74d1c3767f97a1e6dad8acf4602f39e18380 
  include/mesos/v1/scheduler/scheduler.proto 
1fb0254103e5bb4f2cb98ca973e9f4e7b378 
  src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f 


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


Testing
---

Testing details can be found at the end of this chain.


Thanks,

Greg Mann



Re: Review Request 63312: Updated Resources::apply for new operations.

2017-10-25 Thread Mesos Reviewbot

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



Bad patch!

Reviews applied: [63312, 63105, 61946, 61810, 63104, 63094, 62903, 63001]

Failed command: python support/apply-reviews.py -n -r 63312

Error:
2017-10-26 02:34:21 URL:https://reviews.apache.org/r/63312/diff/raw/ 
[15399/15399] -> "63312.patch" [1]
error: patch failed: src/v1/resources.cpp:1814
error: src/v1/resources.cpp: patch does not apply

Full log: https://builds.apache.org/job/Mesos-Reviewbot/19903/console

- Mesos Reviewbot


On Oct. 25, 2017, 2:16 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63312/
> ---
> 
> (Updated Oct. 25, 2017, 2:16 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Gaston Kleiman, Greg Mann, and 
> Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch updated Resources::apply method to support new operations:
> CREATE_VOLUME, DESTROY_VOLUME, CREATE_BLOCK and DESTROY_BLOCK. We
> introduced an optional `convertedResources` field to this method for
> new operations.
> 
> This patch also made sure that we don't call Resources::apply for
> LAUNCH and LAUNCH_GROUP, because it does not really "convert" the
> resources.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp 9f959494f5c0376aa4ebd8f2571f4b1f45c83cb3 
>   include/mesos/v1/resources.hpp a6216858136c82d5ebb5da3bb22c0b8a58a3b75d 
>   src/common/resources.cpp 7ee4dae1389e037531aec533a3d235ee06443ea8 
>   src/examples/persistent_volume_framework.cpp 
> 1f8f4bea7ce4d1a919034e0639b4f70e7c9961cf 
>   src/master/allocator/mesos/hierarchical.cpp 
> 5b6efe5faa3c3b10f1f714f582a155b368f8ccaf 
>   src/v1/resources.cpp 5c0a196cf87b377b345e53a36ed24727d52381ca 
> 
> 
> Diff: https://reviews.apache.org/r/63312/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 63293: Update google-glog to 0.3.5.

2017-10-25 Thread Andrew Schwartzmeyer

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



Is there a pressing need to update this _right now_? As it is, we are waiting 
for 0.3.6 (MESOS-3394) so that we can converge on a single version of Glog for 
all platforms.

- Andrew Schwartzmeyer


On Oct. 25, 2017, 5:28 a.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63293/
> ---
> 
> (Updated Oct. 25, 2017, 5:28 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-3322
> https://issues.apache.org/jira/browse/MESOS-3322
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Version 0.3.5 adds support for PowerPC so no patching is required.
> 
> Fixes: MESOS-3322
> Refs: https://github.com/google/glog/releases/tag/v0.3.5
> 
> 
> Diffs
> -
> 
>   3rdparty/glog-0.3.5.patch ab15bea04d7b8af31a638b7c4ce3256ed2f0df4e 
> 
> 
> Diff: https://reviews.apache.org/r/63293/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>



Re: Review Request 63311: Fixed Flag Tests review issues.

2017-10-25 Thread Andrew Schwartzmeyer

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



This needs to be squashed into the original review (one review per change) and 
this review discarded.

- Andrew Schwartzmeyer


On Oct. 25, 2017, 2:10 p.m., Raluca Miclea wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63311/
> ---
> 
> (Updated Oct. 25, 2017, 2:10 p.m.)
> 
> 
> Review request for mesos and Andrew Schwartzmeyer.
> 
> 
> Bugs: MESOS-3437
> https://issues.apache.org/jira/browse/MESOS-3437
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed Flag Tests review issues.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/flags_tests.cpp 
> 88c8ee57c2bf328251513761498ef98d774be006 
> 
> 
> Diff: https://reviews.apache.org/r/63311/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Raluca Miclea
> 
>



Re: Review Request 63310: Fixed Path Tests review issues.

2017-10-25 Thread Andrew Schwartzmeyer

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



This needs to be squashed into the original review (one review per change) and 
this review discarded.

- Andrew Schwartzmeyer


On Oct. 25, 2017, 2:09 p.m., Raluca Miclea wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63310/
> ---
> 
> (Updated Oct. 25, 2017, 2:09 p.m.)
> 
> 
> Review request for mesos and Andrew Schwartzmeyer.
> 
> 
> Bugs: MESOS-3442
> https://issues.apache.org/jira/browse/MESOS-3442
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed Path Tests review issues.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/path_tests.cpp 
> f8c14d5aefe0b49adb778da784143a328c96183d 
> 
> 
> Diff: https://reviews.apache.org/r/63310/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Raluca Miclea
> 
>



Re: Review Request 63253: Fixed DefaultExecutorTest for proper URI handling on Windows.

2017-10-25 Thread Jeff Coffler

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

(Updated Oct. 25, 2017, 11:19 p.m.)


Review request for mesos, Andrew Schwartzmeyer, Gaston Kleiman, John Kordich, 
Joseph Wu, and Li Li.


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


Repository: mesos


Description
---

Fixed changes from Gaston (git commit 772c8f5 on 10/02/2017)
for proper operation on Windows. Changed to use new URI methods
introduced as part of MESOS-6705, and used powershell on Windows
to replace "test" command on Linux.


Diffs (updated)
-

  src/tests/default_executor_tests.cpp 5078bd4d70698f5cbd14c971fcecfd58f8467a04 


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

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


Testing
---

Ran mesos-tests on both Windows and Linux with no errors.

Specifically ran mesos-tests with 
`--gtest_filter=MesosContainerizer/DefaultExecutorTest.*` to verify that the 
modified tests ran properly.


Thanks,

Jeff Coffler



Re: Review Request 63253: Fixed DefaultExecutorTest for proper URI handling on Windows.

2017-10-25 Thread Andrew Schwartzmeyer

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




src/tests/default_executor_tests.cpp
Lines 1652-1653 (patched)


Nit: I would prefer these to be `powershell.exe` since we know the 
executable we're calling.



src/tests/default_executor_tests.cpp
Lines 1657-1658 (patched)


I think this could be one line now.



src/tests/default_executor_tests.cpp
Line 1693 (original), 1704 (patched)


Nit: s/coffer/coffler/g, s/powershell/PowerShell/g


- Andrew Schwartzmeyer


On Oct. 25, 2017, 3:17 p.m., Jeff Coffler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63253/
> ---
> 
> (Updated Oct. 25, 2017, 3:17 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Gaston Kleiman, John Kordich, 
> Joseph Wu, and Li Li.
> 
> 
> Bugs: MESOS-6705
> https://issues.apache.org/jira/browse/MESOS-6705
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed changes from Gaston (git commit 772c8f5 on 10/02/2017)
> for proper operation on Windows. Changed to use new URI methods
> introduced as part of MESOS-6705, and used powershell on Windows
> to replace "test" command on Linux.
> 
> 
> Diffs
> -
> 
>   src/tests/default_executor_tests.cpp 
> 5078bd4d70698f5cbd14c971fcecfd58f8467a04 
> 
> 
> Diff: https://reviews.apache.org/r/63253/diff/3/
> 
> 
> Testing
> ---
> 
> Ran mesos-tests on both Windows and Linux with no errors.
> 
> Specifically ran mesos-tests with 
> `--gtest_filter=MesosContainerizer/DefaultExecutorTest.*` to verify that the 
> modified tests ran properly.
> 
> 
> Thanks,
> 
> Jeff Coffler
> 
>



Re: Review Request 63253: Fixed DefaultExecutorTest for proper URI handling on Windows.

2017-10-25 Thread Andrew Schwartzmeyer

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




src/tests/default_executor_tests.cpp
Lines 1652-1653 (patched)


Nit: I would prefer these to be `powershell.exe` since we know the 
executable we're calling.



src/tests/default_executor_tests.cpp
Lines 1657-1658 (patched)


I think this could be one line now.



src/tests/default_executor_tests.cpp
Line 1693 (original), 1704 (patched)


Nit: s/coffer/coffler/g, s/powershell/PowerShell/g


- Andrew Schwartzmeyer


On Oct. 25, 2017, 3:17 p.m., Jeff Coffler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63253/
> ---
> 
> (Updated Oct. 25, 2017, 3:17 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Gaston Kleiman, John Kordich, 
> Joseph Wu, and Li Li.
> 
> 
> Bugs: MESOS-6705
> https://issues.apache.org/jira/browse/MESOS-6705
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed changes from Gaston (git commit 772c8f5 on 10/02/2017)
> for proper operation on Windows. Changed to use new URI methods
> introduced as part of MESOS-6705, and used powershell on Windows
> to replace "test" command on Linux.
> 
> 
> Diffs
> -
> 
>   src/tests/default_executor_tests.cpp 
> 5078bd4d70698f5cbd14c971fcecfd58f8467a04 
> 
> 
> Diff: https://reviews.apache.org/r/63253/diff/3/
> 
> 
> Testing
> ---
> 
> Ran mesos-tests on both Windows and Linux with no errors.
> 
> Specifically ran mesos-tests with 
> `--gtest_filter=MesosContainerizer/DefaultExecutorTest.*` to verify that the 
> modified tests ran properly.
> 
> 
> Thanks,
> 
> Jeff Coffler
> 
>



Re: Review Request 63253: Fixed DefaultExecutorTest for proper URI handling on Windows.

2017-10-25 Thread Jeff Coffler

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

(Updated Oct. 25, 2017, 10:17 p.m.)


Review request for mesos, Andrew Schwartzmeyer, Gaston Kleiman, John Kordich, 
Joseph Wu, and Li Li.


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


Repository: mesos


Description
---

Fixed changes from Gaston (git commit 772c8f5 on 10/02/2017)
for proper operation on Windows. Changed to use new URI methods
introduced as part of MESOS-6705, and used powershell on Windows
to replace "test" command on Linux.


Diffs (updated)
-

  src/tests/default_executor_tests.cpp 5078bd4d70698f5cbd14c971fcecfd58f8467a04 


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

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


Testing
---

Ran mesos-tests on both Windows and Linux with no errors.

Specifically ran mesos-tests with 
`--gtest_filter=MesosContainerizer/DefaultExecutorTest.*` to verify that the 
modified tests ran properly.


Thanks,

Jeff Coffler



Re: Review Request 63253: Fixed DefaultExecutorTest for proper URI handling on Windows.

2017-10-25 Thread Jeff Coffler


> On Oct. 25, 2017, 10:07 p.m., Gaston Kleiman wrote:
> > src/tests/default_executor_tests.cpp
> > Lines 1657 (patched)
> > 
> >
> > Nit: do we need this the whitespace after the opening quotes?

There used to be a line before that (to invoke powershell), but it's gone now. 
Space removed after the opening quotes, and removed one space from the line 
below.


- Jeff


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


On Oct. 25, 2017, 7:59 p.m., Jeff Coffler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63253/
> ---
> 
> (Updated Oct. 25, 2017, 7:59 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Gaston Kleiman, John Kordich, 
> Joseph Wu, and Li Li.
> 
> 
> Bugs: MESOS-6705
> https://issues.apache.org/jira/browse/MESOS-6705
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed changes from Gaston (git commit 772c8f5 on 10/02/2017)
> for proper operation on Windows. Changed to use new URI methods
> introduced as part of MESOS-6705, and used powershell on Windows
> to replace "test" command on Linux.
> 
> 
> Diffs
> -
> 
>   src/tests/default_executor_tests.cpp 
> 5078bd4d70698f5cbd14c971fcecfd58f8467a04 
> 
> 
> Diff: https://reviews.apache.org/r/63253/diff/2/
> 
> 
> Testing
> ---
> 
> Ran mesos-tests on both Windows and Linux with no errors.
> 
> Specifically ran mesos-tests with 
> `--gtest_filter=MesosContainerizer/DefaultExecutorTest.*` to verify that the 
> modified tests ran properly.
> 
> 
> Thanks,
> 
> Jeff Coffler
> 
>



Re: Review Request 63253: Fixed DefaultExecutorTest for proper URI handling on Windows.

2017-10-25 Thread Gaston Kleiman

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


Fix it, then Ship it!





src/tests/default_executor_tests.cpp
Lines 1657 (patched)


Nit: do we need this the whitespace after the opening quotes?


- Gaston Kleiman


On Oct. 25, 2017, 12:59 p.m., Jeff Coffler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63253/
> ---
> 
> (Updated Oct. 25, 2017, 12:59 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Gaston Kleiman, John Kordich, 
> Joseph Wu, and Li Li.
> 
> 
> Bugs: MESOS-6705
> https://issues.apache.org/jira/browse/MESOS-6705
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed changes from Gaston (git commit 772c8f5 on 10/02/2017)
> for proper operation on Windows. Changed to use new URI methods
> introduced as part of MESOS-6705, and used powershell on Windows
> to replace "test" command on Linux.
> 
> 
> Diffs
> -
> 
>   src/tests/default_executor_tests.cpp 
> 5078bd4d70698f5cbd14c971fcecfd58f8467a04 
> 
> 
> Diff: https://reviews.apache.org/r/63253/diff/2/
> 
> 
> Testing
> ---
> 
> Ran mesos-tests on both Windows and Linux with no errors.
> 
> Specifically ran mesos-tests with 
> `--gtest_filter=MesosContainerizer/DefaultExecutorTest.*` to verify that the 
> modified tests ran properly.
> 
> 
> Thanks,
> 
> Jeff Coffler
> 
>



Review Request 63315: Allowe override of parallel jobs in docker build.

2017-10-25 Thread Tomasz Janiszewski

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

Review request for mesos, Jie Yu and Vinod Kone.


Repository: mesos


Description
---

Use JOBS env variable to control `make -j` parameter.
By default `JOBS=6`.


Diffs
-

  support/docker-build.sh 50e693ca71d499f71f2935923f4691feef560b12 


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


Testing
---


Thanks,

Tomasz Janiszewski



Re: Review Request 63253: Fixed DefaultExecutorTest for proper URI handling on Windows.

2017-10-25 Thread Mesos Reviewbot Windows

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



PASS: Mesos patch 63253 was successfully built and tested.

Reviews applied: `['60620', '60621', '60622', '60623', '60624', '60626', 
'60628', '63253']`

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

- Mesos Reviewbot Windows


On Oct. 25, 2017, 7:59 p.m., Jeff Coffler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63253/
> ---
> 
> (Updated Oct. 25, 2017, 7:59 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Gaston Kleiman, John Kordich, 
> Joseph Wu, and Li Li.
> 
> 
> Bugs: MESOS-6705
> https://issues.apache.org/jira/browse/MESOS-6705
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed changes from Gaston (git commit 772c8f5 on 10/02/2017)
> for proper operation on Windows. Changed to use new URI methods
> introduced as part of MESOS-6705, and used powershell on Windows
> to replace "test" command on Linux.
> 
> 
> Diffs
> -
> 
>   src/tests/default_executor_tests.cpp 
> 5078bd4d70698f5cbd14c971fcecfd58f8467a04 
> 
> 
> Diff: https://reviews.apache.org/r/63253/diff/2/
> 
> 
> Testing
> ---
> 
> Ran mesos-tests on both Windows and Linux with no errors.
> 
> Specifically ran mesos-tests with 
> `--gtest_filter=MesosContainerizer/DefaultExecutorTest.*` to verify that the 
> modified tests ran properly.
> 
> 
> Thanks,
> 
> Jeff Coffler
> 
>



Review Request 63314: Add support for Ubuntu 16.04 in docker build.

2017-10-25 Thread Tomasz Janiszewski

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

Review request for mesos, Jie Yu and Vinod Kone.


Repository: mesos


Description
---

Add support for Ubuntu 16.04 in docker build.


Diffs
-

  support/docker-build.sh 50e693ca71d499f71f2935923f4691feef560b12 


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


Testing
---


Thanks,

Tomasz Janiszewski



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

2017-10-25 Thread James Peach

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




src/linux/fs.hpp
Lines 276 (patched)


Not "immediate parent", but longest match right?



src/linux/fs.cpp
Lines 236 (patched)


Can you add some tests to `tests/containerizer/fs_tests.cpp`?



src/linux/fs.cpp
Lines 249 (patched)


So if the target was "/mnt/foobar" and we had a mount called "/mnt/foo", 
this would match? Is that what we want here?



src/linux/fs.cpp
Lines 257 (patched)


Did you consider using a `Result` and returning `None()` in this case?


- James Peach


On Oct. 23, 2017, 4:43 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63212/
> ---
> 
> (Updated Oct. 23, 2017, 4:43 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, James Peach, and Joseph Wu.
> 
> 
> Bugs: MESOS-7306
> https://issues.apache.org/jira/browse/MESOS-7306
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This method allows us to get the mount table entry that contains the
> specified target.
> 
> 
> Diffs
> -
> 
>   src/linux/fs.hpp cbc8bf79083ce2bc34fa698808eaf92764a577a9 
>   src/linux/fs.cpp 40c31a3ad2ccbeb59868c5e2d7b1809c613da8de 
> 
> 
> Diff: https://reviews.apache.org/r/63212/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



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

2017-10-25 Thread James Peach

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




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


How does this interact with the "unshare_namespace_mnt" flag? Do we also 
want to mark mounts made in that new namespace as slave mounts?



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


"the mount namespace"



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


"pollute"



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


I didn't quite understand what you meant by "recursively slave 
propagation". Could you clarify this a bit?


- James Peach


On Oct. 23, 2017, 4:42 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63211/
> ---
> 
> (Updated Oct. 23, 2017, 4:42 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, James Peach, and Joseph Wu.
> 
> 
> Bugs: MESOS-7306
> https://issues.apache.org/jira/browse/MESOS-7306
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch prepares for the mount propagation support. In the future,
> we can no longer set the root mount as rslave bindly because some
> mounts need to be shared. Move this logic to the launch helper is a
> necessary step. We will add more logic there to selectively mark some
> mount as shared, and the rest as slave.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
> faf94909f995f7486b5f9cb7532af58a90a9eed3 
>   src/slave/containerizer/mesos/launch.cpp 
> 49f11f1d586672bb46f6eccabcfda9321cc3c607 
> 
> 
> Diff: https://reviews.apache.org/r/63211/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Review Request 63312: Updated Resources::apply for new operations.

2017-10-25 Thread Jie Yu

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

Review request for mesos, Benjamin Bannier, Gaston Kleiman, Greg Mann, and Jan 
Schlicht.


Repository: mesos


Description
---

This patch updated Resources::apply method to support new operations:
CREATE_VOLUME, DESTROY_VOLUME, CREATE_BLOCK and DESTROY_BLOCK. We
introduced an optional `convertedResources` field to this method for
new operations.

This patch also made sure that we don't call Resources::apply for
LAUNCH and LAUNCH_GROUP, because it does not really "convert" the
resources.


Diffs
-

  include/mesos/resources.hpp 9f959494f5c0376aa4ebd8f2571f4b1f45c83cb3 
  include/mesos/v1/resources.hpp a6216858136c82d5ebb5da3bb22c0b8a58a3b75d 
  src/common/resources.cpp 7ee4dae1389e037531aec533a3d235ee06443ea8 
  src/examples/persistent_volume_framework.cpp 
1f8f4bea7ce4d1a919034e0639b4f70e7c9961cf 
  src/master/allocator/mesos/hierarchical.cpp 
5b6efe5faa3c3b10f1f714f582a155b368f8ccaf 
  src/v1/resources.cpp 5c0a196cf87b377b345e53a36ed24727d52381ca 


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


Testing
---

make check


Thanks,

Jie Yu



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

2017-10-25 Thread Jie Yu

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

(Updated Oct. 25, 2017, 9:15 p.m.)


Review request for mesos, Gilbert Song, James Peach, and Joseph Wu.


Changes
---

Fixed a compile issue on OSX.


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


Repository: mesos


Description
---

Added ContainerMountInfo protobuf in ContainerLaunchInfo. It is used
by isolators and MesosContainerizer to describe mounts. Previously,
the additional mounts for each container is specified using
'pre_exec_commands', which is not very flexible and slow (requires a
fork/exec for each mount).


Diffs (updated)
-

  include/mesos/slave/containerizer.proto 
689acfcbbb07f071b6195472118a7a7520a44abd 
  src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
ed6556830c1d6a72df22c1780ac77fe7d6225f99 
  src/slave/containerizer/mesos/isolators/filesystem/linux.hpp 
c7fbda55e619052edfa5cd97114544d8fefc3ea5 
  src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
faf94909f995f7486b5f9cb7532af58a90a9eed3 
  src/slave/containerizer/mesos/isolators/filesystem/shared.cpp 
927d95be2c25ceb324acac99edaee24a28a5d59c 
  src/slave/containerizer/mesos/isolators/gpu/isolator.cpp 
25636b50314cb4ea53d1931c97c87c65ecb658a8 
  src/slave/containerizer/mesos/isolators/namespaces/pid.cpp 
42bc2e1245c9c957604e6625244a48a564ca5a8c 
  src/slave/containerizer/mesos/isolators/volume/host_path.cpp 
949510aba6b1f30a64f5f9a0777a4f03e2701302 
  src/slave/containerizer/mesos/isolators/volume/image.cpp 
35966aa9a8702e53fa95b3e045cba9327623b99a 
  src/slave/containerizer/mesos/isolators/volume/sandbox_path.cpp 
ee5ea3dee3be197e923be544aab96806f0adf1cf 
  src/slave/containerizer/mesos/launch.cpp 
49f11f1d586672bb46f6eccabcfda9321cc3c607 


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

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


Testing
---

sudo make check


Thanks,

Jie Yu



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

2017-10-25 Thread Jie Yu

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

(Updated Oct. 25, 2017, 9:13 p.m.)


Review request for mesos, Gilbert Song, James Peach, and Joseph Wu.


Changes
---

Fixed a compilation issue on osx


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


Repository: mesos


Description
---

Added ContainerMountInfo protobuf in ContainerLaunchInfo. It is used
by isolators and MesosContainerizer to describe mounts. Previously,
the additional mounts for each container is specified using
'pre_exec_commands', which is not very flexible and slow (requires a
fork/exec for each mount).


Diffs (updated)
-

  include/mesos/slave/containerizer.proto 
689acfcbbb07f071b6195472118a7a7520a44abd 
  src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
ed6556830c1d6a72df22c1780ac77fe7d6225f99 
  src/slave/containerizer/mesos/isolators/filesystem/linux.hpp 
c7fbda55e619052edfa5cd97114544d8fefc3ea5 
  src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
faf94909f995f7486b5f9cb7532af58a90a9eed3 
  src/slave/containerizer/mesos/isolators/filesystem/shared.cpp 
927d95be2c25ceb324acac99edaee24a28a5d59c 
  src/slave/containerizer/mesos/isolators/gpu/isolator.cpp 
25636b50314cb4ea53d1931c97c87c65ecb658a8 
  src/slave/containerizer/mesos/isolators/namespaces/pid.cpp 
42bc2e1245c9c957604e6625244a48a564ca5a8c 
  src/slave/containerizer/mesos/isolators/volume/host_path.cpp 
949510aba6b1f30a64f5f9a0777a4f03e2701302 
  src/slave/containerizer/mesos/isolators/volume/image.cpp 
35966aa9a8702e53fa95b3e045cba9327623b99a 
  src/slave/containerizer/mesos/isolators/volume/sandbox_path.cpp 
ee5ea3dee3be197e923be544aab96806f0adf1cf 
  src/slave/containerizer/mesos/launch.cpp 
49f11f1d586672bb46f6eccabcfda9321cc3c607 


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

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


Testing
---

sudo make check


Thanks,

Jie Yu



Review Request 63311: Fixed Flag Tests review issues.

2017-10-25 Thread Raluca Miclea

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

Review request for mesos and Andrew Schwartzmeyer.


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


Repository: mesos


Description
---

Fixed Flag Tests review issues.


Diffs
-

  3rdparty/stout/tests/flags_tests.cpp 88c8ee57c2bf328251513761498ef98d774be006 


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


Testing
---


Thanks,

Raluca Miclea



Review Request 63310: Fixed Path Tests review issues.

2017-10-25 Thread Raluca Miclea

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

Review request for mesos and Andrew Schwartzmeyer.


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


Repository: mesos


Description
---

Fixed Path Tests review issues.


Diffs
-

  3rdparty/stout/tests/path_tests.cpp f8c14d5aefe0b49adb778da784143a328c96183d 


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


Testing
---


Thanks,

Raluca Miclea



Re: Review Request 63304: Removed Flag Tests TODO comments.

2017-10-25 Thread Andrew Schwartzmeyer

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



This review request should squashed into 63239 (as in, amend the commit used to 
post 63239, and then re-run the post-reviews script, it will ask to update the 
existing review, then discard this review).

- Andrew Schwartzmeyer


On Oct. 25, 2017, 1:35 p.m., Raluca Miclea wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63304/
> ---
> 
> (Updated Oct. 25, 2017, 1:35 p.m.)
> 
> 
> Review request for mesos and Andrew Schwartzmeyer.
> 
> 
> Bugs: MESOS-3437
> https://issues.apache.org/jira/browse/MESOS-3437
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed Flag Tests TODO comments.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/flags_tests.cpp 
> 88c8ee57c2bf328251513761498ef98d774be006 
> 
> 
> Diff: https://reviews.apache.org/r/63304/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Raluca Miclea
> 
>



Re: Review Request 63304: Removed Flag Tests TODO comments.

2017-10-25 Thread Raluca Miclea

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

(Updated Oct. 25, 2017, 8:35 p.m.)


Review request for mesos and Andrew Schwartzmeyer.


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


Repository: mesos


Description
---

Removed Flag Tests TODO comments.


Diffs (updated)
-

  3rdparty/stout/tests/flags_tests.cpp 88c8ee57c2bf328251513761498ef98d774be006 


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

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


Testing
---


Thanks,

Raluca Miclea



Re: Review Request 63253: Fixed DefaultExecutorTest for proper URI handling on Windows.

2017-10-25 Thread Jeff Coffler

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

(Updated Oct. 25, 2017, 7:59 p.m.)


Review request for mesos, Andrew Schwartzmeyer, Gaston Kleiman, John Kordich, 
Joseph Wu, and Li Li.


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


Repository: mesos


Description
---

Fixed changes from Gaston (git commit 772c8f5 on 10/02/2017)
for proper operation on Windows. Changed to use new URI methods
introduced as part of MESOS-6705, and used powershell on Windows
to replace "test" command on Linux.


Diffs (updated)
-

  src/tests/default_executor_tests.cpp 5078bd4d70698f5cbd14c971fcecfd58f8467a04 


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

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


Testing
---

Ran mesos-tests on both Windows and Linux with no errors.

Specifically ran mesos-tests with 
`--gtest_filter=MesosContainerizer/DefaultExecutorTest.*` to verify that the 
modified tests ran properly.


Thanks,

Jeff Coffler



Re: Review Request 63253: Fixed DefaultExecutorTest for proper URI handling on Windows.

2017-10-25 Thread Jeff Coffler


> On Oct. 25, 2017, 4:38 p.m., Gaston Kleiman wrote:
> > src/tests/default_executor_tests.cpp
> > Lines 1650 (patched)
> > 
> >
> > This should be:
> > 
> > `" if ((Get-Content testFile) -NotMatch 'pizza')"`
> > 
> > To make sure that the file is copied to the sandbox, and that the 
> > task's working dir is the sandbox.
> 
> Andrew Schwartzmeyer wrote:
> This leads into a larger problem on Windows: `shell` tasks are run under 
> `cmd.exe`, which as a legacy app does not support long paths (actually, it 
> doesn't even recognize them, it thinks they're UNC paths are bails), so it 
> falls back to a temp directory. Until we've swapped the default shell on 
> Windows from `cmd.exe` to `powershell.exe`, this is a problem. The task _is_ 
> started in the sandbox, but `cmd.exe` then changes directory :(
> 
> All that said, we went the route of using `testFilePath`, the full path 
> to the file ensure its contents are correct (that the file was copied right). 
> But you're right, I think it makes more sense to ensure the task was started 
> in the right place, and then test it relatively.
> 
> Jeff, this means to go back to using `set_shell(false)` etc. to 
> `powershell.exe` directly, instead of under `cmd.exe`.

Yup, I knew that when I discussed this with Gaston. All set with this.


- Jeff


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


On Oct. 24, 2017, 6:04 p.m., Jeff Coffler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63253/
> ---
> 
> (Updated Oct. 24, 2017, 6:04 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and 
> Li Li.
> 
> 
> Bugs: MESOS-6705
> https://issues.apache.org/jira/browse/MESOS-6705
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed changes from Gaston (git commit 772c8f5 on 10/02/2017)
> for proper operation on Windows. Changed to use new URI methods
> introduced as part of MESOS-6705, and used powershell on Windows
> to replace "test" command on Linux.
> 
> 
> Diffs
> -
> 
>   src/tests/default_executor_tests.cpp 
> 5078bd4d70698f5cbd14c971fcecfd58f8467a04 
> 
> 
> Diff: https://reviews.apache.org/r/63253/diff/1/
> 
> 
> Testing
> ---
> 
> Ran mesos-tests on both Windows and Linux with no errors.
> 
> Specifically ran mesos-tests with 
> `--gtest_filter=MesosContainerizer/DefaultExecutorTest.*` to verify that the 
> modified tests ran properly.
> 
> 
> Thanks,
> 
> Jeff Coffler
> 
>



Re: Review Request 63253: Fixed DefaultExecutorTest for proper URI handling on Windows.

2017-10-25 Thread Jeff Coffler


> On Oct. 25, 2017, 3:58 p.m., Gaston Kleiman wrote:
> > src/tests/default_executor_tests.cpp
> > Lines 1764 (patched)
> > 
> >
> > The indentation here looks strange.

This was removed since it's specific to Linux (alpine image).


> On Oct. 25, 2017, 3:58 p.m., Gaston Kleiman wrote:
> > src/tests/default_executor_tests.cpp
> > Lines 1766-1770 (patched)
> > 
> >
> > I don't think this will work inside an `alpine` Docker container, will 
> > it?

Removed since this isn't appropriate for alpine container. I missed that the 
first time, thanks for pointing it out!


- Jeff


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


On Oct. 24, 2017, 6:04 p.m., Jeff Coffler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63253/
> ---
> 
> (Updated Oct. 24, 2017, 6:04 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and 
> Li Li.
> 
> 
> Bugs: MESOS-6705
> https://issues.apache.org/jira/browse/MESOS-6705
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed changes from Gaston (git commit 772c8f5 on 10/02/2017)
> for proper operation on Windows. Changed to use new URI methods
> introduced as part of MESOS-6705, and used powershell on Windows
> to replace "test" command on Linux.
> 
> 
> Diffs
> -
> 
>   src/tests/default_executor_tests.cpp 
> 5078bd4d70698f5cbd14c971fcecfd58f8467a04 
> 
> 
> Diff: https://reviews.apache.org/r/63253/diff/1/
> 
> 
> Testing
> ---
> 
> Ran mesos-tests on both Windows and Linux with no errors.
> 
> Specifically ran mesos-tests with 
> `--gtest_filter=MesosContainerizer/DefaultExecutorTest.*` to verify that the 
> modified tests ran properly.
> 
> 
> Thanks,
> 
> Jeff Coffler
> 
>



Re: Review Request 63253: Fixed DefaultExecutorTest for proper URI handling on Windows.

2017-10-25 Thread Jeff Coffler


> On Oct. 25, 2017, 5:25 p.m., Andrew Schwartzmeyer wrote:
> > src/tests/default_executor_tests.cpp
> > Line 1766 (original), 1772 (patched)
> > 
> >
> > Is it just me or did this indentation change?

I don't see that. It's indented two spaces, and it used to be indented two 
spaces. Do you see something different?


- Jeff


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


On Oct. 24, 2017, 6:04 p.m., Jeff Coffler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63253/
> ---
> 
> (Updated Oct. 24, 2017, 6:04 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and 
> Li Li.
> 
> 
> Bugs: MESOS-6705
> https://issues.apache.org/jira/browse/MESOS-6705
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed changes from Gaston (git commit 772c8f5 on 10/02/2017)
> for proper operation on Windows. Changed to use new URI methods
> introduced as part of MESOS-6705, and used powershell on Windows
> to replace "test" command on Linux.
> 
> 
> Diffs
> -
> 
>   src/tests/default_executor_tests.cpp 
> 5078bd4d70698f5cbd14c971fcecfd58f8467a04 
> 
> 
> Diff: https://reviews.apache.org/r/63253/diff/1/
> 
> 
> Testing
> ---
> 
> Ran mesos-tests on both Windows and Linux with no errors.
> 
> Specifically ran mesos-tests with 
> `--gtest_filter=MesosContainerizer/DefaultExecutorTest.*` to verify that the 
> modified tests ran properly.
> 
> 
> Thanks,
> 
> Jeff Coffler
> 
>



Re: Review Request 63304: Removed Flag Tests TODO comments.

2017-10-25 Thread Mesos Reviewbot Windows

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



FAIL: Failed to apply the current review.

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

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

Relevant logs:

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

```
error: patch failed: 3rdparty/stout/tests/flags_tests.cpp:226
error: 3rdparty/stout/tests/flags_tests.cpp: patch does not apply
```

- Mesos Reviewbot Windows


On Oct. 25, 2017, 12:43 p.m., Raluca Miclea wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63304/
> ---
> 
> (Updated Oct. 25, 2017, 12:43 p.m.)
> 
> 
> Review request for mesos and Andrew Schwartzmeyer.
> 
> 
> Bugs: MESOS-3437
> https://issues.apache.org/jira/browse/MESOS-3437
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed Flag Tests TODO comments.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/flags_tests.cpp 
> 88c8ee57c2bf328251513761498ef98d774be006 
> 
> 
> Diff: https://reviews.apache.org/r/63304/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Raluca Miclea
> 
>



Review Request 63304: Removed Flag Tests TODO comments.

2017-10-25 Thread Raluca Miclea

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

Review request for mesos and Andrew Schwartzmeyer.


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


Repository: mesos


Description
---

Removed Flag Tests TODO comments.


Diffs
-

  3rdparty/stout/tests/flags_tests.cpp 88c8ee57c2bf328251513761498ef98d774be006 


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


Testing
---


Thanks,

Raluca Miclea



Re: Review Request 63280: Made the containerizer launch be explicit about O_CLOEXEC.

2017-10-25 Thread Mesos Reviewbot Windows

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



PASS: Mesos patch 63280 was successfully built and tested.

Reviews applied: `['63270', '63280']`

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

- Mesos Reviewbot Windows


On Oct. 24, 2017, 11:29 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63280/
> ---
> 
> (Updated Oct. 24, 2017, 11:29 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Greg Mann, and Jie Yu.
> 
> 
> Bugs: MESOS-8128
> https://issues.apache.org/jira/browse/MESOS-8128
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since the containerizer launch depends on the inherited pipe to signal
> the forked child, be explicit about setting O_CLOEXEC on the pipe file
> descriptors. Make sure to close the pipe on the error paths.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 100e3bbda543d87808da9ff6bea42da5099ea8c5 
> 
> 
> Diff: https://reviews.apache.org/r/63280/diff/1/
> 
> 
> Testing
> ---
> 
> make check (Fedora 25)
> 
> 
> Thanks,
> 
> James Peach
> 
>



Re: Review Request 63294: Update glog patch (config.guess) to support ARM.

2017-10-25 Thread Mesos Reviewbot

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



Bad review!

Reviews applied: []

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

- Mesos Reviewbot


On Oct. 25, 2017, 2:27 p.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63294/
> ---
> 
> (Updated Oct. 25, 2017, 2:27 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `config.guess` in glog 0.3.5 do not recognize ARM processors
> properly. We need to update this script in order to build
> glog on ARM. The patch could be removed after upgrading to 0.3.6.
> 
> 
> Diffs
> -
> 
>   3rdparty/glog-0.3.5.patch PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/63294/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>



Re: Review Request 63253: Fixed DefaultExecutorTest for proper URI handling on Windows.

2017-10-25 Thread Andrew Schwartzmeyer

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




src/tests/default_executor_tests.cpp
Lines 1764 (patched)


+1, I missed this.



src/tests/default_executor_tests.cpp
Lines 1766-1770 (patched)


Well, no but nothing will run `alpine` in a Windows container.

Looking at `docker_containerizer_tests.cpp`, they're all hard-coded to use 
`alpine`, so we should drop this change entirely for now. Another team member 
is working to enable the Docker unit tests on Windows (which will involve 
swapping from `alpine` to `nanoserver` on Windows).



src/tests/default_executor_tests.cpp
Line 1766 (original), 1772 (patched)


Is it just me or did this indentation change?


- Andrew Schwartzmeyer


On Oct. 24, 2017, 11:04 a.m., Jeff Coffler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63253/
> ---
> 
> (Updated Oct. 24, 2017, 11:04 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and 
> Li Li.
> 
> 
> Bugs: MESOS-6705
> https://issues.apache.org/jira/browse/MESOS-6705
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed changes from Gaston (git commit 772c8f5 on 10/02/2017)
> for proper operation on Windows. Changed to use new URI methods
> introduced as part of MESOS-6705, and used powershell on Windows
> to replace "test" command on Linux.
> 
> 
> Diffs
> -
> 
>   src/tests/default_executor_tests.cpp 
> 5078bd4d70698f5cbd14c971fcecfd58f8467a04 
> 
> 
> Diff: https://reviews.apache.org/r/63253/diff/1/
> 
> 
> Testing
> ---
> 
> Ran mesos-tests on both Windows and Linux with no errors.
> 
> Specifically ran mesos-tests with 
> `--gtest_filter=MesosContainerizer/DefaultExecutorTest.*` to verify that the 
> modified tests ran properly.
> 
> 
> Thanks,
> 
> Jeff Coffler
> 
>



Re: Review Request 63238: Ported and enabled Path Tests on Windows.

2017-10-25 Thread Raluca Miclea


> On Oct. 24, 2017, 6:32 p.m., Andrew Schwartzmeyer wrote:
> > 3rdparty/stout/tests/path_tests.cpp
> > Lines 40-61 (original), 43-90 (patched)
> > 
> >
> > Instead of using an `#ifdef` here, we can probably just use 
> > `path::join("a", "b")`, as that will join them with the 
> > [`os::PATH_SEPARATOR`](https://github.com/apache/mesos/blob/edba292659e05d7a0989eb122f7824e1c21db8fb/3rdparty/stout/include/stout/os/constants.hpp#L26)
> >  which is `\` on Windows and `/` elsewhere.

Will do.


> On Oct. 24, 2017, 6:32 p.m., Andrew Schwartzmeyer wrote:
> > 3rdparty/stout/tests/path_tests.cpp
> > Lines 101-134 (patched)
> > 
> >
> > Ditto. We can halve the amount of code by using `path::join`, and for 
> > cases where we have intermittent slashes etc., use `os::PATH_SEPARATOR` 
> > (I'd probably save that into a short local variable as it'll get referenced 
> > a lot in some of these.)

Will do.


> On Oct. 24, 2017, 6:32 p.m., Andrew Schwartzmeyer wrote:
> > 3rdparty/stout/tests/path_tests.cpp
> > Lines 172-187 (patched)
> > 
> >
> > Ditto.

Will do.


> On Oct. 24, 2017, 6:32 p.m., Andrew Schwartzmeyer wrote:
> > 3rdparty/stout/tests/path_tests.cpp
> > Lines 212-235 (patched)
> > 
> >
> > Now this is where it gets especially annoying, since we can't use 
> > `path::join` to test itself. But I would take:
> > 
> > ```
> > EXPECT_EQ("\a\b\c\", path::join("\a\", "\b\", "\c\"));
> > ```
> > 
> > and make it:
> > 
> > ```
> > const string sep = stringify(os::PATH_SEPARATOR);
> > EXPECT_EQ(sep + "a" + sep + "b" + sep + "c" + sep, path::join(sep + "a" 
> > + sep, sep + "b" + sep, sep + "c" + sep));
> > ```
> > 
> > At least this code can be reused, such that if the test gets changed, 
> > tests for both platforms are then updated always at the same time.

Will do.


- Raluca


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


On Oct. 24, 2017, 7:59 a.m., Raluca Miclea wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63238/
> ---
> 
> (Updated Oct. 24, 2017, 7:59 a.m.)
> 
> 
> Review request for mesos and Andrew Schwartzmeyer.
> 
> 
> Bugs: MESOS-3442
> https://issues.apache.org/jira/browse/MESOS-3442
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> I checked #ifdefs in order to decide what tests to build and run on
> Window and Posix systems respectively.
> For Windows tests I mainly replaced the "slash" path separator with
> "backslash".
> I extracted common assertion before the #ifdef directive.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/path_tests.cpp 
> f8c14d5aefe0b49adb778da784143a328c96183d 
> 
> 
> Diff: https://reviews.apache.org/r/63238/diff/1/
> 
> 
> Testing
> ---
> 
> Modified Tests:
> TEST(PathTest, Basename)
> TEST(PathTest, Dirname)
> TEST(PathTest, Extension)
> TEST(PathTest, Join)
> 
> 
> Thanks,
> 
> Raluca Miclea
> 
>



Re: Review Request 63257: Added a few test helpers for creating disk resources.

2017-10-25 Thread Mesos Reviewbot Windows

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



PASS: Mesos patch 63257 was successfully built and tested.

Reviews applied: `['63001', '62903', '63094', '63104', '63254', '63255', 
'63256', '63257']`

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

- Mesos Reviewbot Windows


On Oct. 24, 2017, 6:06 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63257/
> ---
> 
> (Updated Oct. 24, 2017, 6:06 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Gaston Kleiman, Greg Mann, and 
> Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a few test helpers for creating disk resources.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.hpp 4b61f2dbd67a5e8a65a6655519c8f03e5d6c6954 
> 
> 
> Diff: https://reviews.apache.org/r/63257/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 63253: Fixed DefaultExecutorTest for proper URI handling on Windows.

2017-10-25 Thread Gaston Kleiman

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




src/tests/default_executor_tests.cpp
Lines 1650 (patched)


This should be:

`" if ((Get-Content testFile) -NotMatch 'pizza')"`

To make sure that the file is copied to the sandbox, and that the task's 
working dir is the sandbox.


- Gaston Kleiman


On Oct. 24, 2017, 11:04 a.m., Jeff Coffler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63253/
> ---
> 
> (Updated Oct. 24, 2017, 11:04 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and 
> Li Li.
> 
> 
> Bugs: MESOS-6705
> https://issues.apache.org/jira/browse/MESOS-6705
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed changes from Gaston (git commit 772c8f5 on 10/02/2017)
> for proper operation on Windows. Changed to use new URI methods
> introduced as part of MESOS-6705, and used powershell on Windows
> to replace "test" command on Linux.
> 
> 
> Diffs
> -
> 
>   src/tests/default_executor_tests.cpp 
> 5078bd4d70698f5cbd14c971fcecfd58f8467a04 
> 
> 
> Diff: https://reviews.apache.org/r/63253/diff/1/
> 
> 
> Testing
> ---
> 
> Ran mesos-tests on both Windows and Linux with no errors.
> 
> Specifically ran mesos-tests with 
> `--gtest_filter=MesosContainerizer/DefaultExecutorTest.*` to verify that the 
> modified tests ran properly.
> 
> 
> Thanks,
> 
> Jeff Coffler
> 
>



Re: Review Request 63253: Fixed DefaultExecutorTest for proper URI handling on Windows.

2017-10-25 Thread Gaston Kleiman

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




src/tests/default_executor_tests.cpp
Lines 1764 (patched)


The indentation here looks strange.



src/tests/default_executor_tests.cpp
Lines 1766-1770 (patched)


I don't think this will work inside an `alpine` Docker container, will it?


- Gaston Kleiman


On Oct. 24, 2017, 11:04 a.m., Jeff Coffler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63253/
> ---
> 
> (Updated Oct. 24, 2017, 11:04 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and 
> Li Li.
> 
> 
> Bugs: MESOS-6705
> https://issues.apache.org/jira/browse/MESOS-6705
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed changes from Gaston (git commit 772c8f5 on 10/02/2017)
> for proper operation on Windows. Changed to use new URI methods
> introduced as part of MESOS-6705, and used powershell on Windows
> to replace "test" command on Linux.
> 
> 
> Diffs
> -
> 
>   src/tests/default_executor_tests.cpp 
> 5078bd4d70698f5cbd14c971fcecfd58f8467a04 
> 
> 
> Diff: https://reviews.apache.org/r/63253/diff/1/
> 
> 
> Testing
> ---
> 
> Ran mesos-tests on both Windows and Linux with no errors.
> 
> Specifically ran mesos-tests with 
> `--gtest_filter=MesosContainerizer/DefaultExecutorTest.*` to verify that the 
> modified tests ran properly.
> 
> 
> Thanks,
> 
> Jeff Coffler
> 
>



Re: Review Request 63253: Fixed DefaultExecutorTest for proper URI handling on Windows.

2017-10-25 Thread Mesos Reviewbot Windows

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



PASS: Mesos patch 63253 was successfully built and tested.

Reviews applied: `['60620', '60621', '60622', '60623', '60624', '60626', 
'60628', '63253']`

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

- Mesos Reviewbot Windows


On Oct. 24, 2017, 6:04 p.m., Jeff Coffler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63253/
> ---
> 
> (Updated Oct. 24, 2017, 6:04 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and 
> Li Li.
> 
> 
> Bugs: MESOS-6705
> https://issues.apache.org/jira/browse/MESOS-6705
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed changes from Gaston (git commit 772c8f5 on 10/02/2017)
> for proper operation on Windows. Changed to use new URI methods
> introduced as part of MESOS-6705, and used powershell on Windows
> to replace "test" command on Linux.
> 
> 
> Diffs
> -
> 
>   src/tests/default_executor_tests.cpp 
> 5078bd4d70698f5cbd14c971fcecfd58f8467a04 
> 
> 
> Diff: https://reviews.apache.org/r/63253/diff/1/
> 
> 
> Testing
> ---
> 
> Ran mesos-tests on both Windows and Linux with no errors.
> 
> Specifically ran mesos-tests with 
> `--gtest_filter=MesosContainerizer/DefaultExecutorTest.*` to verify that the 
> modified tests ran properly.
> 
> 
> Thanks,
> 
> Jeff Coffler
> 
>



Review Request 63295: Update glog-0.3.3.patch to build on ARM.

2017-10-25 Thread Tomasz Janiszewski

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

Review request for mesos.


Repository: mesos


Description
---

glog has an old version of config.guess this
change updates it to the recent version that properly
detects ARM architecture.


Diffs
-

  3rdparty/glog-0.3.3.patch f1ed584cf12181307552c3c9429d31759be5de65 


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


Testing
---


Thanks,

Tomasz Janiszewski



Re: Review Request 63294: Update glog patch (config.guess) to support ARM.

2017-10-25 Thread Mesos Reviewbot Windows

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



Bad review!

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

- Mesos Reviewbot Windows


On Oct. 25, 2017, 12:27 p.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63294/
> ---
> 
> (Updated Oct. 25, 2017, 12:27 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `config.guess` in glog 0.3.5 do not recognize ARM processors
> properly. We need to update this script in order to build
> glog on ARM. The patch could be removed after upgrading to 0.3.6.
> 
> 
> Diffs
> -
> 
>   3rdparty/glog-0.3.5.patch PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/63294/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>



Re: Review Request 63293: Update google-glog to 0.3.5.

2017-10-25 Thread Tomasz Janiszewski

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

(Updated Oct. 25, 2017, 12:28 p.m.)


Review request for mesos.


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


Repository: mesos


Description
---

Version 0.3.5 adds support for PowerPC so no patching is required.

Fixes: MESOS-3322
Refs: https://github.com/google/glog/releases/tag/v0.3.5


Diffs
-

  3rdparty/glog-0.3.5.patch ab15bea04d7b8af31a638b7c4ce3256ed2f0df4e 


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


Testing
---


Thanks,

Tomasz Janiszewski



Review Request 63293: Update google-glog to 0.3.5.

2017-10-25 Thread Tomasz Janiszewski

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

Review request for mesos.


Repository: mesos


Description
---

Version 0.3.5 adds support for PowerPC so no patching is required.

Fixes: MESOS-3322
Refs: https://github.com/google/glog/releases/tag/v0.3.5


Diffs
-

  3rdparty/glog-0.3.5.patch ab15bea04d7b8af31a638b7c4ce3256ed2f0df4e 


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


Testing
---


Thanks,

Tomasz Janiszewski



Review Request 63294: Update glog patch (config.guess) to support ARM.

2017-10-25 Thread Tomasz Janiszewski

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

Review request for mesos.


Repository: mesos


Description
---

`config.guess` in glog 0.3.5 do not recognize ARM processors
properly. We need to update this script in order to build
glog on ARM. The patch could be removed after upgrading to 0.3.6.


Diffs
-

  3rdparty/glog-0.3.5.patch PRE-CREATION 


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


Testing
---


Thanks,

Tomasz Janiszewski



Re: Review Request 63292: Added lamdba::zip.

2017-10-25 Thread Mesos Reviewbot

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



Patch looks great!

Reviews applied: [63292]

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 Oct. 25, 2017, 10:03 a.m., Benjamin Hindman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63292/
> ---
> 
> (Updated Oct. 25, 2017, 10:03 a.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added lamdba::zip.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/lambda.hpp 
> a61d97b69e7eebd057c94148d39c6e1fc3066017 
>   3rdparty/stout/tests/lambda_tests.cpp 
> ad8c2efddb6b64184670d0cfb33188ef843351ab 
> 
> 
> Diff: https://reviews.apache.org/r/63292/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>



Re: Review Request 63292: Added lamdba::zip.

2017-10-25 Thread Mesos Reviewbot Windows

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



PASS: Mesos patch 63292 was successfully built and tested.

Reviews applied: `['63292']`

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

- Mesos Reviewbot Windows


On Oct. 25, 2017, 10:03 a.m., Benjamin Hindman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63292/
> ---
> 
> (Updated Oct. 25, 2017, 10:03 a.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added lamdba::zip.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/lambda.hpp 
> a61d97b69e7eebd057c94148d39c6e1fc3066017 
>   3rdparty/stout/tests/lambda_tests.cpp 
> ad8c2efddb6b64184670d0cfb33188ef843351ab 
> 
> 
> Diff: https://reviews.apache.org/r/63292/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>



Review Request 63292: Added lamdba::zip.

2017-10-25 Thread Benjamin Hindman

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

Review request for mesos, Alexander Rojas and Vinod Kone.


Repository: mesos


Description
---

Added lamdba::zip.


Diffs
-

  3rdparty/stout/include/stout/lambda.hpp 
a61d97b69e7eebd057c94148d39c6e1fc3066017 
  3rdparty/stout/tests/lambda_tests.cpp 
ad8c2efddb6b64184670d0cfb33188ef843351ab 


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


Testing
---

make check


Thanks,

Benjamin Hindman