Re: Review Request 61473: Do not kill non partition aware tasks.

[Mesos Reviewbot Windows]

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



FAIL: Failed to apply the current review.

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

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

Relevant logs:

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

```
Traceback (most recent call last):
  File ".\support\apply-reviews.py", line 434, in 
main()
  File ".\support\apply-reviews.py", line 429, in main
reviewboard(options)
  File ".\support\apply-reviews.py", line 419, in reviewboard
apply_review(options)
  File ".\support\apply-reviews.py", line 160, in apply_review
commit_patch(options)
  File ".\support\apply-reviews.py", line 271, in commit_patch
message.write(data['message'])
UnicodeEncodeError: 'ascii' codec can't encode character u'\u2019' in position 
655: ordinal not in range(128)
```

- Mesos Reviewbot Windows


On Nov. 16, 2017, 7:20 a.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61473/
> ---
> 
> (Updated Nov. 16, 2017, 7:20 a.m.)
> 
> 
> Review request for mesos, James Peach, Vinod Kone, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7215
> https://issues.apache.org/jira/browse/MESOS-7215
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Master will not kill the tasks for non-Partition aware frameworks
> when an unreachable agent re-registers with the master.
> Master used to send a ShutdownFrameworkMessages to the agent
> to kill the tasks from non partition aware frameworks including the
> ones that are still registered which was problematic because the offer
> from this agent could still go to the same framework which could then
> launch new tasks. The agent would then receive tasks of the same
> framework and ignore them because it thinks the framework is shutting
> down. The framework is not shutting down of course, so from the master
> and the scheduler’s perspective the task is pending in STAGING forever
> until the next agent reregistration, which could happen much later.
> This commit fixes the problem by not shutting down the non-partition
> aware frameworks on such an agent.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto e194093e490741acc552fd3ad328fd710b4b4435 
>   include/mesos/v1/mesos.proto 6fb1139683952877667abbcf8bf84b5b31bcd29e 
>   src/master/http.cpp 10084125deb839a9846a4f64d2e433ff02754c02 
>   src/master/master.hpp a309fc78ee2613762f3d5d22ac7559afc7aac4a3 
>   src/master/master.cpp 2ddd67ada3731803b00883b6a1f32b20c1bb238f 
>   src/tests/partition_tests.cpp e49c474167076b4136a161ed29b11db9a13455a7 
> 
> 
> Diff: https://reviews.apache.org/r/61473/diff/11/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Megha Sharma
> 
>

Re: Review Request 60471: Added tests for pruneImages for containerizer and provisioner.

[Mesos Reviewbot Windows]

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



FAIL: Some Mesos libprocess-tests failed.

Reviews applied: `['55334', '55335', '59687', '62997', '56721', '56722', 
'62853', '60471']`

Failed command: 
`C:\DCOS\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/60471

Relevant logs:

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

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

[--] Global test environment set-up.

[--] 1 test from SSLClientTest

[ RUN  ] SSLClientTest.client

[   OK ] SSLClientTest.client (21 ms)

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



[--] Global test environment tear-down

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

[  PASSED  ] 1 test.

[++]

[  FAILED  ] SSLVerifyIPAdd/SSLTest.RequireCertificate/1, where GetParam() = 
"true" (1017 ms)
[--] 4 tests from SSLVerifyIPAdd/SSLTest (4731 ms total)

[--] Global test environment tear-down
[==] 201 tests from 33 test cases ran. (49240 ms total)
[  PASSED  ] 199 tests.
[  FAILED  ] 2 tests, listed below:
[  FAILED  ] SSLTest.VerifyCertificate
[  FAILED  ] SSLVerifyIPAdd/SSLTest.RequireCertificate/1, where GetParam() = 
"true"

 2 FAILED TESTS
  YOU HAVE 19 DISABLED TESTS

```

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

```
I1116 07:47:55.984385  2084 openssl.cpp:509] CA directory path unspecified! 
NOTE: Set CA directory path with LIBPROCESS_SSL_CA_DIR=
I1116 07:47:55.984385  2084 openssl.cpp:514] Will not verify peer certificate!
NOTE: Set LIBPROCESS_SSL_VERIFY_CERT=1 to enable peWARNING: Logging before 
InitGoogleLogging() is written to STDERR
I1116 07:47:56.86  5560 openssl.cpp:509] CA directory path unspecified! 
NOTE: Set CA directory path with LIBPROCESS_SSL_CA_DIR=
I1116 07:47:56.334388  5560 openssl.cpp:514] Will not verify peer certificate!
NOTE: Set LIBPROCESS_SSL_VERIFY_CERT=1 to enable peer certificate verification
I1116 07:47:56.334388  5560 openssl.cpp:534] LIBPROCESS_SSL_REQUIRE_CERT 
implies peer certificate verification.
LIBPROCESS_SSL_VERIFY_CERT set to true
I1116 07:47:56.335388  5560 openssl.cpp:563] Using CA file: 
C:\Users\mesos\AppData\Local\Temp\MxjpV5\cert.pem
WARNING: Logging before InitGoogleLogging() is written to STDERR
I1116 07:47:57.327318  3356 openssl.cpp:509] CA directory path unspecified! 
NOTE: Set CA directory path with LIBPROCESS_SSL_CA_DIR=
I1116 07:47:57.328320  3356 openssl.cpp:514] Will not verify peer certificate!
NOTE: Set LIBPROCESS_SSL_VERIFY_CERT=1 to enable peer certificate verification
I1116 07:47:57.329620  3356 openssl.cpp:526] Will use IP address verification 
in subject alternative name certificate extension.
I1116 07:47:57.329620  3356 openssl.cpp:534] LIBPROCESS_SSL_REQUIRE_CERT 
implies peer certificate verification.
LIBPROCESS_SSL_VERIFY_CERT set to true
I1116 07:47:57.330322  3356 openssl.cpp:563] Using CA file: 
C:\Users\mesos\AppData\Local\Temp\nsdOec\cert.pem
er certificate verification
I1116 07:47:56.012396  2084 openssl.cpp:534] LIBPROCESS_SSL_REQUIRE_CERT 
implies peer certificate verification.
LIBPROCESS_SSL_VERIFY_CERT set to true
I1116 07:47:56.013386  2084 openssl.cpp:563] Using CA file: 
C:\Users\mesos\AppData\Local\Temp\MxjpV5\cert.pem
I1116 07:47:57.018337  2084 openssl.cpp:509] CA directory path unspecified! 
NOTE: Set CA directory path with LIBPROCESS_SSL_CA_DIR=
I1116 07:47:57.019317  2084 openssl.cpp:514] Will not verify peer certificate!
NOTE: Set LIBPROCESS_SSL_VERIFY_CERT=1 to enable peer certificate verification
I1116 07:47:57.019317  2084 openssl.cpp:526] Will use IP address verification 
in subject alternative name certificate extension.
I1116 07:47:57.019317  2084 openssl.cpp:534] LIBPROCESS_SSL_REQUIRE_CERT 
implies peer certificate verification.
LIBPROCESS_SSL_VERIFY_CERT set to true
I1116 07:47:57.019317  2084 openssl.cpp:563] Using CA file: 
C:\Users\mesos\AppData\Local\Temp\nsdOec\cert.pem
I1116 07:47:57.654356   408 process.cpp:1052] Failed to accept socket: future 
discarded
```

- Mesos Reviewbot Windows


On Nov. 16, 2017, 4:05 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> 

Re: Review Request 63796: Made `CheckerProcess` support IPv6 for HTTP/TCP check.

[Qian Zhang]

> On Nov. 16, 2017, 8:49 a.m., Alexander Rukletsov wrote:
> > src/checks/checker_process.cpp
> > Line 830 (original), 831-833 (patched)
> > 
> >
> > This looks terrible, I wonder on which systems this will break...
> > Reading 
> > http://tech.bluesmoon.info/2011/12/using-curl-with-ipv6-addresses.html , we 
> > probably need to wrap url into double quotes. Can you please also leave a 
> > comment saying why we do these magic movements?

Yes, I have updated the patch by adding a comment to explain it.


- Qian


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


