Re: Review Request 62391: Fixed invalid handle bug in `os::process()`.

2017-09-18 Thread Andrew Schwartzmeyer


> On Sept. 18, 2017, 8:29 p.m., Mesos Reviewbot Windows wrote:
> > FAIL: Some Mesos tests failed.
> > 
> > Reviews applied: `['62391']`
> > 
> > Failed command: `C:\mesos\src\mesos-tests.exe --verbose`
> > 
> > All the build artifacts available at: 
> > http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62391
> > 
> > Relevant logs:
> > 
> > - 
> > [mesos-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62391/logs/mesos-tests-stdout.log):
> > 
> > ```
> > [ RUN  ] ContentType/SchedulerTest.SchedulerReconnect/0
> > [   OK ] ContentType/SchedulerTest.SchedulerReconnect/0 (228 ms)
> > [ RUN  ] ContentType/SchedulerTest.SchedulerReconnect/1
> > [   OK ] ContentType/SchedulerTest.SchedulerReconnect/1 (256 ms)
> > [--] 30 tests from ContentType/SchedulerTest (25650 ms total)
> > 
> > [--] 2 tests from ContentTypeAndSSLConfig/SchedulerSSLTest
> > [ RUN  ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/0
> > [   OK ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/0 
> > (992 ms)
> > [ RUN  ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/1
> > [   OK ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/1 
> > (994 ms)
> > [--] 2 tests from ContentTypeAndSSLConfig/SchedulerSSLTest (2063 ms 
> > total)
> > 
> > [--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest
> > [ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0
> > [   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0 
> > (125 ms)
> > [ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1
> > [   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1 
> > (158 ms)
> > [--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest 
> > (332 ms total)
> > 
> > [--] Global test environment tear-down
> > [==] 627 tests from 66 test cases ran. (341688 ms total)
> > [  PASSED  ] 626 tests.
> > [  FAILED  ] 1 test, listed below:
> > [  FAILED  ] ContentType/MasterAPITest.EventAuthorizationFiltering/1, where 
> > GetParam() = application/json
> > 
> >  1 FAILED TEST
> >   YOU HAVE 174 DISABLED TESTS
> > 
> > ```
> > 
> > - 
> > [mesos-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62391/logs/mesos-tests-stderr.log):
> > 
> > ```
> > I0919 03:28:55.491580 17364 master.cpp:8418] Removing framework 
> > d7323cfa-3c5b-401b-823a-3dc5ab9db8b1- (default)
> > I0919 03:28:55.491580 17364 master.cpp:3267] Deactivating framework 
> > d7323cfa-3c5b-401b-823a-3dc5ab9db8b1- (default)
> > I0919 03:28:55.491580 21948 hierarchical.cpp:412] Deactivated framework 
> > d7323cfa-3c5b-401b-823a-3dc5ab9db8b1-
> > I0919 03:28:55.492578 22180 slave.cpp:3235] Shutting down framework 
> > d7323cfa-3c5b-401b-823a-3dc5ab9db8b1-
> > I0919 03:28:55.492578 17364 master.cpp:8993] Updating the state of task 
> > 1eef69ce-5917-4e39-b018-eea8b719f3f8 of framework 
> > d7323cfa-3c5b-401b-823a-3dc5ab9db8b1- (latest state: TASK_KILLED, 
> > status update state: TASK_KILLED)
> > I0919 03:28:55.492578 22180 slave.cpp:5731] Shutting down executor 
> > 'default' of framework d7323cfa-3c5b-401b-823a-3dc5ab9db8b1- (via HTTP)
> > I0919 03:28:55.504575 17364 master.cpp:9087] Removing task 
> > 1eef69ce-5917-4e39-b018-eea8b719f3f8 with resources 
> > [{"allocation_info":{"role":"*"},"name":"cpus","scalar":{"value":2.0},"type":"SCALAR"},{"allocation_info":{"role":"*"},"name":"mem","scalar":{"value":1024.0},"type":"SCALAR"},{"allocation_info":{"role":"*"},"name":"disk","scalar":{"value":1024.0},"type":"SCALAR"},{"allocation_info":{"role":"*"},"name":"ports","ranges":{"range":[{"begin":31000,"end":32000}]},"type":"RANGES"}]
> >  of framework d7323cfa-3c5b-401b-823a-3dc5ab9db8b1- on agent 
> > d7323cfa-3c5b-401b-823a-3dc5ab9db8b1-S0 at slave(254)@10.3.1.5:54306 
> > (mesos-bld-s1.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
> > I0919 03:28:55.515578 17364 master.cpp:9116] Removing executor 'default' 
> > with resources [] of framework d7323cfa-3c5b-401b-823a-3dc5ab9db8b1- on 
> > agent d7323cfa-3c5b-401b-823a-3dc5ab9db8b1-S0 at slave(254)@10.3.1.5:54306 
> > (mesos-bld-s1.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
> > I0919 03:28:55.527577 22180 hierarchical.cpp:355] Removed framework 
> > d7323cfa-3c5b-401b-823a-3dc5ab9db8b1-
> > E0919 03:28:55.528578 21784 scheduler.cpp:649] End-Of-File received from 
> > master. The master closed the event stream
> > I0919 03:28:55.530576 19512 scheduler.cpp:444] Re-detecting master
> > I0919 03:28:55.533577 19512 scheduler.cpp:470] New master detected at 
> > master@10.3.1.5:54306
> > I0919 03:28:55.545578 17364 slave.cpp:5407] Executor 'default' of framework 
> > d7323cfa-3c5b-401b-823a-3dc5ab9db8b1- exited with status 0
> > I0919 03:28:55.545578 17364 slave.cpp:5511] Cleaning up executor 'default' 
> > of framework 

Re: Review Request 61528: Implemented a registrar for resource provider manager state.

2017-09-18 Thread Jie Yu

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




src/master/registry.proto
Lines 73-75 (patched)


Hum, why we still need this?



src/master/registry.proto
Lines 73-75 (patched)


Do we still need this?



src/resource_provider/registrar.hpp
Lines 83 (patched)


Let's be more consistent. We tend not to use tailing underscore for member 
fields if no needed (we do need if there is a public member accessor with the 
same name). Please fix all that in this patch.



src/resource_provider/registrar.cpp
Lines 128-133 (patched)


See my comments below. Probably don't need this if we tweek the protobuf.



src/resource_provider/registrar.cpp
Lines 190 (patched)


I think we want to tie the lifecycle of resource providers to the lifecycle 
of agents so that when the agent is gone, all the LRP associated with that 
agent is gone too.

Given that we don't change agent ID upon slave machine reboot, i think it 
makes sense to checkpoint the local resource provider information under 
`/slaves//resource_provider_registry/`

Let's introduce a helper in `src/slave/paths.hpp|cpp` for the path helper



src/resource_provider/registrar.cpp
Lines 308 (patched)


Please fix all typos.



src/resource_provider/registrar.cpp
Lines 331 (patched)


Given this is a c++ source file, i'll try to use 'using' clause in the 
begining and avoid namespace prefix if possible.



src/resource_provider/registrar.cpp
Lines 337-338 (patched)


Ditto.



src/resource_provider/registry.proto
Lines 42 (patched)


hum, any reason we don't use:
```
message Registry {
  repeated ResourceProvider resource_providers;
}
```


- Jie Yu


