> On March 31, 2014, 8:03 p.m., Vinod Kone wrote: > > src/slave/containerizer/external_containerizer.cpp, lines 293-295 > > <https://reviews.apache.org/r/17567/diff/15/?file=531801#file531801line293> > > > > This seems a bit unfortunate. How about creating a protobuf that wraps > > all the arguments that are needed for launch (and other methods) and > > passing it on to the external containerizer?
Packed this into ExternalTask which wraps TaskInfo and the mesos-executor path. > On March 31, 2014, 8:03 p.m., Vinod Kone wrote: > > src/slave/containerizer/external_containerizer.cpp, lines 1125-1126 > > <https://reviews.apache.org/r/17567/diff/15/?file=531801#file531801line1125> > > > > Instead of closing the file descriptor, how about just writing the > > length of the protobuf first and then serialized protobuf. That way the > > read end could know the exact size of data to read. > > > > This is how we do serialization and deserialization of protobufs to > > files. See protobuf::write() and protobuf::read(). > > > > > > > > Till Toenshoff wrote: > I like this approach. Will play with this a bit to see if there is > something we are missing. Now implemented that in the latest revision. > On March 31, 2014, 8:03 p.m., Vinod Kone wrote: > > src/slave/containerizer/external_containerizer.cpp, line 1135 > > <https://reviews.apache.org/r/17567/diff/15/?file=531801#file531801line1135> > > > > If we do the above and if we have MESOS-995, can you just leverage > > subprocess here? > > > > Till Toenshoff wrote: > Almost, yes. Let me quote from the header: > // TODO(tillt): Remove and use process::subprocess once that has > // become flexible enough to support all features needed: > // - in-child-context commands (chdir) after fork and before exec. > // - direct execution without the invocation of /bin/sh. > // - external ownership of pipe file descriptors. > > Additionally, we are missing a setsid within the child-context to prevent > a slave suicide once the external containerizer process gets terminated. > > For the reasoning: > chdir() is clear and would be covered by MESOS-995, which is great. > > Direct execution is needed as we currently are depending on proper pipe > close escalation. If /bin/sh is used to invoke and when its child process is > closing a pipe, the shell has to close its end as well. In some tests I did, > I found out that this is not always happening, depending on the shell in use > - specifically dash on ubuntu appears to not do that in certain > configurations. > > External ownership of the pipe handles is needed as the current way of > signaling a complete transmission of those protobufs is done via detection of > an EOF. Subprocess assumes to have full control and does assume the pipes to > be open until the process terminates. > > > Now circling back to your earlier comment on prepending the protobufs by > their size to signal a complete transmission. That might ease the process as > we could remove the need for "direct execution" and "external pipe > ownership". So the only remaining reason to not use subprocess then would be > the missing setsid(). > So now we are only missing setsid() to fully rely upon subprocess. Will propose an overload as soon as I addressed all other comments. - Till ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17567/#review38504 ----------------------------------------------------------- On April 2, 2014, 2:02 a.m., Till Toenshoff wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/17567/ > ----------------------------------------------------------- > > (Updated April 2, 2014, 2:02 a.m.) > > > Review request for mesos, Ian Downes, Niklas Nielsen, and Vinod Kone. > > > Bugs: MESOS-816 > https://issues.apache.org/jira/browse/MESOS-816 > > > Repository: mesos-git > > > Description > ------- > > This patch adds the so-called external 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: > > COMMAND (ADDITIONAL-PARAMETERS) < INPUT-PROTO > RESULT-PROTO > > launch (ContainerID, --mesos-executor, <path>) < TaskInfo > ExternalStatus > update (ContainerID) < ResourceArray > ExternalStatus > usage (ContainerID) > ResourceStatistics > wait (ContainerID) > ExternalTermination > destroy (ContainerID) > ExternalStatus > > When protocol buffers need to be provided, the Mesos side of > the external containerizer implementation will serialize the > protos on stdin and vice-versa for reading protos on stdout as > drafted in the above scheme. > > > NOTE: This is currently work in progress to cover all comments. > > > Diffs > ----- > > configure.ac 5404dc2 > include/mesos/mesos.proto 37f8a7f > src/Makefile.am 0775a0d > src/examples/python/test-containerizer.in PRE-CREATION > src/examples/python/test_containerizer.py PRE-CREATION > src/slave/containerizer/containerizer.cpp 6de091e > src/slave/containerizer/external_containerizer.hpp PRE-CREATION > src/slave/containerizer/external_containerizer.cpp PRE-CREATION > src/slave/flags.hpp c9a627b > src/tests/external_containerizer_test.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/17567/diff/ > > > Testing > ------- > > make check and functional testing. > > > Thanks, > > Till Toenshoff > >
