mesos-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "R.B. Boyer" <boyers...@nexusvector.net>
Subject Re: Review Request 26622: Command Executor is broken when used with shell=false
Date Thu, 23 Oct 2014 22:59:25 GMT


> On Oct. 23, 2014, 4:11 p.m., Timothy Chen wrote:
> > src/slave/slave.cpp, line 2564
> > <https://reviews.apache.org/r/26622/diff/2/?file=719277#file719277line2564>
> >
> >     I wonder if we instead should just copy the arguments we explicitly want? If
we add another field to commandInfo that is not intended here it will break again.

This would be nicer, if you think it's more appropriate I'll update the patch to ONLY copy
"uris" and "environment" as the embedded comment from the original author indicates.


- R.B.


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26622/#review58102
-----------------------------------------------------------


On Oct. 17, 2014, 11:40 p.m., R.B. Boyer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26622/
> -----------------------------------------------------------
> 
> (Updated Oct. 17, 2014, 11:40 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-1873
>     https://issues.apache.org/jira/browse/MESOS-1873
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Basically if you use "shell=false" with a non-empty argument list and the Command Executor
it is completely broken.
> 
> When we clone the env vars to fork "mesos-executor" all of the original cmd args are
cloned as well (unintentionally) due to some protocol-buffer merge shenanigans.
> 
> Don't pass task-related arguments to mesos-executor.
> 
> The description on the linked jira ticket goes into more detail.
> 
> 
> Diffs
> -----
> 
>   src/slave/slave.cpp 0e342ed 
>   src/tests/slave_tests.cpp f585bdd 
> 
> Diff: https://reviews.apache.org/r/26622/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> added new test "SlaveTest.GetExecutorInfo" verifying the explicit desired behavior.
> 
> 
> Thanks,
> 
> R.B. Boyer
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message