Re: Review Request 45546: Implemented status() method of "network/cni" isolator.

2016-04-05 Thread Qian Zhang


> 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.

2016-04-02 Thread Jie Yu

---
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.

2016-04-02 Thread Jie Yu

---
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.

2016-04-02 Thread Qian Zhang


> On April 2, 2016, 6:32 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.hpp, lines 93-95
> > 
> >
> > 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;
> > }
> > ```
> 
> Qian Zhang wrote:
> So you mean we will have two fields in `struct Info`? Then what fields 
> will `ContainerInfo` have?
>   struct Info
>   {
> hashmap networkInfos;
> Option 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 cniNetworkInfo
>   };
> 
>   struct Info
>   {
> Info (const hashmap& 
> _containerNetworks)
>   : containerNetworks (_containerNetworks) {}
> 
> hashmap 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.

2016-04-01 Thread Qian Zhang

---
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.

2016-04-01 Thread Qian Zhang


> On April 2, 2016, 6:32 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.hpp, lines 93-95
> > 
> >
> > 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;
> > }
> > ```
> 
> Qian Zhang wrote:
> So you mean we will have two fields in `struct Info`? Then what fields 
> will `ContainerInfo` have?
>   struct Info
>   {
> hashmap networkInfos;
> Option 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 cniNetworkInfo
>   };
> 
>   struct Info
>   {
> Info (const hashmap& 
> _containerNetworks)
>   : containerNetworks (_containerNetworks) {}
> 
> hashmap 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.

2016-04-01 Thread Jie Yu


> On April 1, 2016, 10:32 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.hpp, lines 93-95
> > 
> >
> > 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;
> > }
> > ```
> 
> Qian Zhang wrote:
> So you mean we will have two fields in `struct Info`? Then what fields 
> will `ContainerInfo` have?
>   struct Info
>   {
> hashmap networkInfos;
> Option 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 cniNetworkInfo
>   };
> 
>   struct Info
>   {
> Info (const hashmap& 
> _containerNetworks)
>   : containerNetworks (_containerNetworks) {}
> 
> hashmap 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.

2016-04-01 Thread Qian Zhang


> On April 2, 2016, 6:32 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.hpp, lines 93-95
> > 
> >
> > 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;
> > }
> > ```

So you mean we will have two fields in `struct Info`? Then what fields will 
`ContainerInfo` have?
  struct Info
  {
hashmap networkInfos;
Option 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 cniNetworkInfo
  };

  struct Info
  {
Info (const hashmap& _containerNetworks)
  : containerNetworks (_containerNetworks) {}

hashmap 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.

2016-04-01 Thread Jie Yu

---
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)


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;
}
```


- 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.

2016-04-01 Thread Avinash sridharan


> On March 31, 2016, 6:31 p.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, line 786
> > 
> >
> > 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.

2016-04-01 Thread Qian Zhang

---
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.

2016-04-01 Thread Qian Zhang

---
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.

2016-04-01 Thread Qian Zhang


> On April 1, 2016, 6:04 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, lines 791-792
> > 
> >
> > 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.

2016-04-01 Thread Qian Zhang


> On April 1, 2016, 2:31 a.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, line 786
> > 
> >
> > 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
> > 
> >
> > 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.

2016-03-31 Thread Jie Yu

---
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)


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.

2016-03-31 Thread Avinash sridharan

---
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)


Shouldn't we return a failure?



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 790)


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.

2016-03-31 Thread Mesos ReviewBot

---
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
> 
>