mesos-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Timothy Chen" <tnac...@apache.org>
Subject Re: Review Request 26622: Command Executor is broken when used with shell=false
Date Fri, 31 Oct 2014 22:40:58 GMT


> On Oct. 24, 2014, 4:40 p.m., Timothy Chen wrote:
> > src/tests/slave_tests.cpp, line 448
> > <https://reviews.apache.org/r/26622/diff/4/?file=731893#file731893line448>
> >
> >     Just tried bin/ls on OSX and realize Mac's ls has no --author flag so this is
going to fail on OSX.
> >     
> >     It's ok if we don't support it but then we need to wrap the test in #ifdef __linux__
> 
> R.B. Boyer wrote:
>     The part that actually breaks is any double-dash option arguments.  If the OSX version
of ls shares any other double-dash arg options with linux then just replace --author with
that.
> 
> Timothy Chen wrote:
>     unfortunately ls doesn't have any double dash options, and I just looked at /usr/bin
and none of the non-intrusive binaries has any double dash options too.
>     I can however do a echo --author, that would have broke earlier right?
> 
> R.B. Boyer wrote:
>     I don't have access to an OSX box, nor will I have time until mid next week to revisit
this ticket.
>     
>     If you can, please try swapping out "[echo, --author]" for my ls example, reverting
slave.cpp, and then:
>     
>     GTEST_FILTER=SlaveTest.MesosExecutorCommandTaskWithArgsList make check
>     
>     To just double check that "echo --author" killed mesos-executor on OSX with the unpatched
protobuf stuff.
> 
> Timothy Chen wrote:
>     Ok I'll do that.
> 
> R.B. Boyer wrote:
>     Is there anything else for me to do on this patch?
> 
> Jie Yu wrote:
>     Tim, are you working on the patch?
> 
> Timothy Chen wrote:
>     Ya sorry was busy with docker patches, will get to this when they're pushed.

Hi R.B., I can't apply your patch cleanly for some reason it can't find your name/email.
Can you try to reupload it again? If I don't apply your patch I can't commit it with your
name/email.


- Timothy


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


On Oct. 24, 2014, 2:04 a.m., R.B. Boyer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26622/
> -----------------------------------------------------------
> 
> (Updated Oct. 24, 2014, 2:04 a.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 55e5264 
>   src/tests/slave_tests.cpp a1bd00c 
> 
> 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