On Sept. 15, 2017, 1:16 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61528/
> ---
> 
> (Updated Sept. 15, 2017, 1:16 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Bugs: MESOS-7555
> https://issues.apache.org/jira/browse/MESOS-7555
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a registry and registrar interface for resource
> provider managers. The registrar interface is modelled after the
> master registrar and supports similar operations. Currently a single,
> LevelDB-backed registrar is implemented which we plan to use for
> resource provider managers in agents.
> 
> Current the registry allows to add and remove resource provider IDs.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 1a828c4351ded36f51ccbbe67147da2f50b9cdb1 
>   src/Makefile.am 93ed2bf55447e3e470d9bea8a0b61ce78aad1900 
>   src/master/registry.proto 362a9fab946e9fb0411df2277f7f3edbadccb61a 
>   src/resource_provider/registrar.hpp PRE-CREATION 
>   src/resource_provider/registrar.cpp PRE-CREATION 
>   src/resource_provider/registry.hpp PRE-CREATION 
>   src/resource_provider/registry.proto PRE-CREATION 
>   src/tests/resource_provider_manager_tests.cpp 
> 3bc56b51526e9dd188423f7349e74246c3295c77 
> 
> 
> Diff: https://reviews.apache.org/r/61528/diff/5/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 62003: Added `network/ports` isolator nested container tests.

2017-09-18 Thread Mesos Reviewbot Windows

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



FAIL: Failed to apply the dependent review: 60491.

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

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

Relevant logs:

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

```
error: patch failed: src/linux/routing/diagnosis/diagnosis.hpp:60
error: src/linux/routing/diagnosis/diagnosis.hpp: patch does not apply
error: patch failed: src/linux/routing/diagnosis/diagnosis.cpp:67
error: src/linux/routing/diagnosis/diagnosis.cpp: patch does not apply
```

- Mesos Reviewbot Windows


On Sept. 8, 2017, 10:44 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62003/
> ---
> 
> (Updated Sept. 8, 2017, 10:44 p.m.)
> 
> 
> Review request for mesos, Qian Zhang and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7675
> https://issues.apache.org/jira/browse/MESOS-7675
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `network/ports` isolator nested container tests using the v1
> TaskGroups API. This tests that rogue port usage by a nested task is
> detected both with and without agent recovery, and that a well-behaved
> task is preserved across agent recovery.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/ports_isolator_tests.cpp PRE-CREATION 
>   src/tests/mesos.hpp b0749db273c91692b91d7ca87f975694b18e422c 
> 
> 
> Diff: https://reviews.apache.org/r/62003/diff/5/
> 
> 
> Testing
> ---
> 
> make check (Fedora 26)
> 
> 
> Thanks,
> 
> James Peach
> 
>



Re: Review Request 62391: Fixed invalid handle bug in `os::process()`.

2017-09-18 Thread Mesos Reviewbot Windows

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



FAIL: Some Mesos tests failed.

Reviews applied: `['62391']`

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

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

Relevant logs:

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

```
[ RUN  ] ContentType/SchedulerTest.SchedulerReconnect/0
[   OK ] ContentType/SchedulerTest.SchedulerReconnect/0 (228 ms)
[ RUN  ] ContentType/SchedulerTest.SchedulerReconnect/1
[   OK ] ContentType/SchedulerTest.SchedulerReconnect/1 (256 ms)
[--] 30 tests from ContentType/SchedulerTest (25650 ms total)

[--] 2 tests from ContentTypeAndSSLConfig/SchedulerSSLTest
[ RUN  ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/0
[   OK ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/0 (992 
ms)
[ RUN  ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/1
[   OK ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/1 (994 
ms)
[--] 2 tests from ContentTypeAndSSLConfig/SchedulerSSLTest (2063 ms 
total)

[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0 (125 
ms)
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1 (158 
ms)
[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest (332 ms 
total)

[--] Global test environment tear-down
[==] 627 tests from 66 test cases ran. (341688 ms total)
[  PASSED  ] 626 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] ContentType/MasterAPITest.EventAuthorizationFiltering/1, where 
GetParam() = application/json

 1 FAILED TEST
  YOU HAVE 174 DISABLED TESTS

```

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

```
I0919 03:28:55.491580 17364 master.cpp:8418] Removing framework 
d7323cfa-3c5b-401b-823a-3dc5ab9db8b1- (default)
I0919 03:28:55.491580 17364 master.cpp:3267] Deactivating framework 
d7323cfa-3c5b-401b-823a-3dc5ab9db8b1- (default)
I0919 03:28:55.491580 21948 hierarchical.cpp:412] Deactivated framework 
d7323cfa-3c5b-401b-823a-3dc5ab9db8b1-
I0919 03:28:55.492578 22180 slave.cpp:3235] Shutting down framework 
d7323cfa-3c5b-401b-823a-3dc5ab9db8b1-
I0919 03:28:55.492578 17364 master.cpp:8993] Updating the state of task 
1eef69ce-5917-4e39-b018-eea8b719f3f8 of framework 
d7323cfa-3c5b-401b-823a-3dc5ab9db8b1- (latest state: TASK_KILLED, status 
update state: TASK_KILLED)
I0919 03:28:55.492578 22180 slave.cpp:5731] Shutting down executor 'default' of 
framework d7323cfa-3c5b-401b-823a-3dc5ab9db8b1- (via HTTP)
I0919 03:28:55.504575 17364 master.cpp:9087] Removing task 
1eef69ce-5917-4e39-b018-eea8b719f3f8 with resources 
[{"allocation_info":{"role":"*"},"name":"cpus","scalar":{"value":2.0},"type":"SCALAR"},{"allocation_info":{"role":"*"},"name":"mem","scalar":{"value":1024.0},"type":"SCALAR"},{"allocation_info":{"role":"*"},"name":"disk","scalar":{"value":1024.0},"type":"SCALAR"},{"allocation_info":{"role":"*"},"name":"ports","ranges":{"range":[{"begin":31000,"end":32000}]},"type":"RANGES"}]
 of framework d7323cfa-3c5b-401b-823a-3dc5ab9db8b1- on agent 
d7323cfa-3c5b-401b-823a-3dc5ab9db8b1-S0 at slave(254)@10.3.1.5:54306 
(mesos-bld-s1.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0919 03:28:55.515578 17364 master.cpp:9116] Removing executor 'default' with 
resources [] of framework d7323cfa-3c5b-401b-823a-3dc5ab9db8b1- on agent 
d7323cfa-3c5b-401b-823a-3dc5ab9db8b1-S0 at slave(254)@10.3.1.5:54306 
(mesos-bld-s1.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0919 03:28:55.527577 22180 hierarchical.cpp:355] Removed framework 
d7323cfa-3c5b-401b-823a-3dc5ab9db8b1-
E0919 03:28:55.528578 21784 scheduler.cpp:649] End-Of-File received from 
master. The master closed the event stream
I0919 03:28:55.530576 19512 scheduler.cpp:444] Re-detecting master
I0919 03:28:55.533577 19512 scheduler.cpp:470] New master detected at 
master@10.3.1.5:54306
I0919 03:28:55.545578 17364 slave.cpp:5407] Executor 'default' of framework 
d7323cfa-3c5b-401b-823a-3dc5ab9db8b1- exited with status 0
I0919 03:28:55.545578 17364 slave.cpp:5511] Cleaning up executor 'default' of 
framework d7323cfa-3c5b-401b-823a-3dc5ab9db8b1- (via HTTP)
W0919 03:28:55.546577 21784 master.cpp:7021] Ignoring unknown exited executor 
'default' of framework d7323cfa-3c5b-401b-823a-3dc5ab9db8b1- on agent 

Re: Review Request 60766: Ignored containers that join CNI networks.

2017-09-18 Thread Qian Zhang

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




src/slave/containerizer/mesos/isolators/network/ports.cpp
Line 511 (original), 586-589 (patched)


I still think it is better to raise the limitation against the nested 
container rather than the root container because that will give framework more 
fine-grained debugging information so that it can know which specific nested 
container triggered the limitation.

So I think we need to enhance the implementation of `waitNestedContainer()` 
to make it propagate the `reason` and `message` to the default executor, and 
then the default executor can send the limitation for the nested container.


- Qian Zhang


On Sept. 6, 2017, 1:57 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60766/
> ---
> 
> (Updated Sept. 6, 2017, 1:57 a.m.)
> 
> 
> Review request for mesos, Qian Zhang and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7675
> https://issues.apache.org/jira/browse/MESOS-7675
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Working on the assumption that containers with CNI networks will
> get their own IP addresses and don't need port isolation, ignore
> any containers that are joining CNI networks.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 4d5dc13f363f5d8886983d7dd06a5cecc177c345 
>   src/slave/containerizer/mesos/isolators/network/ports.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/ports.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60766/diff/18/
> 
> 
> Testing
> ---
> 
> make check (Fedora 26).
> 
> 
> Thanks,
> 
> James Peach
> 
>



Re: Review Request 60495: Added network ports isolator listen socket utilities.

2017-09-18 Thread Qian Zhang

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




src/slave/containerizer/mesos/isolators/network/ports.cpp
Line 112 (original), 112-117 (patched)


Can we change this to `while ((entry = readdir(dir)) != nullptr) {` and do 
`errno = 0` in the end of the while loop? There is already a `errno = 0` (line 
106) right before the while loop, so I think we may not need to do it again at 
the beginning of the loop, instead it would be better to do it in the end of 
the loop.


- Qian Zhang


On Sept. 19, 2017, 8:19 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60495/
> ---
> 
> (Updated Sept. 19, 2017, 8:19 a.m.)
> 
> 
> Review request for mesos, Qian Zhang and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7675
> https://issues.apache.org/jira/browse/MESOS-7675
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented network ports isolator socket utilities to find the
> inode numbers of all the listening sockets, and the inodes of the
> open sockets for a process. Together, these utilities can tell us
> which sockets a process is listening on.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/ports.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/ports.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60495/diff/16/
> 
> 
> Testing
> ---
> 
> make check (Fedora 26)
> 
> 
> Thanks,
> 
> James Peach
> 
>



Re: Review Request 60496: Added socket checking to the network ports isolator.

2017-09-18 Thread James Peach

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

(Updated Sept. 19, 2017, 12:20 a.m.)


Review request for mesos, Qian Zhang and Jiang Yan Xu.


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


Repository: mesos


Description
---

Implemented ports resource restrictions in the network ports isolator.
Periodically, scan for listening sockets and match them up to all
the open sockets in the containers we are tracking in the network.
Check any sockets we find against the ports resource and trigger a
resource limitation if the port has not been allocated.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/network/ports.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/network/ports.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/60496/diff/19/

Changes: https://reviews.apache.org/r/60496/diff/18-19/


Testing
---

make check (Fedora 26)


Thanks,

James Peach



Re: Review Request 60495: Added network ports isolator listen socket utilities.

2017-09-18 Thread James Peach

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

(Updated Sept. 19, 2017, 12:19 a.m.)


Review request for mesos, Qian Zhang and Jiang Yan Xu.


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


Repository: mesos


Description
---

Implemented network ports isolator socket utilities to find the
inode numbers of all the listening sockets, and the inodes of the
open sockets for a process. Together, these utilities can tell us
which sockets a process is listening on.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/network/ports.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/network/ports.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/60495/diff/16/

Changes: https://reviews.apache.org/r/60495/diff/15-16/


Testing
---

make check (Fedora 26)


Thanks,

James Peach



Re: Review Request 60496: Added socket checking to the network ports isolator.

2017-09-18 Thread James Peach


> On Sept. 16, 2017, 3 a.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/network/ports.cpp
> > Lines 421-424 (patched)
> > 
> >
> > We need to log this message like what we did here:
> > 
> > https://github.com/apache/mesos/blob/1.3.1/src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp#L556
> 
> James Peach wrote:
> Do you mean that we should add a second log message of the form "Ports 
> limit exceeded: Requested:  Maximum Used: "? If we 
> are strictly consistent, then "" would be the actual ports used 
> and the operator would need to mentally perform the subtraction. We 
> previously agreed to log only the ports outside the allocated range.
> 
> I'm fine with the general form of the message, but we probably want to 
> rephrase is slightly?

Clarified that we keep the message text as is, but send it to `LOG(INFO)`.


- James


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


On Sept. 8, 2017, 12:09 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60496/
> ---
> 
> (Updated Sept. 8, 2017, 12:09 a.m.)
> 
> 
> Review request for mesos, Qian Zhang and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7675
> https://issues.apache.org/jira/browse/MESOS-7675
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented ports resource restrictions in the network ports isolator.
> Periodically, scan for listening sockets and match them up to all
> the open sockets in the containers we are tracking in the network.
> Check any sockets we find against the ports resource and trigger a
> resource limitation if the port has not been allocated.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/ports.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/ports.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60496/diff/18/
> 
> 
> Testing
> ---
> 
> make check (Fedora 26)
> 
> 
> Thanks,
> 
> James Peach
> 
>



Re: Review Request 60496: Added socket checking to the network ports isolator.

2017-09-18 Thread James Peach


> On Sept. 16, 2017, 3 a.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/network/ports.cpp
> > Lines 421-424 (patched)
> > 
> >
> > I did some experiments and found when I launch a comman task which is 
> > allocated with ports [31001-31005] and the command (`nc`) actually listens 
> > on 31006, then I found this task can be killed, but here the message is:
> > ```
> > "Container aa99dab3-2a25-44e0-bc88-16c485c5c87a is listening on 
> > unallocated port(s) {[31006,31007)}"
> > ```
> > This message seems not correct:
> > 1. why 31007? The `nc` command only listens on 31006. And if I change 
> > the `nc` command to listen on 31009, the message will be "`... 
> > {[31009,31010)}`".
> > 2. The brackets in `{[31006,31007)}` are not correct, it should be 
> > either `[]` or `()` but not `[)`.

This is interval set notation produces by converting an `IntervalSet` to a 
string. `[31006,31007)` means the interval that is `>= 31006` and `< 31007`.


> On Sept. 16, 2017, 3 a.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/network/ports.cpp
> > Lines 421-424 (patched)
> > 
> >
> > We need to log this message like what we did here:
> > 
> > https://github.com/apache/mesos/blob/1.3.1/src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp#L556

Do you mean that we should add a second log message of the form "Ports limit 
exceeded: Requested:  Maximum Used: "? If we are 
strictly consistent, then "" would be the actual ports used and the 
operator would need to mentally perform the subtraction. We previously agreed 
to log only the ports outside the allocated range.

I'm fine with the general form of the message, but we probably want to rephrase 
is slightly?


- James


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


On Sept. 8, 2017, 12:09 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60496/
> ---
> 
> (Updated Sept. 8, 2017, 12:09 a.m.)
> 
> 
> Review request for mesos, Qian Zhang and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7675
> https://issues.apache.org/jira/browse/MESOS-7675
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented ports resource restrictions in the network ports isolator.
> Periodically, scan for listening sockets and match them up to all
> the open sockets in the containers we are tracking in the network.
> Check any sockets we find against the ports resource and trigger a
> resource limitation if the port has not been allocated.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/ports.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/ports.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60496/diff/18/
> 
> 
> Testing
> ---
> 
> make check (Fedora 26)
> 
> 
> Thanks,
> 
> James Peach
> 
>



Re: Review Request 62391: Fixed invalid handle bug in `os::process()`.

2017-09-18 Thread Andrew Schwartzmeyer

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

(Updated Sept. 18, 2017, 4:39 p.m.)


Review request for mesos, James Peach and Joseph Wu.


Changes
---

Fix `stout-tests`.


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


Repository: mesos


Description
---

This patch fixes a bug found by running `mesos-tests` under Application
Verifier on Windows. Mesos was inadvertently attempting to get a process
handle for the System Idle Process with PID `0`, which is not permitted
by the OS. To remedy this, we now check if `os::process` receives `0`
for its argument, and return an error if so. Furthermore, we remove the
PID `0` from the `os::pids` API, as it is not useful to the programmer,
and only serves to cause errors later. Finally, the return value for
`OpenProcess` was being incorrectly checked, as the API returns a
`nullptr` on failure, not `INVALID_HANDLE_VALUE`.


Diffs (updated)
-

  3rdparty/stout/include/stout/windows/os.hpp 
f35bf312a25066a167dc66243ad9bf8333cb36a6 
  3rdparty/stout/tests/os/process_tests.cpp 
963eb4eb3fb64d627e177d760bf42b777ae0e924 


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

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


Testing (updated)
---

Built and ran `mesos-tests`, `libprocess-tests`, and `stout-tests` on Windows 
under Application Verifier. All tests pass, and the error from `OpenProcess` is 
no longer being returned.


Thanks,

Andrew Schwartzmeyer



Re: Review Request 62391: Fixed invalid handle bug in `os::process()`.

2017-09-18 Thread Andrew Schwartzmeyer


> On Sept. 18, 2017, 3:49 p.m., Mesos Reviewbot Windows wrote:
> > FAIL: Some Mesos stdout-tests tests failed.
> > 
> > Reviews applied: `['62391']`
> > 
> > Failed command: `C:\mesos\3rdparty\stout\tests\Debug\stout-tests.exe`
> > 
> > All the build artifacts available at: 
> > http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62391
> > 
> > Relevant logs:
> > 
> > - 
> > [stdout-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62391/logs/stdout-tests-stdout.log):
> > 
> > ```
> > [ RUN  ] SocketTests.InitSocket
> > [   OK ] SocketTests.InitSocket (1 ms)
> > [--] 1 test from SocketTests (17 ms total)
> > 
> > [--] 2 tests from StrerrorTest
> > [ RUN  ] StrerrorTest.ValidErrno
> > [   OK ] StrerrorTest.ValidErrno (0 ms)
> > [ RUN  ] StrerrorTest.InvalidErrno
> > [   OK ] StrerrorTest.InvalidErrno (0 ms)
> > [--] 2 tests from StrerrorTest (58 ms total)
> > 
> > [--] 3 tests from SystemsTests
> > [ RUN  ] SystemsTests.Uname
> > [   OK ] SystemsTests.Uname (1 ms)
> > [ RUN  ] SystemsTests.Sysname
> > [   OK ] SystemsTests.Sysname (1 ms)
> > [ RUN  ] SystemsTests.Release
> > [   OK ] SystemsTests.Release (1 ms)
> > [--] 3 tests from SystemsTests (86 ms total)
> > 
> > [--] Global test environment tear-down
> > [==] 256 tests from 43 test cases ran. (12385 ms total)
> > [  PASSED  ] 255 tests.
> > [  FAILED  ] 1 test, listed below:
> > [  FAILED  ] ProcessTest.Pids
> > 
> >  1 FAILED TEST
> >   YOU HAVE 12 DISABLED TESTS
> > 
> > ```
> > 
> > - 
> > [stdout-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62391/logs/stdout-tests-stderr.log):
> > 
> > ```
> > 'invalid.command' is not recognized as an internal or external command,
> > operable program or batch file.
> > Subcommand 'subcommand' is not available
> > Usage: command  [OPTIONS]
> > 
> > Available subcommands:
> > help
> > subcommand2
> > 
> > Multiple subcommands have name 'subcommand'
> > ```

Heh, I didn't run stout-tests... my bad. Fixing.


- Andrew


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


On Sept. 18, 2017, 1:42 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62391/
> ---
> 
> (Updated Sept. 18, 2017, 1:42 p.m.)
> 
> 
> Review request for mesos, James Peach and Joseph Wu.
> 
> 
> Bugs: MESOS-7988
> https://issues.apache.org/jira/browse/MESOS-7988
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch fixes a bug found by running `mesos-tests` under Application
> Verifier on Windows. Mesos was inadvertently attempting to get a process
> handle for the System Idle Process with PID `0`, which is not permitted
> by the OS. To remedy this, we now check if `os::process` receives `0`
> for its argument, and return an error if so. Furthermore, we remove the
> PID `0` from the `os::pids` API, as it is not useful to the programmer,
> and only serves to cause errors later. Finally, the return value for
> `OpenProcess` was being incorrectly checked, as the API returns a
> `nullptr` on failure, not `INVALID_HANDLE_VALUE`.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/windows/os.hpp 
> f35bf312a25066a167dc66243ad9bf8333cb36a6 
> 
> 
> Diff: https://reviews.apache.org/r/62391/diff/1/
> 
> 
> Testing
> ---
> 
> Built and ran `mesos-tests` on Windows under Application Verifier. All tests 
> pass, and the error from `OpenProcess` is no longer being returned.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 62391: Fixed invalid handle bug in `os::process()`.

2017-09-18 Thread Mesos Reviewbot Windows

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



FAIL: Some Mesos stdout-tests tests failed.

Reviews applied: `['62391']`

Failed command: `C:\mesos\3rdparty\stout\tests\Debug\stout-tests.exe`

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

Relevant logs:

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

```
[ RUN  ] SocketTests.InitSocket
[   OK ] SocketTests.InitSocket (1 ms)
[--] 1 test from SocketTests (17 ms total)

[--] 2 tests from StrerrorTest
[ RUN  ] StrerrorTest.ValidErrno
[   OK ] StrerrorTest.ValidErrno (0 ms)
[ RUN  ] StrerrorTest.InvalidErrno
[   OK ] StrerrorTest.InvalidErrno (0 ms)
[--] 2 tests from StrerrorTest (58 ms total)

[--] 3 tests from SystemsTests
[ RUN  ] SystemsTests.Uname
[   OK ] SystemsTests.Uname (1 ms)
[ RUN  ] SystemsTests.Sysname
[   OK ] SystemsTests.Sysname (1 ms)
[ RUN  ] SystemsTests.Release
[   OK ] SystemsTests.Release (1 ms)
[--] 3 tests from SystemsTests (86 ms total)

[--] Global test environment tear-down
[==] 256 tests from 43 test cases ran. (12385 ms total)
[  PASSED  ] 255 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] ProcessTest.Pids

 1 FAILED TEST
  YOU HAVE 12 DISABLED TESTS

```

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

```
'invalid.command' is not recognized as an internal or external command,
operable program or batch file.
Subcommand 'subcommand' is not available
Usage: command  [OPTIONS]

Available subcommands:
help
subcommand2

Multiple subcommands have name 'subcommand'
```

- Mesos Reviewbot Windows


On Sept. 18, 2017, 8:42 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62391/
> ---
> 
> (Updated Sept. 18, 2017, 8:42 p.m.)
> 
> 
> Review request for mesos, James Peach and Joseph Wu.
> 
> 
> Bugs: MESOS-7988
> https://issues.apache.org/jira/browse/MESOS-7988
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch fixes a bug found by running `mesos-tests` under Application
> Verifier on Windows. Mesos was inadvertently attempting to get a process
> handle for the System Idle Process with PID `0`, which is not permitted
> by the OS. To remedy this, we now check if `os::process` receives `0`
> for its argument, and return an error if so. Furthermore, we remove the
> PID `0` from the `os::pids` API, as it is not useful to the programmer,
> and only serves to cause errors later. Finally, the return value for
> `OpenProcess` was being incorrectly checked, as the API returns a
> `nullptr` on failure, not `INVALID_HANDLE_VALUE`.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/windows/os.hpp 
> f35bf312a25066a167dc66243ad9bf8333cb36a6 
> 
> 
> Diff: https://reviews.apache.org/r/62391/diff/1/
> 
> 
> Testing
> ---
> 
> Built and ran `mesos-tests` on Windows under Application Verifier. All tests 
> pass, and the error from `OpenProcess` is no longer being returned.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 62332: Linted JavaScript files.

2017-09-18 Thread Mesos Reviewbot Windows

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



FAIL: Some Mesos tests failed.

Reviews applied: `['62332']`

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

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

Relevant logs:

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

```
[ RUN  ] ContentType/SchedulerTest.SchedulerReconnect/0
[   OK ] ContentType/SchedulerTest.SchedulerReconnect/0 (283 ms)
[ RUN  ] ContentType/SchedulerTest.SchedulerReconnect/1
[   OK ] ContentType/SchedulerTest.SchedulerReconnect/1 (242 ms)
[--] 30 tests from ContentType/SchedulerTest (25452 ms total)

[--] 2 tests from ContentTypeAndSSLConfig/SchedulerSSLTest
[ RUN  ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/0
[   OK ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/0 
(1166 ms)
[ RUN  ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/1
[   OK ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/1 
(1182 ms)
[--] 2 tests from ContentTypeAndSSLConfig/SchedulerSSLTest (2484 ms 
total)

[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0 (138 
ms)
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1 (152 
ms)
[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest (411 ms 
total)

[--] Global test environment tear-down
[==] 627 tests from 66 test cases ran. (357074 ms total)
[  PASSED  ] 626 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] ContentType/MasterAPITest.EventAuthorizationFiltering/1, where 
GetParam() = application/json

 1 FAILED TEST
  YOU HAVE 174 DISABLED TESTS

```

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

```
I0918 22:15:19.008848 17356 master.cpp:8418] Removing framework 
fa8ea04d-ff42-4f9f-b5ae-27217af37bc9- (default)
I0918 22:15:19.008848 17356 master.cpp:3267] Deactivating framework 
fa8ea04d-ff42-4f9f-b5ae-27217af37bc9- (default)
I0918 22:15:19.009850 21088 hierarchical.cpp:412] Deactivated framework 
fa8ea04d-ff42-4f9f-b5ae-27217af37bc9-
I0918 22:15:19.009850 20792 slave.cpp:3235] Shutting down framework 
fa8ea04d-ff42-4f9f-b5ae-27217af37bc9-
I0918 22:15:19.009850 17356 master.cpp:8993] Updating the state of task 
17f92b4e-58ef-4881-b54b-3fefdf309a97 of framework 
fa8ea04d-ff42-4f9f-b5ae-27217af37bc9- (latest state: TASK_KILLED, status 
update state: TASK_KILLED)
I0918 22:15:19.009850 20792 slave.cpp:5731] Shutting down executor 'default' of 
framework fa8ea04d-ff42-4f9f-b5ae-27217af37bc9- (via HTTP)
I0918 22:15:19.013850 17356 master.cpp:9087] Removing task 
17f92b4e-58ef-4881-b54b-3fefdf309a97 with resources 
[{"allocation_info":{"role":"*"},"name":"cpus","scalar":{"value":2.0},"type":"SCALAR"},{"allocation_info":{"role":"*"},"name":"mem","scalar":{"value":1024.0},"type":"SCALAR"},{"allocation_info":{"role":"*"},"name":"disk","scalar":{"value":1024.0},"type":"SCALAR"},{"allocation_info":{"role":"*"},"name":"ports","ranges":{"range":[{"begin":31000,"end":32000}]},"type":"RANGES"}]
 of framework fa8ea04d-ff42-4f9f-b5ae-27217af37bc9- on agent 
