You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by vi...@apache.org on 2017/02/01 22:18:16 UTC
[1/3] mesos git commit: Changed 'Environment.Variable.Value' from
required to optional.
Repository: mesos
Updated Branches:
refs/heads/master ab0a6b5b9 -> f2f771165
Changed 'Environment.Variable.Value' from required to optional.
To prepare for future work which will enable the
modular fetching of secrets, we should change the
Environment.Variable.Value field from required to
optional. This way, the field can be left empty
and filled in by a secret fetching module.
Review: https://reviews.apache.org/r/55954/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/063ecdee
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/063ecdee
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/063ecdee
Branch: refs/heads/master
Commit: 063ecdee574f7d1d491c490bda2dfb956d8c515e
Parents: ab0a6b5
Author: Greg Mann <gr...@mesosphere.io>
Authored: Wed Feb 1 14:17:57 2017 -0800
Committer: Vinod Kone <vi...@gmail.com>
Committed: Wed Feb 1 14:17:57 2017 -0800
----------------------------------------------------------------------
include/mesos/mesos.proto | 5 +++-
include/mesos/v1/mesos.proto | 5 +++-
src/checks/checker.cpp | 9 +++++++
src/checks/health_checker.cpp | 8 +++++++
src/common/validation.cpp | 22 +++++++++++++++++
src/common/validation.hpp | 2 ++
src/master/validation.cpp | 48 ++++++++++++++++++++++++++++++++++----
src/slave/validation.cpp | 18 ++++++++++++++
8 files changed, 111 insertions(+), 6 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/063ecdee/include/mesos/mesos.proto
----------------------------------------------------------------------
diff --git a/include/mesos/mesos.proto b/include/mesos/mesos.proto
index 7c4afc0..a64e929 100644
--- a/include/mesos/mesos.proto
+++ b/include/mesos/mesos.proto
@@ -1898,7 +1898,10 @@ message Filters {
message Environment {
message Variable {
required string name = 1;
- required string value = 2;
+ // NOTE: The `value` field was made optional in Mesos 1.2 but it
+ // is currently enforced to be set. This constraint will be
+ // removed in a future version.
+ optional string value = 2;
}
repeated Variable variables = 1;
http://git-wip-us.apache.org/repos/asf/mesos/blob/063ecdee/include/mesos/v1/mesos.proto
----------------------------------------------------------------------
diff --git a/include/mesos/v1/mesos.proto b/include/mesos/v1/mesos.proto
index bf32168..cc49df0 100644
--- a/include/mesos/v1/mesos.proto
+++ b/include/mesos/v1/mesos.proto
@@ -1897,7 +1897,10 @@ message Filters {
message Environment {
message Variable {
required string name = 1;
- required string value = 2;
+ // NOTE: The `value` field was made optional in Mesos 1.2 but it
+ // is currently enforced to be set. This constraint will be
+ // removed in a future version.
+ optional string value = 2;
}
repeated Variable variables = 1;
http://git-wip-us.apache.org/repos/asf/mesos/blob/063ecdee/src/checks/checker.cpp
----------------------------------------------------------------------
diff --git a/src/checks/checker.cpp b/src/checks/checker.cpp
index 8d7285a..8716e4c 100644
--- a/src/checks/checker.cpp
+++ b/src/checks/checker.cpp
@@ -22,6 +22,8 @@
#include <stout/strings.hpp>
+#include "common/validation.hpp"
+
using std::string;
namespace mesos {
@@ -51,6 +53,13 @@ Option<Error> checkInfo(const CheckInfo& checkInfo)
return Error("Command check must contain " + commandType);
}
+ Option<Error> error =
+ common::validation::validateCommandInfo(command);
+ if (error.isSome()) {
+ return Error(
+ "Check's `CommandInfo` is invalid: " + error->message);
+ }
+
// TODO(alexr): Make sure irrelevant fields, e.g., `uris` are not set.
break;
http://git-wip-us.apache.org/repos/asf/mesos/blob/063ecdee/src/checks/health_checker.cpp
----------------------------------------------------------------------
diff --git a/src/checks/health_checker.cpp b/src/checks/health_checker.cpp
index e70bd79..58380dc 100644
--- a/src/checks/health_checker.cpp
+++ b/src/checks/health_checker.cpp
@@ -50,6 +50,7 @@
#include <stout/os/killtree.hpp>
#include "common/status_utils.hpp"
+#include "common/validation.hpp"
#ifdef __linux__
#include "linux/ns.hpp"
@@ -682,6 +683,13 @@ Option<Error> healthCheck(const HealthCheck& check)
return Error("Command health check must contain " + commandType);
}
+ Option<Error> error =
+ common::validation::validateCommandInfo(command);
+ if (error.isSome()) {
+ return Error(
+ "Health check's `CommandInfo` is invalid: " + error->message);
+ }
+
break;
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/063ecdee/src/common/validation.cpp
----------------------------------------------------------------------
diff --git a/src/common/validation.cpp b/src/common/validation.cpp
index b2548ad..0f1a022 100644
--- a/src/common/validation.cpp
+++ b/src/common/validation.cpp
@@ -19,6 +19,8 @@
#include <algorithm>
#include <cctype>
+#include <stout/foreach.hpp>
+
#include <stout/os/constants.hpp>
using std::string;
@@ -77,6 +79,26 @@ Option<Error> validateFrameworkID(const FrameworkID& frameworkId)
return validateID(frameworkId.value());
}
+
+// TODO(greggomann): Do more than just validate the `Environment`.
+Option<Error> validateCommandInfo(const CommandInfo& command)
+{
+ // In Mesos 1.2, the `Environment.Variable.Value` message was made
+ // optional, but it is currently required to be set for backward
+ // compatibility. This constraint will be removed in a future
+ // version.
+ foreach (const Environment::Variable& variable,
+ command.environment().variables()) {
+ if (!variable.has_value()) {
+ return Error(
+ "Environment variable '" + variable.name() +
+ "' must have a value set");
+ }
+ }
+
+ return None();
+}
+
} // namespace validation {
} // namespace common {
} // namespace internal {
http://git-wip-us.apache.org/repos/asf/mesos/blob/063ecdee/src/common/validation.hpp
----------------------------------------------------------------------
diff --git a/src/common/validation.hpp b/src/common/validation.hpp
index fee9551..7946f06 100644
--- a/src/common/validation.hpp
+++ b/src/common/validation.hpp
@@ -41,6 +41,8 @@ Option<Error> validateSlaveID(const SlaveID& slaveId);
Option<Error> validateFrameworkID(const FrameworkID& frameworkId);
+Option<Error> validateCommandInfo(const CommandInfo& command);
+
} // namespace validation {
} // namespace common {
} // namespace internal {
http://git-wip-us.apache.org/repos/asf/mesos/blob/063ecdee/src/master/validation.cpp
----------------------------------------------------------------------
diff --git a/src/master/validation.cpp b/src/master/validation.cpp
index e3f71be..f45b644 100644
--- a/src/master/validation.cpp
+++ b/src/master/validation.cpp
@@ -744,6 +744,21 @@ Option<Error> validateResources(const ExecutorInfo& executor)
}
+// Validates the `CommandInfo` contained within an `ExecutorInfo`.
+Option<Error> validateCommandInfo(const ExecutorInfo& executor)
+{
+ if (executor.has_command()) {
+ Option<Error> error =
+ common::validation::validateCommandInfo(executor.command());
+ if (error.isSome()) {
+ return Error("Executor's `CommandInfo` is invalid: " + error->message);
+ }
+ }
+
+ return None();
+}
+
+
Option<Error> validate(
const ExecutorInfo& executor,
Framework* framework,
@@ -759,7 +774,8 @@ Option<Error> validate(
lambda::bind(internal::validateShutdownGracePeriod, executor),
lambda::bind(internal::validateResources, executor),
lambda::bind(
- internal::validateCompatibleExecutorInfo, executor, framework, slave)
+ internal::validateCompatibleExecutorInfo, executor, framework, slave),
+ lambda::bind(internal::validateCommandInfo, executor)
};
foreach (const lambda::function<Option<Error>()>& validator, validators) {
@@ -919,6 +935,21 @@ Option<Error> validateTaskAndExecutorResources(const TaskInfo& task)
}
+// Validates the `CommandInfo` contained within a `TaskInfo`.
+Option<Error> validateCommandInfo(const TaskInfo& task)
+{
+ if (task.has_command()) {
+ Option<Error> error =
+ common::validation::validateCommandInfo(task.command());
+ if (error.isSome()) {
+ return Error("Task's `CommandInfo` is invalid: " + error->message);
+ }
+ }
+
+ return None();
+}
+
+
// Validates task specific fields except its executor (if it exists).
Option<Error> validateTask(
const TaskInfo& task,
@@ -937,11 +968,10 @@ Option<Error> validateTask(
lambda::bind(internal::validateKillPolicy, task),
lambda::bind(internal::validateCheck, task),
lambda::bind(internal::validateHealthCheck, task),
- lambda::bind(internal::validateResources, task)
+ lambda::bind(internal::validateResources, task),
+ lambda::bind(internal::validateCommandInfo, task)
};
- // TODO(jieyu): Add a validateCommandInfo function.
-
foreach (const lambda::function<Option<Error>()>& validator, validators) {
Option<Error> error = validator();
if (error.isSome()) {
@@ -1247,6 +1277,16 @@ Option<Error> validateExecutor(
" its executor are more than available " + stringify(offered));
}
+ if (executor.has_command()) {
+ Option<Error> error =
+ common::validation::validateCommandInfo(executor.command());
+ if (error.isSome()) {
+ return Error(
+ "Executor '" + stringify(executor.executor_id()) + "'" +
+ "contains an invalid command: " + error->message);
+ }
+ }
+
return None();
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/063ecdee/src/slave/validation.cpp
----------------------------------------------------------------------
diff --git a/src/slave/validation.cpp b/src/slave/validation.cpp
index c30a0eb..2c5335b 100644
--- a/src/slave/validation.cpp
+++ b/src/slave/validation.cpp
@@ -170,6 +170,15 @@ Option<Error> validate(
" to be present");
}
+ if (call.launch_nested_container().has_command()) {
+ error = common::validation::validateCommandInfo(
+ call.launch_nested_container().command());
+ if (error.isSome()) {
+ return Error("'launch_nested_container.command' is invalid"
+ ": " + error->message);
+ }
+ }
+
return None();
}
@@ -239,6 +248,15 @@ Option<Error> validate(
" to be present");
}
+ if (call.launch_nested_container_session().has_command()) {
+ error = common::validation::validateCommandInfo(
+ call.launch_nested_container_session().command());
+ if (error.isSome()) {
+ return Error("'launch_nested_container_session.command' is invalid"
+ ": " + error->message);
+ }
+ }
+
return None();
}
[2/3] mesos git commit: Added validation tests to ensure environment
variable value is set.
Posted by vi...@apache.org.
Added validation tests to ensure environment variable value is set.
The `value` field within `Environment::Variable` is being
changed to `optional`, but for the time being we will
enforce that it must be set for backward compatibility.
This patch adds tests to ensure that environment variables
with unset values are correctly rejected.
Review: https://reviews.apache.org/r/55955/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/28983036
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/28983036
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/28983036
Branch: refs/heads/master
Commit: 289830368675afb3a46278202687e30aafc6cb03
Parents: 063ecde
Author: Greg Mann <gr...@mesosphere.io>
Authored: Wed Feb 1 14:18:02 2017 -0800
Committer: Vinod Kone <vi...@gmail.com>
Committed: Wed Feb 1 14:18:02 2017 -0800
----------------------------------------------------------------------
src/tests/check_tests.cpp | 22 ++
src/tests/health_check_tests.cpp | 20 ++
src/tests/master_validation_tests.cpp | 325 +++++++++++++++++++++++++++++
src/tests/slave_validation_tests.cpp | 23 +-
4 files changed, 389 insertions(+), 1 deletion(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/28983036/src/tests/check_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/check_tests.cpp b/src/tests/check_tests.cpp
index c88cd34..b5a5d8e 100644
--- a/src/tests/check_tests.cpp
+++ b/src/tests/check_tests.cpp
@@ -81,6 +81,28 @@ TEST_F(CheckTest, CheckInfoValidation)
EXPECT_EQ("Command check must contain 'shell command'", validate->message);
}
+ // Command check must specify a command with a valid environment. Currently,
+ // `Environment.Variable.Value` must be set, but this constraint will be
+ // removed in a future version.
+ {
+ CheckInfo checkInfo;
+ checkInfo.set_type(CheckInfo::COMMAND);
+ checkInfo.mutable_command()->CopyFrom(CheckInfo::Command());
+ checkInfo.mutable_command()->mutable_command()->CopyFrom(
+ createCommandInfo("exit 0"));
+
+ Option<Error> validate = validation::checkInfo(checkInfo);
+ EXPECT_NONE(validate);
+
+ Environment::Variable* variable =
+ checkInfo.mutable_command()->mutable_command()->mutable_environment()
+ ->mutable_variables()->Add();
+ variable->set_name("ENV_VAR_KEY");
+
+ validate = validation::checkInfo(checkInfo);
+ EXPECT_SOME(validate);
+ }
+
// HTTP check may specify a path starting with '/'.
{
CheckInfo checkInfo;
http://git-wip-us.apache.org/repos/asf/mesos/blob/28983036/src/tests/health_check_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/health_check_tests.cpp b/src/tests/health_check_tests.cpp
index 710cb66..8418cd9 100644
--- a/src/tests/health_check_tests.cpp
+++ b/src/tests/health_check_tests.cpp
@@ -206,6 +206,26 @@ TEST_F(HealthCheckTest, HealthCheckProtobufValidation)
EXPECT_SOME(validate);
}
+ // Command health check must specify a command with a valid environment.
+ // Currently, `Environment.Variable.Value` must be set, but this constraint
+ // will be removed in a future version.
+ {
+ HealthCheck healthCheckProto;
+ healthCheckProto.set_type(HealthCheck::COMMAND);
+ healthCheckProto.mutable_command()->CopyFrom(createCommandInfo("exit 0"));
+
+ Option<Error> validate = validation::healthCheck(healthCheckProto);
+ EXPECT_NONE(validate);
+
+ Environment::Variable* variable =
+ healthCheckProto.mutable_command()->mutable_environment()
+ ->mutable_variables()->Add();
+ variable->set_name("ENV_VAR_KEY");
+
+ validate = validation::healthCheck(healthCheckProto);
+ EXPECT_SOME(validate);
+ }
+
// HTTP health check may specify a known scheme and a path starting with '/'.
{
HealthCheck healthCheckProto;
http://git-wip-us.apache.org/repos/asf/mesos/blob/28983036/src/tests/master_validation_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/master_validation_tests.cpp b/src/tests/master_validation_tests.cpp
index edb5740..1f833aa 100644
--- a/src/tests/master_validation_tests.cpp
+++ b/src/tests/master_validation_tests.cpp
@@ -860,6 +860,65 @@ TEST_F(TaskValidationTest, ExecutorUsesInvalidFrameworkID)
}
+// Verifies that an environment variable in `ExecutorInfo.command.environment`
+// without a value will be correctly rejected. This constraint will be removed
+// in a future version.
+TEST_F(TaskValidationTest, ExecutorEnvironmentVariableWithoutValue)
+{
+ Try<Owned<cluster::Master>> master = StartMaster();
+ ASSERT_SOME(master);
+
+ Owned<MasterDetector> detector = master.get()->createDetector();
+ Try<Owned<cluster::Slave>> slave = StartSlave(detector.get());
+ ASSERT_SOME(slave);
+
+ MockScheduler sched;
+ MesosSchedulerDriver driver(
+ &sched, DEFAULT_FRAMEWORK_INFO, master.get()->pid, DEFAULT_CREDENTIAL);
+
+ EXPECT_CALL(sched, registered(&driver, _, _));
+
+ // Create an executor.
+ ExecutorInfo executor;
+ executor = DEFAULT_EXECUTOR_INFO;
+ Environment::Variable* variable =
+ executor.mutable_command()->mutable_environment()
+ ->mutable_variables()->Add();
+ variable->set_name("ENV_VAR_KEY");
+
+ EXPECT_CALL(sched, resourceOffers(&driver, _))
+ .WillOnce(LaunchTasks(executor, 1, 1, 16, "*"))
+ .WillRepeatedly(Return()); // Ignore subsequent offers.
+
+ Future<TaskStatus> status;
+ EXPECT_CALL(sched, statusUpdate(&driver, _))
+ .WillOnce(FutureArg<1>(&status));
+
+ driver.start();
+
+ AWAIT_READY(status);
+ EXPECT_EQ(TASK_ERROR, status.get().state());
+ EXPECT_EQ(
+ "Executor's `CommandInfo` is invalid: "
+ "Environment variable 'ENV_VAR_KEY' must have a value set",
+ status->message());
+
+ // Make sure the task is not known to master anymore.
+ EXPECT_CALL(sched, statusUpdate(&driver, _))
+ .Times(0);
+
+ driver.reconcileTasks({});
+
+ // We settle the clock here to ensure any updates sent by the master
+ // are received. There shouldn't be any updates in this case.
+ Clock::pause();
+ Clock::settle();
+
+ driver.stop();
+ driver.join();
+}
+
+
// The master should fill in the `ExecutorInfo.framework_id`
// if it is not set by the framework.
TEST_F(TaskValidationTest, ExecutorMissingFrameworkID)
@@ -1804,6 +1863,59 @@ TEST_F(TaskValidationTest, KillPolicyGracePeriodIsNonNegative)
driver.join();
}
+
+// This test verifies that a task containing an environment variable
+// with no value will be rejected correctly. Note that this constraint
+// will be removed in a future version.
+TEST_F(TaskValidationTest, TaskEnvironmentVariableWithoutValue)
+{
+ Try<Owned<cluster::Master>> master = StartMaster();
+ ASSERT_SOME(master);
+
+ Owned<MasterDetector> detector = master.get()->createDetector();
+ Try<Owned<cluster::Slave>> slave = StartSlave(detector.get());
+ ASSERT_SOME(slave);
+
+ MockScheduler sched;
+ MesosSchedulerDriver driver(
+ &sched, DEFAULT_FRAMEWORK_INFO, master.get()->pid, DEFAULT_CREDENTIAL);
+
+ EXPECT_CALL(sched, registered(&driver, _, _));
+
+ Future<vector<Offer>> offers;
+ EXPECT_CALL(sched, resourceOffers(&driver, _))
+ .WillOnce(FutureArg<1>(&offers))
+ .WillRepeatedly(Return()); // Ignore subsequent offers.
+
+ driver.start();
+
+ AWAIT_READY(offers);
+ EXPECT_NE(0u, offers.get().size());
+
+ Future<TaskStatus> status;
+ EXPECT_CALL(sched, statusUpdate(&driver, _))
+ .WillOnce(FutureArg<1>(&status));
+
+ // Create a task that contains a `CommandInfo.Environment` with an
+ // unset environment variable value.
+ TaskInfo task = createTask(offers.get()[0], "exit 0"); // Command task.
+ Environment::Variable* variable =
+ task.mutable_command()->mutable_environment()->mutable_variables()->Add();
+ variable->set_name("ENV_VAR_KEY");
+
+ driver.launchTasks(offers.get()[0].id(), {task});
+
+ AWAIT_READY(status);
+ EXPECT_EQ(TASK_ERROR, status.get().state());
+ EXPECT_EQ(
+ "Task's `CommandInfo` is invalid: "
+ "Environment variable 'ENV_VAR_KEY' must have a value set",
+ status->message());
+
+ driver.stop();
+ driver.join();
+}
+
// TODO(jieyu): Add tests for checking duplicated persistence ID
// against offered resources.
@@ -2542,6 +2654,219 @@ TEST_F(TaskGroupValidationTest, TaskUsesDifferentExecutor)
}
+// Ensures that a task group which specifies an invalid environment in
+// `ExecutorInfo` will be correctly rejected. Currently,
+// `Environment.Variable.Value` must be set, but this constraint will be removed
+// in a future version.
+TEST_F(TaskGroupValidationTest, ExecutorEnvironmentVariableWithoutValue)
+{
+ Try<Owned<cluster::Master>> master = StartMaster();
+ ASSERT_SOME(master);
+
+ Owned<MasterDetector> detector = master.get()->createDetector();
+ Try<Owned<cluster::Slave>> slave = StartSlave(detector.get());
+ ASSERT_SOME(slave);
+
+ FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
+ frameworkInfo.mutable_id()->set_value("Test_Framework");
+
+ MockScheduler sched;
+ MesosSchedulerDriver driver(
+ &sched, frameworkInfo, master.get()->pid, DEFAULT_CREDENTIAL);
+
+ EXPECT_CALL(sched, registered(&driver, _, _));
+
+ Future<vector<Offer>> offers;
+ EXPECT_CALL(sched, resourceOffers(&driver, _))
+ .WillOnce(FutureArg<1>(&offers))
+ .WillRepeatedly(Return()); // Ignore subsequent offers.
+
+ driver.start();
+
+ AWAIT_READY(offers);
+ EXPECT_NE(0u, offers.get().size());
+
+ TaskInfo task1;
+ task1.set_name("1");
+ task1.mutable_task_id()->set_value("1");
+ task1.mutable_slave_id()->MergeFrom(offers.get()[0].slave_id());
+ task1.mutable_resources()->MergeFrom(offers.get()[0].resources());
+
+ TaskInfo task2;
+ task2.set_name("2");
+ task2.mutable_task_id()->set_value("2");
+ task2.mutable_slave_id()->MergeFrom(offers.get()[0].slave_id());
+ task2.mutable_resources()->MergeFrom(offers.get()[0].resources());
+
+ Future<TaskStatus> task1Status;
+ Future<TaskStatus> task2Status;
+ EXPECT_CALL(sched, statusUpdate(&driver, _))
+ .WillOnce(FutureArg<1>(&task1Status))
+ .WillOnce(FutureArg<1>(&task2Status));
+
+ Offer::Operation operation;
+ operation.set_type(Offer::Operation::LAUNCH_GROUP);
+
+ ExecutorInfo executor(DEFAULT_EXECUTOR_INFO);
+ executor.mutable_framework_id()->CopyFrom(frameworkInfo.id());
+ Environment::Variable* variable =
+ executor.mutable_command()->mutable_environment()
+ ->mutable_variables()->Add();
+ variable->set_name("ENV_VAR_KEY");
+
+ TaskGroupInfo taskGroup;
+ taskGroup.add_tasks()->CopyFrom(task1);
+ taskGroup.add_tasks()->CopyFrom(task2);
+
+ Offer::Operation::LaunchGroup* launchGroup =
+ operation.mutable_launch_group();
+
+ launchGroup->mutable_executor()->CopyFrom(executor);
+ launchGroup->mutable_task_group()->CopyFrom(taskGroup);
+
+ driver.acceptOffers({offers.get()[0].id()}, {operation});
+
+ AWAIT_READY(task1Status);
+ EXPECT_EQ(TASK_ERROR, task1Status->state());
+ EXPECT_EQ(TaskStatus::REASON_TASK_GROUP_INVALID, task1Status->reason());
+ EXPECT_EQ(
+ "Executor's `CommandInfo` is invalid: "
+ "Environment variable 'ENV_VAR_KEY' must have a value set",
+ task1Status->message());
+
+ AWAIT_READY(task2Status);
+ EXPECT_EQ(TASK_ERROR, task2Status->state());
+ EXPECT_EQ(TaskStatus::REASON_TASK_GROUP_INVALID, task2Status->reason());
+ EXPECT_EQ(
+ "Executor's `CommandInfo` is invalid: "
+ "Environment variable 'ENV_VAR_KEY' must have a value set",
+ task2Status->message());
+
+ // Make sure the tasks are not known to master anymore.
+ EXPECT_CALL(sched, statusUpdate(&driver, _))
+ .Times(0);
+
+ driver.reconcileTasks({});
+
+ // We settle the clock here to ensure any updates sent by the master
+ // are received. There shouldn't be any updates in this case.
+ Clock::pause();
+ Clock::settle();
+
+ driver.stop();
+ driver.join();
+}
+
+
+// Ensures that a task group which specifies an invalid environment in
+// `TaskGroupInfo` will be correctly rejected. Currently,
+// `Environment.Variable.Value` must be set, but this constraint will be removed
+// in a future version.
+TEST_F(TaskGroupValidationTest, TaskEnvironmentVariableWithoutValue)
+{
+ Try<Owned<cluster::Master>> master = StartMaster();
+ ASSERT_SOME(master);
+
+ Owned<MasterDetector> detector = master.get()->createDetector();
+ Try<Owned<cluster::Slave>> slave = StartSlave(detector.get());
+ ASSERT_SOME(slave);
+
+ FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
+ frameworkInfo.mutable_id()->set_value("Test_Framework");
+
+ MockScheduler sched;
+ MesosSchedulerDriver driver(
+ &sched, frameworkInfo, master.get()->pid, DEFAULT_CREDENTIAL);
+
+ EXPECT_CALL(sched, registered(&driver, _, _));
+
+ Future<vector<Offer>> offers;
+ EXPECT_CALL(sched, resourceOffers(&driver, _))
+ .WillOnce(FutureArg<1>(&offers))
+ .WillRepeatedly(Return()); // Ignore subsequent offers.
+
+ driver.start();
+
+ AWAIT_READY(offers);
+ EXPECT_NE(0u, offers.get().size());
+
+ TaskInfo task1;
+ task1.set_name("1");
+ task1.mutable_task_id()->set_value("1");
+ task1.mutable_slave_id()->MergeFrom(offers.get()[0].slave_id());
+ task1.mutable_resources()->MergeFrom(offers.get()[0].resources());
+
+ Environment::Variable* variable =
+ task1.mutable_command()->mutable_environment()
+ ->mutable_variables()->Add();
+ variable->set_name("ENV_VAR_KEY");
+
+ TaskInfo task2;
+ task2.set_name("2");
+ task2.mutable_task_id()->set_value("2");
+ task2.mutable_slave_id()->MergeFrom(offers.get()[0].slave_id());
+ task2.mutable_resources()->MergeFrom(offers.get()[0].resources());
+
+ Future<TaskStatus> task1Status;
+ Future<TaskStatus> task2Status;
+ EXPECT_CALL(sched, statusUpdate(&driver, _))
+ .WillOnce(FutureArg<1>(&task1Status))
+ .WillOnce(FutureArg<1>(&task2Status));
+
+ Offer::Operation operation;
+ operation.set_type(Offer::Operation::LAUNCH_GROUP);
+
+ ExecutorInfo executor(DEFAULT_EXECUTOR_INFO);
+ executor.mutable_framework_id()->CopyFrom(frameworkInfo.id());
+
+ TaskGroupInfo taskGroup;
+ taskGroup.add_tasks()->CopyFrom(task1);
+ taskGroup.add_tasks()->CopyFrom(task2);
+
+ Offer::Operation::LaunchGroup* launchGroup =
+ operation.mutable_launch_group();
+
+ launchGroup->mutable_executor()->CopyFrom(executor);
+ launchGroup->mutable_task_group()->CopyFrom(taskGroup);
+
+ driver.acceptOffers({offers.get()[0].id()}, {operation});
+
+ AWAIT_READY(task1Status);
+ EXPECT_EQ(TASK_ERROR, task1Status->state());
+ EXPECT_EQ(TaskStatus::REASON_TASK_GROUP_INVALID, task1Status->reason());
+ EXPECT_EQ(
+ "Task '1' is invalid: Task's `CommandInfo` is invalid: Environment "
+ "variable 'ENV_VAR_KEY' must have a value set",
+ task1Status->message());
+
+ AWAIT_READY(task2Status);
+ EXPECT_EQ(TASK_ERROR, task2Status->state());
+ EXPECT_EQ(TaskStatus::REASON_TASK_GROUP_INVALID, task2Status->reason());
+ EXPECT_EQ(
+ "Task '1' is invalid: Task's `CommandInfo` is invalid: Environment "
+ "variable 'ENV_VAR_KEY' must have a value set",
+ task2Status->message());
+
+ // Allow status updates to arrive.
+ Clock::pause();
+ Clock::settle();
+
+ // Make sure the tasks are not known to master anymore.
+ EXPECT_CALL(sched, statusUpdate(&driver, _))
+ .Times(0);
+
+ driver.reconcileTasks({});
+
+ // We settle the clock here to ensure any updates sent by the master
+ // are received. There shouldn't be any updates in this case.
+ Clock::pause();
+ Clock::settle();
+
+ driver.stop();
+ driver.join();
+}
+
+
class FrameworkInfoValidationTest : public MesosTest {};
http://git-wip-us.apache.org/repos/asf/mesos/blob/28983036/src/tests/slave_validation_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/slave_validation_tests.cpp b/src/tests/slave_validation_tests.cpp
index 5de7711..2cbdbb5 100644
--- a/src/tests/slave_validation_tests.cpp
+++ b/src/tests/slave_validation_tests.cpp
@@ -28,6 +28,8 @@
#include "slave/slave.hpp"
#include "slave/validation.hpp"
+#include "tests/mesos.hpp"
+
namespace validation = mesos::internal::slave::validation;
using mesos::internal::slave::Slave;
@@ -114,12 +116,31 @@ TEST(AgentCallValidationTest, LaunchNestedContainer)
error = validation::agent::call::validate(call);
EXPECT_SOME(error);
- // Test the valid case.
+ // Valid `container_id.parent` but invalid `command.environment`. Currently,
+ // `Environment.Variable.Value` must be set, but this constraint will be
+ // removed in a future version.
ContainerID parentContainerId;
parentContainerId.set_value(UUID::random().toString());
launch->mutable_container_id()->mutable_parent()->CopyFrom(parentContainerId);
+ launch->mutable_command()->CopyFrom(createCommandInfo("exit 0"));
+ Environment::Variable* variable = launch
+ ->mutable_command()
+ ->mutable_environment()
+ ->mutable_variables()
+ ->Add();
+ variable->set_name("ENV_VAR_KEY");
+
+ error = validation::agent::call::validate(call);
+ EXPECT_SOME(error);
+ EXPECT_EQ(
+ "'launch_nested_container.command' is invalid: Environment variable "
+ "'ENV_VAR_KEY' must have a value set",
+ error->message);
+
+ // Test the valid case.
+ variable->set_value("env_var_value");
error = validation::agent::call::validate(call);
EXPECT_NONE(error);
[3/3] mesos git commit: Added validation for
'LAUNCH_NESTED_CONTAINER_SESSION'.
Posted by vi...@apache.org.
Added validation for 'LAUNCH_NESTED_CONTAINER_SESSION'.
This patch adds a validation test for the
`LAUNCH_NESTED_CONTAINER_SESSION` call.
Review: https://reviews.apache.org/r/56055/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/f2f77116
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/f2f77116
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/f2f77116
Branch: refs/heads/master
Commit: f2f77116534af4aeb72b31924183d369d018c893
Parents: 2898303
Author: Greg Mann <gr...@mesosphere.io>
Authored: Wed Feb 1 14:18:06 2017 -0800
Committer: Vinod Kone <vi...@gmail.com>
Committed: Wed Feb 1 14:18:06 2017 -0800
----------------------------------------------------------------------
src/tests/slave_validation_tests.cpp | 70 +++++++++++++++++++++++++++++++
1 file changed, 70 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/f2f77116/src/tests/slave_validation_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/slave_validation_tests.cpp b/src/tests/slave_validation_tests.cpp
index 2cbdbb5..3d17799 100644
--- a/src/tests/slave_validation_tests.cpp
+++ b/src/tests/slave_validation_tests.cpp
@@ -219,6 +219,76 @@ TEST(AgentCallValidationTest, KillNestedContainer)
EXPECT_NONE(error);
}
+
+TEST(AgentCallValidationTest, LaunchNestedContainerSession)
+{
+ // Missing `launch_nested_container_session`.
+ agent::Call call;
+ call.set_type(agent::Call::LAUNCH_NESTED_CONTAINER_SESSION);
+
+ Option<Error> error = validation::agent::call::validate(call);
+ EXPECT_SOME(error);
+
+ // `container_id` is not valid.
+ ContainerID badContainerId;
+ badContainerId.set_value("no spaces allowed");
+
+ agent::Call::LaunchNestedContainerSession* launch =
+ call.mutable_launch_nested_container_session();
+
+ launch->mutable_container_id()->CopyFrom(badContainerId);
+
+ error = validation::agent::call::validate(call);
+ EXPECT_SOME(error);
+
+ // Valid `container_id` but missing `container_id.parent`.
+ ContainerID containerId;
+ containerId.set_value(UUID::random().toString());
+
+ launch->mutable_container_id()->CopyFrom(containerId);
+
+ error = validation::agent::call::validate(call);
+ EXPECT_SOME(error);
+
+ // Valid `container_id.parent` but invalid `command.environment`. Currently,
+ // `Environment.Variable.Value` must be set, but this constraint will be
+ // removed in a future version.
+ ContainerID parentContainerId;
+ parentContainerId.set_value(UUID::random().toString());
+
+ launch->mutable_container_id()->mutable_parent()->CopyFrom(parentContainerId);
+ launch->mutable_command()->CopyFrom(createCommandInfo("exit 0"));
+
+ Environment::Variable* variable = launch
+ ->mutable_command()
+ ->mutable_environment()
+ ->mutable_variables()
+ ->Add();
+ variable->set_name("ENV_VAR_KEY");
+
+ error = validation::agent::call::validate(call);
+ EXPECT_SOME(error);
+ EXPECT_EQ(
+ "'launch_nested_container_session.command' is invalid: Environment "
+ "variable 'ENV_VAR_KEY' must have a value set",
+ error->message);
+
+ // Test the valid case.
+ variable->set_value("env_var_value");
+ error = validation::agent::call::validate(call);
+ EXPECT_NONE(error);
+
+ // Any number of parents is valid.
+ ContainerID grandparentContainerId;
+ grandparentContainerId.set_value(UUID::random().toString());
+
+ launch->mutable_container_id()->mutable_parent()->mutable_parent()->CopyFrom(
+ grandparentContainerId);
+
+ error = validation::agent::call::validate(call);
+ EXPECT_NONE(error);
+}
+
} // namespace tests {
} // namespace internal {
} // namespace mesos {