aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bill Farner" <wfar...@apache.org>
Subject Re: Review Request 34337: Add Docker Parameters
Date Thu, 25 Jun 2015 17:24:15 GMT

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


LGTM in general!  Biggest blocker for me is ability to toggle this behavior off.


api/src/main/thrift/org/apache/aurora/gen/api.thrift (lines 207 - 213)
<https://reviews.apache.org/r/34337/#comment141963>

    Move this down below `MesosContainer` so it's closer to `DockerContainer`.
    
    I haven't followed the wiring into mesos, but i assume these are docker command line args?
 If so, it would be handy to call that out explicitly and include a link [1] in our docs.
    
    [1] https://docs.docker.com/reference/commandline/cli/



src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java (lines 186 - 195)
<https://reviews.apache.org/r/34337/#comment141962>

    Similar to the `allowed_container_types` argument used in `ConfigurationManager`, we need
a command line argument to disable this, as it poses a security risk on a shared cluster.



src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java (line 175)
<https://reviews.apache.org/r/34337/#comment141966>

    s/final //



src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java (line 177)
<https://reviews.apache.org/r/34337/#comment141968>

    Consider asserting on the whole list rather than a single value:
    ```
    assertEquals(
        ImmutableList.of(...),
        docker.getParametersList());
    ```


- Bill Farner


On June 20, 2015, 11:42 p.m., Mauricio Garavaglia wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34337/
> -----------------------------------------------------------
> 
> (Updated June 20, 2015, 11:42 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Support Arbitrary Docker Parameters in DockerContainer
> 
> 
> Diffs
> -----
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift d740a90e7732f42b43a79f8cf0afe705c061539c

>   docs/configuration-reference.md 7bfd63381f54b0fe5ef6a4f17b825049b19038db 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java e934f570e4a728470408970485abe0809487d312

>   src/main/python/apache/aurora/config/schema/base.py ec9f983564516afe542ab277d987d4d391f87e45

>   src/main/python/apache/aurora/config/thrift.py 810febb637d168b07c4aea77984e1d1451a39af2

>   src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 1b2a7948ebb946a2e12b0eded6acf4ce3c8e20f9

> 
> Diff: https://reviews.apache.org/r/34337/diff/
> 
> 
> Testing
> -------
> 
> Used Docker as the container of a Job. Included volumes and label parameters which are
correctly picked up by mesos when starting the task. The docker container gets the specified
label and bind mounts the volumes correctly. I've been running multiple PostgreSQL databases
docker containers for several weeks deploying them as aurora jobs.
> 
> 
> Thanks,
> 
> Mauricio Garavaglia
> 
>


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