fa8ea04d-ff42-4f9f-b5ae-27217af37bc9-S0 at slave(254)@10.3.1.5:52150 
(mesos-bld-s1.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0918 22:15:19.036852 17356 master.cpp:9116] Removing executor 'default' with 
resources [] of framework fa8ea04d-ff42-4f9f-b5ae-27217af37bc9- on agent 
fa8ea04d-ff42-4f9f-b5ae-27217af37bc9-S0 at slave(254)@10.3.1.5:52150 
(mesos-bld-s1.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0918 22:15:19.040853 15292 hierarchical.cpp:355] Removed framework 
fa8ea04d-ff42-4f9f-b5ae-27217af37bc9-
E0918 22:15:19.040853 14848 scheduler.cpp:649] End-Of-File received from 
master. The master closed the event stream
I0918 22:15:19.064852 14848 scheduler.cpp:444] Re-detecting master
I0918 22:15:19.066861 21256 scheduler.cpp:470] New master detected at 
master@10.3.1.5:52150
I0918 22:15:19.081854 15292 slave.cpp:5407] Executor 'default' of framework 
fa8ea04d-ff42-4f9f-b5ae-27217af37bc9- exited with status 0
I0918 22:15:19.082854 15292 slave.cpp:5511] Cleaning up executor 'default' of 
framework fa8ea04d-ff42-4f9f-b5ae-27217af37bc9- (via HTTP)
W0918 22:15:19.083854 21020 master.cpp:7021] Ignoring unknown exited executor 
'default' of framework fa8ea04d-ff42-4f9f-b5ae-27217af37bc9- on agent 

Re: Review Request 61183: Triggered 'UpdateSlaveMessage' when 'ResourceProviderManager' updates.

2017-09-18 Thread Jie Yu

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




src/slave/slave.cpp
Lines 1524 (patched)


Can you update for the re-registration case as well?



src/slave/slave.cpp
Lines 3505-3507 (patched)


I am trying very hard to think if this is problematic. It's very hard to 
reason about. My intuition told me that this might be problematic because 
`totalResources` might reflect some new update to the RP's resources that 
haven't synced with the master yet.

Given that `lastSyncedTotalResources` is just an optimization, I'll just 
don't do anything in this function.



src/slave/slave.cpp
Lines 6124-6126 (patched)


No need for this given that you'll update `lastSyncedTotalResources` before 
sending `RegisterSlaveMessage`?



src/slave/slave.cpp
Lines 6609 (patched)


Include the error message?

I would make this a LOG(ERROR) instead



src/slave/slave.cpp
Lines 6617 (patched)


I'd make this a LOG(INFO)



src/slave/slave.cpp
Lines 6620 (patched)


Let's add a parathesis to the `case` statement:
```
case ResourceProviderMessage::Type::UPDATE_TOTAL_RESOURCES: {
  ...
}
```



src/slave/slave.cpp
Lines 6630 (patched)


indentation?



src/slave/slave.cpp
Lines 6669 (patched)


add a `break` for the first level switch? and a default?



src/tests/slave_tests.cpp
Lines 8357 (patched)


can we get rid of the process:: prefix?



src/tests/slave_tests.cpp
Lines 8372 (patched)


Do we need the `mesos::` prefix?


- Jie Yu


