> 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. > > Qian Zhang wrote: > 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. > > Avinash sridharan wrote: > For (1), I think what Jie was mentioning is to have the network/cni > isolator do a no-op. That is return `Nothing` . > > I think (1) and (2) are not related. Consider the case where an operator > specifies a bunch of network configs, and there are frameworks that want to > join these networks. But, there could be frameworks that want to run on the > host network. The frameworks that want to run on host network don't provide a > `name` making them default to the host network. > > Qian Zhang wrote: > OK, so for (1), we all agree that `network/cni` isolator should do a > no-op in that case. > > And for 2), I understand the case you described which does make sense to > me. However my concern is, if the directory specified by > `--network_cni_plugins_dir` is empty (i.e., no any CNI plugins in this > directory) and there are a bunch of network config files under > `--network_cni_config_dir`, then in `create()` do we still need to check > whether the plugin binary specified in each network config file exists under > `--network_cni_plugins_dir`? If we do the check, then it will definitely fail > since there is no CNI plugins under `--network_cni_plugins_dir`, if we do not > do the check, that means a network config file with an inexistent plugin will > be loaded, then an error must happen when launching task to join such network. > > Avinash sridharan wrote: > Semantic check. We should bail out if we detect a bad config. A bad > config, is a malformed config or a config for which the plugin is not present > or the plugin is not executable. IN the case you mentioned missing plugin is > semantic failure and hence isolator should bail out. Not sure what the > problem is here?
The problem is, in `create()`, how do we handle the case the directory specified by `--network_cni_plugins_dir` is empty and there are a bunch of network config files under `--network_cni_config_dir`? In this case, the plugin in each network config file is not present because `--network_cni_plugins_dir` is an empty directory, so do you mean we should always bail out in this case? - 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 > >