You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by an...@apache.org on 2017/11/30 03:28:30 UTC
[11/13] mesos git commit: Windows: Fixed MESOS-6816 to enable
`ExecutorEnvironmentVariables`.
Windows: Fixed MESOS-6816 to enable `ExecutorEnvironmentVariables`.
By removing the explicit system environment override code, and instead
setting the system environment as the default in the `CreateProcess`
wrapper, the `SlaveTest.ExecutorEnvironmentVariables` can be enabled.
Note that this also required fixing `os::host_default_path()` and using
it, because the test used `PATH = /bin`, which breaks on Windows.
Review: https://reviews.apache.org/r/63816
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/0dee8b1c
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/0dee8b1c
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/0dee8b1c
Branch: refs/heads/master
Commit: 0dee8b1c96b94edd39c6fe4eed68371423c435ff
Parents: 76ce648
Author: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
Authored: Fri Nov 3 12:36:22 2017 -0700
Committer: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
Committed: Wed Nov 29 19:24:11 2017 -0800
----------------------------------------------------------------------
src/slave/containerizer/docker.cpp | 14 --------------
src/slave/containerizer/mesos/launch.cpp | 14 --------------
src/tests/slave_tests.cpp | 22 +++++++++++++++-------
3 files changed, 15 insertions(+), 35 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/0dee8b1c/src/slave/containerizer/docker.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/docker.cpp b/src/slave/containerizer/docker.cpp
index 9918d83..d837442 100644
--- a/src/slave/containerizer/docker.cpp
+++ b/src/slave/containerizer/docker.cpp
@@ -1459,20 +1459,6 @@ Future<pid_t> DockerContainerizerProcess::launchExecutorProcess(
environment["PATH"] = os::host_default_path();
}
-#ifdef __WINDOWS__
- // TODO(dpravat): (MESOS-6816) We should allow system environment variables to
- // be overwritten if they are specified by the framework. This might cause
- // applications to not work, but upon overriding system defaults, it becomes
- // the overidder's problem.
- Option<map<std::wstring, std::wstring>> systemEnvironment =
- ::internal::windows::get_system_env();
- foreachpair(const std::wstring& key,
- const std::wstring& value,
- systemEnvironment.get()) {
- environment[stringify(key)] = stringify(value);
- }
-#endif // __WINDOWS__
-
vector<string> argv;
argv.push_back(MESOS_DOCKER_EXECUTOR);
http://git-wip-us.apache.org/repos/asf/mesos/blob/0dee8b1c/src/slave/containerizer/mesos/launch.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/launch.cpp b/src/slave/containerizer/mesos/launch.cpp
index 4fabf92..b1065be 100644
--- a/src/slave/containerizer/mesos/launch.cpp
+++ b/src/slave/containerizer/mesos/launch.cpp
@@ -962,20 +962,6 @@ int MesosContainerizerLaunch::execute()
environment["PATH"] = os::host_default_path();
}
-#ifdef __WINDOWS__
- // TODO(dpravat): (MESOS-6816) We should allow system environment variables
- // to be overwritten if they are specified by the framework. This might
- // cause applications to not work, but upon overriding system defaults, it
- // becomes the overidder's problem.
- Option<std::map<std::wstring, std::wstring>> systemEnvironment =
- ::internal::windows::get_system_env();
- foreachpair (const std::wstring& key,
- const std::wstring& value,
- systemEnvironment.get()) {
- environment[stringify(key)] = stringify(value);
- }
-#endif // __WINDOWS__
-
envp = os::raw::Envp(environment);
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/0dee8b1c/src/tests/slave_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/slave_tests.cpp b/src/tests/slave_tests.cpp
index b35497b..6e4a860 100644
--- a/src/tests/slave_tests.cpp
+++ b/src/tests/slave_tests.cpp
@@ -5222,8 +5222,8 @@ TEST_F(SlaveTest, TaskStatusContainerStatus)
// Test that we can set the executors environment variables and it
-// won't inhert the slaves.
-TEST_F_TEMP_DISABLED_ON_WINDOWS(SlaveTest, ExecutorEnvironmentVariables)
+// won't inherit the slaves.
+TEST_F(SlaveTest, ExecutorEnvironmentVariables)
{
Try<Owned<cluster::Master>> master = StartMaster();
ASSERT_SOME(master);
@@ -5231,11 +5231,9 @@ TEST_F_TEMP_DISABLED_ON_WINDOWS(SlaveTest, ExecutorEnvironmentVariables)
// Need flags for 'executor_environment_variables'.
slave::Flags flags = CreateSlaveFlags();
- Try<JSON::Object> parse = JSON::parse<JSON::Object>("{\"PATH\": \"/bin\"}");
+ const std::string path = os::host_default_path();
- ASSERT_SOME(parse);
-
- flags.executor_environment_variables = parse.get();
+ flags.executor_environment_variables = JSON::Object{{"PATH", path}};
Owned<MasterDetector> detector = master.get()->createDetector();
Try<Owned<cluster::Slave>> slave = StartSlave(detector.get(), flags);
@@ -5266,8 +5264,18 @@ TEST_F_TEMP_DISABLED_ON_WINDOWS(SlaveTest, ExecutorEnvironmentVariables)
// Command executor will run as user running test.
CommandInfo command;
+#ifdef __WINDOWS__
+ command.set_shell(false);
+ command.set_value("powershell.exe");
+ command.add_arguments("powershell.exe");
+ command.add_arguments("-NoProfile");
+ command.add_arguments("-Command");
+ command.add_arguments(
+ "if ($env:PATH -eq '" + path + "') { exit 0 } else { exit 1 }");
+#else
command.set_shell(true);
- command.set_value("test $PATH = /bin");
+ command.set_value("test $PATH = " + path);
+#endif // __WINDOWS__
task.mutable_command()->MergeFrom(command);