> On March 11, 2016, 7:28 p.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.
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? - Avinash ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44555/#review123192 ----------------------------------------------------------- On March 9, 2016, 6:01 a.m., Qian Zhang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/44555/ > ----------------------------------------------------------- > > (Updated March 9, 2016, 6:01 a.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 > >
