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 2018/01/16 23:19:08 UTC
[1/3] mesos git commit: Windows: Fixed docker executor `PATH`
variable.
Repository: mesos
Updated Branches:
refs/heads/1.5.x 8186e9b7b -> af64bcb38
Windows: Fixed docker executor `PATH` variable.
The `docker` executable is not usually installed in
`os::host_default_path()` on Windows, so the Executor cannot find it.
Now, before launching the Executor, the Agent finds the directory
containing `docker` and prepends it to the `PATH` given to the Executor
so that both the Executor and Agent use the same `docker`.
Review: https://reviews.apache.org/r/65147
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/af64bcb3
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/af64bcb3
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/af64bcb3
Branch: refs/heads/1.5.x
Commit: af64bcb38754705020b4e35efccf94e9c85198d6
Parents: 0086ae1
Author: Akash Gupta <ak...@microsoft.com>
Authored: Fri Jan 12 16:23:39 2018 -0800
Committer: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
Committed: Tue Jan 16 13:38:02 2018 -0800
----------------------------------------------------------------------
src/slave/containerizer/docker.cpp | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/af64bcb3/src/slave/containerizer/docker.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/docker.cpp b/src/slave/containerizer/docker.cpp
index b42fe1f..6d9c598 100644
--- a/src/slave/containerizer/docker.cpp
+++ b/src/slave/containerizer/docker.cpp
@@ -43,6 +43,7 @@
#include <stout/uuid.hpp>
#include <stout/os/killtree.hpp>
+#include <stout/os/which.hpp>
#include "common/status_utils.hpp"
@@ -1465,6 +1466,24 @@ Future<pid_t> DockerContainerizerProcess::launchExecutorProcess(
if (environment.count("PATH") == 0) {
environment["PATH"] = os::host_default_path();
+
+ // TODO(andschwa): We will consider removing the `#ifdef` in future, as
+ // other platforms may benefit from being pointed to the same `docker` in
+ // both Agent and Executor (there is a chance that the cleaned path results
+ // in using a different docker, if multiple dockers are installed).
+#ifdef __WINDOWS__
+ // Docker is generally not installed in `os::host_default_path()` on
+ // Windows, so the executor will not be able to find `docker`. We search for
+ // `docker` in `PATH` and prepend the parent directory to
+ // `environment["PATH"]`. We prepend instead of append so that in the off
+ // chance that `docker` is in `host_default_path`, the executor and agent
+ // will use the same `docker`.
+ Option<string> dockerPath = os::which("docker");
+ if (dockerPath.isSome()) {
+ environment["PATH"] =
+ Path(dockerPath.get()).dirname() + ";" + environment["PATH"];
+ }
+#endif // __WINDOWS__
}
vector<string> argv;
[3/3] mesos git commit: Ported `os::which` to Windows.
Posted by an...@apache.org.
Ported `os::which` to Windows.
Because `os::which` still lived in `posix/os.hpp`, it was refactored
into its own `os/which.hpp` (which the respective `os/posix/which.hpp`
and `os/windows/which.hpp`. Consumers of this will need additionally
include the new header, instead of just `os.hpp`.
The differences in implementation from POSIX to Windows are:
* Split the `PATH` on `;` not `:`.
* Also loop over `PATHEXT` because executables on Windows end with one
of a set of extensions.
* Removed the permissions check because it is not applicable on
Windows (instead, an executable ends with an extension in `PATHEXT`,
see above).
Review: https://reviews.apache.org/r/65144
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/b8e66e23
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/b8e66e23
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/b8e66e23
Branch: refs/heads/1.5.x
Commit: b8e66e23339ce02a0129720554a29e0b1b56059b
Parents: 8186e9b
Author: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
Authored: Fri Jan 12 15:14:43 2018 -0800
Committer: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
Committed: Tue Jan 16 13:38:02 2018 -0800
----------------------------------------------------------------------
3rdparty/stout/include/Makefile.am | 3 +
3rdparty/stout/include/stout/os/posix/which.hpp | 72 ++++++++++++++++
3rdparty/stout/include/stout/os/which.hpp | 24 ++++++
.../stout/include/stout/os/windows/which.hpp | 90 ++++++++++++++++++++
3rdparty/stout/include/stout/posix/os.hpp | 39 ---------
3rdparty/stout/tests/os_tests.cpp | 6 +-
6 files changed, 191 insertions(+), 43 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/b8e66e23/3rdparty/stout/include/Makefile.am
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/Makefile.am b/3rdparty/stout/include/Makefile.am
index e5f1104..742bfc4 100644
--- a/3rdparty/stout/include/Makefile.am
+++ b/3rdparty/stout/include/Makefile.am
@@ -115,6 +115,7 @@ nobase_include_HEADERS = \
stout/os/touch.hpp \
stout/os/utime.hpp \
stout/os/wait.hpp \
+ stout/os/which.hpp \
stout/os/write.hpp \
stout/os/xattr.hpp \
stout/os/posix/bootid.hpp \
@@ -152,6 +153,7 @@ nobase_include_HEADERS = \
stout/os/posix/stat.hpp \
stout/os/posix/su.hpp \
stout/os/posix/temp.hpp \
+ stout/os/posix/which.hpp \
stout/os/posix/write.hpp \
stout/os/posix/xattr.hpp \
stout/os/raw/argv.hpp \
@@ -191,6 +193,7 @@ nobase_include_HEADERS = \
stout/os/windows/stat.hpp \
stout/os/windows/su.hpp \
stout/os/windows/temp.hpp \
+ stout/os/windows/which.hpp \
stout/os/windows/write.hpp \
stout/os/windows/xattr.hpp \
stout/path.hpp \
http://git-wip-us.apache.org/repos/asf/mesos/blob/b8e66e23/3rdparty/stout/include/stout/os/posix/which.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/os/posix/which.hpp b/3rdparty/stout/include/stout/os/posix/which.hpp
new file mode 100644
index 0000000..96586dd
--- /dev/null
+++ b/3rdparty/stout/include/stout/os/posix/which.hpp
@@ -0,0 +1,72 @@
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef __STOUT_OS_POSIX_WHICH_HPP__
+#define __STOUT_OS_POSIX_WHICH_HPP__
+
+#include <string>
+#include <vector>
+
+#include <stout/none.hpp>
+#include <stout/option.hpp>
+#include <stout/os.hpp>
+#include <stout/path.hpp>
+#include <stout/strings.hpp>
+
+#include <stout/os/exists.hpp>
+#include <stout/os/permissions.hpp>
+
+
+namespace os {
+
+inline Option<std::string> which(
+ const std::string& command,
+ const Option<std::string>& _path = None())
+{
+ Option<std::string> path = _path;
+
+ if (path.isNone()) {
+ path = getenv("PATH");
+
+ if (path.isNone()) {
+ return None();
+ }
+ }
+
+ std::vector<std::string> tokens = strings::tokenize(path.get(), ":");
+ foreach (const std::string& token, tokens) {
+ const std::string commandPath = path::join(token, command);
+ if (!os::exists(commandPath)) {
+ continue;
+ }
+
+ Try<os::Permissions> permissions = os::permissions(commandPath);
+ if (permissions.isError()) {
+ continue;
+ }
+
+ if (!permissions.get().owner.x &&
+ !permissions.get().group.x &&
+ !permissions.get().others.x) {
+ continue;
+ }
+
+ return commandPath;
+ }
+
+ return None();
+}
+
+} // namespace os {
+
+
+#endif // __STOUT_OS_POSIX_WHICH_HPP__
http://git-wip-us.apache.org/repos/asf/mesos/blob/b8e66e23/3rdparty/stout/include/stout/os/which.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/os/which.hpp b/3rdparty/stout/include/stout/os/which.hpp
new file mode 100644
index 0000000..af0a7e8
--- /dev/null
+++ b/3rdparty/stout/include/stout/os/which.hpp
@@ -0,0 +1,24 @@
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef __STOUT_OS_WHICH_HPP__
+#define __STOUT_OS_WHICH_HPP__
+
+// For readability, we minimize the number of #ifdef blocks in the code by
+// splitting platform specific system calls into separate directories.
+#ifdef __WINDOWS__
+#include <stout/os/windows/which.hpp>
+#else
+#include <stout/os/posix/which.hpp>
+#endif // __WINDOWS__
+
+#endif // __STOUT_OS_WHICH_HPP__
http://git-wip-us.apache.org/repos/asf/mesos/blob/b8e66e23/3rdparty/stout/include/stout/os/windows/which.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/os/windows/which.hpp b/3rdparty/stout/include/stout/os/windows/which.hpp
new file mode 100644
index 0000000..fc541cc
--- /dev/null
+++ b/3rdparty/stout/include/stout/os/windows/which.hpp
@@ -0,0 +1,90 @@
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef __STOUT_OS_WINDOWS_WHICH_HPP__
+#define __STOUT_OS_WINDOWS_WHICH_HPP__
+
+#include <string>
+#include <vector>
+
+#include <stout/none.hpp>
+#include <stout/option.hpp>
+#include <stout/os.hpp>
+#include <stout/path.hpp>
+#include <stout/strings.hpp>
+
+#include <stout/os/exists.hpp>
+
+
+namespace os {
+
+// This behaves "like the user expects" of POSIX `which`, but on Windows. That
+// is, if a path is not specified, we search through the `PATH` environment
+// variable, but explicitly do not search the current working directory (which
+// `CreateProcess` and Windows' `where` would do).
+//
+// Because the executable permission does not work on Windows, the closest
+// equivalent is to check the path extension against those listed in `PATHEXT`.
+// However, we first search for exactly the file name the user specified with
+// `command`, regardless of extension, because it could be an executable. If an
+// exact match is not found, we continue the search with the environment's
+// "executable" extensions.
+inline Option<std::string> which(
+ const std::string& command,
+ const Option<std::string>& _path = None())
+{
+ Option<std::string> path = _path;
+
+ if (path.isNone()) {
+ path = os::getenv("PATH");
+
+ if (path.isNone()) {
+ return None();
+ }
+ }
+
+ Option<std::string> pathext = os::getenv("PATHEXT");
+
+ if (pathext.isNone()) {
+ pathext = ".COM;.EXE;.BAT;.CMD";
+ }
+
+ std::vector<std::string> tokens = strings::tokenize(path.get(), ";");
+ std::vector<std::string> exts = strings::tokenize(pathext.get(), ";");
+
+ // NOTE: This handles the edge case of `command` already having an extension,
+ // e.g. `docker.exe`. By starting with the case of "", we don't have to
+ // special case the loops below.
+ exts.insert(exts.begin(), "");
+
+ // Nested loops, but fairly finite. This is how `where` works on Windows
+ // (which is the equivalent of `which`). The loops are nested such that we
+ // first search through `PATH` for `command`, then through `PATH` for
+ // `command.COM` and so on.
+ foreach (const std::string& ext, exts) {
+ foreach (const std::string& token, tokens) {
+ const std::string commandPath = path::join(token, command + ext);
+ if (!os::exists(commandPath)) {
+ continue;
+ }
+
+ return commandPath;
+ }
+ }
+
+ return None();
+}
+
+} // namespace os {
+
+
+#endif // __STOUT_OS_WINDOWS_WHICH_HPP__
http://git-wip-us.apache.org/repos/asf/mesos/blob/b8e66e23/3rdparty/stout/include/stout/posix/os.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/posix/os.hpp b/3rdparty/stout/include/stout/posix/os.hpp
index 7427bd7..3c52a49 100644
--- a/3rdparty/stout/include/stout/posix/os.hpp
+++ b/3rdparty/stout/include/stout/posix/os.hpp
@@ -403,45 +403,6 @@ inline Try<Version> release()
}
-inline Option<std::string> which(
- const std::string& command,
- const Option<std::string>& _path = None())
-{
- Option<std::string> path = _path;
-
- if (path.isNone()) {
- path = getenv("PATH");
-
- if (path.isNone()) {
- return None();
- }
- }
-
- std::vector<std::string> tokens = strings::tokenize(path.get(), ":");
- foreach (const std::string& token, tokens) {
- const std::string commandPath = path::join(token, command);
- if (!os::exists(commandPath)) {
- continue;
- }
-
- Try<os::Permissions> permissions = os::permissions(commandPath);
- if (permissions.isError()) {
- continue;
- }
-
- if (!permissions.get().owner.x &&
- !permissions.get().group.x &&
- !permissions.get().others.x) {
- continue;
- }
-
- return commandPath;
- }
-
- return None();
-}
-
-
inline Try<std::string> var()
{
return "/var";
http://git-wip-us.apache.org/repos/asf/mesos/blob/b8e66e23/3rdparty/stout/tests/os_tests.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/tests/os_tests.cpp b/3rdparty/stout/tests/os_tests.cpp
index de41077..11f1720 100644
--- a/3rdparty/stout/tests/os_tests.cpp
+++ b/3rdparty/stout/tests/os_tests.cpp
@@ -50,6 +50,7 @@
#include <stout/os/killtree.hpp>
#include <stout/os/realpath.hpp>
#include <stout/os/stat.hpp>
+#include <stout/os/which.hpp>
#include <stout/os/write.hpp>
#if defined(__APPLE__) || defined(__FreeBSD__)
@@ -1035,15 +1036,12 @@ TEST_F(OsTest, SYMLINK_Realpath)
}
-// NOTE: Disabled on Windows because `which` doesn't exist.
-#ifndef __WINDOWS__
TEST_F(OsTest, Which)
{
// TODO(jieyu): Test PATH search ordering and file execution bit.
- Option<string> which = os::which("ls");
+ Option<string> which = os::which("ping");
ASSERT_SOME(which);
which = os::which("bar");
EXPECT_NONE(which);
}
-#endif // __WINDOWS__
[2/3] mesos git commit: Fixed use of `os::which`.
Posted by an...@apache.org.
Fixed use of `os::which`.
Because `os::which` was added to its own header, all uses of it now need
to include said header.
In `tests/environment.cpp`, instead of using `os::system("which foo")`,
we now use `os::which("foo")` to be compatible with Windows.
Review: https://reviews.apache.org/r/65145
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/0086ae1a
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/0086ae1a
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/0086ae1a
Branch: refs/heads/1.5.x
Commit: 0086ae1ae2525a49adbbcaa04f0ffd280b362a26
Parents: b8e66e2
Author: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
Authored: Fri Jan 12 15:17:02 2018 -0800
Committer: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
Committed: Tue Jan 16 13:38:02 2018 -0800
----------------------------------------------------------------------
.../mesos/isolators/docker/volume/isolator.cpp | 1 +
.../mesos/isolators/network/cni/cni.cpp | 1 +
.../cni/plugins/port_mapper/port_mapper.cpp | 2 ++
src/slave/containerizer/mesos/launch.cpp | 1 +
src/tests/environment.cpp | 18 ++++++++++--------
5 files changed, 15 insertions(+), 8 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/0086ae1a/src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp b/src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp
index ba9e20c..44cf9b2 100644
--- a/src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp
+++ b/src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp
@@ -22,6 +22,7 @@
#include <stout/os.hpp>
#include <stout/os/realpath.hpp>
+#include <stout/os/which.hpp>
#include "slave/flags.hpp"
#include "slave/state.hpp"
http://git-wip-us.apache.org/repos/asf/mesos/blob/0086ae1a/src/slave/containerizer/mesos/isolators/network/cni/cni.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/isolators/network/cni/cni.cpp b/src/slave/containerizer/mesos/isolators/network/cni/cni.cpp
index 61de16b..af1d477 100644
--- a/src/slave/containerizer/mesos/isolators/network/cni/cni.cpp
+++ b/src/slave/containerizer/mesos/isolators/network/cni/cni.cpp
@@ -33,6 +33,7 @@
#include <stout/os/constants.hpp>
#include <stout/os/realpath.hpp>
+#include <stout/os/which.hpp>
#include "common/protobuf_utils.hpp"
http://git-wip-us.apache.org/repos/asf/mesos/blob/0086ae1a/src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp b/src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp
index de64d65..c40b57f 100644
--- a/src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp
+++ b/src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp
@@ -17,6 +17,8 @@
#include <stout/os.hpp>
#include <stout/protobuf.hpp>
+#include <stout/os/which.hpp>
+
#include <process/collect.hpp>
#include <process/dispatch.hpp>
#include <process/io.hpp>
http://git-wip-us.apache.org/repos/asf/mesos/blob/0086ae1a/src/slave/containerizer/mesos/launch.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/launch.cpp b/src/slave/containerizer/mesos/launch.cpp
index c45a038..2328ea9 100644
--- a/src/slave/containerizer/mesos/launch.cpp
+++ b/src/slave/containerizer/mesos/launch.cpp
@@ -39,6 +39,7 @@
#include <stout/unreachable.hpp>
#include <stout/os/realpath.hpp>
+#include <stout/os/which.hpp>
#include <stout/os/write.hpp>
#include <mesos/mesos.hpp>
http://git-wip-us.apache.org/repos/asf/mesos/blob/0086ae1a/src/tests/environment.cpp
----------------------------------------------------------------------
diff --git a/src/tests/environment.cpp b/src/tests/environment.cpp
index 72bd621..13a4c95 100644
--- a/src/tests/environment.cpp
+++ b/src/tests/environment.cpp
@@ -50,6 +50,7 @@
#include <stout/os/pstree.hpp>
#include <stout/os/shell.hpp>
#include <stout/os/temp.hpp>
+#include <stout/os/which.hpp>
#ifdef __linux__
#include "linux/cgroups.hpp"
@@ -215,7 +216,7 @@ class CurlFilter : public TestFilter
public:
CurlFilter()
{
- curlError = os::system("which curl") != 0;
+ curlError = os::which("curl").isNone();
if (curlError) {
std::cerr
<< "-------------------------------------------------------------\n"
@@ -240,8 +241,8 @@ class NvidiaGpuFilter : public TestFilter
public:
NvidiaGpuFilter()
{
- exists = os::system("which nvidia-smi") == 0;
- if (!exists) {
+ nvidiaGpuError = os::which("nvidia-smi").isNone();
+ if (nvidiaGpuError) {
std::cerr
<< "-------------------------------------------------------------\n"
<< "No 'nvidia-smi' command found so no Nvidia GPU tests will run\n"
@@ -252,11 +253,11 @@ public:
bool disable(const ::testing::TestInfo* test) const
{
- return matches(test, "NVIDIA_GPU_") && !exists;
+ return matches(test, "NVIDIA_GPU_") && nvidiaGpuError;
}
private:
- bool exists;
+ bool nvidiaGpuError;
};
@@ -396,6 +397,7 @@ public:
InternetFilter()
{
error = os::system("ping -c 1 -W 1 google.com") != 0;
+ // TODO(andschwa): Make ping command cross-platform.
if (error) {
std::cerr
<< "-------------------------------------------------------------\n"
@@ -420,7 +422,7 @@ class LogrotateFilter : public TestFilter
public:
LogrotateFilter()
{
- logrotateError = os::system("which logrotate") != 0;
+ logrotateError = os::which("logrotate").isNone();
if (logrotateError) {
std::cerr
<< "-------------------------------------------------------------\n"
@@ -446,7 +448,7 @@ class NetcatFilter : public TestFilter
public:
NetcatFilter()
{
- netcatError = os::system("which nc") != 0;
+ netcatError = os::which("nc").isNone();
if (netcatError) {
std::cerr
<< "-------------------------------------------------------------\n"
@@ -736,7 +738,7 @@ class UnzipFilter : public TestFilter
public:
UnzipFilter()
{
- unzipError = os::system("which unzip") != 0;
+ unzipError = os::which("unzip").isNone();
if (unzipError) {
std::cerr
<< "-------------------------------------------------------------\n"