----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33318/#review80541 -----------------------------------------------------------
Ship it! src/slave/containerizer/docker.hpp <https://reviews.apache.org/r/33318/#comment130546> There is a big global invariant here that we should call out even more explicitly for future readers of the code. That is, we add the initial task resources to the ExecutorInfo so that the initial container we create is big enough since some ExecutorInfo's might not actually add resources. This is an unfortunate wart that got added that we should really clean up at some point (basically, we shouldn't need to have the Slave add the tasks resources to the ExecutorInfo because the Containerizer should just be able to figure that out itself). In the mean time, we should have a better comment and even: if (task.isSome()) { CHECK(executor.resources() >= task.get().resources()); } - Benjamin Hindman On April 17, 2015, 6:21 p.m., Timothy Chen wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/33318/ > ----------------------------------------------------------- > > (Updated April 17, 2015, 6:21 p.m.) > > > Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Till > Toenshoff. > > > Repository: mesos > > > Description > ------- > > Fix docker containerizer usage and test. > The docker usage test is failing with the most recent change in including > executor resources in docker containerizer. > > > Diffs > ----- > > src/slave/containerizer/docker.hpp 6893684e6d199a5d69fc8bba8e60c4acaae9c3c9 > src/tests/docker_containerizer_tests.cpp > c772d4c836de18b0e87636cb42200356d24ec73d > > Diff: https://reviews.apache.org/r/33318/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Timothy Chen > >
