> On Aug. 27, 2016, 9:12 a.m., Qian Zhang wrote: > > src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp, > > lines 143-144 > > <https://reviews.apache.org/r/51097/diff/12/?file=1487236#file1487236line143> > > > > I do not see the code to check the executable permissions on the plugin.
Actually should be removing this comment. This is still up for discussion, but I don't think it makes sense to check permissions for these binaries. This is in any case a best-effort check. It made sense to do these check in `network/cni` isolator since these checks were being done at startup and we didn't want to wait for container launch to detect these checks. In the case of the CNI plugin, the plugins are going to invoked almost immediately, and it would just make sense to let the OS perform these checks (which are much more stringent) and return the failures if any. Will simply the code a lot. Will go ahead and remove the comment for the time being. - Avinash ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51097/#review147073 ----------------------------------------------------------- On Aug. 28, 2016, 7:14 p.m., Avinash sridharan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51097/ > ----------------------------------------------------------- > > (Updated Aug. 28, 2016, 7:14 p.m.) > > > Review request for mesos, Jie Yu and Qian Zhang. > > > Bugs: MESOS-6023 > https://issues.apache.org/jira/browse/MESOS-6023 > > > Repository: mesos > > > Description > ------- > > This class will embody the logic for implementing the CNI port-mapper > plugin. > > Also added helper functions in `cni::spec` to be able to log CNI > plugin error as a JSON formatted string of `spec::error`. > > > Diffs > ----- > > src/Makefile.am 120a715eeeb7fb222d1169500950b5c7643df554 > > src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/main.cpp > PRE-CREATION > > src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.hpp > PRE-CREATION > > src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp > PRE-CREATION > src/slave/containerizer/mesos/isolators/network/cni/spec.hpp > e6967415a689184f13d65a986f13b0af54364fb2 > src/slave/containerizer/mesos/isolators/network/cni/spec.cpp > 938a9f3bff2a95c8591450f0edafaddb01833e59 > > Diff: https://reviews.apache.org/r/51097/diff/ > > > Testing > ------- > > Tested the port-mapper with the following CNI config: > { > "name": "mynet", > "type": "port-mapper", > "chain": "MESOS", > "delegate": { > "type" : "bridge", > "bridge": "cni0", > "isGateway": true, > "ipMasq": true, > "ipam": { > "type": "host-local", > "subnet": "10.22.0.0/16", > "routes": [ > { "dst": "0.0.0.0/0" } > ] > } > }, > "args" : { > "org.apache.mesos" : { > "network_info" : { > "port_mappings": { > "host_port" : 80, > "container_port" : 80 > } > } > } > } > } > > and the following environment variables: > export CNI_COMMAND="ADD" > export CNI_CONTAINERID="0000000111101110" > export CNI_PATH="$MESOS_INSTALL:/home/vagrant/dev/go/cni/bin" > export CNI_IFNAME="eth0" > export CNI_NETNS="/etc/netns" > > If we remove fields from the above CNI config, or remove certain environment > variables the creation of the `PortMapper` correctly fails. However, if > config and environment variables are passed as is it will create the > `PortMapper` correctly. > > > Thanks, > > Avinash sridharan > >
