Re: Review Request 65721: Windows: Specified byproducts of `sasl2` imported target.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [65719, 65720, 65721]

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 Feb. 20, 2018, 7:34 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65721/
> ---
> 
> (Updated Feb. 20, 2018, 7:34 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Benjamin Bannier, Jeff Coffler, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-8599
> https://issues.apache.org/jira/browse/MESOS-8599
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is necessary to enable the Ninja build. This was not previously
> fixed because we have not yet used Ninja on Windows. The same pattern
> is used for other imported targets.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt da605707b89bbe9b3db9e60bc0b0a26dac46e56e 
> 
> 
> Diff: https://reviews.apache.org/r/65721/diff/1/
> 
> 
> Testing
> ---
> 
> Built with Ninja on Windows in 17 minutes, versus 21 minutes using the VS 
> generator (both builds completely from scratch).
> 
> Note that this isn't perfect; Ninja generates some warnings because the 
> imported libraries have both debug and release configurations pointing to the 
> same location. We can ignore this for now, as the build works, but it's ugly.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 65127: Windows: Enabled docker health checks.

[Akash Gupta]

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

(Updated Feb. 21, 2018, 2:08 a.m.)


Review request for mesos, Alexander Rukletsov and Andrew Schwartzmeyer.


Changes
---

rebased.


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


Repository: mesos


Description
---

The network health checks called curl and then executed setns to
enter to container network namespace, which did not work on Windows.
To do the equivalent, Windows nows calls docker run with powershell's
curl equivalent (Invoke-WebRequest) and uses the
network=container: flag to enter the container's namespace. The
command health check was trivially fixed by replacing the hardcoded
`sh -c`.


Diffs (updated)
-

  src/checks/checker_process.hpp 510f3b2e6e689faaf26595214ce377c2b5518f28 
  src/checks/checker_process.cpp ddb197b8cc2c503fef5ae20af32f5881fff9833f 


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

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


Testing
---


Thanks,

Akash Gupta

Re: Review Request 65419: Fixed quoting issues in docker executor command checks.

[Akash Gupta]

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

(Updated Feb. 21, 2018, 2:07 a.m.)


Review request for mesos, Alexander Rukletsov, Andrew Schwartzmeyer, Gaston 
Kleiman, and Joseph Wu.


Changes
---

Small fix to ensure arguments are clear before adding them.


Bugs: MESOS-4812 and MESOS-8498
https://issues.apache.org/jira/browse/MESOS-4812
https://issues.apache.org/jira/browse/MESOS-8498


Repository: mesos


Description
---

The docker executor was wrapping the docker exec command around with
`sh -c ""`, which was causing quoting issues, especially on Windows.
Now, the comamnd health check simply runs `docker exec` without any
wrapping.


Diffs (updated)
-

  src/checks/checker_process.cpp ddb197b8cc2c503fef5ae20af32f5881fff9833f 


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

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


Testing
---

make check


Thanks,

Akash Gupta

Re: Review Request 65574: Windows: Fixed handle inheritance in `create_process` wrapper.

[Andrew Schwartzmeyer]

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

(Updated Feb. 20, 2018, 5:19 p.m.)


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


Changes
---

Added more commentary.


Bugs: MESOS-6838 and MESOS-8512
https://issues.apache.org/jira/browse/MESOS-6838
https://issues.apache.org/jira/browse/MESOS-8512


Repository: mesos


Description
---

The primary change in this patch is that `create_process` now enables
inheritance for the `pipe` handles passed before starting the child
process. This is required, otherwise the child process will behave
incorrectly (for example, it will write to `stdout` but that will go
nowhere, as the redirection silently failed). After the process is
created, inheritance is disabled to prevent further calls to
`create_process` from inheriting the wrong handles.

The `std::tuple` type was changed to a
`std::array` as it is significantly easier to work
with (it supports iteration).


Diffs (updated)
-

  3rdparty/stout/include/stout/os/windows/shell.hpp 
542039c31f94eda1af121335b12edf9b83725ae5 


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

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


Testing
---


Thanks,

Andrew Schwartzmeyer

Re: Review Request 65718: Allowed empty resource provider selector in `UriDiskProfileAdaptor`.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [65718]

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 Feb. 20, 2018, 6:43 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65718/
> ---
> 
> (Updated Feb. 20, 2018, 6:43 p.m.)
> 
> 
> Review request for mesos, James DeFelice, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-8598
> https://issues.apache.org/jira/browse/MESOS-8598
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allowed empty resource provider selector in `UriDiskProfileAdaptor`.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/storage/disk_profile_utils.cpp 
> a243bd6f89919c69c2fe0a94c60f8251e35e23a0 
> 
> 
> Diff: https://reviews.apache.org/r/65718/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 65469: Windows: Updated `internal::process:createChildProcess`.

[Akash Gupta]

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


Ship it!




Ship It!

- Akash Gupta


On Feb. 20, 2018, 7:40 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65469/
> ---
> 
> (Updated Feb. 20, 2018, 7:40 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Jeff Coffler, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-8512
> https://issues.apache.org/jira/browse/MESOS-8512
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The interface of `internal::windows:create_process` was changed to
> accept a `std::array` instead of a `std::tuple`.
> 
> Furthermore, the error handling in `subprocess` was incorrect. We only
> need to check the success of `createChildProcess`; we do not need to
> check if `pid == -1` on Windows. We also now return the actual error
> from `create_process` if it errored, instead of the whole
> `Try`.
> 
> The TODO about closing the child-ends of the file descriptors was
> resolved. We now close these in `createChildProcess`, immediately after
> `create_process`. This means we only have to do it once.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/subprocess.cpp 
> 785e2e1083d115d25fffde2df74ed8bc1726511c 
>   3rdparty/libprocess/src/subprocess_windows.hpp 
> 0183bb451f68528acf31ed97754320c64f35102b 
> 
> 
> Diff: https://reviews.apache.org/r/65469/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 65394: Added separate structs for health check runtime and check types.

[Joseph Wu]

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


Ship it!




Ship It!

- Joseph Wu


On Feb. 16, 2018, 3:25 p.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65394/
> ---
> 
> (Updated Feb. 16, 2018, 3:25 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Andrew Schwartzmeyer, and 
> Gaston Kleiman.
> 
> 
> Bugs: MESOS-8498
> https://issues.apache.org/jira/browse/MESOS-8498
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch is in preparation of another patch that will refactor the
> health check code using these types.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am a2ba1e6de0868c7a4fe58304ec479dce039bc288 
>   src/checks/checks_runtime.hpp PRE-CREATION 
>   src/checks/checks_types.hpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/65394/diff/5/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 65127: Windows: Enabled docker health checks.

[Joseph Wu]

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


Ship it!




Ship It!

- Joseph Wu


On Feb. 12, 2018, 3:16 p.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65127/
> ---
> 
> (Updated Feb. 12, 2018, 3:16 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Andrew Schwartzmeyer.
> 
> 
> Bugs: MESOS-8498
> https://issues.apache.org/jira/browse/MESOS-8498
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The network health checks called curl and then executed setns to
> enter to container network namespace, which did not work on Windows.
> To do the equivalent, Windows nows calls docker run with powershell's
> curl equivalent (Invoke-WebRequest) and uses the
> network=container: flag to enter the container's namespace. The
> command health check was trivially fixed by replacing the hardcoded
> `sh -c`.
> 
> 
> Diffs
> -
> 
>   src/checks/checker_process.hpp 510f3b2e6e689faaf26595214ce377c2b5518f28 
>   src/checks/checker_process.cpp ddb197b8cc2c503fef5ae20af32f5881fff9833f 
> 
> 
> Diff: https://reviews.apache.org/r/65127/diff/9/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 65574: Windows: Fixed handle inheritance in `create_process` wrapper.

[Andrew Schwartzmeyer]

> On Feb. 20, 2018, 4:01 p.m., Akash Gupta wrote:
> > 3rdparty/stout/include/stout/os/windows/shell.hpp
> > Lines 275 (patched)
> > 
> >
> > It seems like you can have a race condition here if you send the same 
> > handles to two different processes at the same time. If the second process 
> > finishes executing this and then the first process exits and sets the 
> > handles to uninheritable, then you would set uninheritable handles to the 
> > child process. If this function is supposed to have this behavior, maybe 
> > add a comment explaining that this function mutates the handles, so the 
> > caller needs to make sure to synchronize these calls.

> if you send the same handles to two different processes at the same time

This doesn't seem to be a valid scenario to me. When might we want to spawn 
more than one process that are each supposed to inherit the same set of handles?

If we needed two processes to inherit the same handles, it would make sense to 
duplicate those handles. But yeah we can comment it.


> On Feb. 20, 2018, 4:01 p.m., Akash Gupta wrote:
> > 3rdparty/stout/include/stout/os/windows/shell.hpp
> > Line 294 (original), 301 (patched)
> > 
> >
> > Hm, I remember we discussed something like `CreateProcessW` does 
> > process initialization async. Do we need to wait for the process to be 
> > initialized?

I don't believe we need to wait, as inheriting the handles does not seem to be 
part of initialization. It's hard to be sure, as the docs don't state much 
about it, but what they do say is this:

> Note that the function returns before the process has finished 
> initialization. If a required DLL cannot be located or fails to initialize, 
> the process is terminated.

I think the docs are trying to say that `CreateProcess` returns before the 
other process has "finished" starting. But I don't think this would have any 
bearing on handles. It would seem to me that the system call to `CreateProcess` 
processes the handle inheritance, making any handle that was inheritable at the 
point the call was made, inherited in the child process. I do not think the 
child process has to "initialize" its inheritance of those handles.

In any case, it's not particularly clear. I'd say we should make sure to add a 
comment that there may be an issue here, and then come back to it if we it 
actually manifests.


- Andrew


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


On Feb. 20, 2018, 11:39 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65574/
> ---
> 
> (Updated Feb. 20, 2018, 11:39 a.m.)
> 
> 
> Review request for mesos, Akash Gupta, Jeff Coffler, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-6838 and MESOS-8512
> https://issues.apache.org/jira/browse/MESOS-6838
> https://issues.apache.org/jira/browse/MESOS-8512
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The primary change in this patch is that `create_process` now enables
> inheritance for the `pipe` handles passed before starting the child
> process. This is required, otherwise the child process will behave
> incorrectly (for example, it will write to `stdout` but that will go
> nowhere, as the redirection silently failed). After the process is
> created, inheritance is disabled to prevent further calls to
> `create_process` from inheriting the wrong handles.
> 
> The `std::tuple` type was changed to a
> `std::array` as it is significantly easier to work
> with (it supports iteration).
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/shell.hpp 
> 542039c31f94eda1af121335b12edf9b83725ae5 
> 
> 
> Diff: https://reviews.apache.org/r/65574/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 65574: Windows: Fixed handle inheritance in `create_process` wrapper.

[Akash Gupta]

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




3rdparty/stout/include/stout/os/windows/shell.hpp
Lines 275 (patched)


It seems like you can have a race condition here if you send the same 
handles to two different processes at the same time. If the second process 
finishes executing this and then the first process exits and sets the handles 
to uninheritable, then you would set uninheritable handles to the child 
process. If this function is supposed to have this behavior, maybe add a 
comment explaining that this function mutates the handles, so the caller needs 
to make sure to synchronize these calls.



3rdparty/stout/include/stout/os/windows/shell.hpp
Line 294 (original), 301 (patched)


Hm, I remember we discussed something like `CreateProcessW` does process 
initialization async. Do we need to wait for the process to be initialized?


- Akash Gupta


On Feb. 20, 2018, 7:39 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65574/
> ---
> 
> (Updated Feb. 20, 2018, 7:39 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Jeff Coffler, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-6838 and MESOS-8512
> https://issues.apache.org/jira/browse/MESOS-6838
> https://issues.apache.org/jira/browse/MESOS-8512
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The primary change in this patch is that `create_process` now enables
> inheritance for the `pipe` handles passed before starting the child
> process. This is required, otherwise the child process will behave
> incorrectly (for example, it will write to `stdout` but that will go
> nowhere, as the redirection silently failed). After the process is
> created, inheritance is disabled to prevent further calls to
> `create_process` from inheriting the wrong handles.
> 
> The `std::tuple` type was changed to a
> `std::array` as it is significantly easier to work
> with (it supports iteration).
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/shell.hpp 
> 542039c31f94eda1af121335b12edf9b83725ae5 
> 
> 
> Diff: https://reviews.apache.org/r/65574/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 65624: Added fetcher tests for `stderr` output.

[Andrew Schwartzmeyer]

> On Feb. 20, 2018, 11:43 a.m., Jeff Coffler wrote:
> > src/tests/fetcher_tests.cpp
> > Lines 132 (patched)
> > 
> >
> > These tests pretty much check the fetcher in a "stand-alone" 
> > environment. Could you add some tests to check within a container as well, 
> > particularly since fetcher logging is slightly different within a container?
> > 
> > Note that while fetcher output is slightly different within a 
> > container, fetcher itself isn't (docker is sharing the same directory). But 
> > we don't have any sort of tests to validate that - that I'm aware of anyway.
> 
> Andrew Schwartzmeyer wrote:
> This covers the affected scenarios; nothing changes running "in a 
> container" on Windows compared to this unit test with respect to output 
> redirection. When task is started with a URI to fetch, the fetcher is 
> directly invoked with the sandbox directory as done here to fetch the URI(s), 
> before the container starts. In essence, the fetcher is always run 
> "stand-alone".
> 
> Jeff Coffler wrote:
> That's correct for fetching. However, logging is somewhat different since 
> stderr/stdout logging is somewhat different under Docker vs. on a stand-alone 
> system. That's what I was referring to ...

This fetcher is never used under Docker. What scenario are you imagining?


- Andrew


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


On Feb. 20, 2018, 11:40 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65624/
> ---
> 
> (Updated Feb. 20, 2018, 11:40 a.m.)
> 
> 
> Review request for mesos, Akash Gupta, Jeff Coffler, Jie Yu, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The fetcher is supposed to log its `stderr` output to a redirected file
> called `stderr` in the given sandbox directory. There previously existed
> a bug on Windows due to incorrect handle inheritance where this
> redirection failed silently, leaving the log empty. These unit tests
> assert that the correct content is logged to the `stderr` file for both
> a success and failure fetch scenario.
> 
> 
> Diffs
> -
> 
>   src/tests/fetcher_tests.cpp 1b4ba1d286f218731c13180e5189e91f90e3d63f 
> 
> 
> Diff: https://reviews.apache.org/r/65624/diff/1/
> 
> 
> Testing
> ---
> 
> Windows (Linux pending):
> ```
> [ RUN  ] FetcherTest.LogSuccessToStderr
> [   OK ] FetcherTest.LogSuccessToStderr (197 ms)
> [ RUN  ] FetcherTest.LogFailureToStderr
> [   OK ] FetcherTest.LogFailureToStderr (320 ms)
> ```
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 65403: Windows: Disabled `O_CLOEXEC` semantic mapping on Windows.

[Akash Gupta]

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


Ship it!




Ship It!

- Akash Gupta


On Feb. 20, 2018, 7:39 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65403/
> ---
> 
> (Updated Feb. 20, 2018, 7:39 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Jeff Coffler, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-5882
> https://issues.apache.org/jira/browse/MESOS-5882
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously we mapped `O_CLOEXEC` to `O_NOINHERIT`, which is close to the
> intended semantics, but not close enough. Instead, we want all Windows
> handles by default to be non-inheritable, and not tie any Windows
> inheritance semantics to the close-on-exec semantics. So, we define
> `O_CLOEXEC` on Windows to be `0`, and remove the logging from the
> `os::cloexec` family of functions because do not ever intend to
> implement them.
> 
> We achieve the "close" part of the semantics by making all handles
> non-inheritable, as the intent is to avoid leaking handles.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/open.hpp 
> 6711f22d44b0e7c13da3abd51f9fb7b71779e868 
>   3rdparty/stout/include/stout/os/windows/fcntl.hpp 
> 5800ec92f85401a80cb813afd880be2e5a24a3af 
> 
> 
> Diff: https://reviews.apache.org/r/65403/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 65467: Windows: Added `internal::windows::set_inherit(WindowsFD, bool)`.

[Akash Gupta]

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


Ship it!




Ship It!

- Akash Gupta


On Feb. 20, 2018, 7:39 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65467/
> ---
> 
> (Updated Feb. 20, 2018, 7:39 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Jeff Coffler, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-8512
> https://issues.apache.org/jira/browse/MESOS-8512
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This function is used on Windows to explicitly enable or disable
> inheritance on a handle.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/internal/windows/inherit.hpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/65467/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 65419: Fixed quoting issues in docker executor command checks.

[Joseph Wu]

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


Ship it!




Ship It!

- Joseph Wu


On Feb. 16, 2018, 3:29 p.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65419/
> ---
> 
> (Updated Feb. 16, 2018, 3:29 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Andrew Schwartzmeyer, Gaston 
> Kleiman, and Joseph Wu.
> 
> 
> Bugs: MESOS-4812 and MESOS-8498
> https://issues.apache.org/jira/browse/MESOS-4812
> https://issues.apache.org/jira/browse/MESOS-8498
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The docker executor was wrapping the docker exec command around with
> `sh -c ""`, which was causing quoting issues, especially on Windows.
> Now, the comamnd health check simply runs `docker exec` without any
> wrapping.
> 
> 
> Diffs
> -
> 
>   src/checks/checker_process.cpp ddb197b8cc2c503fef5ae20af32f5881fff9833f 
> 
> 
> Diff: https://reviews.apache.org/r/65419/diff/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 65396: Moved docker command check code inside health check library.

[Joseph Wu]

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


Ship it!




Ship It!

- Joseph Wu


On Feb. 8, 2018, 9:50 a.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65396/
> ---
> 
> (Updated Feb. 8, 2018, 9:50 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Andrew Schwartzmeyer, and 
> Gaston Kleiman.
> 
> 
> Bugs: MESOS-8498
> https://issues.apache.org/jira/browse/MESOS-8498
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Now with the health check library refactor, the wrapping of `docker
> exec` for docker command health checks can be easily moved inside
> the library.
> 
> 
> Diffs
> -
> 
>   src/checks/checker_process.cpp ddb197b8cc2c503fef5ae20af32f5881fff9833f 
>   src/docker/executor.cpp 80e2d81169f0d4303ca1ddbcef9fa87fe52601fc 
> 
> 
> Diff: https://reviews.apache.org/r/65396/diff/5/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 65395: Refactored health checks to cleanly separate each different check.

[Joseph Wu]

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


Ship it!




Ship It!

- Joseph Wu


On Feb. 8, 2018, 9:49 a.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65395/
> ---
> 
> (Updated Feb. 8, 2018, 9:49 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Andrew Schwartzmeyer, and 
> Gaston Kleiman.
> 
> 
> Bugs: MESOS-8498
> https://issues.apache.org/jira/browse/MESOS-8498
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Refactored health check code to separate the logic for each check
> type and runtime (Plain/Docker/Nested). Now the matrix of different
> possiblilites is cleanly enumerated, making it easier to add
> future health checks, such as the ones for Windows.
> 
> 
> Diffs
> -
> 
>   src/checks/checker.hpp 93502270f31e80c5f7c94b5b456625e9cdea1837 
>   src/checks/checker.cpp fff0aac504b4283a210f936e00c977fa60d88b3d 
>   src/checks/checker_process.hpp 510f3b2e6e689faaf26595214ce377c2b5518f28 
>   src/checks/checker_process.cpp ddb197b8cc2c503fef5ae20af32f5881fff9833f 
>   src/checks/health_checker.hpp 019fbd791f250ecc28ff59d779f90e7ccbf0c685 
>   src/checks/health_checker.cpp eaf9a18817eeeff7c29c7a4b9d1b183f398760a3 
>   src/docker/executor.cpp 80e2d81169f0d4303ca1ddbcef9fa87fe52601fc 
>   src/launcher/default_executor.cpp 8720dada8bc6ca66f9e0fec6dc265eda3dcc7407 
>   src/launcher/executor.cpp e4b9785fed8cd106edcc178071687e82f54016e3 
> 
> 
> Diff: https://reviews.apache.org/r/65395/diff/5/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 65581: Made all allocator tests allow for slave backoff.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [65717, 65581]

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 Feb. 8, 2018, 6:28 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65581/
> ---
> 
> (Updated Feb. 8, 2018, 6:28 p.m.)
> 
> 
> Review request for mesos and Benjamin Bannier.
> 
> 
> Bugs: MESOS-8463
> https://issues.apache.org/jira/browse/MESOS-8463
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Most of the allocator tests were expecting a single invocation of
> 'AddSave' on the allocator when registering a slave. Due to the nature
> of our slave registration backoff, a single slave possibly sends
> mutiple registration requests in short succession. Any event closely
> connected to a slave registration will therefor be possibly invoked
> multiple times over the lifetime of such test.
> We now allow multiple invocations of 'AddSlave'. This patch also
> enhances the timing control by introducing a paused clock over the
> slave registration phase of these tests.
> 
> 
> Diffs
> -
> 
>   src/tests/master_allocator_tests.cpp 
> e4a63836ba8cae7b9cf2fce9d46a844858749182 
> 
> 
> Diff: https://reviews.apache.org/r/65581/diff/4/
> 
> 
> Testing
> ---
> 
> make check && ./bin/mesos-tests.sh --gtest_filter="MasterAllocatorTest*" 
> --gtest_repeat=1000 --gtest_break_on_failure
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 65696: Allowed base64-decoding with whitespaces.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65696 was successfully built and tested.

Reviews applied: `['65696']`

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

- Mesos Reviewbot Windows


On Feb. 17, 2018, 12:27 a.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65696/
> ---
> 
> (Updated Feb. 17, 2018, 12:27 a.m.)
> 
> 
> Review request for mesos, Michael Park and Till Toenshoff.
> 
> 
> Bugs: MESOS-8569
> https://issues.apache.org/jira/browse/MESOS-8569
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Most base64 decoders do not fail on encountering whitespace characters
> as several encoders embed newlines to enforce line-width in encoded
> data.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/base64.hpp 
> eabc9b0f0ad794ffccda44e338ea855b8c4a796f 
>   3rdparty/stout/tests/base64_tests.cpp 
> a6837c820384e3311d2c030885039a7dd7f01a66 
> 
> 
> Diff: https://reviews.apache.org/r/65696/diff/4/
> 
> 
> Testing
> ---
> 
> Updated base64 tests and ran make check
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 65696: Allowed base64-decoding with whitespaces.

[Till Toenshoff]

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


Ship it!





3rdparty/stout/include/stout/base64.hpp
Line 92 (original), 92 (patched)


Can we add a leading comment on which exact variants of the most recent RFC 
we now support with all of our options here? Maybe reference the whitespace 
handling advices from the RFC to make sure people can validate - either comment 
or commit description.


- Till Toenshoff


On Feb. 17, 2018, 12:27 a.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65696/
> ---
> 
> (Updated Feb. 17, 2018, 12:27 a.m.)
> 
> 
> Review request for mesos, Michael Park and Till Toenshoff.
> 
> 
> Bugs: MESOS-8569
> https://issues.apache.org/jira/browse/MESOS-8569
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Most base64 decoders do not fail on encountering whitespace characters
> as several encoders embed newlines to enforce line-width in encoded
> data.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/base64.hpp 
> eabc9b0f0ad794ffccda44e338ea855b8c4a796f 
>   3rdparty/stout/tests/base64_tests.cpp 
> a6837c820384e3311d2c030885039a7dd7f01a66 
> 
> 
> Diff: https://reviews.apache.org/r/65696/diff/3/
> 
> 
> Testing
> ---
> 
> Updated base64 tests and ran make check
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 65717: Updated Cluster::Slave::shutdown to support a paused Clock.

[Benjamin Bannier]

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


Ship it!




Note to everybody that it is not required anymore to replace an automatically 
inferred and redundant review request description with manual dummy content 
(e.g., _See summary._). With MESOS-7979 we updated `support/apply-reviews.py` 
to automatically remove the redundant descriptions.

- Benjamin Bannier


On Feb. 20, 2018, 6:31 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65717/
> ---
> 
> (Updated Feb. 20, 2018, 6:31 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> see summary.
> 
> 
> Diffs
> -
> 
>   src/tests/cluster.cpp 19a41c7c1c303ad806daa4e5e3765a1e0b55933b 
> 
> 
> Diff: https://reviews.apache.org/r/65717/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 65624: Added fetcher tests for `stderr` output.

[Jeff Coffler]

> On Feb. 20, 2018, 7:43 p.m., Jeff Coffler wrote:
> > src/tests/fetcher_tests.cpp
> > Lines 132 (patched)
> > 
> >
> > These tests pretty much check the fetcher in a "stand-alone" 
> > environment. Could you add some tests to check within a container as well, 
> > particularly since fetcher logging is slightly different within a container?
> > 
> > Note that while fetcher output is slightly different within a 
> > container, fetcher itself isn't (docker is sharing the same directory). But 
> > we don't have any sort of tests to validate that - that I'm aware of anyway.
> 
> Andrew Schwartzmeyer wrote:
> This covers the affected scenarios; nothing changes running "in a 
> container" on Windows compared to this unit test with respect to output 
> redirection. When task is started with a URI to fetch, the fetcher is 
> directly invoked with the sandbox directory as done here to fetch the URI(s), 
> before the container starts. In essence, the fetcher is always run 
> "stand-alone".

That's correct for fetching. However, logging is somewhat different since 
stderr/stdout logging is somewhat different under Docker vs. on a stand-alone 
system. That's what I was referring to ...


- Jeff


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


On Feb. 20, 2018, 7:40 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65624/
> ---
> 
> (Updated Feb. 20, 2018, 7:40 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Jeff Coffler, Jie Yu, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The fetcher is supposed to log its `stderr` output to a redirected file
> called `stderr` in the given sandbox directory. There previously existed
> a bug on Windows due to incorrect handle inheritance where this
> redirection failed silently, leaving the log empty. These unit tests
> assert that the correct content is logged to the `stderr` file for both
> a success and failure fetch scenario.
> 
> 
> Diffs
> -
> 
>   src/tests/fetcher_tests.cpp 1b4ba1d286f218731c13180e5189e91f90e3d63f 
> 
> 
> Diff: https://reviews.apache.org/r/65624/diff/1/
> 
> 
> Testing
> ---
> 
> Windows (Linux pending):
> ```
> [ RUN  ] FetcherTest.LogSuccessToStderr
> [   OK ] FetcherTest.LogSuccessToStderr (197 ms)
> [ RUN  ] FetcherTest.LogFailureToStderr
> [   OK ] FetcherTest.LogFailureToStderr (320 ms)
> ```
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 65721: Windows: Specified byproducts of `sasl2` imported target.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65721 was successfully built and tested.

Reviews applied: `['65719', '65720', '65721']`

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

- Mesos Reviewbot Windows


On Feb. 20, 2018, 7:34 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65721/
> ---
> 
> (Updated Feb. 20, 2018, 7:34 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Benjamin Bannier, Jeff Coffler, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-8599
> https://issues.apache.org/jira/browse/MESOS-8599
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is necessary to enable the Ninja build. This was not previously
> fixed because we have not yet used Ninja on Windows. The same pattern
> is used for other imported targets.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt da605707b89bbe9b3db9e60bc0b0a26dac46e56e 
> 
> 
> Diff: https://reviews.apache.org/r/65721/diff/1/
> 
> 
> Testing
> ---
> 
> Built with Ninja on Windows in 17 minutes, versus 21 minutes using the VS 
> generator (both builds completely from scratch).
> 
> Note that this isn't perfect; Ninja generates some warnings because the 
> imported libraries have both debug and release configurations pointing to the 
> same location. We can ignore this for now, as the build works, but it's ugly.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 65346: Updated Web UI to show quota guarantee and limit for roles.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [65503, 65346]

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 Feb. 4, 2018, 1:18 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65346/
> ---
> 
> (Updated Feb. 4, 2018, 1:18 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Kapil Arya, Michael Park, Meng 
> Zhu, and Till Toenshoff.
> 
> 
> Bugs: MESOS-8486
> https://issues.apache.org/jira/browse/MESOS-8486
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/css/mesos.css 
> 3e3a3b13cfc4d63aea73d0a6572b22a2de94541e 
>   src/webui/master/static/index.html 46cb2843c2221ccebb7d811c0045be2c124afec1 
>   src/webui/master/static/js/app.js 8960f9d4421f7b96ba06deb623afafb06013b622 
>   src/webui/master/static/roles.html 1c84ad33a554e078d854e25b4d5ca311c8507f91 
> 
> 
> Diff: https://reviews.apache.org/r/65346/diff/2/
> 
> 
> Testing
> ---
> 
> New UI:
> ![New UI](https://i.imgur.com/rcSUP8N.png)
> The group columns cannot be ordered as they have sub-columns, this is 
> expected.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 65624: Added fetcher tests for `stderr` output.

[Andrew Schwartzmeyer]

> On Feb. 20, 2018, 11:43 a.m., Jeff Coffler wrote:
> > src/tests/fetcher_tests.cpp
> > Lines 132 (patched)
> > 
> >
> > These tests pretty much check the fetcher in a "stand-alone" 
> > environment. Could you add some tests to check within a container as well, 
> > particularly since fetcher logging is slightly different within a container?
> > 
> > Note that while fetcher output is slightly different within a 
> > container, fetcher itself isn't (docker is sharing the same directory). But 
> > we don't have any sort of tests to validate that - that I'm aware of anyway.

This covers the affected scenarios; nothing changes running "in a container" on 
Windows compared to this unit test with respect to output redirection. When 
task is started with a URI to fetch, the fetcher is directly invoked with the 
sandbox directory as done here to fetch the URI(s), before the container 
starts. In essence, the fetcher is always run "stand-alone".


- Andrew


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


On Feb. 20, 2018, 11:40 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65624/
> ---
> 
> (Updated Feb. 20, 2018, 11:40 a.m.)
> 
> 
> Review request for mesos, Akash Gupta, Jeff Coffler, Jie Yu, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The fetcher is supposed to log its `stderr` output to a redirected file
> called `stderr` in the given sandbox directory. There previously existed
> a bug on Windows due to incorrect handle inheritance where this
> redirection failed silently, leaving the log empty. These unit tests
> assert that the correct content is logged to the `stderr` file for both
> a success and failure fetch scenario.
> 
> 
> Diffs
> -
> 
>   src/tests/fetcher_tests.cpp 1b4ba1d286f218731c13180e5189e91f90e3d63f 
> 
> 
> Diff: https://reviews.apache.org/r/65624/diff/1/
> 
> 
> Testing
> ---
> 
> Windows (Linux pending):
> ```
> [ RUN  ] FetcherTest.LogSuccessToStderr
> [   OK ] FetcherTest.LogSuccessToStderr (197 ms)
> [ RUN  ] FetcherTest.LogFailureToStderr
> [   OK ] FetcherTest.LogFailureToStderr (320 ms)
> ```
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 65624: Added fetcher tests for `stderr` output.

[Jeff Coffler]

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




src/tests/fetcher_tests.cpp
Lines 132 (patched)


These tests pretty much check the fetcher in a "stand-alone" environment. 
Could you add some tests to check within a container as well, particularly 
since fetcher logging is slightly different within a container?

Note that while fetcher output is slightly different within a container, 
fetcher itself isn't (docker is sharing the same directory). But we don't have 
any sort of tests to validate that - that I'm aware of anyway.


- Jeff Coffler


On Feb. 20, 2018, 7:40 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65624/
> ---
> 
> (Updated Feb. 20, 2018, 7:40 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Jeff Coffler, Jie Yu, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The fetcher is supposed to log its `stderr` output to a redirected file
> called `stderr` in the given sandbox directory. There previously existed
> a bug on Windows due to incorrect handle inheritance where this
> redirection failed silently, leaving the log empty. These unit tests
> assert that the correct content is logged to the `stderr` file for both
> a success and failure fetch scenario.
> 
> 
> Diffs
> -
> 
>   src/tests/fetcher_tests.cpp 1b4ba1d286f218731c13180e5189e91f90e3d63f 
> 
> 
> Diff: https://reviews.apache.org/r/65624/diff/1/
> 
> 
> Testing
> ---
> 
> Windows (Linux pending):
> ```
> [ RUN  ] FetcherTest.LogSuccessToStderr
> [   OK ] FetcherTest.LogSuccessToStderr (197 ms)
> [ RUN  ] FetcherTest.LogFailureToStderr
> [   OK ] FetcherTest.LogFailureToStderr (320 ms)
> ```
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 65469: Windows: Updated `internal::process:createChildProcess`.

[Jeff Coffler]

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


Ship it!




Ship It!

- Jeff Coffler


On Feb. 20, 2018, 7:40 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65469/
> ---
> 
> (Updated Feb. 20, 2018, 7:40 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Jeff Coffler, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-8512
> https://issues.apache.org/jira/browse/MESOS-8512
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The interface of `internal::windows:create_process` was changed to
> accept a `std::array` instead of a `std::tuple`.
> 
> Furthermore, the error handling in `subprocess` was incorrect. We only
> need to check the success of `createChildProcess`; we do not need to
> check if `pid == -1` on Windows. We also now return the actual error
> from `create_process` if it errored, instead of the whole
> `Try`.
> 
> The TODO about closing the child-ends of the file descriptors was
> resolved. We now close these in `createChildProcess`, immediately after
> `create_process`. This means we only have to do it once.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/subprocess.cpp 
> 785e2e1083d115d25fffde2df74ed8bc1726511c 
>   3rdparty/libprocess/src/subprocess_windows.hpp 
> 0183bb451f68528acf31ed97754320c64f35102b 
> 
> 
> Diff: https://reviews.apache.org/r/65469/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 65718: Allowed empty resource provider selector in `UriDiskProfileAdaptor`.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65718 was successfully built and tested.

Reviews applied: `['65718']`

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

- Mesos Reviewbot Windows


On Feb. 20, 2018, 6:43 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65718/
> ---
> 
> (Updated Feb. 20, 2018, 6:43 p.m.)
> 
> 
> Review request for mesos, James DeFelice, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-8598
> https://issues.apache.org/jira/browse/MESOS-8598
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allowed empty resource provider selector in `UriDiskProfileAdaptor`.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/storage/disk_profile_utils.cpp 
> a243bd6f89919c69c2fe0a94c60f8251e35e23a0 
> 
> 
> Diff: https://reviews.apache.org/r/65718/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 65467: Windows: Added `internal::windows::set_inherit(WindowsFD, bool)`.

[Jeff Coffler]

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


Ship it!




Ship It!

- Jeff Coffler


On Feb. 8, 2018, 10:35 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65467/
> ---
> 
> (Updated Feb. 8, 2018, 10:35 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-8512
> https://issues.apache.org/jira/browse/MESOS-8512
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This function is used on Windows to explicitly enable or disable
> inheritance on a handle.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/internal/windows/inherit.hpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/65467/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 65403: Windows: Disabled `O_CLOEXEC` semantic mapping on Windows.

[Jeff Coffler]

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


Ship it!




Ship It!

- Jeff Coffler


On Feb. 8, 2018, 10:36 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65403/
> ---
> 
> (Updated Feb. 8, 2018, 10:36 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-5882
> https://issues.apache.org/jira/browse/MESOS-5882
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously we mapped `O_CLOEXEC` to `O_NOINHERIT`, which is close to the
> intended semantics, but not close enough. Instead, we want all Windows
> handles by default to be non-inheritable, and not tie any Windows
> inheritance semantics to the close-on-exec semantics. So, we define
> `O_CLOEXEC` on Windows to be `0`, and remove the logging from the
> `os::cloexec` family of functions because do not ever intend to
> implement them.
> 
> We achieve the "close" part of the semantics by making all handles
> non-inheritable, as the intent is to avoid leaking handles.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/open.hpp 
> 6711f22d44b0e7c13da3abd51f9fb7b71779e868 
>   3rdparty/stout/include/stout/os/windows/fcntl.hpp 
> 5800ec92f85401a80cb813afd880be2e5a24a3af 
> 
> 
> Diff: https://reviews.apache.org/r/65403/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Review Request 65721: Windows: Specified byproducts of `sasl2` imported target.

[Andrew Schwartzmeyer]

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

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


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


Repository: mesos


Description
---

This is necessary to enable the Ninja build. This was not previously
fixed because we have not yet used Ninja on Windows. The same pattern
is used for other imported targets.


Diffs
-

  3rdparty/CMakeLists.txt da605707b89bbe9b3db9e60bc0b0a26dac46e56e 


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


Testing
---

Built with Ninja on Windows in 17 minutes, versus 21 minutes using the VS 
generator (both builds completely from scratch).

Note that this isn't perfect; Ninja generates some warnings because the 
imported libraries have both debug and release configurations pointing to the 
same location. We can ignore this for now, as the build works, but it's ugly.


Thanks,

Andrew Schwartzmeyer

Review Request 65719: Windows: Fixed CMake check of toolset for Ninja.

[Andrew Schwartzmeyer]

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

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


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


Repository: mesos


Description
---

Only the Visual Studio generator(s) should check the toolset. The
Ninja generator requires the environment to be setup in advance, and
does not have the concept of a toolset, so this warning broke the
Ninja build.


Diffs
-

  cmake/CompilationConfigure.cmake 50cddf9476c8c5196c4824a7b060c2680a96b277 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 65720: Windows: Fixed location of imported libraries for Ninja.

[Andrew Schwartzmeyer]

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

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


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


Repository: mesos


Description
---

The Ninja generator does not emit binaries to "Debug" or "Release"
folders (unlike the Visual Studio generator), so we replace this
assumption with a pair of variables `CONFIG_DEBGUG` and
`CONFIG_RELEASE` which are correct for each generator. This is
somewhat ugly, but it's the cost of supporting multiple toolsets.


Diffs
-

  3rdparty/CMakeLists.txt da605707b89bbe9b3db9e60bc0b0a26dac46e56e 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 65718: Allowed 0 resource provider in DiskProfileMapping's selector.

[Chun-Hung Hsiao]

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

Review request for mesos, James DeFelice, Jie Yu, and Joseph Wu.


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


Repository: mesos


Description
---

Allowed empty resource provider selector in `UriDiskProfileAdaptor`.


Diffs
-

  src/resource_provider/storage/disk_profile_utils.cpp 
a243bd6f89919c69c2fe0a94c60f8251e35e23a0 


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


Testing
---

sudo make check


Thanks,

Chun-Hung Hsiao

Re: Review Request 65581: Made all allocator tests allow for slave backoff.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65581 was successfully built and tested.

Reviews applied: `['65717', '65581']`

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

- Mesos Reviewbot Windows


On Feb. 9, 2018, 2:28 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65581/
> ---
> 
> (Updated Feb. 9, 2018, 2:28 a.m.)
> 
> 
> Review request for mesos and Benjamin Bannier.
> 
> 
> Bugs: MESOS-8463
> https://issues.apache.org/jira/browse/MESOS-8463
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Most of the allocator tests were expecting a single invocation of
> 'AddSave' on the allocator when registering a slave. Due to the nature
> of our slave registration backoff, a single slave possibly sends
> mutiple registration requests in short succession. Any event closely
> connected to a slave registration will therefor be possibly invoked
> multiple times over the lifetime of such test.
> We now allow multiple invocations of 'AddSlave'. This patch also
> enhances the timing control by introducing a paused clock over the
> slave registration phase of these tests.
> 
> 
> Diffs
> -
> 
>   src/tests/master_allocator_tests.cpp 
> e4a63836ba8cae7b9cf2fce9d46a844858749182 
> 
> 
> Diff: https://reviews.apache.org/r/65581/diff/4/
> 
> 
> Testing
> ---
> 
> make check && ./bin/mesos-tests.sh --gtest_filter="MasterAllocatorTest*" 
> --gtest_repeat=1000 --gtest_break_on_failure
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 65713: Handled hanging docker `stop`, `inspect` commands in docker executor.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [65713]

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 Feb. 20, 2018, 5:11 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65713/
> ---
> 
> (Updated Feb. 20, 2018, 5:11 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Gilbert Song, Greg Mann, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-8574
> https://issues.apache.org/jira/browse/MESOS-8574
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previosly, if `docker inspect` command hanged, the docker container
> ended up in an unkillable state. This patch adds a timeout for inspect
> command after receiving `killTask` analogically to `reaped` handler.
> In addition we've added a timeout for `docker stop` command. If docker
> `stop` or `inspect` command times out, we discard the related future,
> thus the docker library kills previously spawned docker cli subprocess.
> As a result, a scheduler can retry `killTask` operation to handle
> nasty docker bugs that lead to hanging docker cli.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp 80e2d81169f0d4303ca1ddbcef9fa87fe52601fc 
> 
> 
> Diff: https://reviews.apache.org/r/65713/diff/1/
> 
> 
> Testing
> ---
> 
> internal CI
> 
> Manual testing:
> 1. Build docker from sources: 
> http://oyvindsk.com/writing/docker-build-from-source
> 2. Modify `ContainerInspect` function from `docker/inspect.go`:
> ```
>  func (daemon *Daemon) ContainerInspect(name string, size bool, version 
> string) (interface{}, error) {
> +   time.Sleep(10 * time.Second)
> ```
> 3. Modify `ContainerStop` function from `docker/stop.go`:
> ```
>  func (daemon *Daemon) ContainerStop(name string, seconds *int) error {
> +   rand.Seed(time.Now().UTC().UnixNano())
> +   if rand.Intn(2) == 0 {
> +   time.Sleep(20 * time.Second)
> +   }
> ```
> 4. Rebuild docker: `sudo make build && sudo make binary`
> 5. Stop system docker daemon: `sudo service docker stop`
> 6. Start modified docker daemon: `sudo ./bundles/binary-daemon/dockerd-dev`
> 7. Modify `src/cli/execute.cpp`:
>   a) Add `delay(Seconds(15), self(), ::retryKill, task->task_id(), 
> offer.agent_id());` after 
> https://github.com/apache/mesos/blob/072ea2787ffca6f2a6dcb2d636f68c51823d6665/src/cli/execute.cpp#L606
>   b) Add a new method `retryKill` to `CommandScheduler`:
> ```
>   void retryKill(const TaskID& taskId, const AgentID& agentId)
>   {
> killTask(taskId, agentId);
> delay(Seconds(6), self(), ::retryKill, taskId, agentId);
>   }
> ```
> 8. Rebuild mesos
> 9. Run mesos master: `./bin/mesos-master.sh --work_dir='var/master-1'`
> 10. Run mesos agent: `GLOG_v=1 ./bin/mesos-agent.sh 
> --resources="cpus:1;mem:100" 
> --work_dir='/home/abudnik/mesos/build/var/agent-1' 
> --containerizers="docker,mesos" --master="127.0.1.1:5050"`
> 11. Submit a task for the docker executor: `./src/mesos-execute 
> --master="127.0.1.1:5050" --name="a" --containerizer=docker 
> --docker_image="ubuntu:xenial" --command="sleep "`
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Review Request 65717: Updated Cluster::Slave::shutdown to support a paused Clock.

[Till Toenshoff]

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

Review request for mesos, Benjamin Bannier and Neil Conway.


Repository: mesos


Description
---

see summary.


Diffs
-

  src/tests/cluster.cpp 19a41c7c1c303ad806daa4e5e3765a1e0b55933b 


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


Testing
---

make check


Thanks,

Till Toenshoff

Re: Review Request 64211: Added options to build the Python CLI and run unit tests.

[Benjamin Bannier]

> On Feb. 20, 2018, 4:25 p.m., Benjamin Bannier wrote:
> > configure.ac
> > Lines 2458-2469 (patched)
> > 
> >
> > Let's kill this section and reword below error messages.
> > 
> > Having a good error message is good, but we also should try to minimize 
> > the amount of logic we put into autoconf.
> 
> Kevin Klues wrote:
> There was a reason I originally had this separate, and I don't remember 
> why. Can you take a closer look if something will go wrong if this is simply 
> removed?

I cannot see how the Python version of the bindings and the CLI are related 
(they might be the same, but do not need to be forever). We also immediately 
follow this up with another call to `AM_PATH_PYTHON` which overrides the 
results here. I tested that that this works as expected.


> On Feb. 20, 2018, 4:25 p.m., Benjamin Bannier wrote:
> > configure.ac
> > Lines 2458-2469 (patched)
> > 
> >
> > This seems unrelated (`--enable-python`?) and redundant since we 
> > explicitly check another >=python-2.6 below. Kill this.
> 
> Kevin Klues wrote:
> This again has to do with what I commented on above. I remember having to 
> compensate for cases where enable_python was true/false with making sure 
> python was still available for the CL if `--enable-cli-new` was set.

Like I wrote above, cli and binding dependencies are not necessarily the same. 
If we really want to detect a common Python version we should pull the 
detection out of any conditionals and then only check compatibility where we 
need it. I felt this was overkill for this patch.


- Benjamin


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


On Feb. 20, 2018, 12:09 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64211/
> ---
> 
> (Updated Feb. 20, 2018, 12:09 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Kevin Klues.
> 
> 
> Bugs: MESOS-8240
> https://issues.apache.org/jira/browse/MESOS-8240
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> An update of the discarded review /r/52543.
> 
> Works with Autotools and CMake.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 6702f02245e3867c06bbd9efbbf4e3b961a7d9aa 
>   cmake/CompilationConfigure.cmake 50cddf9476c8c5196c4824a7b060c2680a96b277 
>   configure.ac 30fbadc32d1d96f719d45fa8067f975283c25507 
>   docs/configuration/cmake.md 1e34657ea55fa324f65f865f7d0a67084c6719d9 
>   src/CMakeLists.txt 0c135034982a6a4844509cd0e9b2cc41804333ef 
>   src/Makefile.am a2ba1e6de0868c7a4fe58304ec479dce039bc288 
>   src/python/cli_new/CMakeLists.txt PRE-CREATION 
>   src/python/cli_new/tests/CMakeLists.txt PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/64211/diff/12/
> 
> 
> Testing
> ---
> 
> Testing done on Fedora 25.
> 
> For Autotools:
> ```
> $ ./bootstrap
> $ mkdir build
> $ cd build
> $ ../configure --enable-new-cli --disable-java --disable-python
> $ make check
> ```
> 
> For CMake:
> ```
> $ ./bootstrap
> $ mkdir build
> $ cd build
> $ cmake .. -DENABLE_NEW_CLI=1
> $ cmake --build . -- -j16
> $ ./src/mesos
> Mesos CLI
> 
> Usage:
>   mesos (-h | --help)
>   mesos --version
>   mesos  [...]
> 
> Options:
>   -h --help  Show this screen.
>   --version  Show version info.
> 
> Commands:
>   agent   Interacts with the Mesos agents
>   config  Interacts with the Mesos CLI configuration file
>   taskInteracts with the tasks running in a Mesos cluster
> 
> See 'mesos help ' for more information on a specific command.
> $ cmake --build . --target tests -- -j16
> $ ctest -R CLI
> Test project /home/agrillet/apache-mesos/build
> Start 4: CLITests
> 1/1 Test #4: CLITests .   Passed3.63 sec
> 
> 100% tests passed, 0 tests failed out of 1
> ```
> 
> Checked that the the CLI tests were run, that the content of the directory 
> `build/src/cli` was as expected, and that `build/src/mesos` was correctly 
> running.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 65581: Made all allocator tests allow for slave backoff.

[Benjamin Bannier]

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


Fix it, then Ship it!




Thanks for the cleanup!


src/tests/cluster.cpp
Lines 695-696 (original), 695-706 (patched)


Let's pull these changes out into a patch before this one.


- Benjamin Bannier


On Feb. 9, 2018, 3:28 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65581/
> ---
> 
> (Updated Feb. 9, 2018, 3:28 a.m.)
> 
> 
> Review request for mesos and Benjamin Bannier.
> 
> 
> Bugs: MESOS-8463
> https://issues.apache.org/jira/browse/MESOS-8463
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Most of the allocator tests were expecting a single invocation of
> 'AddSave' on the allocator when registering a slave. Due to the nature
> of our slave registration backoff, a single slave possibly sends
> mutiple registration requests in short succession. Any event closely
> connected to a slave registration will therefor be possibly invoked
> multiple times over the lifetime of such test.
> We now allow multiple invocations of 'AddSlave'. This patch also
> enhances the timing control by introducing a paused clock over the
> slave registration phase of these tests.
> 
> 
> Diffs
> -
> 
>   src/tests/cluster.cpp 19a41c7c1c303ad806daa4e5e3765a1e0b55933b 
>   src/tests/master_allocator_tests.cpp 
> e4a63836ba8cae7b9cf2fce9d46a844858749182 
> 
> 
> Diff: https://reviews.apache.org/r/65581/diff/3/
> 
> 
> Testing
> ---
> 
> make check && ./bin/mesos-tests.sh --gtest_filter="MasterAllocatorTest*" 
> --gtest_repeat=1000 --gtest_break_on_failure
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 65713: Handled hanging docker `stop`, `inspect` commands in docker executor.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65713 was successfully built and tested.

Reviews applied: `['65713']`

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

- Mesos Reviewbot Windows


On Feb. 20, 2018, 3:18 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65713/
> ---
> 
> (Updated Feb. 20, 2018, 3:18 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Gilbert Song, and Greg Mann.
> 
> 
> Bugs: MESOS-8574
> https://issues.apache.org/jira/browse/MESOS-8574
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previosly, if `docker inspect` command hanged, the docker container
> ended up in an unkillable state. This patch adds a timeout for inspect
> command after receiving `killTask` analogically to `reaped` handler.
> In addition we've added a timeout for `docker stop` command. If docker
> `stop` or `inspect` command times out, we discard the related future,
> thus the docker library kills previously spawned docker cli subprocess.
> As a result, a scheduler can retry `killTask` operation to handle
> nasty docker bugs that lead to hanging docker cli.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp 80e2d81169f0d4303ca1ddbcef9fa87fe52601fc 
> 
> 
> Diff: https://reviews.apache.org/r/65713/diff/1/
> 
> 
> Testing
> ---
> 
> internal CI
> 
> Manual testing:
> 1. Build docker from sources: 
> http://oyvindsk.com/writing/docker-build-from-source
> 2. Modify `ContainerInspect` function from `docker/inspect.go`:
> ```
>  func (daemon *Daemon) ContainerInspect(name string, size bool, version 
> string) (interface{}, error) {
> +   time.Sleep(10 * time.Second)
> ```
> 3. Modify `ContainerStop` function from `docker/stop.go`:
> ```
>  func (daemon *Daemon) ContainerStop(name string, seconds *int) error {
> +   rand.Seed(time.Now().UTC().UnixNano())
> +   if rand.Intn(2) == 0 {
> +   time.Sleep(20 * time.Second)
> +   }
> ```
> 4. Rebuild docker: `sudo make build && sudo make binary`
> 5. Stop system docker daemon: `sudo service docker stop`
> 6. Start modified docker daemon: `sudo ./bundles/binary-daemon/dockerd-dev`
> 7. Modify `src/cli/execute.cpp`:
>   a) Add `delay(Seconds(15), self(), ::retryKill, task->task_id(), 
> offer.agent_id());` after 
> https://github.com/apache/mesos/blob/072ea2787ffca6f2a6dcb2d636f68c51823d6665/src/cli/execute.cpp#L606
>   b) Add a new method `retryKill` to `CommandScheduler`:
> ```
>   void retryKill(const TaskID& taskId, const AgentID& agentId)
>   {
> killTask(taskId, agentId);
> delay(Seconds(6), self(), ::retryKill, taskId, agentId);
>   }
> ```
> 8. Rebuild mesos
> 9. Run mesos master: `./bin/mesos-master.sh --work_dir='var/master-1'`
> 10. Run mesos agent: `GLOG_v=1 ./bin/mesos-agent.sh 
> --resources="cpus:1;mem:100" 
> --work_dir='/home/abudnik/mesos/build/var/agent-1' 
> --containerizers="docker,mesos" --master="127.0.1.1:5050"`
> 11. Submit a task for the docker executor: `./src/mesos-execute 
> --master="127.0.1.1:5050" --name="a" --containerizer=docker 
> --docker_image="ubuntu:xenial" --command="sleep "`
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 64211: Added options to build the Python CLI and run unit tests.

[Kevin Klues]

> On Feb. 20, 2018, 3:25 p.m., Benjamin Bannier wrote:
> > configure.ac
> > Lines 2458-2469 (patched)
> > 
> >
> > This seems unrelated (`--enable-python`?) and redundant since we 
> > explicitly check another >=python-2.6 below. Kill this.

This again has to do with what I commented on above. I remember having to 
compensate for cases where enable_python was true/false with making sure python 
was still available for the CL if `--enable-cli-new` was set.


- Kevin


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


On Feb. 20, 2018, 11:09 a.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64211/
> ---
> 
> (Updated Feb. 20, 2018, 11:09 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Kevin Klues.
> 
> 
> Bugs: MESOS-8240
> https://issues.apache.org/jira/browse/MESOS-8240
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> An update of the discarded review /r/52543.
> 
> Works with Autotools and CMake.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 6702f02245e3867c06bbd9efbbf4e3b961a7d9aa 
>   cmake/CompilationConfigure.cmake 50cddf9476c8c5196c4824a7b060c2680a96b277 
>   configure.ac 30fbadc32d1d96f719d45fa8067f975283c25507 
>   docs/configuration/cmake.md 1e34657ea55fa324f65f865f7d0a67084c6719d9 
>   src/CMakeLists.txt 0c135034982a6a4844509cd0e9b2cc41804333ef 
>   src/Makefile.am a2ba1e6de0868c7a4fe58304ec479dce039bc288 
>   src/python/cli_new/CMakeLists.txt PRE-CREATION 
>   src/python/cli_new/tests/CMakeLists.txt PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/64211/diff/12/
> 
> 
> Testing
> ---
> 
> Testing done on Fedora 25.
> 
> For Autotools:
> ```
> $ ./bootstrap
> $ mkdir build
> $ cd build
> $ ../configure --enable-new-cli --disable-java --disable-python
> $ make check
> ```
> 
> For CMake:
> ```
> $ ./bootstrap
> $ mkdir build
> $ cd build
> $ cmake .. -DENABLE_NEW_CLI=1
> $ cmake --build . -- -j16
> $ ./src/mesos
> Mesos CLI
> 
> Usage:
>   mesos (-h | --help)
>   mesos --version
>   mesos  [...]
> 
> Options:
>   -h --help  Show this screen.
>   --version  Show version info.
> 
> Commands:
>   agent   Interacts with the Mesos agents
>   config  Interacts with the Mesos CLI configuration file
>   taskInteracts with the tasks running in a Mesos cluster
> 
> See 'mesos help ' for more information on a specific command.
> $ cmake --build . --target tests -- -j16
> $ ctest -R CLI
> Test project /home/agrillet/apache-mesos/build
> Start 4: CLITests
> 1/1 Test #4: CLITests .   Passed3.63 sec
> 
> 100% tests passed, 0 tests failed out of 1
> ```
> 
> Checked that the the CLI tests were run, that the content of the directory 
> `build/src/cli` was as expected, and that `build/src/mesos` was correctly 
> running.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 64211: Added options to build the Python CLI and run unit tests.

[Kevin Klues]

> On Feb. 20, 2018, 3:25 p.m., Benjamin Bannier wrote:
> > configure.ac
> > Lines 2458-2469 (patched)
> > 
> >
> > Let's kill this section and reword below error messages.
> > 
> > Having a good error message is good, but we also should try to minimize 
> > the amount of logic we put into autoconf.

There was a reason I originally had this separate, and I don't remember why. 
Can you take a closer look if something will go wrong if this is simply removed?


- Kevin


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


On Feb. 20, 2018, 11:09 a.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64211/
> ---
> 
> (Updated Feb. 20, 2018, 11:09 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Kevin Klues.
> 
> 
> Bugs: MESOS-8240
> https://issues.apache.org/jira/browse/MESOS-8240
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> An update of the discarded review /r/52543.
> 
> Works with Autotools and CMake.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 6702f02245e3867c06bbd9efbbf4e3b961a7d9aa 
>   cmake/CompilationConfigure.cmake 50cddf9476c8c5196c4824a7b060c2680a96b277 
>   configure.ac 30fbadc32d1d96f719d45fa8067f975283c25507 
>   docs/configuration/cmake.md 1e34657ea55fa324f65f865f7d0a67084c6719d9 
>   src/CMakeLists.txt 0c135034982a6a4844509cd0e9b2cc41804333ef 
>   src/Makefile.am a2ba1e6de0868c7a4fe58304ec479dce039bc288 
>   src/python/cli_new/CMakeLists.txt PRE-CREATION 
>   src/python/cli_new/tests/CMakeLists.txt PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/64211/diff/12/
> 
> 
> Testing
> ---
> 
> Testing done on Fedora 25.
> 
> For Autotools:
> ```
> $ ./bootstrap
> $ mkdir build
> $ cd build
> $ ../configure --enable-new-cli --disable-java --disable-python
> $ make check
> ```
> 
> For CMake:
> ```
> $ ./bootstrap
> $ mkdir build
> $ cd build
> $ cmake .. -DENABLE_NEW_CLI=1
> $ cmake --build . -- -j16
> $ ./src/mesos
> Mesos CLI
> 
> Usage:
>   mesos (-h | --help)
>   mesos --version
>   mesos  [...]
> 
> Options:
>   -h --help  Show this screen.
>   --version  Show version info.
> 
> Commands:
>   agent   Interacts with the Mesos agents
>   config  Interacts with the Mesos CLI configuration file
>   taskInteracts with the tasks running in a Mesos cluster
> 
> See 'mesos help ' for more information on a specific command.
> $ cmake --build . --target tests -- -j16
> $ ctest -R CLI
> Test project /home/agrillet/apache-mesos/build
> Start 4: CLITests
> 1/1 Test #4: CLITests .   Passed3.63 sec
> 
> 100% tests passed, 0 tests failed out of 1
> ```
> 
> Checked that the the CLI tests were run, that the content of the directory 
> `build/src/cli` was as expected, and that `build/src/mesos` was correctly 
> running.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 64211: Added options to build the Python CLI and run unit tests.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [65585, 65705, 64211]

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 Feb. 20, 2018, 11:09 a.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64211/
> ---
> 
> (Updated Feb. 20, 2018, 11:09 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Kevin Klues.
> 
> 
> Bugs: MESOS-8240
> https://issues.apache.org/jira/browse/MESOS-8240
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> An update of the discarded review /r/52543.
> 
> Works with Autotools and CMake.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 6702f02245e3867c06bbd9efbbf4e3b961a7d9aa 
>   cmake/CompilationConfigure.cmake 50cddf9476c8c5196c4824a7b060c2680a96b277 
>   configure.ac 30fbadc32d1d96f719d45fa8067f975283c25507 
>   docs/configuration/cmake.md 1e34657ea55fa324f65f865f7d0a67084c6719d9 
>   src/CMakeLists.txt 0c135034982a6a4844509cd0e9b2cc41804333ef 
>   src/Makefile.am a2ba1e6de0868c7a4fe58304ec479dce039bc288 
>   src/python/cli_new/CMakeLists.txt PRE-CREATION 
>   src/python/cli_new/tests/CMakeLists.txt PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/64211/diff/12/
> 
> 
> Testing
> ---
> 
> Testing done on Fedora 25.
> 
> For Autotools:
> ```
> $ ./bootstrap
> $ mkdir build
> $ cd build
> $ ../configure --enable-new-cli --disable-java --disable-python
> $ make check
> ```
> 
> For CMake:
> ```
> $ ./bootstrap
> $ mkdir build
> $ cd build
> $ cmake .. -DENABLE_NEW_CLI=1
> $ cmake --build . -- -j16
> $ ./src/mesos
> Mesos CLI
> 
> Usage:
>   mesos (-h | --help)
>   mesos --version
>   mesos  [...]
> 
> Options:
>   -h --help  Show this screen.
>   --version  Show version info.
> 
> Commands:
>   agent   Interacts with the Mesos agents
>   config  Interacts with the Mesos CLI configuration file
>   taskInteracts with the tasks running in a Mesos cluster
> 
> See 'mesos help ' for more information on a specific command.
> $ cmake --build . --target tests -- -j16
> $ ctest -R CLI
> Test project /home/agrillet/apache-mesos/build
> Start 4: CLITests
> 1/1 Test #4: CLITests .   Passed3.63 sec
> 
> 100% tests passed, 0 tests failed out of 1
> ```
> 
> Checked that the the CLI tests were run, that the content of the directory 
> `build/src/cli` was as expected, and that `build/src/mesos` was correctly 
> running.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 64211: Added options to build the Python CLI and run unit tests.

[Benjamin Bannier]

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


Fix it, then Ship it!




Looking great.

I left some comments on the autoconf setup, sorry for the earlier partial 
review. In its final form it could look like this,

AS_IF([test "x$enable_new_cli" = "xyes"], [
  AM_PATH_PYTHON([2.6])

  AX_COMPARE_VERSION([$PYTHON_VERSION], [gt], [2.7],
AC_MSG_ERROR([Python version too new
---
The new CLI requires Python version 2.6 or 2.7 in order to build.
Your Python version is $PYTHON_VERSION.

You may wish to set the PYTHON environment variable to an
appropriate value to assure the right Python executable is found.
---
  ]))

  AC_CHECK_PROG([VIRTUALENV_CHECK], [virtualenv], [ok])
  AS_IF([test "x$VIRTUALENV_CHECK" = "xok"],, [
 AC_MSG_ERROR([Cannot find virtualenv
---
The new CLI requires 'virtualenv' be installed as part of your
Python $PYTHON_VERSION installation.

You may wish to install it via 'pip install virtualenv'.
---
  ])])
])

AM_CONDITIONAL([ENABLE_NEW_CLI], [test "$enable_new_cli" = "xyes"])


configure.ac
Lines 240 (patched)


`s/new-cli/new_cli/` (you can check the output of e.g., `../configure 
--help` to see the difference).



configure.ac
Line 2442 (original), 2447 (patched)


Let's keep this conditional together with the conditionals related to e.g., 
`setuptools` and `pip` below. Suggest to move cli-related checks above this one.



configure.ac
Lines 2449-2453 (patched)


This is not really useful (e.g., displays `yes` even if we fail in 
cli-related checks below). Suggest to get rid of this completely.



configure.ac
Lines 2454 (patched)


We should initialize after the checks so we can avoid the initialization 
with a dummy value here.



configure.ac
Lines 2458-2469 (patched)


Let's kill this section and reword below error messages.

Having a good error message is good, but we also should try to minimize the 
amount of logic we put into autoconf.



configure.ac
Lines 2458-2469 (patched)


This seems unrelated (`--enable-python`?) and redundant since we explicitly 
check another >=python-2.6 below. Kill this.



configure.ac
Lines 2472-2480 (patched)


We can just let the default error message from autoconf propagate; we 
`s/2.6/4.6/` I get

checking for a Python interpreter with version >= 4.6... none
configure: error: no suitable Python interpreter found



configure.ac
Lines 2493 (patched)


Let's quote these variables and add spaces around `,`,

AC_CHECK_PROG([VIRTUALENV_CHECK], [virtualenv], [ok])



configure.ac
Lines 2495 (patched)


Start error message with a captial letter.


- Benjamin Bannier


On Feb. 20, 2018, 12:09 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64211/
> ---
> 
> (Updated Feb. 20, 2018, 12:09 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Kevin Klues.
> 
> 
> Bugs: MESOS-8240
> https://issues.apache.org/jira/browse/MESOS-8240
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> An update of the discarded review /r/52543.
> 
> Works with Autotools and CMake.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 6702f02245e3867c06bbd9efbbf4e3b961a7d9aa 
>   cmake/CompilationConfigure.cmake 50cddf9476c8c5196c4824a7b060c2680a96b277 
>   configure.ac 30fbadc32d1d96f719d45fa8067f975283c25507 
>   docs/configuration/cmake.md 1e34657ea55fa324f65f865f7d0a67084c6719d9 
>   src/CMakeLists.txt 0c135034982a6a4844509cd0e9b2cc41804333ef 
>   src/Makefile.am a2ba1e6de0868c7a4fe58304ec479dce039bc288 
>   src/python/cli_new/CMakeLists.txt PRE-CREATION 
>   src/python/cli_new/tests/CMakeLists.txt PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/64211/diff/12/
> 
> 
> Testing
> ---
> 
> Testing done on Fedora 25.
> 
> For Autotools:
> ```
> $ ./bootstrap
> $ mkdir build
> $ cd build
> $ ../configure 

Review Request 65713: Handled hanging docker `stop`, `inspect` commands in docker executor.

[Andrei Budnik]

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

Review request for mesos, Alexander Rukletsov, Gilbert Song, and Greg Mann.


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


Repository: mesos


Description
---

Previosly, if `docker inspect` command hanged, the docker container
ended up in an unkillable state. This patch adds a timeout for inspect
command after receiving `killTask` analogically to `reaped` handler.
In addition we've added a timeout for `docker stop` command. If docker
`stop` or `inspect` command times out, we discard the related future,
thus the docker library kills previously spawned docker cli subprocess.
As a result, a scheduler can retry `killTask` operation to handle
nasty docker bugs that lead to hanging docker cli.


Diffs
-

  src/docker/executor.cpp 80e2d81169f0d4303ca1ddbcef9fa87fe52601fc 


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


Testing
---


Thanks,

Andrei Budnik

Re: Review Request 65581: Made all allocator tests allow for slave backoff.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [65581]

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 Feb. 9, 2018, 2:28 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65581/
> ---
> 
> (Updated Feb. 9, 2018, 2:28 a.m.)
> 
> 
> Review request for mesos and Benjamin Bannier.
> 
> 
> Bugs: MESOS-8463
> https://issues.apache.org/jira/browse/MESOS-8463
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Most of the allocator tests were expecting a single invocation of
> 'AddSave' on the allocator when registering a slave. Due to the nature
> of our slave registration backoff, a single slave possibly sends
> mutiple registration requests in short succession. Any event closely
> connected to a slave registration will therefor be possibly invoked
> multiple times over the lifetime of such test.
> We now allow multiple invocations of 'AddSlave'. This patch also
> enhances the timing control by introducing a paused clock over the
> slave registration phase of these tests.
> 
> 
> Diffs
> -
> 
>   src/tests/cluster.cpp 19a41c7c1c303ad806daa4e5e3765a1e0b55933b 
>   src/tests/master_allocator_tests.cpp 
> e4a63836ba8cae7b9cf2fce9d46a844858749182 
> 
> 
> Diff: https://reviews.apache.org/r/65581/diff/3/
> 
> 
> Testing
> ---
> 
> make check && ./bin/mesos-tests.sh --gtest_filter="MasterAllocatorTest*" 
> --gtest_repeat=1000 --gtest_break_on_failure
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 65581: Made all allocator tests allow for slave backoff.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65581 was successfully built and tested.

Reviews applied: `['65581']`

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

- Mesos Reviewbot Windows


On Feb. 9, 2018, 2:28 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65581/
> ---
> 
> (Updated Feb. 9, 2018, 2:28 a.m.)
> 
> 
> Review request for mesos and Benjamin Bannier.
> 
> 
> Bugs: MESOS-8463
> https://issues.apache.org/jira/browse/MESOS-8463
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Most of the allocator tests were expecting a single invocation of
> 'AddSave' on the allocator when registering a slave. Due to the nature
> of our slave registration backoff, a single slave possibly sends
> mutiple registration requests in short succession. Any event closely
> connected to a slave registration will therefor be possibly invoked
> multiple times over the lifetime of such test.
> We now allow multiple invocations of 'AddSlave'. This patch also
> enhances the timing control by introducing a paused clock over the
> slave registration phase of these tests.
> 
> 
> Diffs
> -
> 
>   src/tests/cluster.cpp 19a41c7c1c303ad806daa4e5e3765a1e0b55933b 
>   src/tests/master_allocator_tests.cpp 
> e4a63836ba8cae7b9cf2fce9d46a844858749182 
> 
> 
> Diff: https://reviews.apache.org/r/65581/diff/3/
> 
> 
> Testing
> ---
> 
> make check && ./bin/mesos-tests.sh --gtest_filter="MasterAllocatorTest*" 
> --gtest_repeat=1000 --gtest_break_on_failure
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 64211: Added options to build the Python CLI and run unit tests.

[Mesos Reviewbot Windows]

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



FAIL: Some of the unit tests failed. Please check the relevant logs.

Reviews applied: `['65585', '65705', '64211']`

Failed command: `Start-MesosCITesting`

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

Relevant logs:

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

```
[ RUN  ] SSLVerifyIPAdd/SSLTest.RequireCertificate/0
D:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\ssl_tests.cpp(446): error: 
(await_subprocess(client.get(), 0)).failure(): 
[++] Subprocess output.
[==] Running 1 test from 1 test case.

[--] Global test environment set-up.

[--] 1 test from SSLClientTest

[ RUN  ] SSLClientTest.client

[   OK ] SSLClientTest.client (20 ms)

[--] 1 test from SSLClientTest (20 ms total)



[--] Global test environment tear-down

[==] 1 test from 1 test case ran. (20 ms total)

[  PASSED  ] 1 test.

[++]

[  FAILED  ] SSLVerifyIPAdd/SSLTest.RequireCertificate/0, where GetParam() = 
"false" (1020 ms)
[ RUN  ] SSLVerifyIPAdd/SSLTest.RequireCertificate/1
[   OK ] SSLVerifyIPAdd/SSLTest.RequireCertificate/1 (759 ms)
[--] 4 tests from SSLVerifyIPAdd/SSLTest (2733 ms total)

[--] Global test environment tear-down
[==] 225 tests from 35 test cases ran. (36218 ms total)
[  PASSED  ] 224 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] SSLVerifyIPAdd/SSLTest.RequireCertificate/0, where GetParam() = 
"false"

 1 FAILED TEST
  YOU HAVE 19 DISABLED TESTS

```

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

```
I0220 11:22:33.139695  7412 process.cpp:929] Stopped the socket accept loop
I0220 11:22:33.178696  2512 process.cpp:929] Stopped the socket accept loop
I0220 11:22:33.711710  7412 openssl.cpp:514] Will not verify peer certificate!
NOTE: Set LIBPROCESS_SSL_VERIFY_CERT=1 to enable peer certificate verification
I0220 11:22:33.711710  7412 openssl.cpp:534] LIBPROCESS_SSL_REQUIRE_CERT 
implies peer certificate verification.
LIBPROCESS_SSL_VERIFY_CERT set to true
I0220 11:22:33.712712  7412 openssl.cpp:563] Using CA file: 
C:\Users\mesos\AppData\Local\Temp\kMSiBI\cert.pem
I0220 11:22:33.712712  7412 openssl.cpp:566] Using CA dir: 
C:\Users\mesos\AppData\Local\Temp\kMSiBI
I0220 11:22:34.129721  7412 openssl.cpp:514] Will not verify peer certificate!
NOTE: Set LIBPROCESS_SSL_VERIFY_CERT=1 to enable peer certificate verification
I0220 11:22:34.129721  7412 openssl.cpp:526] Will use IP address verification 
in subject alternative name certificate extension.
I0220 11:22:34.129721  7412 openssl.cpp:534] LIBPROCESS_SSL_REQUIRE_CERT 
implies peer certificate verification.
LIBPROCESS_SSL_VERIFY_CERT set to true
I0220 11:22:34.130722  7412 openssl.cpp:563] Using CA file: 
C:\Users\mesos\AppData\Local\Temp\D03t0K\cert.pem
I0220 11:22:34.130722  7412 openssl.cpp:566] Using CA dir: 
C:\Users\mesos\AppData\Local\Temp\D03t0K
I0220 11:22:34.329726  7412 openssl.cpp:509] CA directory path unspecified! 
NOTE: Set CA directory path with LIBPROCESS_SSL_CA_DIR=
I0220 11:22:34.329726  7412 openssl.cpp:514] Will not verify peer certificate!
NOTE: Set LIBPROCESS_SSL_VERIFY_CERT=1 to enable peer certificate verification
I0220 11:22:34.329726  7412 openssl.cpp:534] LIBPROCESS_SSL_REQUIRE_CERT 
implies peer certificate verification.
LIBPROCESS_SSL_VERIFY_CERT set to true
I0220 11:22:34.329726  7412 openssl.cpp:563] Using CA file: 
C:\Users\mesos\AppData\Local\Temp\QKT6bT\cert.pem
I0220 11:22:35.306813  7412 openssl.cpp:509] CA directory path unspecified! 
NOTE: Set CA directory path with LIBPROCESS_SSL_CA_DIR=
I0220 11:22:35.306813  7412 openssl.cpp:514] Will not verify peer certificate!
NOTE: Set LIBPROCESS_SSL_VERIFY_CERT=1 to enable peer certificate verification
I0220 11:22:35.306813  7412 openssl.cpp:526] Will use IP address verification 
in subject alternative name certificate extension.
I0220 11:22:35.306813  7412 openssl.cpp:534] LIBPROCESS_SSL_REQUIRE_CERT 
implies peer certificate verification.
LIBPROCESS_SSL_VERIFY_CERT set to true
I0220 11:22:35.306813  7412 openssl.cpp:563] Using CA file: 
C:\Users\mesos\AppData\Local\Temp\c1fjUJ\cert.pem
I0220 11:22:35.944043  8808 process.cpp:929] Stopped the socket accept loop
```

- Mesos Reviewbot Windows


On Feb. 20, 2018, 11:09 a.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64211/
> ---
> 
> (Updated Feb. 20, 2018, 11:09 

Re: Review Request 64211: Added options to build the Python CLI and run unit tests.

[Armand Grillet]

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

(Updated Feb. 20, 2018, 11:09 a.m.)


Review request for mesos, Benjamin Bannier and Kevin Klues.


Changes
---

Fixed issues.


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


Repository: mesos


Description
---

An update of the discarded review /r/52543.

Works with Autotools and CMake.


Diffs (updated)
-

  CMakeLists.txt 6702f02245e3867c06bbd9efbbf4e3b961a7d9aa 
  cmake/CompilationConfigure.cmake 50cddf9476c8c5196c4824a7b060c2680a96b277 
  configure.ac 30fbadc32d1d96f719d45fa8067f975283c25507 
  docs/configuration/cmake.md 1e34657ea55fa324f65f865f7d0a67084c6719d9 
  src/CMakeLists.txt 0c135034982a6a4844509cd0e9b2cc41804333ef 
  src/Makefile.am a2ba1e6de0868c7a4fe58304ec479dce039bc288 
  src/python/cli_new/CMakeLists.txt PRE-CREATION 
  src/python/cli_new/tests/CMakeLists.txt PRE-CREATION 


Diff: https://reviews.apache.org/r/64211/diff/12/

Changes: https://reviews.apache.org/r/64211/diff/11-12/


Testing
---

Testing done on Fedora 25.

For Autotools:
```
$ ./bootstrap
$ mkdir build
$ cd build
$ ../configure --enable-new-cli --disable-java --disable-python
$ make check
```

For CMake:
```
$ ./bootstrap
$ mkdir build
$ cd build
$ cmake .. -DENABLE_NEW_CLI=1
$ cmake --build . -- -j16
$ ./src/mesos
Mesos CLI

