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:11 UTC
[mesos] 07/10: Moved the container ID prefix generation to
`LocalResourceProvider`.
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 88b28f2cd900a53a8d54b7be837ccc9813e3b764
Author: Chun-Hung Hsiao <ch...@mesosphere.io>
AuthorDate: Thu Sep 20 11:25:11 2018 -0700
Moved the container ID prefix generation to `LocalResourceProvider`.
It is more reasonable to not allow each specific resource provider to
construct their own container ID prefix, otherwise it would be hard to
avoid conflicts. Therefore we now established the convension of how the
prefix is constructed in `LocalResourceProvider`.
Review: https://reviews.apache.org/r/68790
---
src/resource_provider/daemon.cpp | 11 +------
src/resource_provider/local.cpp | 28 +++++++++-------
src/resource_provider/local.hpp | 10 ++++--
src/resource_provider/storage/provider.cpp | 51 +++++++++---------------------
4 files changed, 40 insertions(+), 60 deletions(-)
diff --git a/src/resource_provider/daemon.cpp b/src/resource_provider/daemon.cpp
index b07ffe3..f1a941c 100644
--- a/src/resource_provider/daemon.cpp
+++ b/src/resource_provider/daemon.cpp
@@ -480,16 +480,7 @@ Future<Option<string>> LocalResourceProviderDaemonProcess::generateAuthToken(
return None();
}
- Try<Principal> principal = LocalResourceProvider::principal(info);
-
- if (principal.isError()) {
- return Failure(
- "Failed to generate resource provider principal with type '" +
- info.type() + "' and name '" + info.name() + "': " +
- principal.error());
- }
-
- return secretGenerator->generate(principal.get())
+ return secretGenerator->generate(LocalResourceProvider::principal(info))
.then(defer(self(), [](const Secret& secret) -> Future<Option<string>> {
Option<Error> error = common::validation::validateSecret(secret);
diff --git a/src/resource_provider/local.cpp b/src/resource_provider/local.cpp
index bd443aa..e5efd95 100644
--- a/src/resource_provider/local.cpp
+++ b/src/resource_provider/local.cpp
@@ -36,7 +36,6 @@ namespace internal {
struct ProviderAdaptor
{
decltype(LocalResourceProvider::create)* const create;
- decltype(LocalResourceProvider::principal)* const principal;
decltype(LocalResourceProvider::validate)* const validate;
};
@@ -46,7 +45,6 @@ static const hashmap<string, ProviderAdaptor> adaptors = {
#ifdef __linux__
{"org.apache.mesos.rp.local.storage",
{&StorageLocalResourceProvider::create,
- &StorageLocalResourceProvider::principal,
&StorageLocalResourceProvider::validate}},
#endif
};
@@ -69,24 +67,32 @@ Try<Owned<LocalResourceProvider>> LocalResourceProvider::create(
}
-Try<Principal> LocalResourceProvider::principal(
- const ResourceProviderInfo& info)
+Option<Error> LocalResourceProvider::validate(const ResourceProviderInfo& info)
{
if (!adaptors.contains(info.type())) {
return Error("Unknown local resource provider type '" + info.type() + "'");
}
- return adaptors.at(info.type()).principal(info);
+ return adaptors.at(info.type()).validate(info);
}
-Option<Error> LocalResourceProvider::validate(const ResourceProviderInfo& info)
+Principal LocalResourceProvider::principal(const ResourceProviderInfo& info)
{
- if (!adaptors.contains(info.type())) {
- return Error("Unknown local resource provider type '" + info.type() + "'");
- }
-
- return adaptors.at(info.type()).validate(info);
+ // The 'cid_prefix' for the resource provider is of the following format:
+ // <rp_type>-<rp_name>--
+ // where <rp_type> and <rp_name> are the type and name of the resource
+ // provider, with dots replaced by dashes. We use a double-dash at the end to
+ // explicitly mark the end of the prefix.
+ const string cidPrefix = strings::join(
+ "-",
+ strings::replace(info.type(), ".", "-"),
+ info.name(),
+ "-");
+
+ return Principal(
+ Option<string>::none(),
+ {{"cid_prefix", cidPrefix}});
}
} // namespace internal {
diff --git a/src/resource_provider/local.hpp b/src/resource_provider/local.hpp
index ff4e7f4..75ce0f2 100644
--- a/src/resource_provider/local.hpp
+++ b/src/resource_provider/local.hpp
@@ -41,11 +41,15 @@ public:
const Option<std::string>& authToken,
bool strict);
- static Try<process::http::authentication::Principal> principal(
- const ResourceProviderInfo& info);
-
static Option<Error> validate(const ResourceProviderInfo& info);
+ // Returns a principal that contains a 'cid_prefix' claim based on the type
+ // and name of the resource provider. The resource provider can only launch
+ // standalone containers whose ID is prefixed by the value of 'cid_prefix'.
+ // TODO(chhsiao): We should move this to a utility function.
+ static process::http::authentication::Principal principal(
+ const ResourceProviderInfo& info);
+
virtual ~LocalResourceProvider() = default;
};
diff --git a/src/resource_provider/storage/provider.cpp b/src/resource_provider/storage/provider.cpp
index 18f934b..db783b5 100644
--- a/src/resource_provider/storage/provider.cpp
+++ b/src/resource_provider/storage/provider.cpp
@@ -166,36 +166,21 @@ static bool isValidType(const string& s)
static const Duration CSI_ENDPOINT_CREATION_TIMEOUT = Minutes(1);
-// Returns a prefix for naming standalone containers to run CSI plugins
-// for the resource provider. The prefix is of the following format:
-// <rp_type>-<rp_name>--
-// where <rp_type> and <rp_name> are the type and name of the resource
-// provider, with dots replaced by dashes. We use a double-dash at the
-// end to explicitly mark the end of the prefix.
-static inline string getContainerIdPrefix(const ResourceProviderInfo& info)
-{
- return strings::join(
- "-",
- strings::replace(info.type(), ".", "-"),
- info.name(),
- "-");
-}
-
-
-// Returns the container ID of the standalone container to run a CSI
-// plugin component. The container ID is of the following format:
-// <rp_type>-<rp_name>--<csi_type>-<csi_name>--<list_of_services>
-// where <rp_type> and <rp_name> are the type and name of the resource
-// provider, and <csi_type> and <csi_name> are the type and name of the
-// CSI plugin, with dots replaced by dashes. <list_of_services> lists
-// the CSI services provided by the component, concatenated with dashes.
+// Returns the container ID of the standalone container to run a CSI plugin
+// component. The container ID is of the following format:
+// <cid_prefix><csi_type>-<csi_name>--<list_of_services>
+// where <cid_prefix> comes from the principal of the resource provider,
+// <csi_type> and <csi_name> are the type and name of the CSI plugin, with dots
+// replaced by dashes. <list_of_services> lists the CSI services provided by the
+// component, concatenated with dashes.
static inline ContainerID getContainerId(
const ResourceProviderInfo& info,
const CSIPluginContainerInfo& container)
{
- string value = getContainerIdPrefix(info);
+ const Principal principal = LocalResourceProvider::principal(info);
+ CHECK(principal.claims.contains("cid_prefix"));
- value += strings::join(
+ string value = principal.claims.at("cid_prefix") + strings::join(
"-",
strings::replace(info.storage().plugin().type(), ".", "-"),
info.storage().plugin().name(),
@@ -2101,12 +2086,15 @@ StorageLocalResourceProviderProcess::getContainers()
return Failure("Failed to get containers: " + v1Response.error());
}
- const string prefix = getContainerIdPrefix(info);
+ const Principal principal = LocalResourceProvider::principal(info);
+ CHECK(principal.claims.contains("cid_prefix"));
+
+ const string& cidPrefix = principal.claims.at("cid_prefix");
agent::Response response = devolve(v1Response.get());
foreach (const agent::Response::GetContainers::Container& container,
response.get_containers().containers()) {
- if (strings::startsWith(container.container_id().value(), prefix)) {
+ if (strings::startsWith(container.container_id().value(), cidPrefix)) {
result.put(
container.container_id(),
container.has_container_status()
@@ -3765,15 +3753,6 @@ Try<Owned<LocalResourceProvider>> StorageLocalResourceProvider::create(
}
-Try<Principal> StorageLocalResourceProvider::principal(
- const ResourceProviderInfo& info)
-{
- return Principal(
- Option<string>::none(),
- {{"cid_prefix", getContainerIdPrefix(info)}});
-}
-
-
Option<Error> StorageLocalResourceProvider::validate(
const ResourceProviderInfo& info)
{