You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by jp...@apache.org on 2018/07/17 16:25:40 UTC
mesos git commit: Moved `executor::Call` validation to common
validation library.
Repository: mesos
Updated Branches:
refs/heads/master 4e5e72616 -> 7b785c75b
Moved `executor::Call` validation to common validation library.
`executor::Call` validation is used by both the agent and the
executor driver, so move it to the common validation library.
Review: https://reviews.apache.org/r/67830/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/7b785c75
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/7b785c75
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/7b785c75
Branch: refs/heads/master
Commit: 7b785c75bd4a48dea90a9265f7775a6e11310907
Parents: 4e5e726
Author: James Peach <jp...@apache.org>
Authored: Tue Jul 17 09:03:12 2018 -0700
Committer: James Peach <jp...@apache.org>
Committed: Tue Jul 17 09:03:12 2018 -0700
----------------------------------------------------------------------
src/common/validation.cpp | 104 ++++++++++++++++++++++++++++++++++++++
src/common/validation.hpp | 9 ++++
src/executor/executor.cpp | 8 ++-
src/slave/http.cpp | 3 +-
src/slave/validation.cpp | 110 -----------------------------------------
src/slave/validation.hpp | 11 -----
6 files changed, 118 insertions(+), 127 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/7b785c75/src/common/validation.cpp
----------------------------------------------------------------------
diff --git a/src/common/validation.cpp b/src/common/validation.cpp
index f89b0ca..5f8f288 100644
--- a/src/common/validation.cpp
+++ b/src/common/validation.cpp
@@ -21,6 +21,8 @@
#include <algorithm>
#include <cctype>
+#include <mesos/executor/executor.hpp>
+
#include <mesos/resources.hpp>
#include <stout/foreach.hpp>
@@ -506,6 +508,108 @@ Option<Error> validateCheckStatusInfo(const CheckStatusInfo& checkStatusInfo)
return None();
}
+Option<Error> validateExecutorCall(const mesos::executor::Call& call)
+{
+ if (!call.IsInitialized()) {
+ return Error("Not initialized: " + call.InitializationErrorString());
+ }
+
+ if (!call.has_type()) {
+ return Error("Expecting 'type' to be present");
+ }
+
+ // All calls should have executor id set.
+ if (!call.has_executor_id()) {
+ return Error("Expecting 'executor_id' to be present");
+ }
+
+ // All calls should have framework id set.
+ if (!call.has_framework_id()) {
+ return Error("Expecting 'framework_id' to be present");
+ }
+
+ switch (call.type()) {
+ case mesos::executor::Call::SUBSCRIBE: {
+ if (!call.has_subscribe()) {
+ return Error("Expecting 'subscribe' to be present");
+ }
+ return None();
+ }
+
+ case mesos::executor::Call::UPDATE: {
+ if (!call.has_update()) {
+ return Error("Expecting 'update' to be present");
+ }
+
+ const TaskStatus& status = call.update().status();
+
+ if (!status.has_uuid()) {
+ return Error("Expecting 'uuid' to be present");
+ }
+
+ Try<id::UUID> uuid = id::UUID::fromBytes(status.uuid());
+ if (uuid.isError()) {
+ return uuid.error();
+ }
+
+ if (status.has_executor_id() &&
+ status.executor_id().value()
+ != call.executor_id().value()) {
+ return Error("ExecutorID in Call: " +
+ call.executor_id().value() +
+ " does not match ExecutorID in TaskStatus: " +
+ call.update().status().executor_id().value()
+ );
+ }
+
+ if (status.source() != TaskStatus::SOURCE_EXECUTOR) {
+ return Error("Received Call from executor " +
+ call.executor_id().value() +
+ " of framework " +
+ call.framework_id().value() +
+ " with invalid source, expecting 'SOURCE_EXECUTOR'"
+ );
+ }
+
+ if (status.state() == TASK_STAGING) {
+ return Error("Received TASK_STAGING from executor " +
+ call.executor_id().value() +
+ " of framework " +
+ call.framework_id().value() +
+ " which is not allowed"
+ );
+ }
+
+ // TODO(alexr): Validate `check_status` is present if
+ // the corresponding `TaskInfo.check` has been defined.
+
+ if (status.has_check_status()) {
+ Option<Error> validate =
+ common::validation::validateCheckStatusInfo(status.check_status());
+
+ if (validate.isSome()) {
+ return validate.get();
+ }
+ }
+
+ return None();
+ }
+
+ case mesos::executor::Call::MESSAGE: {
+ if (!call.has_message()) {
+ return Error("Expecting 'message' to be present");
+ }
+ return None();
+ }
+
+ case mesos::executor::Call::UNKNOWN: {
+ return None();
+ }
+ }
+
+ UNREACHABLE();
+}
+
} // namespace validation {
} // namespace common {
} // namespace internal {
http://git-wip-us.apache.org/repos/asf/mesos/blob/7b785c75/src/common/validation.hpp
----------------------------------------------------------------------
diff --git a/src/common/validation.hpp b/src/common/validation.hpp
index fa7f4a7..5929ac2 100644
--- a/src/common/validation.hpp
+++ b/src/common/validation.hpp
@@ -25,6 +25,13 @@
#include <stout/option.hpp>
namespace mesos {
+
+namespace executor {
+
+class Call;
+
+} // namespace executor {
+
namespace internal {
namespace common {
namespace validation {
@@ -60,6 +67,8 @@ Option<Error> validateCheckInfo(const CheckInfo& checkInfo);
Option<Error> validateCheckStatusInfo(const CheckStatusInfo& checkStatusInfo);
+Option<Error> validateExecutorCall(const mesos::executor::Call& call);
+
} // namespace validation {
} // namespace common {
} // namespace internal {
http://git-wip-us.apache.org/repos/asf/mesos/blob/7b785c75/src/executor/executor.cpp
----------------------------------------------------------------------
diff --git a/src/executor/executor.cpp b/src/executor/executor.cpp
index ab67cae..a9e1fcf 100644
--- a/src/executor/executor.cpp
+++ b/src/executor/executor.cpp
@@ -45,14 +45,13 @@
#include "common/http.hpp"
#include "common/recordio.hpp"
+#include "common/validation.hpp"
#include "internal/devolve.hpp"
#include "logging/flags.hpp"
#include "logging/logging.hpp"
-#include "slave/validation.hpp"
-
#include "version/version.hpp"
using namespace mesos;
@@ -64,8 +63,6 @@ using std::string;
using mesos::internal::recordio::Reader;
-using mesos::internal::slave::validation::executor::call::validate;
-
using process::async;
using process::Clock;
using process::delay;
@@ -302,7 +299,8 @@ public:
void send(const Call& call)
{
- Option<Error> error = validate(devolve(call));
+ Option<Error> error =
+ common::validation::validateExecutorCall(devolve(call));
if (error.isSome()) {
drop(call, error->message);
return;
http://git-wip-us.apache.org/repos/asf/mesos/blob/7b785c75/src/slave/http.cpp
----------------------------------------------------------------------
diff --git a/src/slave/http.cpp b/src/slave/http.cpp
index 51d670d..cf93c48 100644
--- a/src/slave/http.cpp
+++ b/src/slave/http.cpp
@@ -61,6 +61,7 @@
#include "common/http.hpp"
#include "common/recordio.hpp"
#include "common/resources_utils.hpp"
+#include "common/validation.hpp"
#include "internal/devolve.hpp"
@@ -767,7 +768,7 @@ Future<Response> Http::executor(
const executor::Call call = devolve(v1Call);
- Option<Error> error = validation::executor::call::validate(call);
+ Option<Error> error = common::validation::validateExecutorCall(call);
if (error.isSome()) {
return BadRequest("Failed to validate Executor::Call: " + error->message);
http://git-wip-us.apache.org/repos/asf/mesos/blob/7b785c75/src/slave/validation.cpp
----------------------------------------------------------------------
diff --git a/src/slave/validation.cpp b/src/slave/validation.cpp
index 3e333a7..df5e137 100644
--- a/src/slave/validation.cpp
+++ b/src/slave/validation.cpp
@@ -25,9 +25,6 @@
#include <stout/stringify.hpp>
#include <stout/unreachable.hpp>
-#include <stout/uuid.hpp>
-
-#include "checks/checker.hpp"
#include "common/resources_utils.hpp"
#include "common/validation.hpp"
@@ -546,113 +543,6 @@ Option<Error> validate(
} // namespace call {
} // namespace agent {
-
-namespace executor {
-namespace call {
-
-Option<Error> validate(const mesos::executor::Call& call)
-{
- if (!call.IsInitialized()) {
- return Error("Not initialized: " + call.InitializationErrorString());
- }
-
- if (!call.has_type()) {
- return Error("Expecting 'type' to be present");
- }
-
- // All calls should have executor id set.
- if (!call.has_executor_id()) {
- return Error("Expecting 'executor_id' to be present");
- }
-
- // All calls should have framework id set.
- if (!call.has_framework_id()) {
- return Error("Expecting 'framework_id' to be present");
- }
-
- switch (call.type()) {
- case mesos::executor::Call::SUBSCRIBE: {
- if (!call.has_subscribe()) {
- return Error("Expecting 'subscribe' to be present");
- }
- return None();
- }
-
- case mesos::executor::Call::UPDATE: {
- if (!call.has_update()) {
- return Error("Expecting 'update' to be present");
- }
-
- const TaskStatus& status = call.update().status();
-
- if (!status.has_uuid()) {
- return Error("Expecting 'uuid' to be present");
- }
-
- Try<id::UUID> uuid = id::UUID::fromBytes(status.uuid());
- if (uuid.isError()) {
- return uuid.error();
- }
-
- if (status.has_executor_id() &&
- status.executor_id().value()
- != call.executor_id().value()) {
- return Error("ExecutorID in Call: " +
- call.executor_id().value() +
- " does not match ExecutorID in TaskStatus: " +
- call.update().status().executor_id().value()
- );
- }
-
- if (status.source() != TaskStatus::SOURCE_EXECUTOR) {
- return Error("Received Call from executor " +
- call.executor_id().value() +
- " of framework " +
- call.framework_id().value() +
- " with invalid source, expecting 'SOURCE_EXECUTOR'"
- );
- }
-
- if (status.state() == TASK_STAGING) {
- return Error("Received TASK_STAGING from executor " +
- call.executor_id().value() +
- " of framework " +
- call.framework_id().value() +
- " which is not allowed"
- );
- }
-
- // TODO(alexr): Validate `check_status` is present if
- // the corresponding `TaskInfo.check` has been defined.
-
- if (status.has_check_status()) {
- Option<Error> validate =
- common::validation::validateCheckStatusInfo(status.check_status());
-
- if (validate.isSome()) {
- return validate.get();
- }
- }
-
- return None();
- }
-
- case mesos::executor::Call::MESSAGE: {
- if (!call.has_message()) {
- return Error("Expecting 'message' to be present");
- }
- return None();
- }
-
- case mesos::executor::Call::UNKNOWN: {
- return None();
- }
- }
- UNREACHABLE();
-}
-
-} // namespace call {
-} // namespace executor {
} // namespace validation {
} // namespace slave {
} // namespace internal {
http://git-wip-us.apache.org/repos/asf/mesos/blob/7b785c75/src/slave/validation.hpp
----------------------------------------------------------------------
diff --git a/src/slave/validation.hpp b/src/slave/validation.hpp
index 3a278e4..c10f615 100644
--- a/src/slave/validation.hpp
+++ b/src/slave/validation.hpp
@@ -19,8 +19,6 @@
#include <mesos/agent/agent.hpp>
-#include <mesos/executor/executor.hpp>
-
#include <stout/error.hpp>
#include <stout/option.hpp>
@@ -47,15 +45,6 @@ Option<Error> validate(
} // namespace call {
} // namespace agent {
-namespace executor {
-namespace call {
-
-// Validates that an executor call is well-formed.
-// TODO(ijimenez): Add unit tests.
-Option<Error> validate(const mesos::executor::Call& call);
-
-} // namespace call {
-} // namespace executor {
} // namespace validation {
} // namespace slave {
} // namespace internal {