Re: Review Request 67457: Windows: Ported docker_containerizer_tests.cpp.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [67384, 67385, 67386, 67387, 67388, 67389, 67685, 67390, 
67391, 67392, 67393, 67456, 67457]

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 June 5, 2018, 8:40 p.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67457/
> ---
> 
> (Updated June 5, 2018, 8:40 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer and Joseph Wu.
> 
> 
> Bugs: MESOS-7342
> https://issues.apache.org/jira/browse/MESOS-7342
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> With some Docker bug fixes and the IOCP backend, the remaining docker
> have been ported. The only remaining Docker tests that aren't ported
> are either due to limitations on Windows Containers or unimplemented
> features in Mesos (Persistent volume and hooks).
> 
> 
> Diffs
> -
> 
>   src/tests/CMakeLists.txt b9c906d7e91e8e2ce3ec76f972169f9b592a6132 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 194308bf9d09a3562e683c49e0da4c9a6463d66e 
> 
> 
> Diff: https://reviews.apache.org/r/67457/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 67457: Windows: Ported docker_containerizer_tests.cpp.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 67457 was successfully built and tested.

Reviews applied: `['67384', '67385', '67386', '67387', '67388', '67389', 
'67685', '67390', '67391', '67392', '67393', '67456', '67457']`

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

- Mesos Reviewbot Windows


On June 5, 2018, 1:40 p.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67457/
> ---
> 
> (Updated June 5, 2018, 1:40 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer and Joseph Wu.
> 
> 
> Bugs: MESOS-7342
> https://issues.apache.org/jira/browse/MESOS-7342
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> With some Docker bug fixes and the IOCP backend, the remaining docker
> have been ported. The only remaining Docker tests that aren't ported
> are either due to limitations on Windows Containers or unimplemented
> features in Mesos (Persistent volume and hooks).
> 
> 
> Diffs
> -
> 
>   src/tests/CMakeLists.txt b9c906d7e91e8e2ce3ec76f972169f9b592a6132 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 194308bf9d09a3562e683c49e0da4c9a6463d66e 
> 
> 
> Diff: https://reviews.apache.org/r/67457/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 67709: Added two tests to the stout cmake build.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [67707, 67708, 67709]

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 June 22, 2018, 2:48 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67709/
> ---
> 
> (Updated June 22, 2018, 2:48 p.m.)
> 
> 
> Review request for mesos and Andrew Schwartzmeyer.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added two tests to the stout cmake build.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/CMakeLists.txt 
> 7c644b7248de86d9f255f7d6914fb01cb1907e62 
> 
> 
> Diff: https://reviews.apache.org/r/67709/diff/1/
> 
> 
> Testing
> ---
> 
> `make check` (cmake)
> `./support/mesos-tidy.sh`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 67709: Added two tests to the stout cmake build.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 67709 was successfully built and tested.

Reviews applied: `['67707', '67708', '67709']`

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

- Mesos Reviewbot Windows


On June 22, 2018, 9:48 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67709/
> ---
> 
> (Updated June 22, 2018, 9:48 p.m.)
> 
> 
> Review request for mesos and Andrew Schwartzmeyer.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added two tests to the stout cmake build.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/CMakeLists.txt 
> 7c644b7248de86d9f255f7d6914fb01cb1907e62 
> 
> 
> Diff: https://reviews.apache.org/r/67709/diff/1/
> 
> 
> Testing
> ---
> 
> `make check` (cmake)
> `./support/mesos-tidy.sh`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 67685: Windows: Added `windows_to_unix_epoch` function.

[Akash Gupta]

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

(Updated June 23, 2018, 12:22 a.m.)


Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, 
John Kordich, Joseph Wu, and Radhika Jandhyala.


Changes
---

moved file.


Bugs: MESOS-8668 and MESOS-8671
https://issues.apache.org/jira/browse/MESOS-8668
https://issues.apache.org/jira/browse/MESOS-8671


Repository: mesos


Description
---

There are a couple of places where we need to convert a Windows
absolute time (epoch at 01/01/1601) to a UNIX absolute time (epoch at
01/01/1970). So, a function has been added to handle it.


Diffs (updated)
-

  3rdparty/stout/include/stout/os/windows/stat.hpp 
7838bacabc4e2b9c92a5283f95543ee74b2d6505 
  3rdparty/stout/include/stout/windows/os.hpp 
46cd667e88853fb0945ec5b14bd57319666895fc 


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

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


Testing
---


Thanks,

Akash Gupta

Re: Review Request 67526: Added container-specific cgroup FS mounts.

[Gilbert Song]

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




src/launcher/executor.cpp
Lines 1281 (patched)


Just bring this up: 

why don't we just call it `container_launch_info`, or `task_launch_info`?

I may prefer `task_launch_info` more, wdyt?



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


Could you remind me again how do we handle centos 6?



src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp
Lines 574 (patched)


const?


- Gilbert Song


On June 19, 2018, 7:48 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67526/
> ---
> 
> (Updated June 19, 2018, 7:48 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jason Lai.
> 
> 
> Bugs: MESOS-8327
> https://issues.apache.org/jira/browse/MESOS-8327
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added container-specific cgroup FS mounts.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp 541ca5b9c4bfb33a6cf341b13007ee8e881a7d89 
>   src/linux/fs.cpp 6b38b4a87984f8a62c64b74eb91c96b847b59643 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
> 81c934318dcc2bcc9df594af0ee25f0334541a65 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 6d146729123b85e46f580a594fb9f9ac37b542f7 
> 
> 
> Diff: https://reviews.apache.org/r/67526/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 67673: Removed an unused member variable `hierarchies` from cgroups isolator.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On June 20, 2018, 7:33 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67673/
> ---
> 
> (Updated June 20, 2018, 7:33 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jason Lai.
> 
> 
> Bugs: MESOS-8327
> https://issues.apache.org/jira/browse/MESOS-8327
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed an unused member variable `hierarchies` from cgroups isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
> 81c934318dcc2bcc9df594af0ee25f0334541a65 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 6d146729123b85e46f580a594fb9f9ac37b542f7 
> 
> 
> Diff: https://reviews.apache.org/r/67673/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 67662: Allowed mounts if the container is launched in a new mount namespace.

[Gilbert Song]

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




src/slave/containerizer/mesos/launch.cpp
Lines 676-680 (original)


I just created https://issues.apache.org/jira/browse/MESOS-9023

Could we add a TODO which mention that we want to add this check back once 
MESOS-9023 is resolved?

The reason we need this check is mount propagation, see `MountPropagation` 
protobuf message in mesos.proto. Currently we do allow users to configure 
whether they want the mounts for a container to propagate back to the host 
filesystems. We don't want to allow it for command task.


- Gilbert Song


On June 19, 2018, 7:37 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67662/
> ---
> 
> (Updated June 19, 2018, 7:37 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jason Lai.
> 
> 
> Bugs: MESOS-8327
> https://issues.apache.org/jira/browse/MESOS-8327
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allowed mounts if the container is launched in a new mount namespace.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.cpp 
> cec6558d0ac61bf0fec87d2e101e8f84730a765a 
> 
> 
> Diff: https://reviews.apache.org/r/67662/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 67693: Added filtering for `GET_OPERATIONS` calls.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [67693]

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 June 21, 2018, 3:53 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67693/
> ---
> 
> (Updated June 21, 2018, 3:53 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Jan Schlicht.
> 
> 
> Bugs: MESOS-8473
> https://issues.apache.org/jira/browse/MESOS-8473
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds filtering of operations reported for `GET_OPERATIONS`
> calls. A principal is allowed to view see all operations for which it
> is allowed to see the role of the underlying resources.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 0492b979e4657a489ca3428e6f8022ef20cb05f5 
>   src/slave/http.cpp aa9cdc5f58f73323958a6872e2721c83317f354c 
>   src/tests/api_tests.cpp 4d6b5b3e938faed934e875e23e29c67fd50b9d6f 
> 
> 
> Diff: https://reviews.apache.org/r/67693/diff/3/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> The added tests fails without the corresponding changes to the master and 
> agent code.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 67693: Added filtering for `GET_OPERATIONS` calls.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 67693 was successfully built and tested.

Reviews applied: `['67693']`

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

- Mesos Reviewbot Windows


On June 21, 2018, 3:53 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67693/
> ---
> 
> (Updated June 21, 2018, 3:53 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Jan Schlicht.
> 
> 
> Bugs: MESOS-8473
> https://issues.apache.org/jira/browse/MESOS-8473
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds filtering of operations reported for `GET_OPERATIONS`
> calls. A principal is allowed to view see all operations for which it
> is allowed to see the role of the underlying resources.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 0492b979e4657a489ca3428e6f8022ef20cb05f5 
>   src/slave/http.cpp aa9cdc5f58f73323958a6872e2721c83317f354c 
>   src/tests/api_tests.cpp 4d6b5b3e938faed934e875e23e29c67fd50b9d6f 
> 
> 
> Diff: https://reviews.apache.org/r/67693/diff/3/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> The added tests fails without the corresponding changes to the master and 
> agent code.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Review Request 67709: Added two tests to the stout cmake build.

[Benjamin Bannier]

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

Review request for mesos and Andrew Schwartzmeyer.


Repository: mesos


Description
---

Added two tests to the stout cmake build.


Diffs
-

  3rdparty/stout/tests/CMakeLists.txt 7c644b7248de86d9f255f7d6914fb01cb1907e62 


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


Testing
---

`make check` (cmake)
`./support/mesos-tidy.sh`


Thanks,

Benjamin Bannier

Review Request 67708: Added `profiler_tests.cpp` to the libprocess cmake build.

[Benjamin Bannier]

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

Review request for mesos and Andrew Schwartzmeyer.


Repository: mesos


Description
---

Added `profiler_tests.cpp` to the libprocess cmake build.


Diffs
-

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


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


Testing
---

`make check` (cmake)
`./support/mesos-tidy.sh`


Thanks,

Benjamin Bannier

Re: Review Request 67384: Windows: Made socket `int_fd` castable to `HANDLE` type.

[Joseph Wu]

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


Ship it!




Ship It!

- Joseph Wu


On June 21, 2018, 4:34 a.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67384/
> ---
> 
> (Updated June 21, 2018, 4:34 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, 
> John Kordich, Joseph Wu, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-8668
> https://issues.apache.org/jira/browse/MESOS-8668
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Many Win32 functions that take in `HANDLE` also work on `SOCKET`, such
> as `ReadFile` or `CreateIoCompletionPort`. IOCP on Windows uses the
> `HANDLE` type uniformly, so we need to be able to convert socket
> `int_fd` to `HANDLE`.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/fd.hpp 
> 89a037af31ad68a275d2519afbe4f161b23efe91 
> 
> 
> Diff: https://reviews.apache.org/r/67384/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>