You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by al...@apache.org on 2018/06/15 13:41:07 UTC
[2/3] mesos git commit: Removed `os::getenv()` calls from
`MesosProcess`.
Removed `os::getenv()` calls from `MesosProcess`.
This patch adds overloaded constructor for `v1::executor::Mesos` class
that accepts `environment` parameter that is passed over to
`MesosProcess` constructor. This change is needed to get rid of
`os::getenv()` calls, so that v1 Executor can be used in tests that
require thread safety.
Review: https://reviews.apache.org/r/67355/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/aea68883
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/aea68883
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/aea68883
Branch: refs/heads/master
Commit: aea688832d8f2ee9ff6b7fff702ffc75e71dfdb1
Parents: 20edb8c
Author: Andrei Budnik <ab...@mesosphere.com>
Authored: Fri Jun 15 15:23:32 2018 +0200
Committer: Alexander Rukletsov <al...@apache.org>
Committed: Fri Jun 15 15:40:54 2018 +0200
----------------------------------------------------------------------
include/mesos/v1/executor.hpp | 12 ++++++++++
src/executor/executor.cpp | 48 ++++++++++++++++++++++++++++++--------
src/tests/mesos.hpp | 6 +++--
3 files changed, 54 insertions(+), 12 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/aea68883/include/mesos/v1/executor.hpp
----------------------------------------------------------------------
diff --git a/include/mesos/v1/executor.hpp b/include/mesos/v1/executor.hpp
index ca48f29..9a2eb45 100644
--- a/include/mesos/v1/executor.hpp
+++ b/include/mesos/v1/executor.hpp
@@ -19,6 +19,7 @@
#include <functional>
#include <queue>
+#include <map>
#include <string>
#include <mesos/http.hpp>
@@ -54,11 +55,22 @@ public:
class Mesos : public MesosBase
{
public:
+ // The other constructor overload that accepts `environment`
+ // argument is preferable to this one in a multithreaded environment,
+ // because the implementation of this one accesses global environment
+ // which is unsafe due to a potential concurrent modification of the
+ // environment by another thread.
Mesos(ContentType contentType,
const std::function<void(void)>& connected,
const std::function<void(void)>& disconnected,
const std::function<void(const std::queue<Event>&)>& received);
+ Mesos(ContentType contentType,
+ const std::function<void(void)>& connected,
+ const std::function<void(void)>& disconnected,
+ const std::function<void(const std::queue<Event>&)>& received,
+ const std::map<std::string, std::string>& environment);
+
// Delete copy constructor.
Mesos(const Mesos& other) = delete;
http://git-wip-us.apache.org/repos/asf/mesos/blob/aea68883/src/executor/executor.cpp
----------------------------------------------------------------------
diff --git a/src/executor/executor.cpp b/src/executor/executor.cpp
index 5e95f99..ab67cae 100644
--- a/src/executor/executor.cpp
+++ b/src/executor/executor.cpp
@@ -157,7 +157,8 @@ public:
ContentType _contentType,
const lambda::function<void(void)>& connected,
const lambda::function<void(void)>& disconnected,
- const lambda::function<void(const queue<Event>&)>& received)
+ const lambda::function<void(const queue<Event>&)>& received,
+ const std::map<std::string, std::string>& environment)
: ProcessBase(generate("executor")),
state(DISCONNECTED),
contentType(_contentType),
@@ -168,7 +169,18 @@ public:
// Load any logging flags from the environment.
logging::Flags flags;
- Try<flags::Warnings> load = flags.load("MESOS_");
+ // Filter out environment variables whose keys don't start with "MESOS_".
+ //
+ // TODO(alexr): This should be supported by `FlagsBase`, see MESOS-9001.
+ std::map<std::string, std::string> mesosEnvironment;
+
+ foreachpair (const string& key, const string& value, environment) {
+ if (strings::startsWith(key, "MESOS_")) {
+ mesosEnvironment.emplace(key, value);
+ }
+ }
+
+ Try<flags::Warnings> load = flags.load(mesosEnvironment, true);
if (load.isError()) {
EXIT(EXIT_FAILURE) << "Failed to load flags: " << load.error();
@@ -193,13 +205,15 @@ public:
spawn(new VersionProcess(), true);
+ hashmap<string, string> env(mesosEnvironment);
+
// Check if this is local (for example, for testing).
- local = os::getenv("MESOS_LOCAL").isSome();
+ local = env.get("MESOS_LOCAL").isSome();
Option<string> value;
// Get agent PID from environment.
- value = os::getenv("MESOS_SLAVE_PID");
+ value = env.get("MESOS_SLAVE_PID");
if (value.isNone()) {
EXIT(EXIT_FAILURE)
<< "Expecting 'MESOS_SLAVE_PID' to be set in the environment";
@@ -223,7 +237,7 @@ public:
upid.id +
"/api/v1/executor");
- value = os::getenv("MESOS_EXECUTOR_AUTHENTICATION_TOKEN");
+ value = env.get("MESOS_EXECUTOR_AUTHENTICATION_TOKEN");
if (value.isSome()) {
authenticationToken = value.get();
}
@@ -233,12 +247,12 @@ public:
os::eraseenv("MESOS_EXECUTOR_AUTHENTICATION_TOKEN");
// Get checkpointing status from environment.
- value = os::getenv("MESOS_CHECKPOINT");
+ value = env.get("MESOS_CHECKPOINT");
checkpoint = value.isSome() && value.get() == "1";
if (checkpoint) {
// Get recovery timeout from environment.
- value = os::getenv("MESOS_RECOVERY_TIMEOUT");
+ value = env.get("MESOS_RECOVERY_TIMEOUT");
if (value.isSome()) {
Try<Duration> _recoveryTimeout = Duration::parse(value.get());
@@ -253,7 +267,7 @@ public:
}
// Get maximum backoff factor from environment.
- value = os::getenv("MESOS_SUBSCRIPTION_BACKOFF_MAX");
+ value = env.get("MESOS_SUBSCRIPTION_BACKOFF_MAX");
if (value.isSome()) {
Try<Duration> _maxBackoff = Duration::parse(value.get());
@@ -270,7 +284,7 @@ public:
}
// Get executor shutdown grace period from the environment.
- value = os::getenv("MESOS_EXECUTOR_SHUTDOWN_GRACE_PERIOD");
+ value = env.get("MESOS_EXECUTOR_SHUTDOWN_GRACE_PERIOD");
if (value.isSome()) {
Try<Duration> _shutdownGracePeriod = Duration::parse(value.get());
@@ -833,7 +847,21 @@ Mesos::Mesos(
const lambda::function<void(void)>& connected,
const lambda::function<void(void)>& disconnected,
const lambda::function<void(const queue<Event>&)>& received)
- : process(new MesosProcess(contentType, connected, disconnected, received))
+ : process(new MesosProcess(
+ contentType, connected, disconnected, received, os::environment()))
+{
+ spawn(process.get());
+}
+
+
+Mesos::Mesos(
+ ContentType contentType,
+ const lambda::function<void(void)>& connected,
+ const lambda::function<void(void)>& disconnected,
+ const lambda::function<void(const queue<Event>&)>& received,
+ const std::map<std::string, std::string>& environment)
+ : process(new MesosProcess(
+ contentType, connected, disconnected, received, environment))
{
spawn(process.get());
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/aea68883/src/tests/mesos.hpp
----------------------------------------------------------------------
diff --git a/src/tests/mesos.hpp b/src/tests/mesos.hpp
index 8f529fa..a23b02c 100644
--- a/src/tests/mesos.hpp
+++ b/src/tests/mesos.hpp
@@ -2836,7 +2836,8 @@ class TestMesos : public Mesos
public:
TestMesos(
ContentType contentType,
- const std::shared_ptr<MockHTTPExecutor<Mesos, Event>>& executor)
+ const std::shared_ptr<MockHTTPExecutor<Mesos, Event>>& executor,
+ const std::map<std::string, std::string>& environment)
: Mesos(
contentType,
lambda::bind(&MockHTTPExecutor<Mesos, Event>::connected,
@@ -2848,7 +2849,8 @@ public:
lambda::bind(&MockHTTPExecutor<Mesos, Event>::events,
executor,
this,
- lambda::_1)) {}
+ lambda::_1),
+ environment) {}
};
} // namespace executor {