> On Oct. 18, 2015, 4:10 a.m., haosdent huang wrote: > > src/docker/docker.cpp, line 430 > > <https://reviews.apache.org/r/39388/diff/2/?file=1100523#file1100523line430> > > > > Seems we already have this env in > > https://github.com/apache/mesos/blob/master/src/slave/containerizer/containerizer.cpp#L254 > > Klaus Ma wrote: > I think those environment are used for `docker run` instead of passing > them to the container. > > Michael Park wrote: > That one is used to pass the `LIBPROCES_IP` to the `subprocess` call when > we're launching a process, whereas this is used to pass the `LIBPROCESS_IP` > to `docker->run` when we're launching a container via `-e`. > > haosdent huang wrote: > Thank you very much for explanation, got it! > > haosdent huang wrote: > Doe we change the line in docker executor could solve this problem? > ``` > diff --git a/src/docker/executor.cpp b/src/docker/executor.cpp > index 1e49013..a125078 100644 > --- a/src/docker/executor.cpp > +++ b/src/docker/executor.cpp > @@ -147,7 +147,7 @@ public: > sandboxDirectory, > mappedDirectory, > task.resources() + task.executor().resources(), > - None(), > + os::environment(), > path::join(sandboxDirectory, "stdout"), > path::join(sandboxDirectory, "stderr")) > .onAny(defer( > ``` > > haosdent huang wrote: > Or how about serialize the environment variables to docker flag and pass > it to `docker->run` if `os::environment` noisy? > ``` > diff --git a/src/slave/containerizer/docker.cpp > b/src/slave/containerizer/docker.cpp > index 7022958..d9b23d2 100644 > --- a/src/slave/containerizer/docker.cpp > +++ b/src/slave/containerizer/docker.cpp > @@ -911,7 +911,7 @@ Future<pid_t> > DockerContainerizerProcess::launchExecutorProcess( > Subprocess::PIPE(), > Subprocess::PATH(path::join(container->directory, "stdout")), > Subprocess::PATH(path::join(container->directory, "stderr")), > - dockerFlags(flags, container->name(), container->directory), > + dockerFlags(flags, container->name(), container->directory, > environment), > environment, > lambda::bind(&setup, container->directory)); > ``` > > Timothy Chen wrote: > We should just solve the cases we know required and avoid passing > environment variables if don't need to. We have found various problems > passing down env variables and doing os::environment or environment will > cause more pain in the future.
Got it. Thank you very much. - haosdent ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39388/#review103050 ----------------------------------------------------------- On Oct. 17, 2015, 11:18 p.m., Michael Park wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/39388/ > ----------------------------------------------------------- > > (Updated Oct. 17, 2015, 11:18 p.m.) > > > Review request for mesos and Niklas Nielsen. > > > Bugs: MESOS-3740 > https://issues.apache.org/jira/browse/MESOS-3740 > > > Repository: mesos > > > Description > ------- > > See summary. > > > Diffs > ----- > > src/docker/docker.cpp 56d63dc75637c9f89a239af371f476a85a570696 > src/tests/containerizer/docker_containerizer_tests.cpp > 4bb65afd0ee61cafef68e064a697fdce65d60058 > > Diff: https://reviews.apache.org/r/39388/diff/ > > > Testing > ------- > > Added `DockerContainerizerTest.ROOT_DOCKER_LaunchWithLibprocessIP` test which > fails without the changes made to `src/docker/docker.cpp`. > > > Thanks, > > Michael Park > >