On Sept. 15, 2017, 1:15 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61183/
> ---
> 
> (Updated Sept. 15, 2017, 1:15 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The agent's resource provider manager sends a
> 'ResourceProviderMessage' when its managed resources change. This
> commit adds handling in the agent so that an 'UpdateSlaveMessage' is
> sent to the master to update the total resource available on the
> agent. We also store this total in the agent memory so that it can be
> resent on agent resubscription.
> 
> In order to provide push-like handling of the resource provider
> manager's message queue, we chain recursive calls to the handler for
> continuous processing. Initially, processing is kicked off from
> 'Slave::initialize'. In this simple implementation we e.g., provide no
> direct way to stop processing of messages, yet, but it can be achieved
> by e.g., replacing the manager with a new instance (this would also
> require updating routes).
> 
> Since the agent can only send an 'UpdateSlaveMessage' when it is
> registered with a master, a simple back-off of 5 s is implemented which
> will defer processing of a ready message should the agent not yet have
> registered.
> 
> To facilitate logging we add a stringification function for
> 'ResourceProviderMessage's.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/message.hpp 3c7c3f2baeb726e04edd6ffbb9784699d7afe521 
>   src/slave/slave.hpp 7d07868451e93d34ba694d40216c1e4036fd4094 
>   src/slave/slave.cpp 6d1516a5d5b5db684f79385e60d892ff75fd00fd 
>   src/tests/slave_tests.cpp 0a578ff0cf264999c95db8ccd16e6e52807fa070 
> 
> 
> Diff: https://reviews.apache.org/r/61183/diff/8/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 62037: Added logging::initialize to main functions that use glog.

2017-09-18 Thread Alexander Rukletsov

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


Ship it!




I'll fix the outstanding issues and commit this for you.

- Alexander Rukletsov


On Sept. 18, 2017, 2:27 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62037/
> ---
> 
> (Updated Sept. 18, 2017, 2:27 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-7586
> https://issues.apache.org/jira/browse/MESOS-7586
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change reduces the number of messages "WARNING: Logging
> before InitGoogleLogging() is written to STDERR".
> 
> 
> Diffs
> -
> 
>   src/checks/tcp_connect.cpp 3cb9077b9f39f69fdc152280e5b180b88e772648 
>   src/cli/execute.cpp e844fa46a43bdfb08246be929b7913a7997f18df 
>   src/cli/resolve.cpp b3cba87f61dbff0e99291775a4d0908bb35ce811 
>   src/examples/balloon_framework.cpp a63dbab2b478b28738940114bd157a56b49d473b 
>   src/examples/disk_full_framework.cpp 
> 2572c72ded86d4befc1dc137e264dfe6ff64a357 
>   src/examples/dynamic_reservation_framework.cpp 
> bb6f58ba8fb87ae37d4ae971c192a204d5a656c5 
>   src/examples/long_lived_framework.cpp 
> da632040ece591717a38b44158b3eedd9f0ae7c4 
>   src/examples/no_executor_framework.cpp 
> 2ca240b4c998f0e33e7b09dbad9f4a7bfa0583cc 
>   src/examples/test_framework.cpp a6b38f02cc29682e3d3e1adf83baea061feda9e7 
>   src/examples/test_http_framework.cpp 
> 693dd47694678e7439f9d4ab0ff77b93950edf6e 
>   src/launcher/fetcher.cpp 42980f5a4a40b72f754156469e9fe60a952d1d87 
>   src/slave/container_loggers/logrotate.cpp 
> 61484b18f4615e85925b26d999afb5ac3b7e32a5 
>   src/slave/containerizer/mesos/io/switchboard_main.cpp 
> fa0b29667258b6b6fb7381d8226b6dda0062629e 
>   src/tests/main.cpp a7dc99b90f84a1a1ea299c937fbfc84e55adad1c 
>   src/usage/main.cpp 5ad4a3ba7959e4a9b3fa05f5a2f49b02f94a0cf0 
> 
> 
> Diff: https://reviews.apache.org/r/62037/diff/6/
> 
> 
> Testing
> ---
> 
> ```
> make check
> ```
> 
> Had an issue with ExamplesTest.DiskFullFramework that is not related.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>



Review Request 62391: Fixed invalid handle bug in `os::process()`.

2017-09-18 Thread Andrew Schwartzmeyer

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

Review request for mesos, James Peach and Joseph Wu.


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


Repository: mesos


Description
---

This patch fixes a bug found by running `mesos-tests` under Application
Verifier on Windows. Mesos was inadvertently attempting to get a process
handle for the System Idle Process with PID `0`, which is not permitted
by the OS. To remedy this, we now check if `os::process` receives `0`
for its argument, and return an error if so. Furthermore, we remove the
PID `0` from the `os::pids` API, as it is not useful to the programmer,
and only serves to cause errors later. Finally, the return value for
`OpenProcess` was being incorrectly checked, as the API returns a
`nullptr` on failure, not `INVALID_HANDLE_VALUE`.


Diffs
-

  3rdparty/stout/include/stout/windows/os.hpp 
f35bf312a25066a167dc66243ad9bf8333cb36a6 


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


Testing
---

Built and ran `mesos-tests` on Windows under Application Verifier. All tests 
pass, and the error from `OpenProcess` is no longer being returned.


Thanks,

Andrew Schwartzmeyer



Re: Review Request 62288: Added Log.Reader.catchup() method to Java bindings.

2017-09-18 Thread Mesos Reviewbot

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



Patch looks great!

Reviews applied: [62283, 62284, 62285, 62286, 62287, 62288]

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

- Mesos Reviewbot


On Sept. 13, 2017, 4:40 p.m., Ilya Pronin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62288/
> ---
> 
> (Updated Sept. 13, 2017, 4:40 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-7973
> https://issues.apache.org/jira/browse/MESOS-7973
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The new method is a wrapper for Log::Reader::catchup() method of the C++
> Replicated Log API.
> 
> 
> Diffs
> -
> 
>   src/java/jni/org_apache_mesos_Log.cpp 
> eb0f54be39dfced0dd75dcefa808850f22a34dc6 
>   src/java/src/org/apache/mesos/Log.java 
> f53d9ebeed3fc30e9e9dc642320e94da18eb8ca1 
> 
> 
> Diff: https://reviews.apache.org/r/62288/diff/1/
> 
> 
> Testing
> ---
> 
> Ran `make check`.
> 
> 
> Thanks,
> 
> Ilya Pronin
> 
>



Re: Review Request 62381: Removed `docker exec` when performing health checks in docker executor.

2017-09-18 Thread Mesos Reviewbot Windows

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



FAIL: Some Mesos tests failed.

Reviews applied: `['62381']`

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

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

Relevant logs:

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

```
[ RUN  ] ContentType/SchedulerTest.SchedulerReconnect/0
[   OK ] ContentType/SchedulerTest.SchedulerReconnect/0 (239 ms)
[ RUN  ] ContentType/SchedulerTest.SchedulerReconnect/1
[   OK ] ContentType/SchedulerTest.SchedulerReconnect/1 (257 ms)
[--] 30 tests from ContentType/SchedulerTest (25468 ms total)

[--] 2 tests from ContentTypeAndSSLConfig/SchedulerSSLTest
[ RUN  ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/0
[   OK ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/0 (952 
ms)
[ RUN  ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/1
[   OK ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/1 
(1018 ms)
[--] 2 tests from ContentTypeAndSSLConfig/SchedulerSSLTest (2049 ms 
total)

[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0 (144 
ms)
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1 (154 
ms)
[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest (333 ms 
total)

[--] Global test environment tear-down
[==] 627 tests from 66 test cases ran. (346444 ms total)
[  PASSED  ] 626 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] ContentType/MasterAPITest.EventAuthorizationFiltering/1, where 
GetParam() = application/json

 1 FAILED TEST
  YOU HAVE 174 DISABLED TESTS

```

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

```
I0918 20:15:15.129381 20048 master.cpp:8418] Removing framework 
1d91d741-8216-4725-96b3-9211950d051a- (default)
I0918 20:15:15.130378 20048 master.cpp:3267] Deactivating framework 
1d91d741-8216-4725-96b3-9211950d051a- (default)
I0918 20:15:15.130378 19020 hierarchical.cpp:412] Deactivated framework 
1d91d741-8216-4725-96b3-9211950d051a-
I0918 20:15:15.130378 17216 slave.cpp:3235] Shutting down framework 
1d91d741-8216-4725-96b3-9211950d051a-
I0918 20:15:15.130378 20048 master.cpp:8993] Updating the state of task 
e2e6acbd-c12d-4aaf-b75a-ab9fcb6cb2d0 of framework 
1d91d741-8216-4725-96b3-9211950d051a- (latest state: TASK_KILLED, status 
update state: TASK_KILLED)
I0918 20:15:15.131378 17216 slave.cpp:5731] Shutting down executor 'default' of 
framework 1d91d741-8216-4725-96b3-9211950d051a- (via HTTP)
I0918 20:15:15.139425 20048 master.cpp:9087] Removing task 
e2e6acbd-c12d-4aaf-b75a-ab9fcb6cb2d0 with resources 
[{"allocation_info":{"role":"*"},"name":"cpus","scalar":{"value":2.0},"type":"SCALAR"},{"allocation_info":{"role":"*"},"name":"mem","scalar":{"value":1024.0},"type":"SCALAR"},{"allocation_info":{"role":"*"},"name":"disk","scalar":{"value":1024.0},"type":"SCALAR"},{"allocation_info":{"role":"*"},"name":"ports","ranges":{"range":[{"begin":31000,"end":32000}]},"type":"RANGES"}]
 of framework 1d91d741-8216-4725-96b3-9211950d051a- on agent 
1d91d741-8216-4725-96b3-9211950d051a-S0 at slave(254)@10.3.1.5:51096 
(mesos-bld-s1.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0918 20:15:15.158382 20048 master.cpp:9116] Removing executor 'default' with 
resources [] of framework 1d91d741-8216-4725-96b3-9211950d051a- on agent 
1d91d741-8216-4725-96b3-9211950d051a-S0 at slave(254)@10.3.1.5:51096 
(mesos-bld-s1.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0918 20:15:15.161382 19988 hierarchical.cpp:355] Removed framework 
1d91d741-8216-4725-96b3-9211950d051a-
E0918 20:15:15.162381 19692 scheduler.cpp:649] End-Of-File received from 
master. The master closed the event stream
I0918 20:15:15.172384 19692 scheduler.cpp:444] Re-detecting master
I0918 20:15:15.174382 19020 scheduler.cpp:470] New master detected at 
master@10.3.1.5:51096
I0918 20:15:15.188383 19988 slave.cpp:5407] Executor 'default' of framework 
1d91d741-8216-4725-96b3-9211950d051a- exited with status 0
I0918 20:15:15.188383 19988 slave.cpp:5511] Cleaning up executor 'default' of 
framework 1d91d741-8216-4725-96b3-9211950d051a- (via HTTP)
W0918 20:15:15.188383 19692 master.cpp:7021] Ignoring unknown exited executor 
'default' of framework 1d91d741-8216-4725-96b3-9211950d051a- on agent 

Re: Review Request 62176: Added cmake dependency check for libsasl2 on non-Windows platforms.

2017-09-18 Thread Andrew Schwartzmeyer


> On Sept. 8, 2017, 4:40 p.m., Mesos Reviewbot Windows wrote:
> > FAIL: Mesos tests failed to build. Please check 
> > http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62176/logs/mesos-tests-cmake-build.log
> >  for any relevant errors
> > 
> > Reviews applied: [62105, 62106, 62176]
> > 
> > Logs available here: 
> > http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62176/logs

This fails because the bundle hasn't been committed to our 3rdparty repo yet, 
which is fine. It just means we need to test manually, and make sure the PR 
gets merged.


- Andrew


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


On Sept. 8, 2017, 3:35 p.m., John Kordich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62176/
> ---
> 
> (Updated Sept. 8, 2017, 3:35 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Jeff Coffler, and Joseph Wu.
> 
> 
> Bugs: MESOS-3110
> https://issues.apache.org/jira/browse/MESOS-3110
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added cmake dependency check for libsasl2 on non-Windows platforms.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 2a183a4a46bf8bc97455fe02648407ba561b38e7 
> 
> 
> Diff: https://reviews.apache.org/r/62176/diff/2/
> 
> 
> Testing
> ---
> 
> I tested this on my Ubuntu 16.04 system. When the libsasl2 library doesn't 
> exist, it fails the cmake configure/build.  I did not test this on any other 
> platform, but this code is in a "if (NOT WIN32)" block and won't affect a 
> Windows build.  I'm uncertain if there is much support for other kinds of 
> builds (like Mac OS), but this should be platform independent.
> 
> 
> Thanks,
> 
> John Kordich
> 
>



Re: Review Request 62037: Added logging::initialize to main functions that use glog.

2017-09-18 Thread Alexander Rukletsov


> On Sept. 18, 2017, 4:18 p.m., Andrei Budnik wrote:
> > src/examples/long_lived_framework.cpp
> > Lines 578 (patched)
> > 
> >
> > Extra newline?
> 
> Alexander Rukletsov wrote:
> This seems to be consistent to other frameworks.

Okay, after looking at this carefully it seems to be inconsistent in general. 
I'll fix it in a separate patch.


- Alexander


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


On Sept. 18, 2017, 2:27 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62037/
> ---
> 
> (Updated Sept. 18, 2017, 2:27 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-7586
> https://issues.apache.org/jira/browse/MESOS-7586
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change reduces the number of messages "WARNING: Logging
> before InitGoogleLogging() is written to STDERR".
> 
> 
> Diffs
> -
> 
>   src/checks/tcp_connect.cpp 3cb9077b9f39f69fdc152280e5b180b88e772648 
>   src/cli/execute.cpp e844fa46a43bdfb08246be929b7913a7997f18df 
>   src/cli/resolve.cpp b3cba87f61dbff0e99291775a4d0908bb35ce811 
>   src/examples/balloon_framework.cpp a63dbab2b478b28738940114bd157a56b49d473b 
>   src/examples/disk_full_framework.cpp 
> 2572c72ded86d4befc1dc137e264dfe6ff64a357 
>   src/examples/dynamic_reservation_framework.cpp 
> bb6f58ba8fb87ae37d4ae971c192a204d5a656c5 
>   src/examples/long_lived_framework.cpp 
> da632040ece591717a38b44158b3eedd9f0ae7c4 
>   src/examples/no_executor_framework.cpp 
> 2ca240b4c998f0e33e7b09dbad9f4a7bfa0583cc 
>   src/examples/test_framework.cpp a6b38f02cc29682e3d3e1adf83baea061feda9e7 
>   src/examples/test_http_framework.cpp 
> 693dd47694678e7439f9d4ab0ff77b93950edf6e 
>   src/launcher/fetcher.cpp 42980f5a4a40b72f754156469e9fe60a952d1d87 
>   src/slave/container_loggers/logrotate.cpp 
> 61484b18f4615e85925b26d999afb5ac3b7e32a5 
>   src/slave/containerizer/mesos/io/switchboard_main.cpp 
> fa0b29667258b6b6fb7381d8226b6dda0062629e 
>   src/tests/main.cpp a7dc99b90f84a1a1ea299c937fbfc84e55adad1c 
>   src/usage/main.cpp 5ad4a3ba7959e4a9b3fa05f5a2f49b02f94a0cf0 
> 
> 
> Diff: https://reviews.apache.org/r/62037/diff/6/
> 
> 
> Testing
> ---
> 
> ```
> make check
> ```
> 
> Had an issue with ExamplesTest.DiskFullFramework that is not related.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>



Re: Review Request 62176: Added cmake dependency check for libsasl2 on non-Windows platforms.

2017-09-18 Thread Andrew Schwartzmeyer


> On Sept. 18, 2017, 12:45 p.m., Andrew Schwartzmeyer wrote:
> > Ship It!

(But wait on required reviews; i.e. this isn't for the whole chain.)


- Andrew


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


On Sept. 8, 2017, 3:35 p.m., John Kordich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62176/
> ---
> 
> (Updated Sept. 8, 2017, 3:35 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Jeff Coffler, and Joseph Wu.
> 
> 
> Bugs: MESOS-3110
> https://issues.apache.org/jira/browse/MESOS-3110
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added cmake dependency check for libsasl2 on non-Windows platforms.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 2a183a4a46bf8bc97455fe02648407ba561b38e7 
> 
> 
> Diff: https://reviews.apache.org/r/62176/diff/2/
> 
> 
> Testing
> ---
> 
> I tested this on my Ubuntu 16.04 system. When the libsasl2 library doesn't 
> exist, it fails the cmake configure/build.  I did not test this on any other 
> platform, but this code is in a "if (NOT WIN32)" block and won't affect a 
> Windows build.  I'm uncertain if there is much support for other kinds of 
> builds (like Mac OS), but this should be platform independent.
> 
> 
> Thanks,
> 
> John Kordich
> 
>



Re: Review Request 62176: Added cmake dependency check for libsasl2 on non-Windows platforms.

2017-09-18 Thread Andrew Schwartzmeyer

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


Ship it!




Ship It!

- Andrew Schwartzmeyer


On Sept. 8, 2017, 3:35 p.m., John Kordich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62176/
> ---
> 
> (Updated Sept. 8, 2017, 3:35 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Jeff Coffler, and Joseph Wu.
> 
> 
> Bugs: MESOS-3110
> https://issues.apache.org/jira/browse/MESOS-3110
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added cmake dependency check for libsasl2 on non-Windows platforms.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 2a183a4a46bf8bc97455fe02648407ba561b38e7 
> 
> 
> Diff: https://reviews.apache.org/r/62176/diff/2/
> 
> 
> Testing
> ---
> 
> I tested this on my Ubuntu 16.04 system. When the libsasl2 library doesn't 
> exist, it fails the cmake configure/build.  I did not test this on any other 
> platform, but this code is in a "if (NOT WIN32)" block and won't affect a 
> Windows build.  I'm uncertain if there is much support for other kinds of 
> builds (like Mac OS), but this should be platform independent.
> 
> 
> Thanks,
> 
> John Kordich
> 
>



Re: Review Request 62106: Enabled CRAM MD5 Authentication on Windows and associated tests.

2017-09-18 Thread Andrew Schwartzmeyer


> On Sept. 18, 2017, 12:42 p.m., Andrew Schwartzmeyer wrote:
> > src/master/master.cpp
> > Lines 494-605 (original), 494-587 (patched)
> > 
> >
> > I believe this to be fine, and I briefly checked with Jie (I think) at 
> > MesosCon about this, but I'd like to make sure we get one more reviewer to 
> > make 100% sure we can remove the `HAS_AUTHENTICATION` definition. I'll get 
> > them added...

I can't edit your review. John, would you add Jie Yu as a reviewer?


- Andrew


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


On Sept. 8, 2017, 3:13 p.m., John Kordich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62106/
> ---
> 
> (Updated Sept. 8, 2017, 3:13 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Jeff Coffler, and Joseph Wu.
> 
> 
> Bugs: MESOS-3384
> https://issues.apache.org/jira/browse/MESOS-3384
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled CRAM MD5 Authentication on Windows and associated tests.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake 4cb8dddf93694722af2377c4a55a195260c8a0cb 
>   docs/configuration-cmake.md d2eb571c9458c7fade415e8077c676dd34e63242 
>   docs/windows.md b47ba5a0db3d41d6f4bb002283b40622a2bb3e0f 
>   src/CMakeLists.txt 1a828c4351ded36f51ccbbe67147da2f50b9cdb1 
>   src/Makefile.am a4f2ee008eba263fe52c4e58c305e3fb9ba42f6b 
>   src/authentication/cram_md5/authenticatee.cpp 
> 7b3f767e29163de489018081089e67a6f22971c5 
>   src/authentication/cram_md5/authenticator.cpp 
> 2a12bb3f4aec79056ec6c82a8c896ee3af15bb4c 
>   src/authentication/cram_md5/auxprop.hpp 
> dedcbe514283dd86a924d92ccdd17173ebe878cb 
>   src/master/master.cpp 14d94cfa0e96881f77dd6e0c3449fdc0613e36ce 
>   src/sched/sched.cpp ef73c1dccfd736b79f40a057951f022df7f60644 
>   src/slave/slave.cpp 6d1516a5d5b5db684f79385e60d892ff75fd00fd 
>   src/tests/CMakeLists.txt def233d13f627f58362fdc61ac3e9c104d65fe7e 
>   src/tests/authentication_tests.cpp b04be76d23788fbde337eaf85716ec7436ade691 
>   src/tests/mesos.cpp fc7f8cb09e223000ea160aa4ab83a003cf4af2fb 
>   support/windows-build.bat 100013ed68ecc2f431c20a3f081f8aa6eca961ef 
> 
> 
> Diff: https://reviews.apache.org/r/62106/diff/4/
> 
> 
> Testing
> ---
> 
> Ran the automake build and the cmake build on Linux, as well as the cmake 
> build on Windows, with no errors.  Ran tests on both platforms as well.  
> Note: The mesos-3rdparty pull request which contains the cyrus-sasl package 
> needs to be merged to master first before these patches will work. (Or, have 
> the cyrus-sasl tarball available in a local 3rdparty directory, configured 
> with mesos)
> 
> 
> Thanks,
> 
> John Kordich
> 
>



Re: Review Request 62106: Enabled CRAM MD5 Authentication on Windows and associated tests.

2017-09-18 Thread Andrew Schwartzmeyer

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




src/CMakeLists.txt
Line 617 (original), 613-614 (patched)


Just an ordering suggestion: would you move `sasl2` up above the 
conditional libraries, maybe between `process` and `zookeeper`?



src/Makefile.am
Line 234 (original)


This is what confirms (at least for me) that we've not been supporting 
disabling this flag for a long time.



src/authentication/cram_md5/authenticatee.cpp
Line 21 (original), 21-25 (patched)


For this and the below, should we see if we can get the SASL build to put 
the headers under `include/sasl` instead of just `include` so that all this 
code change can go away?



src/master/master.cpp
Lines 494-605 (original), 494-587 (patched)


I believe this to be fine, and I briefly checked with Jie (I think) at 
MesosCon about this, but I'd like to make sure we get one more reviewer to make 
100% sure we can remove the `HAS_AUTHENTICATION` definition. I'll get them 
added...


- Andrew Schwartzmeyer


On Sept. 8, 2017, 3:13 p.m., John Kordich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62106/
> ---
> 
> (Updated Sept. 8, 2017, 3:13 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Jeff Coffler, and Joseph Wu.
> 
> 
> Bugs: MESOS-3384
> https://issues.apache.org/jira/browse/MESOS-3384
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled CRAM MD5 Authentication on Windows and associated tests.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake 4cb8dddf93694722af2377c4a55a195260c8a0cb 
>   docs/configuration-cmake.md d2eb571c9458c7fade415e8077c676dd34e63242 
>   docs/windows.md b47ba5a0db3d41d6f4bb002283b40622a2bb3e0f 
>   src/CMakeLists.txt 1a828c4351ded36f51ccbbe67147da2f50b9cdb1 
>   src/Makefile.am a4f2ee008eba263fe52c4e58c305e3fb9ba42f6b 
>   src/authentication/cram_md5/authenticatee.cpp 
> 7b3f767e29163de489018081089e67a6f22971c5 
>   src/authentication/cram_md5/authenticator.cpp 
> 2a12bb3f4aec79056ec6c82a8c896ee3af15bb4c 
>   src/authentication/cram_md5/auxprop.hpp 
> dedcbe514283dd86a924d92ccdd17173ebe878cb 
>   src/master/master.cpp 14d94cfa0e96881f77dd6e0c3449fdc0613e36ce 
>   src/sched/sched.cpp ef73c1dccfd736b79f40a057951f022df7f60644 
>   src/slave/slave.cpp 6d1516a5d5b5db684f79385e60d892ff75fd00fd 
>   src/tests/CMakeLists.txt def233d13f627f58362fdc61ac3e9c104d65fe7e 
>   src/tests/authentication_tests.cpp b04be76d23788fbde337eaf85716ec7436ade691 
>   src/tests/mesos.cpp fc7f8cb09e223000ea160aa4ab83a003cf4af2fb 
>   support/windows-build.bat 100013ed68ecc2f431c20a3f081f8aa6eca961ef 
> 
> 
> Diff: https://reviews.apache.org/r/62106/diff/4/
> 
> 
> Testing
> ---
> 
> Ran the automake build and the cmake build on Linux, as well as the cmake 
> build on Windows, with no errors.  Ran tests on both platforms as well.  
> Note: The mesos-3rdparty pull request which contains the cyrus-sasl package 
> needs to be merged to master first before these patches will work. (Or, have 
> the cyrus-sasl tarball available in a local 3rdparty directory, configured 
> with mesos)
> 
> 
> Thanks,
> 
> John Kordich
> 
>



Re: Review Request 62037: Added logging::initialize to main functions that use glog.

2017-09-18 Thread Alexander Rukletsov

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




src/cli/execute.cpp
Line 1026 (original), 1021 (patched)


Two spaces : )



src/cli/execute.cpp
Lines 1076-1079 (original), 1069-1072 (patched)


This looks nicer:
```
EXIT(EXIT_FAILURE)
  << "Flags '--framework_capabilities' specifies an unknown"
  << " capability '" << capability << "'";
```



src/cli/execute.cpp
Lines 1083-1086 (original), 1076-1079 (patched)


Ditto.



src/examples/no_executor_framework.cpp
Line 312 (original), 319 (patched)


Why did you change it from
```
logging::initialize(argv[0], true, flags); // Catch signals.
```
?


- Alexander Rukletsov


On Sept. 18, 2017, 2:27 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62037/
> ---
> 
> (Updated Sept. 18, 2017, 2:27 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-7586
> https://issues.apache.org/jira/browse/MESOS-7586
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change reduces the number of messages "WARNING: Logging
> before InitGoogleLogging() is written to STDERR".
> 
> 
> Diffs
> -
> 
>   src/checks/tcp_connect.cpp 3cb9077b9f39f69fdc152280e5b180b88e772648 
>   src/cli/execute.cpp e844fa46a43bdfb08246be929b7913a7997f18df 
>   src/cli/resolve.cpp b3cba87f61dbff0e99291775a4d0908bb35ce811 
>   src/examples/balloon_framework.cpp a63dbab2b478b28738940114bd157a56b49d473b 
>   src/examples/disk_full_framework.cpp 
> 2572c72ded86d4befc1dc137e264dfe6ff64a357 
>   src/examples/dynamic_reservation_framework.cpp 
> bb6f58ba8fb87ae37d4ae971c192a204d5a656c5 
>   src/examples/long_lived_framework.cpp 
> da632040ece591717a38b44158b3eedd9f0ae7c4 
>   src/examples/no_executor_framework.cpp 
> 2ca240b4c998f0e33e7b09dbad9f4a7bfa0583cc 
>   src/examples/test_framework.cpp a6b38f02cc29682e3d3e1adf83baea061feda9e7 
>   src/examples/test_http_framework.cpp 
> 693dd47694678e7439f9d4ab0ff77b93950edf6e 
>   src/launcher/fetcher.cpp 42980f5a4a40b72f754156469e9fe60a952d1d87 
>   src/slave/container_loggers/logrotate.cpp 
> 61484b18f4615e85925b26d999afb5ac3b7e32a5 
>   src/slave/containerizer/mesos/io/switchboard_main.cpp 
> fa0b29667258b6b6fb7381d8226b6dda0062629e 
>   src/tests/main.cpp a7dc99b90f84a1a1ea299c937fbfc84e55adad1c 
>   src/usage/main.cpp 5ad4a3ba7959e4a9b3fa05f5a2f49b02f94a0cf0 
> 
> 
> Diff: https://reviews.apache.org/r/62037/diff/6/
> 
> 
> Testing
> ---
> 
> ```
> make check
> ```
> 
> Had an issue with ExamplesTest.DiskFullFramework that is not related.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>



Re: Review Request 62105: Enabled building of the Cyrus SASL CRAM MD5 lib on Windows.

2017-09-18 Thread Andrew Schwartzmeyer

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




3rdparty/CMakeLists.txt
Lines 189-213 (patched)


Formatting issues:

Since this is under a conditional, it all needs to be indented two spaces.

The call to `external` we ended up standardizing as `EXTERNAL` to 
distinguish our functions from CMake built-ins.

The `MAKE_INCLUDE_DIR` should come after `ExternalProject_Add` for 
consistency with the rest of the file.



3rdparty/CMakeLists.txt
Lines 192 (patched)


And this is just me being super nitpicky, but I was trying to keep the 
`` headers to the length of the comments above them, and I added a short 
project description after the the name so anyone new can easily know what the 
dependency is for. Final thing is that the link is to an FTP server, but I 
would recommend just linking to the homepage "https://cyrusimap.org/sasl/;.



3rdparty/CMakeLists.txt
Lines 201-202 (patched)


Since this is an unconditionally static library, the `IMPLIB` properties do 
not need to be defined (these are specifically for dealing with DLLs and their 
associated imports).



3rdparty/CMakeLists.txt
Lines 203-204 (patched)


Perfect!


- Andrew Schwartzmeyer


On Sept. 8, 2017, 3:13 p.m., John Kordich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62105/
> ---
> 
> (Updated Sept. 8, 2017, 3:13 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Jeff Coffler, and Joseph Wu.
> 
> 
> Bugs: MESOS-3384
> https://issues.apache.org/jira/browse/MESOS-3384
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled building of the Cyrus SASL CRAM MD5 lib on Windows.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 2a183a4a46bf8bc97455fe02648407ba561b38e7 
>   3rdparty/cmake/Versions.cmake 0b62ae96817ade616f8dfc52cf8df995f8fd8927 
>   3rdparty/cyrus-sasl-2.1.27rc3.patch PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/62105/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> John Kordich
> 
>



Re: Review Request 62357: Updated logging initialization arguments.

2017-09-18 Thread Alexander Rukletsov

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


Ship it!




Ship It!

- Alexander Rukletsov


On Sept. 18, 2017, 6:33 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62357/
> ---
> 
> (Updated Sept. 18, 2017, 6:33 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-7586
> https://issues.apache.org/jira/browse/MESOS-7586
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The arguments `flags` and `installFailureSignalHandler` are now
> swapped and `flags` is optional. The will allow the logging
> initialization to be more broadly used in a future patch.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp e9949f652cd8527991ebfdfbf14e68b4c958fe79 
>   src/examples/load_generator_framework.cpp 
> 653ae815df2009613b8f89308ab1b5f19757825a 
>   src/examples/no_executor_framework.cpp 
> 2ca240b4c998f0e33e7b09dbad9f4a7bfa0583cc 
>   src/examples/persistent_volume_framework.cpp 
> bce2a2d1c73a3a1efab6d107529d9f6cfe9854da 
>   src/examples/test_framework.cpp a6b38f02cc29682e3d3e1adf83baea061feda9e7 
>   src/examples/test_http_framework.cpp 
> 693dd47694678e7439f9d4ab0ff77b93950edf6e 
>   src/exec/exec.cpp 65c4575f8d44a1f4d4e99350b770eb8b921c0644 
>   src/executor/executor.cpp 91bbd1e7651754620324e2384b7becbad9240a26 
>   src/launcher/default_executor.cpp 106b7f2e0244d211c66b237b5d1c51f43fc6e529 
>   src/launcher/executor.cpp 951597b576b4912541dd87d52dcb981393e58082 
>   src/launcher/fetcher.cpp 42980f5a4a40b72f754156469e9fe60a952d1d87 
>   src/local/main.cpp f6145b71c7ff7b454a63fd1f1865dbf7bf7345d2 
>   src/log/tool/benchmark.cpp 8264fdaa3316175cdb305202fd7b4968e896b968 
>   src/log/tool/initialize.cpp 6a12505119bebd4c5cfbe00aeb45a51ad9d62bec 
>   src/log/tool/read.cpp 7b308986012fc14d5f01c88d2d09ceb45ad5db75 
>   src/log/tool/replica.cpp 45bd1f4bc54cf4c84af207c977fb78b23f042873 
>   src/logging/logging.hpp 0e448f25de6c66d772f5bc481fbaf4aa493fd8af 
>   src/logging/logging.cpp 70d66a5c396f709e8f27ad0d51315ed6d257f73b 
>   src/master/main.cpp 7e815bf89e018921757b19dc8e16e2d5139248ee 
>   src/sched/sched.cpp ef73c1dccfd736b79f40a057951f022df7f60644 
>   src/scheduler/scheduler.cpp ce69258027ed50867569374d2d827fc3cc651744 
>   src/slave/main.cpp d53ff5013e00421ef1e13593584bd132a59738fb 
>   src/tests/main.cpp a7dc99b90f84a1a1ea299c937fbfc84e55adad1c 
> 
> 
> Diff: https://reviews.apache.org/r/62357/diff/2/
> 
> 
> Testing
> ---
> 
> ```
> make check
> ```
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>



Re: Review Request 62018: Updated logging::initialize to make flags optional.

2017-09-18 Thread Alexander Rukletsov

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


Ship it!




Ship It!

- Alexander Rukletsov


On Sept. 18, 2017, 1:05 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62018/
> ---
> 
> (Updated Sept. 18, 2017, 1:05 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-7586
> https://issues.apache.org/jira/browse/MESOS-7586
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This will be used to initialize logging in the main functions
> that do not use `mesos::internal::logging::Flags`.
> 
> 
> Diffs
> -
> 
>   src/logging/logging.hpp 0e448f25de6c66d772f5bc481fbaf4aa493fd8af 
>   src/logging/logging.cpp 70d66a5c396f709e8f27ad0d51315ed6d257f73b 
> 
> 
> Diff: https://reviews.apache.org/r/62018/diff/4/
> 
> 
> Testing
> ---
> 
> ```
> $ make check
> ```
> 
> Also tested manually to see if using `LOG()` without initializing GLOG gives 
> the same result as using it after running 
> `mesos::internal::logging::initialize` without flags.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>



Re: Review Request 62122: Fixed parameter name in uri/fetcher.hpp.

2017-09-18 Thread Alexander Rukletsov

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


Ship it!




Ship It!

- Alexander Rukletsov


On Sept. 6, 2017, 2:43 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62122/
> ---
> 
> (Updated Sept. 6, 2017, 2:43 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The parameter used in uri/fetcher.cpp is named '_flags', it is
> also now the case in uri/fetcher.hpp.
> 
> 
> Diffs
> -
> 
>   src/uri/fetcher.hpp 7b910108adf877a24203e347c8ff8eed07d48483 
> 
> 
> Diff: https://reviews.apache.org/r/62122/diff/1/
> 
> 
> Testing
> ---
> 
> ```
> $ make check
> ```
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>



Re: Review Request 62037: Added logging::initialize to main functions that use glog.

2017-09-18 Thread Alexander Rukletsov


> On Sept. 18, 2017, 4:18 p.m., Andrei Budnik wrote:
> > src/examples/long_lived_framework.cpp
> > Lines 578 (patched)
> > 
> >
> > Extra newline?

This seems to be consistent to other frameworks.


> On Sept. 18, 2017, 4:18 p.m., Andrei Budnik wrote:
> > src/examples/no_executor_framework.cpp
> > Lines 311 (patched)
> > 
> >
> > Is extra `else` statement necessary here?

Does not seem so.


- Alexander


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


On Sept. 18, 2017, 2:27 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62037/
> ---
> 
> (Updated Sept. 18, 2017, 2:27 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-7586
> https://issues.apache.org/jira/browse/MESOS-7586
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change reduces the number of messages "WARNING: Logging
> before InitGoogleLogging() is written to STDERR".
> 
> 
> Diffs
> -
> 
>   src/checks/tcp_connect.cpp 3cb9077b9f39f69fdc152280e5b180b88e772648 
>   src/cli/execute.cpp e844fa46a43bdfb08246be929b7913a7997f18df 
>   src/cli/resolve.cpp b3cba87f61dbff0e99291775a4d0908bb35ce811 
>   src/examples/balloon_framework.cpp a63dbab2b478b28738940114bd157a56b49d473b 
>   src/examples/disk_full_framework.cpp 
> 2572c72ded86d4befc1dc137e264dfe6ff64a357 
>   src/examples/dynamic_reservation_framework.cpp 
> bb6f58ba8fb87ae37d4ae971c192a204d5a656c5 
>   src/examples/long_lived_framework.cpp 
> da632040ece591717a38b44158b3eedd9f0ae7c4 
>   src/examples/no_executor_framework.cpp 
> 2ca240b4c998f0e33e7b09dbad9f4a7bfa0583cc 
>   src/examples/test_framework.cpp a6b38f02cc29682e3d3e1adf83baea061feda9e7 
>   src/examples/test_http_framework.cpp 
> 693dd47694678e7439f9d4ab0ff77b93950edf6e 
>   src/launcher/fetcher.cpp 42980f5a4a40b72f754156469e9fe60a952d1d87 
>   src/slave/container_loggers/logrotate.cpp 
> 61484b18f4615e85925b26d999afb5ac3b7e32a5 
>   src/slave/containerizer/mesos/io/switchboard_main.cpp 
> fa0b29667258b6b6fb7381d8226b6dda0062629e 
>   src/tests/main.cpp a7dc99b90f84a1a1ea299c937fbfc84e55adad1c 
>   src/usage/main.cpp 5ad4a3ba7959e4a9b3fa05f5a2f49b02f94a0cf0 
> 
> 
> Diff: https://reviews.apache.org/r/62037/diff/6/
> 
> 
> Testing
> ---
> 
> ```
> make check
> ```
> 
> Had an issue with ExamplesTest.DiskFullFramework that is not related.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>



Re: Review Request 62365: Added the missing isolators to the doc.

2017-09-18 Thread Gilbert Song

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


Ship it!




Ship It!

- Gilbert Song


On Sept. 15, 2017, 6:10 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62365/
> ---
> 
> (Updated Sept. 15, 2017, 6:10 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and James Peach.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added the missing isolators to the doc.
> 
> 
> Diffs
> -
> 
>   docs/mesos-containerizer.md feaaa556c9646435c6c9d677569962576ebf4c74 
> 
> 
> Diff: https://reviews.apache.org/r/62365/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 62308: Added the description for the isolators section.

2017-09-18 Thread Gilbert Song

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


Ship it!




Ship It!

- Gilbert Song


On Sept. 15, 2017, 6:49 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62308/
> ---
> 
> (Updated Sept. 15, 2017, 6:49 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and James Peach.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added the description for the isolators section.
> 
> 
> Diffs
> -
> 
>   docs/mesos-containerizer.md feaaa556c9646435c6c9d677569962576ebf4c74 
> 
> 
> Diff: https://reviews.apache.org/r/62308/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 62307: Added gpu/nvidia isolator to the index.

2017-09-18 Thread Gilbert Song

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


Ship it!




Ship It!

- Gilbert Song


On Sept. 13, 2017, 1:51 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62307/
> ---
> 
> (Updated Sept. 13, 2017, 1:51 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and James Peach.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added gpu/nvidia isolator to the index.
> 
> 
> Diffs
> -
> 
>   docs/mesos-containerizer.md feaaa556c9646435c6c9d677569962576ebf4c74 
> 
> 
> Diff: https://reviews.apache.org/r/62307/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 62306: Added an index in the mesos containerizer doc about isolators.

2017-09-18 Thread Gilbert Song

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


Ship it!




Ship It!

- Gilbert Song


On Sept. 13, 2017, 1:51 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62306/
> ---
> 
> (Updated Sept. 13, 2017, 1:51 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and James Peach.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added an index in the mesos containerizer doc about isolators.
> 
> 
> Diffs
> -
> 
>   docs/mesos-containerizer.md feaaa556c9646435c6c9d677569962576ebf4c74 
> 
> 
> Diff: https://reviews.apache.org/r/62306/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 62304: Moved linux capabilities isolator doc to the isolators folder.

2017-09-18 Thread Gilbert Song

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


Ship it!




Ship It!

- Gilbert Song


On Sept. 13, 2017, 1:51 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62304/
> ---
> 
> (Updated Sept. 13, 2017, 1:51 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and James Peach.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Moved linux capabilities isolator doc to the isolators folder.
> 
> 
> Diffs
> -
> 
>   docs/container-image.md 5e680ce0bdc82a1e33646fd4773ec14f364c9036 
>   docs/linux_capabilities.md d45c66cf3be12420d779ad44da7deaab7f6f9363 
>   docs/mesos-containerizer.md feaaa556c9646435c6c9d677569962576ebf4c74 
> 
> 
> Diff: https://reviews.apache.org/r/62304/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 62305: Moved posix rlimits isolator doc to the isolators folder.

2017-09-18 Thread Gilbert Song

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


Ship it!




Ship It!

- Gilbert Song


On Sept. 13, 2017, 1:51 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62305/
> ---
> 
> (Updated Sept. 13, 2017, 1:51 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and James Peach.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Moved posix rlimits isolator doc to the isolators folder.
> 
> 
> Diffs
> -
> 
>   docs/mesos-containerizer.md feaaa556c9646435c6c9d677569962576ebf4c74 
>   docs/posix_rlimits.md c2d1520f32b0ee55d66cc239f0c26a1b5b6c9dc9 
> 
> 
> Diff: https://reviews.apache.org/r/62305/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 62303: Moved IPC namespace isolator doc to the isolators folder.

2017-09-18 Thread Gilbert Song

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


Ship it!




Ship It!

- Gilbert Song


On Sept. 13, 2017, 1:50 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62303/
> ---
> 
> (Updated Sept. 13, 2017, 1:50 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and James Peach.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Moved IPC namespace isolator doc to the isolators folder.
> 
> 
> Diffs
> -
> 
>   docs/isolators/namespaces-ipc.md PRE-CREATION 
>   docs/mesos-containerizer.md feaaa556c9646435c6c9d677569962576ebf4c74 
> 
> 
> Diff: https://reviews.apache.org/r/62303/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 62302: Moved docker volume isolator doc to the isolators folder.

2017-09-18 Thread Gilbert Song

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


Ship it!




Ship It!

- Gilbert Song


On Sept. 15, 2017, 6:46 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62302/
> ---
> 
> (Updated Sept. 15, 2017, 6:46 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and James Peach.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Moved docker volume isolator doc to the isolators folder.
> 
> 
> Diffs
> -
> 
>   docs/docker-volume.md a8cb099e9c586a57793e0e40616165f710204d7c 
>   docs/home.md dcb6235fdb12593fe8e9416c862ee547c0d77242 
>   docs/mesos-containerizer.md feaaa556c9646435c6c9d677569962576ebf4c74 
> 
> 
> Diff: https://reviews.apache.org/r/62302/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 62357: Updated logging initialization arguments.

2017-09-18 Thread Armand Grillet

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

(Updated Sept. 18, 2017, 6:33 p.m.)


Review request for mesos, Andrei Budnik and Alexander Rukletsov.


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


Repository: mesos


Description
---

The arguments `flags` and `installFailureSignalHandler` are now
swapped and `flags` is optional. The will allow the logging
initialization to be more broadly used in a future patch.


Diffs
-

  src/docker/executor.cpp e9949f652cd8527991ebfdfbf14e68b4c958fe79 
  src/examples/load_generator_framework.cpp 
653ae815df2009613b8f89308ab1b5f19757825a 
  src/examples/no_executor_framework.cpp 
2ca240b4c998f0e33e7b09dbad9f4a7bfa0583cc 
  src/examples/persistent_volume_framework.cpp 
bce2a2d1c73a3a1efab6d107529d9f6cfe9854da 
  src/examples/test_framework.cpp a6b38f02cc29682e3d3e1adf83baea061feda9e7 
  src/examples/test_http_framework.cpp 693dd47694678e7439f9d4ab0ff77b93950edf6e 
  src/exec/exec.cpp 65c4575f8d44a1f4d4e99350b770eb8b921c0644 
  src/executor/executor.cpp 91bbd1e7651754620324e2384b7becbad9240a26 
  src/launcher/default_executor.cpp 106b7f2e0244d211c66b237b5d1c51f43fc6e529 
  src/launcher/executor.cpp 951597b576b4912541dd87d52dcb981393e58082 
  src/launcher/fetcher.cpp 42980f5a4a40b72f754156469e9fe60a952d1d87 
  src/local/main.cpp f6145b71c7ff7b454a63fd1f1865dbf7bf7345d2 
  src/log/tool/benchmark.cpp 8264fdaa3316175cdb305202fd7b4968e896b968 
  src/log/tool/initialize.cpp 6a12505119bebd4c5cfbe00aeb45a51ad9d62bec 
  src/log/tool/read.cpp 7b308986012fc14d5f01c88d2d09ceb45ad5db75 
  src/log/tool/replica.cpp 45bd1f4bc54cf4c84af207c977fb78b23f042873 
  src/logging/logging.hpp 0e448f25de6c66d772f5bc481fbaf4aa493fd8af 
  src/logging/logging.cpp 70d66a5c396f709e8f27ad0d51315ed6d257f73b 
  src/master/main.cpp 7e815bf89e018921757b19dc8e16e2d5139248ee 
  src/sched/sched.cpp ef73c1dccfd736b79f40a057951f022df7f60644 
  src/scheduler/scheduler.cpp ce69258027ed50867569374d2d827fc3cc651744 
  src/slave/main.cpp d53ff5013e00421ef1e13593584bd132a59738fb 
  src/tests/main.cpp a7dc99b90f84a1a1ea299c937fbfc84e55adad1c 


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


Testing
---

```
make check
```


Thanks,

Armand Grillet



Re: Review Request 62301: Moved cgroups net_cls isolator doc to the isolators folder.

2017-09-18 Thread Gilbert Song

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


Ship it!




Ship It!

- Gilbert Song


On Sept. 15, 2017, 6:45 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62301/
> ---
> 
> (Updated Sept. 15, 2017, 6:45 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and James Peach.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Moved cgroups net_cls isolator doc to the isolators folder.
> 
> 
> Diffs
> -
> 
>   docs/isolators/cgroups-net-cls.md PRE-CREATION 
>   docs/mesos-containerizer.md feaaa556c9646435c6c9d677569962576ebf4c74 
> 
> 
> Diff: https://reviews.apache.org/r/62301/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 62300: Moved docker runtime isolator doc to the isolators folder.

2017-09-18 Thread Gilbert Song

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


Ship it!




Ship It!

- Gilbert Song


On Sept. 15, 2017, 6:44 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62300/
> ---
> 
> (Updated Sept. 15, 2017, 6:44 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and James Peach.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Moved docker runtime isolator doc to the isolators folder.
> 
> 
> Diffs
> -
> 
>   docs/isolators/docker-runtime.md PRE-CREATION 
>   docs/mesos-containerizer.md feaaa556c9646435c6c9d677569962576ebf4c74 
> 
> 
> Diff: https://reviews.apache.org/r/62300/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 62297: Moved pid namespace isolator doc to the isolators folder.

2017-09-18 Thread Gilbert Song

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


Ship it!




Ship It!

- Gilbert Song


On Sept. 15, 2017, 6:42 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62297/
> ---
> 
> (Updated Sept. 15, 2017, 6:42 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and James Peach.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Moved pid namespace isolator doc to the isolators folder.
> 
> 
> Diffs
> -
> 
>   docs/isolators/namespaces-pid.md PRE-CREATION 
>   docs/mesos-containerizer.md feaaa556c9646435c6c9d677569962576ebf4c74 
> 
> 
> Diff: https://reviews.apache.org/r/62297/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 62158: Rescinded offers possibly affected by updates to agent total resources.

2017-09-18 Thread Jie Yu

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


Fix it, then Ship it!





src/master/master.cpp
Lines 6779-6781 (original), 6787-6789 (patched)


Why change the formatting. I think the old style is better.



src/master/master.cpp
Lines 6797-6798 (patched)


Ditto on logging style. Please be consistent with others in this file.


- Jie Yu


On Sept. 15, 2017, 1:15 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62158/
> ---
> 
> (Updated Sept. 15, 2017, 1:15 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Bugs: MESOS-7757
> https://issues.apache.org/jira/browse/MESOS-7757
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When an agent changes its resources, the master should rescind any
> offers affected by the change. We already performed the rescind for
> updates to the agent's oversubscribed resources; this patch adds offer
> rescinding when an update an agent's total resources is processed.
> 
> While for updates to an agent's oversubscribed resources we currently
> only rescind offers containing revocable resources to e.g., reduce
> offer churn, we for updates to the total we here currently rescind all
> offers for resources on the agent.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 6d84a26bff970b842b58dfb69dbf232ba5c16a20 
> 
> 
> Diff: https://reviews.apache.org/r/62158/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 58048: Added 'id' and 'metadata' fields to 'Resource.DiskInfo.Source'.

2017-09-18 Thread Jie Yu

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


Fix it, then Ship it!





src/tests/resources_tests.cpp
Lines 2251 (patched)


s/ResourcesTest/DiskResourcesTest/

In fact, this test a bit dense and has a bunch of if/elses, making it hard 
to read.

Either split this into 3 tests (or 6 tests):
```
DiskResourcesTest.RawSource
DiskResourcesTest.MountBlockSource
DiskResourcesTest.PathSource

DiskResourcesTest.RawSourceWithID
DiskResourcesTest.MountBlockSourceWithID
DiskResourcesTest.PathSourceWithID
```

Or parameterize this test:
```
DiskResourcesSourceTest
```

Given that expectation is different for each source type, i am leaning 
towards option 1 (3 tests or 6 tests). And you don't need to print the param 
anymore.


- Jie Yu


On Sept. 13, 2017, 3:23 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58048/
> ---
> 
> (Updated Sept. 13, 2017, 3:23 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Bugs: MESOS-7312
> https://issues.apache.org/jira/browse/MESOS-7312
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ids will allow to create distinguishable resources, e.g., of RAW or
> BLOCK type. We also add a metadata field which can be used to expose
> additional disk information.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto fa475dcc3aa0b15ee4b2b740a24ab3c0b9a3274c 
>   include/mesos/v1/mesos.proto dfba4181203e23f7eedf67c19379d031e0993fd5 
>   src/common/resources.cpp 14b600ca1577be4910164396c75b866b53439ade 
>   src/tests/resources_tests.cpp 8a86efe60024d9e36294f0acb3f43bdd3f52f5bc 
>   src/v1/resources.cpp a5cc15591b7611274148139c43465adae44c1dbb 
> 
> 
> Diff: https://reviews.apache.org/r/58048/diff/6/
> 
> 
> Testing
> ---
> 
> make check (OS X, Fedora25)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 62037: Added logging::initialize to main functions that use glog.

2017-09-18 Thread Mesos Reviewbot Windows

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



PASS: Mesos patch 62037 was successfully built and tested.

Reviews applied: `['62357', '62037']`

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

- Mesos Reviewbot Windows


On Sept. 18, 2017, 2:27 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62037/
> ---
> 
> (Updated Sept. 18, 2017, 2:27 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-7586
> https://issues.apache.org/jira/browse/MESOS-7586
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change reduces the number of messages "WARNING: Logging
> before InitGoogleLogging() is written to STDERR".
> 
> 
> Diffs
> -
> 
>   src/checks/tcp_connect.cpp 3cb9077b9f39f69fdc152280e5b180b88e772648 
>   src/cli/execute.cpp e844fa46a43bdfb08246be929b7913a7997f18df 
>   src/cli/resolve.cpp b3cba87f61dbff0e99291775a4d0908bb35ce811 
>   src/examples/balloon_framework.cpp a63dbab2b478b28738940114bd157a56b49d473b 
>   src/examples/disk_full_framework.cpp 
> 2572c72ded86d4befc1dc137e264dfe6ff64a357 
>   src/examples/dynamic_reservation_framework.cpp 
> bb6f58ba8fb87ae37d4ae971c192a204d5a656c5 
>   src/examples/long_lived_framework.cpp 
> da632040ece591717a38b44158b3eedd9f0ae7c4 
>   src/examples/no_executor_framework.cpp 
> 2ca240b4c998f0e33e7b09dbad9f4a7bfa0583cc 
>   src/examples/test_framework.cpp a6b38f02cc29682e3d3e1adf83baea061feda9e7 
>   src/examples/test_http_framework.cpp 
> 693dd47694678e7439f9d4ab0ff77b93950edf6e 
>   src/launcher/fetcher.cpp 42980f5a4a40b72f754156469e9fe60a952d1d87 
>   src/slave/container_loggers/logrotate.cpp 
> 61484b18f4615e85925b26d999afb5ac3b7e32a5 
>   src/slave/containerizer/mesos/io/switchboard_main.cpp 
> fa0b29667258b6b6fb7381d8226b6dda0062629e 
>   src/tests/main.cpp a7dc99b90f84a1a1ea299c937fbfc84e55adad1c 
>   src/usage/main.cpp 5ad4a3ba7959e4a9b3fa05f5a2f49b02f94a0cf0 
> 
> 
> Diff: https://reviews.apache.org/r/62037/diff/6/
> 
> 
> Testing
> ---
> 
> ```
> make check
> ```
> 
> Had an issue with ExamplesTest.DiskFullFramework that is not related.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>



Re: Review Request 62357: Updated logging initialization arguments.

2017-09-18 Thread Andrei Budnik

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


Ship it!




Ship It!

- Andrei Budnik


On Sept. 18, 2017, 2:24 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62357/
> ---
> 
> (Updated Sept. 18, 2017, 2:24 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-7586
> https://issues.apache.org/jira/browse/MESOS-7586
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The arguments `flags` and `installFailureSignalHandler` are now
> swapped and `flags` is optional. The will allow the logging
> initialization to be more broadly used in a future patch.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp e9949f652cd8527991ebfdfbf14e68b4c958fe79 
>   src/examples/load_generator_framework.cpp 
> 653ae815df2009613b8f89308ab1b5f19757825a 
>   src/examples/no_executor_framework.cpp 
> 2ca240b4c998f0e33e7b09dbad9f4a7bfa0583cc 
>   src/examples/persistent_volume_framework.cpp 
> bce2a2d1c73a3a1efab6d107529d9f6cfe9854da 
>   src/examples/test_framework.cpp a6b38f02cc29682e3d3e1adf83baea061feda9e7 
>   src/examples/test_http_framework.cpp 
> 693dd47694678e7439f9d4ab0ff77b93950edf6e 
>   src/exec/exec.cpp 65c4575f8d44a1f4d4e99350b770eb8b921c0644 
>   src/executor/executor.cpp 91bbd1e7651754620324e2384b7becbad9240a26 
>   src/launcher/default_executor.cpp 106b7f2e0244d211c66b237b5d1c51f43fc6e529 
>   src/launcher/executor.cpp 951597b576b4912541dd87d52dcb981393e58082 
>   src/launcher/fetcher.cpp 42980f5a4a40b72f754156469e9fe60a952d1d87 
>   src/local/main.cpp f6145b71c7ff7b454a63fd1f1865dbf7bf7345d2 
>   src/log/tool/benchmark.cpp 8264fdaa3316175cdb305202fd7b4968e896b968 
>   src/log/tool/initialize.cpp 6a12505119bebd4c5cfbe00aeb45a51ad9d62bec 
>   src/log/tool/read.cpp 7b308986012fc14d5f01c88d2d09ceb45ad5db75 
>   src/log/tool/replica.cpp 45bd1f4bc54cf4c84af207c977fb78b23f042873 
>   src/logging/logging.hpp 0e448f25de6c66d772f5bc481fbaf4aa493fd8af 
>   src/logging/logging.cpp 70d66a5c396f709e8f27ad0d51315ed6d257f73b 
>   src/master/main.cpp 7e815bf89e018921757b19dc8e16e2d5139248ee 
>   src/sched/sched.cpp ef73c1dccfd736b79f40a057951f022df7f60644 
>   src/scheduler/scheduler.cpp ce69258027ed50867569374d2d827fc3cc651744 
>   src/slave/main.cpp d53ff5013e00421ef1e13593584bd132a59738fb 
>   src/tests/main.cpp a7dc99b90f84a1a1ea299c937fbfc84e55adad1c 
> 
> 
> Diff: https://reviews.apache.org/r/62357/diff/2/
> 
> 
> Testing
> ---
> 
> ```
> make check
> ```
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>



Re: Review Request 62214: Added JavaScript linter.

2017-09-18 Thread Armand Grillet

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

(Updated Sept. 18, 2017, 3:10 p.m.)


Review request for mesos, Benjamin Mahler and Kevin Klues.


Changes
---

Updated dependency.


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


Repository: mesos


Description
---

This linter runs when changes on a JavaScript file are being committed.
The linter used is ESLint with a configuration based on our current JS
code base. The linter and its dependencies (i.e. Node.js) are installed
in a environment using Virtualenv and then Nodeenv.


Diffs
-

  src/webui/.eslintrc.js PRE-CREATION 
  src/webui/.gitignore PRE-CREATION 
  src/webui/bootstrap PRE-CREATION 
  src/webui/pip-requirements.txt PRE-CREATION 
  support/mesos-style.py cf37d9f4da4ab90b92f0136a1dcd5dd8bbae5785 


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


Testing
---

Following this commit, I have tried to commit a change on a JavaScript file and 
checked that ESLinter was correctly running.


Thanks,

Armand Grillet



Re: Review Request 62018: Updated logging::initialize to make flags optional.

2017-09-18 Thread Mesos Reviewbot Windows

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



FAIL: Some Mesos libprocess-tests failed.

Reviews applied: `['62018']`

Failed command: 
`C:\mesos\3rdparty\libprocess\src\tests\Debug\libprocess-tests.exe`

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

Relevant logs:

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

```
[ RUN  ] FutureTest.ArrowOperator
[   OK ] FutureTest.ArrowOperator (0 ms)
[ RUN  ] FutureTest.UndiscardableFuture
[   OK ] FutureTest.UndiscardableFuture (0 ms)
[ RUN  ] FutureTest.UndiscardableLambda
[   OK ] FutureTest.UndiscardableLambda (1 ms)
[--] 17 tests from FutureTest (378 ms total)

[--] 1 test from HTTPTest
[ RUN  ] HTTPTest.PipeReadAll
[   OK ] HTTPTest.PipeReadAll (2 ms)
[--] 1 test from HTTPTest (21 ms total)

[--] 8 tests from HTTPConnectionTest
[ RUN  ] HTTPConnectionTest.GzipRequestBody
[   OK ] HTTPConnectionTest.GzipRequestBody (28 ms)
[ RUN  ] HTTPConnectionTest.Serial
[   OK ] HTTPConnectionTest.Serial (36 ms)
[ RUN  ] HTTPConnectionTest.Pipeline
[   OK ] HTTPConnectionTest.Pipeline (61 ms)
[ RUN  ] HTTPConnectionTest.ClosingRequest
[   OK ] HTTPConnectionTest.ClosingRequest (31 ms)
[ RUN  ] HTTPConnectionTest.ClosingResponse
[   OK ] HTTPConnectionTest.ClosingResponse (28 ms)
[ RUN  ] HTTPConnectionTest.ReferenceCounting
[   OK ] HTTPConnectionTest.ReferenceCounting (5 ms)
[ RUN  ] HTTPConnectionTest.Equality
[   OK ] HTTPConnectionTest.Equality (7 ms)
[ RUN  ] HTTPConnectionTest.RequestStreaming
```

- Mesos Reviewbot Windows


On Sept. 18, 2017, 1:05 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62018/
> ---
> 
> (Updated Sept. 18, 2017, 1:05 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-7586
> https://issues.apache.org/jira/browse/MESOS-7586
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This will be used to initialize logging in the main functions
> that do not use `mesos::internal::logging::Flags`.
> 
> 
> Diffs
> -
> 
>   src/logging/logging.hpp 0e448f25de6c66d772f5bc481fbaf4aa493fd8af 
>   src/logging/logging.cpp 70d66a5c396f709e8f27ad0d51315ed6d257f73b 
> 
> 
> Diff: https://reviews.apache.org/r/62018/diff/4/
> 
> 
> Testing
> ---
> 
> ```
> $ make check
> ```
> 
> Also tested manually to see if using `LOG()` without initializing GLOG gives 
> the same result as using it after running 
> `mesos::internal::logging::initialize` without flags.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>



Re: Review Request 62381: Removed `docker exec` when performing health checks in docker executor.

2017-09-18 Thread Mesos Reviewbot Windows

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



FAIL: Mesos tests failed to build.

Reviews applied: `['62381']`

Failed command: `cmake.exe --build . --target mesos-tests --config Debug`

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

Relevant logs:

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

```


"C:\mesos\src\tests\mesos-tests.vcxproj" (default target) (1) ->
"C:\mesos\src\checks\mesos-tcp-connect.vcxproj" (default target) (26) ->
  C:\mesos\mesos\src\checks\tcp_connect.cpp(93): warning C4244: 'initializing': 
conversion from 'SOCKET' to 'int', possible loss of data 
[C:\mesos\src\checks\mesos-tcp-connect.vcxproj]


"C:\mesos\src\tests\mesos-tests.vcxproj" (default target) (1) ->
"C:\mesos\src\tests\test-helper.vcxproj" (default target) (28) ->
  C:\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(56): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\mesos\mesos\src\tests\active_user_test_helper.cpp) 
[C:\mesos\src\tests\test-helper.vcxproj]
  C:\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(436): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\mesos\mesos\src\tests\active_user_test_helper.cpp) 
[C:\mesos\src\tests\test-helper.vcxproj]
  C:\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(56): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\mesos\mesos\src\tests\http_server_test_helper.cpp) 
