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 64998: Added a SLRP test for CSI plugin restart.
Date Wed, 17 Jan 2018 02:34:11 GMT

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



Nice test!!


src/slave/container_daemon_process.hpp
Lines 26-27 (patched)
<https://reviews.apache.org/r/64998/#comment274718>

    Let's include <process/http.hpp> as well, to bring in the declaration of `process::http::URL`,
which you use below.



src/tests/storage_local_resource_provider_tests.cpp
Lines 1552-1559 (patched)
<https://reviews.apache.org/r/64998/#comment274730>

    Could this be racy? Since we're not awaiting on the UpdateSlaveMessage containing the
RP resources, is it possible that the first offer the scheduler receives will not contain
any raw disk?



src/tests/storage_local_resource_provider_tests.cpp
Lines 1562-1565 (patched)
<https://reviews.apache.org/r/64998/#comment274731>

    Do you really need the lambda here? What about:
    
    ```
    Future<hashset<ContainerID>> pluginContainers = containerizer->containers();
    
    AWAIT_READY(pluginContainers);
    ASSERT_EQ(1u, pluginContainers->size());
    
    const ContainerID containerId = *(pluginContainers.begin());
    ```


- Greg Mann


On Jan. 10, 2018, 2:33 a.m., Chun-Hung Hsiao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64998/
> -----------------------------------------------------------
> 
> (Updated Jan. 10, 2018, 2:33 a.m.)
> 
> 
> Review request for mesos, Gaston Kleiman, Greg Mann, and Jie Yu.
> 
> 
> Bugs: MESOS-8408
>     https://issues.apache.org/jira/browse/MESOS-8408
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The test does the same as the `PublishResources` test, but it kills the
> CSI plugin contaier between each operation.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am bfe9eb1609c71c980c11e7cb0149caf5bdd7ab9b 
>   src/slave/container_daemon.cpp d74fa5105c61a6cadfb2610790e559d387ca13a0 
>   src/slave/container_daemon_process.hpp PRE-CREATION 
>   src/tests/storage_local_resource_provider_tests.cpp bbfe95e9818f25fdd5405db3ad2fe355e023f743

> 
> 
> Diff: https://reviews.apache.org/r/64998/diff/4/
> 
> 
> Testing
> -------
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>


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