You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ch...@apache.org on 2018/09/22 06:34:06 UTC
[mesos] 02/10: Performed RP-specific validations when
adding/updating RP configs.
This is an automated email from the ASF dual-hosted git repository.
chhsiao pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 1adceaacc81bb33332a52a9f28a864ac928ac706
Author: Chun-Hung Hsiao <ch...@mesosphere.io>
AuthorDate: Tue Sep 18 14:40:55 2018 -0700
Performed RP-specific validations when adding/updating RP configs.
Each type of RP might have some specific validations for RP info. For
example, SLRP requires the `storage` field to be set. This patch makes
the local RP daemon to perform such validations when adding/updating
configs, so the `ADD_RESOURCE_PROVIDER_CONFIG` and
`UPDATE_RESOURCE_PROVIDER_CONFIG` calls can fail fast.
Review: https://reviews.apache.org/r/68756
---
src/resource_provider/daemon.cpp | 2 +-
src/resource_provider/local.cpp | 61 ++++++++++++++++++------------
src/resource_provider/local.hpp | 4 ++
src/resource_provider/storage/provider.cpp | 34 +++++++++++------
src/resource_provider/storage/provider.hpp | 9 +----
src/slave/http.cpp | 18 +++++++++
6 files changed, 85 insertions(+), 43 deletions(-)
diff --git a/src/resource_provider/daemon.cpp b/src/resource_provider/daemon.cpp
index 0a76cc6..b07ffe3 100644
--- a/src/resource_provider/daemon.cpp
+++ b/src/resource_provider/daemon.cpp
@@ -466,7 +466,7 @@ Future<Nothing> LocalResourceProviderDaemonProcess::_launch(
"' and name '" + name + "': " + provider.error());
}
- data.provider = provider.get();
+ data.provider = std::move(provider.get());
return Nothing();
}
diff --git a/src/resource_provider/local.cpp b/src/resource_provider/local.cpp
index 801e6c4..bd443aa 100644
--- a/src/resource_provider/local.cpp
+++ b/src/resource_provider/local.cpp
@@ -14,8 +14,9 @@
// See the License for the specific language governing permissions and
// limitations under the License.
+#include <type_traits>
+
#include <stout/hashmap.hpp>
-#include <stout/lambda.hpp>
#include "resource_provider/local.hpp"
@@ -32,6 +33,25 @@ using process::http::authentication::Principal;
namespace mesos {
namespace internal {
+struct ProviderAdaptor
+{
+ decltype(LocalResourceProvider::create)* const create;
+ decltype(LocalResourceProvider::principal)* const principal;
+ decltype(LocalResourceProvider::validate)* const validate;
+};
+
+
+// TODO(jieyu): Document the built-in local resource providers.
+static const hashmap<string, ProviderAdaptor> adaptors = {
+#ifdef __linux__
+ {"org.apache.mesos.rp.local.storage",
+ {&StorageLocalResourceProvider::create,
+ &StorageLocalResourceProvider::principal,
+ &StorageLocalResourceProvider::validate}},
+#endif
+};
+
+
Try<Owned<LocalResourceProvider>> LocalResourceProvider::create(
const http::URL& url,
const string& workDir,
@@ -40,40 +60,33 @@ Try<Owned<LocalResourceProvider>> LocalResourceProvider::create(
const Option<string>& authToken,
bool strict)
{
- // TODO(jieyu): Document the built-in local resource providers.
- const hashmap<string, lambda::function<decltype(create)>> creators = {
-#if defined(__linux__)
- {"org.apache.mesos.rp.local.storage", &StorageLocalResourceProvider::create}
-#endif
- };
-
- if (creators.contains(info.type())) {
- return creators.at(info.type())(
- url, workDir, info, slaveId, authToken, strict);
+ if (!adaptors.contains(info.type())) {
+ return Error("Unknown local resource provider type '" + info.type() + "'");
}
- return Error("Unknown local resource provider type '" + info.type() + "'");
+ return adaptors.at(info.type()).create(
+ url, workDir, info, slaveId, authToken, strict);
}
Try<Principal> LocalResourceProvider::principal(
const ResourceProviderInfo& info)
{
- // TODO(chhsiao): Document the principals for built-in local resource
- // providers.
- const hashmap<string, lambda::function<decltype(principal)>>
- principalGenerators = {
-#if defined(__linux__)
- {"org.apache.mesos.rp.local.storage",
- &StorageLocalResourceProvider::principal}
-#endif
- };
+ if (!adaptors.contains(info.type())) {
+ return Error("Unknown local resource provider type '" + info.type() + "'");
+ }
+
+ return adaptors.at(info.type()).principal(info);
+}
+
- if (principalGenerators.contains(info.type())) {
- return principalGenerators.at(info.type())(info);
+Option<Error> LocalResourceProvider::validate(const ResourceProviderInfo& info)
+{
+ if (!adaptors.contains(info.type())) {
+ return Error("Unknown local resource provider type '" + info.type() + "'");
}
- return Error("Unknown local resource provider type '" + info.type() + "'");
+ return adaptors.at(info.type()).validate(info);
}
} // namespace internal {
diff --git a/src/resource_provider/local.hpp b/src/resource_provider/local.hpp
index 20bcc78..ff4e7f4 100644
--- a/src/resource_provider/local.hpp
+++ b/src/resource_provider/local.hpp
@@ -23,6 +23,8 @@
#include <process/http.hpp>
#include <process/owned.hpp>
+#include <stout/error.hpp>
+#include <stout/option.hpp>
#include <stout/try.hpp>
namespace mesos {
@@ -42,6 +44,8 @@ public:
static Try<process::http::authentication::Principal> principal(
const ResourceProviderInfo& info);
+ static Option<Error> validate(const ResourceProviderInfo& info);
+
virtual ~LocalResourceProvider() = default;
};
diff --git a/src/resource_provider/storage/provider.cpp b/src/resource_provider/storage/provider.cpp
index 6475f65..18f934b 100644
--- a/src/resource_provider/storage/provider.cpp
+++ b/src/resource_provider/storage/provider.cpp
@@ -3755,6 +3755,28 @@ Try<Owned<LocalResourceProvider>> StorageLocalResourceProvider::create(
const Option<string>& authToken,
bool strict)
{
+ Option<Error> error = validate(info);
+ if (error.isSome()) {
+ return error.get();
+ }
+
+ return Owned<LocalResourceProvider>(new StorageLocalResourceProvider(
+ url, workDir, info, slaveId, authToken, strict));
+}
+
+
+Try<Principal> StorageLocalResourceProvider::principal(
+ const ResourceProviderInfo& info)
+{
+ return Principal(
+ Option<string>::none(),
+ {{"cid_prefix", getContainerIdPrefix(info)}});
+}
+
+
+Option<Error> StorageLocalResourceProvider::validate(
+ const ResourceProviderInfo& info)
+{
if (info.has_id()) {
return Error("'ResourceProviderInfo.id' must not be set");
}
@@ -3805,17 +3827,7 @@ Try<Owned<LocalResourceProvider>> StorageLocalResourceProvider::create(
stringify(CSIPluginContainerInfo::NODE_SERVICE) + " not found");
}
- return Owned<LocalResourceProvider>(new StorageLocalResourceProvider(
- url, workDir, info, slaveId, authToken, strict));
-}
-
-
-Try<Principal> StorageLocalResourceProvider::principal(
- const ResourceProviderInfo& info)
-{
- return Principal(
- Option<string>::none(),
- {{"cid_prefix", getContainerIdPrefix(info)}});
+ return None();
}
diff --git a/src/resource_provider/storage/provider.hpp b/src/resource_provider/storage/provider.hpp
index 5a371b1..331f7b7 100644
--- a/src/resource_provider/storage/provider.hpp
+++ b/src/resource_provider/storage/provider.hpp
@@ -17,13 +17,6 @@
#ifndef __RESOURCE_PROVIDER_STORAGE_PROVIDER_HPP__
#define __RESOURCE_PROVIDER_STORAGE_PROVIDER_HPP__
-#include <process/http.hpp>
-#include <process/owned.hpp>
-
-#include <stout/try.hpp>
-
-#include <mesos/mesos.hpp>
-
#include "resource_provider/local.hpp"
namespace mesos {
@@ -47,6 +40,8 @@ public:
static Try<process::http::authentication::Principal> principal(
const mesos::ResourceProviderInfo& info);
+ static Option<Error> validate(const mesos::ResourceProviderInfo& info);
+
~StorageLocalResourceProvider() override;
StorageLocalResourceProvider(
diff --git a/src/slave/http.cpp b/src/slave/http.cpp
index 0adc7c0..a4db532 100644
--- a/src/slave/http.cpp
+++ b/src/slave/http.cpp
@@ -68,6 +68,8 @@
#include "mesos/mesos.hpp"
#include "mesos/resources.hpp"
+#include "resource_provider/local.hpp"
+
#include "slave/http.hpp"
#include "slave/slave.hpp"
#include "slave/validation.hpp"
@@ -3206,6 +3208,14 @@ Future<Response> Http::addResourceProviderConfig(
<< "Processing ADD_RESOURCE_PROVIDER_CONFIG call with type '"
<< info.type() << "' and name '" << info.name() << "'";
+ Option<Error> error = LocalResourceProvider::validate(info);
+ if (error.isSome()) {
+ return BadRequest(
+ "Failed to validate resource provider config with type '" +
+ info.type() + "' and name '" + info.name() + "': " +
+ error->message);
+ }
+
return slave->localResourceProviderDaemon->add(info)
.then([](bool added) -> Response {
if (!added) {
@@ -3243,6 +3253,14 @@ Future<Response> Http::updateResourceProviderConfig(
<< "Processing UPDATE_RESOURCE_PROVIDER_CONFIG call with type '"
<< info.type() << "' and name '" << info.name() << "'";
+ Option<Error> error = LocalResourceProvider::validate(info);
+ if (error.isSome()) {
+ return BadRequest(
+ "Failed to validate resource provider config with type '" +
+ info.type() + "' and name '" + info.name() + "': " +
+ error->message);
+ }
+
return slave->localResourceProviderDaemon->update(info)
.then([](bool updated) -> Response {
if (!updated) {