On Nov. 16, 2017, 3:22 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63796/
> ---
> 
> (Updated Nov. 16, 2017, 3:22 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Avinash sridharan.
> 
> 
> Bugs: MESOS-8050
> https://issues.apache.org/jira/browse/MESOS-8050
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made `CheckerProcess` support IPv6 for HTTP/TCP check.
> 
> 
> Diffs
> -
> 
>   src/checks/checker_process.cpp 7985e8d4dc6115f81a4daef0ce4eee67f4233c9f 
> 
> 
> Diff: https://reviews.apache.org/r/63796/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 63796: Made `CheckerProcess` support IPv6 for HTTP/TCP check.

[Qian Zhang]

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

(Updated Nov. 16, 2017, 3:22 p.m.)


Review request for mesos, Alexander Rukletsov and Avinash sridharan.


Changes
---

Addressed review comments.


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


Repository: mesos


Description
---

Made `CheckerProcess` support IPv6 for HTTP/TCP check.


Diffs (updated)
-

  src/checks/checker_process.cpp 7985e8d4dc6115f81a4daef0ce4eee67f4233c9f 


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

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


Testing
---

sudo make check


Thanks,

Qian Zhang

Re: Review Request 63795: Made `mesos-tcp-connect` support IPv6.

[Qian Zhang]

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

(Updated Nov. 16, 2017, 3:21 p.m.)


Review request for mesos, Alexander Rukletsov and Avinash sridharan.


Changes
---

Addressed review comments.


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


Repository: mesos


Description
---

Made `mesos-tcp-connect` support IPv6.


Diffs (updated)
-

  src/checks/tcp_connect.cpp 2514f4aebcc634b18cd2b3c36529222fe456e903 


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

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


Testing
---


Thanks,

Qian Zhang

Re: Review Request 63794: Added a new member field `ipv6` to the `CheckerProcess` class.

[Qian Zhang]

> On Nov. 16, 2017, 8:49 a.m., Alexander Rukletsov wrote:
> > src/checks/health_checker.cpp
> > Lines 266-273 (patched)
> > 
> >
> > Hm, maybe this is more readable?
> > ```
> >   if ((healthCheck.type() == HealthCheck.HTTP &&
> >healthCheck.http().protocol() == NetworkInfo::IPv6) ||
> >   (healthCheck.type() == HealthCheck.TCP &&
> >healthCheck.tcp().protocol() == NetworkInfo::IPv6)) {
> > ipv6 = true;
> >   }
> > ```

Agree!


- Qian


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


On Nov. 16, 2017, 3:20 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63794/
> ---
> 
> (Updated Nov. 16, 2017, 3:20 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Avinash sridharan.
> 
> 
> Bugs: MESOS-8050
> https://issues.apache.org/jira/browse/MESOS-8050
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a new member field `ipv6` to the `CheckerProcess` class.
> 
> 
> Diffs
> -
> 
>   src/checks/checker_process.hpp 5d0c36dd3e1edf68527fc109d8c0708f09b7e12f 
>   src/checks/checker_process.cpp 7985e8d4dc6115f81a4daef0ce4eee67f4233c9f 
>   src/checks/health_checker.cpp d4bda6ed1747ae4c970619bbfa336321aeea52ea 
> 
> 
> Diff: https://reviews.apache.org/r/63794/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 61473: Do not kill non partition aware tasks.

[Megha Sharma]

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

(Updated Nov. 16, 2017, 7:20 a.m.)


Review request for mesos, James Peach, Vinod Kone, and Jiang Yan Xu.


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


Repository: mesos


Description
---

Master will not kill the tasks for non-Partition aware frameworks
when an unreachable agent re-registers with the master.
Master used to send a ShutdownFrameworkMessages to the agent
to kill the tasks from non partition aware frameworks including the
ones that are still registered which was problematic because the offer
from this agent could still go to the same framework which could then
launch new tasks. The agent would then receive tasks of the same
framework and ignore them because it thinks the framework is shutting
down. The framework is not shutting down of course, so from the master
and the scheduler’s perspective the task is pending in STAGING forever
until the next agent reregistration, which could happen much later.
This commit fixes the problem by not shutting down the non-partition
aware frameworks on such an agent.


Diffs (updated)
-

  include/mesos/mesos.proto e194093e490741acc552fd3ad328fd710b4b4435 
  include/mesos/v1/mesos.proto 6fb1139683952877667abbcf8bf84b5b31bcd29e 
  src/master/http.cpp 10084125deb839a9846a4f64d2e433ff02754c02 
  src/master/master.hpp a309fc78ee2613762f3d5d22ac7559afc7aac4a3 
  src/master/master.cpp 2ddd67ada3731803b00883b6a1f32b20c1bb238f 
  src/tests/partition_tests.cpp e49c474167076b4136a161ed29b11db9a13455a7 


Diff: https://reviews.apache.org/r/61473/diff/11/

Changes: https://reviews.apache.org/r/61473/diff/10-11/


Testing
---

make check


Thanks,

Megha Sharma

Re: Review Request 63794: Added a new member field `ipv6` to the `CheckerProcess` class.

[Qian Zhang]

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

(Updated Nov. 16, 2017, 3:20 p.m.)


Review request for mesos, Alexander Rukletsov and Avinash sridharan.


Changes
---

Addressed review comments.


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


Repository: mesos


Description
---

Added a new member field `ipv6` to the `CheckerProcess` class.


Diffs (updated)
-

  src/checks/checker_process.hpp 5d0c36dd3e1edf68527fc109d8c0708f09b7e12f 
  src/checks/checker_process.cpp 7985e8d4dc6115f81a4daef0ce4eee67f4233c9f 
  src/checks/health_checker.cpp d4bda6ed1747ae4c970619bbfa336321aeea52ea 


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

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


Testing
---


Thanks,

Qian Zhang

Review Request 63831: Fixed a bug that removed the suppressed framework from sorter.

[Jiang Yan Xu]

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

Review request for mesos and Alexander Rukletsov.


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


Repository: mesos


Description
---

In `updateFramework`, we currently treat frameworks suppressing a role the same 
way as frameworks moving off a role and this could lead to the framework being 
removed from the framework sorter, which is unexpected. We should only remove 
the framework from a framework sorter when it's moving away from the 
corresponding role.


Diffs
-

  src/master/allocator/mesos/hierarchical.cpp 
f0fa7754e5968288edb15929efc9d244b177 
  src/tests/scheduler_tests.cpp 45fc9c0cfccdb22c2e3e8d5de30c04575814a0e9 


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


Testing
---

make check.

Added a test.


Thanks,

Jiang Yan Xu

Re: Review Request 59687: Added tests for recovering ContainerConfig.

[Gilbert Song]

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


Fix it, then Ship it!





src/tests/containerizer/nested_mesos_containerizer_tests.cpp
Lines 1742 (patched)


seems like we just updated it to be `LaunchResult` for standalone container.


- Gilbert Song


On Oct. 27, 2017, 11:02 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59687/
> ---
> 
> (Updated Oct. 27, 2017, 11:02 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jason Lai.
> 
> 
> Bugs: MESOS-6894
> https://issues.apache.org/jira/browse/MESOS-6894
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added tests for recovering ContainerConfig.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
> fbd2887800ccfd64c8628c5d6fd77a511c8f91d5 
> 
> 
> Diff: https://reviews.apache.org/r/59687/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 63853: Added the "task" prefix to the name of the status update manager files.

[Mesos Reviewbot Windows]

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



FAIL: Mesos failed to build.

Reviews applied: `['63852', '63853']`

Failed command: `cmake.exe C:\DCOS\mesos\mesos -G "Visual Studio 15 2017 Win64" 
-T host=x64 -DENABLE_LIBEVENT=ON -DHAS_AUTHENTICATION=ON -DENABLE_JAVA=ON 
-DENABLE_SSL=ON`

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

Relevant logs:

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

```
-- Detecting CXX compiler ABI info - done
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- cotire 1.7.10 loaded.
-- 
-- * Beginning Mesos CMake configuration step *
-- 
-- INSTALLATION PREFIX: C:/Program Files/Mesos
-- MACHINE SPECS:
-- Hostname: 
-- OS:   WINDOWS(10.0.14393)
-- Arch: AMD64
-- BitMode:  
-- BuildID:  
-- 
-- Found Java: C:/Program Files/Java/jdk1.8.0_151/bin/java.exe (found version 
"1.8.0.151") 
-- Found Java: C:/Program Files/Java/jdk1.8.0_151/bin/java.exe (found version 
"1.8.0.151") found components:  Development 
-- Found JNI: C:/Program Files/Java/jdk1.8.0_151/lib/jawt.lib  
-- Performing Test COMPILER_SUPPORTS_CXX11
-- Performing Test COMPILER_SUPPORTS_CXX11 - Success
-- GnuWin32 patch.exe exists at: C:/Program Files (x86)/GnuWin32/bin/patch.exe
-- Looking for pthread.h
-- Looking for pthread.h - not found
-- Found Threads: TRUE  
-- Found OpenSSL: 
optimized;C:/OpenSSL-Win64/lib/VC/libeay32MT.lib;debug;C:/OpenSSL-Win64/lib/VC/libeay32MTd.lib
 (found version "1.0.2l") 
-- CXX target mesos cotired without unity build.
-- Configuring done
-- Generating done
-- Build files have been written to: C:/DCOS/mesos
```

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

```
  On Windows, the required versions of:

* curl
* apr
* zlib
* glog

  do not come rebundled in the Mesos repository.  They will be downloaded
  from the Internet, even though the `REBUNDLED` flag was set.
Call Stack (most recent call first):
  cmake/MesosConfigure.cmake:51 (include)
  CMakeLists.txt:48 (include)


CMake Error at src/tests/CMakeLists.txt:253 (add_executable):
  Cannot find source file:

status_update_manager_tests.cpp

  Tried extensions .c .C .c++ .cc .cpp .cxx .m .M .mm .h .hh .h++ .hm .hpp
  .hxx .in .txx


CMake Warning:
  Manually-specified variables were not used by the project:

HAS_AUTHENTICATION


```

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

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

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

InitializeBuildStatus:

  Creating "cmTC_779ee.dir\Debug\cmTC_779ee.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_779ee.dir\Debug\" /Fd"cmTC_779ee.dir\Debug\vc141.pdb" /Gd /TP 
/errorReport:queue C:\DCOS\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_779ee.dir\Debug\" /Fd"cmTC_779ee.dir\Debug\vc141.pdb" /Gd /TP 
/errorReport:queue C:\DCOS\mesos\CMakeFiles\CMakeTmp\src.cxx

  src.cxx

  

Link:

  C:\Program Files (x86)\Microsoft Visual 
Studio\2017\Community\VC\Tools\MSVC\14.10.25017\bin\HostX64\x64\link.exe 
/ERRORREPORT:QUEUE 
/OUT:"C:\DCOS\mesos\CMakeFiles\CMakeTmp\Debug\cmTC_779ee.exe" /INCREMENTAL 
/NOLOGO kernel32.lib user32.lib gdi32.lib winspool.lib shell32.lib ole32.lib 
oleaut32.lib uuid.lib comdlg32.lib advapi32.lib /MANIFEST 
/MANIFESTUAC:"level='asInvoker' uiAccess='false'" /manifest:embed /DEBUG 
/PDB:"C:/DCOS/mesos/CMakeFiles/CMakeTmp/Debug/cmTC_779ee.pdb" 
/SUBSYSTEM:CONSOLE /TLBID:1 /DYNAMICBASE /NXCOMPAT 
/IMPLIB:"C:/DCOS/mesos/CMakeFiles/CMakeTmp/Debug/cmTC_779ee.lib" /MACHINE:X64  
/machine:x64 cmTC_779ee.dir\Debug\src.obj

  

Re: Review Request 63844: Removed acknowledged offer operation status updates.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 63844 was successfully built and tested.

Reviews applied: `['63730', '63842', '63731', '63732', '63843', '63844']`

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

- Mesos Reviewbot Windows


On Nov. 15, 2017, 11:01 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63844/
> ---
> 
> (Updated Nov. 15, 2017, 11:01 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Jie Yu, and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp d8bacebc74790e955490a158c37ac0d9e75fd6b5 
> 
> 
> Diff: https://reviews.apache.org/r/63844/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 61473: Do not kill non partition aware tasks.

[Megha Sharma]

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

(Updated Nov. 16, 2017, 5:43 a.m.)


Review request for mesos, James Peach, Vinod Kone, and Jiang Yan Xu.


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


Repository: mesos


Description
---

Master will not kill the tasks for non-Partition aware frameworks
when an unreachable agent re-registers with the master.
Master used to send a ShutdownFrameworkMessages to the agent
to kill the tasks from non partition aware frameworks including the
ones that are still registered which was problematic because the offer
from this agent could still go to the same framework which could then
launch new tasks. The agent would then receive tasks of the same
framework and ignore them because it thinks the framework is shutting
down. The framework is not shutting down of course, so from the master
and the scheduler’s perspective the task is pending in STAGING forever
until the next agent reregistration, which could happen much later.
This commit fixes the problem by not shutting down the non-partition
aware frameworks on such an agent.


Diffs (updated)
-

  include/mesos/mesos.proto e194093e490741acc552fd3ad328fd710b4b4435 
  include/mesos/v1/mesos.proto 6fb1139683952877667abbcf8bf84b5b31bcd29e 
  src/master/http.cpp 10084125deb839a9846a4f64d2e433ff02754c02 
  src/master/master.hpp a309fc78ee2613762f3d5d22ac7559afc7aac4a3 
  src/master/master.cpp 2ddd67ada3731803b00883b6a1f32b20c1bb238f 
  src/tests/partition_tests.cpp e49c474167076b4136a161ed29b11db9a13455a7 


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

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


Testing
---

make check


Thanks,

Megha Sharma

Re: Review Request 61473: Do not kill non partition aware tasks.

[Megha Sharma]

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

(Updated Nov. 16, 2017, 5:24 a.m.)


Review request for mesos, James Peach, Vinod Kone, and Jiang Yan Xu.


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


Repository: mesos


Description
---

Master will not kill the tasks for non-Partition aware frameworks
when an unreachable agent re-registers with the master.
Master used to send a ShutdownFrameworkMessages to the agent
to kill the tasks from non partition aware frameworks including the
ones that are still registered which was problematic because the offer
from this agent could still go to the same framework which could then
launch new tasks. The agent would then receive tasks of the same
framework and ignore them because it thinks the framework is shutting
down. The framework is not shutting down of course, so from the master
and the scheduler’s perspective the task is pending in STAGING forever
until the next agent reregistration, which could happen much later.
This commit fixes the problem by not shutting down the non-partition
aware frameworks on such an agent.


Diffs (updated)
-

  include/mesos/mesos.proto e194093e490741acc552fd3ad328fd710b4b4435 
  include/mesos/v1/mesos.proto 6fb1139683952877667abbcf8bf84b5b31bcd29e 
  src/master/http.cpp 10084125deb839a9846a4f64d2e433ff02754c02 
  src/master/master.hpp a309fc78ee2613762f3d5d22ac7559afc7aac4a3 
  src/master/master.cpp 2ddd67ada3731803b00883b6a1f32b20c1bb238f 
  src/tests/partition_tests.cpp e49c474167076b4136a161ed29b11db9a13455a7 


Diff: https://reviews.apache.org/r/61473/diff/9/

Changes: https://reviews.apache.org/r/61473/diff/8-9/


Testing
---

make check


Thanks,

Megha Sharma

Re: Review Request 61473: Do not kill non partition aware tasks.

[Megha Sharma]

> On Oct. 26, 2017, 8:23 a.m., Jiang Yan Xu wrote:
> > src/master/master.hpp
> > Lines 2591 (patched)
> > 
> >
> > You made a redundant copy here but I understand this line may go away 
> > anyways. :)

That's right!


- Megha


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


On Oct. 16, 2017, 8:59 a.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61473/
> ---
> 
> (Updated Oct. 16, 2017, 8:59 a.m.)
> 
> 
> Review request for mesos, James Peach, Vinod Kone, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7215
> https://issues.apache.org/jira/browse/MESOS-7215
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Master will not kill the tasks for non-Partition aware frameworks
> when an unreachable agent re-registers with the master.
> Master used to send a ShutdownFrameworkMessages to the agent
> to kill the tasks from non partition aware frameworks including the
> ones that are still registered which was problematic because the offer
> from this agent could still go to the same framework which could then
> launch new tasks. The agent would then receive tasks of the same
> framework and ignore them because it thinks the framework is shutting
> down. The framework is not shutting down of course, so from the master
> and the scheduler’s perspective the task is pending in STAGING forever
> until the next agent reregistration, which could happen much later.
> This commit fixes the problem by not shutting down the non-partition
> aware frameworks on such an agent.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 42139bec519d36316e324ef921157c49cdf2d043 
>   src/master/master.hpp 0ddc98259f64b3921d08f5f4ec81543bb0826378 
>   src/master/master.cpp 3603878f02ae3dba82811a4a5770dd21ec790ef6 
>   src/tests/partition_tests.cpp 0597bd2afaa60121245e0d43b81ac223257e377a 
> 
> 
> Diff: https://reviews.apache.org/r/61473/diff/8/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Megha Sharma
> 
>

Re: Review Request 55334: Checkpoint and recover `ContainerConfig` in Mesos containerizer.

[Zhitao Li]

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

(Updated Nov. 15, 2017, 8:33 p.m.)


Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.


Changes
---

Fixed the comment message < 72 char.


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


Repository: mesos


Description (updated)
---

This patch includes the following change:
- Checkpointed `ContainerConfig` used to launch a container;
- Added helper function to read checkpointed `ContainerConfig`;
- Recovered `ContainerConfig`.
- Make `ContainerConfig` Option in `Container` struct to indicate
  whether recovered.


Diffs
-

  src/slave/containerizer/mesos/containerizer.hpp 
f5d5146de9abdd9804ede6a4b17a965a4b27c05a 
  src/slave/containerizer/mesos/containerizer.cpp 
db5f044f7415386c94e4930f365dfc14d403414a 
  src/slave/containerizer/mesos/paths.hpp 
7b67ccf670d991cbc339d74ebf4fccf3ae9a98a8 
  src/slave/containerizer/mesos/paths.cpp 
23f1fee3667aa48e8c31c15e0b69201268e44d37 


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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 63850: Used fully qualified namespace in the scheduler library for consistency.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 63850 was successfully built and tested.

Reviews applied: `['63767', '63768', '63804', '63850']`

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

- Mesos Reviewbot Windows


On Nov. 16, 2017, 5:54 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63850/
> ---
> 
> (Updated Nov. 16, 2017, 5:54 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Gaston Kleiman, and Jie Yu.
> 
> 
> Bugs: MESOS-8198
> https://issues.apache.org/jira/browse/MESOS-8198
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used fully qualified namespace in the scheduler library for consistency.
> 
> 
> Diffs
> -
> 
>   src/scheduler/scheduler.cpp 286a7603352a1e639a0502e496d4409be11bf9d7 
> 
> 
> Diff: https://reviews.apache.org/r/63850/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 63848: Added test to check standalone containers in GET_CONTAINERS.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 63848 was successfully built and tested.

Reviews applied: `['63848']`

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

- Mesos Reviewbot Windows


On Nov. 15, 2017, 7:42 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63848/
> ---
> 
> (Updated Nov. 15, 2017, 7:42 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Standalone containers are *NOT* expected to show up in the output
> of the agent GET_CONTAINERS call.  This is because the standalone
> containers lack some of the required metadata, like a FrameworkID,
> ExecutorID, and Executor name.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp c9c50b91ddd7c3317c1188f878753ad3bd5c3941 
> 
> 
> Diff: https://reviews.apache.org/r/63848/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 60471: Added tests for pruneImages for containerizer and provisioner.

[Zhitao Li]

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

(Updated Nov. 16, 2017, 4:05 a.m.)


Review request for mesos, Gilbert Song and Jason Lai.


Changes
---

Rebase only.


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


Repository: mesos


Description
---

Added tests for pruneImages for containerizer and provisioner.


Diffs (updated)
-

  src/tests/containerizer/mesos_containerizer_tests.cpp 
ce67def65aa65188aff10f5316fcd8b745d0abf2 
  src/tests/containerizer/provisioner_docker_tests.cpp 
832c81fe88d753b0f00dfab870d7725cf556fcef 


Diff: https://reviews.apache.org/r/60471/diff/9/

Changes: https://reviews.apache.org/r/60471/diff/8-9/


Testing
---


Thanks,

Zhitao Li

Re: Review Request 56722: Added a new operator API for `PRUNE_IMAGES`.

[Zhitao Li]

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

(Updated Nov. 16, 2017, 4:03 a.m.)


Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.


Changes
---

Rebase after standalone container support.


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


Repository: mesos


Description
---

Added a new operator API for `PRUNE_IMAGES`.


Diffs (updated)
-

  include/mesos/agent/agent.proto 0f92f73ba0f7729f0ba7cd89a692ab3685125e8b 
  include/mesos/v1/agent/agent.proto 012ffef5f0dd7720fa95ae484c99479aaf256d7b 
  src/slave/http.hpp a51831cdcebc1998ce6d4c3c19285e598a4ec9a3 
  src/slave/http.cpp 394e91013dc11e0a79e2e00534864281cc74ad2f 
  src/slave/validation.cpp 32781fd8f124f71e61744804aec3fe4da59a5df2 


Diff: https://reviews.apache.org/r/56722/diff/6/

Changes: https://reviews.apache.org/r/56722/diff/5-6/


Testing
---

1. New unit test to trigger the new call.
2. Manually tested that images previous pulled but not running can be purged 
through new operator API call while active images (running or being pulled) are 
not affected.


Thanks,

Zhitao Li

Re: Review Request 56721: Implemented pruneImages with a mark and sweep in docker store.

[Zhitao Li]

> On Nov. 16, 2017, 12:04 a.m., Gilbert Song wrote:
> > src/slave/containerizer/mesos/provisioner/provisioner.cpp
> > Lines 472 (patched)
> > 
> >
> > is it possible to pass by value for our ReadWriteLock? so that we could 
> > s/this/rwLock/g?

pass by value requires creating local copy of the lock, or pass by reference 
requires c++14, so I'm sticking with this style.


- Zhitao


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


On Nov. 16, 2017, 4 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56721/
> ---
> 
> (Updated Nov. 16, 2017, 4 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
> 
> 
> Bugs: MESOS-4945
> https://issues.apache.org/jira/browse/MESOS-4945
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This includes the following changes:
> - add a `pruneImages()` function on the chain of relevant classes;
> - implement prune in docker store;
> - fix mock interface to keep existing tests pass.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/composing.hpp 
> c2689cf36575d1e7c6ef8bd2ab3e25681a2ecfcc 
>   src/slave/containerizer/composing.cpp 
> 64919ef1e61a984956c2280ae6b1890c4d135ad1 
>   src/slave/containerizer/containerizer.hpp 
> 2027bd93cc439ed9b2f544e57183765ee032603b 
>   src/slave/containerizer/docker.hpp 105c0680b6714aded1b3e05699930a072545e681 
>   src/slave/containerizer/docker.cpp 63432a99f58aab00cd8612164e00fe6c3a6cf5dd 
>   src/slave/containerizer/mesos/containerizer.hpp 
> f5d5146de9abdd9804ede6a4b17a965a4b27c05a 
>   src/slave/containerizer/mesos/containerizer.cpp 
> db5f044f7415386c94e4930f365dfc14d403414a 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 
> 954da1681778878c8aff6150002e52ecb648d1bb 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 
> d86afd2a6ff0bf87e624db1c99255c85068bf6ab 
>   src/slave/containerizer/mesos/provisioner/docker/paths.hpp 
> 232c027f8f96da0cb30b957bce4607d3695050d2 
>   src/slave/containerizer/mesos/provisioner/docker/paths.cpp 
> cd684b33eb308ce1eeb4539a5b2d51985d835db7 
>   src/slave/containerizer/mesos/provisioner/docker/store.hpp 
> 1cf68665d33bd40a7605d26c96fb7b618407fdd0 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> f357710cb19aec3654b0604f7909d068eaf20095 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 
> 7cba54ce490d1e6e17081cd7e04fd6759ceddb8e 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> 450a3b32d69d2882973a6ed4e94e169a0256056b 
>   src/slave/containerizer/mesos/provisioner/store.hpp 
> 01ab83dca79e51b8c96d18ee65705912b0ac8324 
>   src/slave/containerizer/mesos/provisioner/store.cpp 
> cc5cc81e05f29bb0e11ffa13cdb8d63d4397114f 
>   src/tests/containerizer.hpp c98913f1a833aa6a60779b66463610ccd5911bbf 
>   src/tests/containerizer.cpp c6f1ec0b000781270b7c79d5e776575c6df778aa 
>   src/tests/containerizer/mock_containerizer.hpp 
> 5befcccecdb76f3b70993642128745a0134ffa65 
> 
> 
> Diff: https://reviews.apache.org/r/56721/diff/17/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 56721: Implemented pruneImages with a mark and sweep in docker store.

[Zhitao Li]

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

(Updated Nov. 16, 2017, 4 a.m.)


Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.


Changes
---

Rebase and review comments.


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


Repository: mesos


Description
---

This includes the following changes:
- add a `pruneImages()` function on the chain of relevant classes;
- implement prune in docker store;
- fix mock interface to keep existing tests pass.


Diffs (updated)
-

  src/slave/containerizer/composing.hpp 
c2689cf36575d1e7c6ef8bd2ab3e25681a2ecfcc 
  src/slave/containerizer/composing.cpp 
64919ef1e61a984956c2280ae6b1890c4d135ad1 
  src/slave/containerizer/containerizer.hpp 
2027bd93cc439ed9b2f544e57183765ee032603b 
  src/slave/containerizer/docker.hpp 105c0680b6714aded1b3e05699930a072545e681 
  src/slave/containerizer/docker.cpp 63432a99f58aab00cd8612164e00fe6c3a6cf5dd 
  src/slave/containerizer/mesos/containerizer.hpp 
f5d5146de9abdd9804ede6a4b17a965a4b27c05a 
  src/slave/containerizer/mesos/containerizer.cpp 
db5f044f7415386c94e4930f365dfc14d403414a 
  src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 
954da1681778878c8aff6150002e52ecb648d1bb 
  src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 
d86afd2a6ff0bf87e624db1c99255c85068bf6ab 
  src/slave/containerizer/mesos/provisioner/docker/paths.hpp 
232c027f8f96da0cb30b957bce4607d3695050d2 
  src/slave/containerizer/mesos/provisioner/docker/paths.cpp 
cd684b33eb308ce1eeb4539a5b2d51985d835db7 
  src/slave/containerizer/mesos/provisioner/docker/store.hpp 
1cf68665d33bd40a7605d26c96fb7b618407fdd0 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
f357710cb19aec3654b0604f7909d068eaf20095 
  src/slave/containerizer/mesos/provisioner/provisioner.hpp 
7cba54ce490d1e6e17081cd7e04fd6759ceddb8e 
  src/slave/containerizer/mesos/provisioner/provisioner.cpp 
450a3b32d69d2882973a6ed4e94e169a0256056b 
  src/slave/containerizer/mesos/provisioner/store.hpp 
01ab83dca79e51b8c96d18ee65705912b0ac8324 
  src/slave/containerizer/mesos/provisioner/store.cpp 
cc5cc81e05f29bb0e11ffa13cdb8d63d4397114f 
  src/tests/containerizer.hpp c98913f1a833aa6a60779b66463610ccd5911bbf 
  src/tests/containerizer.cpp c6f1ec0b000781270b7c79d5e776575c6df778aa 
  src/tests/containerizer/mock_containerizer.hpp 
5befcccecdb76f3b70993642128745a0134ffa65 


Diff: https://reviews.apache.org/r/56721/diff/17/

Changes: https://reviews.apache.org/r/56721/diff/16-17/


Testing
---


Thanks,

Zhitao Li

Re: Review Request 55334: Checkpoint and recover `ContainerConfig` in Mesos containerizer.

[Zhitao Li]

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

(Updated Nov. 16, 2017, 3:58 a.m.)


Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.


Changes
---

Rebase after standalone container support is added.


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


Repository: mesos


Description
---

This patch includes the following change:
- Checkpointed `ContainerConfig` used to launch a container;
- Added helper function to read checkpointed `ContainerConfig`;
- Recovered `ContainerConfig`.
- Make `ContainerConfig` Option in `Container` struct to indicate whether 
recovered.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.hpp 
f5d5146de9abdd9804ede6a4b17a965a4b27c05a 
  src/slave/containerizer/mesos/containerizer.cpp 
db5f044f7415386c94e4930f365dfc14d403414a 
  src/slave/containerizer/mesos/paths.hpp 
7b67ccf670d991cbc339d74ebf4fccf3ae9a98a8 
  src/slave/containerizer/mesos/paths.cpp 
23f1fee3667aa48e8c31c15e0b69201268e44d37 


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

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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 63021: Added `getService()` function to launch CSI plugins.

[Joseph Wu]

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




src/resource_provider/storage/provider.cpp
Lines 124 (patched)


Usually a bad idea to use Path helpers with URIs.  

Mostly because... Windows...  I doubt SLRP items will reach Windows before 
we have an actual URI class with helpers, so it should be alright to leave a 
TODO here.



src/resource_provider/storage/provider.cpp
Lines 130-141 (patched)


Hm... Can't you use this?
http://www.cplusplus.com/reference/algorithm/find/



src/resource_provider/storage/provider.cpp
Lines 272-275 (patched)


Does this mean we need to manually update this version each time we bump 
the CSI 3rdparty bundle?



src/resource_provider/storage/provider.cpp
Lines 394 (patched)


Whoops, double period..


- Joseph Wu


On Nov. 14, 2017, 6:40 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63021/
> ---
> 
> (Updated Nov. 14, 2017, 6:40 p.m.)
> 
> 
> Review request for mesos, James DeFelice, Jie Yu, Joseph Wu, and Jan Schlicht.
> 
> 
> Bugs: MESOS-8032
> https://issues.apache.org/jira/browse/MESOS-8032
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `getService()` method first checks if there is already a container
> daemon for the specified plugin, and creates a new one if not. The
> post-start hook for the container daemon will call `connect()` to
> wait for the endpoint socket file to appear and connect to it, then
> set up the corresponding promise of CSI client. The post-stop hook will
> remove the socket file and create a new promise for the next call to the
> post-start hook to set it up.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/storage/provider.cpp 
> 46224997430ac0c568904d80014166a6f059907f 
> 
> 
> Diff: https://reviews.apache.org/r/63021/diff/8/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 62777: Fixed build dependency for `protobuf_tests.proto`.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 62777 was successfully built and tested.

Reviews applied: `['62777']`

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

- Mesos Reviewbot Windows


On Nov. 15, 2017, 9:40 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62777/
> ---
> 
> (Updated Nov. 15, 2017, 9:40 p.m.)
> 
> 
> Review request for mesos and Benjamin Bannier.
> 
> 
> Bugs: MESOS-8052
> https://issues.apache.org/jira/browse/MESOS-8052
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The protobuf hpp and cpp files for `protobuf_tests.proto` should be
> built after `BUNDLED_DEPS`, not within it.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/Makefile.am 4386017acd6ca465be3f735460c11d50b440ccc5 
> 
> 
> Diff: https://reviews.apache.org/r/62777/diff/3/
> 
> 
> Testing
> ---
> 
> make -j4 check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 63830: Fixed 'NoOffersWithAllRolesSuppressed' test.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 63830 was successfully built and tested.

Reviews applied: `['63741', '63830']`

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

- Mesos Reviewbot Windows


On Nov. 15, 2017, 6:45 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63830/
> ---
> 
> (Updated Nov. 15, 2017, 6:45 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-8200
> https://issues.apache.org/jira/browse/MESOS-8200
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The previous iteration of the test failed to reveal the bug in MESOS-8200 due 
> to a race condition.
> 
> 
> Diffs
> -
> 
>   src/tests/scheduler_tests.cpp 45fc9c0cfccdb22c2e3e8d5de30c04575814a0e9 
> 
> 
> Diff: https://reviews.apache.org/r/63830/diff/1/
> 
> 
> Testing
> ---
> 
> As expected, this test would reliably fail without /r/63741/ with the 
> following
> 
> ```
> I1115 07:48:28.536093 3117031 scheduler.cpp:741] Enqueuing event SUBSCRIBED 
> received from http://17.121.128.13:38423/master/api/v1/scheduler
> I1115 07:48:28.536224 3117031 scheduler.cpp:741] Enqueuing event HEARTBEAT 
> received from http://17.121.128.13:38423/master/api/v1/scheduler
> I1115 07:48:28.536244 3117029 master.cpp:8115] Sending 1 offers to framework 
> 560bd0e1-8ab7-4e7e-be7a-54cb5f399531- (default)
> I1115 07:48:28.536650 3117029 scheduler.cpp:741] Enqueuing event OFFERS 
> received from http://17.121.128.13:38423/master/api/v1/scheduler
> ../../src/tests/scheduler_tests.cpp:1497: Failure
> Mock function called more times than expected - returning directly.
> Function call: offers(0x7fffabd197a0, @0x7f7d8c005460 48-byte object 
> <50-98 0B-B0 7D-7F 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 
> 00-00 00-00 00-00 00-00 01-00 00-00 04-00 00-00 A0-54 00-8C 7D-7F 00-00>)
>  Expected: to be never called
>Actual: called once - over-saturated and active
> ```
> 
> The test passes with /r/63741/.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 63732: Reconciled offer operations between agent and master.

[Jie Yu]

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




src/master/master.cpp
Line 7044 (original), 7044 (patched)


Do you need to do that for resources in the operations?



src/master/master.cpp
Lines 7103-7120 (patched)


I feel that long term, this way of removing all and then add back will 
probably not work.

Removing offer operation means we'll need to send status update (if the 
current state is not terminal). I'd suggest we only remove those that are not 
in the new list, and add those that are not in the old list.

Same comments apply to the agent chagne.



src/master/master.cpp
Lines 7107-7121 (patched)


Do you need to also update allocator for added or removed new operations?

For instance, the allocator currently think the new operation A uses 2cpus. 
Now, if A is removed (because it's dropped), do we need to tell the allocator 
that the 2cpus are no longer used and they can be allocated to others?



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


Think about the case where agent crashes and restarts, not all RP has 
re-registered yet. In that case, some operation from some not yet re-registered 
RP will not be part of this operation list.

I don't think we want to remove those operations just yet. I think we 
should remove those operations only if the corresponding RP has re-registered 
with the agent and show up in the offer operation list (or total resources).

This is similar to we don't remove tasks when agent disconnects. In fact, 
you should follow the similar patter in reconcileKnownSlave here. If an 
operation is unknown, instead of calling `removeOfferOperation` directly, we 
should probably send a `reconcileOfferOperationMessage` to the agent to ask the 
RP to generate a status update, and rely on status update handler to properly 
handle the resource accounting.

Also realized that we probably should also do the same in the agent code. 
Instead of directly calling removeOfferOperation and addOfferOperation, send a 
`RECONCILE` message to RP to asking the RP to generate a status udpate. If it's 
unknown to the RP, RP will send OFFER_OPERATION_DROPPED, which is terminal.


- Jie Yu


On Nov. 15, 2017, 5:31 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63732/
> ---
> 
> (Updated Nov. 15, 2017, 5:31 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Bugs: MESOS-8207
> https://issues.apache.org/jira/browse/MESOS-8207
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 59a533940736f5cfd5ec31e0ed924f0b2ab13f9c 
> 
> 
> Diff: https://reviews.apache.org/r/63732/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`, still need to implement dedicated tests.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 63019: Encapsulated common error handling in CSI clients.

[Chun-Hung Hsiao]

> On Nov. 16, 2017, 12:47 a.m., Joseph Wu wrote:
> > src/csi/client.cpp
> > Lines 34-49 (patched)
> > 
> >
> > Can you add a comment explaining why these are recoverable?  And how to 
> > recover from them (or a TODO if that has yet to be implemented).

These are the idempotent behaviors. There'll be some CSI spec changes in error 
handling in the next CSI bump, and the code here will be changed accordingly.


- Chun-Hung


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


On Oct. 17, 2017, 12:37 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63019/
> ---
> 
> (Updated Oct. 17, 2017, 12:37 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> By implementing the validation logic and error handling in
> `mesos::csi::Client`, the implementation of the storage local resource
> provider could be cleaner. In the furute, we could also implement the
> retry logic here.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am b60a54a031260de6f1fb43584ae5083df2dc7e31 
>   src/csi/client.cpp e171f030efedc60521053399055d67bf99d3fcd2 
>   src/tests/mock_csi_plugin.hpp b7926c4b4ca980e37131c3e9b227343e80f5c002 
>   src/tests/mock_csi_plugin.cpp 46ca398e48578537393e4050c3a9920b367b8946 
> 
> 
> Diff: https://reviews.apache.org/r/63019/diff/3/
> 
> 
> Testing
> ---
> 
> make test
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 63795: Made `mesos-tcp-connect` support IPv6.

[Alexander Rukletsov]

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




src/checks/tcp_connect.cpp
Line 86 (original), 84 (patched)


We usually call these variables like the action we performed, i.e., 
s/ip/parse. In this case you can also restore original argument names : )



src/checks/tcp_connect.cpp
Lines 98-102 (patched)


Let's explicitly check for `AF_INET6` and add another `else` where we error 
out for unsupported family.


- Alexander Rukletsov


On Nov. 14, 2017, 1:30 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63795/
> ---
> 
> (Updated Nov. 14, 2017, 1:30 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Avinash sridharan.
> 
> 
> Bugs: MESOS-8050
> https://issues.apache.org/jira/browse/MESOS-8050
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made `mesos-tcp-connect` support IPv6.
> 
> 
> Diffs
> -
> 
>   src/checks/tcp_connect.cpp 2514f4aebcc634b18cd2b3c36529222fe456e903 
> 
> 
> Diff: https://reviews.apache.org/r/63795/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 63794: Added a new member field `ipv6` to the `CheckerProcess` class.

[Alexander Rukletsov]

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




src/checks/health_checker.cpp
Lines 266-273 (patched)


Hm, maybe this is more readable?
```
  if ((healthCheck.type() == HealthCheck.HTTP &&
   healthCheck.http().protocol() == NetworkInfo::IPv6) ||
  (healthCheck.type() == HealthCheck.TCP &&
   healthCheck.tcp().protocol() == NetworkInfo::IPv6)) {
ipv6 = true;
  }
```


- Alexander Rukletsov


On Nov. 14, 2017, 1:27 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63794/
> ---
> 
> (Updated Nov. 14, 2017, 1:27 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Avinash sridharan.
> 
> 
> Bugs: MESOS-8050
> https://issues.apache.org/jira/browse/MESOS-8050
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a new member field `ipv6` to the `CheckerProcess` class.
> 
> 
> Diffs
> -
> 
>   src/checks/checker_process.hpp 5d0c36dd3e1edf68527fc109d8c0708f09b7e12f 
>   src/checks/checker_process.cpp 7985e8d4dc6115f81a4daef0ce4eee67f4233c9f 
>   src/checks/health_checker.cpp d4bda6ed1747ae4c970619bbfa336321aeea52ea 
> 
> 
> Diff: https://reviews.apache.org/r/63794/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 63796: Made `CheckerProcess` support IPv6 for HTTP/TCP check.

[Alexander Rukletsov]

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




src/checks/checker_process.cpp
Line 830 (original), 831-833 (patched)


This looks terrible, I wonder on which systems this will break...
Reading 
http://tech.bluesmoon.info/2011/12/using-curl-with-ipv6-addresses.html , we 
probably need to wrap url into double quotes. Can you please also leave a 
comment saying why we do these magic movements?


- Alexander Rukletsov


On Nov. 14, 2017, 1:31 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63796/
> ---
> 
> (Updated Nov. 14, 2017, 1:31 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Avinash sridharan.
> 
> 
> Bugs: MESOS-8050
> https://issues.apache.org/jira/browse/MESOS-8050
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made `CheckerProcess` support IPv6 for HTTP/TCP check.
> 
> 
> Diffs
> -
> 
>   src/checks/checker_process.cpp 7985e8d4dc6115f81a4daef0ce4eee67f4233c9f 
> 
> 
> Diff: https://reviews.apache.org/r/63796/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 63019: Encapsulated common error handling in CSI clients.

[Joseph Wu]

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




src/csi/client.cpp
Lines 34-49 (patched)


Can you add a comment explaining why these are recoverable?  And how to 
recover from them (or a TODO if that has yet to be implemented).


- Joseph Wu


On Oct. 16, 2017, 5:37 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63019/
> ---
> 
> (Updated Oct. 16, 2017, 5:37 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> By implementing the validation logic and error handling in
> `mesos::csi::Client`, the implementation of the storage local resource
> provider could be cleaner. In the furute, we could also implement the
> retry logic here.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am b60a54a031260de6f1fb43584ae5083df2dc7e31 
>   src/csi/client.cpp e171f030efedc60521053399055d67bf99d3fcd2 
>   src/tests/mock_csi_plugin.hpp b7926c4b4ca980e37131c3e9b227343e80f5c002 
>   src/tests/mock_csi_plugin.cpp 46ca398e48578537393e4050c3a9920b367b8946 
> 
> 
> Diff: https://reviews.apache.org/r/63019/diff/3/
> 
> 
> Testing
> ---
> 
> make test
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 63732: Reconciled offer operations between agent and master.

[Jie Yu]

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




src/master/master.cpp
Lines 7090-7091 (patched)


I don't quite follow the comments here. What do you mean by we cannot add 
those before?



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


Ditto. You probably want to change `addOfferOperation` to not update total 
resources. That means you need to goto callsites to manually apply then if it's 
a speculative operation.


- Jie Yu


On Nov. 15, 2017, 5:31 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63732/
> ---
> 
> (Updated Nov. 15, 2017, 5:31 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Bugs: MESOS-8207
> https://issues.apache.org/jira/browse/MESOS-8207
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 59a533940736f5cfd5ec31e0ed924f0b2ab13f9c 
> 
> 
> Diff: https://reviews.apache.org/r/63732/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`, still need to implement dedicated tests.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 63385: Added utility functions for CSI responses and volume ID and metadata.

[Joseph Wu]

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




src/csi/utils.cpp
Lines 41-46 (patched)


This comparison _looks_ wrong :)

You should note that the object is empty, so they must be equal.



src/csi/utils.cpp
Lines 62-71 (patched)


Does this version of the proto have the `access_mode` field?


https://github.com/container-storage-interface/spec/blob/2561947ce5899bcb80e5dbaecb99afdce66e91d4/csi.proto#L228

If not, consider adding a TODO?

---

Also, it might be a good idea to add a note about the Proto3 `oneof` 
notation which enforces the existence of either `block` or `mount`.  I'm 
guessing most contributors reading through this code might be more familiar 
with Proto2 rules.



src/csi/utils.cpp
Lines 125-127 (patched)


Probably want to leave a TODO here, since `VolumeMetaData` no longer exists:

https://github.com/container-storage-interface/spec/commit/d2bdb9177e192ecee994c549f1b166fc073ff40b


- Joseph Wu


On Oct. 27, 2017, 4:59 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63385/
> ---
> 
> (Updated Oct. 27, 2017, 4:59 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added utility functions for CSI responses and volume ID and metadata.
> 
> 
> Diffs
> -
> 
>   src/csi/utils.hpp PRE-CREATION 
>   src/csi/utils.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/63385/diff/1/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 63731: Reconciled pending resource provider operations in agent.

[Jie Yu]

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




src/slave/slave.cpp
Line 6722 (original), 6722 (patched)


I suggest we change this to `UPDATE_STATE` to align with the RP API because 
we also update operations (not just total resources).



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


I would add a big comment here explaining in what scenarios, there might be 
operations that are known by the slave, but not known by the RP. For instance, 
RP got disconnected and the offer operation is dropped due to that. Is there 
any other cases that this will happen?

Also, please comment on whether it's possible that RP knows some operation 
that the slave does not know about? If not, can we add some CHECK here?



src/slave/slave.cpp
Lines 6759-6772 (patched)


In Jan's patch, for old operations, `addOfferOperation` will update slave's 
`totalResources` because the operation will be speculatively applied.

Then, what does that mean to remove those offer opeartion? RP's total 
resources should already contains all the conversions for old operations.

Maybe it's not a good idea to update `totalResources` in 
`addOfferOperation`? `addOfferOperation` should only update `used` resources? 
Please think more on that.

Also, if we decide to do the above, make sure master code is consistent as 
well.

Please sync with Jan on this.


- Jie Yu


On Nov. 15, 2017, 5:31 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63731/
> ---
> 
> (Updated Nov. 15, 2017, 5:31 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Jie Yu, and Jan Schlicht.
> 
> 
> Bugs: MESOS-8207
> https://issues.apache.org/jira/browse/MESOS-8207
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When resource providers update their state they send a list of
> pending or unacknowledged operations to the agent. This patch add
> tracking for such operations to the agent. The agent can then forward
> these operations to the master, e.g., for calculating the unused
> resources behind an agent.
> 
> We track an operation until we either receive a updated list of
> pending or unacknowledged operations from a resource provider, or
> until we see an acknowledgement from a framework. This keeps the list
> of operations bounded and ensures that we maintain the latest
> information in the agent.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp d8bacebc74790e955490a158c37ac0d9e75fd6b5 
> 
> 
> Diff: https://reviews.apache.org/r/63731/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`, still need to implement dedicated tests.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 63377: Added filesystem layout for storage resource providers.

[Joseph Wu]

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




src/resource_provider/storage/paths.hpp
Lines 31-34 (patched)


Consider changing this comment to refer back to `src/slave/paths.hpp`.  
Specifically `getResourceProviderAgentRootDir`.

```
//   root (/resource_providers///)
```

It looks like the `rootDir` parameters below all expect an input which 
includes those elements.



src/resource_provider/storage/paths.cpp
Lines 87-88 (patched)


It's still a good idea to use `os::temp` even if that means some 
configurations will fail to create unix sockets.

Instead of hardcoding `/tmp`, just check if the resulting path is too long 
and error out here.


- Joseph Wu


On Nov. 3, 2017, 5:58 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63377/
> ---
> 
> (Updated Nov. 3, 2017, 5:58 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Joseph Wu, and Jan Schlicht.
> 
> 
> Bugs: MESOS-8141
> https://issues.apache.org/jira/browse/MESOS-8141
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A storage resource provider can now store the following persistent CSI
> data in `/resource_providers///csi`:
> 
> 1. Mount points of CSI volumes: `volumes/`
> 2. States of CSI volumes: `states//state`
> 3. Directory to place CSI endpoints: `/tmp/mesos-csi-XX`, which
>would be symlinked by `plugins//endpoint`.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 1c97b1fd8151f87c4e9e6d62884b0ef7d582c312 
>   src/resource_provider/storage/paths.hpp PRE-CREATION 
>   src/resource_provider/storage/paths.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/63377/diff/2/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 55334: Checkpoint and recover `ContainerConfig` in Mesos containerizer.

[Gilbert Song]

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


Ship it!




Could you rebase this patch? Cannot apply.

- Gilbert Song


On Nov. 10, 2017, 11:30 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55334/
> ---
> 
> (Updated Nov. 10, 2017, 11:30 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
> 
> 
> Bugs: MESOS-6894
> https://issues.apache.org/jira/browse/MESOS-6894
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch includes the following change:
> - Checkpointed `ContainerConfig` used to launch a container;
> - Added helper function to read checkpointed `ContainerConfig`;
> - Recovered `ContainerConfig`.
> - Make `ContainerConfig` Option in `Container` struct to indicate whether 
> recovered.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 6d356ccf82f36df8c6f558fb0ace7d9f982a3d6b 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 100e3bbda543d87808da9ff6bea42da5099ea8c5 
>   src/slave/containerizer/mesos/paths.hpp 
> a03f15e01e8eb8a1326baad8d7db96a79d785482 
>   src/slave/containerizer/mesos/paths.cpp 
> 0c61c20c345a327ec469b382558aaeed0280e754 
> 
> 
> Diff: https://reviews.apache.org/r/55334/diff/15/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 56721: Implemented pruneImages with a mark and sweep in docker store.

[Gilbert Song]

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


Fix it, then Ship it!




LGTM! Thank you for patientlt addressing my comments, Zhitao!


src/slave/containerizer/mesos/containerizer.cpp
Line 2796 (original), 2798 (patched)


one more space



src/slave/containerizer/mesos/provisioner/provisioner.cpp
Line 489 (original), 469 (patched)


sorry, my comment was not clear enough. just want to pull into a lambda. 
`this` may not be needed here?



src/slave/containerizer/mesos/provisioner/provisioner.cpp
Lines 472 (patched)


is it possible to pass by value for our ReadWriteLock? so that we could 
s/this/rwLock/g?



src/slave/containerizer/mesos/provisioner/provisioner.cpp
Lines 572-573 (original), 561-564 (patched)


ditto.



src/slave/containerizer/mesos/provisioner/provisioner.cpp
Lines 717-718 (original), 708-711 (patched)


ditto.



src/slave/containerizer/mesos/provisioner/provisioner.hpp
Lines 110 (patched)


Should we use `excludedImages` consistently in this patch? In favor of 
introducing it in the operator API.


- Gilbert Song


On Nov. 13, 2017, 10:21 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56721/
> ---
> 
> (Updated Nov. 13, 2017, 10:21 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
> 
> 
> Bugs: MESOS-4945
> https://issues.apache.org/jira/browse/MESOS-4945
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This includes the following changes:
> - add a `pruneImages()` function on the chain of relevant classes;
> - implement prune in docker store;
> - fix mock interface to keep existing tests pass.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/composing.hpp 
> 06d68eef5de7745e32f0e808f11016bcc285dd8f 
>   src/slave/containerizer/composing.cpp 
> 587f009384f0c7ef87482686578dc822d3d5b208 
>   src/slave/containerizer/containerizer.hpp 
> 449bb5d0902936faae7bf9bae9c703b219aed842 
>   src/slave/containerizer/docker.hpp b602a5698cae12686f51c4b9370a06042cda6270 
>   src/slave/containerizer/docker.cpp 292eecbca246edf068ec8c262aff4f3ce9cd8c67 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 6d356ccf82f36df8c6f558fb0ace7d9f982a3d6b 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 100e3bbda543d87808da9ff6bea42da5099ea8c5 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 
> 954da1681778878c8aff6150002e52ecb648d1bb 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 
> d86afd2a6ff0bf87e624db1c99255c85068bf6ab 
>   src/slave/containerizer/mesos/provisioner/docker/paths.hpp 
> 232c027f8f96da0cb30b957bce4607d3695050d2 
>   src/slave/containerizer/mesos/provisioner/docker/paths.cpp 
> cd684b33eb308ce1eeb4539a5b2d51985d835db7 
>   src/slave/containerizer/mesos/provisioner/docker/store.hpp 
> 1cf68665d33bd40a7605d26c96fb7b618407fdd0 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> f357710cb19aec3654b0604f7909d068eaf20095 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 
> 7cba54ce490d1e6e17081cd7e04fd6759ceddb8e 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> 450a3b32d69d2882973a6ed4e94e169a0256056b 
>   src/slave/containerizer/mesos/provisioner/store.hpp 
> 01ab83dca79e51b8c96d18ee65705912b0ac8324 
>   src/slave/containerizer/mesos/provisioner/store.cpp 
> cc5cc81e05f29bb0e11ffa13cdb8d63d4397114f 
>   src/tests/containerizer.hpp a778b8581904bacea9eec3ff50c3c009959b5dac 
>   src/tests/containerizer.cpp cd140f4263621a0a33a34b7e062a9ca6cf426e7a 
>   src/tests/containerizer/mock_containerizer.hpp 
> 0adcb01e6c12d6cc4abed1f14fa2df833ffc6569 
> 
> 
> Diff: https://reviews.apache.org/r/56721/diff/16/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 63799: Improved log messages of offer operations.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Nov. 14, 2017, 2:12 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63799/
> ---
> 
> (Updated Nov. 14, 2017, 2:12 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improved log messages of offer operations.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/message.hpp 05879cdc01dca88440ce89beb487168c033fab37 
>   src/slave/slave.cpp d8bacebc74790e955490a158c37ac0d9e75fd6b5 
> 
> 
> Diff: https://reviews.apache.org/r/63799/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 63798: Added resource provider support for all offer operations.

[Jie Yu]

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




src/slave/slave.cpp
Line 3737 (original), 3737 (patched)


Can you update `needCheckpointing` to just look at agent default resources?



src/slave/slave.cpp
Lines 3737-3749 (original), 3737-3742 (patched)


As I mentioned earlier, `chckpointResources` will overwrite 
`totalResources`, which is wrong with resource provider resources.



src/tests/resource_provider_manager_tests.cpp
Lines 810-811 (original), 833-834 (patched)


This does not sound like a valid sequence. I'm surprised that we allow 
CREATE on a RAW disk resources. Can you add some validation to disallow that?

Another suggestion is that we should add some default action to 
MockResourceProvider so that we can reduce the size of this test. The default 
behavior for handling an operation is to just return the converted resources 
(for new operations), and success for old operations. One can override the 
default behavior. Take a look at `TestAllocator`


- Jie Yu


On Nov. 15, 2017, 11:35 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63798/
> ---
> 
> (Updated Nov. 15, 2017, 11:35 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-8218
> https://issues.apache.org/jira/browse/MESOS-8218
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `RESERVE`/`UNRESERVE`/`CREATE`/`DESTROY` operations for
> reservations and persistent volumes have been enabled for resource
> provider resources. Similar to agent resources, they are speculatively
> applied to allow offer pipelining.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp d8edc5e6bbfa265bca4d19bbaa7db3063949dbc0 
>   src/tests/resource_provider_manager_tests.cpp 
> ecfe2b4c0952838d6312df603f8eb2f458725175 
> 
> 
> Diff: https://reviews.apache.org/r/63798/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Review Request 63853: Added the "task" prefix to the name of the status update manager files.

[Gaston Kleiman]

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

Review request for mesos and Greg Mann.


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


Repository: mesos


Description
---

Added the "task" prefix to the name of the status update manager files.


Diffs
-

  src/CMakeLists.txt 4f114184df067f4438c852cff4a40fa01861316a 
  src/Makefile.am 49dec55ced32945ad0414c32eb4e00247f4b54f2 
  src/local/local.cpp 99c1b087d75a1759ed61a4e6178e15919bee1e77 
  src/slave/main.cpp 94431b036524ef4db16a594c80f45b64a45728f7 
  src/slave/slave.cpp d8edc5e6bbfa265bca4d19bbaa7db3063949dbc0 
  src/slave/status_update_manager.hpp 14ead42f22e342d03c117eabe7af635b48052703 
  src/slave/status_update_manager.cpp e0d029b855de1024882a0db3c2556523240179e5 
  src/tests/cluster.hpp 65634121b7fe076a7cd9a1c5aa6101a96b1c009d 
  src/tests/cluster.cpp d657da6d84e5ea28e0dad180dce069de569c5d38 
  src/tests/mock_slave.cpp db24f9e5b71f558d2f811f0da8a9cc9c7c2dd341 
  src/tests/status_update_manager_tests.cpp 
24d6a9951b164369b2a5875e1c54b7e69e22d66b 


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


Testing
---

Tests still pass on GNU/Linux.


Thanks,

Gaston Kleiman

Review Request 63852: Added "Task" prefix to status update manager related classes/methods.

[Gaston Kleiman]

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

Review request for mesos and Greg Mann.


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


Repository: mesos


Description
---

We're about to add an offer operation status update manager and a
generic `StatusUpdateManagerProcess` class, which would conflict with
the name of the current classes/methods/variables.


Diffs
-

  src/local/local.cpp 99c1b087d75a1759ed61a4e6178e15919bee1e77 
  src/slave/main.cpp 94431b036524ef4db16a594c80f45b64a45728f7 
  src/slave/slave.hpp 40442f25ae1b223380e11f8602e6256c9203ef47 
  src/slave/slave.cpp d8edc5e6bbfa265bca4d19bbaa7db3063949dbc0 
  src/slave/status_update_manager.hpp 14ead42f22e342d03c117eabe7af635b48052703 
  src/slave/status_update_manager.cpp e0d029b855de1024882a0db3c2556523240179e5 
  src/tests/cluster.hpp 65634121b7fe076a7cd9a1c5aa6101a96b1c009d 
  src/tests/cluster.cpp d657da6d84e5ea28e0dad180dce069de569c5d38 
  src/tests/mock_slave.hpp 57189cee20511d145ae6b47a4dc2c66a14638dad 
  src/tests/mock_slave.cpp db24f9e5b71f558d2f811f0da8a9cc9c7c2dd341 
  src/tests/status_update_manager_tests.cpp 
24d6a9951b164369b2a5875e1c54b7e69e22d66b 


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


Testing
---

Tests still pass on GNU/Linux.


Thanks,

Gaston Kleiman

Re: Review Request 63751: Triggered 'ApplyOfferOperationMessage' for agent local resources.

[Jie Yu]

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




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


For old operations, I think we should apply the conversion (i.e., change 
`totalResources`) in `addOfferOperation` becuase we speculatively assume that 
it'll succeeds.

This makes sense for old operation that has a resource provider. The 
current code is buggy because we don't update agent `totalResources` for those 
old operations.

Then, you don't have to do this assignment here anymore. We still need to 
calculate the checkpointed resources so that we can call the old 
checkpointResources handler.

One unfortunate thing is that `checkpointResources` method will also set 
`totalResources`. This is in fact a bug:

```
  // This is a sanity check to verify that the new checkpointed
  // resources are compatible with the agent resources specified
  // through the '--resources' command line flag. The resources
  // should be guaranteed compatible by the master.
  Try _totalResources = applyCheckpointedResources(
  info.resources(),
  newCheckpointedResources);

  CHECK_SOME(_totalResources)
<< "Failed to apply checkpointed resources "
<< newCheckpointedResources << " to agent's resources "
<< info.resources();

  totalResources = _totalResources.get();
```

Setting agent resources to `_totalResources.get()` will break the agent 
resource accounting (if resource provider has resources). We might need to 
introduce a parameter to `checkpointResources` to not update `totalResources`.



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


Can you add a comment about what that `bool` represents? Ditto in 
ReservationTest



src/tests/persistent_volume_tests.cpp
Line 355 (original)


Hum, i don't expect you removing those checks. Just checking the offer is 
not sufficient because we speculate.

We should figure out a way to test that the reservation or persistent 
volume has actually been checkpointed.

One way is to add a helper in the fixture:
```
struct OperationMessage
{
  // The resources in the message.
  // 1) For CheckpointResourcesMessage, it's the checkpointed resources.
  // 2) For ApplyOfferOperationMessage, it's the resources in the operation.
  Resources resources;
};

Future message1 = getOperationMessage(slave.get()->pid);

...

AWAIT_READY(message1);
EXPECT_TRUE(message1.resources.contains(volume1));

AWAIT_READY(message2);
EXPECT_TRUE(message2.resources.contains(volume2));
```

Ditto elsewhere.


- Jie Yu


On Nov. 15, 2017, 4:06 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63751/
> ---
> 
> (Updated Nov. 15, 2017, 4:06 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-8211
> https://issues.apache.org/jira/browse/MESOS-8211
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Agents the have the 'RESOURCE_PROVIDER' capability set, should get sent
> 'ApplyOfferOperationMessage' instead of 'CheckpointResourcesMessage'.
> The agent will then figure out how to apply the operation. For agent
> local resources the agent will checkpoint resources.
> 
> 
> Diffs
> -
> 
>   src/common/protobuf_utils.hpp b2aa3654db2fe7d7d9d275ded81c6d54244654ee 
>   src/common/protobuf_utils.cpp 34054846f93f19ba550afe58e2a899d111ad38dc 
>   src/master/master.cpp 2ddd67ada3731803b00883b6a1f32b20c1bb238f 
>   src/slave/slave.cpp d8edc5e6bbfa265bca4d19bbaa7db3063949dbc0 
>   src/tests/persistent_volume_tests.cpp 
> acfeac16884b00581a3523607ff26f44f6dca53a 
>   src/tests/reservation_tests.cpp 470f7341686e69d0a71fb234a26b277c45c29780 
> 
> 
> Diff: https://reviews.apache.org/r/63751/diff/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 60620: Modifed os::write to write binary files on Windows.

[Michael Park]

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


Ship it!




Ship It!

- Michael Park


On Nov. 6, 2017, 10:11 a.m., Jeff Coffler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60620/
> ---
> 
> (Updated Nov. 6, 2017, 10:11 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, Li 
> Li, and Michael Park.
> 
> 
> Bugs: MESOS-6705
> https://issues.apache.org/jira/browse/MESOS-6705
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> By default, ::write operations on Windows will include a \r\n (CR/LF)
> at the end of each line, which is not compatible with Linux. Mesos
> expects only \n (LF) characters between lines.
> 
> This commit will modify Windows behavior to only include \n at the
> end of each line written to a text file.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/write.hpp 
> 9ff749f209e6dd6ca3695907108a029c9a2b4f05 
> 
> 
> Diff: https://reviews.apache.org/r/60620/diff/7/
> 
> 
> Testing
> ---
> 
> Built successfully on both Linux (with autotools and cmake) and Windows (with 
> cmake).
> 
> Ran stout-tests and mesos-tests successfully on both Windows and Linux.
> 
> 
> Thanks,
> 
> Jeff Coffler
> 
>

Review Request 63851: Changed backend validation logic from exclusive to inclusive.

[Meng Zhu]

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

Review request for mesos and Gilbert Song.


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


Repository: mesos


Description
---

Changed backend validation logic from exclusive to inclusive.


Diffs
-

  src/slave/containerizer/mesos/provisioner/provisioner.cpp 
450a3b32d69d2882973a6ed4e94e169a0256056b 


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


Testing
---

`make check` pass.


Thanks,

Meng Zhu

Re: Review Request 63678: Improved the signal safety of `ns::clone`.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 63678 was successfully built and tested.

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

- Mesos Reviewbot Windows


On Nov. 14, 2017, 7:11 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63678/
> ---
> 
> (Updated Nov. 14, 2017, 7:11 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-8159
> https://issues.apache.org/jira/browse/MESOS-8159
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `ns::clone` was implicitly using malloc to allocate a stack
> after fork but before exec, where we should be avoiding
> `malloc`.
> 
> Updated the second clone to use `os::signal_safe::clone`. We
> still explicitly allocate a stack after the fork, but since
> that is allocated with `mmap` is it safe in this particular
> case.
> 
> 
> Diffs
> -
> 
>   src/linux/ns.cpp 33751b518e95f4f67e6864728e1fdf621f50dd52 
> 
> 
> Diff: https://reviews.apache.org/r/63678/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check (Fedora 26)
> 
> NOTE: I tried to avoid changing `ns::clone` too much in this patch because 
> [MESOS-8155](https://issues.apache.org/jira/browse/MESOS-8155) and 
> [MESOS-8156](https://issues.apache.org/jira/browse/MESOS-8156) are also going 
> to also hit this code.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 62637: Added an object approver to authorize requests from resource providers.

[Joseph Wu]

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




src/authorizer/local/authorizer.cpp
Lines 558-567 (patched)


Note that the committed iteration of the standalone AuthZ code does not 
pass anything to the Approver.

It should be easy enough to:
1) Create an `ObjectApprover` object that takes an `ContainerID` i.e. Part 
of this discarded change: https://reviews.apache.org/r/62144/
2) Modify the 4 APIs with 
`s/!authorizer->accept()/!authorizer->accept(containerId)/`


- Joseph Wu


On Oct. 16, 2017, 4:15 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62637/
> ---
> 
> (Updated Oct. 16, 2017, 4:15 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas, Greg Mann, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-8100
> https://issues.apache.org/jira/browse/MESOS-8100
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds `LocalImplicitResourceProviderObjectApprover`, which
> authorize standalone container calls from a resource provider if the
> container IDs are prefixed with the namespace string.
> 
> 
> Diffs
> -
> 
>   src/authorizer/local/authorizer.cpp 
> 2fe7b879e649b13322cfcb300c21ef1ed0fea410 
> 
> 
> Diff: https://reviews.apache.org/r/62637/diff/2/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 63850: Used fully qualified namespace in the scheduler library for consistency.

[Gaston Kleiman]

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


Ship it!




<3

- Gaston Kleiman


On Nov. 15, 2017, 1:54 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63850/
> ---
> 
> (Updated Nov. 15, 2017, 1:54 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Gaston Kleiman, and Jie Yu.
> 
> 
> Bugs: MESOS-8198
> https://issues.apache.org/jira/browse/MESOS-8198
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used fully qualified namespace in the scheduler library for consistency.
> 
> 
> Diffs
> -
> 
>   src/scheduler/scheduler.cpp 286a7603352a1e639a0502e496d4409be11bf9d7 
> 
> 
> Diff: https://reviews.apache.org/r/63850/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 63680: Added a container daemon to monitor a long-running standalone container.

[Joseph Wu]

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




src/slave/container_daemon.hpp
Lines 40 (patched)


We typically don't use javadoc-style comments for internal classes.



src/slave/container_daemon.hpp
Lines 88-90 (patched)


Looks like the intent of this method is to return when a loop-breaking 
error occurs.  You should probably mention this.

And that the only way to retry (given the current interface) is to 
reconstruct the ContainerDaemon object.



src/slave/container_daemon.hpp
Lines 89 (patched)


s/discraded/discarded/



src/slave/container_daemon.cpp
Lines 136-147 (patched)


Style nit: Single newline needed before each `if` statement.

Also, try spacing the protobuf mutation like:
```
launchCall.mutable_launch_container()
  ->mutable_command()->CopyFrom(commandInfo.get());
```



src/slave/container_daemon.cpp
Lines 170 (patched)


s/contanier/container/



src/slave/container_daemon.cpp
Lines 188-198 (patched)


Unless you have plans to add more than one call site for this helper 
method, you should just inline the contents into `launchContainer`.  Same thing 
with `stopped` and `waitContainer`.



src/slave/container_daemon.cpp
Lines 216-219 (patched)


It doesn't seem necessary to have a mutex here, especially since all the 
calls are initiated from within this process.  There are no other actors that 
can Launch or Kill.

There's only one place where the Kill is used anyway.  So there's no way 
for the mutex to come into play.  When the `force` parameter is true, the 
control flow is basically linear:

kill -> wait -> 
  launch -> wait -> 
  ...


- Joseph Wu


On Nov. 15, 2017, 1:21 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63680/
> ---
> 
> (Updated Nov. 15, 2017, 1:21 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-8183
> https://issues.apache.org/jira/browse/MESOS-8183
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `ContanierDaemon` class is responsible to monitor if a long-running
> service running in a standalone container, and restart the service
> container after it exits. It takes the following hook functions:
> 
> * `postStartHook`: called after the container is launched every time.
> * `postStopHook`: called after the container exits every time.
> 
> `ContainerDaemon` does not manage the lifecycle of the contanier it
> monitors, so the container persists across `ContainerDaemon`s. However,
> it provides a `terminate()` method to completely shutdown the service
> container.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 955f01a665019d7750980ff16f126ad63b524594 
>   src/slave/container_daemon.hpp PRE-CREATION 
>   src/slave/container_daemon.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/63680/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 63639: Enabled rvalue reference parameters in protobuf handlers.

[Benjamin Mahler]

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


Ship it!





3rdparty/libprocess/include/process/protobuf.hpp
Lines 143-145 (patched)


As a separate change, it looks like allocating an M is not necessary, AFAICT

```
M::descriptor()->full_name()
```

is equivalent to

```
google::protobuf::Message* m = new M();
m->GetTypeName()
```


- Benjamin Mahler


On Nov. 7, 2017, 4:52 p.m., Dmitry Zhuk wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63639/
> ---
> 
> (Updated Nov. 7, 2017, 4:52 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Michael Park.
> 
> 
> Bugs: MESOS-6972
> https://issues.apache.org/jira/browse/MESOS-6972
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Using rvalue reference parameter in protobuf handler opts-out of arena
> usage, and allows moving message further in dispatch chain.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/protobuf.hpp 
> fe152f273332470ac50f9715291897bb04cf95b9 
> 
> 
> Diff: https://reviews.apache.org/r/63639/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Dmitry Zhuk
> 
>

Re: Review Request 63752: Updated documentation on protobuf version requirements.

[Benjamin Mahler]

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


Ship it!




Ship It!

- Benjamin Mahler


On Nov. 13, 2017, 1:35 p.m., Dmitry Zhuk wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63752/
> ---
> 
> (Updated Nov. 13, 2017, 1:35 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-8139
> https://issues.apache.org/jira/browse/MESOS-8139
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated documentation on protobuf version requirements.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/README.md 2637f1408403815bc7cd861b08796286118474b2 
> 
> 
> Diff: https://reviews.apache.org/r/63752/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Dmitry Zhuk
> 
>

Re: Review Request 63850: Used fully qualified namespace in the scheduler library for consistency.

[Greg Mann]

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

(Updated Nov. 15, 2017, 9:54 p.m.)


Review request for mesos, Benjamin Bannier, Gaston Kleiman, and Jie Yu.


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


Repository: mesos


Description
---

Used fully qualified namespace in the scheduler library for consistency.


Diffs
-

  src/scheduler/scheduler.cpp 286a7603352a1e639a0502e496d4409be11bf9d7 


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


Testing
---

make check


Thanks,

Greg Mann

Re: Review Request 63768: Added 'ReconcileOfferOperations' response to scheduler API.

[Greg Mann]

> On Nov. 14, 2017, 10:12 p.m., Gaston Kleiman wrote:
> > src/scheduler/scheduler.cpp
> > Lines 123-124 (original), 123 (patched)
> > 
> >
> > For consistency I think that it'd also be nice to remove the `using 
> > process::http::Request` statement.
> > 
> > Also I noticed that we use `::Request` in this file... so removing the 
> > `using` statement and replacing `::Request` with the more explicit 
> > `process::http::Request` feels to me like an improvement.

Good call; I added another patch to do this: https://reviews.apache.org/r/63850/


- Greg


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


On Nov. 14, 2017, 9:44 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63768/
> ---
> 
> (Updated Nov. 14, 2017, 9:44 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Gaston Kleiman, Jie Yu, and Jan 
> Schlicht.
> 
> 
> Bugs: MESOS-8198
> https://issues.apache.org/jira/browse/MESOS-8198
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a first-class `Response` message to the scheduler API
> along with its first member, the `ReconcileOfferOperations` response
> message.
> 
> 
> Diffs
> -
> 
>   include/mesos/scheduler/scheduler.proto 
> f5dda10a076ecae53a56875d0ed4d201a1c92f12 
>   include/mesos/v1/scheduler/scheduler.proto 
> 4fa6606b0db701aebe208b962b7768da68b180ab 
>   src/scheduler/scheduler.cpp 286a7603352a1e639a0502e496d4409be11bf9d7 
> 
> 
> Diff: https://reviews.apache.org/r/63768/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 63751: Triggered 'ApplyOfferOperationMessage' for agent local resources.

[Jie Yu]

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




src/slave/slave.cpp
Lines 3751-3757 (patched)


Are we leaking `OfferOperation` in the agent memory if we don't call 
`updateOfferOperation` and then `removeOfferOperation`?


- Jie Yu


On Nov. 15, 2017, 4:06 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63751/
> ---
> 
> (Updated Nov. 15, 2017, 4:06 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-8211
> https://issues.apache.org/jira/browse/MESOS-8211
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Agents the have the 'RESOURCE_PROVIDER' capability set, should get sent
> 'ApplyOfferOperationMessage' instead of 'CheckpointResourcesMessage'.
> The agent will then figure out how to apply the operation. For agent
> local resources the agent will checkpoint resources.
> 
> 
> Diffs
> -
> 
>   src/common/protobuf_utils.hpp b2aa3654db2fe7d7d9d275ded81c6d54244654ee 
>   src/common/protobuf_utils.cpp 34054846f93f19ba550afe58e2a899d111ad38dc 
>   src/master/master.cpp 2ddd67ada3731803b00883b6a1f32b20c1bb238f 
>   src/slave/slave.cpp d8edc5e6bbfa265bca4d19bbaa7db3063949dbc0 
>   src/tests/persistent_volume_tests.cpp 
> acfeac16884b00581a3523607ff26f44f6dca53a 
>   src/tests/reservation_tests.cpp 470f7341686e69d0a71fb234a26b277c45c29780 
> 
> 
> Diff: https://reviews.apache.org/r/63751/diff/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Review Request 63850: Used fully qualified namespace in the scheduler library for consistency.

[Greg Mann]

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

Review request for mesos and Gaston Kleiman.


Repository: mesos


Description
---

Used fully qualified namespace in the scheduler library for consistency.


Diffs
-

  src/scheduler/scheduler.cpp 286a7603352a1e639a0502e496d4409be11bf9d7 


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


Testing
---

make check


Thanks,

Greg Mann

Re: Review Request 63804: Added plumbing for operation reconciliation between master and agent.

[Greg Mann]

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

(Updated Nov. 15, 2017, 9:51 p.m.)


Review request for mesos, Benjamin Bannier, Gaston Kleiman, Jie Yu, and Jan 
Schlicht.


Changes
---

Rebase.


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


Repository: mesos


Description
---

This patch adds new internal protobuf messages to be used by the master
and agent when reconciling pending/unacknowledged offer operations.


Diffs (updated)
-

  include/mesos/resource_provider/resource_provider.proto 
1e44b952691fa57b546e979bc5876df3d82d746f 
  src/messages/messages.proto 33732e28d087a17080eb9aa7d9a91c367172be7f 
  src/resource_provider/storage/provider.cpp 
46224997430ac0c568904d80014166a6f059907f 
  src/slave/slave.hpp 40442f25ae1b223380e11f8602e6256c9203ef47 
  src/slave/slave.cpp d8edc5e6bbfa265bca4d19bbaa7db3063949dbc0 


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

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


Testing
---

make check


Thanks,

Greg Mann

Re: Review Request 62777: Fixed build dependency for `protobuf_tests.proto`.

[Chun-Hung Hsiao]

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

(Updated Nov. 15, 2017, 9:40 p.m.)


Review request for mesos and Benjamin Bannier.


Changes
---

Addressed bbannier's comment.


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


Repository: mesos


Description
---

The protobuf hpp and cpp files for `protobuf_tests.proto` should be
built after `BUNDLED_DEPS`, not within it.


Diffs (updated)
-

  3rdparty/stout/Makefile.am 4386017acd6ca465be3f735460c11d50b440ccc5 


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

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


Testing
---

make -j4 check


Thanks,

Chun-Hung Hsiao

Re: Review Request 62777: Fixed build dependency for `protobuf_tests.proto`.

[Benjamin Bannier]

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


Fix it, then Ship it!





3rdparty/stout/Makefile.am
Line 122 (original), 122 (patched)


The dependency on protobuf only works if we used the bundled protobuf; for 
an unbundled protobuf the build stamp file would never get created.

Could we instead depend on `BUNDLED_DEPS`? While this might build more than 
we want, I believe it should be populated correctly regardless of whether we 
use bundled deps or not.


- Benjamin Bannier


On Oct. 13, 2017, 8:24 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62777/
> ---
> 
> (Updated Oct. 13, 2017, 8:24 p.m.)
> 
> 
> Review request for mesos and Benjamin Bannier.
> 
> 
> Bugs: MESOS-8052
> https://issues.apache.org/jira/browse/MESOS-8052
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The protobuf hpp and cpp files for `protobuf_tests.proto` should be
> built after `BUNDLED_DEPS`, not within it.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/Makefile.am 4386017acd6ca465be3f735460c11d50b440ccc5 
> 
> 
> Diff: https://reviews.apache.org/r/62777/diff/2/
> 
> 
> Testing
> ---
> 
> make -j4 check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 63680: Added a container daemon to monitor a long-running standalone container.

[Chun-Hung Hsiao]

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

(Updated Nov. 15, 2017, 9:21 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, and Joseph Wu.


Changes
---

Added a `force` option to force a clean restart during deamon creation.


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


Repository: mesos


Description
---

The `ContanierDaemon` class is responsible to monitor if a long-running
service running in a standalone container, and restart the service
container after it exits. It takes the following hook functions:

* `postStartHook`: called after the container is launched every time.
* `postStopHook`: called after the container exits every time.

`ContainerDaemon` does not manage the lifecycle of the contanier it
monitors, so the container persists across `ContainerDaemon`s. However,
it provides a `terminate()` method to completely shutdown the service
container.


Diffs (updated)
-

  src/Makefile.am 955f01a665019d7750980ff16f126ad63b524594 
  src/slave/container_daemon.hpp PRE-CREATION 
  src/slave/container_daemon.cpp PRE-CREATION 


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

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


Testing
---


Thanks,

Chun-Hung Hsiao

Re: Review Request 63842: Allowed removing non-terminal offer operations.

[Jie Yu]

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




src/master/master.cpp
Lines 9816-9817 (original)


If you remove this CHECK, i'd like to see the resoures used by the 
operation being released. For instance, if a new operation is in non terminal 
status, you want to release the resources that it currently uses when removing 
the operation. That ensures that we don't leak any resources.


- Jie Yu


On Nov. 15, 2017, 5:31 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63842/
> ---
> 
> (Updated Nov. 15, 2017, 5:31 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Jie Yu, and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> During reconcilation we might be required to remove non-terminal offer
> operations from bookkeeping.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp a309fc78ee2613762f3d5d22ac7559afc7aac4a3 
>   src/master/master.cpp 59a533940736f5cfd5ec31e0ed924f0b2ab13f9c 
>   src/slave/slave.cpp d8bacebc74790e955490a158c37ac0d9e75fd6b5 
> 
> 
> Diff: https://reviews.apache.org/r/63842/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 63815: Windows: Fixed environment priorities in `shell.hpp`.

[Andrew Schwartzmeyer]

> On Nov. 15, 2017, 12:53 p.m., John Kordich wrote:
> > Nice fix, that would have been annoying otherwise.

Indeed, and we need to follow it up with properly propogating `GLOG_v`.


- Andrew


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


On Nov. 14, 2017, 3:16 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63815/
> ---
> 
> (Updated Nov. 14, 2017, 3:16 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Jie Yu, John Kordich, Joseph Wu, and 
> Michael Park.
> 
> 
> Bugs: MESOS-6816
> https://issues.apache.org/jira/browse/MESOS-6816
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Legacy code had caused the system environment to explicitly override the
> supplied environment, but this is incorrect. The system environment
> should be the default, with any supplied environment variables
> overriding those.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/shell.hpp 
> 5c711837a3adbc89ad8793666e68eff7102566b3 
> 
> 
> Diff: https://reviews.apache.org/r/63815/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 63810: Windows: Added internal `fullpath` API to normalize paths.

[Andrew Schwartzmeyer]

> On Nov. 15, 2017, 12:38 p.m., John Kordich wrote:
> > 3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp
> > Lines 178 (patched)
> > 
> >
> > I've seen this function "stringify" before, but I don't have any 
> > context about it. Does it convert from wide string to UTF-8?  Do we need to 
> > worry about non-ascii paths?

It's [right 
here](https://github.com/apache/mesos/blob/0eec1f39a60d75547b1ca714f193df0bda7beb7a/3rdparty/stout/include/stout/stringify.hpp#L59),
 it converts UTF-16 to UTF-8. Non-ASCII is therefore handled (to the extent 
that the rest of Mesos handles them).


- Andrew


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


On Nov. 14, 2017, 3:14 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63810/
> ---
> 
> (Updated Nov. 14, 2017, 3:14 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Jie Yu, John Kordich, Joseph Wu, and 
> Michael Park.
> 
> 
> Bugs: MESOS-6735
> https://issues.apache.org/jira/browse/MESOS-6735
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This explicitly does not check for existence, nor does it follow
> symlinks. It simply normalizes a given path to an absolute path. This
> removes the `reparsepoint.hpp` and `symlink.hpp` dependency os
> `os::realpath`.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp 
> e3d04480cf793482d958ea9571d96a29b6bf09b1 
>   3rdparty/stout/include/stout/internal/windows/symlink.hpp 
> b9cf1229ec0b6c70fe9b9d358e867e91e475dfaa 
>   3rdparty/stout/include/stout/windows/fs.hpp 
> d7b883ca1f3f8972e1bf9fbd211cc564c7c30f14 
> 
> 
> Diff: https://reviews.apache.org/r/63810/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 63816: Windows: Fixed MESOS-6816 to enable `ExecutorEnvironmentVariables`.

[John Kordich via Review Board]

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


Ship it!




- John Kordich


On Nov. 14, 2017, 11:17 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63816/
> ---
> 
> (Updated Nov. 14, 2017, 11:17 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Jie Yu, John Kordich, Joseph Wu, and 
> Michael Park.
> 
> 
> Bugs: MESOS-6816
> https://issues.apache.org/jira/browse/MESOS-6816
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> By removing the explicit system environment override code, and instead
> setting the system environment as the default in the `CreateProcess`
> wrapper, the `SlaveTest.ExecutorEnvironmentVariables` can be enabled.
> Note that this also required fixing `os::host_default_path()` and using
> it, because the test used `PATH = /bin`, which breaks on Windows.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp 292eecbca246edf068ec8c262aff4f3ce9cd8c67 
>   src/slave/containerizer/mesos/launch.cpp 
> b1584ff292ada5463917792908a13e00859fd1ae 
>   src/tests/slave_tests.cpp 61dd940e4452a1dbc6c0361bfc917bdc2dd16be8 
> 
> 
> Diff: https://reviews.apache.org/r/63816/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 63815: Windows: Fixed environment priorities in `shell.hpp`.

[John Kordich via Review Board]

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


Ship it!




Nice fix, that would have been annoying otherwise.

- John Kordich


On Nov. 14, 2017, 11:16 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63815/
> ---
> 
> (Updated Nov. 14, 2017, 11:16 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Jie Yu, John Kordich, Joseph Wu, and 
> Michael Park.
> 
> 
> Bugs: MESOS-6816
> https://issues.apache.org/jira/browse/MESOS-6816
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Legacy code had caused the system environment to explicitly override the
> supplied environment, but this is incorrect. The system environment
> should be the default, with any supplied environment variables
> overriding those.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/shell.hpp 
> 5c711837a3adbc89ad8793666e68eff7102566b3 
> 
> 
> Diff: https://reviews.apache.org/r/63815/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 63814: Windows: Fixed `os::host_default_path()`.

[John Kordich via Review Board]

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


Ship it!




- John Kordich


On Nov. 14, 2017, 11:16 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63814/
> ---
> 
> (Updated Nov. 14, 2017, 11:16 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Jie Yu, John Kordich, Joseph Wu, and 
> Michael Park.
> 
> 
> Bugs: MESOS-6816
> https://issues.apache.org/jira/browse/MESOS-6816
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This adds the necessary and expected default paths to the default `PATH`
> value represented by this function. Especially needed was the
> `WindowsPowerShell` folder so that tasks using PowerShell can run.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/windows/os.hpp 
> 09ddec6d69472cd13b453fe1a77fdbe343fc23c8 
> 
> 
> Diff: https://reviews.apache.org/r/63814/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 63812: Windows: Fixed `os::realpath` to behave like POSIX version.

[John Kordich via Review Board]

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


Ship it!




- John Kordich


On Nov. 14, 2017, 11:14 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63812/
> ---
> 
> (Updated Nov. 14, 2017, 11:14 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Jie Yu, John Kordich, Joseph Wu, and 
> Michael Park.
> 
> 
> Bugs: MESOS-6735
> https://issues.apache.org/jira/browse/MESOS-6735
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The existing implementation (1) did not resolve symlinks and (2) did not
> check for existence of the path. This resulted in unexpected behaviors
> by users of the function.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/realpath.hpp 
> 81a33bd2708c0871c4f23c12fdd29fc8d35682e3 
> 
> 
> Diff: https://reviews.apache.org/r/63812/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 63813: Windows: Fixed name of default executor.

[John Kordich via Review Board]

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


Ship it!




- John Kordich


On Nov. 14, 2017, 11:15 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63813/
> ---
> 
> (Updated Nov. 14, 2017, 11:15 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Jie Yu, John Kordich, Joseph Wu, and 
> Michael Park.
> 
> 
> Bugs: MESOS-6735
> https://issues.apache.org/jira/browse/MESOS-6735
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This path is checked with `os::realpath`, which now correctly errors if
> the path does not exist. Therefore the name must be correct.
> 
> 
> Diffs
> -
> 
>   src/slave/constants.hpp 401590605350ef1749b45b32c659afdaa43f43ad 
> 
> 
> Diff: https://reviews.apache.org/r/63813/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 63811: Windows: Added `get_handle_follow` which follows symlinks.

[John Kordich via Review Board]

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


Ship it!




I'm not an expert, but I don't see any problems here. Now I know how to 
dereference Windows Symlinks when opening them using the Windows API. Cool!

- John Kordich


On Nov. 14, 2017, 11:14 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63811/
> ---
> 
> (Updated Nov. 14, 2017, 11:14 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Jie Yu, John Kordich, Joseph Wu, and 
> Michael Park.
> 
> 
> Bugs: MESOS-6735
> https://issues.apache.org/jira/browse/MESOS-6735
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is the opposite of `get_handle_no_follow`. They both use the
> inappropriately named Windows API `CreateFile` to retrieve a `HANDLE` to
> an existing file or directory. This existing `get_handle_no_follow`
> explicitly gets a handle to the symlink itself, and the new
> `get_handle_follow` explicitly resolves symlinks and gets a handle to
> the target.
> 
> This may fail if the target doesn't exist.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp 
> e3d04480cf793482d958ea9571d96a29b6bf09b1 
> 
> 
> Diff: https://reviews.apache.org/r/63811/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 63810: Windows: Added internal `fullpath` API to normalize paths.

[John Kordich via Review Board]

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


Fix it, then Ship it!





3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp
Lines 178 (patched)


I've seen this function "stringify" before, but I don't have any context 
about it. Does it convert from wide string to UTF-8?  Do we need to worry about 
non-ascii paths?


- John Kordich


On Nov. 14, 2017, 11:14 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63810/
> ---
> 
> (Updated Nov. 14, 2017, 11:14 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Jie Yu, John Kordich, Joseph Wu, and 
> Michael Park.
> 
> 
> Bugs: MESOS-6735
> https://issues.apache.org/jira/browse/MESOS-6735
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This explicitly does not check for existence, nor does it follow
> symlinks. It simply normalizes a given path to an absolute path. This
> removes the `reparsepoint.hpp` and `symlink.hpp` dependency os
> `os::realpath`.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp 
> e3d04480cf793482d958ea9571d96a29b6bf09b1 
>   3rdparty/stout/include/stout/internal/windows/symlink.hpp 
> b9cf1229ec0b6c70fe9b9d358e867e91e475dfaa 
>   3rdparty/stout/include/stout/windows/fs.hpp 
> d7b883ca1f3f8972e1bf9fbd211cc564c7c30f14 
> 
> 
> Diff: https://reviews.apache.org/r/63810/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 63844: Removed acknowledged offer operation status updates.

[Greg Mann]

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




src/slave/slave.cpp
Lines 3791-3798 (patched)


The `operation_uuid` here is the Mesos internal UUID, not the 
framework-generated `OperationID`; the translation between these two occurs in 
the master, so that the agent/RP never needs to interact with the 
framework-generated ID.

So we probably don't need the quotes around this UUID in our logging, and 
perhaps we could use a `CHECK_SOME` instead of the `if` after we convert to 
`UUID`?


- Greg Mann


On Nov. 15, 2017, 5:31 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63844/
> ---
> 
> (Updated Nov. 15, 2017, 5:31 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Jie Yu, and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp d8bacebc74790e955490a158c37ac0d9e75fd6b5 
> 
> 
> Diff: https://reviews.apache.org/r/63844/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 63809: Windows: Fixed symlink code to not need admin privileges.

[John Kordich via Review Board]

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


Ship it!




- John Kordich


On Nov. 14, 2017, 11:14 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63809/
> ---
> 
> (Updated Nov. 14, 2017, 11:14 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Jie Yu, John Kordich, Joseph Wu, and 
> Michael Park.
> 
> 
> Bugs: MESOS-7370
> https://issues.apache.org/jira/browse/MESOS-7370
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This new feature of Windows 10 allows us to make symlinks without being
> an administrator. In the future, we may want to do a version check, as
> this is likely to fail on versions of Windows which don't support this
> flag. Resolves MESOS-7370.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp 
> e3d04480cf793482d958ea9571d96a29b6bf09b1 
> 
> 
> Diff: https://reviews.apache.org/r/63809/diff/1/
> 
> 
> Testing
> ---
> 
> Ran `mesos-tests` not in an admin prompt!
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 63846: Updated test PortMappingMesosTest.CGROUPS_ROOT_CleanUpOrphan.

[Benno Evers]

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

(Updated Nov. 15, 2017, 8:14 p.m.)


Review request for mesos and Alexander Rukletsov.


Repository: mesos


Description
---

This test needed to be adapted to the new behaviour of the
built-in executors that are now sending TASK_STARTING
updates.


Diffs
-

  src/tests/containerizer/port_mapping_tests.cpp 
7b0c3e361d5fd66ec8d2a3fa4cc0aece3d1e9d54 


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


Testing (updated)
---

make check on various linux distros


Thanks,

Benno Evers

Re: Review Request 63846: Updated test PortMappingMesosTest.CGROUPS_ROOT_CleanUpOrphan.

[Alexander Rukletsov]

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


Ship it!




Ship It!

- Alexander Rukletsov


On Nov. 15, 2017, 6:15 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63846/
> ---
> 
> (Updated Nov. 15, 2017, 6:15 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test needed to be adapted to the new behaviour of the
> built-in executors that are now sending TASK_STARTING
> updates.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/port_mapping_tests.cpp 
> 7b0c3e361d5fd66ec8d2a3fa4cc0aece3d1e9d54 
> 
> 
> Diff: https://reviews.apache.org/r/63846/diff/1/
> 
> 
> Testing
> ---
> 
> https://jenkins.mesosphere.com/service/jenkins/view/Mesos/job/mesos/job/Mesos_CI-build/2159/
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 63848: Added test to check standalone containers in GET_CONTAINERS.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Nov. 15, 2017, 7:42 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63848/
> ---
> 
> (Updated Nov. 15, 2017, 7:42 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Standalone containers are *NOT* expected to show up in the output
> of the agent GET_CONTAINERS call.  This is because the standalone
> containers lack some of the required metadata, like a FrameworkID,
> ExecutorID, and Executor name.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp c9c50b91ddd7c3317c1188f878753ad3bd5c3941 
> 
> 
> Diff: https://reviews.apache.org/r/63848/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Review Request 63848: Added test to check standalone containers in GET_CONTAINERS.

[Joseph Wu]

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

Review request for mesos, Gilbert Song and Jie Yu.


Repository: mesos


Description
---

Standalone containers are *NOT* expected to show up in the output
of the agent GET_CONTAINERS call.  This is because the standalone
containers lack some of the required metadata, like a FrameworkID,
ExecutorID, and Executor name.


Diffs
-

  src/tests/api_tests.cpp c9c50b91ddd7c3317c1188f878753ad3bd5c3941 


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


Testing
---

make check


Thanks,

Joseph Wu

Re: Review Request 63818: Added quick start instructions for CMake.

[Andrew Schwartzmeyer]

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

(Updated Nov. 15, 2017, 11:08 a.m.)


Review request for mesos, Gaston Kleiman and Greg Mann.


Changes
---

Updated.


Repository: mesos


Description
---

Added quick start instructions for CMake.


Diffs (updated)
-

  docs/cmake.md b8f9e7caa84372404256767a0220ac1df9db8bc8 


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

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


Testing
---


Thanks,

Andrew Schwartzmeyer

Re: Review Request 63731: Reconciled pending resource provider operations in agent.

[Benjamin Bannier]

> On Nov. 13, 2017, 11:07 p.m., Greg Mann wrote:
> > src/slave/slave.cpp
> > Lines 3800-3803 (patched)
> > 
> >
> > Am I correct in thinking that we usually don't use quotes around IDs 
> > that we generate ourselves in Mesos? We may be able to remove the quotes 
> > around `operation_uuid` here.

I explicitly put quotes here since the `UUID` which probably comes from outside 
(i.e., a framework) could _not_ be deserialized which is suspicous. Makes 
sense? In any case I'll move this change to a separate RR, 
https://reviews.apache.org/r/63844/.


> On Nov. 13, 2017, 11:07 p.m., Greg Mann wrote:
> > src/slave/slave.cpp
> > Lines 3814 (patched)
> > 
> >
> > This currently assumes that all operation update acknowledgements are 
> > for terminal operation updates - is the plan to write it this way for now, 
> > and then update later if we add intermediate, non-terminal operation states?

I added a patch to remove that assumption, https://reviews.apache.org/r/63842/.


> On Nov. 13, 2017, 11:07 p.m., Greg Mann wrote:
> > src/slave/slave.cpp
> > Lines 6777 (patched)
> > 
> >
> > Note that in `addOfferOperation()`, we do `CHECK_SOME(uuid)` after 
> > grabbing `operation.operation_uuid`.
> > 
> > I'm not sure that always checking the error case is necessary when the 
> > messages are generated by Mesos, but I mention it here because in the 
> > `offerOperationUpdateAcknowledgement` handler, you're performing an `if 
> > (operationUuid.isError())` check on a UUID out of a message from master, so 
> > the treatment of errors seems inconsistent.

These `UUID`s come from us (a resource provider) while above `UUID` came from a 
framework. I think that justifies the differing treatment.


- Benjamin


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


On Nov. 15, 2017, 6:31 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63731/
> ---
> 
> (Updated Nov. 15, 2017, 6:31 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Jie Yu, and Jan Schlicht.
> 
> 
> Bugs: MESOS-8207
> https://issues.apache.org/jira/browse/MESOS-8207
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When resource providers update their state they send a list of
> pending or unacknowledged operations to the agent. This patch add
> tracking for such operations to the agent. The agent can then forward
> these operations to the master, e.g., for calculating the unused
> resources behind an agent.
> 
> We track an operation until we either receive a updated list of
> pending or unacknowledged operations from a resource provider, or
> until we see an acknowledgement from a framework. This keeps the list
> of operations bounded and ensures that we maintain the latest
> information in the agent.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp d8bacebc74790e955490a158c37ac0d9e75fd6b5 
> 
> 
> Diff: https://reviews.apache.org/r/63731/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`, still need to implement dedicated tests.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Review Request 63830: Fixed 'NoOffersWithAllRolesSuppressed' test.

[Jiang Yan Xu]

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

Review request for mesos and Alexander Rukletsov.


Repository: mesos


Description
---

The previous iteration of the test failed to reveal the bug in MESOS-8200 due 
to a race condition.


Diffs
-

  src/tests/scheduler_tests.cpp 45fc9c0cfccdb22c2e3e8d5de30c04575814a0e9 


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


Testing
---

As expected, this test would reliably fail without /r/63741/ with the following

```
I1115 07:48:28.536093 3117031 scheduler.cpp:741] Enqueuing event SUBSCRIBED 
received from http://17.121.128.13:38423/master/api/v1/scheduler
I1115 07:48:28.536224 3117031 scheduler.cpp:741] Enqueuing event HEARTBEAT 
received from http://17.121.128.13:38423/master/api/v1/scheduler
I1115 07:48:28.536244 3117029 master.cpp:8115] Sending 1 offers to framework 
560bd0e1-8ab7-4e7e-be7a-54cb5f399531- (default)
I1115 07:48:28.536650 3117029 scheduler.cpp:741] Enqueuing event OFFERS 
received from http://17.121.128.13:38423/master/api/v1/scheduler
../../src/tests/scheduler_tests.cpp:1497: Failure
Mock function called more times than expected - returning directly.
Function call: offers(0x7fffabd197a0, @0x7f7d8c005460 48-byte object <50-98 
0B-B0 7D-7F 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 
00-00 00-00 01-00 00-00 04-00 00-00 A0-54 00-8C 7D-7F 00-00>)
 Expected: to be never called
   Actual: called once - over-saturated and active
```

The test passes with /r/63741/.


Thanks,

Jiang Yan Xu

Review Request 63741: Fixed framework subscription with suppressed roles.

[Jiang Yan Xu]

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

Review request for mesos and Alexander Rukletsov.


Repository: mesos


Description
---

Also improved logging to show the suppressed roles.


Diffs
-

  src/internal/devolve.cpp 3a02490d734ef4a558b448871f9a1054d832f457 
  src/master/master.cpp 81192d5ce7c095495a5faa534d4792601e540e0e 


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


Testing
---

Tested together with /r/63830/.


Thanks,

Jiang Yan Xu

Re: Review Request 63818: Added quick start instructions for CMake.

[Andrew Schwartzmeyer]

> On Nov. 14, 2017, 5:40 p.m., Gaston Kleiman wrote:
> > docs/cmake.md
> > Lines 43 (patched)
> > 
> >
> > I had to look up how to pass more parameters to make in order to use 
> > more threads:
> > 
> > `cmake --build . -- -j 4`
> > 
> > It might be useful to mention that here =).

Oh yeah, that's a good point. I usually run `-- /m` on Windows. I'll add it.


- Andrew


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


On Nov. 14, 2017, 3:12 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63818/
> ---
> 
> (Updated Nov. 14, 2017, 3:12 p.m.)
> 
> 
> Review request for mesos, Gaston Kleiman and Greg Mann.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added quick start instructions for CMake.
> 
> 
> Diffs
> -
> 
>   docs/cmake.md b8f9e7caa84372404256767a0220ac1df9db8bc8 
> 
> 
> Diff: https://reviews.apache.org/r/63818/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 63818: Added quick start instructions for CMake.

[Greg Mann]

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


Ship it!




Thanks for the doc update!!!


docs/cmake.md
Lines 50 (patched)


s/is/produces/

I can fix this when committing.


- Greg Mann


On Nov. 14, 2017, 11:12 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63818/
> ---
> 
> (Updated Nov. 14, 2017, 11:12 p.m.)
> 
> 
> Review request for mesos, Gaston Kleiman and Greg Mann.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added quick start instructions for CMake.
> 
> 
> Diffs
> -
> 
>   docs/cmake.md b8f9e7caa84372404256767a0220ac1df9db8bc8 
> 
> 
> Diff: https://reviews.apache.org/r/63818/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Review Request 63846: Updated test PortMappingMesosTest.CGROUPS_ROOT_CleanUpOrphan.

[Benno Evers]

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

Review request for mesos and Alexander Rukletsov.


Repository: mesos


Description
---

This test needed to be adapted to the new behaviour of the
built-in executors that are now sending TASK_STARTING
updates.


Diffs
-

  src/tests/containerizer/port_mapping_tests.cpp 
7b0c3e361d5fd66ec8d2a3fa4cc0aece3d1e9d54 


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


Testing
---

https://jenkins.mesosphere.com/service/jenkins/view/Mesos/job/mesos/job/Mesos_CI-build/2159/


Thanks,

Benno Evers

Re: Review Request 63730: Passed operations from resource provider to agent.

[Benjamin Bannier]

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

(Updated Nov. 15, 2017, 6:31 p.m.)


Review request for mesos, Jie Yu and Jan Schlicht.


Changes
---

Addressed comment from Jan.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/resource_provider/manager.cpp 6dfc42900a1e4249f37cec585f7fe50f5aa94e43 
  src/resource_provider/message.hpp 05879cdc01dca88440ce89beb487168c033fab37 


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

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


Testing
---

`make check`, still need to implement dedicated tests.


Thanks,

Benjamin Bannier

Review Request 63842: Allowed removing non-terminal offer operations.

[Benjamin Bannier]

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

Review request for mesos, Greg Mann, Jie Yu, and Jan Schlicht.


Repository: mesos


Description
---

During reconcilation we might be required to remove non-terminal offer
operations from bookkeeping.


Diffs
-

  src/master/master.hpp a309fc78ee2613762f3d5d22ac7559afc7aac4a3 
  src/master/master.cpp 59a533940736f5cfd5ec31e0ed924f0b2ab13f9c 
  src/slave/slave.cpp d8bacebc74790e955490a158c37ac0d9e75fd6b5 


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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Review Request 63844: Removed acknowledged offer operation status updates.

[Benjamin Bannier]

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

Review request for mesos, Greg Mann, Jie Yu, and Jan Schlicht.


Repository: mesos


Description
---

See summary.


Diffs
-

  src/slave/slave.cpp d8bacebc74790e955490a158c37ac0d9e75fd6b5 


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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 63732: Reconciled offer operations between agent and master.

[Benjamin Bannier]

> On Nov. 14, 2017, 4:14 p.m., Jan Schlicht wrote:
> > src/master/master.cpp
> > Lines 7093 (patched)
> > 
> >
> > This needs to handle the case where the master might have offer 
> > operations that the agent doesn't report here, e.g. after an agent 
> > failover. We should send operation feedback to frameworks in that case, 
> > indicating an error with the operation. Otherwise frameworks will receive 
> > no feedback for these operations.

For now I added a `TODO` to that effect here and in the agent code.


- Benjamin


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


On Nov. 15, 2017, 6:31 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63732/
> ---
> 
> (Updated Nov. 15, 2017, 6:31 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Bugs: MESOS-8207
> https://issues.apache.org/jira/browse/MESOS-8207
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 59a533940736f5cfd5ec31e0ed924f0b2ab13f9c 
> 
> 
> Diff: https://reviews.apache.org/r/63732/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`, still need to implement dedicated tests.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Review Request 63843: Implemented a test of offer operation reconcilation.

[Benjamin Bannier]

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

Review request for mesos, Greg Mann, Jie Yu, and Jan Schlicht.


Repository: mesos


Description
---

Whenever a speculated operation fails in a resource provider, we
expect the agent to trigger a 'UpdateSlaveMessage' to the master so it
can rollback its agent state. This patch adds such a test.


Diffs
-

  src/tests/slave_tests.cpp 61dd940e4452a1dbc6c0361bfc917bdc2dd16be8 


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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 63732: Reconciled offer operations between agent and master.

[Benjamin Bannier]

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

(Updated Nov. 15, 2017, 6:31 p.m.)


Review request for mesos, Jie Yu and Jan Schlicht.


Changes
---

Applied operation in the right place.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/master/master.cpp 59a533940736f5cfd5ec31e0ed924f0b2ab13f9c 


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

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


Testing
---

`make check`, still need to implement dedicated tests.


Thanks,

Benjamin Bannier

Re: Review Request 63731: Reconciled pending resource provider operations in agent.

[Benjamin Bannier]

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

(Updated Nov. 15, 2017, 6:31 p.m.)


Review request for mesos, Greg Mann, Jie Yu, and Jan Schlicht.


Changes
---

Broke out acknowledgement changes into separate patch.


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


Repository: mesos


Description
---

When resource providers update their state they send a list of
pending or unacknowledged operations to the agent. This patch add
tracking for such operations to the agent. The agent can then forward
these operations to the master, e.g., for calculating the unused
resources behind an agent.

We track an operation until we either receive a updated list of
pending or unacknowledged operations from a resource provider, or
until we see an acknowledgement from a framework. This keeps the list
of operations bounded and ensures that we maintain the latest
information in the agent.


Diffs (updated)
-

  src/slave/slave.cpp d8bacebc74790e955490a158c37ac0d9e75fd6b5 


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

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


Testing
---

`make check`, still need to implement dedicated tests.


Thanks,

Benjamin Bannier

Re: Review Request 63577: Fixed a task status update race in default executor tests.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 63577 was successfully built and tested.

Reviews applied: `['63577']`

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

- Mesos Reviewbot Windows


On Nov. 15, 2017, 6:21 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63577/
> ---
> 
> (Updated Nov. 15, 2017, 6:21 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Gaston Kleiman.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously in the test `DefaultExecutorTest.KillMultipleTasks` and
> `DefaultExecutorTest.KillTaskGroupOnTaskFailure`, when launching a
> task group which has multiple tasks, we expected the scheduler will
> receive all the TASK_STARTING status updates before receiving any
> TASK_RUNNING status updates. However this is not guaranteed, e.g.,
> it is possible for the scheduler to receive TASK_RUNNING for the
> first task before receiving TASK_STARTING for the second task.
> 
> So in this patch, we used `Sequence` to guarantee the order of
> TASK_STARTING and TASK_RUNNING for each task but do not care about
> the order between tasks.
> 
> The following 3 tests have their own solutions to handle this issue,
> in this patch, I updated them to use the above solution.
>   `DefaultExecutorTest.KillTask`
>   `DefaultExecutorTest.CommitSuicideOnKillTask`
>   `DefaultExecutorTest.ROOT_ContainerStatusForTask`
> 
> 
> Diffs
> -
> 
>   src/tests/default_executor_tests.cpp 
> 65113985acc0a8fac00374b074ef568aaa22c7ac 
>   src/tests/mesos.hpp e25defeb55608136e77363aa48cf820092a13a59 
> 
> 
> Diff: https://reviews.apache.org/r/63577/diff/3/
> 
> 
> Testing
> ---
> 
> This patch touched 5 tests:
> 1. KillTask
> 2. KillMultipleTasks
> 3. KillTaskGroupOnTaskFailure
> 4. CommitSuicideOnKillTask
> 5. ROOT_ContainerStatusForTask
> 
> I ran each of them repeatedly (20 times), and all of them succeeded.
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 63751: Triggered 'ApplyOfferOperationMessage' for agent local resources.

[Jan Schlicht]

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

(Updated Nov. 15, 2017, 5:06 p.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


Changes
---

Parameterized reservation and persistent volume tests.


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


Repository: mesos


Description
---

Agents the have the 'RESOURCE_PROVIDER' capability set, should get sent
'ApplyOfferOperationMessage' instead of 'CheckpointResourcesMessage'.
The agent will then figure out how to apply the operation. For agent
local resources the agent will checkpoint resources.


Diffs (updated)
-

  src/common/protobuf_utils.hpp b2aa3654db2fe7d7d9d275ded81c6d54244654ee 
  src/common/protobuf_utils.cpp 34054846f93f19ba550afe58e2a899d111ad38dc 
  src/master/master.cpp 2ddd67ada3731803b00883b6a1f32b20c1bb238f 
  src/slave/slave.cpp d8edc5e6bbfa265bca4d19bbaa7db3063949dbc0 
  src/tests/persistent_volume_tests.cpp 
acfeac16884b00581a3523607ff26f44f6dca53a 
  src/tests/reservation_tests.cpp 470f7341686e69d0a71fb234a26b277c45c29780 


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

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


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 63833: Added context to new agent processing calls.

[Alexander Rukletsov]

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


Ship it!




Ship It!

- Alexander Rukletsov


On Nov. 15, 2017, 12:12 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63833/
> ---
> 
> (Updated Nov. 15, 2017, 12:12 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-7305
> https://issues.apache.org/jira/browse/MESOS-7305
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds context information to log entries for agent HTTP
> API handlers. It impacts the calls `LAUNCH_CONTAINER`,
> `WAIT_CONTAINER`, `KILL_CONTAINER`, and `REMOVE_CONTAINER`.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp c4d8098df8cf9dd2cc2301b76460dd5e3bdb2bad 
> 
> 
> Diff: https://reviews.apache.org/r/63833/diff/1/
> 
> 
> Testing
> ---
> 
> ```
> $ make check
> ```
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Review Request 63833: Added context to new agent processing calls.

[Armand Grillet]

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

Review request for mesos, Alexander Rukletsov, Jie Yu, and Joseph Wu.


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


Repository: mesos


Description
---

This change adds context information to log entries for agent HTTP
API handlers. It impacts the calls `LAUNCH_CONTAINER`,
`WAIT_CONTAINER`, `KILL_CONTAINER`, and `REMOVE_CONTAINER`.


Diffs
-

  src/slave/http.cpp c4d8098df8cf9dd2cc2301b76460dd5e3bdb2bad 


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


Testing
---

```
$ make check
```


Thanks,

Armand Grillet

Re: Review Request 63677: Changed `os:Stack` to allocate with `mmap`.

[Andrei Budnik]

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


Ship it!




Ship It!

- Andrei Budnik


On Nov. 14, 2017, 7:11 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63677/
> ---
> 
> (Updated Nov. 14, 2017, 7:11 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-8159
> https://issues.apache.org/jira/browse/MESOS-8159
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If `os:Stack` is allocated with `mmap`, this automatically
> guarantees the correct alignment (since `mmap` is page-aligned
> by definition).
> 
> Although `mmap` is not required to be async signal safe
> (i.e. it might not be safe to call from a signal handler), in
> practice it should be safe to call between `fork` and `exec`
> since it is a raw system call that does not require any user
> space locks to be held.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/linux.hpp 
> 68b4090aa5e6f23609c487128d91301755ae0e46 
> 
> 
> Diff: https://reviews.apache.org/r/63677/diff/3/
> 
> 
> Testing
> ---
> 
> sudo make check (Fedora 26)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 63828: Added authorization tests for standalone container APIs.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 63828 was successfully built and tested.

Reviews applied: `['63828']`

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

- Mesos Reviewbot Windows


On Nov. 15, 2017, 8:42 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63828/
> ---
> 
> (Updated Nov. 15, 2017, 8:42 a.m.)
> 
> 
> Review request for mesos, Alexander Rojas, Gilbert Song, and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The usage of the standalone container APIs is rather coarse-grained.
> A principal can either use the APIs, or it cannot.  This is the case
> for all four actions: Launch, Wait, Kill, and Remove.
> 
> 
> Diffs
> -
> 
>   src/tests/authorization_tests.cpp f2e86e22f789a4d65105f737e6546bc050872bb2 
> 
> 
> Diff: https://reviews.apache.org/r/63828/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 63675: Added a non-allocating variant of `os::clone`.

[Andrei Budnik]

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


Ship it!




Ship It!

- Andrei Budnik


On Nov. 14, 2017, 7:06 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63675/
> ---
> 
> (Updated Nov. 14, 2017, 7:06 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-8159
> https://issues.apache.org/jira/browse/MESOS-8159
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a non-allocating variant of `os::clone` in the
> `os::signal_safe` namespace. This encapsulates the lambda
> trampoline, but requires the caller to bring their own stack
> since we can't know when it is safe to implicitly allocate
> a stack.
> 
> Now that we have `os::signal_safe::clone`, there is no need for
> `os::clone` to take an optional `os::Stack` argument. We can
> remove that and simplify the cleanup logic.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/linux.hpp 
> 68b4090aa5e6f23609c487128d91301755ae0e46 
> 
> 
> Diff: https://reviews.apache.org/r/63675/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check (Fedora 26)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 63678: Improved the signal safety of `ns::clone`.

[Andrei Budnik]

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


Ship it!




Ship It!

- Andrei Budnik


On Nov. 14, 2017, 7:11 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63678/
> ---
> 
> (Updated Nov. 14, 2017, 7:11 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-8159
> https://issues.apache.org/jira/browse/MESOS-8159
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `ns::clone` was implicitly using malloc to allocate a stack
> after fork but before exec, where we should be avoiding
> `malloc`.
> 
> Updated the second clone to use `os::signal_safe::clone`. We
> still explicitly allocate a stack after the fork, but since
> that is allocated with `mmap` is it safe in this particular
> case.
> 
> 
> Diffs
> -
> 
>   src/linux/ns.cpp 33751b518e95f4f67e6864728e1fdf621f50dd52 
> 
> 
> Diff: https://reviews.apache.org/r/63678/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check (Fedora 26)
> 
> NOTE: I tried to avoid changing `ns::clone` too much in this patch because 
> [MESOS-8155](https://issues.apache.org/jira/browse/MESOS-8155) and 
> [MESOS-8156](https://issues.apache.org/jira/browse/MESOS-8156) are also going 
> to also hit this code.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 63798: Added resource provider support for all offer operations.

[Jan Schlicht]

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

(Updated Nov. 15, 2017, 12:35 p.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


Changes
---

Addressed issues.


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


Repository: mesos


Description
---

The `RESERVE`/`UNRESERVE`/`CREATE`/`DESTROY` operations for
reservations and persistent volumes have been enabled for resource
provider resources. Similar to agent resources, they are speculatively
applied to allow offer pipelining.


Diffs (updated)
-

  src/slave/slave.cpp d8edc5e6bbfa265bca4d19bbaa7db3063949dbc0 
  src/tests/resource_provider_manager_tests.cpp 
ecfe2b4c0952838d6312df603f8eb2f458725175 


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

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


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 63751: Triggered 'ApplyOfferOperationMessage' for agent local resources.

[Jan Schlicht]

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

(Updated Nov. 15, 2017, 12:34 p.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


Changes
---

Addressed issues, added a helper to create status updates.


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


Repository: mesos


Description
---

Agents the have the 'RESOURCE_PROVIDER' capability set, should get sent
'ApplyOfferOperationMessage' instead of 'CheckpointResourcesMessage'.
The agent will then figure out how to apply the operation. For agent
local resources the agent will checkpoint resources.


Diffs (updated)
-

  src/common/protobuf_utils.hpp b2aa3654db2fe7d7d9d275ded81c6d54244654ee 
  src/common/protobuf_utils.cpp 34054846f93f19ba550afe58e2a899d111ad38dc 
  src/master/master.cpp 2ddd67ada3731803b00883b6a1f32b20c1bb238f 
  src/slave/slave.cpp d8edc5e6bbfa265bca4d19bbaa7db3063949dbc0 
  src/tests/reservation_tests.cpp 470f7341686e69d0a71fb234a26b277c45c29780 


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

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


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 63828: Added authorization tests for standalone container APIs.

[Alexander Rojas]

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


Ship it!




Ship It!

- Alexander Rojas


On Nov. 15, 2017, 9:42 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63828/
> ---
> 
> (Updated Nov. 15, 2017, 9:42 a.m.)
> 
> 
> Review request for mesos, Alexander Rojas, Gilbert Song, and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The usage of the standalone container APIs is rather coarse-grained.
> A principal can either use the APIs, or it cannot.  This is the case
> for all four actions: Launch, Wait, Kill, and Remove.
> 
> 
> Diffs
> -
> 
>   src/tests/authorization_tests.cpp f2e86e22f789a4d65105f737e6546bc050872bb2 
> 
> 
> Diff: https://reviews.apache.org/r/63828/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>