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