> On Feb. 12, 2014, 2:02 a.m., Tom Arnfeld wrote: > >
First of all, sorry for that late reply on your very helpful feedback. Thanks Tom! As you have noticed, many things are still very rough and drafty and in the process of change. Still, it was my goal to get feedback just like yours to achieve some confirmation on the first steps. > On Feb. 12, 2014, 2:02 a.m., Tom Arnfeld wrote: > > src/slave/containerizer/pluggable_containerizer.cpp, line 595 > > <https://reviews.apache.org/r/17567/diff/2/?file=482402#file482402line595> > > > > Does this method assume killing the external containerizer process will > > successfully kill the container? In the case of docker, if you kill the > > `docker run` call it will certainly kill cleanly, but the container is > > still running. You need to execute a `docker kill XXX` command to cleanly > > kill the running container (which will exit non-0 if it fails). Let me describe the now updated way of handling this situation; We are sending a "destroy" command to the containerizer plugin first. The plugin is then responsible to answer that command-request with a status. Once that status has been received the PluggableContainerizer will signal a SIGTERM (with SIGTERM elevation) towards the plugin. So the addition basically is a "wait for result/response" on destroy, allowing for synchronized flows. Would that fit your needs? > On Feb. 12, 2014, 2:02 a.m., Tom Arnfeld wrote: > > src/slave/containerizer/pluggable_containerizer.cpp, line 547 > > <https://reviews.apache.org/r/17567/diff/2/?file=482402#file482402line547> > > > > Might be nice to have a more detailed log message, if I see this in > > logs it doesn't tell me which containerID it's referencing. Good point, added that. > On Feb. 12, 2014, 2:02 a.m., Tom Arnfeld wrote: > > src/slave/containerizer/pluggable_containerizer.cpp, line 388 > > <https://reviews.apache.org/r/17567/diff/2/?file=482402#file482402line388> > > > > Hmm. If the container hasn't launched and we disregard the update > > request, does this mean we're going to completely ignore a change to > > resources? > > > > Would it not make sense to pass the update request on to the external > > containerizer anyway and give them the opportunity to make this decision? > > The external containerizer might want to make note of "pending" resource > > changes and apply them once the container is up (or wait for the container > > to come up). That could indeed be a valid and still persisting point. Will look into that... > On Feb. 12, 2014, 2:02 a.m., Tom Arnfeld wrote: > > src/slave/containerizer/pluggable_containerizer.hpp, line 140 > > <https://reviews.apache.org/r/17567/diff/2/?file=482401#file482401line140> > > > > This seems a little odd to me... surely a 0 exit code should suggest a > > successful invoke no matter what the external containerizer writes to > > stdout? Within the updated implementation, we now expect protobuffed result messages for all commands. > On Feb. 12, 2014, 2:02 a.m., Tom Arnfeld wrote: > > src/examples/python/test_containerizer.py, line 33 > > <https://reviews.apache.org/r/17567/diff/2/?file=482396#file482396line33> > > > > Should this be hard coded? Seems to me like this will fail if i'm > > running mesos from the build directory without installing. Correct, we now fetch that path from the slave-flags (launcher_dir). - TILL ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17567/#review34249 ----------------------------------------------------------- On Feb. 18, 2014, 4:30 p.m., TILL TOENSHOFF wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/17567/ > ----------------------------------------------------------- > > (Updated Feb. 18, 2014, 4:30 p.m.) > > > Review request for mesos, Benjamin Hindman and Niklas Nielsen. > > > Repository: mesos-git > > > Description > ------- > > This patch adds the so-called pluggable containerizer. This > containerizer delegates all containerizer calls directly to > an external containerizer program (which can be specified on > start-up). Few calls have internal fall-back implementations > such as wait(), destroy() and usage(). > The protocol for the interactions with the external program > is as follows: > ./containerizer <command> <container-id> (<optional parameters>) > When protocol buffers need to be provided, the Mesos side of > the pluggable containerizer will serialize the protos on stdin > and vice-versa for reading protos on stdout. > > > NOTE: Still work in progress and not yet reviewable. > > > Diffs > ----- > > configure.ac a8bc8a8 > include/mesos/mesos.proto 69a4a60 > src/Makefile.am 768c66a > src/examples/python/test-containerizer.in PRE-CREATION > src/examples/python/test_containerizer.py PRE-CREATION > src/slave/containerizer/containerizer.hpp d9ae326 > src/slave/containerizer/containerizer.cpp d0a1023 > src/slave/containerizer/mesos_containerizer.hpp ee1fd30 > src/slave/containerizer/mesos_containerizer.cpp 6d990cb > src/slave/containerizer/pluggable_containerizer.hpp PRE-CREATION > src/slave/containerizer/pluggable_containerizer.cpp PRE-CREATION > src/slave/flags.hpp e4d98a5 > src/slave/slave.hpp d82d4e9 > src/slave/slave.cpp 8ad955a > src/slave/status_update_manager.cpp a88bb18 > src/tests/containerizer.hpp 5686398 > src/tests/containerizer.cpp bfb9341 > src/tests/mesos.cpp 98333a7 > src/tests/pluggable_containerizer_test.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/17567/diff/ > > > Testing > ------- > > > Thanks, > > TILL TOENSHOFF > >
