> On Feb. 3, 2017, 7:28 p.m., Jie Yu wrote: > > This is a partial review. Is there way to run this in your staging/prod > > environment. This patch touches the core part of Docker containerizer which > > we don't have a good test coverage, and has a very board impact.
I will run all tests with a `DOCKER` filter, and also use `mesos-execute` with docker containerizer to test this out. If we want more testing, I can try to build an actual binary to test it out in our staging environment but it'll take sometime to get it done, and even that doesn't test every combinations of arguments. > On Feb. 3, 2017, 7:28 p.m., Jie Yu wrote: > > src/docker/docker.hpp, line 194 > > <https://reviews.apache.org/r/54821/diff/7/?file=1623559#file1623559line194> > > > > I'd try to avoid mesos:: prefixed type in this file. Can this just be a > > map? This could be a `vector<string>`, but a `map` would destroy initial order, which used to be sensitive to docker daemon. > On Feb. 3, 2017, 7:28 p.m., Jie Yu wrote: > > src/docker/docker.cpp, line 527 > > <https://reviews.apache.org/r/54821/diff/7/?file=1623560#file1623560line527> > > > > This changes the logic? Where is MIN_MEMORY? Adding the previous logic back right now. - Zhitao ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54821/#review164143 ----------------------------------------------------------- On Feb. 3, 2017, 6:55 p.m., Zhitao Li wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/54821/ > ----------------------------------------------------------- > > (Updated Feb. 3, 2017, 6:55 p.m.) > > > Review request for mesos, Xiaojian Huang, haosdent huang, and Jie Yu. > > > Bugs: MESOS-6808 > https://issues.apache.org/jira/browse/MESOS-6808 > > > Repository: mesos > > > Description > ------- > > This patch creates a wrapper struct for all recognizable docker cli > options, and separate logic of creating these options to a different > common function. > > This also enables us to overcome gmock's 10 argument limit. > > No logic change happens in this refactoring patch. > > > Diffs > ----- > > src/docker/docker.hpp 9093371afc8ea792ba94f61c6875703e547ea6b0 > src/docker/docker.cpp b63b060ba1c1d39dc1702368cf667831edbd39bd > src/docker/executor.cpp d8c72b081fb9a37b3927b73cea3b47956a076691 > src/slave/containerizer/docker.cpp 886528f0f706ef2a3c07246406f3ee1ebc45565d > src/tests/containerizer/docker_containerizer_tests.cpp > 31d63b1f239055d82470ace9024b584a2096dce4 > src/tests/containerizer/docker_tests.cpp > 9667d434486c1832f180a297a39a3d5dae6a26bd > src/tests/mock_docker.hpp 829a760d54ad8c7b88256ae5df4c88c9fb18df71 > src/tests/mock_docker.cpp 02b6065a01e7e52b0edb38676dfb1bb475584502 > > Diff: https://reviews.apache.org/r/54821/diff/ > > > Testing > ------- > > `make check` with ROOT and DOCKER filter. > > > Thanks, > > Zhitao Li > >
