Re: Review Request 44269: Added the framework of 'network/cni' isolator.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44269/#review122686 --- I will discard this patch since I have broken it into two:

Re: Review Request 44269: Added the framework of 'network/cni' isolator.

> On March 9, 2016, 2:09 a.m., Gilbert Song wrote: > > src/slave/containerizer/mesos/isolators/network/cni.cpp, lines 95-288 > > > > > > I would strongly recommend move all `parse` and `validation` operations > > to

Re: Review Request 44269: Added the framework of 'network/cni' isolator.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44269/#review122546 --- src/slave/containerizer/mesos/isolators/network/cni.cpp (line

Re: Review Request 44269: Added the framework of 'network/cni' isolator.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44269/ --- (Updated March 8, 2016, 10:42 p.m.) Review request for mesos, Avinash

Re: Review Request 44269: Added the framework of 'network/cni' isolator.

> On March 4, 2016, 6:55 p.m., Avinash sridharan wrote: > > src/slave/containerizer/mesos/isolators/network/cni.cpp, line 61 > > > > > > Add a `+` at the end. While not necessary, makes it more readable. > > Qian

Re: Review Request 44269: Added the framework of 'network/cni' isolator.

> On March 4, 2016, 11:30 p.m., Avinash sridharan wrote: > > src/slave/containerizer/mesos/isolators/network/cni.cpp, line 134 > > > > > > I don't understand this comment. We just made sure the plugin does not > >

Re: Review Request 44269: Added the framework of 'network/cni' isolator.

> On March 4, 2016, 3:30 p.m., Avinash sridharan wrote: > > src/slave/containerizer/mesos/isolators/network/cni.cpp, line 134 > > > > > > I don't understand this comment. We just made sure the plugin does not > >

Re: Review Request 44269: Added the framework of 'network/cni' isolator.

> On March 4, 2016, 11:30 p.m., Avinash sridharan wrote: > > src/slave/containerizer/mesos/isolators/network/cni.cpp, line 134 > > > > > > I don't understand this comment. We just made sure the plugin does not > >

Re: Review Request 44269: Added the framework of 'network/cni' isolator.

> On March 4, 2016, 3:30 p.m., Avinash sridharan wrote: > > src/slave/containerizer/mesos/isolators/network/cni.cpp, line 134 > > > > > > I don't understand this comment. We just made sure the plugin does not > >

Re: Review Request 44269: Added the framework of 'network/cni' isolator.

> On March 4, 2016, 11:30 p.m., Avinash sridharan wrote: > > src/slave/containerizer/mesos/isolators/network/cni.cpp, line 134 > > > > > > I don't understand this comment. We just made sure the plugin does not > >

Re: Review Request 44269: Added the framework of 'network/cni' isolator.

> On March 4, 2016, 3:30 p.m., Avinash sridharan wrote: > > src/slave/containerizer/mesos/isolators/network/cni.cpp, lines 108-115 > > > > > > isn't this the same as !nameValue.isSome() ? > > > > Also

Re: Review Request 44269: Added the framework of 'network/cni' isolator.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44269/#review122235 --- Patch looks great! Reviews applied: [44004, 44200, 44269]

Re: Review Request 44269: Added the framework of 'network/cni' isolator.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44269/ --- (Updated March 7, 2016, 12:03 a.m.) Review request for mesos, Avinash

Re: Review Request 44269: Added the framework of 'network/cni' isolator.

> On March 4, 2016, 11:30 p.m., Avinash sridharan wrote: > > src/slave/containerizer/mesos/isolators/network/cni.cpp, line 134 > > > > > > I don't understand this comment. We just made sure the plugin does not > >

Re: Review Request 44269: Added the framework of 'network/cni' isolator.

> On March 5, 2016, 2:55 a.m., Avinash sridharan wrote: > > src/slave/containerizer/mesos/isolators/network/cni.cpp, line 61 > > > > > > Add a `+` at the end. While not necessary, makes it more readable. > > Qian

Re: Review Request 44269: Added the framework of 'network/cni' isolator.

> On March 5, 2016, 2:55 a.m., Avinash sridharan wrote: > > src/slave/containerizer/mesos/isolators/network/cni.cpp, line 45 > > > > > > Remove the period at the end. We don't have terminal sentences in ERROR > >

Re: Review Request 44269: Added the framework of 'network/cni' isolator.

