You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Till Toenshoff via Review Board <no...@reviews.apache.org> on 2019/01/25 15:13:30 UTC

Review Request 69839: Fix scheduler library on multiple SUBSCRIBE requests per connection.

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

Review request for mesos, Alexander Rukletsov and Vinod Kone.


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


Repository: mesos


Description
-------

The HTTP scheduler API dictates that on a single connection, the scheduler
may only send a single SUBSCRIBE request. Due to some authentication
related refactorings, this contract got broken.
Adds an additional test validating that the library was enforcing the
above contract.


Diffs
-----

  src/scheduler/scheduler.cpp cb24ba9c8e1d04b8c62bdf07b12758a61b3bf036 
  src/tests/scheduler_tests.cpp b571bb1d20744b943580677a26db4c12c7c311d1 


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


Testing
-------

manual testing; 
Running the included test without patching `scheduler.cpp` -> fails as the master does in fact receive two SUBSCRIBE requests.

`make check`


Thanks,

Till Toenshoff


Re: Review Request 69839: Fixed scheduler library on multiple SUBSCRIBE requests per connection.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69839/#review212439
-----------------------------------------------------------



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

Reviews applied: `['69839']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2830/mesos-review-69839

Relevant logs:

- [mesos-tests.log](http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2830/mesos-review-69839/logs/mesos-tests.log):

```
I0130 05:42:03.596961 22928 master.cpp:11288] Removing task 19ffc0d7-0a0f-41a2-99f1-90db569a057c with resources cpus(allocated: *):4; mem(allocated: *):2048; disk(allocated: *):1024; ports(allocated: *):[31000-32000] of framework 325a24e2-09bf-4319-8075-512a05075e2c-0000 on agent 325a24e2-09bf-4319-8075-512a05075e2c-S0 at slave(473)@192.10.1.6:60486 (windows-02.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net)
I0130 05:42:03.599964 22928 master.cpp:1269] Agent 325a24e2-09bf-4319-8075-512a05075e2c-S0 at slave(473)@192.10.1.6:60486 (windows-02.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net) disconnected
I0130 05:42:03.599964 22928 master.cpp:3272] Disconnecting agent 325a24e2-09bf-4319-8075-512a05075e2c-S0 at slave(473)@192.10.1.6:60486 (windows-02.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net)
I0130 05:42:03.599964 22928 master.cpp:3291] Deactivating agent 325a24e2-09bf-4319-8075-512a05075e2c-S0 at slave(473)@192.10.1.6:60486 (windows-02.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net)
I0130 05:42:03.600972 20728 hierarchical.cpp:358] Removed framework 325a24e2-09bf-4319-8075-512a05075e2c-0000
I0130 05:42:03.600972 20728 hierarchical.cpp:793] Agent 325a24e2-09bf-4319-8075-512a05075e2c-S0 deactivated
I0130 05:42:03.600972 12804 containerizer.cpp:2477] Destroying container d44b872d-7a49-4804-ad73-b1c1238f925f in RUNNING state
I0130 05:42:03.600972 12804 containerizer.cpp:3144] Transitioning the state of container d44b872d-7a49-4804-ad73-b1c1238f925f from RUNNING to DESTROYING
I0130 05:42:03.601981 12804 launcher.cpp:161] Asked to destroy container d44b872d-7a49-4804-ad73-b1c1238f925f
W0130 05:42:03.602957 22940 process.cpp:1423] Failed to recv on socket WindowsFD::Type::SOCKET=4548 to peer '192.10.1.6:62375': IO failed with error code: The specified network name is no longer available.

W0130 05:42:03.602957 22940 process.cpp:838] Failed to recv [       OK ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0 (686 ms)
[----------] 1 test from IsolationFlag/MemoryIsolatorTest (703 ms total)

[----------] Global test environment tear-down
[==========] 1092 tests from 104 test cases ran. (497127 ms total)
[  PASSED  ] 1091 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] DockerFetcherPluginTest.INTERNET_CURL_InvokeFetchByName

 1 FAILED TEST
  YOU HAVE 231 DISABLED TESTS

on socket WindowsFD::Type::SOCKET=4524 to peer '192.10.1.6:62376': IO failed with error code: The specified network name is no longer available.