[C:\mesos\src\tests\test-helper.vcxproj]
  C:\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(436): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\mesos\mesos\src\tests\http_server_test_helper.cpp) 
[C:\mesos\src\tests\test-helper.vcxproj]
  C:\mesos\mesos\src\tests\resources_utils.cpp(93): warning C4267: 'argument': 
conversion from 'size_t' to 'int', possible loss of data 
[C:\mesos\src\tests\test-helper.vcxproj]
  C:\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(56): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\mesos\mesos\src\tests\flags.cpp) [C:\mesos\src\tests\test-helper.vcxproj]
  C:\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(436): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\mesos\mesos\src\tests\flags.cpp) [C:\mesos\src\tests\test-helper.vcxproj]
  C:\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(56): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\mesos\mesos\src\tests\utils.cpp) [C:\mesos\src\tests\test-helper.vcxproj]
  C:\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(436): warning 
C4996: 'GetVersionExW': was declared deprecated (compiling source file 
C:\mesos\mesos\src\tests\utils.cpp) [C:\mesos\src\tests\test-helper.vcxproj]


"C:\mesos\src\tests\mesos-tests.vcxproj" (default target) (1) ->
"C:\mesos\src\docker\mesos-docker-executor.vcxproj" (default target) (24) ->
(ClCompile target) -> 
  C:\mesos\mesos\src\linux/ns.hpp(22): fatal error C1189: #error:  