Usage:
  mesos (-h | --help)
  mesos --version
  mesos  [...]

Options:
  -h --help  Show this screen.
  --version  Show version info.

Commands:
  agent   Interacts with the Mesos agents
  config  Interacts with the Mesos CLI configuration file
  taskInteracts with the tasks running in a Mesos cluster

See 'mesos help ' for more information on a specific command.
$ cmake --build . --target tests -- -j16
$ ctest -R CLI
Test project /home/agrillet/apache-mesos/build
Start 4: CLITests
1/1 Test #4: CLITests .   Passed3.63 sec

100% tests passed, 0 tests failed out of 1
```

Checked that the the CLI tests were run, that the content of the directory 
`build/src/cli` was as expected, and that `build/src/mesos` was correctly 
running.


Thanks,

Armand Grillet

Re: Review Request 64608: Reverted unintended edit to cpplint.py.

[Jan Schlicht]

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


Ship it!




Ship It!

- Jan Schlicht


On Feb. 20, 2018, 10:08 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64608/
> ---
> 
> (Updated Feb. 20, 2018, 10:08 a.m.)
> 
> 
> Review request for mesos, Gaojin CAO, Michael Park, and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In 'bea8c3c9' we replaced '> >' with '>>' in C++ templates in a sweep
> across the codebase. We also unintentionally modified a comment in
> 'cpplint.py' which was e.g., not reflected in its patch file and also
> lead to no functional changes.
> 
> This patch reverts this modification to 'cpplint.py'.
> 
> 
> Diffs
> -
> 
>   support/cpplint.py d8f93cf51bdc0a547f12cc3aee60313a384317dd 
> 
> 
> Diff: https://reviews.apache.org/r/64608/diff/1/
> 
> 
> Testing
> ---
> 
> `./support/mesos-style.py 3rdparty/stout/tests/none_tests.cpp`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 64608: Reverted unintended edit to cpplint.py.

[Benjamin Bannier]

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

(Updated Feb. 20, 2018, 10:08 a.m.)


Review request for mesos, Gaojin CAO, Michael Park, and Jan Schlicht.


Repository: mesos


Description
---

In 'bea8c3c9' we replaced '> >' with '>>' in C++ templates in a sweep
across the codebase. We also unintentionally modified a comment in
'cpplint.py' which was e.g., not reflected in its patch file and also
lead to no functional changes.

This patch reverts this modification to 'cpplint.py'.


Diffs
-

  support/cpplint.py d8f93cf51bdc0a547f12cc3aee60313a384317dd 


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


Testing
---

`./support/mesos-style.py 3rdparty/stout/tests/none_tests.cpp`


Thanks,

Benjamin Bannier

Re: Review Request 65712: Added a unit test for docker daemon hangs while pulling.

[Mesos Reviewbot Windows]

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



FAIL: Some of the unit tests failed. Please check the relevant logs.

Reviews applied: `['65711', '65689', '65712']`

Failed command: `Start-MesosCITesting`

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

Relevant logs:

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

```
[ RUN  ] ExecutorAuthorizationTest.RunTaskGroup
[   OK ] ExecutorAuthorizationTest.RunTaskGroup (2226 ms)
[ RUN  ] ExecutorAuthorizationTest.FailedSubscribe
[   OK ] ExecutorAuthorizationTest.FailedSubscribe (269 ms)
[ RUN  ] ExecutorAuthorizationTest.FailedApiCalls
[   OK ] ExecutorAuthorizationTest.FailedApiCalls (395 ms)
[--] 3 tests from ExecutorAuthorizationTest (2950 ms total)

