Repository: mesos Updated Branches: refs/heads/1.2.x e88229982 -> 89d141c6d
Docker environment gets passed on docker run command. Removes the use of --env_file as that does not support newlines in environment variable values. Also avoids leaking of possibly sensitive environment variables to the log. Manually backported for 1.2.x. Review: https://reviews.apache.org/r/57860/ Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/89d141c6 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/89d141c6 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/89d141c6 Branch: refs/heads/1.2.x Commit: 89d141c6dddfa944e6f6c271a78a1c00bbd1ca14 Parents: e882299 Author: Till Toenshoff <[email protected]> Authored: Wed Mar 22 21:21:22 2017 +0100 Committer: Till Toenshoff <[email protected]> Committed: Fri Mar 24 06:44:47 2017 +0100 ---------------------------------------------------------------------- src/docker/docker.cpp | 67 ++++++++++++---------------------------------- 1 file changed, 17 insertions(+), 50 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/89d141c6/src/docker/docker.cpp ---------------------------------------------------------------------- diff --git a/src/docker/docker.cpp b/src/docker/docker.cpp index 68042fd..d423d56 100755 --- a/src/docker/docker.cpp +++ b/src/docker/docker.cpp @@ -532,58 +532,34 @@ Future<Option<int>> Docker::run( } } - string environmentVariables; + map<string, string> environmentVariables; if (env.isSome()) { - foreachpair (const string& key, const string& value, env.get()) { - environmentVariables += key + "=" + value + "\n"; - } + environmentVariables = env.get(); } + // Note that this is not following our common overwriting schema + // when it comes to duplicates. We are leaving this unchanged in + // 1.2.x to avoid surprises for usages that depend on this long + // established behaviour. foreach (const Environment::Variable& variable, commandInfo.environment().variables()) { - if (env.isSome() && - env.get().find(variable.name()) != env.get().end()) { + if (environmentVariables.find(variable.name()) != + environmentVariables.end()) { // Skip to avoid duplicate environment variables. continue; } - environmentVariables += variable.name() + "=" + variable.value() + "\n"; - } - - environmentVariables += "MESOS_SANDBOX=" + mappedDirectory + "\n"; - environmentVariables += "MESOS_CONTAINER_NAME=" + name + "\n"; - - Try<string> environmentFile_ = os::mktemp(); - if (environmentFile_.isError()) { - return Failure("Failed to create temporary docker environment " - "file: " + environmentFile_.error()); + environmentVariables[variable.name()] = variable.value(); } - const string& environmentFile = environmentFile_.get(); + environmentVariables["MESOS_SANDBOX"] = mappedDirectory; + environmentVariables["MESOS_CONTAINER_NAME"] = name; - Try<int_fd> fd = os::open( - environmentFile, - O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC, - S_IRUSR | S_IWUSR); - - if (fd.isError()) { - return Failure( - "Failed to open file '" + environmentFile + "': " + fd.error()); + foreachpair(const string& key, const string& value, environmentVariables) { + argv.push_back("-e"); + argv.push_back(key + "=" + value); } - Try<Nothing> write = os::write(fd.get(), environmentVariables); - - os::close(fd.get()); - - if (write.isError()) { - return Failure( - "Failed to write docker environment file to '" + environmentFile + - "': " + write.error()); - } - - argv.push_back("--env-file"); - argv.push_back(environmentFile); - Option<string> volumeDriver; foreach (const Volume& volume, containerInfo.volumes()) { // The 'container_path' can be either an absolute path or a @@ -847,7 +823,7 @@ Future<Option<int>> Docker::run( string cmd = strings::join(" ", argv); - LOG(INFO) << "Running " << cmd; + VLOG(1) << "Running " << cmd; map<string, string> environment = os::environment(); @@ -865,19 +841,10 @@ Future<Option<int>> Docker::run( environment); if (s.isError()) { - return Failure("Failed to create subprocess '" + cmd + "': " + s.error()); + return Failure("Failed to create subprocess '" + path + "': " + s.error()); } - s->status() - .onDiscard(lambda::bind(&commandDiscarded, s.get(), cmd)) - .onAny([environmentFile]() { - Try<Nothing> rm = os::rm(environmentFile); - - if (rm.isError()) { - LOG(WARNING) << "Failed to remove temporary docker environment file " - << "'" << environmentFile << "': " << rm.error(); - } - }); + s->status().onDiscard(lambda::bind(&commandDiscarded, s.get(), cmd)); // Ideally we could capture the stderr when docker itself fails, // however due to the stderr redirection used here we cannot.