I0130 05:42:03.659978 20728 containerizer.cpp:2983] Container d44b872d-7a49-4804-ad73-b1c1238f925f has exited
I0130 05:42:03.690968 25564 master.cpp:1109] Master terminating
I0130 05:42:03.691982 23232 hierarchical.cpp:644] Removed agent 325a24e2-09bf-4319-8075-512a05075e2c-S0
I0130 05:42:04.053977 22940 process.cpp:927] Stopped the socket accept loop
```

- Mesos Reviewbot Windows


On Jan. 30, 2019, 4:34 a.m., Till Toenshoff wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69839/
> -----------------------------------------------------------
> 
> (Updated Jan. 30, 2019, 4:34 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Vinod Kone.
> 
> 
> Bugs: MESOS-9210
>     https://issues.apache.org/jira/browse/MESOS-9210
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The HTTP scheduler API dictates that on a single connection, the
> scheduler may only send a single SUBSCRIBE request. Due to recent
> authentication related changes, this contract got broken. This patch
> restores the contract and adds a test validating that the library is
> enforcing it.
> 
> 
> Diffs
> -----
> 
>   src/scheduler/scheduler.cpp cb24ba9c8e1d04b8c62bdf07b12758a61b3bf036 
>   src/tests/scheduler_tests.cpp b571bb1d20744b943580677a26db4c12c7c311d1 
> 
> 
> Diff: https://reviews.apache.org/r/69839/diff/2/
> 
> 
> Testing
> -------
> 
> manual testing; 
> Running the included test without patching `scheduler.cpp` -> fails as the master does in fact receive two SUBSCRIBE requests.
> 
> `make check`
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>


Re: Review Request 69839: Fixed scheduler library on multiple SUBSCRIBE requests per connection.

Posted by Till Toenshoff via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69839/
-----------------------------------------------------------

(Updated Jan. 30, 2019, 4:34 a.m.)


Review request for mesos, Alexander Rukletsov and Vinod Kone.


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


Repository: mesos


Description (updated)
-------

The HTTP scheduler API dictates that on a single connection, the
scheduler may only send a single SUBSCRIBE request. Due to recent
authentication related changes, this contract got broken. This patch
restores the contract and adds a test validating that the library is
enforcing it.


Diffs (updated)
-----

  src/scheduler/scheduler.cpp cb24ba9c8e1d04b8c62bdf07b12758a61b3bf036 
  src/tests/scheduler_tests.cpp b571bb1d20744b943580677a26db4c12c7c311d1 


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

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


Testing
-------

manual testing; 
Running the included test without patching `scheduler.cpp` -> fails as the master does in fact receive two SUBSCRIBE requests.

`make check`


Thanks,

Till Toenshoff


Re: Review Request 69839: Fixed scheduler library on multiple SUBSCRIBE requests per connection.

Posted by Alexander Rukletsov <ru...@gmail.com>.

> On Jan. 25, 2019, 4:07 p.m., Alexander Rukletsov wrote:
> > src/scheduler/scheduler.cpp
> > Line 234 (original), 234 (patched)
> > <https://reviews.apache.org/r/69839/diff/1/?file=2122187#file2122187line234>
> >
> >     I would move it closer to the invokation of `authenticate()` for clarity.

Ups, didn't mean it to be an issue, just a mere suggestion; feel free to ignore.


- Alexander


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


On Jan. 25, 2019, 3:41 p.m., Till Toenshoff wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69839/
> -----------------------------------------------------------
> 
> (Updated Jan. 25, 2019, 3:41 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Vinod Kone.
> 
> 
> Bugs: MESOS-9210
>     https://issues.apache.org/jira/browse/MESOS-9210
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The HTTP scheduler API dictates that on a single connection, the scheduler may
> only send a single SUBSCRIBE request. Due to recent authentication related
> changes, this contract got broken. This patch restores the contract and adds a
> test validating that the library is enforcing it.
> 
> 
> Diffs
> -----
> 
>   src/scheduler/scheduler.cpp cb24ba9c8e1d04b8c62bdf07b12758a61b3bf036 
>   src/tests/scheduler_tests.cpp b571bb1d20744b943580677a26db4c12c7c311d1 
> 
> 
> Diff: https://reviews.apache.org/r/69839/diff/1/
> 
> 
> Testing
> -------
> 
> manual testing; 
> Running the included test without patching `scheduler.cpp` -> fails as the master does in fact receive two SUBSCRIBE requests.
> 
> `make check`
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>


Re: Review Request 69839: Fixed scheduler library on multiple SUBSCRIBE requests per connection.

Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69839/#review212327
-----------------------------------------------------------


Fix it, then Ship it!





src/scheduler/scheduler.cpp
Line 234 (original), 234 (patched)
<https://reviews.apache.org/r/69839/#comment298088>

    I would move it closer to the invokation of `authenticate()` for clarity.


- Alexander Rukletsov


On Jan. 25, 2019, 3:41 p.m., Till Toenshoff wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69839/
> -----------------------------------------------------------
> 
> (Updated Jan. 25, 2019, 3:41 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Vinod Kone.
> 
> 
> Bugs: MESOS-9210
>     https://issues.apache.org/jira/browse/MESOS-9210
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The HTTP scheduler API dictates that on a single connection, the scheduler may
> only send a single SUBSCRIBE request. Due to recent authentication related
> changes, this contract got broken. This patch restores the contract and adds a
> test validating that the library is enforcing it.
> 
> 
> Diffs
> -----
> 
>   src/scheduler/scheduler.cpp cb24ba9c8e1d04b8c62bdf07b12758a61b3bf036 
>   src/tests/scheduler_tests.cpp b571bb1d20744b943580677a26db4c12c7c311d1 
> 
> 
> Diff: https://reviews.apache.org/r/69839/diff/1/
> 
> 
> Testing
> -------
> 
> manual testing; 
> Running the included test without patching `scheduler.cpp` -> fails as the master does in fact receive two SUBSCRIBE requests.
> 
> `make check`
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>


Re: Review Request 69839: Fixed scheduler library on multiple SUBSCRIBE requests per connection.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69839/#review212334
-----------------------------------------------------------



PASS: Mesos patch 69839 was successfully built and tested.

Reviews applied: `['69839']`

All the build artifacts available at: http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2818/mesos-review-69839

- Mesos Reviewbot Windows


On Jan. 25, 2019, 4:41 p.m., Till Toenshoff wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69839/
> -----------------------------------------------------------
> 
> (Updated Jan. 25, 2019, 4:41 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Vinod Kone.
> 
> 
> Bugs: MESOS-9210
>     https://issues.apache.org/jira/browse/MESOS-9210
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The HTTP scheduler API dictates that on a single connection, the scheduler may
> only send a single SUBSCRIBE request. Due to recent authentication related
> changes, this contract got broken. This patch restores the contract and adds a
> test validating that the library is enforcing it.
> 
> 
> Diffs
> -----
> 
>   src/scheduler/scheduler.cpp cb24ba9c8e1d04b8c62bdf07b12758a61b3bf036 
>   src/tests/scheduler_tests.cpp b571bb1d20744b943580677a26db4c12c7c311d1 
> 
> 
> Diff: https://reviews.apache.org/r/69839/diff/1/
> 
> 
> Testing
> -------
> 
> manual testing; 
> Running the included test without patching `scheduler.cpp` -> fails as the master does in fact receive two SUBSCRIBE requests.
> 
> `make check`
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>


Re: Review Request 69839: Fixed scheduler library on multiple SUBSCRIBE requests per connection.

Posted by Vinod Kone <vi...@apache.org>.

> On Jan. 25, 2019, 4:01 p.m., Vinod Kone wrote:
> > src/scheduler/scheduler.cpp
> > Line 234 (original), 234 (patched)
> > <https://reviews.apache.org/r/69839/diff/1/?file=2122187#file2122187line234>
> >
> >     s/Augmenting/Authenticating/
> 
> Alexander Rukletsov wrote:
>     Please no: authenticatee does not really authenticate, but adds authentication-related headers to the request.

Oh right. I remember that. How about "Adding authentication headers to ..." or something like that?

"Augmenting ..." is very opaque and doesn't convey any useful information for someone looking at logs.


- Vinod


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


On Jan. 25, 2019, 3:41 p.m., Till Toenshoff wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69839/
> -----------------------------------------------------------
> 
> (Updated Jan. 25, 2019, 3:41 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Vinod Kone.
> 
> 
> Bugs: MESOS-9210
>     https://issues.apache.org/jira/browse/MESOS-9210
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The HTTP scheduler API dictates that on a single connection, the scheduler may
> only send a single SUBSCRIBE request. Due to recent authentication related
> changes, this contract got broken. This patch restores the contract and adds a
> test validating that the library is enforcing it.
> 
> 
> Diffs
> -----
> 
>   src/scheduler/scheduler.cpp cb24ba9c8e1d04b8c62bdf07b12758a61b3bf036 
>   src/tests/scheduler_tests.cpp b571bb1d20744b943580677a26db4c12c7c311d1 
> 
> 
> Diff: https://reviews.apache.org/r/69839/diff/1/
> 
> 
> Testing
> -------
> 
> manual testing; 
> Running the included test without patching `scheduler.cpp` -> fails as the master does in fact receive two SUBSCRIBE requests.
> 
> `make check`
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>


Re: Review Request 69839: Fixed scheduler library on multiple SUBSCRIBE requests per connection.

Posted by Alexander Rukletsov <ru...@gmail.com>.

> On Jan. 25, 2019, 4:01 p.m., Vinod Kone wrote:
> > src/scheduler/scheduler.cpp
> > Line 234 (original), 234 (patched)
> > <https://reviews.apache.org/r/69839/diff/1/?file=2122187#file2122187line234>
> >
> >     s/Augmenting/Authenticating/

Please no: authenticatee does not really authenticate, but adds authentication-related headers to the request.


> On Jan. 25, 2019, 4:01 p.m., Vinod Kone wrote:
> > src/scheduler/scheduler.cpp
> > Line 587 (original), 573 (patched)
> > <https://reviews.apache.org/r/69839/diff/1/?file=2122187#file2122187line587>
> >
> >     s/to augment request//
> >     
> >     I didn't know "<< future" is thing!

I would keep this for clarity.


- Alexander


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


On Jan. 25, 2019, 3:41 p.m., Till Toenshoff wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69839/
> -----------------------------------------------------------
> 
> (Updated Jan. 25, 2019, 3:41 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Vinod Kone.
> 
> 
> Bugs: MESOS-9210
>     https://issues.apache.org/jira/browse/MESOS-9210
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The HTTP scheduler API dictates that on a single connection, the scheduler may
> only send a single SUBSCRIBE request. Due to recent authentication related
> changes, this contract got broken. This patch restores the contract and adds a
> test validating that the library is enforcing it.
> 
> 
> Diffs
> -----
> 
>   src/scheduler/scheduler.cpp cb24ba9c8e1d04b8c62bdf07b12758a61b3bf036 
>   src/tests/scheduler_tests.cpp b571bb1d20744b943580677a26db4c12c7c311d1 
> 
> 
> Diff: https://reviews.apache.org/r/69839/diff/1/
> 
> 
> Testing
> -------
> 
> manual testing; 
> Running the included test without patching `scheduler.cpp` -> fails as the master does in fact receive two SUBSCRIBE requests.
> 
> `make check`
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>


Re: Review Request 69839: Fixed scheduler library on multiple SUBSCRIBE requests per connection.

Posted by Vinod Kone <vi...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69839/#review212325
-----------------------------------------------------------




src/scheduler/scheduler.cpp
Line 234 (original), 234 (patched)
<https://reviews.apache.org/r/69839/#comment298083>

    s/Augmenting/Authenticating/



src/scheduler/scheduler.cpp
Line 587 (original), 573 (patched)
<https://reviews.apache.org/r/69839/#comment298087>

    s/to augment request//
    
    I didn't know "<< future" is thing!


- Vinod Kone


On Jan. 25, 2019, 3:41 p.m., Till Toenshoff wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69839/
> -----------------------------------------------------------
> 
> (Updated Jan. 25, 2019, 3:41 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Vinod Kone.
> 
> 
> Bugs: MESOS-9210
>     https://issues.apache.org/jira/browse/MESOS-9210
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The HTTP scheduler API dictates that on a single connection, the scheduler may
> only send a single SUBSCRIBE request. Due to recent authentication related
> changes, this contract got broken. This patch restores the contract and adds a
> test validating that the library is enforcing it.
> 
> 
> Diffs
> -----
> 
>   src/scheduler/scheduler.cpp cb24ba9c8e1d04b8c62bdf07b12758a61b3bf036 
>   src/tests/scheduler_tests.cpp b571bb1d20744b943580677a26db4c12c7c311d1 
> 
> 
> Diff: https://reviews.apache.org/r/69839/diff/1/
> 
> 
> Testing
> -------
> 
> manual testing; 
> Running the included test without patching `scheduler.cpp` -> fails as the master does in fact receive two SUBSCRIBE requests.
> 
> `make check`
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>


Re: Review Request 69839: Fixed scheduler library on multiple SUBSCRIBE requests per connection.

Posted by Till Toenshoff via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69839/
-----------------------------------------------------------

(Updated Jan. 25, 2019, 3:41 p.m.)


Review request for mesos, Alexander Rukletsov and Vinod Kone.


Summary (updated)
-----------------

Fixed scheduler library on multiple SUBSCRIBE requests per connection.


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


Repository: mesos


Description (updated)
-------

The HTTP scheduler API dictates that on a single connection, the scheduler may
only send a single SUBSCRIBE request. Due to recent authentication related
changes, this contract got broken. This patch restores the contract and adds a
test validating that the library is enforcing it.


Diffs
-----

  src/scheduler/scheduler.cpp cb24ba9c8e1d04b8c62bdf07b12758a61b3bf036 
  src/tests/scheduler_tests.cpp b571bb1d20744b943580677a26db4c12c7c311d1 


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


Testing
-------

manual testing; 
Running the included test without patching `scheduler.cpp` -> fails as the master does in fact receive two SUBSCRIBE requests.

`make check`


Thanks,

Till Toenshoff