[--] 4 tests from SlaveCompatibilityTest
[ RUN  ] SlaveCompatibilityTest.Equal
[   OK ] SlaveCompatibilityTest.Equal (6 ms)
[ RUN  ] SlaveCompatibilityTest.Additive
[   OK ] SlaveCompatibilityTest.Additive (10 ms)
[ RUN  ] SlaveCompatibilityTest.AdditiveWithReservations
[   OK ] SlaveCompatibilityTest.AdditiveWithReservations (4 ms)
[ RUN  ] SlaveCompatibilityTest.Disks
[   OK ] SlaveCompatibilityTest.Disks (4 ms)
[--] 4 tests from SlaveCompatibilityTest (27 ms total)

[--] 75 tests from SlaveTest
[ RUN  ] SlaveTest.Shutdown
[   OK ] SlaveTest.Shutdown (238 ms)
[ RUN  ] SlaveTest.DuplicateTerminalUpdateBeforeAck
[   OK ] SlaveTest.DuplicateTerminalUpdateBeforeAck (291 ms)
[ RUN  ] SlaveTest.ShutdownUnregisteredExecutor
D:\DCOS\mesos\mesos\src\tests\slave_tests.cpp(440): error: Failed to wait 
15secs for status
D:\DCOS\mesos\mesos\src\tests\slave_tests.cpp(426): error: Actual function call 
count doesn't match EXPECT_CALL(sched, statusUpdate(, _))...
 Expected: to be called once
   Actual: never called - unsatisfied and active
