> On March 21, 2016, 3:08 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.hpp, lines 32-56
> > <https://reviews.apache.org/r/44706/diff/2/?file=1307514#file1307514line32>
> >
> > Can we introduce paths.hpp|cpp under cni/ directory for the canonical
> > locations.
> >
> > ```
> > constexpr char ROOT_DIR[] = "...";
> >
> > string cni::paths::getNamespaceHandle(
> > const string& rootDir,
> > const ContainerID& containerId);
> >
> > string cni::paths::getNetworkPath(
> > const string& rootDir,
> > const ContainerID& containerId,
> > const string& name);
> >
> > string cni::paths::getIPv4Path(
> > const string& rootDir,
> > const ContainerID& containerId,
> > const string& name,
> > const string& ifname);
> >
> > string cni::paths::getIPv6Path(
> > const string& rootDir,
> > const ContainerID& containerId,
> > const string& name,
> > const string& ifname);
> > ```
Thanks for the comments, I have two questions:
1. Do you mean we define rootDir as a const string like this: `constexpr char
ROOT_DIR[] = "/var/run/mesos/isolators/network/cni/"`? If so, I'd like to know
why we still need `rootDir` as the first parameter of those methods (e.g.,
`getNamespaceHandle`), I think we should be able to directly use that const
string inside of those methods rather than passing it as a parameter, right?
2. Take `getNamespaceHandle` as an example, so it will be like:
string getNamespaceHandle(const ContainerID& containerId)
{
return path::join(ROOT_DIR, containerId);
}
I think each of those `getXXX` method will be just a call to `path::join()`, so
what is its advantage against directly calling `path::join()` in cni.cpp?
> On March 21, 2016, 3:08 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, lines 206-212
> > <https://reviews.apache.org/r/44706/diff/2/?file=1307515#file1307515line206>
> >
> > I suggest we save a rootDir in the isolator process. We can easily
> > switch to use a flag later. Also, we need to call 'realpath' here to make
> > sure it's a realpath.
> >
> > We also need to make sure ROOT_DIR is a self bind mounted directory
> > (slave+shared) so that namespace bind mount does not leak into containers.
Do you mean we call `realpath()` to get the real path of the const string
`ROOT_DIR` first and then call `mkdir` with the real path as its parameter to
create the directory?
And can you please elaborate why the namespace bind mount can be leaked into
containers if we do not make `ROOT_DIR` as a self bind mounted directory? I
just want to know the rationale behind it :-)
- Qian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44706/#review124460
-----------------------------------------------------------
On March 21, 2016, 12:27 a.m., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44706/
> -----------------------------------------------------------
>
> (Updated March 21, 2016, 12:27 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 isolate() method of "network/cni" isolator.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/network/cni/cni.hpp PRE-CREATION
> src/slave/containerizer/mesos/isolators/network/cni/cni.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/44706/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Qian Zhang
>
>
