----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41820/#review113375 -----------------------------------------------------------
src/slave/containerizer/mesos/containerizer.cpp (lines 482 - 483) <https://reviews.apache.org/r/41820/#comment173953> We should recover the isolators first before recover provisioner (the reverse order of launching a container because recover might do cleanup on unknown containers). YOu might want to add `recoverIsolators` and `recoverProvisioner`. Change the `_recover` function to be: ``` return recoverIsolators(recoverable, orphans) .then(defer(self(), &Self::recoverProvisioner, recoverable, orphans)) .then(defer(self(), &Self::__recover, recoverable, orphans)); ``` src/slave/containerizer/mesos/containerizer.cpp (lines 689 - 690) <https://reviews.apache.org/r/41820/#comment173983> We mutate the executorInfo here and that mutated executorInfo here needs to be passed to `prepare` and `_launch`. This is important for command executors. I guess what you needs to do here is: rename the existing `_launch` to `__launch` and add a new `_launch`. In `launch`, do the following: ``` return provisioner->provision(containerId, executorInfo) .then(defer(self(), &Self::_launch, containerId, executorInfo, directory, user, slaveId, slavePid, checkpoint, lambda::_1)); ``` The new `_launch` should be similar to what you have here for `_provision`, but in the end, if will call `__launch` with mutated executorInfo. src/slave/containerizer/mesos/containerizer.cpp (line 711) <https://reviews.apache.org/r/41820/#comment173985> I realized another issue with command executors. We don't have to resolve it in the patch, but I just want to mention it here so that we don't forget. You probably can add a TODO comments here. For command executors, we modify the executorInfo so that the user image will be mounted in as a volume. However, we also need to figure out a way to support passing and handling those runtime configurations in the image. cc @tnachen src/slave/containerizer/mesos/containerizer.cpp (lines 1447 - 1451) <https://reviews.apache.org/r/41820/#comment173984> Again, we should call provisioner->destroy after all isolators have been cleaned up. Also, it does not make sense to put provisioner->destroy in cleanupIsolators. src/slave/containerizer/mesos/isolators/filesystem/linux.hpp (lines 45 - 46) <https://reviews.apache.org/r/41820/#comment173955> This fits in one line? src/slave/containerizer/mesos/isolators/filesystem/linux.hpp (lines 77 - 78) <https://reviews.apache.org/r/41820/#comment173956> Ditto. src/slave/containerizer/mesos/isolators/filesystem/linux.cpp (lines 62 - 63) <https://reviews.apache.org/r/41820/#comment173957> Ditto. src/slave/containerizer/mesos/isolators/filesystem/linux.cpp (lines 173 - 174) <https://reviews.apache.org/r/41820/#comment173958> Fits in one line? src/tests/containerizer/filesystem_isolator_tests.cpp (lines 157 - 158) <https://reviews.apache.org/r/41820/#comment173959> Fits in one line? src/tests/containerizer/filesystem_isolator_tests.cpp (lines 1008 - 1009) <https://reviews.apache.org/r/41820/#comment173960> FIts in one line? - Jie Yu On Jan. 6, 2016, 11:41 p.m., Gilbert Song wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/41820/ > ----------------------------------------------------------- > > (Updated Jan. 6, 2016, 11:41 p.m.) > > > Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen. > > > Bugs: MESOS-4240 > https://issues.apache.org/jira/browse/MESOS-4240 > > > Repository: mesos > > > Description > ------- > > Pulled out provisioner from linux filesystem isolator. > > > Diffs > ----- > > src/slave/containerizer/mesos/containerizer.hpp > 89d18624b08f2c953b9fa21663aeda694d4aac12 > src/slave/containerizer/mesos/containerizer.cpp > f3c370aeb331beb6202fd30cd0278877da0b42e0 > src/slave/containerizer/mesos/isolators/filesystem/linux.hpp > a29f9d184a0d1088577b3168a12bc8c06493a477 > src/slave/containerizer/mesos/isolators/filesystem/linux.cpp > 8af2cecdca947e55d8d39e26d2fc3d42212f36c3 > src/tests/containerizer/filesystem_isolator_tests.cpp > 5bb85034c22caef64054c1629f6fd55d227e48b1 > > Diff: https://reviews.apache.org/r/41820/diff/ > > > Testing > ------- > > make check (ubuntu14.04 + clang-3.6) > > > Thanks, > > Gilbert Song > >
