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 2015/06/10 22:55:00 UTC
mesos git commit: Moved validation of ExecutorInfo.framework_id()
from scheduler driver to the master.
Repository: mesos
Updated Branches:
refs/heads/master f55dc9818 -> 90610189c
Moved validation of ExecutorInfo.framework_id() from scheduler driver
to the master.
Review: https://reviews.apache.org/r/30952
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/90610189
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/90610189
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/90610189
Branch: refs/heads/master
Commit: 90610189c868c37bd5310741fdf424204f7ddeeb
Parents: f55dc98
Author: Isabel Jimenez <co...@isabeljimenez.com>
Authored: Wed Jun 10 13:54:06 2015 -0700
Committer: Vinod Kone <vi...@gmail.com>
Committed: Wed Jun 10 13:54:08 2015 -0700
----------------------------------------------------------------------
src/master/master.cpp | 27 +++++++++++++++++++--------
src/master/validation.cpp | 7 +------
src/sched/sched.cpp | 2 ++
src/scheduler/scheduler.cpp | 19 -------------------
4 files changed, 22 insertions(+), 33 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/90610189/src/master/master.cpp
----------------------------------------------------------------------
diff --git a/src/master/master.cpp b/src/master/master.cpp
index e3b7081..fe2b2ae 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -2703,8 +2703,19 @@ void Master::_accept(
}
// Validate the task.
+
+ // Make a copy of the original task so that we can
+ // fill the missing `framework_id` in ExecutorInfo
+ // if needed. This field was added to the API later
+ // and thus was made optional.
+ TaskInfo task_(task);
+ if (task.has_executor() && !task.executor().has_framework_id()) {
+ task_.mutable_executor()
+ ->mutable_framework_id()->CopyFrom(framework->id());
+ }
+
const Option<Error>& validationError = validation::task::validate(
- task,
+ task_,
framework,
slave,
_offeredResources);
@@ -2712,8 +2723,8 @@ void Master::_accept(
if (validationError.isSome()) {
const StatusUpdate& update = protobuf::createStatusUpdate(
framework->id(),
- task.slave_id(),
- task.task_id(),
+ task_.slave_id(),
+ task_.task_id(),
TASK_ERROR,
TaskStatus::SOURCE_MASTER,
validationError.get().message,
@@ -2733,25 +2744,25 @@ void Master::_accept(
// Add task.
if (pending) {
- _offeredResources -= addTask(task, framework, slave);
+ _offeredResources -= addTask(task_, framework, slave);
// TODO(bmahler): Consider updating this log message to
// indicate when the executor is also being launched.
- LOG(INFO) << "Launching task " << task.task_id()
+ LOG(INFO) << "Launching task " << task_.task_id()
<< " of framework " << *framework
- << " with resources " << task.resources()
+ << " with resources " << task_.resources()
<< " on slave " << *slave;
RunTaskMessage message;
message.mutable_framework()->MergeFrom(framework->info);
message.mutable_framework_id()->MergeFrom(framework->id());
message.set_pid(framework->pid);
- message.mutable_task()->MergeFrom(task);
+ message.mutable_task()->MergeFrom(task_);
// Set labels retrieved from label-decorator hooks.
message.mutable_task()->mutable_labels()->CopyFrom(
HookManager::masterLaunchTaskLabelDecorator(
- task,
+ task_,
framework->info,
slave->info));
http://git-wip-us.apache.org/repos/asf/mesos/blob/90610189/src/master/validation.cpp
----------------------------------------------------------------------
diff --git a/src/master/validation.cpp b/src/master/validation.cpp
index 1793b0e..6c9dc04 100644
--- a/src/master/validation.cpp
+++ b/src/master/validation.cpp
@@ -245,12 +245,7 @@ Option<Error> validateExecutorInfo(
// set even though it is an optional field. Currently, the
// scheduler driver ensures that the field is set. For schedulers
// not using the driver, we need to do the validation here.
- // TODO(bmahler): Set this field in the master instead of
- // depending on the scheduler driver do it.
- if (!task.executor().has_framework_id()) {
- return Error(
- "Task has invalid ExecutorInfo: missing field 'framework_id'");
- }
+ CHECK(task.executor().has_framework_id());
if (task.executor().framework_id() != framework->id()) {
return Error(
http://git-wip-us.apache.org/repos/asf/mesos/blob/90610189/src/sched/sched.cpp
----------------------------------------------------------------------
diff --git a/src/sched/sched.cpp b/src/sched/sched.cpp
index 8c366ec..9423607 100644
--- a/src/sched/sched.cpp
+++ b/src/sched/sched.cpp
@@ -934,6 +934,7 @@ protected:
}
// Set TaskInfo.executor.framework_id, if it's missing.
+ // TODO(ijimenez): Remove this validation in 0.24.0.
vector<TaskInfo> result;
foreach (TaskInfo task, tasks) {
if (task.has_executor() && !task.executor().has_framework_id()) {
@@ -1032,6 +1033,7 @@ protected:
}
// Set TaskInfo.executor.framework_id, if it's missing.
+ // TODO(ijimenez): Remove this validation in 0.24.0.
foreach (TaskInfo& task,
*operation->mutable_launch()->mutable_task_infos()) {
if (task.has_executor() && !task.executor().has_framework_id()) {
http://git-wip-us.apache.org/repos/asf/mesos/blob/90610189/src/scheduler/scheduler.cpp
----------------------------------------------------------------------
diff --git a/src/scheduler/scheduler.cpp b/src/scheduler/scheduler.cpp
index f613ca4..af18c6b 100644
--- a/src/scheduler/scheduler.cpp
+++ b/src/scheduler/scheduler.cpp
@@ -253,25 +253,6 @@ public:
drop(call, "Expecting 'accept' to be present");
return;
}
- // We do some local validation here, but really this should
- // all happen in the master so it's only implemented once.
- foreach (Offer::Operation& operation,
- *call.mutable_accept()->mutable_operations()) {
- if (operation.type() != Offer::Operation::LAUNCH) {
- continue;
- }
-
- foreach (TaskInfo& task,
- *operation.mutable_launch()->mutable_task_infos()) {
- // Set ExecutorInfo::framework_id if missing since this
- // field was added to the API later and thus was made
- // optional.
- if (task.has_executor() && !task.executor().has_framework_id()) {
- task.mutable_executor()->mutable_framework_id()->CopyFrom(
- call.framework_info().id());
- }
- }
- }
send(master.get(), call);
break;
}