> On May 11, 2017, 3:19 a.m., Jie Yu wrote: > > src/slave/containerizer/containerizer.hpp > > Lines 85-86 (original), 85-86 (patched) > > <https://reviews.apache.org/r/58899/diff/1/?file=1705355#file1705355line85> > > > > Can we make the field in the protobuf optional and add this note there > > as well?
`ContainerConfig` is currently the public interface between containerizer and isolator. The isolator(s) definitely expect the `directory` field to be present, so I feel it is more appropriate to leave the `directory` as required. I'll include some protobuf commenting additions in a subsequent patch (those comments will be applicable with or without this patch chain, for the most part). - Joseph ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58899/#review174626 ----------------------------------------------------------- On May 1, 2017, 7:01 p.m., Joseph Wu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58899/ > ----------------------------------------------------------- > > (Updated May 1, 2017, 7:01 p.m.) > > > Review request for mesos and Jie Yu. > > > Bugs: MESOS-7449 > https://issues.apache.org/jira/browse/MESOS-7449 > > > Repository: mesos > > > Description > ------- > > When nested container support was added, we added a separate `launch` > path in the containerizer because nested containers do not need > an explicit TaskInfo/ExecutorInfo. Nested containers basically > only need the CommandInfo and ContainerInfo. > > This commit combines the two launch methods by replacing most of the > "Infos" (Task, Executor, Command, Container) with a `ContainerConfig` > argument, which may contain multiple combinations of the "Infos". > > The goal is to support three launch paths for containers: > 1) When the `ContainerConfig` contains a TaskInfo/ExecutorInfo, > launch a task or executor. > 2) When the `ContainerID` has a parent, launch a nested container. > 3) (Not implemented yet) When there is no TaskInfo/ExecutorInfo or > parent container, launch a standalone container. > > There are two other notable changes to the interface: > * The `SlaveID` field has been removed entirely. The code that > requires this (in the fetcher and Docker containerizer) will be > addressed in a separate commit. > * The `checkpoint` bool has been replaced by an Option<string>, > which contains the path that should be used for checkpointing. > This path includes the filename. > This is also one of the reasons why `SlaveID` was an argument. > > > Diffs > ----- > > src/slave/containerizer/containerizer.hpp > 4c31a1f5c853c1dc66480c7b4c867a87a1bb5c41 > > > Diff: https://reviews.apache.org/r/58899/diff/1/ > > > Testing > ------- > > See last patch in chain. > > > Thanks, > > Joseph Wu > >
