Repository: mesos Updated Branches: refs/heads/master 0f647a619 -> 7c054d188
Made ephemeral ports a resource and killed private resources. Review: https://reviews.apache.org/r/24269 Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/51aaff38 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/51aaff38 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/51aaff38 Branch: refs/heads/master Commit: 51aaff382d977b61d7d2a49eb4e561e12a500897 Parents: 0f647a6 Author: Jie Yu <yujie....@gmail.com> Authored: Mon Aug 4 15:14:49 2014 -0700 Committer: Jie Yu <yujie....@gmail.com> Committed: Wed Aug 6 09:23:09 2014 -0700 ---------------------------------------------------------------------- include/mesos/mesos.proto | 4 -- include/mesos/resources.hpp | 5 ++ src/common/resources.cpp | 21 ++++++++ .../isolators/network/port_mapping.cpp | 56 ++++++++++++-------- .../isolators/network/port_mapping.hpp | 9 ++-- src/slave/flags.hpp | 10 ---- src/slave/slave.cpp | 17 ------ src/tests/mesos.cpp | 11 +++- src/tests/port_mapping_tests.cpp | 10 ++-- 9 files changed, 78 insertions(+), 65 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/51aaff38/include/mesos/mesos.proto ---------------------------------------------------------------------- diff --git a/include/mesos/mesos.proto b/include/mesos/mesos.proto index 6d4fd14..628cce1 100644 --- a/include/mesos/mesos.proto +++ b/include/mesos/mesos.proto @@ -292,10 +292,6 @@ message SlaveInfo { optional SlaveID id = 6; optional bool checkpoint = 7 [default = false]; - // Resources that are managed locally by the slave, and not exposed - // to master and frameworks. - repeated Resource private_resources = 9; - // Deprecated! optional string webui_hostname = 2; optional int32 webui_port = 4 [default = 8081]; http://git-wip-us.apache.org/repos/asf/mesos/blob/51aaff38/include/mesos/resources.hpp ---------------------------------------------------------------------- diff --git a/include/mesos/resources.hpp b/include/mesos/resources.hpp index daaa6bf..12adedc 100644 --- a/include/mesos/resources.hpp +++ b/include/mesos/resources.hpp @@ -188,9 +188,14 @@ public: Option<double> cpus() const; Option<Bytes> mem() const; Option<Bytes> disk() const; + // TODO(vinod): Provide a Ranges abstraction. Option<Value::Ranges> ports() const; + // TODO(jieyu): Consider returning an EphemeralPorts abstraction + // which holds the ephemeral ports allocation logic. + Option<Value::Ranges> ephemeral_ports() const; + typedef google::protobuf::RepeatedPtrField<Resource>::iterator iterator; http://git-wip-us.apache.org/repos/asf/mesos/blob/51aaff38/src/common/resources.cpp ---------------------------------------------------------------------- diff --git a/src/common/resources.cpp b/src/common/resources.cpp index ade2a71..edf36b1 100644 --- a/src/common/resources.cpp +++ b/src/common/resources.cpp @@ -517,6 +517,27 @@ Option<Value::Ranges> Resources::ports() const } +Option<Value::Ranges> Resources::ephemeral_ports() const +{ + Value::Ranges total; + bool found = false; + + foreach (const Resource& resource, resources) { + if (resource.name() == "ephemeral_ports" && + resource.type() == Value::RANGES) { + total += resource.ranges(); + found = true; + } + } + + if (found) { + return total; + } + + return None(); +} + + Try<Resource> Resources::parse( const string& name, const string& text, http://git-wip-us.apache.org/repos/asf/mesos/blob/51aaff38/src/slave/containerizer/isolators/network/port_mapping.cpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/isolators/network/port_mapping.cpp b/src/slave/containerizer/isolators/network/port_mapping.cpp index 8445874..daf89cc 100644 --- a/src/slave/containerizer/isolators/network/port_mapping.cpp +++ b/src/slave/containerizer/isolators/network/port_mapping.cpp @@ -644,10 +644,6 @@ Try<Isolator*> PortMappingIsolatorProcess::create(const Flags& flags) stringify(checkCommandIp.get())); } - // Get 'ports' resource from 'resources' flag. These ports will be - // treated as non-ephemeral ports. - IntervalSet<uint16_t> nonEphemeralPorts; - Try<Resources> resources = Resources::parse( flags.resources.get(""), flags.default_role); @@ -656,31 +652,25 @@ Try<Isolator*> PortMappingIsolatorProcess::create(const Flags& flags) return Error("Failed to parse --resources: " + resources.error()); } + // Get 'ports' resource from 'resources' flag. These ports will be + // treated as non-ephemeral ports. + IntervalSet<uint16_t> nonEphemeralPorts; if (resources.get().ports().isSome()) { nonEphemeralPorts = getIntervalSet(resources.get().ports().get()); } - // Get 'ports' resource from 'private_resources' flag. These ports + // Get 'ephemeral_ports' resource from 'resources' flag. These ports // will be allocated to each container as ephemeral ports. IntervalSet<uint16_t> ephemeralPorts; - - resources = Resources::parse( - flags.private_resources.get(""), - flags.default_role); - - if (resources.isError()) { - return Error("Failed to parse --private_resources: " + resources.error()); - } - - if (resources.get().ports().isSome()) { - ephemeralPorts = getIntervalSet(resources.get().ports().get()); + if (resources.get().ephemeral_ports().isSome()) { + ephemeralPorts = getIntervalSet(resources.get().ephemeral_ports().get()); } // Each container requires at least one ephemeral port for slave - // executor communication. If no 'ports' resource is found, we will - // return error. + // executor communication. If no 'ephemeral_ports' resource is + // found, we will return error. if (ephemeralPorts.empty()) { - return Error("Private resources do not contain ports"); + return Error("Ephemeral ports are not specified"); } // Sanity check to make sure that the ephemeral ports specified do @@ -1290,17 +1280,27 @@ Future<Option<CommandInfo> > PortMappingIsolatorProcess::prepare( } } - // Determine the ephemeral ports used by this container. + // TODO(jieyu): For now, we simply ignore the 'ephemeral_ports' + // specified in the executor info. However, this behavior needs to + // be changed once the master can make default allocations for + // ephemeral ports. + if (resources.ephemeral_ports().isSome()) { + LOG(WARNING) << "Ignoring the specified ephemeral_ports '" + << resources.ephemeral_ports().get() + << "' for container" << containerId + << " of executor " << executorInfo.executor_id(); + } + + // Allocate the ephemeral ports used by this container. Try<Interval<uint16_t> > ephemeralPorts = ephemeralPortsAllocator->allocate(); if (ephemeralPorts.isError()) { return Failure( "Failed to allocate ephemeral ports: " + ephemeralPorts.error()); } - infos[containerId] = - CHECK_NOTNULL(new Info(nonEphemeralPorts, ephemeralPorts.get())); + infos[containerId] = new Info(nonEphemeralPorts, ephemeralPorts.get()); - LOG(INFO) << "Allocated non-ephemeral ports " << nonEphemeralPorts + LOG(INFO) << "Using non-ephemeral ports " << nonEphemeralPorts << " and ephemeral ports " << ephemeralPorts.get() << " for container " << containerId << " of executor " << executorInfo.executor_id(); @@ -1605,6 +1605,16 @@ Future<Nothing> PortMappingIsolatorProcess::update( return Nothing(); } + // TODO(jieyu): For now, we simply ignore the 'ephemeral_ports' + // specified in 'resources'. However, this behavior needs to be + // changed once the master can make default allocations for + // ephemeral ports. + if (resources.ephemeral_ports().isSome()) { + LOG(WARNING) << "Ignoring the specified ephemeral_ports '" + << resources.ephemeral_ports().get() + << "' for container" << containerId; + } + Info* info = CHECK_NOTNULL(infos[containerId]); if (info->pid.isNone()) { http://git-wip-us.apache.org/repos/asf/mesos/blob/51aaff38/src/slave/containerizer/isolators/network/port_mapping.hpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/isolators/network/port_mapping.hpp b/src/slave/containerizer/isolators/network/port_mapping.hpp index f75429c..a417f55 100644 --- a/src/slave/containerizer/isolators/network/port_mapping.hpp +++ b/src/slave/containerizer/isolators/network/port_mapping.hpp @@ -75,15 +75,14 @@ public: // exhausting available ephemeral ports). Try<Interval<uint16_t> > allocate(); - // Mark an ephemeral port range allocated. This is used in - // 'recover'. + // Mark the specified ephemeral port range as allocated. void allocate(const Interval<uint16_t>& ports); - // Deallocate an ephemeral port range. + // Deallocate the specified ephemeral port range. void deallocate(const Interval<uint16_t>& ports); - // Return true if the port range 'ports' is managed by the - // allocator, regardless it has been allocated to use or not. + // Return true if the specified ephemeral port range is managed by + // the allocator, regardless it has been allocated to use or not. bool isManaged(const Interval<uint16_t>& ports) { return (free + used).contains(ports); http://git-wip-us.apache.org/repos/asf/mesos/blob/51aaff38/src/slave/flags.hpp ---------------------------------------------------------------------- diff --git a/src/slave/flags.hpp b/src/slave/flags.hpp index c348109..841de23 100644 --- a/src/slave/flags.hpp +++ b/src/slave/flags.hpp @@ -289,15 +289,6 @@ public: "isolator. This number has to be a power of 2.\n", DEFAULT_EPHEMERAL_PORTS_PER_CONTAINER); - add(&Flags::private_resources, - "private_resources", - "The resources that will be manged by the slave locally, and not\n" - "exposed to Mesos master and frameworks. It shares the same format\n" - "as the 'resources' flag. One example of such type of resources\n" - "is ephemeral ports when port mapping network isolator is enabled.\n" - "Use 'ports:[x-y]' to specify the ephemeral ports that will be\n" - "locally managed.\n"); - add(&Flags::eth0_name, "eth0_name", "The name of the public network interface (e.g., eth0). If it is\n" @@ -350,7 +341,6 @@ public: std::string docker; #ifdef WITH_NETWORK_ISOLATOR uint16_t ephemeral_ports_per_container; - Option<std::string> private_resources; Option<std::string> eth0_name; Option<std::string> lo_name; #endif http://git-wip-us.apache.org/repos/asf/mesos/blob/51aaff38/src/slave/slave.cpp ---------------------------------------------------------------------- diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp index c56cac8..f5ccb73 100644 --- a/src/slave/slave.cpp +++ b/src/slave/slave.cpp @@ -277,19 +277,6 @@ void Slave::initialize() } LOG(INFO) << "Slave resources: " << resources.get(); -#ifdef WITH_NETWORK_ISOLATOR - // Resources that are not exposed to the frameworks, but managed by - // the slave privately. - Try<Resources> privateResources = Resources::parse( - flags.private_resources.get(""), flags.default_role); - - if (privateResources.isError()) { - EXIT(1) << "Failed to determine slave private resources: " - << privateResources.error(); - } - LOG(INFO) << "Slave private resources: " << privateResources.get(); -#endif - if (flags.attributes.isSome()) { attributes = Attributes::parse(flags.attributes.get()); } @@ -316,10 +303,6 @@ void Slave::initialize() info.mutable_attributes()->CopyFrom(attributes); info.set_checkpoint(flags.checkpoint); -#ifdef WITH_NETWORK_ISOLATOR - info.mutable_private_resources()->CopyFrom(privateResources.get()); -#endif - LOG(INFO) << "Slave hostname: " << info.hostname(); LOG(INFO) << "Slave checkpoint: " << stringify(flags.checkpoint); http://git-wip-us.apache.org/repos/asf/mesos/blob/51aaff38/src/tests/mesos.cpp ---------------------------------------------------------------------- diff --git a/src/tests/mesos.cpp b/src/tests/mesos.cpp index 35c94fa..b51954e 100644 --- a/src/tests/mesos.cpp +++ b/src/tests/mesos.cpp @@ -345,8 +345,15 @@ slave::Flags ContainerizerTest<slave::MesosContainerizer>::CreateSlaveFlags() #ifdef WITH_NETWORK_ISOLATOR if (user.get() == "root" && routing::check().isSome()) { - flags.isolation += ",network/port_mapping"; - flags.private_resources = "ports:" + slave::DEFAULT_EPHEMERAL_PORTS; + flags.isolation = strings::join( + ",", + flags.isolation, + "network/port_mapping"); + + flags.resources = strings::join( + ";", + flags.resources.get(), + "ephemeral_ports:" + slave::DEFAULT_EPHEMERAL_PORTS); } #endif http://git-wip-us.apache.org/repos/asf/mesos/blob/51aaff38/src/tests/port_mapping_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/port_mapping_tests.cpp b/src/tests/port_mapping_tests.cpp index 393b578..682325a 100644 --- a/src/tests/port_mapping_tests.cpp +++ b/src/tests/port_mapping_tests.cpp @@ -216,7 +216,7 @@ protected: port = 31001; - // errorPort is not in resources and private_resources. + // 'errorPort' is not in 'ports' or 'ephemeral_ports'. errorPort = 32502; container1Ready = path::join(os::getcwd(), "container1_ready"); @@ -237,9 +237,12 @@ protected: slave::Flags flags; flags.launcher_dir = path::join(tests::flags.build_dir, "src"); - flags.resources = "cpus:2;mem:1024;disk:1024;ports:[31000-32000]"; + + flags.resources = + ("cpus:2;mem:1024;disk:1024;ports:[31000-32000];ephemeral_ports:" + + slave::DEFAULT_EPHEMERAL_PORTS); + flags.isolation = "network/port_mapping"; - flags.private_resources = "ports:" + slave::DEFAULT_EPHEMERAL_PORTS; return flags; } @@ -1286,7 +1289,6 @@ TEST_F(PortMappingMesosTest, ROOT_RecoverMixedContainersTest) // slave. isolations.erase(it); flagsNoNetworkIsolator.isolation = strings::join(",", isolations); - flagsNoNetworkIsolator.private_resources = ""; Try<PID<Master> > master = StartMaster(); ASSERT_SOME(master);
