mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Mann <g...@mesosphere.io>
Subject Re: Review Request 67791: Prevented master from asking agents to shutdown on auth failures.
Date Tue, 03 Jul 2018 14:13:44 GMT

-----------------------------------------------------------
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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message