> On Jan. 18, 2017, 12:47 a.m., Joseph Wu wrote: > > src/slave/containerizer/mesos/containerizer.cpp, lines 1608-1617 > > <https://reviews.apache.org/r/55023/diff/2/?file=1605568#file1605568line1608> > > > > The effect of this is to change `argv` from: > > > > ``` > > mesos-containerizer launch > > ``` > > > > To: > > > > ``` > > .../mesos/libexec/mesos-containerizer launch > > ``` > > > > This change is harmless on POSIX, so there's no need for ifdef-ing. > > > > If so, can't you accomplish the same thing with a diff like: > > ``` > > diff --git a/src/slave/containerizer/mesos/containerizer.cpp > > b/src/slave/containerizer/mesos/containerizer.cpp > > index 8bf8a77..c760130 100644 > > --- a/src/slave/containerizer/mesos/containerizer.cpp > > +++ b/src/slave/containerizer/mesos/containerizer.cpp > > @@ -1603,12 +1603,12 @@ Future<bool> MesosContainerizerProcess::_launch( > > > > // Fork the child using launcher. > > vector<string> argv(2); > > - argv[0] = MESOS_CONTAINERIZER; > > + argv[0] = path::join(flags.launcher_dir, MESOS_CONTAINERIZER); > > argv[1] = MesosContainerizerLaunch::NAME; > > > > Try<pid_t> forked = launcher->fork( > > containerId, > > - path::join(flags.launcher_dir, MESOS_CONTAINERIZER), > > + argv[0], > > argv, > > in.isSome() ? in.get() : Subprocess::FD(STDIN_FILENO), > > out.isSome() ? out.get() : Subprocess::FD(STDOUT_FILENO), > > ```
We've historically been super conservative about changing the POSIX path, but with your sign-off, I'm happy to do something like this. > On Jan. 18, 2017, 12:47 a.m., Joseph Wu wrote: > > src/slave/containerizer/mesos/containerizer.cpp, lines 1608-1609 > > <https://reviews.apache.org/r/55023/diff/2/?file=1605568#file1605568line1608> > > > > It would appear that this TODO no longer applies. This issue is not yet resolved, actually. The point I was trying to get at is that the first argument to `subprocess` (if memory serves) is a no-op. So this call is likely to change when we refactor `subprocess`. We can still delete it if you want, though, but I think it's reasonable to keep it. - Alex ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55023/#review162001 ----------------------------------------------------------- On Jan. 15, 2017, 10:46 a.m., Alex Clemmer wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/55023/ > ----------------------------------------------------------- > > (Updated Jan. 15, 2017, 10:46 a.m.) > > > Review request for mesos, Andrew Schwartzmeyer, Daniel Pravat, John Kordich, > and Joseph Wu. > > > Repository: mesos > > > Description > ------- > > Currently in `MesosContainerizerProcess::_launch`, we are passing a > malformatted shell command to the launcher. This causes the > containerizer process to crash immediately upon invocation in all > executor tests. > > This commit will fix this command. > > > Diffs > ----- > > src/slave/containerizer/mesos/containerizer.cpp > 8bf8a7774a38131c53f6d91c7f09f5dedd9d4cb4 > > Diff: https://reviews.apache.org/r/55023/diff/ > > > Testing > ------- > > > Thanks, > > Alex Clemmer > >
