You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Chun-Hung Hsiao <ch...@mesosphere.io> on 2017/12/08 02:58:47 UTC
Review Request 64439: Added `LocalResourceProviderDaemon` methods to
modify configs.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64439/
-----------------------------------------------------------
Review request for mesos, James DeFelice, Jie Yu, and Joseph Wu.
Bugs: MESOS-8244
https://issues.apache.org/jira/browse/MESOS-8244
Repository: mesos
Description
-------
The `add()` and `update()` methods adds a new config file and updates an
existing config file of a local resource provider respectively. It will
then trigger a rolead on the resource provider asynchronously.
The `remove()` method removes the config file, and triggers the resource
provider to terminate asynchronously.
Diffs
-----
src/resource_provider/daemon.cpp 354054e8aab0131dcf4c47362975b11dfa96c16b
Diff: https://reviews.apache.org/r/64439/diff/1/
Testing
-------
Thanks,
Chun-Hung Hsiao
Re: Review Request 64439: Added `LocalResourceProviderDaemon` methods
to modify configs.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64439/#review193318
-----------------------------------------------------------
src/resource_provider/daemon.cpp
Lines 192 (patched)
<https://reviews.apache.org/r/64439/#comment271880>
The write is not atomic. To prevent from writting a file partially (e.g., agent crashes while writting the file), we should write to a temp location and then do a rename.
The tmp file has to be in the same device as the final destination.
Maybe we should create a `staging` directory under `configDir` and write the tmp file there and then do a rename.
That also means that the daemon has to skip the `staging` directory.
- Jie Yu
On Dec. 8, 2017, 10 p.m., Chun-Hung Hsiao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64439/
> -----------------------------------------------------------
>
> (Updated Dec. 8, 2017, 10 p.m.)
>
>
> Review request for mesos, James DeFelice, Jie Yu, and Joseph Wu.
>
>
> Bugs: MESOS-8244
> https://issues.apache.org/jira/browse/MESOS-8244
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The `add()` and `update()` methods adds a new config file and updates an
> existing config file of a local resource provider respectively. It will
> then trigger a rolead on the resource provider asynchronously.
>
> The `remove()` method removes the config file, and triggers the resource
> provider to terminate asynchronously.
>
>
> Diffs
> -----
>
> src/resource_provider/daemon.cpp 354054e8aab0131dcf4c47362975b11dfa96c16b
>
>
> Diff: https://reviews.apache.org/r/64439/diff/3/
>
>
> Testing
> -------
>
> See later in the chain.
>
>
> Thanks,
>
> Chun-Hung Hsiao
>
>
Re: Review Request 64439: Added `LocalResourceProviderDaemon` methods
to modify configs.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64439/#review193437
-----------------------------------------------------------
Fix it, then Ship it!
src/resource_provider/daemon.cpp
Lines 194 (patched)
<https://reviews.apache.org/r/64439/#comment271981>
Why you need this given it's uuid already?
- Jie Yu
On Dec. 11, 2017, 3:56 a.m., Chun-Hung Hsiao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64439/
> -----------------------------------------------------------
>
> (Updated Dec. 11, 2017, 3:56 a.m.)
>
>
> Review request for mesos, James DeFelice, Jie Yu, and Joseph Wu.
>
>
> Bugs: MESOS-8244
> https://issues.apache.org/jira/browse/MESOS-8244
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The `add()` and `update()` methods adds a new config file and updates an
> existing config file of a local resource provider respectively. It will
> then trigger a rolead on the resource provider asynchronously.
>
> The `remove()` method removes the config file, and triggers the resource
> provider to terminate asynchronously.
>
>
> Diffs
> -----
>
> src/resource_provider/daemon.cpp 61599828e1bb32b69e657cca90a535af3ec0c4dd
>
>
> Diff: https://reviews.apache.org/r/64439/diff/4/
>
>
> Testing
> -------
>
> See later in the chain.
>
>
> Thanks,
>
> Chun-Hung Hsiao
>
>
Re: Review Request 64439: Added `LocalResourceProviderDaemon` methods
to modify configs.
Posted by Chun-Hung Hsiao <ch...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64439/
-----------------------------------------------------------
(Updated Dec. 11, 2017, 10:44 p.m.)
Review request for mesos, James DeFelice, Jie Yu, and Joseph Wu.
Changes
-------
Addressed Jie's comments.
Bugs: MESOS-8244
https://issues.apache.org/jira/browse/MESOS-8244
Repository: mesos
Description
-------
The `add()` and `update()` methods adds a new config file and updates an
existing config file of a local resource provider respectively. It will
then trigger a rolead on the resource provider asynchronously.
The `remove()` method removes the config file, and triggers the resource
provider to terminate asynchronously.
Diffs (updated)
-----
src/resource_provider/daemon.cpp 61599828e1bb32b69e657cca90a535af3ec0c4dd
Diff: https://reviews.apache.org/r/64439/diff/5/
Changes: https://reviews.apache.org/r/64439/diff/4-5/
Testing
-------
See later in the chain.
Thanks,
Chun-Hung Hsiao
Re: Review Request 64439: Added `LocalResourceProviderDaemon` methods
to modify configs.
Posted by Chun-Hung Hsiao <ch...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64439/
-----------------------------------------------------------
(Updated Dec. 11, 2017, 3:56 a.m.)
Review request for mesos, James DeFelice, Jie Yu, and Joseph Wu.
Changes
-------
Addressed Jie's comments.
Bugs: MESOS-8244
https://issues.apache.org/jira/browse/MESOS-8244
Repository: mesos
Description
-------
The `add()` and `update()` methods adds a new config file and updates an
existing config file of a local resource provider respectively. It will
then trigger a rolead on the resource provider asynchronously.
The `remove()` method removes the config file, and triggers the resource
provider to terminate asynchronously.
Diffs (updated)
-----
src/resource_provider/daemon.cpp 61599828e1bb32b69e657cca90a535af3ec0c4dd
Diff: https://reviews.apache.org/r/64439/diff/4/
Changes: https://reviews.apache.org/r/64439/diff/3-4/
Testing
-------
See later in the chain.
Thanks,
Chun-Hung Hsiao
Re: Review Request 64439: Added `LocalResourceProviderDaemon` methods
to modify configs.
Posted by Chun-Hung Hsiao <ch...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64439/
-----------------------------------------------------------
(Updated Dec. 8, 2017, 10 p.m.)
Review request for mesos, James DeFelice, Jie Yu, and Joseph Wu.
Changes
-------
Fixed an issue for concurrent modification calls.
Bugs: MESOS-8244
https://issues.apache.org/jira/browse/MESOS-8244
Repository: mesos
Description
-------
The `add()` and `update()` methods adds a new config file and updates an
existing config file of a local resource provider respectively. It will
then trigger a rolead on the resource provider asynchronously.
The `remove()` method removes the config file, and triggers the resource
provider to terminate asynchronously.
Diffs (updated)
-----
src/resource_provider/daemon.cpp 354054e8aab0131dcf4c47362975b11dfa96c16b
Diff: https://reviews.apache.org/r/64439/diff/3/
Changes: https://reviews.apache.org/r/64439/diff/2-3/
Testing (updated)
-------
See later in the chain.
Thanks,
Chun-Hung Hsiao
Re: Review Request 64439: Added `LocalResourceProviderDaemon` methods
to modify configs.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64439/#review193239
-----------------------------------------------------------
FAIL: Some Mesos libprocess-tests failed.
Reviews applied: `['64441', '64437', '63901', '64440', '64438', '64439']`
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/64439
Relevant logs:
- [libprocess-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64439/logs/libprocess-tests-stdout.log):
```
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\ssl_tests.cpp(440): error: Failed to wait 15secs for socket
[ FAILED ] SSLVerifyIPAdd/SSLTest.RequireCertificate/0, where GetParam() = "false" (16685 ms)
[ RUN ] SSLVerifyIPAdd/SSLTest.RequireCertificate/1
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\ssl_tests.cpp(440): error: Failed to wait 15secs for socket
[ FAILED ] SSLVerifyIPAdd/SSLTest.RequireCertificate/1, where GetParam() = "true" (16501 ms)
[----------] 4 tests from SSLVerifyIPAdd/SSLTest (34235 ms total)
[----------] Global test environment tear-down
[==========] 223 tests from 35 test cases ran. (261760 ms total)
[ PASSED ] 209 tests.
[ FAILED ] 14 tests, listed below:
[ FAILED ] SSLTest.SSLSocket
[ FAILED ] SSLTest.NonSSLSocket
[ FAILED ] SSLTest.NoVerifyBadCA
[ FAILED ] SSLTest.RequireBadCA
[ FAILED ] SSLTest.VerifyBadCA
[ FAILED ] SSLTest.VerifyCertificate
[ FAILED ] SSLTest.ProtocolMismatch
[ FAILED ] SSLTest.ECDHESupport
[ FAILED ] SSLTest.ValidDowngrade
[ FAILED ] SSLTest.NoValidDowngrade
[ FAILED ] SSLTest.ValidDowngradeEachProtocol
[ FAILED ] SSLTest.NoValidDowngradeEachProtocol
[ FAILED ] SSLVerifyIPAdd/SSLTest.RequireCertificate/0, where GetParam() = "false"
[ FAILED ] SSLVerifyIPAdd/SSLTest.RequireCertificate/1, where GetParam() = "true"
14 FAILED TESTS
YOU HAVE 19 DISABLED TESTS
```
- [libprocess-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64439/logs/libprocess-tests-stderr.log):
```
I1208 11:05:46.937746 8476 process.cpp:887] Failed to accept socket: future discarded
I1208 11:05:47.153018 9532 process.cpp:887] Failed to accept socket: future discarded
I1208 11:05:47.697387 6644 openssl.cpp:514] Will not verify peer certificate!
NOTE: Set LIBPROCESS_SSL_VERIFY_CERT=1 to enable peer certificate verification
I1208 11:05:47.697387 6644 openssl.cpp:534] LIBPROCESS_SSL_REQUIRE_CERT implies peer certificate verification.
LIBPROCESS_SSL_VERIFY_CERT set to true
I1208 11:05:47.698388 6644 openssl.cpp:563] Using CA file: C:\Users\mesos\AppData\Local\Temp\WPxJRV\cert.pem
I1208 11:05:47.698388 6644 openssl.cpp:566] Using CA dir: C:\Users\mesos\AppData\Local\Temp\WPxJRV
I1208 11:05:48.246990 6644 openssl.cpp:514] Will not verify peer certificate!
NOTE: Set LIBPROCESS_SSL_VERIFY_CERT=1 to enable peer certificate verification
I1208 11:05:48.246990 6644 openssl.cpp:526] Will use IP address verification in subject alternative name certificate extension.
I1208 11:05:48.246990 6644 openssl.cpp:534] LIBPROCESS_SSL_REQUIRE_CERT implies peer certificate verification.
LIBPROCESS_SSL_VERIFY_CERT set to true
I1208 11:05:48.261992 6644 openssl.cpp:563] Using CA file: C:\Users\mesos\AppData\Local\Temp\Gc8HEj\cert.pem
I1208 11:05:48.261992 6644 openssl.cpp:566] Using CA dir: C:\Users\mesos\AppData\Local\Temp\Gc8HEj
I1208 11:05:48.963059 6644 openssl.cpp:509] CA directory path unspecified! NOTE: Set CA directory path with LIBPROCESS_SSL_CA_DIR=<dirpath>
I1208 11:05:48.963059 6644 openssl.cpp:514] Will not verify peer certificate!
NOTE: Set LIBPROCESS_SSL_VERIFY_CERT=1 to enable peer certificate verification
I1208 11:05:48.963059 6644 openssl.cpp:534] LIBPROCESS_SSL_REQUIRE_CERT implies peer certificate verification.
LIBPROCESS_SSL_VERIFY_CERT set to true
I1208 11:05:48.964059 6644 openssl.cpp:563] Using CA file: C:\Users\mesos\AppData\Local\Temp\mmlCq1\cert.pem
I1208 11:06:05.453600 6644 openssl.cpp:509] CA directory path unspecified! NOTE: Set CA directory path with LIBPROCESS_SSL_CA_DIR=<dirpath>
I1208 11:06:05.453600 6644 openssl.cpp:514] Will not verify peer certificate!
NOTE: Set LIBPROCESS_SSL_VERIFY_CERT=1 to enable peer certificate verification
I1208 11:06:05.453600 6644 openssl.cpp:526] Will use IP address verification in subject alternative name certificate extension.
I1208 11:06:05.453600 6644 openssl.cpp:534] LIBPROCESS_SSL_REQUIRE_CERT implies peer certificate verification.
LIBPROCESS_SSL_VERIFY_CERT set to true
I1208 11:06:05.454601 6644 openssl.cpp:563] Using CA file: C:\Users\mesos\AppData\Local\Temp\dST1FI\cert.pem
I1208 11:06:21.604385 4120 process.cpp:887] Failed to accept socket: future discarded
```
- Mesos Reviewbot Windows
On Dec. 8, 2017, 2:58 a.m., Chun-Hung Hsiao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64439/
> -----------------------------------------------------------
>
> (Updated Dec. 8, 2017, 2:58 a.m.)
>
>
> Review request for mesos, James DeFelice, Jie Yu, and Joseph Wu.
>
>
> Bugs: MESOS-8244
> https://issues.apache.org/jira/browse/MESOS-8244
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The `add()` and `update()` methods adds a new config file and updates an
> existing config file of a local resource provider respectively. It will
> then trigger a rolead on the resource provider asynchronously.
>
> The `remove()` method removes the config file, and triggers the resource
> provider to terminate asynchronously.
>
>
> Diffs
> -----
>
> src/resource_provider/daemon.cpp 354054e8aab0131dcf4c47362975b11dfa96c16b
>
>
> Diff: https://reviews.apache.org/r/64439/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Chun-Hung Hsiao
>
>