You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Chi Zhang <ch...@gmail.com> on 2014/06/11 20:44:31 UTC
Re: Review Request 21594: Port-Range Based Network Isolator for Linux
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21594/
-----------------------------------------------------------
(Updated June 11, 2014, 6:44 p.m.)
Review request for mesos, Ian Downes, Jie Yu, Vinod Kone, and Cong Wang.
Changes
-------
a new version that includes implementation of update, bug fixes and style fixes.
Summary (updated)
-----------------
Port-Range Based Network Isolator for Linux
Bugs: https://issues.apache.org/jira/browse/MESOS-1324
https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-1324
Repository: mesos-git
Description
-------
Added a network isolator using port-range based traffic redirection on Linux.
- Containers are assigned non-ephemeral ports by the scheduler and ephemeral ports by the network isolator.
- Virtual ethernet devices and Traffic Control filters are set up so that network traffic in and out of the containers is isolated based on the ports assigned to them.
- Containers run inside their own network namespaces with separate network stacks, from which per-container network statistics can be retrieved.
A joint work with:
- Cong Wang (cwang@twopensource.com)
- Jie Yu (yujie.jay@gmail.com)
- Ian Downes (ian.downes@gmail.com)
Diffs (updated)
-----
include/mesos/mesos.proto 62f69d2
src/Makefile.am 4a3f2e1
src/launcher/main.cpp b497e98
src/slave/constants.hpp ace4590
src/slave/constants.cpp 51f65bb
src/slave/containerizer/isolators/network/port_mapping.hpp PRE-CREATION
src/slave/containerizer/isolators/network/port_mapping.cpp PRE-CREATION
src/slave/containerizer/linux_launcher.cpp c17724b
src/slave/containerizer/mesos_containerizer.cpp a38564f
src/slave/flags.hpp 15e5b64
src/slave/main.cpp 8c2b70c
src/tests/environment.cpp 005fc54
src/tests/mesos.cpp ea6a1c0
Diff: https://reviews.apache.org/r/21594/diff/
Testing
-------
make check on linux. more test cases are being written.
Thanks,
Chi Zhang
Re: Review Request 21594: Port-Range Based Network Isolator for Linux
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21594/#review46111
-----------------------------------------------------------
src/launcher/main.cpp
<https://reviews.apache.org/r/21594/#comment81287>
FYI, we will do a refactor on the launcher/Operation interfaces per discussion with BenH.
We will need to create a separate binary (e.g., mesos-network-utils) to launcher the 'Update' operation.
Also, we might wanna add some more operations to mesos-network-utils like cleanup the hosts (removing ingress qdisc on eth0 and lo and remove all veths) to ease the cleanup of a host.
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81280>
Probably adjust the comments here. In theory, should should be able to support an arbitrary positive size, right? Say that this is for performance and debugability concern.
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81283>
return Error(
"The number of ephemeral ports for each container (" +
stringify(flags.per_container_ephemeral_port_size) +
") is not a power of 2");
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81285>
I don't like splitting into 'configureNetwork':
1) it's functionality is not atomic and it's not clear what does this function do from its name
2) it's not used anywhere else
3) people needs to jump while reading the code
- Jie Yu
On June 18, 2014, 1:16 a.m., Chi Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21594/
> -----------------------------------------------------------
>
> (Updated June 18, 2014, 1:16 a.m.)
>
>
> Review request for mesos, Ian Downes, Jie Yu, Vinod Kone, and Cong Wang.
>
>
> Bugs: https://issues.apache.org/jira/browse/MESOS-1324
> https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-1324
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Added a network isolator using port-range based traffic redirection on Linux.
>
> - Containers are assigned non-ephemeral ports by the scheduler and ephemeral ports by the network isolator.
> - Virtual ethernet devices and Traffic Control filters are set up so that network traffic in and out of the containers is isolated based on the ports assigned to them.
> - Containers run inside their own network namespaces with separate network stacks, from which per-container network statistics can be retrieved.
>
> A joint work with:
> - Cong Wang (cwang@twopensource.com)
> - Jie Yu (yujie.jay@gmail.com)
> - Ian Downes (ian.downes@gmail.com)
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 2f6be05
> src/Makefile.am b1b7d2d
> src/launcher/main.cpp b497e98
> src/slave/constants.hpp c65a62d
> src/slave/constants.cpp 51f65bb
> src/slave/containerizer/isolators/network/port_mapping.hpp PRE-CREATION
> src/slave/containerizer/isolators/network/port_mapping.cpp PRE-CREATION
> src/slave/containerizer/linux_launcher.cpp 85c74f0
> src/slave/containerizer/mesos_containerizer.cpp 4d97d49
> src/slave/flags.hpp 3b8ba08
> src/slave/main.cpp 8c2b70c
> src/tests/environment.cpp 005fc54
> src/tests/mesos.cpp f197da6
>
> Diff: https://reviews.apache.org/r/21594/diff/
>
>
> Testing
> -------
>
> make check on linux. more test cases are being written.
>
>
> Thanks,
>
> Chi Zhang
>
>
Re: Review Request 21594: Port-Range Based Network Isolator for Linux
Posted by Vinod Kone <vi...@gmail.com>.
> On June 19, 2014, 2:06 a.m., Vinod Kone wrote:
> > src/slave/containerizer/isolators/network/port_mapping.cpp, lines 1009-1010
> > <https://reviews.apache.org/r/21594/diff/4/?file=611965#file611965line1009>
> >
> > What if it's None?
>
> Chi Zhang wrote:
> it means the link is not found. we have verified that earlier in the same func.
you need to do a CHECK(hostEthoMUT.isSome()) then. i don't think it is obvious from this code here.
> On June 19, 2014, 2:06 a.m., Vinod Kone wrote:
> > src/slave/containerizer/isolators/network/port_mapping.cpp, lines 347-349
> > <https://reviews.apache.org/r/21594/diff/4/?file=611965#file611965line347>
> >
> > Why do we drop these? Are there no apps out there which spoof the source ip?
can you explain why this is dropped? as a courtesy to reviewers, we always expect dropped issues to have an explanation. http://mesos.apache.org/documentation/latest/mesos-developers-guide/
- Vinod
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21594/#review46101
-----------------------------------------------------------
On June 20, 2014, 12:13 a.m., Chi Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21594/
> -----------------------------------------------------------
>
> (Updated June 20, 2014, 12:13 a.m.)
>
>
> Review request for mesos, Ian Downes, Jie Yu, Vinod Kone, and Cong Wang.
>
>
> Bugs: https://issues.apache.org/jira/browse/MESOS-1324
> https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-1324
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Added a network isolator using port-range based traffic redirection on Linux.
>
> - Containers are assigned non-ephemeral ports by the scheduler and ephemeral ports by the network isolator.
> - Virtual ethernet devices and Traffic Control filters are set up so that network traffic in and out of the containers is isolated based on the ports assigned to them.
> - Containers run inside their own network namespaces with separate network stacks, from which per-container network statistics can be retrieved.
>
> A joint work with:
> - Cong Wang (cwang@twopensource.com)
> - Jie Yu (yujie.jay@gmail.com)
> - Ian Downes (ian.downes@gmail.com)
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 2f6be05
> src/Makefile.am b1b7d2d
> src/launcher/main.cpp b497e98
> src/slave/constants.hpp c65a62d
> src/slave/constants.cpp 51f65bb
> src/slave/containerizer/isolators/network/port_mapping.hpp PRE-CREATION
> src/slave/containerizer/isolators/network/port_mapping.cpp PRE-CREATION
> src/slave/containerizer/linux_launcher.cpp 85c74f0
> src/slave/containerizer/mesos_containerizer.cpp 61c0a8d
> src/slave/flags.hpp 3b8ba08
> src/slave/main.cpp 8c2b70c
> src/tests/environment.cpp 21b9d1d
> src/tests/mesos.cpp 1037420
>
> Diff: https://reviews.apache.org/r/21594/diff/
>
>
> Testing
> -------
>
> make check on linux. more test cases are being written.
>
>
> Thanks,
>
> Chi Zhang
>
>
Re: Review Request 21594: Port-Range Based Network Isolator for Linux
Posted by Chi Zhang <ch...@gmail.com>.
> On June 19, 2014, 2:06 a.m., Vinod Kone wrote:
> > src/slave/containerizer/isolators/network/port_mapping.hpp, line 210
> > <https://reviews.apache.org/r/21594/diff/4/?file=611964#file611964line210>
> >
> > What does this mean? Are there un-managed nonEphemeral ports?
commented to say that it's passed in by flags.resources.
> On June 19, 2014, 2:06 a.m., Vinod Kone wrote:
> > src/slave/containerizer/isolators/network/port_mapping.cpp, line 315
> > <https://reviews.apache.org/r/21594/diff/4/?file=611965#file611965line315>
> >
> > s/host// ?
> >
> > "host etho" would be weird in the log? looks like etho is hostname.
> >
> > here and everywhere else.
We have 'host eth0' as well as 'eth0 in the container' that we config on. We are explicit about this through out the code.
> On June 19, 2014, 2:06 a.m., Vinod Kone wrote:
> > src/slave/containerizer/isolators/network/port_mapping.cpp, lines 1009-1010
> > <https://reviews.apache.org/r/21594/diff/4/?file=611965#file611965line1009>
> >
> > What if it's None?
it means the link is not found. we have verified that earlier in the same func.
> On June 19, 2014, 2:06 a.m., Vinod Kone wrote:
> > src/slave/containerizer/isolators/network/port_mapping.hpp, lines 71-72
> > <https://reviews.apache.org/r/21594/diff/4/?file=611964#file611964line71>
> >
> > Should this not return an error if the range is already allocated?
We have CHECKs inside, since it is only used by the network isolator.
- Chi
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21594/#review46101
-----------------------------------------------------------
On June 20, 2014, 12:13 a.m., Chi Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21594/
> -----------------------------------------------------------
>
> (Updated June 20, 2014, 12:13 a.m.)
>
>
> Review request for mesos, Ian Downes, Jie Yu, Vinod Kone, and Cong Wang.
>
>
> Bugs: https://issues.apache.org/jira/browse/MESOS-1324
> https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-1324
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Added a network isolator using port-range based traffic redirection on Linux.
>
> - Containers are assigned non-ephemeral ports by the scheduler and ephemeral ports by the network isolator.
> - Virtual ethernet devices and Traffic Control filters are set up so that network traffic in and out of the containers is isolated based on the ports assigned to them.
> - Containers run inside their own network namespaces with separate network stacks, from which per-container network statistics can be retrieved.
>
> A joint work with:
> - Cong Wang (cwang@twopensource.com)
> - Jie Yu (yujie.jay@gmail.com)
> - Ian Downes (ian.downes@gmail.com)
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 2f6be05
> src/Makefile.am b1b7d2d
> src/launcher/main.cpp b497e98
> src/slave/constants.hpp c65a62d
> src/slave/constants.cpp 51f65bb
> src/slave/containerizer/isolators/network/port_mapping.hpp PRE-CREATION
> src/slave/containerizer/isolators/network/port_mapping.cpp PRE-CREATION
> src/slave/containerizer/linux_launcher.cpp 85c74f0
> src/slave/containerizer/mesos_containerizer.cpp 61c0a8d
> src/slave/flags.hpp 3b8ba08
> src/slave/main.cpp 8c2b70c
> src/tests/environment.cpp 21b9d1d
> src/tests/mesos.cpp 1037420
>
> Diff: https://reviews.apache.org/r/21594/diff/
>
>
> Testing
> -------
>
> make check on linux. more test cases are being written.
>
>
> Thanks,
>
> Chi Zhang
>
>
Re: Review Request 21594: Port-Range Based Network Isolator for Linux
Posted by Chi Zhang <ch...@gmail.com>.
> On June 19, 2014, 2:06 a.m., Vinod Kone wrote:
> > src/slave/containerizer/isolators/network/port_mapping.cpp, lines 347-349
> > <https://reviews.apache.org/r/21594/diff/4/?file=611965#file611965line347>
> >
> > Why do we drop these? Are there no apps out there which spoof the source ip?
>
> Vinod Kone wrote:
> can you explain why this is dropped? as a courtesy to reviewers, we always expect dropped issues to have an explanation. http://mesos.apache.org/documentation/latest/mesos-developers-guide/
We did have a discussion on this but I missed to comment on this. It generally takes root permission to spoof the source ip, which we don't have right now.
- Chi
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21594/#review46101
-----------------------------------------------------------
On June 24, 2014, 7:26 a.m., Chi Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21594/
> -----------------------------------------------------------
>
> (Updated June 24, 2014, 7:26 a.m.)
>
>
> Review request for mesos, Ian Downes, Jie Yu, Vinod Kone, and Cong Wang.
>
>
> Bugs: https://issues.apache.org/jira/browse/MESOS-1324
> https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-1324
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Added a network isolator using port-range based traffic redirection on Linux.
>
> - Containers are assigned non-ephemeral ports by the scheduler and ephemeral ports by the network isolator.
> - Virtual ethernet devices and Traffic Control filters are set up so that network traffic in and out of the containers is isolated based on the ports assigned to them.
> - Containers run inside their own network namespaces with separate network stacks, from which per-container network statistics can be retrieved.
>
> A joint work with:
> - Cong Wang (cwang@twopensource.com)
> - Jie Yu (yujie.jay@gmail.com)
> - Ian Downes (ian.downes@gmail.com)
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 2f6be05
> src/Makefile.am 5e5ccd5
> src/slave/constants.hpp c65a62d
> src/slave/constants.cpp 51f65bb
> src/slave/containerizer/isolators/network/helper.cpp PRE-CREATION
> src/slave/containerizer/isolators/network/port_mapping.hpp PRE-CREATION
> src/slave/containerizer/isolators/network/port_mapping.cpp PRE-CREATION
> src/slave/containerizer/linux_launcher.cpp acaf9b5
> src/slave/containerizer/mesos_containerizer.cpp 917eebf
> src/slave/flags.hpp 3b8ba08
> src/tests/environment.cpp e991d57
> src/tests/isolator_tests.cpp 5a141e3
> src/tests/mesos.cpp 1037420
>
> Diff: https://reviews.apache.org/r/21594/diff/
>
>
> Testing
> -------
>
> make check on linux. more test cases are being written.
>
>
> Thanks,
>
> Chi Zhang
>
>
Re: Review Request 21594: Port-Range Based Network Isolator for Linux
Posted by Jie Yu <yu...@gmail.com>.
> On June 19, 2014, 2:06 a.m., Vinod Kone wrote:
> > src/slave/containerizer/isolators/network/port_mapping.cpp, line 825
> > <https://reviews.apache.org/r/21594/diff/4/?file=611965#file611965line825>
> >
> > as commented earlier, these should really be called "ephemeral_ports" in the flag.
Hum, in fact, I like 'ports' more. It is indeed 'ports' resources (type) and how we use these resources (assign to containers as ephemeral ports) should not affect the resources type. For example, we can reserve some local cpus resources to start some background jobs, we should not call it "background_cpus", right?
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21594/#review46101
-----------------------------------------------------------
On June 18, 2014, 1:16 a.m., Chi Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21594/
> -----------------------------------------------------------
>
> (Updated June 18, 2014, 1:16 a.m.)
>
>
> Review request for mesos, Ian Downes, Jie Yu, Vinod Kone, and Cong Wang.
>
>
> Bugs: https://issues.apache.org/jira/browse/MESOS-1324
> https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-1324
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Added a network isolator using port-range based traffic redirection on Linux.
>
> - Containers are assigned non-ephemeral ports by the scheduler and ephemeral ports by the network isolator.
> - Virtual ethernet devices and Traffic Control filters are set up so that network traffic in and out of the containers is isolated based on the ports assigned to them.
> - Containers run inside their own network namespaces with separate network stacks, from which per-container network statistics can be retrieved.
>
> A joint work with:
> - Cong Wang (cwang@twopensource.com)
> - Jie Yu (yujie.jay@gmail.com)
> - Ian Downes (ian.downes@gmail.com)
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 2f6be05
> src/Makefile.am b1b7d2d
> src/launcher/main.cpp b497e98
> src/slave/constants.hpp c65a62d
> src/slave/constants.cpp 51f65bb
> src/slave/containerizer/isolators/network/port_mapping.hpp PRE-CREATION
> src/slave/containerizer/isolators/network/port_mapping.cpp PRE-CREATION
> src/slave/containerizer/linux_launcher.cpp 85c74f0
> src/slave/containerizer/mesos_containerizer.cpp 4d97d49
> src/slave/flags.hpp 3b8ba08
> src/slave/main.cpp 8c2b70c
> src/tests/environment.cpp 005fc54
> src/tests/mesos.cpp f197da6
>
> Diff: https://reviews.apache.org/r/21594/diff/
>
>
> Testing
> -------
>
> make check on linux. more test cases are being written.
>
>
> Thanks,
>
> Chi Zhang
>
>
Re: Review Request 21594: Port-Range Based Network Isolator for Linux
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21594/#review46101
-----------------------------------------------------------
include/mesos/mesos.proto
<https://reviews.apache.org/r/21594/#comment81261>
I think this merits its own review. Can you split this out? also, put benh on that review.
src/slave/constants.hpp
<https://reviews.apache.org/r/21594/#comment81263>
s/DEFAULT_PER_CONTAINER_EPHEMERAL_PORT_SIZE/DEFAULT_EPHEMERAL_PORTS_PER_CONTAINER/ ?
src/slave/constants.hpp
<https://reviews.apache.org/r/21594/#comment81262>
s/ports/ports range/
src/slave/containerizer/isolators/network/port_mapping.hpp
<https://reviews.apache.org/r/21594/#comment81271>
s/perContainerSize/portsPerContainer/ ?
src/slave/containerizer/isolators/network/port_mapping.hpp
<https://reviews.apache.org/r/21594/#comment81272>
Should this not return an error if the range is already allocated?
src/slave/containerizer/isolators/network/port_mapping.hpp
<https://reviews.apache.org/r/21594/#comment81270>
s/perContainerSize_/portsPerContainer_/ ?
src/slave/containerizer/isolators/network/port_mapping.hpp
<https://reviews.apache.org/r/21594/#comment81273>
s/re-use/reuse/
src/slave/containerizer/isolators/network/port_mapping.hpp
<https://reviews.apache.org/r/21594/#comment81275>
How about a single constructor that takes an Option<pid_t> with maybe a default of None()?
src/slave/containerizer/isolators/network/port_mapping.hpp
<https://reviews.apache.org/r/21594/#comment81274>
s/port/ports/
src/slave/containerizer/isolators/network/port_mapping.hpp
<https://reviews.apache.org/r/21594/#comment81279>
s/childIsolationScript/scripts/ ?
src/slave/containerizer/isolators/network/port_mapping.hpp
<https://reviews.apache.org/r/21594/#comment81278>
What does this mean? Are there un-managed nonEphemeral ports?
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81281>
s/purpose/purposes/ ?
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81284>
how about "t is aligned to power of 2" ?
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81286>
Not sure I understand this algorithm. Maybe some comments?
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81380>
why is this pulled out into a function? seems like it is used only once?
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81288>
print the value.get()?
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81289>
print split[0]?
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81290>
print split[1]?
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81381>
ditto. don't pull out.
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81292>
const?
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81294>
s/host// ?
"host etho" would be weird in the log? looks like etho is hostname.
here and everywhere else.
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81385>
Why do we drop these? Are there no apps out there which spoof the source ip?
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81387>
ditto. why drop?
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81393>
print the pid?
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81395>
What should users do when they encounter this error? Maybe include that in the message?
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81396>
s/checkRoutingLibrary/check/
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81397>
s/checkCommandTc/shell/
or
s/checkCommandTc/status/
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81398>
reuse the variable from above.
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81399>
as commented earlier, these should really be called "ephemeral_ports" in the flag.
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81400>
hmm. this name "previousPowerofTwo" here doesn't really convey what you are looking for. is there a better name?
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81401>
print the sizes.
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81402>
print that it needs atleast MIN_EPHEMERAL_PORTS_SIZE
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81403>
s/hostEthoExists/exists/.
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81404>
How about
if (!hostIP.isSome()) {
return Error("Failed to get the public IP of " + eth0.get() +
": " + (hostIP.isError() ? hostIP.error() : "does not have IPv4 address"));
}
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81405>
ditto.
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81406>
s/fair/fairly/.
Also, a blurb about why we need to do this?
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81407>
What if it's None?
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81408>
+1
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81413>
I don't think the containerizer is supposed to ask isolators to recover a run that doesn't have a forked pid. So this should be a CHECK instead?
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81416>
This would log once for every container! We don't do this with other isolators. Do you want a VLOG instead?
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81420>
s/expectedly/unexpectedly/ ?
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81421>
s/should/we should/
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81422>
VLOG?
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81423>
also print the executor id.
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81425>
"target to source" or "source to target"?
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81429>
s/is/to be/
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81432>
Add a comment on what this function does?
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81435>
s/allocator/ephemeral ports allocator/
src/slave/flags.hpp
<https://reviews.apache.org/r/21594/#comment81268>
Maybe add a blurb about port mapping isolation here? You probably also want to mention about the configure option.
src/slave/flags.hpp
<https://reviews.apache.org/r/21594/#comment81264>
s/per_container_epehemeral_port_size/epehemeral_ports_per_container/ ?
src/slave/flags.hpp
<https://reviews.apache.org/r/21594/#comment81265>
maybe also explicitly mention that these are not exposed to master/frameworks.
src/slave/flags.hpp
<https://reviews.apache.org/r/21594/#comment81266>
s/type/type of/
src/slave/flags.hpp
<https://reviews.apache.org/r/21594/#comment81267>
s/ports/ephemeral_ports/ to be more explicit?
- Vinod Kone
On June 18, 2014, 1:16 a.m., Chi Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21594/
> -----------------------------------------------------------
>
> (Updated June 18, 2014, 1:16 a.m.)
>
>
> Review request for mesos, Ian Downes, Jie Yu, Vinod Kone, and Cong Wang.
>
>
> Bugs: https://issues.apache.org/jira/browse/MESOS-1324
> https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-1324
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Added a network isolator using port-range based traffic redirection on Linux.
>
> - Containers are assigned non-ephemeral ports by the scheduler and ephemeral ports by the network isolator.
> - Virtual ethernet devices and Traffic Control filters are set up so that network traffic in and out of the containers is isolated based on the ports assigned to them.
> - Containers run inside their own network namespaces with separate network stacks, from which per-container network statistics can be retrieved.
>
> A joint work with:
> - Cong Wang (cwang@twopensource.com)
> - Jie Yu (yujie.jay@gmail.com)
> - Ian Downes (ian.downes@gmail.com)
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 2f6be05
> src/Makefile.am b1b7d2d
> src/launcher/main.cpp b497e98
> src/slave/constants.hpp c65a62d
> src/slave/constants.cpp 51f65bb
> src/slave/containerizer/isolators/network/port_mapping.hpp PRE-CREATION
> src/slave/containerizer/isolators/network/port_mapping.cpp PRE-CREATION
> src/slave/containerizer/linux_launcher.cpp 85c74f0
> src/slave/containerizer/mesos_containerizer.cpp 4d97d49
> src/slave/flags.hpp 3b8ba08
> src/slave/main.cpp 8c2b70c
> src/tests/environment.cpp 005fc54
> src/tests/mesos.cpp f197da6
>
> Diff: https://reviews.apache.org/r/21594/diff/
>
>
> Testing
> -------
>
> make check on linux. more test cases are being written.
>
>
> Thanks,
>
> Chi Zhang
>
>
Re: Review Request 21594: Port-Range Based Network Isolator for Linux
Posted by Chi Zhang <ch...@gmail.com>.
> On June 24, 2014, 7:42 p.m., Vinod Kone wrote:
> > src/slave/containerizer/isolators/network/port_mapping.cpp, line 1945
> > <https://reviews.apache.org/r/21594/diff/7/?file=616163#file616163line1945>
> >
> > Instead of scripts can we use the new Subcommand abstraction here?
I think for ::prepare, mesos_containerizer collects all the preparation scripts and run them with a launch of mesoscontainerizelaunch, which is a subcommand.
- Chi
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21594/#review46568
-----------------------------------------------------------
On June 24, 2014, 10:03 p.m., Chi Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21594/
> -----------------------------------------------------------
>
> (Updated June 24, 2014, 10:03 p.m.)
>
>
> Review request for mesos, Ian Downes, Jie Yu, Vinod Kone, and Cong Wang.
>
>
> Bugs: https://issues.apache.org/jira/browse/MESOS-1324
> https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-1324
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Added a network isolator using port-range based traffic redirection on Linux.
>
> - Containers are assigned non-ephemeral ports by the scheduler and ephemeral ports by the network isolator.
> - Virtual ethernet devices and Traffic Control filters are set up so that network traffic in and out of the containers is isolated based on the ports assigned to them.
> - Containers run inside their own network namespaces with separate network stacks, from which per-container network statistics can be retrieved.
>
> A joint work with:
> - Cong Wang (cwang@twopensource.com)
> - Jie Yu (yujie.jay@gmail.com)
> - Ian Downes (ian.downes@gmail.com)
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 2f6be05
> src/Makefile.am a87eb21
> src/slave/constants.hpp c65a62d
> src/slave/constants.cpp 51f65bb
> src/slave/containerizer/isolators/network/helper.cpp PRE-CREATION
> src/slave/containerizer/isolators/network/port_mapping.hpp PRE-CREATION
> src/slave/containerizer/isolators/network/port_mapping.cpp PRE-CREATION
> src/slave/containerizer/linux_launcher.cpp 85c74f0
> src/slave/containerizer/mesos_containerizer.cpp 61c0a8d
> src/slave/flags.hpp 3b8ba08
> src/tests/environment.cpp 21b9d1d
> src/tests/isolator_tests.cpp 0bbec09
> src/tests/mesos.cpp 1037420
>
> Diff: https://reviews.apache.org/r/21594/diff/
>
>
> Testing
> -------
>
> make check on linux. more test cases are being written.
>
>
> Thanks,
>
> Chi Zhang
>
>
Re: Review Request 21594: Port-Range Based Network Isolator for Linux
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21594/#review46568
-----------------------------------------------------------
Ship it!
lgtm
include/mesos/mesos.proto
<https://reviews.apache.org/r/21594/#comment82017>
...and not exposed to master and frameworks.
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment82018>
Instead of scripts can we use the new Subcommand abstraction here?
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment82019>
make this a static member of EphemeralPortsAllocator.
src/slave/flags.hpp
<https://reviews.apache.org/r/21594/#comment82016>
s/per_container_epehemeral_prot_size/ephemeral_ports_per_container/
- Vinod Kone
On June 24, 2014, 5:01 p.m., Chi Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21594/
> -----------------------------------------------------------
>
> (Updated June 24, 2014, 5:01 p.m.)
>
>
> Review request for mesos, Ian Downes, Jie Yu, Vinod Kone, and Cong Wang.
>
>
> Bugs: https://issues.apache.org/jira/browse/MESOS-1324
> https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-1324
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Added a network isolator using port-range based traffic redirection on Linux.
>
> - Containers are assigned non-ephemeral ports by the scheduler and ephemeral ports by the network isolator.
> - Virtual ethernet devices and Traffic Control filters are set up so that network traffic in and out of the containers is isolated based on the ports assigned to them.
> - Containers run inside their own network namespaces with separate network stacks, from which per-container network statistics can be retrieved.
>
> A joint work with:
> - Cong Wang (cwang@twopensource.com)
> - Jie Yu (yujie.jay@gmail.com)
> - Ian Downes (ian.downes@gmail.com)
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 2f6be05
> src/Makefile.am a87eb21
> src/slave/constants.hpp c65a62d
> src/slave/constants.cpp 51f65bb
> src/slave/containerizer/isolators/network/helper.cpp PRE-CREATION
> src/slave/containerizer/isolators/network/port_mapping.hpp PRE-CREATION
> src/slave/containerizer/isolators/network/port_mapping.cpp PRE-CREATION
> src/slave/containerizer/linux_launcher.cpp 85c74f0
> src/slave/containerizer/mesos_containerizer.cpp 61c0a8d
> src/slave/flags.hpp 3b8ba08
> src/tests/environment.cpp 21b9d1d
> src/tests/isolator_tests.cpp 0bbec09
> src/tests/mesos.cpp 1037420
>
> Diff: https://reviews.apache.org/r/21594/diff/
>
>
> Testing
> -------
>
> make check on linux. more test cases are being written.
>
>
> Thanks,
>
> Chi Zhang
>
>
Re: Review Request 21594: Port-Range Based Network Isolator for Linux
Posted by Chi Zhang <ch...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21594/
-----------------------------------------------------------
(Updated June 25, 2014, 10:04 p.m.)
Review request for mesos, Ian Downes, Jie Yu, Vinod Kone, and Cong Wang.
Changes
-------
rebased master.
Bugs: https://issues.apache.org/jira/browse/MESOS-1324
https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-1324
Repository: mesos-git
Description
-------
Added a network isolator using port-range based traffic redirection on Linux.
- Containers are assigned non-ephemeral ports by the scheduler and ephemeral ports by the network isolator.
- Virtual ethernet devices and Traffic Control filters are set up so that network traffic in and out of the containers is isolated based on the ports assigned to them.
- Containers run inside their own network namespaces with separate network stacks, from which per-container network statistics can be retrieved.
A joint work with:
- Cong Wang (cwang@twopensource.com)
- Jie Yu (yujie.jay@gmail.com)
- Ian Downes (ian.downes@gmail.com)
Diffs (updated)
-----
include/mesos/mesos.proto c02b8ec
src/Makefile.am fb3af9d
src/slave/constants.hpp c65a62d
src/slave/constants.cpp 51f65bb
src/slave/containerizer/isolators/network/helper.cpp PRE-CREATION
src/slave/containerizer/isolators/network/port_mapping.hpp PRE-CREATION
src/slave/containerizer/isolators/network/port_mapping.cpp PRE-CREATION
src/slave/containerizer/linux_launcher.cpp 1ce03e3
src/slave/containerizer/mesos/containerizer.cpp 27f8e09
src/slave/flags.hpp 3b8ba08
src/slave/slave.cpp 0cb98cd
src/tests/environment.cpp e991d57
src/tests/isolator_tests.cpp 5a141e3
src/tests/mesos.cpp 216bd6f
Diff: https://reviews.apache.org/r/21594/diff/
Testing
-------
make check on linux. more test cases are being written.
Thanks,
Chi Zhang
Re: Review Request 21594: Port-Range Based Network Isolator for Linux
Posted by Chi Zhang <ch...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21594/
-----------------------------------------------------------
(Updated June 24, 2014, 10:05 p.m.)
Review request for mesos, Ian Downes, Jie Yu, Vinod Kone, and Cong Wang.
Changes
-------
added dependency
Bugs: https://issues.apache.org/jira/browse/MESOS-1324
https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-1324
Repository: mesos-git
Description
-------
Added a network isolator using port-range based traffic redirection on Linux.
- Containers are assigned non-ephemeral ports by the scheduler and ephemeral ports by the network isolator.
- Virtual ethernet devices and Traffic Control filters are set up so that network traffic in and out of the containers is isolated based on the ports assigned to them.
- Containers run inside their own network namespaces with separate network stacks, from which per-container network statistics can be retrieved.
A joint work with:
- Cong Wang (cwang@twopensource.com)
- Jie Yu (yujie.jay@gmail.com)
- Ian Downes (ian.downes@gmail.com)
Diffs
-----
include/mesos/mesos.proto 2f6be05
src/Makefile.am a87eb21
src/slave/constants.hpp c65a62d
src/slave/constants.cpp 51f65bb
src/slave/containerizer/isolators/network/helper.cpp PRE-CREATION
src/slave/containerizer/isolators/network/port_mapping.hpp PRE-CREATION
src/slave/containerizer/isolators/network/port_mapping.cpp PRE-CREATION
src/slave/containerizer/linux_launcher.cpp 85c74f0
src/slave/containerizer/mesos_containerizer.cpp 61c0a8d
src/slave/flags.hpp 3b8ba08
src/tests/environment.cpp 21b9d1d
src/tests/isolator_tests.cpp 0bbec09
src/tests/mesos.cpp 1037420
Diff: https://reviews.apache.org/r/21594/diff/
Testing
-------
make check on linux. more test cases are being written.
Thanks,
Chi Zhang
Re: Review Request 21594: Port-Range Based Network Isolator for Linux
Posted by Chi Zhang <ch...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21594/
-----------------------------------------------------------
(Updated June 24, 2014, 10:03 p.m.)
Review request for mesos, Ian Downes, Jie Yu, Vinod Kone, and Cong Wang.
Changes
-------
addressed comments.
Bugs: https://issues.apache.org/jira/browse/MESOS-1324
https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-1324
Repository: mesos-git
Description
-------
Added a network isolator using port-range based traffic redirection on Linux.
- Containers are assigned non-ephemeral ports by the scheduler and ephemeral ports by the network isolator.
- Virtual ethernet devices and Traffic Control filters are set up so that network traffic in and out of the containers is isolated based on the ports assigned to them.
- Containers run inside their own network namespaces with separate network stacks, from which per-container network statistics can be retrieved.
A joint work with:
- Cong Wang (cwang@twopensource.com)
- Jie Yu (yujie.jay@gmail.com)
- Ian Downes (ian.downes@gmail.com)
Diffs (updated)
-----
include/mesos/mesos.proto 2f6be05
src/Makefile.am a87eb21
src/slave/constants.hpp c65a62d
src/slave/constants.cpp 51f65bb
src/slave/containerizer/isolators/network/helper.cpp PRE-CREATION
src/slave/containerizer/isolators/network/port_mapping.hpp PRE-CREATION
src/slave/containerizer/isolators/network/port_mapping.cpp PRE-CREATION
src/slave/containerizer/linux_launcher.cpp 85c74f0
src/slave/containerizer/mesos_containerizer.cpp 61c0a8d
src/slave/flags.hpp 3b8ba08
src/tests/environment.cpp 21b9d1d
src/tests/isolator_tests.cpp 0bbec09
src/tests/mesos.cpp 1037420
Diff: https://reviews.apache.org/r/21594/diff/
Testing
-------
make check on linux. more test cases are being written.
Thanks,
Chi Zhang
Re: Review Request 21594: Port-Range Based Network Isolator for Linux
Posted by Chi Zhang <ch...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21594/
-----------------------------------------------------------
(Updated June 24, 2014, 5:01 p.m.)
Review request for mesos, Ian Downes, Jie Yu, Vinod Kone, and Cong Wang.
Changes
-------
re-added dependency.
Bugs: https://issues.apache.org/jira/browse/MESOS-1324
https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-1324
Repository: mesos-git
Description
-------
Added a network isolator using port-range based traffic redirection on Linux.
- Containers are assigned non-ephemeral ports by the scheduler and ephemeral ports by the network isolator.
- Virtual ethernet devices and Traffic Control filters are set up so that network traffic in and out of the containers is isolated based on the ports assigned to them.
- Containers run inside their own network namespaces with separate network stacks, from which per-container network statistics can be retrieved.
A joint work with:
- Cong Wang (cwang@twopensource.com)
- Jie Yu (yujie.jay@gmail.com)
- Ian Downes (ian.downes@gmail.com)
Diffs
-----
include/mesos/mesos.proto 2f6be05
src/Makefile.am a87eb21
src/slave/constants.hpp c65a62d
src/slave/constants.cpp 51f65bb
src/slave/containerizer/isolators/network/helper.cpp PRE-CREATION
src/slave/containerizer/isolators/network/port_mapping.hpp PRE-CREATION
src/slave/containerizer/isolators/network/port_mapping.cpp PRE-CREATION
src/slave/containerizer/linux_launcher.cpp 85c74f0
src/slave/containerizer/mesos_containerizer.cpp 61c0a8d
src/slave/flags.hpp 3b8ba08
src/tests/environment.cpp 21b9d1d
src/tests/isolator_tests.cpp 0bbec09
src/tests/mesos.cpp 1037420
Diff: https://reviews.apache.org/r/21594/diff/
Testing
-------
make check on linux. more test cases are being written.
Thanks,
Chi Zhang
Re: Review Request 21594: Port-Range Based Network Isolator for Linux
Posted by Chi Zhang <ch...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21594/
-----------------------------------------------------------
(Updated June 24, 2014, 5 p.m.)
Review request for mesos, Ian Downes, Jie Yu, Vinod Kone, and Cong Wang.
Changes
-------
fixed diff.
Bugs: https://issues.apache.org/jira/browse/MESOS-1324
https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-1324
Repository: mesos-git
Description
-------
Added a network isolator using port-range based traffic redirection on Linux.
- Containers are assigned non-ephemeral ports by the scheduler and ephemeral ports by the network isolator.
- Virtual ethernet devices and Traffic Control filters are set up so that network traffic in and out of the containers is isolated based on the ports assigned to them.
- Containers run inside their own network namespaces with separate network stacks, from which per-container network statistics can be retrieved.
A joint work with:
- Cong Wang (cwang@twopensource.com)
- Jie Yu (yujie.jay@gmail.com)
- Ian Downes (ian.downes@gmail.com)
Diffs (updated)
-----
include/mesos/mesos.proto 2f6be05
src/Makefile.am a87eb21
src/slave/constants.hpp c65a62d
src/slave/constants.cpp 51f65bb
src/slave/containerizer/isolators/network/helper.cpp PRE-CREATION
src/slave/containerizer/isolators/network/port_mapping.hpp PRE-CREATION
src/slave/containerizer/isolators/network/port_mapping.cpp PRE-CREATION
src/slave/containerizer/linux_launcher.cpp 85c74f0
src/slave/containerizer/mesos_containerizer.cpp 61c0a8d
src/slave/flags.hpp 3b8ba08
src/tests/environment.cpp 21b9d1d
src/tests/isolator_tests.cpp 0bbec09
src/tests/mesos.cpp 1037420
Diff: https://reviews.apache.org/r/21594/diff/
Testing
-------
make check on linux. more test cases are being written.
Thanks,
Chi Zhang
Re: Review Request 21594: Port-Range Based Network Isolator for Linux
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21594/#review46542
-----------------------------------------------------------
Bad diff?
- Jie Yu
On June 24, 2014, 7:28 a.m., Chi Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21594/
> -----------------------------------------------------------
>
> (Updated June 24, 2014, 7:28 a.m.)
>
>
> Review request for mesos, Ian Downes, Jie Yu, Vinod Kone, and Cong Wang.
>
>
> Bugs: https://issues.apache.org/jira/browse/MESOS-1324
> https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-1324
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Added a network isolator using port-range based traffic redirection on Linux.
>
> - Containers are assigned non-ephemeral ports by the scheduler and ephemeral ports by the network isolator.
> - Virtual ethernet devices and Traffic Control filters are set up so that network traffic in and out of the containers is isolated based on the ports assigned to them.
> - Containers run inside their own network namespaces with separate network stacks, from which per-container network statistics can be retrieved.
>
> A joint work with:
> - Cong Wang (cwang@twopensource.com)
> - Jie Yu (yujie.jay@gmail.com)
> - Ian Downes (ian.downes@gmail.com)
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 2f6be05
> src/Makefile.am 5e5ccd5
> src/slave/constants.hpp c65a62d
> src/slave/constants.cpp 51f65bb
> src/slave/containerizer/isolators/network/helper.cpp PRE-CREATION
> src/slave/containerizer/isolators/network/port_mapping.hpp PRE-CREATION
> src/slave/containerizer/isolators/network/port_mapping.cpp PRE-CREATION
> src/slave/containerizer/linux_launcher.cpp acaf9b5
> src/slave/containerizer/mesos_containerizer.cpp 917eebf
> src/slave/flags.hpp 3b8ba08
> src/tests/environment.cpp e991d57
> src/tests/isolator_tests.cpp 5a141e3
> src/tests/mesos.cpp 1037420
>
> Diff: https://reviews.apache.org/r/21594/diff/
>
>
> Testing
> -------
>
> make check on linux. more test cases are being written.
>
>
> Thanks,
>
> Chi Zhang
>
>
Re: Review Request 21594: Port-Range Based Network Isolator for Linux
Posted by Chi Zhang <ch...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21594/
-----------------------------------------------------------
(Updated June 24, 2014, 7:28 a.m.)
Review request for mesos, Ian Downes, Jie Yu, Vinod Kone, and Cong Wang.
Changes
-------
depends-on: 22852
Bugs: https://issues.apache.org/jira/browse/MESOS-1324
https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-1324
Repository: mesos-git
Description
-------
Added a network isolator using port-range based traffic redirection on Linux.
- Containers are assigned non-ephemeral ports by the scheduler and ephemeral ports by the network isolator.
- Virtual ethernet devices and Traffic Control filters are set up so that network traffic in and out of the containers is isolated based on the ports assigned to them.
- Containers run inside their own network namespaces with separate network stacks, from which per-container network statistics can be retrieved.
A joint work with:
- Cong Wang (cwang@twopensource.com)
- Jie Yu (yujie.jay@gmail.com)
- Ian Downes (ian.downes@gmail.com)
Diffs
-----
include/mesos/mesos.proto 2f6be05
src/Makefile.am 5e5ccd5
src/slave/constants.hpp c65a62d
src/slave/constants.cpp 51f65bb
src/slave/containerizer/isolators/network/helper.cpp PRE-CREATION
src/slave/containerizer/isolators/network/port_mapping.hpp PRE-CREATION
src/slave/containerizer/isolators/network/port_mapping.cpp PRE-CREATION
src/slave/containerizer/linux_launcher.cpp acaf9b5
src/slave/containerizer/mesos_containerizer.cpp 917eebf
src/slave/flags.hpp 3b8ba08
src/tests/environment.cpp e991d57
src/tests/isolator_tests.cpp 5a141e3
src/tests/mesos.cpp 1037420
Diff: https://reviews.apache.org/r/21594/diff/
Testing
-------
make check on linux. more test cases are being written.
Thanks,
Chi Zhang
Re: Review Request 21594: Port-Range Based Network Isolator for Linux
Posted by Chi Zhang <ch...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21594/
-----------------------------------------------------------
(Updated June 24, 2014, 7:26 a.m.)
Review request for mesos, Ian Downes, Jie Yu, Vinod Kone, and Cong Wang.
Changes
-------
address comments. this version was mostly done by Jie.
Bugs: https://issues.apache.org/jira/browse/MESOS-1324
https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-1324
Repository: mesos-git
Description
-------
Added a network isolator using port-range based traffic redirection on Linux.
- Containers are assigned non-ephemeral ports by the scheduler and ephemeral ports by the network isolator.
- Virtual ethernet devices and Traffic Control filters are set up so that network traffic in and out of the containers is isolated based on the ports assigned to them.
- Containers run inside their own network namespaces with separate network stacks, from which per-container network statistics can be retrieved.
A joint work with:
- Cong Wang (cwang@twopensource.com)
- Jie Yu (yujie.jay@gmail.com)
- Ian Downes (ian.downes@gmail.com)
Diffs (updated)
-----
include/mesos/mesos.proto 2f6be05
src/Makefile.am 5e5ccd5
src/slave/constants.hpp c65a62d
src/slave/constants.cpp 51f65bb
src/slave/containerizer/isolators/network/helper.cpp PRE-CREATION
src/slave/containerizer/isolators/network/port_mapping.hpp PRE-CREATION
src/slave/containerizer/isolators/network/port_mapping.cpp PRE-CREATION
src/slave/containerizer/linux_launcher.cpp acaf9b5
src/slave/containerizer/mesos_containerizer.cpp 917eebf
src/slave/flags.hpp 3b8ba08
src/tests/environment.cpp e991d57
src/tests/isolator_tests.cpp 5a141e3
src/tests/mesos.cpp 1037420
Diff: https://reviews.apache.org/r/21594/diff/
Testing
-------
make check on linux. more test cases are being written.
Thanks,
Chi Zhang
Re: Review Request 21594: Port-Range Based Network Isolator for Linux
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21594/#review46404
-----------------------------------------------------------
Ship it!
This is looking very good! A few style nits to address. Everything else LGTM.
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81783>
Style nits:
return Error(
"Unexpected format from host ip_local_port_range: " + value.get());
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81785>
Ditto.
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81786>
Ditto.
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81787>
Style nits. Would you please move this line down?
return Error(
"Network Isolator is given ephemeral ports of size: " +
...);
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81788>
Do not capitalize
"of executor "
src/slave/containerizer/isolators/network/port_mapping.hpp
<https://reviews.apache.org/r/21594/#comment81764>
You don't need this as the default constructor will be used.
src/slave/containerizer/isolators/network/port_mapping.hpp
<https://reviews.apache.org/r/21594/#comment81765>
Insert a blank line here.
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81767>
We can kill the local variable here by doing:
if (os::namespaces().count("net") == 0) {
...
}
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81766>
Style nits:
return Error(
"Using network isolator requires network namespace. "
"Make sure your kernel is newer than 3.4");
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81769>
I would suggest revert the variable name here. The name 'shell' is not as explicit as 'checkCommandTc'.
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81768>
Stick to style guide. We put '+' in the end:
return Error(
"Check command 'tc' failed: non-zero exit code:" +
shell.get());
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81770>
Please add a comment to explain what this piece of code is doing:
// Obtain the host ephemeral port range by reading 'ip_local_port_range' in /proc.
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81771>
Stick to style guide. We put '!=' in the end:
if (roundDownToPowerOfTwo(flags.per_container_ephemeral_port_size) !=
flags.per_container_ephemeral_port_size) {
...
}
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81773>
Stick to style guide:
return Error(
"Network Isolator failed to find a public interface: " +
eth0.error());
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81774>
Ditto.
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81775>
Ditto.
return Error(
"Failed to get the public IP of " + eth0.get() + ": " +
(hostIP.isError() ? hostIP.error() : "does not have an IPv4 address"));
Also, remove the tailing spaces (you can run a style checker before submitting the patch).
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81776>
ditto.
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81777>
Style issue. Move '+' to the end.
return Failure(
"Failed to create an ICMP packet filter from host " + eth0 +
" to " + veth(pid) + ": " + icmpEth0ToVeth.error());
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81778>
Style issue:
return Failure(
"The ICMP packet filter on host " + eth0 + " already exists");
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81779>
Ditto.
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81780>
Ditto.
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81781>
Ditto.
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81782>
Ditto.
- Jie Yu
On June 20, 2014, 12:13 a.m., Chi Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21594/
> -----------------------------------------------------------
>
> (Updated June 20, 2014, 12:13 a.m.)
>
>
> Review request for mesos, Ian Downes, Jie Yu, Vinod Kone, and Cong Wang.
>
>
> Bugs: https://issues.apache.org/jira/browse/MESOS-1324
> https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-1324
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Added a network isolator using port-range based traffic redirection on Linux.
>
> - Containers are assigned non-ephemeral ports by the scheduler and ephemeral ports by the network isolator.
> - Virtual ethernet devices and Traffic Control filters are set up so that network traffic in and out of the containers is isolated based on the ports assigned to them.
> - Containers run inside their own network namespaces with separate network stacks, from which per-container network statistics can be retrieved.
>
> A joint work with:
> - Cong Wang (cwang@twopensource.com)
> - Jie Yu (yujie.jay@gmail.com)
> - Ian Downes (ian.downes@gmail.com)
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 2f6be05
> src/Makefile.am b1b7d2d
> src/launcher/main.cpp b497e98
> src/slave/constants.hpp c65a62d
> src/slave/constants.cpp 51f65bb
> src/slave/containerizer/isolators/network/port_mapping.hpp PRE-CREATION
> src/slave/containerizer/isolators/network/port_mapping.cpp PRE-CREATION
> src/slave/containerizer/linux_launcher.cpp 85c74f0
> src/slave/containerizer/mesos_containerizer.cpp 61c0a8d
> src/slave/flags.hpp 3b8ba08
> src/slave/main.cpp 8c2b70c
> src/tests/environment.cpp 21b9d1d
> src/tests/mesos.cpp 1037420
>
> Diff: https://reviews.apache.org/r/21594/diff/
>
>
> Testing
> -------
>
> make check on linux. more test cases are being written.
>
>
> Thanks,
>
> Chi Zhang
>
>
Re: Review Request 21594: Port-Range Based Network Isolator for Linux
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21594/#review46415
-----------------------------------------------------------
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment81800>
kill white space.
are you using our git pre commit hook? see the style guide on how to enable it.
- Vinod Kone
On June 20, 2014, 12:13 a.m., Chi Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21594/
> -----------------------------------------------------------
>
> (Updated June 20, 2014, 12:13 a.m.)
>
>
> Review request for mesos, Ian Downes, Jie Yu, Vinod Kone, and Cong Wang.
>
>
> Bugs: https://issues.apache.org/jira/browse/MESOS-1324
> https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-1324
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Added a network isolator using port-range based traffic redirection on Linux.
>
> - Containers are assigned non-ephemeral ports by the scheduler and ephemeral ports by the network isolator.
> - Virtual ethernet devices and Traffic Control filters are set up so that network traffic in and out of the containers is isolated based on the ports assigned to them.
> - Containers run inside their own network namespaces with separate network stacks, from which per-container network statistics can be retrieved.
>
> A joint work with:
> - Cong Wang (cwang@twopensource.com)
> - Jie Yu (yujie.jay@gmail.com)
> - Ian Downes (ian.downes@gmail.com)
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 2f6be05
> src/Makefile.am b1b7d2d
> src/launcher/main.cpp b497e98
> src/slave/constants.hpp c65a62d
> src/slave/constants.cpp 51f65bb
> src/slave/containerizer/isolators/network/port_mapping.hpp PRE-CREATION
> src/slave/containerizer/isolators/network/port_mapping.cpp PRE-CREATION
> src/slave/containerizer/linux_launcher.cpp 85c74f0
> src/slave/containerizer/mesos_containerizer.cpp 61c0a8d
> src/slave/flags.hpp 3b8ba08
> src/slave/main.cpp 8c2b70c
> src/tests/environment.cpp 21b9d1d
> src/tests/mesos.cpp 1037420
>
> Diff: https://reviews.apache.org/r/21594/diff/
>
>
> Testing
> -------
>
> make check on linux. more test cases are being written.
>
>
> Thanks,
>
> Chi Zhang
>
>
Re: Review Request 21594: Port-Range Based Network Isolator for Linux
Posted by Chi Zhang <ch...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21594/
-----------------------------------------------------------
(Updated June 20, 2014, 12:13 a.m.)
Review request for mesos, Ian Downes, Jie Yu, Vinod Kone, and Cong Wang.
Bugs: https://issues.apache.org/jira/browse/MESOS-1324
https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-1324
Repository: mesos-git
Description
-------
Added a network isolator using port-range based traffic redirection on Linux.
- Containers are assigned non-ephemeral ports by the scheduler and ephemeral ports by the network isolator.
- Virtual ethernet devices and Traffic Control filters are set up so that network traffic in and out of the containers is isolated based on the ports assigned to them.
- Containers run inside their own network namespaces with separate network stacks, from which per-container network statistics can be retrieved.
A joint work with:
- Cong Wang (cwang@twopensource.com)
- Jie Yu (yujie.jay@gmail.com)
- Ian Downes (ian.downes@gmail.com)
Diffs
-----
include/mesos/mesos.proto 2f6be05
src/Makefile.am b1b7d2d
src/launcher/main.cpp b497e98
src/slave/constants.hpp c65a62d
src/slave/constants.cpp 51f65bb
src/slave/containerizer/isolators/network/port_mapping.hpp PRE-CREATION
src/slave/containerizer/isolators/network/port_mapping.cpp PRE-CREATION
src/slave/containerizer/linux_launcher.cpp 85c74f0
src/slave/containerizer/mesos_containerizer.cpp 61c0a8d
src/slave/flags.hpp 3b8ba08
src/slave/main.cpp 8c2b70c
src/tests/environment.cpp 21b9d1d
src/tests/mesos.cpp 1037420
Diff: https://reviews.apache.org/r/21594/diff/
Testing
-------
make check on linux. more test cases are being written.
Thanks,
Chi Zhang
Re: Review Request 21594: Port-Range Based Network Isolator for Linux
Posted by Chi Zhang <ch...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21594/
-----------------------------------------------------------
(Updated June 20, 2014, 12:13 a.m.)
Review request for mesos, Ian Downes, Jie Yu, Vinod Kone, and Cong Wang.
Changes
-------
addressed comments.
Bugs: https://issues.apache.org/jira/browse/MESOS-1324
https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-1324
Repository: mesos-git
Description
-------
Added a network isolator using port-range based traffic redirection on Linux.
- Containers are assigned non-ephemeral ports by the scheduler and ephemeral ports by the network isolator.
- Virtual ethernet devices and Traffic Control filters are set up so that network traffic in and out of the containers is isolated based on the ports assigned to them.
- Containers run inside their own network namespaces with separate network stacks, from which per-container network statistics can be retrieved.
A joint work with:
- Cong Wang (cwang@twopensource.com)
- Jie Yu (yujie.jay@gmail.com)
- Ian Downes (ian.downes@gmail.com)
Diffs (updated)
-----
include/mesos/mesos.proto 2f6be05
src/Makefile.am b1b7d2d
src/launcher/main.cpp b497e98
src/slave/constants.hpp c65a62d
src/slave/constants.cpp 51f65bb
src/slave/containerizer/isolators/network/port_mapping.hpp PRE-CREATION
src/slave/containerizer/isolators/network/port_mapping.cpp PRE-CREATION
src/slave/containerizer/linux_launcher.cpp 85c74f0
src/slave/containerizer/mesos_containerizer.cpp 61c0a8d
src/slave/flags.hpp 3b8ba08
src/slave/main.cpp 8c2b70c
src/tests/environment.cpp 21b9d1d
src/tests/mesos.cpp 1037420
Diff: https://reviews.apache.org/r/21594/diff/
Testing
-------
make check on linux. more test cases are being written.
Thanks,
Chi Zhang
Re: Review Request 21594: Port-Range Based Network Isolator for Linux
Posted by Chi Zhang <ch...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21594/
-----------------------------------------------------------
(Updated June 18, 2014, 1:16 a.m.)
Review request for mesos, Ian Downes, Jie Yu, Vinod Kone, and Cong Wang.
Changes
-------
addressed comments.
Bugs: https://issues.apache.org/jira/browse/MESOS-1324
https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-1324
Repository: mesos-git
Description
-------
Added a network isolator using port-range based traffic redirection on Linux.
- Containers are assigned non-ephemeral ports by the scheduler and ephemeral ports by the network isolator.
- Virtual ethernet devices and Traffic Control filters are set up so that network traffic in and out of the containers is isolated based on the ports assigned to them.
- Containers run inside their own network namespaces with separate network stacks, from which per-container network statistics can be retrieved.
A joint work with:
- Cong Wang (cwang@twopensource.com)
- Jie Yu (yujie.jay@gmail.com)
- Ian Downes (ian.downes@gmail.com)
Diffs (updated)
-----
include/mesos/mesos.proto 2f6be05
src/Makefile.am b1b7d2d
src/launcher/main.cpp b497e98
src/slave/constants.hpp c65a62d
src/slave/constants.cpp 51f65bb
src/slave/containerizer/isolators/network/port_mapping.hpp PRE-CREATION
src/slave/containerizer/isolators/network/port_mapping.cpp PRE-CREATION
src/slave/containerizer/linux_launcher.cpp 85c74f0
src/slave/containerizer/mesos_containerizer.cpp 4d97d49
src/slave/flags.hpp 3b8ba08
src/slave/main.cpp 8c2b70c
src/tests/environment.cpp 005fc54
src/tests/mesos.cpp f197da6
Diff: https://reviews.apache.org/r/21594/diff/
Testing
-------
make check on linux. more test cases are being written.
Thanks,
Chi Zhang
Re: Review Request 21594: Port-Range Based Network Isolator for Linux
Posted by Jie Yu <yu...@gmail.com>.
> On June 13, 2014, 6:05 p.m., Ian Downes wrote:
> > src/slave/containerizer/isolators/network/port_mapping.hpp, line 79
> > <https://reviews.apache.org/r/21594/diff/2/?file=607517#file607517line79>
> >
> > Why not statically allocate the ranges at construction and then you allocate a range which is unused, tagging it as such? This seems vastly simpler than calculating ranges at each request!
We thought about static allocation too from the very beginning. The main problem with static allocation is that it cannot handle ephemeral-ports-per-container size change easily. For example, when slave restarts, it uses a different 'max_containers' flag (which leads to different ephemeral-ports-per-container size). When recovering existing running containers, how the static allocator deals with mixed container sizes?
> On June 13, 2014, 6:05 p.m., Ian Downes wrote:
> > src/slave/containerizer/isolators/network/port_mapping.hpp, line 66
> > <https://reviews.apache.org/r/21594/diff/2/?file=607517#file607517line66>
> >
> > Why is there allocate(), and use(), and used in the constructor? Can this not be simplified?
> >
> > Shouldn't the interface simply be to get a range and then to release it when finished?
> >
> > Is use() necessary for recovery?
Yes, use() is necessary for recovery.
> On June 13, 2014, 6:05 p.m., Ian Downes wrote:
> > src/slave/containerizer/isolators/network/port_mapping.hpp, line 54
> > <https://reviews.apache.org/r/21594/diff/2/?file=607517#file607517line54>
> >
> > What about constructing with the total range to use and then some separate recovery which specifies the ranges already in use.
It's tricky because allocator is created during "create" before 'recover' is done.
> On June 13, 2014, 6:05 p.m., Ian Downes wrote:
> > src/slave/containerizer/isolators/network/port_mapping.cpp, line 918
> > <https://reviews.apache.org/r/21594/diff/2/?file=607518#file607518line918>
> >
> > Does this require recent kernel patches?
Yes, it does. If it fails, the slave will exit cleanly (like a check).
> On June 13, 2014, 6:05 p.m., Ian Downes wrote:
> > src/slave/containerizer/isolators/network/port_mapping.cpp, line 954
> > <https://reviews.apache.org/r/21594/diff/2/?file=607518#file607518line954>
> >
> > call variable createQdisc and reuse?
I would prefer a more explicit variable name than reusing the variable. What's the benefit of reusing the variable? I don't think register pressure is a problem:)
> On June 13, 2014, 6:05 p.m., Ian Downes wrote:
> > src/slave/containerizer/isolators/network/port_mapping.cpp, line 1394
> > <https://reviews.apache.org/r/21594/diff/2/?file=607518#file607518line1394>
> >
> > name create and reuse?
Again, I prefer more explicit variable names:)
> On June 13, 2014, 6:05 p.m., Ian Downes wrote:
> > src/slave/containerizer/isolators/network/port_mapping.cpp, line 1722
> > <https://reviews.apache.org/r/21594/diff/2/?file=607518#file607518line1722>
> >
> > why use the temporary?
> >
> > if (stat.get().contains("rx_packets")) {
> > result.set_net_rx_packets(stat.get()["rx_packets"]);
> > }
stat.get()["rx_packets"]
This won't compile as '[]' is not a const method.
> On June 13, 2014, 6:05 p.m., Ian Downes wrote:
> > src/slave/containerizer/isolators/network/port_mapping.cpp, line 1854
> > <https://reviews.apache.org/r/21594/diff/2/?file=607518#file607518line1854>
> >
> > Rename to remove and reuse for icmp and arp?
Ditto.
> On June 13, 2014, 6:05 p.m., Ian Downes wrote:
> > src/slave/containerizer/isolators/network/port_mapping.cpp, line 1887
> > <https://reviews.apache.org/r/21594/diff/2/?file=607518#file607518line1887>
> >
> > rename to update and use for icmp and arp?
Ditto.
> On June 13, 2014, 6:05 p.m., Ian Downes wrote:
> > src/slave/containerizer/isolators/network/port_mapping.cpp, line 1953
> > <https://reviews.apache.org/r/21594/diff/2/?file=607518#file607518line1953>
> >
> > Again, why not just use XXX.address() - it seems much, much cleaner than introducing XXXNoPrefix locals?
The routing lib interfaces expect an net::IP. We have a reason because we want to support subnet in the routing lib in the near future.
> On June 13, 2014, 6:05 p.m., Ian Downes wrote:
> > src/slave/flags.hpp, line 235
> > <https://reviews.apache.org/r/21594/diff/2/?file=607521#file607521line235>
> >
> > It doesn't feel correct to specify the maximum number of containers here. What about specifying the size of the ephemeral range, i.e., number of ephemeral ports per container?
The reason for 'max_container' is that it is independent of the ephemeral ports specified by the slave. For example, if we use the size of the ephemeral port range for each container (say 1024), what if the slave says that the ephemeral ports available is less than 1024?
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21594/#review45513
-----------------------------------------------------------
On June 12, 2014, 6:45 p.m., Chi Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21594/
> -----------------------------------------------------------
>
> (Updated June 12, 2014, 6:45 p.m.)
>
>
> Review request for mesos, Ian Downes, Jie Yu, Vinod Kone, and Cong Wang.
>
>
> Bugs: https://issues.apache.org/jira/browse/MESOS-1324
> https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-1324
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Added a network isolator using port-range based traffic redirection on Linux.
>
> - Containers are assigned non-ephemeral ports by the scheduler and ephemeral ports by the network isolator.
> - Virtual ethernet devices and Traffic Control filters are set up so that network traffic in and out of the containers is isolated based on the ports assigned to them.
> - Containers run inside their own network namespaces with separate network stacks, from which per-container network statistics can be retrieved.
>
> A joint work with:
> - Cong Wang (cwang@twopensource.com)
> - Jie Yu (yujie.jay@gmail.com)
> - Ian Downes (ian.downes@gmail.com)
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 102289b
> src/Makefile.am c91b438
> src/launcher/main.cpp b497e98
> src/slave/constants.hpp ace4590
> src/slave/constants.cpp 51f65bb
> src/slave/containerizer/isolators/network/port_mapping.hpp PRE-CREATION
> src/slave/containerizer/isolators/network/port_mapping.cpp PRE-CREATION
> src/slave/containerizer/linux_launcher.cpp c17724b
> src/slave/containerizer/mesos_containerizer.cpp a38564f
> src/slave/flags.hpp 15e5b64
> src/slave/main.cpp 8c2b70c
> src/tests/environment.cpp 005fc54
> src/tests/mesos.cpp e6d807c
>
> Diff: https://reviews.apache.org/r/21594/diff/
>
>
> Testing
> -------
>
> make check on linux. more test cases are being written.
>
>
> Thanks,
>
> Chi Zhang
>
>
Re: Review Request 21594: Port-Range Based Network Isolator for Linux
Posted by Chi Zhang <ch...@gmail.com>.
> On June 13, 2014, 6:05 p.m., Ian Downes wrote:
> > src/slave/containerizer/isolators/network/port_mapping.cpp, line 918
> > <https://reviews.apache.org/r/21594/diff/2/?file=607518#file607518line918>
> >
> > Does this require recent kernel patches?
>
> Jie Yu wrote:
> Yes, it does. If it fails, the slave will exit cleanly (like a check).
added a comment to mention the kernel patch.
> On June 13, 2014, 6:05 p.m., Ian Downes wrote:
> > src/slave/containerizer/isolators/network/port_mapping.cpp, line 1073
> > <https://reviews.apache.org/r/21594/diff/2/?file=607518#file607518line1073>
> >
> > Add comment about what it's ignoring here? ethX, lo and other veths?
commented.
> On June 13, 2014, 6:05 p.m., Ian Downes wrote:
> > src/slave/containerizer/isolators/network/port_mapping.cpp, line 1804
> > <https://reviews.apache.org/r/21594/diff/2/?file=607518#file607518line1804>
> >
> > But don't the filters get set up before any pid is isolated!?
when prepare fails for reasons like no more ephemeral ports available, cleanup is called by containerizer, at which point pid isn't isolated yet.
> On June 13, 2014, 6:05 p.m., Ian Downes wrote:
> > src/slave/containerizer/isolators/network/port_mapping.cpp, line 1932
> > <https://reviews.apache.org/r/21594/diff/2/?file=607518#file607518line1932>
> >
> > The contract is that the launcher ensures all processes are killed before calling cleanup on the isolators. If there are processes remaining it's a bug in the launcher.
I don't think the comment reflects the reason we added this.
we saw a pathological case in testing where the veth doesn't get cleaned up quickly by the kernel (a ~10m delay) even when all the processes of a container have been killed. This is a simple work around to just kill it instead of waiting for the kernel.
> On June 13, 2014, 6:05 p.m., Ian Downes wrote:
> > src/slave/containerizer/isolators/network/port_mapping.cpp, line 756
> > <https://reviews.apache.org/r/21594/diff/2/?file=607518#file607518line756>
> >
> > This does more than check for resources.get().ports.isSome() - it validates the range doesn't it?
could you elaborate on this?
> On June 13, 2014, 6:05 p.m., Ian Downes wrote:
> > src/slave/containerizer/isolators/network/port_mapping.cpp, line 1803
> > <https://reviews.apache.org/r/21594/diff/2/?file=607518#file607518line1803>
> >
> > Include containerId in log message.
This code path is used to clean up both normal containers and orphaned containers. We don't have a containerId for a recovered orphaned container.
> On June 13, 2014, 6:05 p.m., Ian Downes wrote:
> > src/slave/containerizer/isolators/network/port_mapping.cpp, line 887
> > <https://reviews.apache.org/r/21594/diff/2/?file=607518#file607518line887>
> >
> > Can this be pulled out into stout and tested?
I think it's a good idea and could be useful for tests, but here we need the name of the eth0 anyway.
> On June 13, 2014, 6:05 p.m., Ian Downes wrote:
> > src/slave/containerizer/isolators/network/port_mapping.cpp, line 1311
> > <https://reviews.apache.org/r/21594/diff/2/?file=607518#file607518line1311>
> >
> > Hasn't this been created during initialization of the isolator?
removed. Jie, I don't think this is particularly useful. Do you try to detect if someone has removed the directory after 'create'?
> On June 13, 2014, 6:05 p.m., Ian Downes wrote:
> > src/slave/containerizer/isolators/network/port_mapping.hpp, line 66
> > <https://reviews.apache.org/r/21594/diff/2/?file=607517#file607517line66>
> >
> > Why is there allocate(), and use(), and used in the constructor? Can this not be simplified?
> >
> > Shouldn't the interface simply be to get a range and then to release it when finished?
> >
> > Is use() necessary for recovery?
>
> Jie Yu wrote:
> Yes, use() is necessary for recovery.
changed use to allocate as well, so we have an overloaded allocate now.
changed free to deallocate.
adjusted comments.
> On June 13, 2014, 6:05 p.m., Ian Downes wrote:
> > src/slave/containerizer/isolators/network/port_mapping.hpp, line 54
> > <https://reviews.apache.org/r/21594/diff/2/?file=607517#file607517line54>
> >
> > What about constructing with the total range to use and then some separate recovery which specifies the ranges already in use.
>
> Jie Yu wrote:
> It's tricky because allocator is created during "create" before 'recover' is done.
I like the idea of having a constructor with only the 'total' resources for the allocator to manage. having a free and a used is less intuitive. this constructor is called only once in create and 'used' is useless there. having only a 'total' can also simplify test cases.
Jie is right about rocover. Let's use the overloaded allocate.
> On June 13, 2014, 6:05 p.m., Ian Downes wrote:
> > src/slave/containerizer/isolators/network/port_mapping.cpp, line 1991
> > <https://reviews.apache.org/r/21594/diff/2/?file=607518#file607518line1991>
> >
> > This is checking the host proc for existence - wouldn't it be better to check the container's proc instead?
> > if [ -e /proc/sys/... ]; then ...; fi
at this point, /proc has been remounted, so we are checking container's proc.
> On June 13, 2014, 6:05 p.m., Ian Downes wrote:
> > src/tests/mesos.cpp, line 347
> > <https://reviews.apache.org/r/21594/diff/2/?file=607524#file607524line347>
> >
> > This will include the network isolator in all tests if it was compiled in? Perhaps we should start with selectively enabling it in tests?
this only affects ContainerizeTest, which means only SlaveRecoveryTest.
> On June 13, 2014, 6:05 p.m., Ian Downes wrote:
> > src/slave/containerizer/isolators/network/port_mapping.cpp, line 1850
> > <https://reviews.apache.org/r/21594/diff/2/?file=607518#file607518line1850>
> >
> > Is there a test to ensure these get added back correctly when a container is launched? i.e., start a container, stop container, start container.
yeah, I'll add a test to check 'clean up' is indeed finished in the system after containers exit.
> On June 13, 2014, 6:05 p.m., Ian Downes wrote:
> > src/slave/containerizer/isolators/network/port_mapping.cpp, line 1918
> > <https://reviews.apache.org/r/21594/diff/2/?file=607518#file607518line1918>
> >
> > Do we verify that happens? Do we wait until it's completed before returning from cleanup?
I fixed the comments since now we delete veth in cleanup. all the fiters on it go away when veth is deleted.
> On June 13, 2014, 6:05 p.m., Ian Downes wrote:
> > src/slave/containerizer/isolators/network/port_mapping.cpp, line 1323
> > <https://reviews.apache.org/r/21594/diff/2/?file=607518#file607518line1323>
> >
> > What about a bind mount abstraction that took source and target and did the validation, touch, etc.?
not much gain from this, since there is only two calls and there is only one place for them.
more, 'touch' does not have a recursive 'touch' semantic.
- Chi
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21594/#review45513
-----------------------------------------------------------
On June 18, 2014, 1:16 a.m., Chi Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21594/
> -----------------------------------------------------------
>
> (Updated June 18, 2014, 1:16 a.m.)
>
>
> Review request for mesos, Ian Downes, Jie Yu, Vinod Kone, and Cong Wang.
>
>
> Bugs: https://issues.apache.org/jira/browse/MESOS-1324
> https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-1324
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Added a network isolator using port-range based traffic redirection on Linux.
>
> - Containers are assigned non-ephemeral ports by the scheduler and ephemeral ports by the network isolator.
> - Virtual ethernet devices and Traffic Control filters are set up so that network traffic in and out of the containers is isolated based on the ports assigned to them.
> - Containers run inside their own network namespaces with separate network stacks, from which per-container network statistics can be retrieved.
>
> A joint work with:
> - Cong Wang (cwang@twopensource.com)
> - Jie Yu (yujie.jay@gmail.com)
> - Ian Downes (ian.downes@gmail.com)
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 2f6be05
> src/Makefile.am b1b7d2d
> src/launcher/main.cpp b497e98
> src/slave/constants.hpp c65a62d
> src/slave/constants.cpp 51f65bb
> src/slave/containerizer/isolators/network/port_mapping.hpp PRE-CREATION
> src/slave/containerizer/isolators/network/port_mapping.cpp PRE-CREATION
> src/slave/containerizer/linux_launcher.cpp 85c74f0
> src/slave/containerizer/mesos_containerizer.cpp 4d97d49
> src/slave/flags.hpp 3b8ba08
> src/slave/main.cpp 8c2b70c
> src/tests/environment.cpp 005fc54
> src/tests/mesos.cpp f197da6
>
> Diff: https://reviews.apache.org/r/21594/diff/
>
>
> Testing
> -------
>
> make check on linux. more test cases are being written.
>
>
> Thanks,
>
> Chi Zhang
>
>
Re: Review Request 21594: Port-Range Based Network Isolator for Linux
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21594/#review45513
-----------------------------------------------------------
src/slave/containerizer/isolators/network/port_mapping.hpp
<https://reviews.apache.org/r/21594/#comment80445>
What about constructing with the total range to use and then some separate recovery which specifies the ranges already in use.
src/slave/containerizer/isolators/network/port_mapping.hpp
<https://reviews.apache.org/r/21594/#comment80403>
s/running out of free/exhausting available/
src/slave/containerizer/isolators/network/port_mapping.hpp
<https://reviews.apache.org/r/21594/#comment80442>
Why is there allocate(), and use(), and used in the constructor? Can this not be simplified?
Shouldn't the interface simply be to get a range and then to release it when finished?
Is use() necessary for recovery?
src/slave/containerizer/isolators/network/port_mapping.hpp
<https://reviews.apache.org/r/21594/#comment80405>
More precisely, doesn't it return true if *any* port in ports is in-use?
src/slave/containerizer/isolators/network/port_mapping.hpp
<https://reviews.apache.org/r/21594/#comment80443>
Why not statically allocate the ranges at construction and then you allocate a range which is unused, tagging it as such? This seems vastly simpler than calculating ranges at each request!
src/slave/containerizer/isolators/network/port_mapping.hpp
<https://reviews.apache.org/r/21594/#comment80406>
s/wanna/want to/
src/slave/containerizer/isolators/network/port_mapping.hpp
<https://reviews.apache.org/r/21594/#comment80407>
s/is not enough/are insufficient/
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80408>
s/minimal/minimum/
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80452>
Private IP would normally mean the IP on a private network. This should be LOOPBACK_IP or LOOPBACK_NETWORK.
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80411>
s/Due to .../The kernel restricts link names to 16 characters or less so we.../
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80413>
s/setup/set up/
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80416>
This should use os::namespaces() from stout/setns.
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80417>
Include the error code?
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80418>
Include the error code?
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80419>
s/not/no/
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80420>
This does more than check for resources.get().ports.isSome() - it validates the range doesn't it?
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80421>
s/ / /
kill the double whitespace
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80448>
Can this be pulled out into stout and tested?
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80447>
Can this be pulled out into stout and tested?
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80449>
Can this be pulled out into stout and tested?
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80423>
Does this require recent kernel patches?
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80424>
How can setHostLoMac not be SOME, given the lines above - why not remove this?
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80425>
ditto
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80428>
call variable createQdisc and reuse?
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80429>
These direct writes to /proc control files should be abstracted out into helpers.
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80430>
ditto
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80431>
ditto
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80432>
ditto
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80433>
ditto
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80434>
Add comment about what it's ignoring here? ethX, lo and other veths?
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80450>
forkedPid is an Option - how are you sure it's isSome()?
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80435>
Technically, you're not 'marking' it. What about "Remove the successfully recovered pid" or some such.
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80436>
s/is/has been/
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80437>
ditto
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80438>
What about "Network isolator recovery complete" to be more inline with other log messages.
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80451>
s/intersected/intersecting/
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80441>
merge into one line like 1274.
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80453>
Single line.
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80454>
Hasn't this been created during initialization of the isolator?
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80455>
What about a bind mount abstraction that took source and target and did the validation, touch, etc.?
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80456>
s/adding/add/
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80457>
name create and reuse?
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80460>
Single line.
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80458>
Single line.
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80462>
Change to ignore and return Nothing() for unknown containers.
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80463>
Single line.
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80464>
Remove this log warning as nothing is really wrong here.
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80465>
What happens if stat isNone?
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80466>
why use the temporary?
if (stat.get().contains("rx_packets")) {
result.set_net_rx_packets(stat.get()["rx_packets"]);
}
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80500>
ditto and all the others
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80501>
Single line.
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80510>
Can you elaborate on this? What's the problem and how is this used to work around it.
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80502>
Why do this - why not delete in the caller since it has the pointer?
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80505>
Include containerId in log message.
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80503>
But don't the filters get set up before any pid is isolated!?
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80509>
I think just using hostIP.address() everywhere instead is cleaner?
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80506>
Include containerId in log message.
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80504>
Include containerId in log message.
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80507>
Is there a test to ensure these get added back correctly when a container is launched? i.e., start a container, stop container, start container.
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80511>
Rename to remove and reuse for icmp and arp?
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80512>
rename to update and use for icmp and arp?
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80513>
Do we verify that happens? Do we wait until it's completed before returning from cleanup?
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80514>
The contract is that the launcher ensures all processes are killed before calling cleanup on the isolators. If there are processes remaining it's a bug in the launcher.
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80515>
"Successfully cleaned up veth " << veth(pid) " for container " << containerId
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80516>
s/logics/logic/
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80517>
Again, why not just use XXX.address() - it seems much, much cleaner than introducing XXXNoPrefix locals?
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80518>
Comment that this will be done by a FilesystemIsolator in the future.
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80519>
s/Setup/Set up/
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80520>
s/setup/set up/
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80521>
s/exits/exists/
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80522>
This is checking the host proc for existence - wouldn't it be better to check the container's proc instead?
if [ -e /proc/sys/... ]; then ...; fi
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/21594/#comment80528>
As mentioned above, why not pre-compute all of these at initialization rather than at each call?
src/slave/containerizer/linux_launcher.cpp
<https://reviews.apache.org/r/21594/#comment80402>
Isn't this now defined in stout/setns?
src/slave/flags.hpp
<https://reviews.apache.org/r/21594/#comment80422>
It doesn't feel correct to specify the maximum number of containers here. What about specifying the size of the ephemeral range, i.e., number of ephemeral ports per container?
src/slave/main.cpp
<https://reviews.apache.org/r/21594/#comment80367>
s/Setup/Set up/
src/tests/environment.cpp
<https://reviews.apache.org/r/21594/#comment80529>
either
s/, network/, the network/
or
s/isolator/isolation/
src/tests/mesos.cpp
<https://reviews.apache.org/r/21594/#comment80530>
This will include the network isolator in all tests if it was compiled in? Perhaps we should start with selectively enabling it in tests?
- Ian Downes
On June 12, 2014, 11:45 a.m., Chi Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21594/
> -----------------------------------------------------------
>
> (Updated June 12, 2014, 11:45 a.m.)
>
>
> Review request for mesos, Ian Downes, Jie Yu, Vinod Kone, and Cong Wang.
>
>
> Bugs: https://issues.apache.org/jira/browse/MESOS-1324
> https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-1324
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Added a network isolator using port-range based traffic redirection on Linux.
>
> - Containers are assigned non-ephemeral ports by the scheduler and ephemeral ports by the network isolator.
> - Virtual ethernet devices and Traffic Control filters are set up so that network traffic in and out of the containers is isolated based on the ports assigned to them.
> - Containers run inside their own network namespaces with separate network stacks, from which per-container network statistics can be retrieved.
>
> A joint work with:
> - Cong Wang (cwang@twopensource.com)
> - Jie Yu (yujie.jay@gmail.com)
> - Ian Downes (ian.downes@gmail.com)
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 102289b
> src/Makefile.am c91b438
> src/launcher/main.cpp b497e98
> src/slave/constants.hpp ace4590
> src/slave/constants.cpp 51f65bb
> src/slave/containerizer/isolators/network/port_mapping.hpp PRE-CREATION
> src/slave/containerizer/isolators/network/port_mapping.cpp PRE-CREATION
> src/slave/containerizer/linux_launcher.cpp c17724b
> src/slave/containerizer/mesos_containerizer.cpp a38564f
> src/slave/flags.hpp 15e5b64
> src/slave/main.cpp 8c2b70c
> src/tests/environment.cpp 005fc54
> src/tests/mesos.cpp e6d807c
>
> Diff: https://reviews.apache.org/r/21594/diff/
>
>
> Testing
> -------
>
> make check on linux. more test cases are being written.
>
>
> Thanks,
>
> Chi Zhang
>
>
Re: Review Request 21594: Port-Range Based Network Isolator for Linux
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21594/#review45572
-----------------------------------------------------------
Patch looks great!
Reviews applied: [21594]
All tests passed.
- Mesos ReviewBot
On June 12, 2014, 6:45 p.m., Chi Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21594/
> -----------------------------------------------------------
>
> (Updated June 12, 2014, 6:45 p.m.)
>
>
> Review request for mesos, Ian Downes, Jie Yu, Vinod Kone, and Cong Wang.
>
>
> Bugs: https://issues.apache.org/jira/browse/MESOS-1324
> https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-1324
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Added a network isolator using port-range based traffic redirection on Linux.
>
> - Containers are assigned non-ephemeral ports by the scheduler and ephemeral ports by the network isolator.
> - Virtual ethernet devices and Traffic Control filters are set up so that network traffic in and out of the containers is isolated based on the ports assigned to them.
> - Containers run inside their own network namespaces with separate network stacks, from which per-container network statistics can be retrieved.
>
> A joint work with:
> - Cong Wang (cwang@twopensource.com)
> - Jie Yu (yujie.jay@gmail.com)
> - Ian Downes (ian.downes@gmail.com)
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 102289b
> src/Makefile.am c91b438
> src/launcher/main.cpp b497e98
> src/slave/constants.hpp ace4590
> src/slave/constants.cpp 51f65bb
> src/slave/containerizer/isolators/network/port_mapping.hpp PRE-CREATION
> src/slave/containerizer/isolators/network/port_mapping.cpp PRE-CREATION
> src/slave/containerizer/linux_launcher.cpp c17724b
> src/slave/containerizer/mesos_containerizer.cpp a38564f
> src/slave/flags.hpp 15e5b64
> src/slave/main.cpp 8c2b70c
> src/tests/environment.cpp 005fc54
> src/tests/mesos.cpp e6d807c
>
> Diff: https://reviews.apache.org/r/21594/diff/
>
>
> Testing
> -------
>
> make check on linux. more test cases are being written.
>
>
> Thanks,
>
> Chi Zhang
>
>
Re: Review Request 21594: Port-Range Based Network Isolator for Linux
Posted by Chi Zhang <ch...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21594/
-----------------------------------------------------------
(Updated June 12, 2014, 6:45 p.m.)
Review request for mesos, Ian Downes, Jie Yu, Vinod Kone, and Cong Wang.
Changes
-------
rebased to please the bot.
Bugs: https://issues.apache.org/jira/browse/MESOS-1324
https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-1324
Repository: mesos-git
Description
-------
Added a network isolator using port-range based traffic redirection on Linux.
- Containers are assigned non-ephemeral ports by the scheduler and ephemeral ports by the network isolator.
- Virtual ethernet devices and Traffic Control filters are set up so that network traffic in and out of the containers is isolated based on the ports assigned to them.
- Containers run inside their own network namespaces with separate network stacks, from which per-container network statistics can be retrieved.
A joint work with:
- Cong Wang (cwang@twopensource.com)
- Jie Yu (yujie.jay@gmail.com)
- Ian Downes (ian.downes@gmail.com)
Diffs (updated)
-----
include/mesos/mesos.proto 102289b
src/Makefile.am c91b438
src/launcher/main.cpp b497e98
src/slave/constants.hpp ace4590
src/slave/constants.cpp 51f65bb
src/slave/containerizer/isolators/network/port_mapping.hpp PRE-CREATION
src/slave/containerizer/isolators/network/port_mapping.cpp PRE-CREATION
src/slave/containerizer/linux_launcher.cpp c17724b
src/slave/containerizer/mesos_containerizer.cpp a38564f
src/slave/flags.hpp 15e5b64
src/slave/main.cpp 8c2b70c
src/tests/environment.cpp 005fc54
src/tests/mesos.cpp e6d807c
Diff: https://reviews.apache.org/r/21594/diff/
Testing
-------
make check on linux. more test cases are being written.
Thanks,
Chi Zhang
Re: Review Request 21594: Port-Range Based Network Isolator for Linux
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21594/#review45455
-----------------------------------------------------------
Bad patch!
Reviews applied: [21594]
Failed command: git apply --index 21594.patch
Error:
error: patch failed: src/Makefile.am:323
error: src/Makefile.am: patch does not apply
- Mesos ReviewBot
On June 11, 2014, 6:44 p.m., Chi Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21594/
> -----------------------------------------------------------
>
> (Updated June 11, 2014, 6:44 p.m.)
>
>
> Review request for mesos, Ian Downes, Jie Yu, Vinod Kone, and Cong Wang.
>
>
> Bugs: https://issues.apache.org/jira/browse/MESOS-1324
> https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-1324
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Added a network isolator using port-range based traffic redirection on Linux.
>
> - Containers are assigned non-ephemeral ports by the scheduler and ephemeral ports by the network isolator.
> - Virtual ethernet devices and Traffic Control filters are set up so that network traffic in and out of the containers is isolated based on the ports assigned to them.
> - Containers run inside their own network namespaces with separate network stacks, from which per-container network statistics can be retrieved.
>
> A joint work with:
> - Cong Wang (cwang@twopensource.com)
> - Jie Yu (yujie.jay@gmail.com)
> - Ian Downes (ian.downes@gmail.com)
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 62f69d2
> src/Makefile.am 4a3f2e1
> src/launcher/main.cpp b497e98
> src/slave/constants.hpp ace4590
> src/slave/constants.cpp 51f65bb
> src/slave/containerizer/isolators/network/port_mapping.hpp PRE-CREATION
> src/slave/containerizer/isolators/network/port_mapping.cpp PRE-CREATION
> src/slave/containerizer/linux_launcher.cpp c17724b
> src/slave/containerizer/mesos_containerizer.cpp a38564f
> src/slave/flags.hpp 15e5b64
> src/slave/main.cpp 8c2b70c
> src/tests/environment.cpp 005fc54
> src/tests/mesos.cpp ea6a1c0
>
> Diff: https://reviews.apache.org/r/21594/diff/
>
>
> Testing
> -------
>
> make check on linux. more test cases are being written.
>
>
> Thanks,
>
> Chi Zhang
>
>