[3/3] mesos git commit: Fixed environment duplication in command executor.

[al...@apache.org]

Fixed environment duplication in command executor.

Review: https://reviews.apache.org/r/57762/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/a16fb6ce
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/a16fb6ce
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/a16fb6ce

Branch: refs/heads/1.2.x
Commit: a16fb6ce1864a89d73fa3713514fb0a699b1cb59
Parents: f62f570
Author: Till Toenshoff <to...@me.com>
Authored: Fri Mar 24 06:57:42 2017 +0100
Committer: Alexander Rukletsov <al...@apache.org>
Committed: Fri Mar 24 07:12:53 2017 +0100

----------------------------------------------------------------------
 src/launcher/executor.cpp | 44 ++++++++++++++++++++++++++++++++++++------
 1 file changed, 38 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/a16fb6ce/src/launcher/executor.cpp
----------------------------------------------------------------------
diff --git a/src/launcher/executor.cpp b/src/launcher/executor.cpp
index d9417ce..59ce5f8 100644
--- a/src/launcher/executor.cpp
+++ b/src/launcher/executor.cpp
@@ -403,20 +403,52 @@ protected:
     // TODO(josephw): Windows tasks will inherit the environment
     // from the executor for now. Change this if a Windows isolator
     // ever uses the `--task_environment` flag.
-    Environment launchEnvironment;
+    //
+    // TODO(tillt): Consider logging in detail the original environment
+    // variable source and overwriting source.
+    //
+    // TODO(tillt): Consider implementing a generic, reusable solution
+    // for merging these environments. See MESOS-7299.
+    //
+    // Note that we can not use protobuf message merging as that could
+    // cause duplicate keys in the resulting environment.
+    hashmap<string, Environment::Variable> environment;
 
     foreachpair (const string& name, const string& value, os::environment()) {
-      Environment::Variable* variable = launchEnvironment.add_variables();
-      variable->set_name(name);
-      variable->set_value(value);
+      Environment::Variable variable;
+      variable.set_name(name);
+      variable.set_type(Environment::Variable::VALUE);
+      variable.set_value(value);
+      environment[name] = variable;
     }
 
     if (taskEnvironment.isSome()) {
-      launchEnvironment.MergeFrom(taskEnvironment.get());
+      foreach (const Environment::Variable& variable,
+               taskEnvironment->variables()) {
+        const string& name = variable.name();
+        if (environment.contains(name) &&
+            environment[name].value() != variable.value()) {
+          cout << "Overwriting environment variable '" << name << "'" << endl;
+        }
+        environment[name] = variable;
+      }
     }
 
     if (command.has_environment()) {
-      launchEnvironment.MergeFrom(command.environment());
+      foreach (const Environment::Variable& variable,
+               command.environment().variables()) {
+        const string& name = variable.name();
+        if (environment.contains(name) &&
+            environment[name].value() != variable.value()) {
+          cout << "Overwriting environment variable '" << name << "'" << endl;
+        }
+        environment[name] = variable;
+      }
+    }
+
+    Environment launchEnvironment;
+    foreachvalue (const Environment::Variable& variable, environment) {
+      launchEnvironment.add_variables()->CopyFrom(variable);
     }
 
     cout << "Starting task " << unacknowledgedTask->task_id() << endl;