mesos-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Timothy St. Clair" <tstcl...@redhat.com>
Subject Re: Review Request 23771: Added a Docker containerizer.
Date Tue, 22 Jul 2014 20:38:02 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23771/#review48402
-----------------------------------------------------------


only 1/2 way through, this may take a bit. 


src/docker/docker.hpp
<https://reviews.apache.org/r/23771/#comment84964>

    So imho we should try to match parity with the API where possible, to provide an abstraction
which is consistent.  
    
    https://docs.docker.com/reference/api/docker_remote_api_v1.13/  
    
    IMHO - It looks more like a cli wrap vs. docker API abstraction.  
    
    Perhaps we could open another JIRA after this?  



src/docker/docker.hpp
<https://reviews.apache.org/r/23771/#comment84976>

    Graceful termination?  How does signal escalation get handled? 



src/docker/docker.cpp
<https://reviews.apache.org/r/23771/#comment84965>

    code standard - header ordering. 



src/docker/docker.cpp
<https://reviews.apache.org/r/23771/#comment84968>

    This is pretty weird to have here and would only exist on a pre-EL7.  All relatively newer
distro(s) it would be guaranteed true.  
    Do folks want to support the pre-EL7 route? Because it may kneecap docker.



src/docker/docker.cpp
<https://reviews.apache.org/r/23771/#comment84969>

    Might want to consider placing into a serializable struct. 



src/docker/docker.cpp
<https://reviews.apache.org/r/23771/#comment84971>

    See other comment re: shares and groups. 



src/docker/docker.cpp
<https://reviews.apache.org/r/23771/#comment84972>

    Input args will need to override defaults re: net, there is a separate JIRA on *this.




src/docker/docker.cpp
<https://reviews.apache.org/r/23771/#comment84979>

    Should this be here?  
    
    Also manifest constant for the chunck is probably wise for page size. 



src/docker/docker.cpp
<https://reviews.apache.org/r/23771/#comment84982>

    It almost seems like at certain points the REST API would be simpler/cleaner.  
    
    e.g. GET /containers/(id)/json
    
    Perhaps we should open a JIRA?



src/slave/containerizer/isolators/cgroups/cpushare.cpp
<https://reviews.apache.org/r/23771/#comment84970>

    Umm this is not always true, it's a weight based on the total across all groups.  In systemd
land this means promoting docker to a top level group and assigning a weight that has meaning.
    
    
    ref - http://timothysc.github.io/blog/2013/06/14/systemd-cgroup-sla/


- Timothy St. Clair


On July 22, 2014, 6:12 p.m., Timothy Chen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23771/
> -----------------------------------------------------------
> 
> (Updated July 22, 2014, 6:12 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ian Downes, and Jie Yu.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Docker implementation.
> This is all the docker code Ben, I and Yifan worked on excluding the composing containerizer
patches.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 45afcd1 
>   src/common/status_utils.hpp 1980551 
>   src/docker/docker.hpp PRE-CREATION 
>   src/docker/docker.cpp PRE-CREATION 
>   src/examples/docker_no_executor_framework.cpp PRE-CREATION 
>   src/health-check/main.cpp 707810a 
>   src/launcher/executor.cpp 9c80848 
>   src/linux/cgroups.hpp decad9d 
>   src/linux/cgroups.cpp 6a73dd7 
>   src/master/master.cpp 251b699 
>   src/slave/containerizer/containerizer.cpp 1b71f33 
>   src/slave/containerizer/docker.hpp PRE-CREATION 
>   src/slave/containerizer/docker.cpp PRE-CREATION 
>   src/slave/containerizer/external_containerizer.cpp 3f28d85 
>   src/slave/containerizer/isolators/cgroups/cpushare.hpp 780037b 
>   src/slave/containerizer/isolators/cgroups/cpushare.cpp 3265a80 
>   src/slave/containerizer/isolators/cgroups/mem.hpp 8c476c7 
>   src/slave/containerizer/isolators/cgroups/mem.cpp e8d1e35 
>   src/slave/containerizer/isolators/posix.hpp 17bbd10 
>   src/slave/flags.hpp 1fe7b7d 
>   src/slave/slave.cpp f42ab60 
>   src/tests/docker_containerizer_tests.cpp PRE-CREATION 
>   src/tests/docker_tests.cpp PRE-CREATION 
>   src/tests/environment.cpp 434b3f7 
>   src/tests/flags.hpp a003e7f 
>   src/tests/mesos_test_executor_docker_image/Dockerfile PRE-CREATION 
>   src/tests/mesos_test_executor_docker_image/install.sh PRE-CREATION 
>   src/tests/script.cpp 15a6542 
>   src/usage/usage.hpp 5a76746 
>   src/usage/usage.cpp 29014d1 
> 
> Diff: https://reviews.apache.org/r/23771/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>


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