"linux/ns.hpp is only available on Linux systems." 
[C:\mesos\src\docker\mesos-docker-executor.vcxproj]

141 Warning(s)
1 Error(s)

Time Elapsed 00:38:37.69
```

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

```
  Creating directory "C:\mesos\CMakeFiles\CMakeTmp\Debug\".

  Creating directory "cmTC_2e741.dir\Debug\cmTC_2e741.tlog\".

InitializeBuildStatus:

  Creating "cmTC_2e741.dir\Debug\cmTC_2e741.tlog\unsuccessfulbuild" because 
"AlwaysCreate" was specified.

ClCompile:

  C:\Program Files (x86)\Microsoft Visual 
Studio\2017\Community\VC\Tools\MSVC\14.10.25017\bin\HostX64\x64\CL.exe /c /Zi 
/W3 /WX- /diagnostics:classic /Od /Ob0 /D WIN32 /D _WINDOWS /D 
COMPILER_SUPPORTS_CXX11 /D "CMAKE_INTDIR=\"Debug\"" /D _MBCS /Gm- /EHsc /RTC1 
/MDd /GS /fp:precise /Zc:wchar_t /Zc:forScope /Zc:inline /GR 
/Fo"cmTC_2e741.dir\Debug\" /Fd"cmTC_2e741.dir\Debug\vc141.pdb" /Gd /TP 
/errorReport:queue C:\mesos\CMakeFiles\CMakeTmp\src.cxx

  Microsoft (R) C/C++ Optimizing Compiler Version 19.10.25019 for x64

  Copyright (C) Microsoft Corporation.  All rights reserved.

  

  cl /c /Zi /W3 /WX- /diagnostics:classic /Od /Ob0 /D WIN32 /D _WINDOWS /D 
