----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57763/#review169410 -----------------------------------------------------------
We already discussed this offline, and I still (maybe even stronger) feel that cases of duplicate env vars with differing values should be errors, dispite the fact that in the current implementation Mesos makes sure that isolators' `prepare` are called in deterministic order (there is no guarantee on the interface level). We should strongly consider tightening the behavior here if we do not have to support the current behavior for e.g., backwards compatibility reasons. Env vars potentially mix user input from `CommandInfo` with output from isolators configured by an operator. Conflicting values cause settings by either the user or an isolator to be silently dropped. The first case would provide a bad UX, while the second one could be a configuration bug or worse from incompatible isolators. Diagnosing either of these through non-fatal log messages seems to do to little. As a user it would not be enough to just manually make sure none of these non-fatal errors are reported in `stdout` (not even `stderr`) when developing the task since an administrator might later on introduce introduce isolators making use of already taken user variables which would now be be broken by the task's setup. As an operator I wouldn't even be able to see these messages a user might not care about since they are reported in the task's output, not e.g., the agent log. It we'd just rejected tasks with conflicting env settings outright, the situation would improve for both users and operators. If we'd go that route I believe it would make sense to add duplicate detection to `validateEnvironment` and directly invoke it here. - Benjamin Bannier On March 20, 2017, 3:51 a.m., Till Toenshoff wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57763/ > ----------------------------------------------------------- > > (Updated March 20, 2017, 3:51 a.m.) > > > Review request for mesos, Adam B, Benjamin Bannier, Jie Yu, and Joseph Wu. > > > Bugs: MESOS-7264 > https://issues.apache.org/jira/browse/MESOS-7264 > > > Repository: mesos > > > Description > ------- > > See summary. > > > Diffs > ----- > > src/slave/containerizer/mesos/launch.cpp > 8658525b00e78bed9227d6d400eccccae2cf25dd > > > Diff: https://reviews.apache.org/r/57763/diff/1/ > > > Testing > ------- > > make check & functional testing... > > ``` > ./src/mesos-execute --name="test" --env='{"key1":"value1"}' --command='sleep > 1000' --master=127.0.0.1:5050 > ``` > > Within the contents of `stdout` before applying this: > ``` > Overwriting environment variable 'key1', original: 'value1', new: 'value1' > ``` > > After applying this there is no mention of the actually duplicate but equally > valued variable anymore. Note, this is before applying > https://reviews.apache.org/r/57762. > > > Thanks, > > Till Toenshoff > >
