mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Qian Zhang <zhq527...@gmail.com>
Subject Re: Review Request 72690: Implemented the framework and `create` method of "volume/csi" isolator.
Date Mon, 20 Jul 2020 02:22:46 GMT


> On July 17, 2020, 11:22 p.m., Andrei Budnik wrote:
> > src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp
> > Lines 106 (patched)
> > <https://reviews.apache.org/r/72690/diff/1/?file=2235927#file2235927line106>
> >
> >     Why don't we return an error here? If an operator makes a mistake, the CSI isolator
only prints an error and takes the first entry. It may lead to subtle and hard-to-detect errors
in runtime caused by the misconfiguration of CSI plugin.

That is how we handle CNI configurations and resource provider configurations:
https://github.com/apache/mesos/blob/1.10.0/src/slave/containerizer/mesos/isolators/network/cni/cni.cpp#L338:L344
https://github.com/apache/mesos/blob/1.10.0/src/resource_provider/daemon.cpp#L383:L387

So I'd like to be consistent with them.


> On July 17, 2020, 11:22 p.m., Andrei Budnik wrote:
> > src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp
> > Lines 143 (patched)
> > <https://reviews.apache.org/r/72690/diff/1/?file=2235927#file2235927line143>
> >
> >     Using the agent's API to launch every container with CSI Volume specified may
be quite inefficient from both performance and usability standpoints. Can we somehow use an
object that will provide a simple interface to the isolator? Maybe we should create a special
class (Facade design pattern) that will translate calls from the CSI isolator to corresponding
calls of underlying objects (SLRP, VolumeManager, etc.)? An instance of this class will be
passed to the Mesos containerizer.

> Using the agent's API to launch every container with CSI Volume specified may be quite
inefficient from both performance and usability standpoints.

We are not going to use the agent's API to launch every container with CSI Volume specified
(the code flow of launching executor/task containers will not be changed), instead the agentURL
here is used to launch the specified CSI plugin as standalone container which I assume should
not happen frequently.


- Qian


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


On July 17, 2020, 4:58 p.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72690/
> -----------------------------------------------------------
> 
> (Updated July 17, 2020, 4:58 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Greg Mann.
> 
> 
> Bugs: MESOS-10152
>     https://issues.apache.org/jira/browse/MESOS-10152
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Implemented the framework and `create` method of "volume/csi" isolator.
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt bcb5128f0e61af0d541502e4ed833da0487b7792 
>   src/Makefile.am a89919dd7d5ccbc4c5fa79d9a83616608f84ef49 
>   src/slave/containerizer/mesos/isolators/volume/csi/isolator.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/72690/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>


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