> On March 4, 2016, 3:30 p.m., Avinash sridharan wrote: > > src/slave/containerizer/mesos/isolators/network/cni.cpp, line 129 > > > > > > Can we also check if the plugin is executable? Since later we are going > >

Re: Review Request 44269: Added the framework of 'network/cni' isolator.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44269/#review122155 --- Patch looks great! Reviews applied: [44004, 44200, 44269]

Re: Review Request 44269: Added the framework of 'network/cni' isolator.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44269/#review122123 --- src/slave/containerizer/mesos/isolators/network/cni.cpp (line

Re: Review Request 44269: Added the framework of 'network/cni' isolator.

> On March 4, 2016, 3:30 p.m., Avinash sridharan wrote: > > src/slave/containerizer/mesos/isolators/network/cni.cpp, line 80 > > > > > > Shouldn't we do a size check here as well ? Bail out if we don't have > > any

Re: Review Request 44269: Added the framework of 'network/cni' isolator.

> On March 4, 2016, 3:30 p.m., Avinash sridharan wrote: > > src/slave/containerizer/mesos/isolators/network/cni.cpp, lines 108-115 > > > > > > isn't this the same as !nameValue.isSome() ? > > > > Also

Re: Review Request 44269: Added the framework of 'network/cni' isolator.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44269/ --- (Updated March 5, 2016, 1:07 a.m.) Review request for mesos, Avinash

Re: Review Request 44269: Added the framework of 'network/cni' isolator.

> On March 4, 2016, 11:30 p.m., Avinash sridharan wrote: > > src/slave/containerizer/mesos/isolators/network/cni.hpp, line 28 > > > > > > Maybe: > > The isolator implements support for Container Network

Re: Review Request 44269: Added the framework of 'network/cni' isolator.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44269/#review122092 --- Patch looks great! Reviews applied: [44200, 44269] Passed

Re: Review Request 44269: Added the framework of 'network/cni' isolator.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44269/#review122077 --- src/slave/containerizer/mesos/isolators/network/cni.hpp (line

Re: Review Request 44269: Added the framework of 'network/cni' isolator.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44269/ --- (Updated March 4, 2016, 10:34 p.m.) Review request for mesos, Avinash

Re: Review Request 44269: Added the framework of 'network/cni' isolator.

> On March 4, 2016, 1:15 a.m., Gilbert Song wrote: > > src/slave/containerizer/mesos/isolators/network/cni.cpp, line 24 > > > > > > We try to avoid using this now. Let's use 'using process::XXX' > > explicitly here.

Re: Review Request 44269: Added the framework of 'network/cni' isolator.

> On March 4, 2016, 1:19 a.m., Gilbert Song wrote: > > src/slave/containerizer/mesos/isolators/network/cni.cpp, lines 26-27 > > > > > > Depending on the comment above, please move `using std::XX` above > > `using

Re: Review Request 44269: Added the framework of 'network/cni' isolator.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44269/#review121953 --- Patch looks great! Reviews applied: [44200, 44269] Passed

Re: Review Request 44269: Added the framework of 'network/cni' isolator.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44269/#review121885 --- src/slave/containerizer/mesos/isolators/network/cni.cpp (lines

Re: Review Request 44269: Added the framework of 'network/cni' isolator.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44269/#review121884 --- src/slave/containerizer/mesos/isolators/network/cni.cpp (line

Re: Review Request 44269: Added the framework of 'network/cni' isolator.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44269/ --- (Updated March 3, 2016, 9:57 p.m.) Review request for mesos, Avinash

Re: Review Request 44269: Added the framework of 'network/cni' isolator.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44269/ --- (Updated March 3, 2016, 5:58 p.m.) Review request for mesos, Avinash

Re: Review Request 44269: Added the framework of 'network/cni' isolator.

> On March 3, 2016, 12:24 a.m., Avinash sridharan wrote: > > src/slave/containerizer/mesos/containerizer.cpp, lines 81-84 > > > > > > Lets do this in a separate patch once the implementation for the > > isolator is

Re: Review Request 44269: Added the framework of 'network/cni' isolator.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44269/#review121706 --- Patch looks great! Reviews applied: [44200, 44269] Passed

Re: Review Request 44269: Added the framework of 'network/cni' isolator.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44269/#review121663 --- src/slave/containerizer/mesos/containerizer.cpp (lines 81 - 84)