mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Avinash sridharan <avin...@mesosphere.io>
Subject Re: Review Request 44269: Added the framework of 'network/cni' isolator.
Date Fri, 04 Mar 2016 18:32:00 GMT


> On March 4, 2016, 3:30 p.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni.cpp, lines 108-115
> > <https://reviews.apache.org/r/44269/diff/4/?file=1280944#file1280944line108>
> >
> >     isn't this the same as !nameValue.isSome() ?
> >     
> >     Also LOG(WARNING) should be LOG(ERROR)
> 
> Qian Zhang wrote:
>     Please see https://github.com/apache/mesos/blob/0.27.1/src/docker/docker.cpp#L255:L260
for the existing way to handle the return value of `json.get().find()`.

Well... !isSome implies isNone or isError is true which is what you are aiming for ? Will
simplify things I guess.


> On March 4, 2016, 3:30 p.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni.cpp, line 134
> > <https://reviews.apache.org/r/44269/diff/4/?file=1280944#file1280944line134>
> >
> >     I don't understand this comment. We just made sure the plugin does not exist?
So what does the comment imply "it can
> >             // still be valid as long as operator puts the CNI plugin binary
> >             // that it uses under '--network_cni_plugins_dir'." ?
> >             
> >     I think at this point we should return an error. If can't find an executable
for a named network, the behavior will become undefined. We should bail at this point.
> 
> Qian Zhang wrote:
>     My point is, if we can not find a plugin for a named network during initilization,
log a warning message to let operator know this issue, and afterward operator can put the
plugin in the plugin directory without restarting agent, then the named network can still
work.

Lets not rely on the operator heeding WARNING messages and fixing the problem. My concern
is that this is a `FATAL` error since before the operator can rectify the error if containers
are launched the behavior becomes undefined.


- Avinash


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


On March 4, 2016, 5:07 p.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44269/
> -----------------------------------------------------------
> 
> (Updated March 4, 2016, 5:07 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: MESOS-4759
>     https://issues.apache.org/jira/browse/MESOS-4759
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added the framework of 'network/cni' isolator.
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt 0eabfad66f43adff12d08dbf34ca3db1b801c49e 
>   src/Makefile.am 5b54fe04734afc5faa1bba3b84f43a1b8efe8753 
>   src/slave/containerizer/mesos/isolators/network/cni.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/cni.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44269/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>


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