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




src/tests/authentication_tests.cpp
Line 82 (original), 82 (patched)
<https://reviews.apache.org/r/67791/#comment288559>

    s/shutdown/shut down/



src/tests/master_authorization_tests.cpp
Line 2394 (original), 2394 (patched)
<https://reviews.apache.org/r/67791/#comment288551>

    s/but not shut down/and is not shut down/



src/tests/master_authorization_tests.cpp
Line 2409 (original), 2409 (patched)
<https://reviews.apache.org/r/67791/#comment288552>

    Maybe leave a comment here explaining to future readers why this is here?
    
    "Previously, agents were shut down when registration failed due to 
authorization. We verify that this no longer occurs."
    
    Here and elsewhere.



src/tests/master_authorization_tests.cpp
Line 2416 (original), 2421-2427 (patched)
<https://reviews.apache.org/r/67791/#comment288557>

    I think that this block is correct, but it doesn't read very intuitively to 
me. Since the clock is resumed and then we await the registration message, it's 
not immediately obvious that we're verifying anything about what happens 
_after_ the registration message is sent.
    
    I would recommend either pausing the entire test (which makes it clear that 
the previous `settle()` has caused all work related to the registration message 
to occur already), or perhaps placing the `AWAIT_READY(registerSlaveMessage)` 
inside the paused block?
    
    WDYT?
    
    Here and elsewhere.


- Greg Mann


On July 2, 2018, 5:04 a.m., Gastón Kleiman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67791/
> -----------------------------------------------------------
> 
> (Updated July 2, 2018, 5:04 a.m.)
> 
> 
> Review request for mesos, Alexander Rojas, Greg Mann, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-8987
>     https://issues.apache.org/jira/browse/MESOS-8987
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The Mesos master sends a `ShutdownMessage` to an agent if there is an
> authentication or an authorization error during agent (re)registration.
> 
> Upon receipt of this message, the agent kills alls its tasks and commits
> suicide. This means that transient auth errors can lead to whole agents
> being killed along with it's tasks.
> 
> This patch prevents the master from sending a `ShutdownMessage` in these
> cases.
> 
> 
> Diffs
> -----
> 
>   src/master/master.cpp ddc8df0ea82241be6c733237feef1553c7669eb2 
>   src/tests/authentication_tests.cpp bd46cbc6d565ea8f2f6956c0424a76ad58607017 
>   src/tests/master_authorization_tests.cpp 
> 80b9d49ba334b915461ff5d6df6c9f922d7593e3 
> 
> 
> Diff: https://reviews.apache.org/r/67791/diff/1/
> 
> 
> Testing
> -------
> 
> `bin/mesos-tests.sh` on GNU/Linux
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Reply via email to