You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Jie Yu <yu...@gmail.com> on 2016/04/01 03:08:26 UTC

Review Request 45571: A few cleanups and simplifications in CNI isolator.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45571/
-----------------------------------------------------------

Review request for mesos, Avinash sridharan and Qian Zhang.


Bugs: MESOS-4759
    https://issues.apache.org/jira/browse/MESOS-4759


Repository: mesos


Description
-------

A few cleanups and simplifications in 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/45571/diff/


Testing
-------

make check


Thanks,

Jie Yu


Re: Review Request 45571: A few cleanups and simplifications in CNI isolator.

Posted by Jie Yu <yu...@gmail.com>.

> On April 1, 2016, 4:46 p.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, line 429
> > <https://reviews.apache.org/r/45571/diff/1/?file=1321722#file1321722line429>
> >
> >     You had mentioned that if we return an error during `recover` the agent will restart. This state from checkpointed information that is not changing. After restart would the Agent end up hitting this condition again ?
> 
> Jie Yu wrote:
>     Yes, but this is not possible, right? If that happens, human needs to be involved.
> 
> Avinash sridharan wrote:
>     Yeah, we can't handle this condition. Should we add a `LOG(ERROR)` here or would this be highlighted upstream? Wanted to see if we can explicitly indicate this condition to the operator.

I think the Error message will be visible to the operator? Why you need a LOG(ERROR) here. The agent (or containerizer) will do the LOG(ERROR).


- Jie


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45571/#review126592
-----------------------------------------------------------


On April 1, 2016, 5:36 p.m., Jie Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45571/
> -----------------------------------------------------------
> 
> (Updated April 1, 2016, 5:36 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan and Qian Zhang.
> 
> 
> Bugs: MESOS-4759
>     https://issues.apache.org/jira/browse/MESOS-4759
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> A few cleanups and simplifications in 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/45571/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>


Re: Review Request 45571: A few cleanups and simplifications in CNI isolator.

Posted by Jie Yu <yu...@gmail.com>.

> On April 1, 2016, 4:46 p.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, line 374
> > <https://reviews.apache.org/r/45571/diff/1/?file=1321722#file1321722line374>
> >
> >     s/on/only
> >     instead of `non-host network(s)` maybe `joined a CNI network(s) ?
> >     
> >     Didn't follow the last part of the sentence "and cleanup _might_ be required for that container." What does this mean ?

Add a few more to explain that does that mean.


> On April 1, 2016, 4:46 p.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, line 429
> > <https://reviews.apache.org/r/45571/diff/1/?file=1321722#file1321722line429>
> >
> >     You had mentioned that if we return an error during `recover` the agent will restart. This state from checkpointed information that is not changing. After restart would the Agent end up hitting this condition again ?

Yes, but this is not possible, right? If that happens, human needs to be involved.


> On April 1, 2016, 4:46 p.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, line 475
> > <https://reviews.apache.org/r/45571/diff/1/?file=1321722#file1321722line475>
> >
> >     s/crashes/crashed/
> >     Why would cleanup be called on this container? Cause they are `orphans`?

No, the agent hasn't realized that the container cleanup has done. So after restart, it'll call cleanup again.


> On April 1, 2016, 4:46 p.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, line 1016
> > <https://reviews.apache.org/r/45571/diff/1/?file=1321722#file1321722line1016>
> >
> >     Maybe "failed to detech container " + containerId + " from network '" .....

'containerId' is not needed here because the caller will print it.


- Jie


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45571/#review126592
-----------------------------------------------------------


On April 1, 2016, 1:08 a.m., Jie Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45571/
> -----------------------------------------------------------
> 
> (Updated April 1, 2016, 1:08 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan and Qian Zhang.
> 
> 
> Bugs: MESOS-4759
>     https://issues.apache.org/jira/browse/MESOS-4759
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> A few cleanups and simplifications in 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/45571/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>


Re: Review Request 45571: A few cleanups and simplifications in CNI isolator.

Posted by Avinash sridharan <av...@mesosphere.io>.

> On April 1, 2016, 4:46 p.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, line 429
> > <https://reviews.apache.org/r/45571/diff/1/?file=1321722#file1321722line429>
> >
> >     You had mentioned that if we return an error during `recover` the agent will restart. This state from checkpointed information that is not changing. After restart would the Agent end up hitting this condition again ?
> 
> Jie Yu wrote:
>     Yes, but this is not possible, right? If that happens, human needs to be involved.

Yeah, we can't handle this condition. Should we add a `LOG(ERROR)` here or would this be highlighted upstream? Wanted to see if we can explicitly indicate this condition to the operator.


- Avinash


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45571/#review126592
-----------------------------------------------------------


On April 1, 2016, 5:36 p.m., Jie Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45571/
> -----------------------------------------------------------
> 
> (Updated April 1, 2016, 5:36 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan and Qian Zhang.
> 
> 
> Bugs: MESOS-4759
>     https://issues.apache.org/jira/browse/MESOS-4759
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> A few cleanups and simplifications in 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/45571/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>


Re: Review Request 45571: A few cleanups and simplifications in CNI isolator.

Posted by Avinash sridharan <av...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45571/#review126592
-----------------------------------------------------------




src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 371)
<https://reviews.apache.org/r/45571/#comment189602>

    s/on/only
    instead of `non-host network(s)` maybe `joined a CNI network(s) ?
    
    Didn't follow the last part of the sentence "and cleanup _might_ be required for that container." What does this mean ?



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 379)
<https://reviews.apache.org/r/45571/#comment189608>

    Another case where the container directory might not exist is if the container was launched on the host network? We are doing the right thing here by just returning `Nothing`



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 421)
<https://reviews.apache.org/r/45571/#comment189611>

    Maybe "Currently a container can have only one interface attached to a CNI network."



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 424)
<https://reviews.apache.org/r/45571/#comment189612>

    You had mentioned that if we return an error during `recover` the agent will restart. This state from checkpointed information that is not changing. After restart would the Agent end up hitting this condition again ?



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 470)
<https://reviews.apache.org/r/45571/#comment189613>

    s/crashes/crashed/
    Why would cleanup be called on this container? Cause they are `orphans`?



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 803)
<https://reviews.apache.org/r/45571/#comment189614>

    We should add a comment here explaining why we don't throw a `Failure` as with other isolators. "It is possible that this container is on the host network instead of a CNI network. We only store information for containers that have joined a CNI network."



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 976)
<https://reviews.apache.org/r/45571/#comment189615>

    Maybe "failed to detech container " + containerId + " from network '" .....


