You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Andrei Sekretenko <as...@mesosphere.io> on 2019/04/03 16:04:31 UTC
Review Request 70379: Added validation that the principal stays the
same on resubscription.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70379/
-----------------------------------------------------------
Review request for mesos and Gastón Kleiman.
Bugs: MESOS-2842
https://issues.apache.org/jira/browse/MESOS-2842
Repository: mesos
Description
-------
Added validation that the principal stays the same on a framework resubscription.
This solves MESOS-2842.
There are at least two objectionable places in this patch - see TODO in the diff.
Diffs
-----
src/master/master.hpp 94891af9deeaddb3333fc9d6eabb243aed97f7b7
src/master/master.cpp cf5caa0893ba1387a1f3a9d129ecd7d974f776bd
Diff: https://reviews.apache.org/r/70379/diff/1/
Testing
-------
make check
Now the two failing tests from https://reviews.apache.org/r/70377/ pass.
Thanks,
Andrei Sekretenko
Re: Review Request 70379: Added validation that the principal stays
the same on resubscription.
Posted by Andrei Sekretenko <as...@mesosphere.io>.
> On April 4, 2019, 9:53 p.m., Gastón Kleiman wrote:
> > src/master/master.cpp
> > Lines 2558-2560 (patched)
> > <https://reviews.apache.org/r/70379/diff/1/?file=2137245#file2137245line2558>
> >
> > We use stout's `stringify()` for this, which delegates to the corresponding `operator<<` if one exists.
Implementing `operator<<(const Option<T>&)` without breaking `UniversalPrint`-related magic in googletest turns out to be a somewhat nontrivial task wich does not fit into this ticket.
Also, the proper output of such operator will definitely require some discussion.
So, regarding your previous comment, I decided to drop this altogether.
> On April 4, 2019, 9:53 p.m., Gastón Kleiman wrote:
> > src/master/master.cpp
> > Lines 2562-2563 (patched)
> > <https://reviews.apache.org/r/70379/diff/1/?file=2137245#file2137245line2562>
> >
> > I would follow the current pattern in `Master::subscribe` and instead of adding this method add one with this signature: `bool Master::frameworkPrincipalChanged(const FrameworkInfo& frameworkInfo)`.
After running a diff against the two Master::subscribe() methods I relaized that they have identical (except for comments) validation part. After refactoring this away (it will be the review PRECEDING the review with the tests) this method becomes unneeded.
> On April 4, 2019, 9:53 p.m., Gastón Kleiman wrote:
> > src/master/master.cpp
> > Lines 2570 (patched)
> > <https://reviews.apache.org/r/70379/diff/1/?file=2137245#file2137245line2570>
> >
> > we tend to use `auto` only for complex types or inside `foreach` statements.
> >
> > To stay consistent with what's normally used, I would do:
> >
> > `Framework* framework = getFramework(frameworkInfo.id());`
I have no strong opinion on the "almost always/never auto" debate. Changing this from `const auto*` to `const Framework*`.
> On April 4, 2019, 9:53 p.m., Gastón Kleiman wrote:
> > src/master/master.cpp
> > Lines 2592-2596 (patched)
> > <https://reviews.apache.org/r/70379/diff/1/?file=2137245#file2137245line2592>
> >
> > We don't end lines with the `<<` operator. We start new lines with it instead, so this would look like:
> >
> > ```
> > LOG(WARNING)
> > << "Framework " << frameworkInfo.id() << " which had a principal '"
> > << oldPrincipal << "' tried to resubscribe with a new principal '"
> > << newPrincipal << "'";
> > ```
> >
> > However we don't have to log anything here, because `Master::subscribe()` already logs validation errors.
The only reason I added this logging is that there will be no information about the old and the new principals in the validation error message.
Having such information on hands would be nice for the guys who will run into a buggy framework.
If there already is a more or less convenient way for the operator to extract the information about the old principal from master, I'll add the new principal to the error and gladly drop this line.
- Andrei
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70379/#review214382
-----------------------------------------------------------
On April 5, 2019, 3:12 p.m., Andrei Sekretenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70379/
> -----------------------------------------------------------
>
> (Updated April 5, 2019, 3:12 p.m.)
>
>
> Review request for mesos and Gastón Kleiman.
>
>
> Bugs: MESOS-2842
> https://issues.apache.org/jira/browse/MESOS-2842
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added validation that the principal stays the same on resubscription.
>
>
> Diffs
> -----
>
> src/master/master.cpp cf5caa0893ba1387a1f3a9d129ecd7d974f776bd
>
>
> Diff: https://reviews.apache.org/r/70379/diff/2/
>
>
> Testing
> -------
>
> make check
>
> Now the two failing tests from https://reviews.apache.org/r/70377/ pass.
>
>
> Thanks,
>
> Andrei Sekretenko
>
>
Re: Review Request 70379: Added validation that the principal stays
the same on resubscription.
Posted by Andrei Sekretenko <as...@mesosphere.io>.
> On April 4, 2019, 9:53 p.m., Gastón Kleiman wrote:
> > src/master/master.hpp
> > Lines 1194-1195 (patched)
> > <https://reviews.apache.org/r/70379/diff/1/?file=2137244#file2137244line1194>
> >
> > I'd write something like this:
> >
> > ```
> > Validates that the principal provided on resubscription matches the one already known by the master.
> > ```
This code is not a separate method anymore.
- Andrei
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70379/#review214382
-----------------------------------------------------------
On April 10, 2019, 3:35 p.m., Andrei Sekretenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70379/
> -----------------------------------------------------------
>
> (Updated April 10, 2019, 3:35 p.m.)
>
>
> Review request for mesos and Gastón Kleiman.
>
>
> Bugs: MESOS-2842
> https://issues.apache.org/jira/browse/MESOS-2842
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added validation that the principal stays the same on resubscription.
>
>
> Diffs
> -----
>
> src/master/master.cpp ad54ae217863a08f4e6d743b39c176b171353084
>
>
> Diff: https://reviews.apache.org/r/70379/diff/3/
>
>
> Testing
> -------
>
> make check
>
> Now the two failing tests from https://reviews.apache.org/r/70377/ pass.
>
>
> Thanks,
>
> Andrei Sekretenko
>
>
Re: Review Request 70379: Added validation that the principal stays
the same on resubscription.
Posted by Gastón Kleiman <ga...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70379/#review214382
-----------------------------------------------------------
src/master/master.hpp
Lines 1194-1195 (patched)
<https://reviews.apache.org/r/70379/#comment300573>
I'd write something like this:
```
Validates that the principal provided on resubscription matches the one already known by the master.
```
src/master/master.cpp
Lines 2525-2527 (patched)
<https://reviews.apache.org/r/70379/#comment300575>
We tend to prefer inlining statements like this one instead of creating static methods.
src/master/master.cpp
Lines 2558-2560 (patched)
<https://reviews.apache.org/r/70379/#comment300574>
We use stout's `stringify()` for this, which delegates to the corresponding `operator<<` if one exists.
src/master/master.cpp
Lines 2562-2563 (patched)
<https://reviews.apache.org/r/70379/#comment300589>
I would follow the current pattern in `Master::subscribe` and instead of adding this method add one with this signature: `bool Master::frameworkPrincipalChanged(const FrameworkInfo& frameworkInfo)`.
src/master/master.cpp
Lines 2570 (patched)
<https://reviews.apache.org/r/70379/#comment300577>
we tend to use `auto` only for complex types or inside `foreach` statements.
To stay consistent with what's normally used, I would do:
`Framework* framework = getFramework(frameworkInfo.id());`
src/master/master.cpp
Lines 2573-2574 (patched)
<https://reviews.apache.org/r/70379/#comment300580>
I would change this to:
```
// TODO(asekretenko): Masters don't store `FrameworkInfo` messages in the replicated log, so it is possible that the previous principal is still unknown at the time of re-registration. This has to be changed if we decide to start storing `FrameworkInfo` messages.
```
And add an empty line before the TODO.
src/master/master.cpp
Lines 2592-2596 (patched)
<https://reviews.apache.org/r/70379/#comment300584>
We don't end lines with the `<<` operator. We start new lines with it instead, so this would look like:
```
LOG(WARNING)
<< "Framework " << frameworkInfo.id() << " which had a principal '"
<< oldPrincipal << "' tried to resubscribe with a new principal '"
<< newPrincipal << "'";
```
However we don't have to log anything here, because `Master::subscribe()` already logs validation errors.
src/master/master.cpp
Lines 2598 (patched)
<https://reviews.apache.org/r/70379/#comment300593>
I think I'd rather not mention the old principal to prevent leaking information.
src/master/master.cpp
Lines 10763 (patched)
<https://reviews.apache.org/r/70379/#comment300591>
Looks like this variable is not used.
- Gastón Kleiman
On April 3, 2019, 9:04 a.m., Andrei Sekretenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70379/
> -----------------------------------------------------------
>
> (Updated April 3, 2019, 9:04 a.m.)
>
>
> Review request for mesos and Gastón Kleiman.
>
>
> Bugs: MESOS-2842
> https://issues.apache.org/jira/browse/MESOS-2842
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added validation that the principal stays the same on a framework resubscription.
> This solves MESOS-2842.
>
> There are at least two objectionable places in this patch - see TODO in the diff.
>
>
> Diffs
> -----
>
> src/master/master.hpp 94891af9deeaddb3333fc9d6eabb243aed97f7b7
> src/master/master.cpp cf5caa0893ba1387a1f3a9d129ecd7d974f776bd
>
>
> Diff: https://reviews.apache.org/r/70379/diff/1/
>
>
> Testing
> -------
>
> make check
>
> Now the two failing tests from https://reviews.apache.org/r/70377/ pass.
>
>
> Thanks,
>
> Andrei Sekretenko
>
>
Re: Review Request 70379: Added validation that the principal stays
the same on resubscription.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70379/#review214427
-----------------------------------------------------------
FAIL: Some of the unit tests failed. Please check the relevant logs.
Reviews applied: `['70408', '70377', '70379']`
Failed command: `Start-MesosCITesting`
All the build artifacts available at: http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/3095/mesos-review-70379
Relevant logs:
- [mesos-tests.log](http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/3095/mesos-review-70379/logs/mesos-tests.log):
```
I0405 17:37:10.754954 38820 master.cpp:1295] Agent 00ba462d-c978-4efc-bd79-99895f3755a2-S0 at slave(501)@192.10.1.4:62190 (windows-01.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net) disconnected
I0405 17:37:10.755946 38820 master.cpp:3312] Disconnecting agent 00ba462d-c978-4efc-bd79-99895f3755a2-S0 at slave(501)@192.10.1.4:62190 (windows-01.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net)
I0405 17:37:10.755946 38820 master.cpp:3331] Deactivating agent 00ba462d-c978-4efc-bd79-99895f3755a2-S0 at slave(501)@192.10.1.4:62190 (windows-01.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net)
I0405 17:37:10.755946 33868 hierarchical.cpp:392] Removed framework 00ba462d-c978-4efc-bd79-99895f3755a2-0000
I0405 17:37:10.756963 33868 hierarchical.cpp:829] Agent 00ba462d-c978-4efc-bd79-99895f3755a2-S0 deactivated
I0405 17:37:10.756963 34116 containerizer.cpp:2576] Destroying container 5c13e4ad-ea2d-49aa-8437-dc0669685b27 in RUNNING state
I0405 17:37:10.757952 34116 containerizer.cpp:3278] Transitioning the state of container 5c13e4ad-ea2d-49aa-8437-dc0669685b27 from RUNNING to DESTROYING
I0405 17:37:10.757952 34116 launcher.cpp:161] Asked to destroy container 5c13e4ad-ea2d-49aa-8437-dc0669685b27
W0405 17:37:10.758962 30280 process.cpp:1423] Failed to recv on socket WindowsFD::Type::SOCKET=2712 to peer '192.10.1.4:64594': IO failed with error code: The specified network name is no longer available.
W0405 17:37:10.759946 30280 process.cpp:838] Failed to recv on socket WindowsFD::Type::SOCKET=2576 to peer '192.10.1.4:64595': IO failed with error code: The specified network name is no longer available.
I0405 17:37:10.796885 38820 containerizer.cpp:3117] Container 5c13e4ad-ea2d-49aa-8437-dc0669685b2[ OK ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0 (688 ms)
[----------] 1 test from IsolationFlag/MemoryIsolatorTest (707 ms total)
[----------] Global test environment tear-down
[==========] 1160 tests from 107 test cases ran. (579363 ms total)
[ PASSED ] 1158 tests.
[ FAILED ] 2 tests, listed below:
[ FAILED ] DockerFetcherPluginTest.INTERNET_CURL_FetchImage
[ FAILED ] DockerFetcherPluginTest.INTERNET_CURL_InvokeFetchByName
2 FAILED TESTS
YOU HAVE 231 DISABLED TESTS
7 has exited
I0405 17:37:10.827756 37900 master.cpp:1135] Master terminating
I0405 17:37:10.829753 38656 hierarchical.cpp:680] Removed agent 00ba462d-c978-4efc-bd79-99895f3755a2-S0
I0405 17:37:11.435853 30280 process.cpp:927] Stopped the socket accept loop
```
- Mesos Reviewbot Windows
On April 5, 2019, 8:12 a.m., Andrei Sekretenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70379/
> -----------------------------------------------------------
>
> (Updated April 5, 2019, 8:12 a.m.)
>
>
> Review request for mesos and Gastón Kleiman.
>
>
> Bugs: MESOS-2842
> https://issues.apache.org/jira/browse/MESOS-2842
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added validation that the principal stays the same on resubscription.
>
>
> Diffs
> -----
>
> src/master/master.cpp cf5caa0893ba1387a1f3a9d129ecd7d974f776bd
>
>
> Diff: https://reviews.apache.org/r/70379/diff/2/
>
>
> Testing
> -------
>
> make check
>
> Now the two failing tests from https://reviews.apache.org/r/70377/ pass.
>
>
> Thanks,
>
> Andrei Sekretenko
>
>
Re: Review Request 70379: Added validation that the principal stays
the same on resubscription.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70379/#review214543
-----------------------------------------------------------
Patch looks great!
Reviews applied: [70408, 70377, 70379]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers --disable-parallel-test-execution' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On April 10, 2019, 3:35 p.m., Andrei Sekretenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70379/
> -----------------------------------------------------------
>
> (Updated April 10, 2019, 3:35 p.m.)
>
>
> Review request for mesos and Gastón Kleiman.
>
>
> Bugs: MESOS-2842
> https://issues.apache.org/jira/browse/MESOS-2842
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added validation that the principal stays the same on resubscription.
>
>
> Diffs
> -----
>
> src/master/master.cpp ad54ae217863a08f4e6d743b39c176b171353084
>
>
> Diff: https://reviews.apache.org/r/70379/diff/3/
>
>
> Testing
> -------
>
> make check
>
> Now the two failing tests from https://reviews.apache.org/r/70377/ pass.
>
>
> Thanks,
>
> Andrei Sekretenko
>
>
Re: Review Request 70379: Added validation that the principal stays
the same on resubscription.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70379/#review214760
-----------------------------------------------------------
FAIL: Some of the unit tests failed. Please check the relevant logs.
Reviews applied: `['70408', '70377', '70379']`
Failed command: `Start-MesosCITesting`
All the build artifacts available at: http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/3210/mesos-review-70379
Relevant logs:
- [mesos-tests.log](http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/3210/mesos-review-70379/logs/mesos-tests.log):
```
I0419 12:27:02.265796 69092 master.cpp:3312] Disconnecting agent 1335cf5c-b547-4d71-a8f6-cc602c838005-S0 at slave(502)@192.10.1.4:51217 (windows-01.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net)
I0419 12:27:02.266831 69092 master.cpp:3331] Deactivating agent 1335cf5c-b547-4d71-a8f6-cc602c838005-S0 at slave(502)@192.10.1.4:51217 (windows-01.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net)
I0419 12:27:02.266831 65884 hierarchical.cpp:392] Removed framework 1335cf5c-b547-4d71-a8f6-cc602c838005-0000
I0419 12:27:02.266831 65884 hierarchical.cpp:829] Agent 1335cf5c-b547-4d71-a8f6-cc602c838005-S0 deactivated
I0419 12:27:02.268786 70112 containerizer.cpp:2576] Destroying container 68c1542c-c00a-4faf-95d4-00876586b00f in RUNNING state
I0419 12:27:02.268786 70112 containerizer.cpp:3278] Transitioning the state of container 68c1542c-c00a-4faf-95d4-00876586b00f from RUNNING to DESTROYING
I0419 12:27:02.269791 70112 launcher.cpp:161] Asked to destroy container 68c1542c-c00a-4faf-95d4-00876586b00f
W0419 12:27:02.270807 70444 process.cpp:1423] Failed to recv on socket WindowsFD::[ OK ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0 (778 ms)
[----------] 1 test from IsolationFlag/MemoryIsolatorTest (794 ms total)
[----------] Global test environment tear-down
[==========] 1163 tests from 109 test cases ran. (581968 ms total)
[ PASSED ] 1160 tests.
[ FAILED ] 3 tests, listed below:
[ FAILED ] DockerFetcherPluginTest.INTERNET_CURL_FetchManifest
[ FAILED ] DockerFetcherPluginTest.INTERNET_CURL_FetchImage
[ FAILED ] DockerFetcherPluginTest.INTERNET_CURL_InvokeFetchByName
3 FAILED TESTS
YOU HAVE 233 DISABLED TESTS
Type::SOCKET=5516 to peer '192.10.1.4:53610': IO failed with error code: The specified network name is no longer available.
W0419 12:27:02.270807 70444 process.cpp:838] Failed to recv on socket WindowsFD::Type::SOCKET=3868 to peer '192.10.1.4:53611': IO failed with error code: The specified network name is no longer available.
I0419 12:27:02.349812 69092 containerizer.cpp:3117] Container 68c1542c-c00a-4faf-95d4-00876586b00f has exited
I0419 12:27:02.380800 67712 master.cpp:1135] Master terminating
I0419 12:27:02.381808 70468 hierarchical.cpp:680] Removed agent 1335cf5c-b547-4d71-a8f6-cc602c838005-S0
I0419 12:27:03.390813 70444 process.cpp:927] Stopped the socket accept loop
```
- Mesos Reviewbot Windows
On April 17, 2019, 2:08 p.m., Andrei Sekretenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70379/
> -----------------------------------------------------------
>
> (Updated April 17, 2019, 2:08 p.m.)
>
>
> Review request for mesos and Gastón Kleiman.
>
>
> Bugs: MESOS-2842
> https://issues.apache.org/jira/browse/MESOS-2842
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added validation that the principal stays the same on resubscription.
>
>
> Diffs
> -----
>
> src/master/master.cpp ad54ae217863a08f4e6d743b39c176b171353084
>
>
> Diff: https://reviews.apache.org/r/70379/diff/5/
>
>
> Testing
> -------
>
> make check
>
> Now the two failing tests from https://reviews.apache.org/r/70377/ pass.
>
>
> Thanks,
>
> Andrei Sekretenko
>
>
Re: Review Request 70379: Added validation that the principal stays
the same on resubscription.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70379/#review214726
-----------------------------------------------------------
Patch looks great!
Reviews applied: [70408, 70377, 70379]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers --disable-parallel-test-execution' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On April 17, 2019, 2:08 p.m., Andrei Sekretenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70379/
> -----------------------------------------------------------
>
> (Updated April 17, 2019, 2:08 p.m.)
>
>
> Review request for mesos and Gastón Kleiman.
>
>
> Bugs: MESOS-2842
> https://issues.apache.org/jira/browse/MESOS-2842
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added validation that the principal stays the same on resubscription.
>
>
> Diffs
> -----
>
> src/master/master.cpp ad54ae217863a08f4e6d743b39c176b171353084
>
>
> Diff: https://reviews.apache.org/r/70379/diff/5/
>
>
> Testing
> -------
>
> make check
>
> Now the two failing tests from https://reviews.apache.org/r/70377/ pass.
>
>
> Thanks,
>
> Andrei Sekretenko
>
>
Re: Review Request 70379: Added validation that the principal stays
the same on resubscription.
Posted by Andrei Sekretenko <as...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70379/
-----------------------------------------------------------
(Updated April 17, 2019, 2:08 p.m.)
Review request for mesos and Gastón Kleiman.
Bugs: MESOS-2842
https://issues.apache.org/jira/browse/MESOS-2842
Repository: mesos
Description
-------
Added validation that the principal stays the same on resubscription.
Diffs (updated)
-----
src/master/master.cpp ad54ae217863a08f4e6d743b39c176b171353084
Diff: https://reviews.apache.org/r/70379/diff/5/
Changes: https://reviews.apache.org/r/70379/diff/4-5/
Testing
-------
make check
Now the two failing tests from https://reviews.apache.org/r/70377/ pass.
Thanks,
Andrei Sekretenko
Re: Review Request 70379: Added validation that the principal stays
the same on resubscription.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70379/#review214686
-----------------------------------------------------------
FAIL: Some of the unit tests failed. Please check the relevant logs.
Reviews applied: `['70408', '70377', '70379']`
Failed command: `Start-MesosCITesting`
All the build artifacts available at: http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/3183/mesos-review-70379
Relevant logs:
- [mesos-tests.log](http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/3183/mesos-review-70379/logs/mesos-tests.log):
```
I0416 08:47:55.246440 61944 master.cpp:1295] Agent 4cbe623d-f6dc-4ad2-b5c8-bf225b82dccc-S0 at slave(502)@192.10.1.4:57419 (windows-01.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net) disconnected
I0416 08:47:55.246440 61944 master.cpp:3312] Disconnecting agent 4cbe623d-f6dc-4ad2-b5c8-bf225b82dccc-S0 at slave(502)@192.10.1.4:57419 (windows-01.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net)
I0416 08:47:55.246440 61944 master.cpp:3331] Deactivating agent 4cbe623d-f6dc-4ad2-b5c8-bf225b82dccc-S0 at slave(502)@192.10.1.4:57419 (windows-01.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net)
I0416 08:47:55.247483 62808 hierarchical.cpp:392] Removed framework 4cbe623d-f6dc-4ad2-b5c8-bf225b82dccc-0000
I0416 08:47:55.247483 62808 hierarchical.cpp:829] Agent 4cbe623d-f6dc-4ad2-b5c8-bf225b82dccc-S0 deactivated
I0416 08:47:55.248437 61944 containerizer.cpp:2576] Destroying container f3074bf2-f283-4456-8856-06304272ee1d in RUNNING state
I0416 08:47:55.248437 61944 containerizer.cpp:3278] Transitioning the state of container f3074bf2-f283-4456-8856-06304272ee1d from RUNNING to DESTROYING
I0416 08:47:55.249483 61944 launcher.cpp:161] Asked to destroy container f3074bf2-f283-4456-8856-06304272ee1d
W0416 08:47:55.250437 56356 process.cpp:1423] Failed to recv on socket WindowsFD::Type::SOCKET=2[ OK ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0 (791 ms)
[----------] 1 test from IsolationFlag/MemoryIsolatorTest (807 ms total)
[----------] Global test environment tear-down
[==========] 1162 tests from 108 test cases ran. (637981 ms total)
[ PASSED ] 1160 tests.
[ FAILED ] 2 tests, listed below:
[ FAILED ] DockerFetcherPluginTest.INTERNET_CURL_FetchImage
[ FAILED ] DockerFetcherPluginTest.INTERNET_CURL_InvokeFetchByName
2 FAILED TESTS
YOU HAVE 233 DISABLED TESTS
600 to peer '192.10.1.4:59859': IO failed with error code: The specified network name is no longer available.
W0416 08:47:55.250437 56356 process.cpp:838] Failed to recv on socket WindowsFD::Type::SOCKET=3796 to peer '192.10.1.4:59860': IO failed with error code: The specified network name is no longer available.
I0416 08:47:55.347033 55440 containerizer.cpp:3117] Container f3074bf2-f283-4456-8856-06304272ee1d has exited
I0416 08:47:55.376034 63180 master.cpp:1135] Master terminating
I0416 08:47:55.377029 58804 hierarchical.cpp:680] Removed agent 4cbe623d-f6dc-4ad2-b5c8-bf225b82dccc-S0
I0416 08:47:56.164043 56356 process.cpp:927] Stopped the socket accept loop
```
- Mesos Reviewbot Windows
On April 15, 2019, 12:20 p.m., Andrei Sekretenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70379/
> -----------------------------------------------------------
>
> (Updated April 15, 2019, 12:20 p.m.)
>
>
> Review request for mesos and Gastón Kleiman.
>
>
> Bugs: MESOS-2842
> https://issues.apache.org/jira/browse/MESOS-2842
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added validation that the principal stays the same on resubscription.
>
>
> Diffs
> -----
>
> src/master/master.cpp ad54ae217863a08f4e6d743b39c176b171353084
>
>
> Diff: https://reviews.apache.org/r/70379/diff/4/
>
>
> Testing
> -------
>
> make check
>
> Now the two failing tests from https://reviews.apache.org/r/70377/ pass.
>
>
> Thanks,
>
> Andrei Sekretenko
>
>
Re: Review Request 70379: Added validation that the principal stays
the same on resubscription.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70379/#review214661
-----------------------------------------------------------
Patch looks great!
Reviews applied: [70408, 70377, 70379]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers --disable-parallel-test-execution' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On April 15, 2019, 8:20 p.m., Andrei Sekretenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70379/
> -----------------------------------------------------------
>
> (Updated April 15, 2019, 8:20 p.m.)
>
>
> Review request for mesos and Gastón Kleiman.
>
>
> Bugs: MESOS-2842
> https://issues.apache.org/jira/browse/MESOS-2842
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added validation that the principal stays the same on resubscription.
>
>
> Diffs
> -----
>
> src/master/master.cpp ad54ae217863a08f4e6d743b39c176b171353084
>
>
> Diff: https://reviews.apache.org/r/70379/diff/4/
>
>
> Testing
> -------
>
> make check
>
> Now the two failing tests from https://reviews.apache.org/r/70377/ pass.
>
>
> Thanks,
>
> Andrei Sekretenko
>
>
Re: Review Request 70379: Added validation that the principal stays
the same on resubscription.
Posted by Andrei Sekretenko <as...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70379/
-----------------------------------------------------------
(Updated April 15, 2019, 12:20 p.m.)
Review request for mesos and Gastón Kleiman.
Bugs: MESOS-2842
https://issues.apache.org/jira/browse/MESOS-2842
Repository: mesos
Description
-------
Added validation that the principal stays the same on resubscription.
Diffs (updated)
-----
src/master/master.cpp ad54ae217863a08f4e6d743b39c176b171353084
Diff: https://reviews.apache.org/r/70379/diff/4/
Changes: https://reviews.apache.org/r/70379/diff/3-4/
Testing
-------
make check
Now the two failing tests from https://reviews.apache.org/r/70377/ pass.
Thanks,
Andrei Sekretenko
Re: Review Request 70379: Added validation that the principal stays
the same on resubscription.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70379/#review214555
-----------------------------------------------------------
FAIL: Some of the unit tests failed. Please check the relevant logs.
Reviews applied: `['70408', '70377', '70379']`
Failed command: `Start-MesosCITesting`
All the build artifacts available at: http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/3146/mesos-review-70379
Relevant logs:
- [mesos-tests.log](http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/3146/mesos-review-70379/logs/mesos-tests.log):
```
I0410 23:13:48.871886 50116 master.cpp:1295] Agent d786d7f3-0e0d-4f9d-9dcd-55d26dbb5134-S0 at slave(502)@192.10.1.4:65061 (windows-01.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net) disconnected
I0410 23:13:48.871886 50116 master.cpp:3312] Disconnecting agent d786d7f3-0e0d-4f9d-9dcd-55d26dbb5134-S0 at slave(502)@192.10.1.4:65061 (windows-01.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net)
I0410 23:13:48.871886 50116 master.cpp:3331] Deactivating agent d786d7f3-0e0d-4f9d-9dcd-55d26dbb5134-S0 at slave(502)@192.10.1.4:65061 (windows-01.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net)
I0410 23:13:48.872923 47192 hierarchical.cpp:392] Removed framework d786d7f3-0e0d-4f9d-9dcd-55d26dbb5134-0000
I0410 23:13:48.873930 47192 hierarchical.cpp:829] Agent d786d7f3-0e0d-4f9d-9dcd-55d26dbb5134-S0 deactivated
I0410 23:13:48.874927 50116 containerizer.cpp:2576] Destroying container 9526b8c8-734f-43e6-96e3-89076d59e4c5 in RUNNING state
I0[ OK ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0 (786 ms)
[----------] 1 test from IsolationFlag/MemoryIsolatorTest (805 ms total)
[----------] Global test environment tear-down
[==========] 1161 tests from 108 test cases ran. (596301 ms total)
[ PASSED ] 1159 tests.
[ FAILED ] 2 tests, listed below:
[ FAILED ] DockerFetcherPluginTest.INTERNET_CURL_FetchImage
[ FAILED ] DockerFetcherPluginTest.INTERNET_CURL_InvokeFetchByName
2 FAILED TESTS
YOU HAVE 233 DISABLED TESTS
410 23:13:48.874927 50116 containerizer.cpp:3278] Transitioning the state of container 9526b8c8-734f-43e6-96e3-89076d59e4c5 from RUNNING to DESTROYING
I0410 23:13:48.875892 50116 launcher.cpp:161] Asked to destroy container 9526b8c8-734f-43e6-96e3-89076d59e4c5
W0410 23:13:48.876893 47252 process.cpp:1423] Failed to recv on socket WindowsFD::Type::SOCKET=12924 to peer '192.10.1.4:51133': IO failed with error code: The specified network name is no longer available.
W0410 23:13:48.876893 47252 process.cpp:838] Failed to recv on socket WindowsFD::Type::SOCKET=12636 to peer '192.10.1.4:51134': IO failed with error code: The specified network name is no longer available.
I0410 23:13:48.943886 49708 containerizer.cpp:3117] Container 9526b8c8-734f-43e6-96e3-89076d59e4c5 has exited
I0410 23:13:48.973886 48696 master.cpp:1135] Master terminating
I0410 23:13:48.974884 50116 hierarchical.cpp:680] Removed agent d786d7f3-0e0d-4f9d-9dcd-55d26dbb5134-S0
I0410 23:13:49.584882 47252 process.cpp:927] Stopped the socket accept loop
```
- Mesos Reviewbot Windows
On April 10, 2019, 3:35 p.m., Andrei Sekretenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70379/
> -----------------------------------------------------------
>
> (Updated April 10, 2019, 3:35 p.m.)
>
>
> Review request for mesos and Gastón Kleiman.
>
>
> Bugs: MESOS-2842
> https://issues.apache.org/jira/browse/MESOS-2842
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added validation that the principal stays the same on resubscription.
>
>
> Diffs
> -----
>
> src/master/master.cpp ad54ae217863a08f4e6d743b39c176b171353084
>
>
> Diff: https://reviews.apache.org/r/70379/diff/3/
>
>
> Testing
> -------
>
> make check
>
> Now the two failing tests from https://reviews.apache.org/r/70377/ pass.
>
>
> Thanks,
>
> Andrei Sekretenko
>
>
Re: Review Request 70379: Added validation that the principal stays
the same on resubscription.
Posted by Gastón Kleiman <ga...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70379/#review214640
-----------------------------------------------------------
Ship it!
Ship It!
- Gastón Kleiman
On April 10, 2019, 8:35 a.m., Andrei Sekretenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70379/
> -----------------------------------------------------------
>
> (Updated April 10, 2019, 8:35 a.m.)
>
>
> Review request for mesos and Gastón Kleiman.
>
>
> Bugs: MESOS-2842
> https://issues.apache.org/jira/browse/MESOS-2842
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added validation that the principal stays the same on resubscription.
>
>
> Diffs
> -----
>
> src/master/master.cpp ad54ae217863a08f4e6d743b39c176b171353084
>
>
> Diff: https://reviews.apache.org/r/70379/diff/3/
>
>
> Testing
> -------
>
> make check
>
> Now the two failing tests from https://reviews.apache.org/r/70377/ pass.
>
>
> Thanks,
>
> Andrei Sekretenko
>
>
Re: Review Request 70379: Added validation that the principal stays
the same on resubscription.
Posted by Andrei Sekretenko <as...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70379/
-----------------------------------------------------------
(Updated April 10, 2019, 3:35 p.m.)
Review request for mesos and Gastón Kleiman.
Bugs: MESOS-2842
https://issues.apache.org/jira/browse/MESOS-2842
Repository: mesos
Description
-------
Added validation that the principal stays the same on resubscription.
Diffs (updated)
-----
src/master/master.cpp ad54ae217863a08f4e6d743b39c176b171353084
Diff: https://reviews.apache.org/r/70379/diff/3/
Changes: https://reviews.apache.org/r/70379/diff/2-3/
Testing
-------
make check
Now the two failing tests from https://reviews.apache.org/r/70377/ pass.
Thanks,
Andrei Sekretenko
Re: Review Request 70379: Added validation that the principal stays
the same on resubscription.
Posted by Andrei Sekretenko <as...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70379/
-----------------------------------------------------------
(Updated April 5, 2019, 3:12 p.m.)
Review request for mesos and Gastón Kleiman.
Changes
-------
Now this is based on subscribe() validation refactiring in https://reviews.apache.org/r/70408/
Bugs: MESOS-2842
https://issues.apache.org/jira/browse/MESOS-2842
Repository: mesos
Description (updated)
-------
Added validation that the principal stays the same on resubscription.
Diffs (updated)
-----
src/master/master.cpp cf5caa0893ba1387a1f3a9d129ecd7d974f776bd
Diff: https://reviews.apache.org/r/70379/diff/2/
Changes: https://reviews.apache.org/r/70379/diff/1-2/
Testing
-------
make check
Now the two failing tests from https://reviews.apache.org/r/70377/ pass.
Thanks,
Andrei Sekretenko
Re: Review Request 70379: Added validation that the principal stays
the same on resubscription.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70379/#review214339
-----------------------------------------------------------
FAIL: Some of the unit tests failed. Please check the relevant logs.
Reviews applied: `['70377', '70378', '70379']`
Failed command: `Start-MesosCITesting`
All the build artifacts available at: http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/3073/mesos-review-70379
Relevant logs:
- [mesos-tests.log](http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/3073/mesos-review-70379/logs/mesos-tests.log):
```
I0403 18:19:23.011040 24528 ex20efa6551-0000 because it is terminating
I0403 18:19:23.013039 28892 master.cpp:1295] Agent 3e901d88-ac44-4d8d-bcf4-11120efa6551-S0 at slave(501)@192.10.1.4:64572 (windows-01.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net) disconnected
I0403 18:19:23.013039 28892 master.cpp:3391] Disconnecting agent 3e901d88-ac44-4d8d-bcf4-11120efa6551-S0 at slave(501)@192.10.1.4:64572 (windows-01.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net)
I0403 18:19:23.013039 28892 master.cpp:3410] Deactivating agent 3e901d88-ac44-4d8d-bcf4-11120efa6551-S0 at slave(501)@192.10.1.4:64572 (windows-01.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net)
I0403 18:19:23.014039 29372 hierarchical.cpp:391] Removed framework 3e901d88-ac44-4d8d-bcf4-11120efa6551-0000
I0403 18:19:23.014039 29372 hierarchical.cpp:828] Agent 3e901d88-ac44-4d8d-bcf4-11120efa6551-S0 deactivated
I0403 18:19:23.015053 28892 containerizer.cpp:2576] Destroying container 03960b1d-79a4-4e2e-8f39-dc49b3d4e314 in RUNNING state
I0403 18:19:23.015053 28892 container[ OK ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0 (783 ms)
[----------] 1 test from IsolationFlag/MemoryIsolatorTest (800 ms total)
[----------] Global test environment tear-down
[==========] 1134 tests from 107 test cases ran. (579318 ms total)
[ PASSED ] 1133 tests.
[ FAILED ] 1 test, listed below:
[ FAILED ] DockerFetcherPluginTest.INTERNET_CURL_InvokeFetchByName
1 FAILED TEST
YOU HAVE 231 DISABLED TESTS
izer.cpp:3278] Transitioning the state of container 03960b1d-79a4-4e2e-8f39-dc49b3d4e314 from RUNNING to DESTROYING
I0403 18:19:23.015053 28892 launcher.cpp:161] Asked to destroy container 03960b1d-79a4-4e2e-8f39-dc49b3d4e314
W0403 18:19:23.017051 11776 process.cpp:1423] Failed to recv on socket WindowsFD::Type::SOCKET=8528 to peer '192.10.1.4:50635': IO failed with error code: The specified network name is no longer available.
W0403 18:19:23.017051 11776 process.cpp:838] Failed to recv on socket WindowsFD::Type::SOCKET=9016 to peer '192.10.1.4:50636': IO failed with error code: The specified network name is no longer available.
I0403 18:19:23.093060 29372 containerizer.cpp:3117] Container 03960b1d-79a4-4e2e-8f39-dc49b3d4e314 has exited
I0403 18:19:23.125057 29556 master.cpp:1135] Master terminating
I0403 18:19:23.127055 29372 hierarchical.cpp:679] Removed agent 3e901d88-ac44-4d8d-bcf4-11120efa6551-S0
I0403 18:19:23.541079 11776 process.cpp:927] Stopped the socket accept loop
```
- Mesos Reviewbot Windows
On April 3, 2019, 4:04 p.m., Andrei Sekretenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70379/
> -----------------------------------------------------------
>
> (Updated April 3, 2019, 4:04 p.m.)
>
>
> Review request for mesos and Gastón Kleiman.
>
>
> Bugs: MESOS-2842
> https://issues.apache.org/jira/browse/MESOS-2842
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added validation that the principal stays the same on a framework resubscription.
> This solves MESOS-2842.
>
> There are at least two objectionable places in this patch - see TODO in the diff.
>
>
> Diffs
> -----
>
> src/master/master.hpp 94891af9deeaddb3333fc9d6eabb243aed97f7b7
> src/master/master.cpp cf5caa0893ba1387a1f3a9d129ecd7d974f776bd
>
>
> Diff: https://reviews.apache.org/r/70379/diff/1/
>
>
> Testing
> -------
>
> make check
>
> Now the two failing tests from https://reviews.apache.org/r/70377/ pass.
>
>
> Thanks,
>
> Andrei Sekretenko
>
>
Re: Review Request 70379: Added validation that the principal stays
the same on resubscription.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70379/#review214338
-----------------------------------------------------------
Patch looks great!
Reviews applied: [70377, 70378, 70379]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers --disable-parallel-test-execution' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On April 3, 2019, 4:04 p.m., Andrei Sekretenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70379/
> -----------------------------------------------------------
>
> (Updated April 3, 2019, 4:04 p.m.)
>
>
> Review request for mesos and Gastón Kleiman.
>
>
> Bugs: MESOS-2842
> https://issues.apache.org/jira/browse/MESOS-2842
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added validation that the principal stays the same on a framework resubscription.
> This solves MESOS-2842.
>
> There are at least two objectionable places in this patch - see TODO in the diff.
>
>
> Diffs
> -----
>
> src/master/master.hpp 94891af9deeaddb3333fc9d6eabb243aed97f7b7
> src/master/master.cpp cf5caa0893ba1387a1f3a9d129ecd7d974f776bd
>
>
> Diff: https://reviews.apache.org/r/70379/diff/1/
>
>
> Testing
> -------
>
> make check
>
> Now the two failing tests from https://reviews.apache.org/r/70377/ pass.
>
>
> Thanks,
>
> Andrei Sekretenko
>
>