mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Zhitao Li <zhitaoli...@gmail.com>
Subject Re: Review Request 54821: Refactored Docker::run() to make it only aware of docker cli options.
Date Fri, 03 Feb 2017 20:33:02 GMT


> 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message