You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ji...@apache.org on 2017/12/11 23:31:53 UTC
[1/4] mesos git commit: Added `LocalResourceProviderDaemon` methods
to modify configs.
Repository: mesos
Updated Branches:
refs/heads/master 848767b4f -> adf4fa3f2
Added `LocalResourceProviderDaemon` methods to modify configs.
The `add()` and `update()` methods adds a new config file and updates an
existing config file of a local resource provider respectively. It will
then trigger a rolead on the resource provider asynchronously.
The `remove()` method removes the config file, and triggers the resource
provider to terminate asynchronously.
Review: https://reviews.apache.org/r/64439/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/b9073b83
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/b9073b83
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/b9073b83
Branch: refs/heads/master
Commit: b9073b83d43739b472105a3d9c233dca51cabeea
Parents: 848767b
Author: Chun-Hung Hsiao <ch...@mesosphere.io>
Authored: Mon Dec 11 15:09:35 2017 -0800
Committer: Jie Yu <yu...@gmail.com>
Committed: Mon Dec 11 15:09:35 2017 -0800
----------------------------------------------------------------------
src/resource_provider/daemon.cpp | 277 +++++++++++++++++++++++++++++++---
1 file changed, 252 insertions(+), 25 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/b9073b83/src/resource_provider/daemon.cpp
----------------------------------------------------------------------
diff --git a/src/resource_provider/daemon.cpp b/src/resource_provider/daemon.cpp
index 6159982..7c783e3 100644
--- a/src/resource_provider/daemon.cpp
+++ b/src/resource_provider/daemon.cpp
@@ -63,6 +63,12 @@ using process::http::authentication::Principal;
namespace mesos {
namespace internal {
+// Directory under the resource provider config directory to store
+// temporarily files for adding and updating resource provider config
+// files atomically (all-or-nothing).
+constexpr char STAGING_DIR[] = ".staging";
+
+
class LocalResourceProviderDaemonProcess
: public Process<LocalResourceProviderDaemonProcess>
{
@@ -86,22 +92,41 @@ public:
void start(const SlaveID& _slaveId);
+ Future<bool> add(const ResourceProviderInfo& info);
+ Future<bool> update(const ResourceProviderInfo& info);
+ Future<bool> remove(const string& type, const string& name);
+
protected:
void initialize() override;
private:
struct ProviderData
{
- ProviderData(const ResourceProviderInfo& _info)
- : info(_info) {}
+ ProviderData(const string& _path, const ResourceProviderInfo& _info)
+ : path(_path), info(_info), version(UUID::random()) {}
- const ResourceProviderInfo info;
+ const string path;
+ ResourceProviderInfo info;
+
+ // The `version` is used to check if `provider` holds a resource
+ // provider instance that is in sync with the current config.
+ UUID version;
Owned<LocalResourceProvider> provider;
};
Try<Nothing> load(const string& path);
-
- Future<Nothing> launch(const string& type, const string& name);
+ Try<Nothing> save(const string& path, const ResourceProviderInfo& info);
+
+ // NOTE: `launch` should only be called once for each config version.
+ // It will pick up the latest config to launch the resource provider.
+ Future<Nothing> launch(
+ const string& type,
+ const string& name);
+ Future<Nothing> _launch(
+ const string& type,
+ const string& name,
+ const UUID& version,
+ const Option<string>& authToken);
Future<Option<string>> generateAuthToken(const ResourceProviderInfo& info);
@@ -146,6 +171,121 @@ void LocalResourceProviderDaemonProcess::start(const SlaveID& _slaveId)
}
+Future<bool> LocalResourceProviderDaemonProcess::add(
+ const ResourceProviderInfo& info)
+{
+ if (configDir.isNone()) {
+ return Failure("`--resource_provider_config_dir` must be specified");
+ }
+
+ if (providers[info.type()].contains(info.name())) {
+ return false;
+ }
+
+ // Generate a filename for the the config.
+ // NOTE: We use the following template `<type>.<name>.<uuid>.json`
+ // with a random UUID to generate a new filename to avoid any conflict
+ // with existing ad-hoc config files.
+ const string path = path::join(
+ configDir.get(),
+ strings::join(".", info.type(), info.name(), UUID::random(), "json"));
+
+ LOG(INFO) << "Creating new config file '" << path << "'";
+
+ Try<Nothing> _save = save(path, info);
+ if (_save.isError()) {
+ return Failure(
+ "Failed to write config file '" + path + "': " + _save.error());
+ }
+
+ providers[info.type()].put(info.name(), {path, info});
+
+ // Launch the resource provider if the daemon is already started.
+ if (slaveId.isSome()) {
+ auto err = [](const ResourceProviderInfo& info, const string& message) {
+ LOG(ERROR)
+ << "Failed to launch resource provider with type '" << info.type()
+ << "' and name '" << info.name() << "': " << message;
+ };
+
+ launch(info.type(), info.name())
+ .onFailed(std::bind(err, info, lambda::_1))
+ .onDiscarded(std::bind(err, info, "future discarded"));
+ }
+
+ return true;
+}
+
+
+Future<bool> LocalResourceProviderDaemonProcess::update(
+ const ResourceProviderInfo& info)
+{
+ if (configDir.isNone()) {
+ return Failure("`--resource_provider_config_dir` must be specified");
+ }
+
+ if (!providers[info.type()].contains(info.name())) {
+ return false;
+ }
+
+ ProviderData& data = providers[info.type()].at(info.name());
+
+ Try<Nothing> _save = save(data.path, info);
+ if (_save.isError()) {
+ return Failure(
+ "Failed to write config file '" + data.path + "': " + _save.error());
+ }
+
+ data.info = info;
+
+ // Update `version` to indicate that the config has been updated.
+ data.version = UUID::random();
+
+ // Launch the resource provider if the daemon is already started.
+ if (slaveId.isSome()) {
+ auto err = [](const ResourceProviderInfo& info, const string& message) {
+ LOG(ERROR)
+ << "Failed to launch resource provider with type '" << info.type()
+ << "' and name '" << info.name() << "': " << message;
+ };
+
+ launch(info.type(), info.name())
+ .onFailed(std::bind(err, info, lambda::_1))
+ .onDiscarded(std::bind(err, info, "future discarded"));
+ }
+
+ return true;
+}
+
+
+Future<bool> LocalResourceProviderDaemonProcess::remove(
+ const string& type,
+ const string& name)
+{
+ if (configDir.isNone()) {
+ return Failure("`--resource_provider_config_dir` must be specified");
+ }
+
+ if (!providers[type].contains(name)) {
+ return false;
+ }
+
+ const string path = providers[type].at(name).path;
+
+ Try<Nothing> rm = os::rm(path);
+ if (rm.isError()) {
+ return Failure(
+ "Failed to remove config file '" + path + "': " + rm.error());
+ }
+
+ // Removing the provider data from `providers` will cause the resource
+ // provider to be destructed.
+ providers[type].erase(name);
+
+ return true;
+}
+
+
void LocalResourceProviderDaemonProcess::initialize()
{
if (configDir.isNone()) {
@@ -162,6 +302,8 @@ void LocalResourceProviderDaemonProcess::initialize()
foreach (const string& entry, entries.get()) {
const string path = path::join(configDir.get(), entry);
+ // Skip subdirectories in the resource provider config directory,
+ // including the staging directory.
if (os::stat::isdir(path)) {
continue;
}
@@ -201,7 +343,54 @@ Try<Nothing> LocalResourceProviderDaemonProcess::load(const string& path)
"' and name '" + info->name() + "'");
}
- providers[info->type()].put(info->name(), info.get());
+ providers[info->type()].put(info->name(), {path, std::move(info.get())});
+
+ return Nothing();
+}
+
+
+// NOTE: We provide atomic (all-or-nothing) semantics here by always
+// writing to a temporary file to the staging directory first then using
+// `os::rename` to atomically move it to the desired path.
+Try<Nothing> LocalResourceProviderDaemonProcess::save(
+ const string& path,
+ const ResourceProviderInfo& info)
+{
+ CHECK_SOME(configDir);
+
+ // NOTE: We create the staging direcotry in the resource provider
+ // config directory to make sure that the renaming below does not
+ // cross devices (MESOS-2319).
+ // TODO(chhsiao): Consider adding a way to garbage collect the staging
+ // directory.
+ const string stagingDir = path::join(configDir.get(), STAGING_DIR);
+ Try<Nothing> mkdir = os::mkdir(stagingDir);
+ if (mkdir.isError()) {
+ return Error(
+ "Failed to create directory '" + stagingDir + "': " + mkdir.error());
+ }
+
+ const string stagingPath = path::join(stagingDir, Path(path).basename());
+
+ Try<Nothing> write = os::write(stagingPath, stringify(JSON::protobuf(info)));
+ if (write.isError()) {
+ // Try to remove the temporary file on error.
+ os::rm(stagingPath);
+
+ return Error(
+ "Failed to write temporary file '" + stagingPath + "': " +
+ write.error());
+ }
+
+ Try<Nothing> rename = os::rename(stagingPath, path);
+ if (rename.isError()) {
+ // Try to remove the temporary file on error.
+ os::rm(stagingPath);
+
+ return Error(
+ "Failed to rename '" + stagingPath + "' to '" + path + "': " +
+ rename.error());
+ }
return Nothing();
}
@@ -212,27 +401,55 @@ Future<Nothing> LocalResourceProviderDaemonProcess::launch(
const string& name)
{
CHECK_SOME(slaveId);
- CHECK(providers[type].contains(name));
- return generateAuthToken(providers[type].at(name).info)
- .then(defer(self(), [=](
- const Option<string>& authToken) -> Future<Nothing> {
- ProviderData& data = providers[type].at(name);
+ // If the resource provider config is removed, nothing needs to be done.
+ if (!providers[type].contains(name)) {
+ return Nothing();
+ }
+
+ ProviderData& data = providers[type].at(name);
- Try<Owned<LocalResourceProvider>> provider =
- LocalResourceProvider::create(
- url, workDir, data.info, slaveId.get(), authToken);
+ // Destruct the previous resource provider (which will synchronously
+ // terminate its actor and driver) if there is one.
+ data.provider.reset();
- if (provider.isError()) {
- return Failure(
- "Failed to create resource provider with type '" + type +
- "' and name '" + name + "': " + provider.error());
- }
+ return generateAuthToken(data.info)
+ .then(defer(self(), &Self::_launch, type, name, data.version, lambda::_1));
+}
- data.provider = provider.get();
- return Nothing();
- }));
+Future<Nothing> LocalResourceProviderDaemonProcess::_launch(
+ const string& type,
+ const string& name,
+ const UUID& version,
+ const Option<string>& authToken)
+{
+ // If the resource provider config is removed, abort the launch sequence.
+ if (!providers[type].contains(name)) {
+ return Nothing();
+ }
+
+ ProviderData& data = providers[type].at(name);
+
+ // If there is a version mismatch, abort the launch sequence since
+ // `authToken` might be outdated. The callback updating the version
+ // should have dispatched another launch sequence.
+ if (version != data.version) {
+ return Nothing();
+ }
+
+ Try<Owned<LocalResourceProvider>> provider = LocalResourceProvider::create(
+ url, workDir, data.info, slaveId.get(), authToken);
+
+ if (provider.isError()) {
+ return Failure(
+ "Failed to create resource provider with type '" + type +
+ "' and name '" + name + "': " + provider.error());
+ }
+
+ data.provider = provider.get();
+
+ return Nothing();
}
@@ -323,14 +540,20 @@ void LocalResourceProviderDaemon::start(const SlaveID& slaveId)
Future<bool> LocalResourceProviderDaemon::add(const ResourceProviderInfo& info)
{
- return Failure("Unimplemented");
+ return dispatch(
+ process.get(),
+ &LocalResourceProviderDaemonProcess::add,
+ info);
}
Future<bool> LocalResourceProviderDaemon::update(
const ResourceProviderInfo& info)
{
- return Failure("Unimplemented");
+ return dispatch(
+ process.get(),
+ &LocalResourceProviderDaemonProcess::update,
+ info);
}
@@ -338,7 +561,11 @@ Future<bool> LocalResourceProviderDaemon::remove(
const string& type,
const string& name)
{
- return Failure("Unimplemented");
+ return dispatch(
+ process.get(),
+ &LocalResourceProviderDaemonProcess::remove,
+ type,
+ name);
}
} // namespace internal {
[4/4] mesos git commit: Fixed a typo in resource provider config API
and added validation tests.
Posted by ji...@apache.org.
Fixed a typo in resource provider config API and added validation tests.
Review: https://reviews.apache.org/r/64495/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/adf4fa3f
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/adf4fa3f
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/adf4fa3f
Branch: refs/heads/master
Commit: adf4fa3f267666b7dedfcc786a222fc57e1dfb82
Parents: 24d17b2
Author: Chun-Hung Hsiao <ch...@mesosphere.io>
Authored: Mon Dec 11 15:09:46 2017 -0800
Committer: Jie Yu <yu...@gmail.com>
Committed: Mon Dec 11 15:09:46 2017 -0800
----------------------------------------------------------------------
include/mesos/agent/agent.proto | 2 +-
include/mesos/v1/agent/agent.proto | 2 +-
src/tests/slave_validation_tests.cpp | 70 +++++++++++++++++++++++++++++++
3 files changed, 72 insertions(+), 2 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/adf4fa3f/include/mesos/agent/agent.proto
----------------------------------------------------------------------
diff --git a/include/mesos/agent/agent.proto b/include/mesos/agent/agent.proto
index d464afe..6fcca6a 100644
--- a/include/mesos/agent/agent.proto
+++ b/include/mesos/agent/agent.proto
@@ -363,7 +363,7 @@ message Call {
optional KillContainer kill_container = 15;
optional RemoveContainer remove_container = 16;
- optional UpdateResourceProviderConfig add_resource_provider_config = 17;
+ optional AddResourceProviderConfig add_resource_provider_config = 17;
optional UpdateResourceProviderConfig update_resource_provider_config = 18;
optional RemoveResourceProviderConfig remove_resource_provider_config = 19;
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/adf4fa3f/include/mesos/v1/agent/agent.proto
----------------------------------------------------------------------
diff --git a/include/mesos/v1/agent/agent.proto b/include/mesos/v1/agent/agent.proto
index 91fe5e1..57c3518 100644
--- a/include/mesos/v1/agent/agent.proto
+++ b/include/mesos/v1/agent/agent.proto
@@ -363,7 +363,7 @@ message Call {
optional KillContainer kill_container = 15;
optional RemoveContainer remove_container = 16;
- optional UpdateResourceProviderConfig add_resource_provider_config = 17;
+ optional AddResourceProviderConfig add_resource_provider_config = 17;
optional UpdateResourceProviderConfig update_resource_provider_config = 18;
optional RemoveResourceProviderConfig remove_resource_provider_config = 19;
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/adf4fa3f/src/tests/slave_validation_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/slave_validation_tests.cpp b/src/tests/slave_validation_tests.cpp
index e81b65b..d6e0931 100644
--- a/src/tests/slave_validation_tests.cpp
+++ b/src/tests/slave_validation_tests.cpp
@@ -500,6 +500,76 @@ TEST(AgentCallValidationTest, LaunchNestedContainerSession)
EXPECT_NONE(error);
}
+
+TEST(AgentCallValidationTest, AddResourceProviderConfig)
+{
+ // Expecting `add_resource_provider_config`.
+ agent::Call call;
+ call.set_type(agent::Call::ADD_RESOURCE_PROVIDER_CONFIG);
+
+ Option<Error> error = validation::agent::call::validate(call);
+ EXPECT_SOME(error);
+
+ // Expecting `info.id` to be unset.
+ ResourceProviderInfo* info =
+ call.mutable_add_resource_provider_config()->mutable_info();
+ info->set_type("org.apache.mesos.rp.type");
+ info->set_name("name");
+ info->mutable_id()->set_value("id");
+
+ error = validation::agent::call::validate(call);
+ EXPECT_SOME(error);
+
+ info->clear_id();
+
+ error = validation::agent::call::validate(call);
+ EXPECT_NONE(error);
+}
+
+
+TEST(AgentCallValidationTest, UpdateResourceProviderConfig)
+{
+ // Expecting `update_resource_provider_config`.
+ agent::Call call;
+ call.set_type(agent::Call::UPDATE_RESOURCE_PROVIDER_CONFIG);
+
+ Option<Error> error = validation::agent::call::validate(call);
+ EXPECT_SOME(error);
+
+ // Expecting `info.id` to be unset.
+ ResourceProviderInfo* info =
+ call.mutable_update_resource_provider_config()->mutable_info();
+ info->set_type("org.apache.mesos.rp.type");
+ info->set_name("name");
+ info->mutable_id()->set_value("id");
+
+ error = validation::agent::call::validate(call);
+ EXPECT_SOME(error);
+
+ info->clear_id();
+
+ error = validation::agent::call::validate(call);
+ EXPECT_NONE(error);
+}
+
+
+TEST(AgentCallValidationTest, RemoveResourceProviderConfig)
+{
+ // Expecting `remove_resource_provider_config`.
+ agent::Call call;
+ call.set_type(agent::Call::REMOVE_RESOURCE_PROVIDER_CONFIG);
+
+ Option<Error> error = validation::agent::call::validate(call);
+ EXPECT_SOME(error);
+
+ call.mutable_remove_resource_provider_config()
+ ->set_type("org.apache.mesos.rp.type");
+ call.mutable_remove_resource_provider_config()->set_name("name");
+
+ error = validation::agent::call::validate(call);
+ EXPECT_NONE(error);
+}
+
} // namespace tests {
} // namespace internal {
} // namespace mesos {
[2/4] mesos git commit: Added unit tests for resource provider config
modification API.
Posted by ji...@apache.org.
Added unit tests for resource provider config modification API.
Review: https://reviews.apache.org/r/64469/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/2c606ae6
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/2c606ae6
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/2c606ae6
Branch: refs/heads/master
Commit: 2c606ae6dde368089adb755eb9413af3967fffec
Parents: b9073b8
Author: Chun-Hung Hsiao <ch...@mesosphere.io>
Authored: Mon Dec 11 15:09:38 2017 -0800
Committer: Jie Yu <yu...@gmail.com>
Committed: Mon Dec 11 15:09:38 2017 -0800
----------------------------------------------------------------------
include/mesos/type_utils.hpp | 8 +
include/mesos/v1/mesos.hpp | 8 +
src/Makefile.am | 1 +
src/resource_provider/manager.cpp | 6 +
...agent_resource_provider_config_api_tests.cpp | 670 +++++++++++++++++++
5 files changed, 693 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/2c606ae6/include/mesos/type_utils.hpp
----------------------------------------------------------------------
diff --git a/include/mesos/type_utils.hpp b/include/mesos/type_utils.hpp
index 506f667..1bcc521 100644
--- a/include/mesos/type_utils.hpp
+++ b/include/mesos/type_utils.hpp
@@ -286,6 +286,14 @@ inline bool operator!=(const SlaveID& left, const SlaveID& right)
}
+inline bool operator!=(
+ const ResourceProviderInfo& left,
+ const ResourceProviderInfo& right)
+{
+ return !(left == right);
+}
+
+
inline bool operator!=(const TimeInfo& left, const TimeInfo& right)
{
return !(left == right);
http://git-wip-us.apache.org/repos/asf/mesos/blob/2c606ae6/include/mesos/v1/mesos.hpp
----------------------------------------------------------------------
diff --git a/include/mesos/v1/mesos.hpp b/include/mesos/v1/mesos.hpp
index f393ed5..d163f0b 100644
--- a/include/mesos/v1/mesos.hpp
+++ b/include/mesos/v1/mesos.hpp
@@ -271,6 +271,14 @@ inline bool operator!=(
}
+inline bool operator!=(
+ const ResourceProviderInfo& left,
+ const ResourceProviderInfo& right)
+{
+ return !(left == right);
+}
+
+
inline bool operator!=(const AgentID& left, const AgentID& right)
{
return left.value() != right.value();
http://git-wip-us.apache.org/repos/asf/mesos/blob/2c606ae6/src/Makefile.am
----------------------------------------------------------------------
diff --git a/src/Makefile.am b/src/Makefile.am
index 47f4528..f5a4edd 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -2621,6 +2621,7 @@ mesos_tests_SOURCES += \
if OS_LINUX
mesos_tests_SOURCES += \
+ tests/agent_resource_provider_config_api_tests.cpp \
tests/storage_local_resource_provider_tests.cpp
endif
endif
http://git-wip-us.apache.org/repos/asf/mesos/blob/2c606ae6/src/resource_provider/manager.cpp
----------------------------------------------------------------------
diff --git a/src/resource_provider/manager.cpp b/src/resource_provider/manager.cpp
index bfc917f..fd138b9 100644
--- a/src/resource_provider/manager.cpp
+++ b/src/resource_provider/manager.cpp
@@ -131,6 +131,8 @@ struct ResourceProvider
~ResourceProvider()
{
+ LOG(INFO) << "Terminating resource provider " << info.id();
+
http.close();
foreachvalue (const Owned<Promise<Nothing>>& publish, publishes) {
@@ -661,6 +663,10 @@ void ResourceProviderManagerProcess::updateState(
offerOperations.put(uuid.get(), operation);
}
+ LOG(INFO)
+ << "Received UPDATE_STATE call with resources '" << update.resources()
+ << "' from resource provider " << resourceProvider->info.id();
+
ResourceProviderMessage::UpdateState updateState{
resourceProvider->info,
resourceVersion.get(),
http://git-wip-us.apache.org/repos/asf/mesos/blob/2c606ae6/src/tests/agent_resource_provider_config_api_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/agent_resource_provider_config_api_tests.cpp b/src/tests/agent_resource_provider_config_api_tests.cpp
new file mode 100644
index 0000000..5a50e82
--- /dev/null
+++ b/src/tests/agent_resource_provider_config_api_tests.cpp
@@ -0,0 +1,670 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include <mesos/type_utils.hpp>
+
+#include <process/gtest.hpp>
+#include <process/gmock.hpp>
+
+#include <stout/fs.hpp>
+#include <stout/json.hpp>
+#include <stout/protobuf.hpp>
+#include <stout/stringify.hpp>
+
+#include "common/http.hpp"
+
+#include "internal/evolve.hpp"
+
+#include "slave/slave.hpp"
+
+#include "tests/flags.hpp"
+#include "tests/mesos.hpp"
+
+namespace http = process::http;
+
+using std::list;
+using std::string;
+using std::vector;
+
+using mesos::internal::slave::Slave;
+
+using mesos::master::detector::MasterDetector;
+
+using process::Future;
+using process::Owned;
+using process::PID;
+
+using testing::Values;
+using testing::WithParamInterface;
+
+namespace mesos {
+namespace internal {
+namespace tests {
+
+class AgentResourceProviderConfigApiTest
+ : public MesosTest,
+ public WithParamInterface<ContentType>
+{
+public:
+ virtual void SetUp()
+ {
+ MesosTest::SetUp();
+
+ resourceProviderConfigDir =
+ path::join(sandbox.get(), "resource_provider_configs");
+
+ ASSERT_SOME(os::mkdir(resourceProviderConfigDir));
+ }
+
+ ResourceProviderInfo createResourceProviderInfo(const Bytes& capacity)
+ {
+ const string testCsiPluginWorkDir = path::join(sandbox.get(), "test");
+ CHECK_SOME(os::mkdir(testCsiPluginWorkDir));
+
+ string testCsiPluginPath =
+ path::join(tests::flags.build_dir, "src", "test-csi-plugin");
+
+ Try<string> resourceProviderConfig = strings::format(
+ R"~(
+ {
+ "type": "org.apache.mesos.rp.local.storage",
+ "name": "test",
+ "default_reservations": [
+ {
+ "type": "DYNAMIC",
+ "role": "storage"
+ }
+ ],
+ "storage": {
+ "plugin": {
+ "type": "org.apache.mesos.csi.test",
+ "name": "slrp_test",
+ "containers": [
+ {
+ "services": [
+ "CONTROLLER_SERVICE",
+ "NODE_SERVICE"
+ ],
+ "command": {
+ "shell": false,
+ "value": "%s",
+ "arguments": [
+ "%s",
+ "--total_capacity=%s",
+ "--work_dir=%s"
+ ]
+ }
+ }
+ ]
+ }
+ }
+ }
+ )~",
+ testCsiPluginPath,
+ testCsiPluginPath,
+ stringify(capacity),
+ testCsiPluginWorkDir);
+
+ CHECK_SOME(resourceProviderConfig);
+
+ Try<JSON::Object> json =
+ JSON::parse<JSON::Object>(resourceProviderConfig.get());
+ CHECK_SOME(json);
+
+ Try<ResourceProviderInfo> info =
+ ::protobuf::parse<ResourceProviderInfo>(json.get());
+ CHECK_SOME(info);
+
+ return info.get();
+ }
+
+ Future<http::Response> addResourceProviderConfig(
+ const PID<Slave>& pid,
+ const ContentType& contentType,
+ const ResourceProviderInfo& info)
+ {
+ http::Headers headers = createBasicAuthHeaders(DEFAULT_CREDENTIAL);
+ headers["Accept"] = stringify(contentType);
+
+ agent::Call call;
+ call.set_type(agent::Call::ADD_RESOURCE_PROVIDER_CONFIG);
+ call.mutable_add_resource_provider_config()
+ ->mutable_info()->CopyFrom(info);
+
+ return http::post(
+ pid,
+ "api/v1",
+ headers,
+ serialize(contentType, evolve(call)),
+ stringify(contentType));
+ }
+
+ Future<http::Response> updateResourceProviderConfig(
+ const PID<Slave>& pid,
+ const ContentType& contentType,
+ const ResourceProviderInfo& info)
+ {
+ http::Headers headers = createBasicAuthHeaders(DEFAULT_CREDENTIAL);
+ headers["Accept"] = stringify(contentType);
+
+ agent::Call call;
+ call.set_type(agent::Call::UPDATE_RESOURCE_PROVIDER_CONFIG);
+ call.mutable_update_resource_provider_config()
+ ->mutable_info()->CopyFrom(info);
+
+ return http::post(
+ pid,
+ "api/v1",
+ headers,
+ serialize(contentType, evolve(call)),
+ stringify(contentType));
+ }
+
+ Future<http::Response> removeResourceProviderConfig(
+ const PID<Slave>& pid,
+ const ContentType& contentType,
+ const string& type,
+ const string& name)
+ {
+ http::Headers headers = createBasicAuthHeaders(DEFAULT_CREDENTIAL);
+ headers["Accept"] = stringify(contentType);
+
+ agent::Call call;
+ call.set_type(agent::Call::REMOVE_RESOURCE_PROVIDER_CONFIG);
+ call.mutable_remove_resource_provider_config()->set_type(type);
+ call.mutable_remove_resource_provider_config()->set_name(name);
+
+ return http::post(
+ pid,
+ "api/v1",
+ headers,
+ serialize(contentType, evolve(call)),
+ stringify(contentType));
+ }
+
+protected:
+ string resourceProviderConfigDir;
+};
+
+
+// The tests are parameterized by the content type of the request.
+INSTANTIATE_TEST_CASE_P(
+ ContentType,
+ AgentResourceProviderConfigApiTest,
+ Values(ContentType::PROTOBUF, ContentType::JSON));
+
+
+// This test adds a new resource provider config on the fly.
+TEST_P(AgentResourceProviderConfigApiTest, ROOT_Add)
+{
+ const ContentType contentType = GetParam();
+
+ Try<Owned<cluster::Master>> master = StartMaster();
+ ASSERT_SOME(master);
+
+ Owned<MasterDetector> detector = master.get()->createDetector();
+
+ slave::Flags flags = CreateSlaveFlags();
+ flags.isolation = "filesystem/linux";
+
+ // Disable HTTP authentication to simplify resource provider interactions.
+ flags.authenticate_http_readwrite = false;
+
+ // Set the resource provider capability and other required capabilities.
+ constexpr SlaveInfo::Capability::Type capabilities[] = {
+ SlaveInfo::Capability::MULTI_ROLE,
+ SlaveInfo::Capability::HIERARCHICAL_ROLE,
+ SlaveInfo::Capability::RESERVATION_REFINEMENT,
+ SlaveInfo::Capability::RESOURCE_PROVIDER
+ };
+
+ flags.agent_features = SlaveCapabilities();
+ foreach (SlaveInfo::Capability::Type type, capabilities) {
+ flags.agent_features->add_capabilities()->set_type(type);
+ }
+
+ flags.resource_provider_config_dir = resourceProviderConfigDir;
+
+ Future<SlaveRegisteredMessage> slaveRegisteredMessage =
+ FUTURE_PROTOBUF(SlaveRegisteredMessage(), _, _);
+
+ Try<Owned<cluster::Slave>> slave = StartSlave(detector.get(), flags);
+ ASSERT_SOME(slave);
+
+ AWAIT_READY(slaveRegisteredMessage);
+
+ // Register a framework to wait for an offer having the provider
+ // resource.
+ FrameworkInfo framework = DEFAULT_FRAMEWORK_INFO;
+ framework.set_roles(0, "storage");
+
+ MockScheduler sched;
+ MesosSchedulerDriver driver(
+ &sched, framework, master.get()->pid, DEFAULT_CREDENTIAL);
+
+ EXPECT_CALL(sched, registered(&driver, _, _));
+
+ Future<vector<Offer>> offers;
+
+ // Decline offers that contain only the agent's default resources.
+ EXPECT_CALL(sched, resourceOffers(&driver, _))
+ .WillRepeatedly(DeclineOffers());
+
+ EXPECT_CALL(sched, resourceOffers(&driver, OffersHaveAnyResource(
+ std::bind(&Resources::hasResourceProvider, lambda::_1))))
+ .WillOnce(FutureArg<1>(&offers));
+
+ driver.start();
+
+ // Add a new resource provider.
+ ResourceProviderInfo info = createResourceProviderInfo(Gigabytes(4));
+
+ AWAIT_EXPECT_RESPONSE_STATUS_EQ(
+ http::OK().status,
+ addResourceProviderConfig(slave.get()->pid, contentType, info));
+
+ // Check that a new config file is created.
+ Try<list<string>> configPaths =
+ fs::list(path::join(resourceProviderConfigDir, "*"));
+ ASSERT_SOME(configPaths);
+ EXPECT_EQ(1u, configPaths->size());
+
+ Try<string> read = os::read(configPaths->back());
+ ASSERT_SOME(read);
+
+ Try<JSON::Object> json = JSON::parse<JSON::Object>(read.get());
+ ASSERT_SOME(json);
+
+ Try<ResourceProviderInfo> _info =
+ ::protobuf::parse<ResourceProviderInfo>(json.get());
+ ASSERT_SOME(_info);
+ EXPECT_EQ(_info.get(), info);
+
+ // Wait for an offer having the provider resource.
+ AWAIT_READY(offers);
+}
+
+
+// This test checks that adding a resource provider config that already
+// exists is not allowed.
+TEST_P(AgentResourceProviderConfigApiTest, ROOT_AddConflict)
+{
+ const ContentType contentType = GetParam();
+
+ Try<Owned<cluster::Master>> master = StartMaster();
+ ASSERT_SOME(master);
+
+ Owned<MasterDetector> detector = master.get()->createDetector();
+
+ slave::Flags flags = CreateSlaveFlags();
+ flags.isolation = "filesystem/linux";
+
+ // Disable HTTP authentication to simplify resource provider interactions.
+ flags.authenticate_http_readwrite = false;
+
+ // Set the resource provider capability and other required capabilities.
+ constexpr SlaveInfo::Capability::Type capabilities[] = {
+ SlaveInfo::Capability::MULTI_ROLE,
+ SlaveInfo::Capability::HIERARCHICAL_ROLE,
+ SlaveInfo::Capability::RESERVATION_REFINEMENT,
+ SlaveInfo::Capability::RESOURCE_PROVIDER
+ };
+
+ flags.agent_features = SlaveCapabilities();
+ foreach (SlaveInfo::Capability::Type type, capabilities) {
+ flags.agent_features->add_capabilities()->set_type(type);
+ }
+
+ flags.resource_provider_config_dir = resourceProviderConfigDir;
+
+ // Generate a pre-existing config.
+ const string configPath = path::join(resourceProviderConfigDir, "test.json");
+ ASSERT_SOME(os::write(
+ configPath,
+ stringify(JSON::protobuf(createResourceProviderInfo(Gigabytes(4))))));
+
+ Future<SlaveRegisteredMessage> slaveRegisteredMessage =
+ FUTURE_PROTOBUF(SlaveRegisteredMessage(), _, _);
+
+ Try<Owned<cluster::Slave>> slave = StartSlave(detector.get(), flags);
+ ASSERT_SOME(slave);
+
+ AWAIT_READY(slaveRegisteredMessage);
+
+ ResourceProviderInfo info = createResourceProviderInfo(Gigabytes(2));
+
+ AWAIT_EXPECT_RESPONSE_STATUS_EQ(
+ http::Conflict().status,
+ addResourceProviderConfig(slave.get()->pid, contentType, info));
+
+ // Check that no new config is created, and the existing one is not
+ // overwritten.
+ Try<list<string>> configPaths =
+ fs::list(path::join(resourceProviderConfigDir, "*"));
+ ASSERT_SOME(configPaths);
+ EXPECT_EQ(1u, configPaths->size());
+ EXPECT_EQ(configPath, configPaths->back());
+
+ Try<string> read = os::read(configPath);
+ ASSERT_SOME(read);
+
+ Try<JSON::Object> json = JSON::parse<JSON::Object>(read.get());
+ ASSERT_SOME(json);
+
+ Try<ResourceProviderInfo> _info =
+ ::protobuf::parse<ResourceProviderInfo>(json.get());
+ ASSERT_SOME(_info);
+ EXPECT_NE(_info.get(), info);
+}
+
+
+// This test updates an existing resource provider config on the fly.
+TEST_P(AgentResourceProviderConfigApiTest, ROOT_Update)
+{
+ const ContentType contentType = GetParam();
+
+ Try<Owned<cluster::Master>> master = StartMaster();
+ ASSERT_SOME(master);
+
+ Owned<MasterDetector> detector = master.get()->createDetector();
+
+ slave::Flags flags = CreateSlaveFlags();
+ flags.isolation = "filesystem/linux";
+
+ // Disable HTTP authentication to simplify resource provider interactions.
+ flags.authenticate_http_readwrite = false;
+
+ // Set the resource provider capability and other required capabilities.
+ constexpr SlaveInfo::Capability::Type capabilities[] = {
+ SlaveInfo::Capability::MULTI_ROLE,
+ SlaveInfo::Capability::HIERARCHICAL_ROLE,
+ SlaveInfo::Capability::RESERVATION_REFINEMENT,
+ SlaveInfo::Capability::RESOURCE_PROVIDER
+ };
+
+ flags.agent_features = SlaveCapabilities();
+ foreach (SlaveInfo::Capability::Type type, capabilities) {
+ flags.agent_features->add_capabilities()->set_type(type);
+ }
+
+ flags.resource_provider_config_dir = resourceProviderConfigDir;
+
+ // Generate a pre-existing config.
+ const string configPath = path::join(resourceProviderConfigDir, "test.json");
+ ASSERT_SOME(os::write(
+ configPath,
+ stringify(JSON::protobuf(createResourceProviderInfo(Gigabytes(4))))));
+
+ Future<SlaveRegisteredMessage> slaveRegisteredMessage =
+ FUTURE_PROTOBUF(SlaveRegisteredMessage(), _, _);
+
+ Try<Owned<cluster::Slave>> slave = StartSlave(detector.get(), flags);
+ ASSERT_SOME(slave);
+
+ AWAIT_READY(slaveRegisteredMessage);
+
+ // Register a framework to wait for an offer having the provider
+ // resource.
+ FrameworkInfo framework = DEFAULT_FRAMEWORK_INFO;
+ framework.set_roles(0, "storage");
+
+ MockScheduler sched;
+ MesosSchedulerDriver driver(
+ &sched, framework, master.get()->pid, DEFAULT_CREDENTIAL);
+
+ EXPECT_CALL(sched, registered(&driver, _, _));
+
+ Future<vector<Offer>> oldOffers;
+ Future<vector<Offer>> newOffers;
+
+ // Decline offers that contain only the agent's default resources.
+ EXPECT_CALL(sched, resourceOffers(&driver, _))
+ .WillRepeatedly(DeclineOffers());
+
+ EXPECT_CALL(sched, resourceOffers(&driver, OffersHaveAnyResource(
+ std::bind(&Resources::hasResourceProvider, lambda::_1))))
+ .WillOnce(FutureArg<1>(&oldOffers))
+ .WillOnce(FutureArg<1>(&newOffers));
+
+ Future<OfferID> rescinded;
+
+ EXPECT_CALL(sched, offerRescinded(&driver, _))
+ .WillOnce(FutureArg<1>(&rescinded));
+
+ driver.start();
+
+ // Wait for an offer having the old provider resource.
+ AWAIT_READY(oldOffers);
+
+ ResourceProviderInfo info = createResourceProviderInfo(Gigabytes(2));
+
+ AWAIT_EXPECT_RESPONSE_STATUS_EQ(
+ http::OK().status,
+ updateResourceProviderConfig(slave.get()->pid, contentType, info));
+
+ // Check that no new config is created, and the existing one is overwritten.
+ Try<list<string>> configPaths =
+ fs::list(path::join(resourceProviderConfigDir, "*"));
+ ASSERT_SOME(configPaths);
+ EXPECT_EQ(1u, configPaths->size());
+ EXPECT_EQ(configPath, configPaths->back());
+
+ Try<string> read = os::read(configPath);
+ ASSERT_SOME(read);
+
+ Try<JSON::Object> json = JSON::parse<JSON::Object>(read.get());
+ ASSERT_SOME(json);
+
+ Try<ResourceProviderInfo> _info =
+ ::protobuf::parse<ResourceProviderInfo>(json.get());
+ ASSERT_SOME(_info);
+ EXPECT_EQ(_info.get(), info);
+
+ // Wait for the old offer to be rescinded.
+ AWAIT_READY(rescinded);
+
+ // Wait for an offer having the new provider resource.
+ AWAIT_READY(newOffers);
+
+ // The new provider resource is smaller than the old provider resource.
+ EXPECT_FALSE(Resources(newOffers->at(0).resources()).contains(
+ oldOffers->at(0).resources()));
+}
+
+
+// This test checks that updating a nonexistent resource provider config
+// is not allowed.
+TEST_P(AgentResourceProviderConfigApiTest, UpdateNotFound)
+{
+ const ContentType contentType = GetParam();
+
+ Try<Owned<cluster::Master>> master = StartMaster();
+ ASSERT_SOME(master);
+
+ Owned<MasterDetector> detector = master.get()->createDetector();
+
+ slave::Flags flags = CreateSlaveFlags();
+
+ // Set the resource provider capability and other required capabilities.
+ constexpr SlaveInfo::Capability::Type capabilities[] = {
+ SlaveInfo::Capability::MULTI_ROLE,
+ SlaveInfo::Capability::HIERARCHICAL_ROLE,
+ SlaveInfo::Capability::RESERVATION_REFINEMENT,
+ SlaveInfo::Capability::RESOURCE_PROVIDER
+ };
+
+ flags.agent_features = SlaveCapabilities();
+ foreach (SlaveInfo::Capability::Type type, capabilities) {
+ flags.agent_features->add_capabilities()->set_type(type);
+ }
+
+ flags.resource_provider_config_dir = resourceProviderConfigDir;
+
+ Future<SlaveRegisteredMessage> slaveRegisteredMessage =
+ FUTURE_PROTOBUF(SlaveRegisteredMessage(), _, _);
+
+ Try<Owned<cluster::Slave>> slave = StartSlave(detector.get(), flags);
+ ASSERT_SOME(slave);
+
+ AWAIT_READY(slaveRegisteredMessage);
+
+ ResourceProviderInfo info = createResourceProviderInfo(Gigabytes(4));
+
+ AWAIT_EXPECT_RESPONSE_STATUS_EQ(
+ http::NotFound().status,
+ updateResourceProviderConfig(slave.get()->pid, contentType, info));
+
+ // Check that no new config is created.
+ Try<list<string>> configPaths =
+ fs::list(path::join(resourceProviderConfigDir, "*"));
+ ASSERT_SOME(configPaths);
+ EXPECT_TRUE(configPaths->empty());
+}
+
+
+// This test removes an existing resource provider config on the fly.
+TEST_P(AgentResourceProviderConfigApiTest, ROOT_Remove)
+{
+ const ContentType contentType = GetParam();
+
+ Try<Owned<cluster::Master>> master = StartMaster();
+ ASSERT_SOME(master);
+
+ Owned<MasterDetector> detector = master.get()->createDetector();
+
+ slave::Flags flags = CreateSlaveFlags();
+ flags.isolation = "filesystem/linux";
+
+ // Disable HTTP authentication to simplify resource provider interactions.
+ flags.authenticate_http_readwrite = false;
+
+ // Set the resource provider capability and other required capabilities.
+ constexpr SlaveInfo::Capability::Type capabilities[] = {
+ SlaveInfo::Capability::MULTI_ROLE,
+ SlaveInfo::Capability::HIERARCHICAL_ROLE,
+ SlaveInfo::Capability::RESERVATION_REFINEMENT,
+ SlaveInfo::Capability::RESOURCE_PROVIDER
+ };
+
+ flags.agent_features = SlaveCapabilities();
+ foreach (SlaveInfo::Capability::Type type, capabilities) {
+ flags.agent_features->add_capabilities()->set_type(type);
+ }
+
+ flags.resource_provider_config_dir = resourceProviderConfigDir;
+
+ // Generate a pre-existing config.
+ const string configPath = path::join(resourceProviderConfigDir, "test.json");
+ ResourceProviderInfo info = createResourceProviderInfo(Gigabytes(4));
+ ASSERT_SOME(os::write(configPath, stringify(JSON::protobuf(info))));
+
+ Future<SlaveRegisteredMessage> slaveRegisteredMessage =
+ FUTURE_PROTOBUF(SlaveRegisteredMessage(), _, _);
+
+ Try<Owned<cluster::Slave>> slave = StartSlave(detector.get(), flags);
+ ASSERT_SOME(slave);
+
+ AWAIT_READY(slaveRegisteredMessage);
+
+ // Register a framework to wait for an offer having the provider
+ // resource.
+ FrameworkInfo framework = DEFAULT_FRAMEWORK_INFO;
+ framework.set_roles(0, "storage");
+
+ MockScheduler sched;
+ MesosSchedulerDriver driver(
+ &sched, framework, master.get()->pid, DEFAULT_CREDENTIAL);
+
+ EXPECT_CALL(sched, registered(&driver, _, _));
+
+ Future<vector<Offer>> oldOffers;
+
+ // Decline offers that contain only the agent's default resources.
+ EXPECT_CALL(sched, resourceOffers(&driver, _))
+ .WillRepeatedly(DeclineOffers());
+
+ EXPECT_CALL(sched, resourceOffers(&driver, OffersHaveAnyResource(
+ std::bind(&Resources::hasResourceProvider, lambda::_1))))
+ .WillOnce(FutureArg<1>(&oldOffers));
+
+ // TODO(chhsiao): Wait for an rescinded offer once we implemented the
+ // logic to send `UpdateSlaveMessage` upon removal of a resource
+ // provider.
+
+ driver.start();
+
+ // Wait for an offer having the old provider resource.
+ AWAIT_READY(oldOffers);
+
+ AWAIT_EXPECT_RESPONSE_STATUS_EQ(
+ http::OK().status,
+ removeResourceProviderConfig(
+ slave.get()->pid, contentType, info.type(), info.name()));
+
+ // Check that the existing config is removed.
+ EXPECT_FALSE(os::exists(configPath));
+
+ // TODO(chhsiao): Wait for the old offer to be rescinded.
+}
+
+
+// This test checks that removing a nonexistent resource provider config
+// is not allowed.
+TEST_P(AgentResourceProviderConfigApiTest, RemoveNotFound)
+{
+ const ContentType contentType = GetParam();
+
+ Try<Owned<cluster::Master>> master = StartMaster();
+ ASSERT_SOME(master);
+
+ Owned<MasterDetector> detector = master.get()->createDetector();
+
+ slave::Flags flags = CreateSlaveFlags();
+
+ // Set the resource provider capability and other required capabilities.
+ constexpr SlaveInfo::Capability::Type capabilities[] = {
+ SlaveInfo::Capability::MULTI_ROLE,
+ SlaveInfo::Capability::HIERARCHICAL_ROLE,
+ SlaveInfo::Capability::RESERVATION_REFINEMENT,
+ SlaveInfo::Capability::RESOURCE_PROVIDER
+ };
+
+ flags.agent_features = SlaveCapabilities();
+ foreach (SlaveInfo::Capability::Type type, capabilities) {
+ flags.agent_features->add_capabilities()->set_type(type);
+ }
+
+ flags.resource_provider_config_dir = resourceProviderConfigDir;
+
+ Future<SlaveRegisteredMessage> slaveRegisteredMessage =
+ FUTURE_PROTOBUF(SlaveRegisteredMessage(), _, _);
+
+ Try<Owned<cluster::Slave>> slave = StartSlave(detector.get(), flags);
+ ASSERT_SOME(slave);
+
+ AWAIT_READY(slaveRegisteredMessage);
+
+ ResourceProviderInfo info = createResourceProviderInfo(Gigabytes(4));
+
+ AWAIT_EXPECT_RESPONSE_STATUS_EQ(
+ http::NotFound().status,
+ removeResourceProviderConfig(
+ slave.get()->pid, contentType, info.type(), info.name()));
+}
+
+} // namespace tests {
+} // namespace internal {
+} // namespace mesos {
[3/4] mesos git commit: Added a test for
`MODIFY_RESOURE_PROVIDER_CONFIG` authorization.
Posted by ji...@apache.org.
Added a test for `MODIFY_RESOURE_PROVIDER_CONFIG` authorization.
This patch adds a unit test for the new authorization for adding,
updating and removing resource provider config files. It also renames
the ACL entity field to fit in the context better.
Review: https://reviews.apache.org/r/64479/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/24d17b29
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/24d17b29
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/24d17b29
Branch: refs/heads/master
Commit: 24d17b297a1743fb01414ebb2bd95e8d4bb2b0e9
Parents: 2c606ae
Author: Chun-Hung Hsiao <ch...@mesosphere.io>
Authored: Mon Dec 11 15:09:42 2017 -0800
Committer: Jie Yu <yu...@gmail.com>
Committed: Mon Dec 11 15:09:42 2017 -0800
----------------------------------------------------------------------
include/mesos/authorizer/acls.proto | 6 ++--
src/authorizer/local/authorizer.cpp | 13 +++++--
src/tests/authorization_tests.cpp | 60 ++++++++++++++++++++++++++++++++
3 files changed, 74 insertions(+), 5 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/24d17b29/include/mesos/authorizer/acls.proto
----------------------------------------------------------------------
diff --git a/include/mesos/authorizer/acls.proto b/include/mesos/authorizer/acls.proto
index aca9aa8..40a1425 100644
--- a/include/mesos/authorizer/acls.proto
+++ b/include/mesos/authorizer/acls.proto
@@ -478,8 +478,8 @@ message ACL {
// Use Entity type ANY or NONE to allow or deny access.
//
// TODO(chhsiao): Consider allowing granular permission to act upon
- // SOME particular operating system users (e.g., linux users).
- required Entity users = 2;
+ // SOME resource provider types and names.
+ required Entity resource_providers = 2;
}
}
@@ -556,5 +556,5 @@ message ACLs {
repeated ACL.KillStandaloneContainer kill_standalone_container = 42;
repeated ACL.WaitStandaloneContainer wait_standalone_container = 43;
repeated ACL.RemoveStandaloneContainer remove_standalone_container = 44;
- repeated ACL.ModifyResourceProviderConfig modify_resource_provider_config = 45;
+ repeated ACL.ModifyResourceProviderConfig modify_resource_provider_configs = 45;
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/24d17b29/src/authorizer/local/authorizer.cpp
----------------------------------------------------------------------
diff --git a/src/authorizer/local/authorizer.cpp b/src/authorizer/local/authorizer.cpp
index 809c2e4..09f2618 100644
--- a/src/authorizer/local/authorizer.cpp
+++ b/src/authorizer/local/authorizer.cpp
@@ -1480,10 +1480,10 @@ private:
return acls_;
case authorization::MODIFY_RESOURCE_PROVIDER_CONFIG:
foreach (const ACL::ModifyResourceProviderConfig& acl,
- acls.modify_resource_provider_config()) {
+ acls.modify_resource_provider_configs()) {
GenericACL acl_;
acl_.subjects = acl.principals();
- acl_.objects = acl.users();
+ acl_.objects = acl.resource_providers();
acls_.push_back(acl_);
}
@@ -1652,6 +1652,15 @@ Option<Error> LocalAuthorizer::validate(const ACLs& acls)
}
}
+ foreach (const ACL::ModifyResourceProviderConfig& acl,
+ acls.modify_resource_provider_configs()) {
+ if (acl.resource_providers().type() == ACL::Entity::SOME) {
+ return Error(
+ "acls.modify_resource_provider_config type must be either NONE or "
+ "ANY");
+ }
+ }
+
// TODO(alexr): Consider validating not only protobuf, but also the original
// JSON in order to spot misspelled names. A misspelled action may affect
// authorization result and hence lead to a security issue (e.g. when there
http://git-wip-us.apache.org/repos/asf/mesos/blob/24d17b29/src/tests/authorization_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/authorization_tests.cpp b/src/tests/authorization_tests.cpp
index f2e86e2..35eecc8 100644
--- a/src/tests/authorization_tests.cpp
+++ b/src/tests/authorization_tests.cpp
@@ -5375,6 +5375,66 @@ TYPED_TEST(AuthorizationTest, GetMaintenanceStatus)
}
}
+
+// This tests the authorization of requests to ModifyResourceProviderConfig.
+TYPED_TEST(AuthorizationTest, ModifyResourceProviderConfig)
+{
+ ACLs acls;
+
+ {
+ // "foo" principal can modify resource provider configs on agents.
+ mesos::ACL::ModifyResourceProviderConfig* acl =
+ acls.add_modify_resource_provider_configs();
+ acl->mutable_principals()->add_values("foo");
+ acl->mutable_resource_providers()->set_type(mesos::ACL::Entity::ANY);
+ }
+
+ {
+ // Nobody else can modify resource provider configs.
+ mesos::ACL::ModifyResourceProviderConfig* acl =
+ acls.add_modify_resource_provider_configs();
+ acl->mutable_principals()->set_type(mesos::ACL::Entity::ANY);
+ acl->mutable_resource_providers()->set_type(mesos::ACL::Entity::NONE);
+ }
+
+ Try<Authorizer*> create = TypeParam::create(parameterize(acls));
+ ASSERT_SOME(create);
+ Owned<Authorizer> authorizer(create.get());
+
+ {
+ // "foo" is allowed to modify resource provider configs. The request
+ // should succeed.
+ authorization::Request request;
+ request.set_action(authorization::MODIFY_RESOURCE_PROVIDER_CONFIG);
+ request.mutable_subject()->set_value("foo");
+
+ AWAIT_EXPECT_TRUE(authorizer->authorized(request));
+ }
+
+ {
+ // "bar" is not allowed to modify resource provider configs. The
+ // request should fail.
+ authorization::Request request;
+ request.set_action(authorization::MODIFY_RESOURCE_PROVIDER_CONFIG);
+ request.mutable_subject()->set_value("bar");
+
+ AWAIT_EXPECT_FALSE(authorizer->authorized(request));
+ }
+
+ {
+ // Test that no authorizer is created with invalid ACLs.
+ ACLs invalid;
+
+ mesos::ACL::ModifyResourceProviderConfig* acl =
+ invalid.add_modify_resource_provider_configs();
+ acl->mutable_principals()->add_values("foo");
+ acl->mutable_resource_providers()->add_values("yoda");
+
+ Try<Authorizer*> create = TypeParam::create(parameterize(invalid));
+ EXPECT_ERROR(create);
+ }
+}
+
} // namespace tests {
} // namespace internal {
} // namespace mesos {