- Avinash sridharan


On April 1, 2016, 1:08 a.m., Jie Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45571/
> -----------------------------------------------------------
> 
> (Updated April 1, 2016, 1:08 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan and Qian Zhang.
> 
> 
> Bugs: MESOS-4759
>     https://issues.apache.org/jira/browse/MESOS-4759
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> A few cleanups and simplifications in 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/45571/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>


Re: Review Request 45571: A few cleanups and simplifications in CNI isolator.

Posted by Jie Yu <yu...@gmail.com>.

> On April 1, 2016, 2:55 a.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, lines 418-424
> > <https://reviews.apache.org/r/45571/diff/1/?file=1321722#file1321722line418>
> >
> >     I do not think we need this. If agent crashes after removing the interface directory in `_detach`, then we should not get into this `foreach` loop because `networkNames` returned by `paths::getNetworkNames` should be empty.
> 
> Jie Yu wrote:
>     Hum, I don't think we delete network dir, do we? We delete ifdir first, if all are successful, we delete containerDir. But it's possible that we delete the ifdir, but agent crashes before the containerDir is deleted, right?
> 
> Qian Zhang wrote:
>     Yes, you are right. However, since there is only one ifdir in a network dir, I am thinking we may change `_detach` to remove the network dir rather than remove the ifdir and leave an empty network dir there, and then here we will not need this code:
>       if (interfaces->empty()) {
>         continue;
>       }

We will support adding more than one interface to a network dir, so I think the approach in my patch is better.


- Jie


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45571/#review126511
-----------------------------------------------------------


On April 1, 2016, 1:08 a.m., Jie Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45571/
> -----------------------------------------------------------
> 
> (Updated April 1, 2016, 1:08 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan and Qian Zhang.
> 
> 
> Bugs: MESOS-4759
>     https://issues.apache.org/jira/browse/MESOS-4759
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> A few cleanups and simplifications in 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/45571/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>


Re: Review Request 45571: A few cleanups and simplifications in CNI isolator.

Posted by Qian Zhang <zh...@cn.ibm.com>.

> On April 1, 2016, 10:55 a.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, lines 418-424
> > <https://reviews.apache.org/r/45571/diff/1/?file=1321722#file1321722line418>
> >
> >     I do not think we need this. If agent crashes after removing the interface directory in `_detach`, then we should not get into this `foreach` loop because `networkNames` returned by `paths::getNetworkNames` should be empty.
> 
> Jie Yu wrote:
>     Hum, I don't think we delete network dir, do we? We delete ifdir first, if all are successful, we delete containerDir. But it's possible that we delete the ifdir, but agent crashes before the containerDir is deleted, right?

Yes, you are right. However, since there is only one ifdir in a network dir, I am thinking we may change `_detach` to remove the network dir rather than remove the ifdir and leave an empty network dir there, and then here we will not need this code:
  if (interfaces->empty()) {
    continue;
  }


- Qian


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45571/#review126511
-----------------------------------------------------------


On April 1, 2016, 9:08 a.m., Jie Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45571/
> -----------------------------------------------------------
> 
> (Updated April 1, 2016, 9:08 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan and Qian Zhang.
> 
> 
> Bugs: MESOS-4759
>     https://issues.apache.org/jira/browse/MESOS-4759
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> A few cleanups and simplifications in 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/45571/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>


Re: Review Request 45571: A few cleanups and simplifications in CNI isolator.

Posted by Jie Yu <yu...@gmail.com>.

> On April 1, 2016, 2:55 a.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, lines 418-424
> > <https://reviews.apache.org/r/45571/diff/1/?file=1321722#file1321722line418>
> >
> >     I do not think we need this. If agent crashes after removing the interface directory in `_detach`, then we should not get into this `foreach` loop because `networkNames` returned by `paths::getNetworkNames` should be empty.

Hum, I don't think we delete network dir, do we? We delete ifdir first, if all are successful, we delete containerDir. But it's possible that we delete the ifdir, but agent crashes before the containerDir is deleted, right?


- Jie


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45571/#review126511
-----------------------------------------------------------


On April 1, 2016, 1:08 a.m., Jie Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45571/
> -----------------------------------------------------------
> 
> (Updated April 1, 2016, 1:08 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan and Qian Zhang.
> 
> 
> Bugs: MESOS-4759
>     https://issues.apache.org/jira/browse/MESOS-4759
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> A few cleanups and simplifications in 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/45571/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>


Re: Review Request 45571: A few cleanups and simplifications in CNI isolator.

Posted by Jie Yu <yu...@gmail.com>.

> On April 1, 2016, 2:55 a.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, lines 462-467
> > <https://reviews.apache.org/r/45571/diff/1/?file=1321722#file1321722line462>
> >
> >     Is it possible that agent crashes when checkpointing the output of CNI plugin in `_attach`? Will it cause this file corrupt or only contain partial output of plugin? If so, then we may log a warning message and just continue here.

Yeah, I'll add a TODO.


- Jie


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45571/#review126511
-----------------------------------------------------------


On April 1, 2016, 1:08 a.m., Jie Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45571/
> -----------------------------------------------------------
> 
> (Updated April 1, 2016, 1:08 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan and Qian Zhang.
> 
> 
> Bugs: MESOS-4759
>     https://issues.apache.org/jira/browse/MESOS-4759
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> A few cleanups and simplifications in 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/45571/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>


Re: Review Request 45571: A few cleanups and simplifications in 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/45571/#review126511
-----------------------------------------------------------




src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (lines 413 - 419)
<https://reviews.apache.org/r/45571/#comment189517>

    I do not think we need this. If agent crashes after removing the interface directory in `_detach`, then we should not get into this `foreach` loop because `networkNames` returned by `paths::getNetworkNames` should be empty.



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (lines 457 - 462)
<https://reviews.apache.org/r/45571/#comment189523>

    Is it possible that agent crashes when checkpointing the output of CNI plugin in `_attach`? Will it cause this file corrupt or only contain partial output of plugin? If so, then we may log a warning message and just continue here.



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 826)
<https://reviews.apache.org/r/45571/#comment189525>

    I think we need a `CHECK(infos.contains(containerId));` here.


- Qian Zhang


On April 1, 2016, 9:08 a.m., Jie Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45571/
> -----------------------------------------------------------
> 
> (Updated April 1, 2016, 9:08 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan and Qian Zhang.
> 
> 
> Bugs: MESOS-4759
>     https://issues.apache.org/jira/browse/MESOS-4759
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> A few cleanups and simplifications in 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/45571/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>


Re: Review Request 45571: A few cleanups and simplifications in CNI isolator.

Posted by Avinash sridharan <av...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45571/#review126617
-----------------------------------------------------------


Ship it!




Ship It!

- Avinash sridharan


On April 1, 2016, 5:36 p.m., Jie Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45571/
> -----------------------------------------------------------
> 
> (Updated April 1, 2016, 5:36 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan and Qian Zhang.
> 
> 
> Bugs: MESOS-4759
>     https://issues.apache.org/jira/browse/MESOS-4759
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> A few cleanups and simplifications in 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/45571/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>


Re: Review Request 45571: A few cleanups and simplifications in 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/45571/#review126638
-----------------------------------------------------------



Patch looks great!

Reviews applied: [45571]

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 April 1, 2016, 5:36 p.m., Jie Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45571/
> -----------------------------------------------------------
> 
> (Updated April 1, 2016, 5:36 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan and Qian Zhang.
> 
> 
> Bugs: MESOS-4759
>     https://issues.apache.org/jira/browse/MESOS-4759
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> A few cleanups and simplifications in 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/45571/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>


Re: Review Request 45571: A few cleanups and simplifications in CNI isolator.

Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45571/
-----------------------------------------------------------

(Updated April 1, 2016, 5:36 p.m.)


Review request for mesos, Avinash sridharan and Qian Zhang.


Changes
-------

Review comments.


Bugs: MESOS-4759
    https://issues.apache.org/jira/browse/MESOS-4759


Repository: mesos


Description
-------

A few cleanups and simplifications in 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/45571/diff/


Testing
-------

make check


Thanks,

Jie Yu


Re: Review Request 45571: A few cleanups and simplifications in 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/45571/#review126530
-----------------------------------------------------------



Patch looks great!

Reviews applied: [45571]

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 April 1, 2016, 1:08 a.m., Jie Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45571/
> -----------------------------------------------------------
> 
> (Updated April 1, 2016, 1:08 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan and Qian Zhang.
> 
> 
> Bugs: MESOS-4759
>     https://issues.apache.org/jira/browse/MESOS-4759
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> A few cleanups and simplifications in 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/45571/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>