```

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

```
I0220 08:03:30.996676   612 master.cpp:1422] Framework 
3209feb6-7464-40c8-9833-1b3cc0df94eb- (default) at 
scheduler-11106046-a8a2-404f-8044-998e010b112c@10.3.1.5:49156 disconnected
I0220 08:03:30.997678   612 master.cpp:3240] Deactivating framework 
3209feb6-7464-40c8-9833-1b3cc0df94eb- (default) at 
scheduler-11106046-a8a2-404f-8044-998e010b112c@10.3.1.5:49156
I0220 08:03:30.997678   612 master.cpp:3217] Disconnecting framework 
3209feb6-7464-40c8-9833-1b3cc0df94eb- (default) at 
scheduler-11106046-a8a2-404f-8044-998e010b112c@10.3.1.5:49156
I0220 08:03:30.997678  4800 hierarchical.cpp:405] Deactivated framework 
3209feb6-7464-40c8-9833-1b3cc0df94eb-
I0220 08:03:30.997678   612 master.cpp:1437] Giving framework 
3209feb6-7464-40c8-9833-1b3cc0df94eb- (default) at 
scheduler-11106046-a8a2-404f-8044-998e010b112c@10.3.1.5:49156 0ns to failover
I0220 08:03:30.998677  3688 master.cpp:8668] Framework failover timeout, 
removing framework 3209feb6-7464-40c8-9833-1b3cc0df94eb- (default) at 
scheduler-11106046-a8a2-404f-8044-998e010b112c@10.3.1.5:49156
I0220 08:03:30.998677  3688 master.cpp:9545] Removing framework 
3209feb6-7464-40c8-9833-1b3cc0df94eb- (default) at 
scheduler-11106046-a8a2-404f-8044-998e010b112c@10.3.1.5:49156
I0220 08:03:30.999676  3688 master.cpp:10249] Updating the state of task 1 of 
framework 3209feb6-7464-40c8-9833-1b3cc0df94eb- (latest state: TASK_KILLED, 
status update state: TASK_KILLED)
I0220 08:03:31.001677  3688 master.cpp:10348]@   7FF69D84F35A  
),mesos::FrameworkID,mesos::ExecutorID,process::Future,std::_Ph<1> >,process::ProcessBase * __ptr64>
@   7FF69D8FCCFC  
std::invoke,std::_Ph<1> >,process::ProcessBase * __ptr64>
@   7FF69D857321  
),std::_Ph<1> >,process::ProcessBase * __ptr64
@   7FF69D990FF6  process::ProcessBase * 
__ptr64)>::CallableFn,std::_Ph<1> > >::operator(
@   7FF69F3D269D  process::ProcessBase * __ptr64)>::operator(
@   7FF69F2AB969  process::ProcessBase::consume
@   7FF69F425F5A  process::DispatchEvent::consume
@   7FF69B5FEF17  process::ProcessBase::serve
@   7FF69F2B9880  process::ProcessManager::resume
@   7FF69F3C2DF1   ?? 
@   7FF69F301AE0  
std::_Invoker_functor::_Call< >
@   7FF69F357730  std::invoke< 
>
@   7FF69F31085C