> On Nov. 8, 2017, 11:23 a.m., Jie Yu wrote: > > src/slave/containerizer/docker.cpp > > Lines 1259-1260 (original), 1259-1271 (patched) > > <https://reviews.apache.org/r/63063/diff/2/?file=1879186#file1879186line1259> > > > > This is a bit weird. I think the interface of `reapExecutor` is made in > > such a way so that it can be chained. > > > > Can you change `reapExecutor`'s interface to return Nothing, and chain > > a lambda that returns a SUCCESS > > ``` > > return container->launch = fetch(containerId) > > ... > > .then(defer(self(), [=](pid_t pid) { > > return reapExecutor(containerId, pid); > > })) > > .then([]() { > > return Containerizer::LaunchResult::SUCCESS; > > })); > > ```
I will apply this change, but because it touches some other parts that I want to call out in the commit message, I'll pull it into a separate patch. - Joseph ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63063/#review190481 ----------------------------------------------------------- On Nov. 2, 2017, 9:02 a.m., Joseph Wu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63063/ > ----------------------------------------------------------- > > (Updated Nov. 2, 2017, 9:02 a.m.) > > > Review request for mesos, Gilbert Song and Jie Yu. > > > Bugs: MESOS-7305 > https://issues.apache.org/jira/browse/MESOS-7305 > > > Repository: mesos > > > Description > ------- > > There is some existing tech debt around requiring the caller of > `Containerizer::launch` to call `Containerizer::destroy` if the launch > fails (see MESOS-6214). For nested and standalone containers, the > side effect of this results in accidentally destroying running > containers if you make the same call an even number of times. > > For example, suppose the user launches a valid nested container > with an ID of 'parent.child'. If the user issues the same call to > launch 'parent.child' again, this second call will fail *and* will > also destroy the first container. > > This commit prevents repeated launch calls from destroying containers > by changing the return value of `Containerizer::launch`. There are > now four possible return values: > * The launch succeeded. > * The standalone/nested container already exists. > * The given ContainerConfig is not supported. > * The launch failed. > > > Diffs > ----- > > src/slave/containerizer/composing.hpp > 06d68eef5de7745e32f0e808f11016bcc285dd8f > src/slave/containerizer/composing.cpp > 587f009384f0c7ef87482686578dc822d3d5b208 > src/slave/containerizer/containerizer.hpp > 449bb5d0902936faae7bf9bae9c703b219aed842 > src/slave/containerizer/docker.hpp b602a5698cae12686f51c4b9370a06042cda6270 > src/slave/containerizer/docker.cpp 292eecbca246edf068ec8c262aff4f3ce9cd8c67 > src/slave/containerizer/mesos/containerizer.hpp > 6d356ccf82f36df8c6f558fb0ace7d9f982a3d6b > src/slave/containerizer/mesos/containerizer.cpp > 100e3bbda543d87808da9ff6bea42da5099ea8c5 > src/slave/http.cpp f2e06aff95e0628624b6ed25de222fd3f3577a0b > src/slave/slave.hpp df1b0205124555dcb6a0efa5c237f5e77fa2bdf7 > src/slave/slave.cpp 337083dbe60bba2d3773b785bdceeaf0b8fcd070 > src/tests/agent_container_api_tests.cpp PRE-CREATION > > > Diff: https://reviews.apache.org/r/63063/diff/2/ > > > Testing > ------- > > Requires the next review in the chain (test changes). > > > Thanks, > > Joseph Wu > >
