This is an automated email from the ASF dual-hosted git repository. qianzhang pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 3592abaddeb66a9c12614d7eb52adfa6151543e3 Author: Qian Zhang <[email protected]> AuthorDate: Thu Jan 23 10:55:49 2020 +0800 Updated the `LaunchContainer` agent API to support resource limits. Review: https://reviews.apache.org/r/72040 --- include/mesos/agent/agent.proto | 3 ++ include/mesos/v1/agent/agent.proto | 3 ++ src/slave/http.cpp | 14 ++++++++-- src/slave/http.hpp | 4 ++- src/slave/validation.cpp | 57 +++++++++++++++++--------------------- 5 files changed, 45 insertions(+), 36 deletions(-) diff --git a/include/mesos/agent/agent.proto b/include/mesos/agent/agent.proto index 030ed61..a062e58 100644 --- a/include/mesos/agent/agent.proto +++ b/include/mesos/agent/agent.proto @@ -249,6 +249,9 @@ message Call { repeated Resource resources = 3; optional ContainerInfo container = 4; + + // Resource limits associated with the container during launch. + map<string, Value.Scalar> limits = 5; } // Waits for the standalone or nested container to terminate diff --git a/include/mesos/v1/agent/agent.proto b/include/mesos/v1/agent/agent.proto index be7db86..37167fd 100644 --- a/include/mesos/v1/agent/agent.proto +++ b/include/mesos/v1/agent/agent.proto @@ -249,6 +249,9 @@ message Call { repeated Resource resources = 3; optional ContainerInfo container = 4; + + // Resource limits associated with the container during launch. + map<string, Value.Scalar> limits = 5; } // Waits for the standalone or nested container to terminate diff --git a/src/slave/http.cpp b/src/slave/http.cpp index 4493abf..ff3408f 100644 --- a/src/slave/http.cpp +++ b/src/slave/http.cpp @@ -3267,6 +3267,7 @@ Future<Response> Http::launchNestedContainer( call.launch_nested_container().container_id(), call.launch_nested_container().command(), None(), + None(), call.launch_nested_container().has_container() ? call.launch_nested_container().container() : Option<ContainerInfo>::none(), @@ -3319,6 +3320,7 @@ Future<Response> Http::launchContainer( call.launch_container().container_id(), call.launch_container().command(), call.launch_container().resources(), + call.launch_container().limits(), call.launch_container().has_container() ? call.launch_container().container() : Option<ContainerInfo>::none(), @@ -3333,7 +3335,8 @@ template <authorization::Action action> Future<Response> Http::_launchContainer( const ContainerID& containerId, const CommandInfo& commandInfo, - const Option<Resources>& resources, + const Option<Resources>& resourceRequests, + const Option<google::protobuf::Map<string, Value::Scalar>>& resourceLimits, const Option<ContainerInfo>& containerInfo, const Option<ContainerClass>& containerClass, ContentType, @@ -3367,8 +3370,12 @@ Future<Response> Http::_launchContainer( } #endif // __WINDOWS__ - if (resources.isSome()) { - containerConfig.mutable_resources()->CopyFrom(resources.get()); + if (resourceRequests.isSome()) { + containerConfig.mutable_resources()->CopyFrom(resourceRequests.get()); + } + + if (resourceLimits.isSome()) { + *containerConfig.mutable_limits() = resourceLimits.get(); } if (containerInfo.isSome()) { @@ -4228,6 +4235,7 @@ Future<Response> Http::launchNestedContainerSession( call.launch_nested_container_session().container_id(), call.launch_nested_container_session().command(), None(), + None(), call.launch_nested_container_session().has_container() ? call.launch_nested_container_session().container() : Option<ContainerInfo>::none(), diff --git a/src/slave/http.hpp b/src/slave/http.hpp index 71fc6f7..92114ea 100644 --- a/src/slave/http.hpp +++ b/src/slave/http.hpp @@ -253,7 +253,9 @@ private: process::Future<process::http::Response> _launchContainer( const ContainerID& containerId, const CommandInfo& commandInfo, - const Option<Resources>& resources, + const Option<Resources>& resourceRequests, + const Option< + google::protobuf::Map<std::string, Value::Scalar>>& resourceLimits, const Option<ContainerInfo>& containerInfo, const Option<mesos::slave::ContainerClass>& containerClass, ContentType acceptType, diff --git a/src/slave/validation.cpp b/src/slave/validation.cpp index 99b17c9..efb2e0c 100644 --- a/src/slave/validation.cpp +++ b/src/slave/validation.cpp @@ -375,15 +375,6 @@ Option<Error> validate( "'launch_container.container_id' is invalid: " + error->message); } - // Nested containers share resources with their parent so are - // not allowed to specify resources in this call. - if (call.launch_container().container_id().has_parent() && - call.launch_container().resources().size() != 0) { - return Error( - "Resources may not be specified when using " - "'launch_container' to launch nested containers"); - } - // General resource validation first. error = Resources::validate(call.launch_container().resources()); if (error.isSome()) { @@ -399,31 +390,33 @@ Option<Error> validate( // Because standalone containers are launched outside of the master's // offer cycle, some resource types or fields may not be specified. - foreach (Resource resource, call.launch_container().resources()) { - // Upgrade the resources (in place) to simplify validation. - upgradeResource(&resource); - - // Standalone containers may only use unreserved resources. - // There is no accounting in the master for resources consumed - // by standalone containers, so allowing reserved resources would - // only increase code complexity with no change in behavior. - if (Resources::isReserved(resource)) { - return Error("'launch_container.resources' must be unreserved"); - } + if (!call.launch_container().container_id().has_parent()) { + foreach (Resource resource, call.launch_container().resources()) { + // Upgrade the resources (in place) to simplify validation. + upgradeResource(&resource); + + // Standalone containers may only use unreserved resources. + // There is no accounting in the master for resources consumed + // by standalone containers, so allowing reserved resources would + // only increase code complexity with no change in behavior. + if (Resources::isReserved(resource)) { + return Error("'launch_container.resources' must be unreserved"); + } - // NOTE: The master normally requires all volumes be persistent, - // and that all persistent volumes belong to a role. Standalone - // containers therefore cannot use persistent volumes. - if (Resources::isPersistentVolume(resource)) { - return Error( - "'launch_container.resources' may not use persistent volumes"); - } + // NOTE: The master normally requires all volumes be persistent, + // and that all persistent volumes belong to a role. Standalone + // containers therefore cannot use persistent volumes. + if (Resources::isPersistentVolume(resource)) { + return Error( + "'launch_container.resources' may not use persistent volumes"); + } - // Standalone containers are expected to occupy resources *not* - // advertised by the agent and hence do not need to worry about - // being preempted or throttled. - if (Resources::isRevocable(resource)) { - return Error("'launch_container.resources' must be non-revocable"); + // Standalone containers are expected to occupy resources *not* + // advertised by the agent and hence do not need to worry about + // being preempted or throttled. + if (Resources::isRevocable(resource)) { + return Error("'launch_container.resources' must be non-revocable"); + } } }