COMPILER_SUPPORTS_CXX11 /D "CMAKE_INTDIR=\"Debug\"" /D _MBCS /Gm- /EHsc /RTC1 
/MDd /GS /fp:precise /Zc:wchar_t /Zc:forScope /Zc:inline /GR 
/Fo"cmTC_2e741.dir\Debug\" /Fd"cmTC_2e741.dir\Debug\vc141.pdb" /Gd /TP 
/errorReport:queue C:\mesos\CMakeFiles\CMakeTmp\src.cxx

  src.cxx

  

Link:

  C:\Program Files (x86)\Microsoft Visual 

Re: Review Request 62018: Updated logging::initialize to make flags optional.

2017-09-18 Thread Armand Grillet

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

(Updated Sept. 18, 2017, 1:05 p.m.)


Review request for mesos, Andrei Budnik and Alexander Rukletsov.


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


Repository: mesos


Description (updated)
---

This will be used to initialize logging in the main functions
that do not use `mesos::internal::logging::Flags`.


Diffs
-

  src/logging/logging.hpp 0e448f25de6c66d772f5bc481fbaf4aa493fd8af 
  src/logging/logging.cpp 70d66a5c396f709e8f27ad0d51315ed6d257f73b 


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


Testing (updated)
---

```
$ make check
```

Also tested manually to see if using `LOG()` without initializing GLOG gives 
the same result as using it after running 
`mesos::internal::logging::initialize` without flags.


Thanks,

Armand Grillet



Review Request 62381: Removed `docker exec` when performing health checks in docker executor.

2017-09-18 Thread Andrei Budnik

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

Review request for mesos, Alexander Rukletsov, Gastón Kleiman, haosdent huang, 
and Lukas Loesche.


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


Repository: mesos


Description
---

Mesos can enter namespaces via `setns` for launched subprocesses, hence
we got rid of `docker exec` for command health checks. This change
leads to more stable command health checks by making them independent
from docker daemon that might hang. Also, this commit fixes the issue
with incorrect escaping of quote characters in command health checks.


Diffs
-

  src/docker/executor.cpp e9949f652cd8527991ebfdfbf14e68b4c958fe79 
  src/tests/health_check_tests.cpp f4b50b1cb505084f64bf2dd279d9189ca65c8cdc 


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


Testing
---

internal CI


Thanks,

Andrei Budnik