> On March 12, 2016, 3:28 a.m., Jie Yu wrote: > > src/slave/containerizer/mesos/isolators/network/cni.cpp, line 48 > > <https://reviews.apache.org/r/44555/diff/1/?file=1293049#file1293049line48> > > > > Let's remove this 'empty' check here since you hae the os::exists check > > below.
They are for different purposes, `os::exist` is used to check if the directory exists or not, and here `flags.network_cni_plugins_dir.get().empty()` is used to check if operator does not specify the value of `--network_cni_plugins_dir` (e.g., "`--network_cni_plugins_dir=`") when starting agent. > On March 12, 2016, 3:28 a.m., Jie Yu wrote: > > src/slave/containerizer/mesos/isolators/network/cni.cpp, line 53 > > <https://reviews.apache.org/r/44555/diff/1/?file=1293049#file1293049line53> > > > > Ditto on removing this check. Ditto. > On March 12, 2016, 3:28 a.m., Jie Yu wrote: > > src/slave/containerizer/mesos/isolators/network/cni.cpp, lines 73-77 > > <https://reviews.apache.org/r/44555/diff/1/?file=1293049#file1293049line73> > > > > I think the CNI plugin should also support the case where the user does > > not specify a name for the network. In other words, the CNI network > > isolator should support host network as well. So it's OK to have no plugins. I have two questions: 1. How are we going to define the design behavior of supporting host network in CNI network isolator? I mean if user does not specify name in NetworkInfo when launching task, what should CNI network isolator do in `prepare()` and `isolate()`? And what if user even does not specify the whole NetworkInfo in ContainerInfo? My thinking is, CNI network isolator should not do anything in this case. 2. In the code below, I check whether the plugin binary specified in each network config file exists under `--network_cni_plugins` or not, if here we allow the directory specified by `--network_cni_plugins` to be empty, then I should remove that check too, right? But that way, we can not bail out agent in `create()` to let operator know the issue which will be left to launching task. - Qian ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44555/#review123192 ----------------------------------------------------------- On March 9, 2016, 2:01 p.m., Qian Zhang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/44555/ > ----------------------------------------------------------- > > (Updated March 9, 2016, 2:01 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 > ------- > > Implemented the framework and create() method of "network/cni" isolator. > > > Diffs > ----- > > src/CMakeLists.txt 8f57a5701073bf1eaaa223383e928cf5db8f8ae4 > src/Makefile.am a41e95ddeb838fdebf4ced953c4a29181916e261 > 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/44555/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Qian Zhang > >
