You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Qian Zhang <zh...@cn.ibm.com> on 2016/03/31 16:55:26 UTC
Review Request 45546: Implemented status() method of "network/cni"
isolator.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45546/
-----------------------------------------------------------
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 status() method of "network/cni" isolator.
Diffs
-----
src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 1c8e231813c0579b79681c5d18b1f799a727ead7
Diff: https://reviews.apache.org/r/45546/diff/
Testing
-------
Thanks,
Qian Zhang
Re: Review Request 45546: Implemented status() method of "network/cni"
isolator.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45546/#review126696
-----------------------------------------------------------
Ship it!
Ship It!
- Jie Yu
On April 2, 2016, 3:18 a.m., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45546/
> -----------------------------------------------------------
>
> (Updated April 2, 2016, 3:18 a.m.)
>
>
> Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.
>
>
> Bugs: MESOS-4764
> https://issues.apache.org/jira/browse/MESOS-4764
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Implemented status() method of "network/cni" isolator.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 9fe394a7e837293cb6c9dab7097ddeeb2e38290c
> src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 991139fac2b2583a35981a5c85f971510515cfab
>
> Diff: https://reviews.apache.org/r/45546/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 45546: Implemented status() method of "network/cni"
isolator.
Posted by Qian Zhang <zh...@cn.ibm.com>.
> On April 3, 2016, 2:42 a.m., Jie Yu wrote:
> > Qian, I followed up with a patch here. Please review it:
> > https://reviews.apache.org/r/45620/
Jie, I have posted my comments on that patch.
BTW, for this patch, it seems it has been submitted, so can you please mark it as submitted here?
- Qian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45546/#review126698
-----------------------------------------------------------
On April 2, 2016, 11:18 a.m., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45546/
> -----------------------------------------------------------
>
> (Updated April 2, 2016, 11:18 a.m.)
>
>
> Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.
>
>
> Bugs: MESOS-4764
> https://issues.apache.org/jira/browse/MESOS-4764
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Implemented status() method of "network/cni" isolator.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 9fe394a7e837293cb6c9dab7097ddeeb2e38290c
> src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 991139fac2b2583a35981a5c85f971510515cfab
>
> Diff: https://reviews.apache.org/r/45546/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 45546: Implemented status() method of "network/cni"
isolator.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45546/#review126698
-----------------------------------------------------------
Qian, I followed up with a patch here. Please review it:
https://reviews.apache.org/r/45620/
- Jie Yu
On April 2, 2016, 3:18 a.m., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45546/
> -----------------------------------------------------------
>
> (Updated April 2, 2016, 3:18 a.m.)
>
>
> Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.
>
>
> Bugs: MESOS-4764
> https://issues.apache.org/jira/browse/MESOS-4764
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Implemented status() method of "network/cni" isolator.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 9fe394a7e837293cb6c9dab7097ddeeb2e38290c
> src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 991139fac2b2583a35981a5c85f971510515cfab
>
> Diff: https://reviews.apache.org/r/45546/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 45546: Implemented status() method of "network/cni"
isolator.
Posted by Qian Zhang <zh...@cn.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45546/
-----------------------------------------------------------
(Updated April 2, 2016, 11:18 a.m.)
Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.
Changes
-------
Rebase
Bugs: MESOS-4764
https://issues.apache.org/jira/browse/MESOS-4764
Repository: mesos
Description
-------
Implemented status() method of "network/cni" isolator.
Diffs (updated)
-----
src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 9fe394a7e837293cb6c9dab7097ddeeb2e38290c
src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 991139fac2b2583a35981a5c85f971510515cfab
Diff: https://reviews.apache.org/r/45546/diff/
Testing
-------
Thanks,
Qian Zhang
Re: Review Request 45546: Implemented status() method of "network/cni"
isolator.
Posted by Qian Zhang <zh...@cn.ibm.com>.
> On April 2, 2016, 6:32 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.hpp, lines 93-95
> > <https://reviews.apache.org/r/45546/diff/2/?file=1322022#file1322022line93>
> >
> > This is very confusing. We have both 'network' and 'networkInfo' in 'NetworkInfo'!
> >
> > How about this: we introduce an optional 'ContainerInfo' in 'Info':
> >
> > ```
> > struct Info
> > {
> > Option<ContainerInfo> containerInfo;
> > }
> > ```
So you mean we will have two fields in `struct Info`? Then what fields will `ContainerInfo` have?
struct Info
{
hashmap<std::string, NetworkInfo> networkInfos;
Option<ContainerInfo> containerInfo;
}
How about just rename `struct NetworkInfo` to `struct ContainerNetwork` and rename its field `network` to `cniNetworkInfo` like below?
struct ContainerNetwork
{
std::string networkName;
std::string ifName;
mesos::NetworkInfo networkInfo;
Option<cni::spec::NetworkInfo> cniNetworkInfo
};
struct Info
{
Info (const hashmap<std::string, ContainerNetwork>& _containerNetworks)
: containerNetworks (_containerNetworks) {}
hashmap<std::string, ContainerNetwork> containerNetworks;
};
- Qian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45546/#review126659
-----------------------------------------------------------
On April 1, 2016, 10:17 p.m., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45546/
> -----------------------------------------------------------
>
> (Updated April 1, 2016, 10:17 p.m.)
>
>
> Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.
>
>
> Bugs: MESOS-4764
> https://issues.apache.org/jira/browse/MESOS-4764
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Implemented status() method of "network/cni" isolator.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 3a07540909ed771d1bd3b22888e04d5fb451710d
> src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 486c382365d5293cd9d53b8b239f70a543c46792
>
> Diff: https://reviews.apache.org/r/45546/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 45546: Implemented status() method of "network/cni"
isolator.
Posted by Qian Zhang <zh...@cn.ibm.com>.
> On April 2, 2016, 6:32 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.hpp, lines 93-95
> > <https://reviews.apache.org/r/45546/diff/2/?file=1322022#file1322022line93>
> >
> > This is very confusing. We have both 'network' and 'networkInfo' in 'NetworkInfo'!
> >
> > How about this: we introduce an optional 'ContainerInfo' in 'Info':
> >
> > ```
> > struct Info
> > {
> > Option<ContainerInfo> containerInfo;
> > }
> > ```
>
> Qian Zhang wrote:
> So you mean we will have two fields in `struct Info`? Then what fields will `ContainerInfo` have?
> struct Info
> {
> hashmap<std::string, NetworkInfo> networkInfos;
> Option<ContainerInfo> containerInfo;
> }
>
> How about just rename `struct NetworkInfo` to `struct ContainerNetwork` and rename its field `network` to `cniNetworkInfo` like below?
> struct ContainerNetwork
> {
> std::string networkName;
>
> std::string ifName;
>
> mesos::NetworkInfo networkInfo;
>
> Option<cni::spec::NetworkInfo> cniNetworkInfo
> };
>
> struct Info
> {
> Info (const hashmap<std::string, ContainerNetwork>& _containerNetworks)
> : containerNetworks (_containerNetworks) {}
>
> hashmap<std::string, ContainerNetwork> containerNetworks;
> };
>
> Jie Yu wrote:
> yeah, that's much better!
>
> Qian Zhang wrote:
> Great, I'd like to do the renaming in a separate patch since it is not related to this one (`status()`). So Jie, please review this one, and I will post a separate renaming patch soon.
The renaming patch: https://reviews.apache.org/r/45616/
- Qian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45546/#review126659
-----------------------------------------------------------
On April 2, 2016, 11:18 a.m., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45546/
> -----------------------------------------------------------
>
> (Updated April 2, 2016, 11:18 a.m.)
>
>
> Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.
>
>
> Bugs: MESOS-4764
> https://issues.apache.org/jira/browse/MESOS-4764
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Implemented status() method of "network/cni" isolator.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 9fe394a7e837293cb6c9dab7097ddeeb2e38290c
> src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 991139fac2b2583a35981a5c85f971510515cfab
>
> Diff: https://reviews.apache.org/r/45546/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 45546: Implemented status() method of "network/cni"
isolator.
Posted by Qian Zhang <zh...@cn.ibm.com>.
> On April 2, 2016, 6:32 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.hpp, lines 93-95
> > <https://reviews.apache.org/r/45546/diff/2/?file=1322022#file1322022line93>
> >
> > This is very confusing. We have both 'network' and 'networkInfo' in 'NetworkInfo'!
> >
> > How about this: we introduce an optional 'ContainerInfo' in 'Info':
> >
> > ```
> > struct Info
> > {
> > Option<ContainerInfo> containerInfo;
> > }
> > ```
>
> Qian Zhang wrote:
> So you mean we will have two fields in `struct Info`? Then what fields will `ContainerInfo` have?
> struct Info
> {
> hashmap<std::string, NetworkInfo> networkInfos;
> Option<ContainerInfo> containerInfo;
> }
>
> How about just rename `struct NetworkInfo` to `struct ContainerNetwork` and rename its field `network` to `cniNetworkInfo` like below?
> struct ContainerNetwork
> {
> std::string networkName;
>
> std::string ifName;
>
> mesos::NetworkInfo networkInfo;
>
> Option<cni::spec::NetworkInfo> cniNetworkInfo
> };
>
> struct Info
> {
> Info (const hashmap<std::string, ContainerNetwork>& _containerNetworks)
> : containerNetworks (_containerNetworks) {}
>
> hashmap<std::string, ContainerNetwork> containerNetworks;
> };
>
> Jie Yu wrote:
> yeah, that's much better!
Great, I'd like to do the renaming in a separate patch since it is not related to this one (`status()`). So Jie, please review this one, and I will post a separate renaming patch soon.
- Qian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45546/#review126659
-----------------------------------------------------------
On April 1, 2016, 10:17 p.m., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45546/
> -----------------------------------------------------------
>
> (Updated April 1, 2016, 10:17 p.m.)
>
>
> Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.
>
>
> Bugs: MESOS-4764
> https://issues.apache.org/jira/browse/MESOS-4764
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Implemented status() method of "network/cni" isolator.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 3a07540909ed771d1bd3b22888e04d5fb451710d
> src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 486c382365d5293cd9d53b8b239f70a543c46792
>
> Diff: https://reviews.apache.org/r/45546/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 45546: Implemented status() method of "network/cni"
isolator.
Posted by Jie Yu <yu...@gmail.com>.
> On April 1, 2016, 10:32 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.hpp, lines 93-95
> > <https://reviews.apache.org/r/45546/diff/2/?file=1322022#file1322022line93>
> >
> > This is very confusing. We have both 'network' and 'networkInfo' in 'NetworkInfo'!
> >
> > How about this: we introduce an optional 'ContainerInfo' in 'Info':
> >
> > ```
> > struct Info
> > {
> > Option<ContainerInfo> containerInfo;
> > }
> > ```
>
> Qian Zhang wrote:
> So you mean we will have two fields in `struct Info`? Then what fields will `ContainerInfo` have?
> struct Info
> {
> hashmap<std::string, NetworkInfo> networkInfos;
> Option<ContainerInfo> containerInfo;
> }
>
> How about just rename `struct NetworkInfo` to `struct ContainerNetwork` and rename its field `network` to `cniNetworkInfo` like below?
> struct ContainerNetwork
> {
> std::string networkName;
>
> std::string ifName;
>
> mesos::NetworkInfo networkInfo;
>
> Option<cni::spec::NetworkInfo> cniNetworkInfo
> };
>
> struct Info
> {
> Info (const hashmap<std::string, ContainerNetwork>& _containerNetworks)
> : containerNetworks (_containerNetworks) {}
>
> hashmap<std::string, ContainerNetwork> containerNetworks;
> };
yeah, that's much better!
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45546/#review126659
-----------------------------------------------------------
On April 1, 2016, 2:17 p.m., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45546/
> -----------------------------------------------------------
>
> (Updated April 1, 2016, 2:17 p.m.)
>
>
> Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.
>
>
> Bugs: MESOS-4764
> https://issues.apache.org/jira/browse/MESOS-4764
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Implemented status() method of "network/cni" isolator.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 3a07540909ed771d1bd3b22888e04d5fb451710d
> src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 486c382365d5293cd9d53b8b239f70a543c46792
>
> Diff: https://reviews.apache.org/r/45546/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 45546: Implemented status() method of "network/cni"
isolator.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45546/#review126659
-----------------------------------------------------------
src/slave/containerizer/mesos/isolators/network/cni/cni.hpp (lines 93 - 95)
<https://reviews.apache.org/r/45546/#comment189671>
This is very confusing. We have both 'network' and 'networkInfo' in 'NetworkInfo'!
How about this: we introduce an optional 'ContainerInfo' in 'Info':
```
struct Info
{
Option<ContainerInfo> containerInfo;
}
```
- Jie Yu
On April 1, 2016, 2:17 p.m., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45546/
> -----------------------------------------------------------
>
> (Updated April 1, 2016, 2:17 p.m.)
>
>
> Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.
>
>
> Bugs: MESOS-4764
> https://issues.apache.org/jira/browse/MESOS-4764
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Implemented status() method of "network/cni" isolator.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 3a07540909ed771d1bd3b22888e04d5fb451710d
> src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 486c382365d5293cd9d53b8b239f70a543c46792
>
> Diff: https://reviews.apache.org/r/45546/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 45546: Implemented status() method of "network/cni"
isolator.
Posted by Qian Zhang <zh...@cn.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45546/
-----------------------------------------------------------
(Updated April 1, 2016, 10:17 p.m.)
Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.
Bugs: MESOS-4764
https://issues.apache.org/jira/browse/MESOS-4764
Repository: mesos
Description
-------
Implemented status() method of "network/cni" isolator.
Diffs
-----
src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 3a07540909ed771d1bd3b22888e04d5fb451710d
src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 486c382365d5293cd9d53b8b239f70a543c46792
Diff: https://reviews.apache.org/r/45546/diff/
Testing
-------
Thanks,
Qian Zhang
Re: Review Request 45546: Implemented status() method of "network/cni"
isolator.
Posted by Qian Zhang <zh...@cn.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45546/
-----------------------------------------------------------
(Updated April 1, 2016, 5:53 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 status() method of "network/cni" isolator.
Diffs (updated)
-----
src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 3a07540909ed771d1bd3b22888e04d5fb451710d
src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 486c382365d5293cd9d53b8b239f70a543c46792
Diff: https://reviews.apache.org/r/45546/diff/
Testing
-------
Thanks,
Qian Zhang
Re: Review Request 45546: Implemented status() method of "network/cni"
isolator.
Posted by Qian Zhang <zh...@cn.ibm.com>.
> On April 1, 2016, 2:31 a.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, line 786
> > <https://reviews.apache.org/r/45546/diff/1/?file=1320825#file1320825line786>
> >
> > Shouldn't we return a failure?
I think we should not return a failure because there can be some containers do not want to opt in CNI networks.
> On April 1, 2016, 2:31 a.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, line 790
> > <https://reviews.apache.org/r/45546/diff/1/?file=1320825#file1320825line790>
> >
> > Maybe s/netInfo/_networkInfo
Agree, and let me fix it in `prepare` method as well.
- Qian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45546/#review126388
-----------------------------------------------------------
On March 31, 2016, 10:55 p.m., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45546/
> -----------------------------------------------------------
>
> (Updated March 31, 2016, 10:55 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 status() method of "network/cni" isolator.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 1c8e231813c0579b79681c5d18b1f799a727ead7
>
> Diff: https://reviews.apache.org/r/45546/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 45546: Implemented status() method of "network/cni"
isolator.
Posted by Avinash sridharan <av...@mesosphere.io>.
> On March 31, 2016, 6:31 p.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, line 786
> > <https://reviews.apache.org/r/45546/diff/1/?file=1320825#file1320825line786>
> >
> > Shouldn't we return a failure?
>
> Qian Zhang wrote:
> I think we should not return a failure because there can be some containers do not want to opt in CNI networks.
I see your point. Looks like we are not storing container information for containers that are not connected to a CNI network. This behavior is different compared to other isolators (they store container information for all containers). We should definitely add a comment here explaining this behavior.
- Avinash
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45546/#review126388
-----------------------------------------------------------
On April 1, 2016, 2:17 p.m., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45546/
> -----------------------------------------------------------
>
> (Updated April 1, 2016, 2:17 p.m.)
>
>
> Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.
>
>
> Bugs: MESOS-4764
> https://issues.apache.org/jira/browse/MESOS-4764
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Implemented status() method of "network/cni" isolator.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 3a07540909ed771d1bd3b22888e04d5fb451710d
> src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 486c382365d5293cd9d53b8b239f70a543c46792
>
> Diff: https://reviews.apache.org/r/45546/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 45546: Implemented status() method of "network/cni"
isolator.
Posted by Avinash sridharan <av...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45546/#review126388
-----------------------------------------------------------
Fix it, then Ship it!
Ship It!
src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 786)
<https://reviews.apache.org/r/45546/#comment189379>
Shouldn't we return a failure?
src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 790)
<https://reviews.apache.org/r/45546/#comment189380>
Maybe s/netInfo/_networkInfo
- Avinash sridharan
On March 31, 2016, 2:55 p.m., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45546/
> -----------------------------------------------------------
>
> (Updated March 31, 2016, 2:55 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 status() method of "network/cni" isolator.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 1c8e231813c0579b79681c5d18b1f799a727ead7
>
> Diff: https://reviews.apache.org/r/45546/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 45546: Implemented status() method of "network/cni"
isolator.
Posted by Qian Zhang <zh...@cn.ibm.com>.
> On April 1, 2016, 6:04 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, lines 791-792
> > <https://reviews.apache.org/r/45546/diff/1/?file=1320825#file1320825line791>
> >
> > hum, what about labels and groups in NetworkInfo? I think we should save a copy during launch/recover, and fill the missing ip if needed here.
Agree, will fix it soon.
- Qian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45546/#review126469
-----------------------------------------------------------
On March 31, 2016, 10:55 p.m., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45546/
> -----------------------------------------------------------
>
> (Updated March 31, 2016, 10:55 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 status() method of "network/cni" isolator.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 1c8e231813c0579b79681c5d18b1f799a727ead7
>
> Diff: https://reviews.apache.org/r/45546/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 45546: Implemented status() method of "network/cni"
isolator.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45546/#review126469
-----------------------------------------------------------
src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (lines 791 - 792)
<https://reviews.apache.org/r/45546/#comment189454>
hum, what about labels and groups in NetworkInfo? I think we should save a copy during launch/recover, and fill the missing ip if needed here.
- Jie Yu
On March 31, 2016, 2:55 p.m., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45546/
> -----------------------------------------------------------
>
> (Updated March 31, 2016, 2:55 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 status() method of "network/cni" isolator.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 1c8e231813c0579b79681c5d18b1f799a727ead7
>
> Diff: https://reviews.apache.org/r/45546/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 45546: Implemented status() method of "network/cni"
isolator.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45546/#review126376
-----------------------------------------------------------
Patch looks great!
Reviews applied: [44004, 44200, 44549, 44555, 44622, 44514, 44706, 45082, 45383, 45531, 45532, 45546]
Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh
- Mesos ReviewBot
On March 31, 2016, 2:55 p.m., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45546/
> -----------------------------------------------------------
>
> (Updated March 31, 2016, 2:55 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 status() method of "network/cni" isolator.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 1c8e231813c0579b79681c5d18b1f799a727ead7
>
> Diff: https://reviews.apache.org/r/45546/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Qian Zhang
>
>