Re: Review Request 31338: Added a command line flag, -global_container_mounts, to allow mounting paths from the slaves into the (docker) containers they run.

2015-03-03 Thread Steve Niemitz

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

(Updated March 3, 2015, 3:21 p.m.)


Review request for Aurora, Jay Buffington and Bill Farner.


Bugs: AURORA-1107
https://issues.apache.org/jira/browse/AURORA-1107


Repository: aurora


Description
---

Added a command line flag, -global_container_mounts, to allow mounting paths 
from the slaves into the (docker) containers they run.

This is the first portion of allowing per-job mounts, however, I wanted to get 
this out first since more people want it.  I'll implement per-job mounts in a 
future review.


Diffs (updated)
-

  api/src/main/thrift/org/apache/aurora/gen/api.thrift 
a5198dfe1c56e05d50e32dd14222a984515c0d07 
  config/checkstyle/checkstyle.xml 580d9d3737c0f52da2d33f5b99ef9406b55c5807 
  docs/deploying-aurora-scheduler.md d1123359961fd59ddb8c1a07f80f293bdd46019f 
  src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
3239eaa139e35e8c3acdacf6375f492de2b5bfee 
  src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
24b61c1e4f615295acf28d904588e1512972d3f4 
  src/main/java/org/apache/aurora/scheduler/app/VolumeParser.java PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
baacb71403d55c5b90fc11cb2a23f552a32e8ba5 
  src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
5340d651b298ec8aa079e73d6d2f652fdf876293 
  src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
6575b7d420f17ec68d6e2a83e7b380f684577d4f 
  src/test/java/org/apache/aurora/scheduler/app/VolumeParserTest.java 
PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
444d6d3fdaf86eb84612f846eaa326eb75c49898 
  src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
efe62ceb502ead88a2f0cd6d09a76664e465d9bc 

Diff: https://reviews.apache.org/r/31338/diff/


Testing
---


Thanks,

Steve Niemitz



Re: Review Request 31338: Added a command line flag, -global_container_mounts, to allow mounting paths from the slaves into the (docker) containers they run.

2015-03-03 Thread Bill Farner

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


@ReviewBot retry

- Bill Farner


On March 3, 2015, 3:23 p.m., Steve Niemitz wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/31338/
 ---
 
 (Updated March 3, 2015, 3:23 p.m.)
 
 
 Review request for Aurora, Jay Buffington and Bill Farner.
 
 
 Bugs: AURORA-1107
 https://issues.apache.org/jira/browse/AURORA-1107
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Added a command line flag, -global_container_mounts, to allow mounting paths 
 from the slaves into the (docker) containers they run.
 
 This is the first portion of allowing per-job mounts, however, I wanted to 
 get this out first since more people want it.  I'll implement per-job mounts 
 in a future review.
 
 
 Diffs
 -
 
   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
 a5198dfe1c56e05d50e32dd14222a984515c0d07 
   config/checkstyle/checkstyle.xml 580d9d3737c0f52da2d33f5b99ef9406b55c5807 
   docs/deploying-aurora-scheduler.md d1123359961fd59ddb8c1a07f80f293bdd46019f 
   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
 3239eaa139e35e8c3acdacf6375f492de2b5bfee 
   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
 24b61c1e4f615295acf28d904588e1512972d3f4 
   src/main/java/org/apache/aurora/scheduler/app/VolumeParser.java 
 PRE-CREATION 
   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
 baacb71403d55c5b90fc11cb2a23f552a32e8ba5 
   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
 5340d651b298ec8aa079e73d6d2f652fdf876293 
   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
 6575b7d420f17ec68d6e2a83e7b380f684577d4f 
   src/test/java/org/apache/aurora/scheduler/app/VolumeParserTest.java 
 PRE-CREATION 
   
 src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
 444d6d3fdaf86eb84612f846eaa326eb75c49898 
   src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
 efe62ceb502ead88a2f0cd6d09a76664e465d9bc 
 
 Diff: https://reviews.apache.org/r/31338/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Steve Niemitz
 




Re: Review Request 31338: Added a command line flag, -global_container_mounts, to allow mounting paths from the slaves into the (docker) containers they run.

2015-03-03 Thread Bill Farner


 On March 3, 2015, 6:28 p.m., Aurora ReviewBot wrote:
  Master (782e3e7) is red with this patch.
./build-support/jenkins/build.sh
  
   src.test.python.apache.aurora.client.cli.update
   .   SUCCESS
   src.test.python.apache.aurora.client.cli.version   
   .   SUCCESS
   src.test.python.apache.aurora.client.config
   .   SUCCESS
   src.test.python.apache.aurora.client.factory   
   .   SUCCESS
   src.test.python.apache.aurora.client.hooks.hooked_api  
   .   SUCCESS
   
  src.test.python.apache.aurora.client.hooks.non_hooked_api   
  .   SUCCESS
   
  src.test.python.apache.aurora.common.test_aurora_job_key
  .   SUCCESS
   src.test.python.apache.aurora.common.test_cluster  
   .   SUCCESS
   
  src.test.python.apache.aurora.common.test_cluster_option
  .   SUCCESS
   src.test.python.apache.aurora.common.test_clusters 
   .   SUCCESS
   
  src.test.python.apache.aurora.common.test_http_signaler 
  .   SUCCESS
   src.test.python.apache.aurora.common.test_pex_version  
   .   SUCCESS
   src.test.python.apache.aurora.common.test_shellify 
   .   SUCCESS
   src.test.python.apache.aurora.common.test_transport
   .   SUCCESS
   src.test.python.apache.aurora.config.test_base 
   .   SUCCESS
   
  src.test.python.apache.aurora.config.test_constraint_parsing
  .   SUCCESS
   src.test.python.apache.aurora.config.test_loader   
   .   SUCCESS
   src.test.python.apache.aurora.config.test_thrift   
   .   SUCCESS
   
  src.test.python.apache.aurora.executor.common.announcer 
  .   SUCCESS
   
  src.test.python.apache.aurora.executor.common.directory_sandbox 
  .   SUCCESS
   
  src.test.python.apache.aurora.executor.common.executor_detector 
  .   SUCCESS
   
  src.test.python.apache.aurora.executor.common.executor_timeout  
  .   SUCCESS
   
  src.test.python.apache.aurora.executor.common.health_checker
  .   SUCCESS
   
  src.test.python.apache.aurora.executor.common.kill_manager  
  .   SUCCESS
   
  src.test.python.apache.aurora.executor.common.path_detector 
  .   SUCCESS
   
  src.test.python.apache.aurora.executor.common.status_checker
  .   SUCCESS
   
  src.test.python.apache.aurora.executor.common.task_info 
  .   SUCCESS
   src.test.python.apache.aurora.executor.executor_base   
   .   SUCCESS
   src.test.python.apache.aurora.executor.executor_vars   
   .   SUCCESS
   src.test.python.apache.aurora.executor.gc_executor 
   .   FAILURE
   src.test.python.apache.aurora.executor.status_manager  
   .   SUCCESS
   
  src.test.python.apache.aurora.executor.thermos_task_runner  
  .   SUCCESS
   src.test.python.apache.thermos.common.test_pathspec
   .   SUCCESS
   
  src.test.python.apache.thermos.core.test_runner_integration 
  .   SUCCESS
   src.test.python.apache.thermos.monitoring.test_disk
   .   SUCCESS
   
  FAILURE
  
  
 FAILURE
  
  
  I will refresh this build result if you post a review containing 
  @ReviewBot retry

Whack-a-mole https://issues.apache.org/jira/browse/AURORA-1162


- Bill


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


On March 3, 2015, 3:23 p.m., Steve Niemitz wrote:
 
 

Re: Review Request 31338: Added a command line flag, -global_container_mounts, to allow mounting paths from the slaves into the (docker) containers they run.

2015-03-03 Thread Bill Farner

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


@ReviewBot retry

- Bill Farner


On March 3, 2015, 3:23 p.m., Steve Niemitz wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/31338/
 ---
 
 (Updated March 3, 2015, 3:23 p.m.)
 
 
 Review request for Aurora, Jay Buffington and Bill Farner.
 
 
 Bugs: AURORA-1107
 https://issues.apache.org/jira/browse/AURORA-1107
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Added a command line flag, -global_container_mounts, to allow mounting paths 
 from the slaves into the (docker) containers they run.
 
 This is the first portion of allowing per-job mounts, however, I wanted to 
 get this out first since more people want it.  I'll implement per-job mounts 
 in a future review.
 
 
 Diffs
 -
 
   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
 a5198dfe1c56e05d50e32dd14222a984515c0d07 
   config/checkstyle/checkstyle.xml 580d9d3737c0f52da2d33f5b99ef9406b55c5807 
   docs/deploying-aurora-scheduler.md d1123359961fd59ddb8c1a07f80f293bdd46019f 
   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
 3239eaa139e35e8c3acdacf6375f492de2b5bfee 
   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
 24b61c1e4f615295acf28d904588e1512972d3f4 
   src/main/java/org/apache/aurora/scheduler/app/VolumeParser.java 
 PRE-CREATION 
   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
 baacb71403d55c5b90fc11cb2a23f552a32e8ba5 
   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
 5340d651b298ec8aa079e73d6d2f652fdf876293 
   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
 6575b7d420f17ec68d6e2a83e7b380f684577d4f 
   src/test/java/org/apache/aurora/scheduler/app/VolumeParserTest.java 
 PRE-CREATION 
   
 src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
 444d6d3fdaf86eb84612f846eaa326eb75c49898 
   src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
 efe62ceb502ead88a2f0cd6d09a76664e465d9bc 
 
 Diff: https://reviews.apache.org/r/31338/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Steve Niemitz
 




Re: Review Request 31338: Added a command line flag, -global_container_mounts, to allow mounting paths from the slaves into the (docker) containers they run.

2015-03-03 Thread Aurora ReviewBot

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

Ship it!


Master (782e3e7) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing @ReviewBot 
retry

- Aurora ReviewBot


On March 3, 2015, 3:23 p.m., Steve Niemitz wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/31338/
 ---
 
 (Updated March 3, 2015, 3:23 p.m.)
 
 
 Review request for Aurora, Jay Buffington and Bill Farner.
 
 
 Bugs: AURORA-1107
 https://issues.apache.org/jira/browse/AURORA-1107
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Added a command line flag, -global_container_mounts, to allow mounting paths 
 from the slaves into the (docker) containers they run.
 
 This is the first portion of allowing per-job mounts, however, I wanted to 
 get this out first since more people want it.  I'll implement per-job mounts 
 in a future review.
 
 
 Diffs
 -
 
   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
 a5198dfe1c56e05d50e32dd14222a984515c0d07 
   config/checkstyle/checkstyle.xml 580d9d3737c0f52da2d33f5b99ef9406b55c5807 
   docs/deploying-aurora-scheduler.md d1123359961fd59ddb8c1a07f80f293bdd46019f 
   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
 3239eaa139e35e8c3acdacf6375f492de2b5bfee 
   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
 24b61c1e4f615295acf28d904588e1512972d3f4 
   src/main/java/org/apache/aurora/scheduler/app/VolumeParser.java 
 PRE-CREATION 
   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
 baacb71403d55c5b90fc11cb2a23f552a32e8ba5 
   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
 5340d651b298ec8aa079e73d6d2f652fdf876293 
   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
 6575b7d420f17ec68d6e2a83e7b380f684577d4f 
   src/test/java/org/apache/aurora/scheduler/app/VolumeParserTest.java 
 PRE-CREATION 
   
 src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
 444d6d3fdaf86eb84612f846eaa326eb75c49898 
   src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
 efe62ceb502ead88a2f0cd6d09a76664e465d9bc 
 
 Diff: https://reviews.apache.org/r/31338/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Steve Niemitz
 




Re: Review Request 31338: Added a command line flag, -global_container_mounts, to allow mounting paths from the slaves into the (docker) containers they run.

2015-03-02 Thread Steve Niemitz

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

(Updated March 2, 2015, 9:03 p.m.)


Review request for Aurora, Jay Buffington and Bill Farner.


Bugs: AURORA-1107
https://issues.apache.org/jira/browse/AURORA-1107


Repository: aurora


Description
---

Added a command line flag, -global_container_mounts, to allow mounting paths 
from the slaves into the (docker) containers they run.

This is the first portion of allowing per-job mounts, however, I wanted to get 
this out first since more people want it.  I'll implement per-job mounts in a 
future review.


Diffs (updated)
-

  api/src/main/thrift/org/apache/aurora/gen/api.thrift 
6f6124a4c844a1abcf07401d80c3e50eb8b4 
  config/checkstyle/checkstyle.xml 580d9d3737c0f52da2d33f5b99ef9406b55c5807 
  docs/deploying-aurora-scheduler.md d1123359961fd59ddb8c1a07f80f293bdd46019f 
  src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
bacfbfeb237ecddf82f58679e05be012c5214e61 
  src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
24b61c1e4f615295acf28d904588e1512972d3f4 
  src/main/java/org/apache/aurora/scheduler/app/VolumeParser.java PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
baacb71403d55c5b90fc11cb2a23f552a32e8ba5 
  src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
5340d651b298ec8aa079e73d6d2f652fdf876293 
  src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
6575b7d420f17ec68d6e2a83e7b380f684577d4f 
  src/test/java/org/apache/aurora/scheduler/app/VolumeParserTest.java 
PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
444d6d3fdaf86eb84612f846eaa326eb75c49898 
  src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
efe62ceb502ead88a2f0cd6d09a76664e465d9bc 

Diff: https://reviews.apache.org/r/31338/diff/


Testing
---


Thanks,

Steve Niemitz



Re: Review Request 31338: Added a command line flag, -global_container_mounts, to allow mounting paths from the slaves into the (docker) containers they run.

2015-03-02 Thread Steve Niemitz


 On March 2, 2015, 6:56 p.m., Bill Farner wrote:
  src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java, line 133
  https://reviews.apache.org/r/31338/diff/8/?file=882131#file882131line133
 
  I think you can drop this since it's == `Resources.NONE` (default).

Done


 On March 2, 2015, 6:56 p.m., Bill Farner wrote:
  src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java, line 
  137
  https://reviews.apache.org/r/31338/diff/8/?file=882134#file882134line137
 
  remove empty line

Done


 On March 2, 2015, 6:56 p.m., Bill Farner wrote:
  src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java,
   lines 216-222
  https://reviews.apache.org/r/31338/diff/8/?file=882138#file882138line216
 
  This can collapse to:
  
  ```
  
  assertTrue(taskInfo.getExecutor().getContainer().getVolumesList().contains(expected));
  ```

Done


- Steve


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


On March 2, 2015, 4:59 p.m., Steve Niemitz wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/31338/
 ---
 
 (Updated March 2, 2015, 4:59 p.m.)
 
 
 Review request for Aurora, Jay Buffington and Bill Farner.
 
 
 Bugs: AURORA-1107
 https://issues.apache.org/jira/browse/AURORA-1107
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Added a command line flag, -global_container_mounts, to allow mounting paths 
 from the slaves into the (docker) containers they run.
 
 This is the first portion of allowing per-job mounts, however, I wanted to 
 get this out first since more people want it.  I'll implement per-job mounts 
 in a future review.
 
 
 Diffs
 -
 
   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
 6f6124a4c844a1abcf07401d80c3e50eb8b4 
   config/checkstyle/checkstyle.xml 580d9d3737c0f52da2d33f5b99ef9406b55c5807 
   docs/deploying-aurora-scheduler.md d1123359961fd59ddb8c1a07f80f293bdd46019f 
   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
 bacfbfeb237ecddf82f58679e05be012c5214e61 
   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
 24b61c1e4f615295acf28d904588e1512972d3f4 
   src/main/java/org/apache/aurora/scheduler/app/VolumeParser.java 
 PRE-CREATION 
   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
 baacb71403d55c5b90fc11cb2a23f552a32e8ba5 
   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
 5340d651b298ec8aa079e73d6d2f652fdf876293 
   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
 6575b7d420f17ec68d6e2a83e7b380f684577d4f 
   src/test/java/org/apache/aurora/scheduler/app/VolumeParserTest.java 
 PRE-CREATION 
   
 src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
 444d6d3fdaf86eb84612f846eaa326eb75c49898 
   src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
 efe62ceb502ead88a2f0cd6d09a76664e465d9bc 
 
 Diff: https://reviews.apache.org/r/31338/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Steve Niemitz
 




Re: Review Request 31338: Added a command line flag, -global_container_mounts, to allow mounting paths from the slaves into the (docker) containers they run.

2015-03-02 Thread Aurora ReviewBot

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


This patch does not apply cleanly on master (4cca6a6), do you need to rebase?

I will refresh this build result if you post a review containing @ReviewBot 
retry

- Aurora ReviewBot


On March 2, 2015, 9:03 p.m., Steve Niemitz wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/31338/
 ---
 
 (Updated March 2, 2015, 9:03 p.m.)
 
 
 Review request for Aurora, Jay Buffington and Bill Farner.
 
 
 Bugs: AURORA-1107
 https://issues.apache.org/jira/browse/AURORA-1107
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Added a command line flag, -global_container_mounts, to allow mounting paths 
 from the slaves into the (docker) containers they run.
 
 This is the first portion of allowing per-job mounts, however, I wanted to 
 get this out first since more people want it.  I'll implement per-job mounts 
 in a future review.
 
 
 Diffs
 -
 
   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
 6f6124a4c844a1abcf07401d80c3e50eb8b4 
   config/checkstyle/checkstyle.xml 580d9d3737c0f52da2d33f5b99ef9406b55c5807 
   docs/deploying-aurora-scheduler.md d1123359961fd59ddb8c1a07f80f293bdd46019f 
   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
 bacfbfeb237ecddf82f58679e05be012c5214e61 
   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
 24b61c1e4f615295acf28d904588e1512972d3f4 
   src/main/java/org/apache/aurora/scheduler/app/VolumeParser.java 
 PRE-CREATION 
   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
 baacb71403d55c5b90fc11cb2a23f552a32e8ba5 
   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
 5340d651b298ec8aa079e73d6d2f652fdf876293 
   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
 6575b7d420f17ec68d6e2a83e7b380f684577d4f 
   src/test/java/org/apache/aurora/scheduler/app/VolumeParserTest.java 
 PRE-CREATION 
   
 src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
 444d6d3fdaf86eb84612f846eaa326eb75c49898 
   src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
 efe62ceb502ead88a2f0cd6d09a76664e465d9bc 
 
 Diff: https://reviews.apache.org/r/31338/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Steve Niemitz
 




Re: Review Request 31338: Added a command line flag, -global_container_mounts, to allow mounting paths from the slaves into the (docker) containers they run.

2015-03-02 Thread Steve Niemitz

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


@ReviewBot retry

- Steve Niemitz


On March 2, 2015, 4:59 p.m., Steve Niemitz wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/31338/
 ---
 
 (Updated March 2, 2015, 4:59 p.m.)
 
 
 Review request for Aurora, Jay Buffington and Bill Farner.
 
 
 Bugs: AURORA-1107
 https://issues.apache.org/jira/browse/AURORA-1107
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Added a command line flag, -global_container_mounts, to allow mounting paths 
 from the slaves into the (docker) containers they run.
 
 This is the first portion of allowing per-job mounts, however, I wanted to 
 get this out first since more people want it.  I'll implement per-job mounts 
 in a future review.
 
 
 Diffs
 -
 
   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
 6f6124a4c844a1abcf07401d80c3e50eb8b4 
   config/checkstyle/checkstyle.xml 580d9d3737c0f52da2d33f5b99ef9406b55c5807 
   docs/deploying-aurora-scheduler.md d1123359961fd59ddb8c1a07f80f293bdd46019f 
   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
 bacfbfeb237ecddf82f58679e05be012c5214e61 
   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
 24b61c1e4f615295acf28d904588e1512972d3f4 
   src/main/java/org/apache/aurora/scheduler/app/VolumeParser.java 
 PRE-CREATION 
   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
 baacb71403d55c5b90fc11cb2a23f552a32e8ba5 
   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
 5340d651b298ec8aa079e73d6d2f652fdf876293 
   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
 6575b7d420f17ec68d6e2a83e7b380f684577d4f 
   src/test/java/org/apache/aurora/scheduler/app/VolumeParserTest.java 
 PRE-CREATION 
   
 src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
 444d6d3fdaf86eb84612f846eaa326eb75c49898 
   src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
 efe62ceb502ead88a2f0cd6d09a76664e465d9bc 
 
 Diff: https://reviews.apache.org/r/31338/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Steve Niemitz
 




Re: Review Request 31338: Added a command line flag, -global_container_mounts, to allow mounting paths from the slaves into the (docker) containers they run.

2015-03-02 Thread Aurora ReviewBot

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


Master (4cca6a6) is red with this patch.
  ./build-support/jenkins/build.sh

 src.test.python.apache.aurora.client.cli.sla   
 .   SUCCESS
 src.test.python.apache.aurora.client.cli.supdate   
 .   SUCCESS
 src.test.python.apache.aurora.client.cli.task  
 .   SUCCESS
 src.test.python.apache.aurora.client.cli.update
 .   SUCCESS
 src.test.python.apache.aurora.client.cli.version   
 .   SUCCESS
 src.test.python.apache.aurora.client.config
 .   SUCCESS
 src.test.python.apache.aurora.client.factory   
 .   SUCCESS
 src.test.python.apache.aurora.client.hooks.hooked_api  
 .   SUCCESS
 src.test.python.apache.aurora.client.hooks.non_hooked_api  
 .   SUCCESS
 src.test.python.apache.aurora.common.test_aurora_job_key   
 .   SUCCESS
 src.test.python.apache.aurora.common.test_cluster  
 .   SUCCESS
 src.test.python.apache.aurora.common.test_cluster_option   
 .   SUCCESS
 src.test.python.apache.aurora.common.test_clusters 
 .   SUCCESS
 src.test.python.apache.aurora.common.test_http_signaler
 .   SUCCESS
 src.test.python.apache.aurora.common.test_pex_version  
 .   SUCCESS
 src.test.python.apache.aurora.common.test_shellify 
 .   SUCCESS
 src.test.python.apache.aurora.common.test_transport
 .   SUCCESS
 src.test.python.apache.aurora.config.test_base 
 .   SUCCESS
 
src.test.python.apache.aurora.config.test_constraint_parsing
.   SUCCESS
 src.test.python.apache.aurora.config.test_loader   
 .   SUCCESS
 src.test.python.apache.aurora.config.test_thrift   
 .   SUCCESS
 src.test.python.apache.aurora.executor.common.announcer
 .   SUCCESS
 
src.test.python.apache.aurora.executor.common.directory_sandbox 
.   SUCCESS
 
src.test.python.apache.aurora.executor.common.executor_detector 
.   SUCCESS
 
src.test.python.apache.aurora.executor.common.executor_timeout  
.   SUCCESS
 
src.test.python.apache.aurora.executor.common.health_checker
.   FAILURE
 
src.test.python.apache.aurora.executor.common.path_detector 
.   SUCCESS
 src.test.python.apache.aurora.executor.common.task_info
 .   SUCCESS
 src.test.python.apache.aurora.executor.executor_base   
 .   SUCCESS
 src.test.python.apache.aurora.executor.executor_vars   
 .   SUCCESS
 src.test.python.apache.aurora.executor.status_manager  
 .   SUCCESS
 src.test.python.apache.aurora.executor.thermos_task_runner 
 .   SUCCESS
 src.test.python.apache.thermos.common.test_pathspec
 .   SUCCESS
 
src.test.python.apache.thermos.core.test_runner_integration 
.   SUCCESS
 src.test.python.apache.thermos.monitoring.test_disk
 .   SUCCESS
 
FAILURE


   FAILURE


I will refresh this build result if you post a review containing @ReviewBot 
retry

- Aurora ReviewBot


On March 2, 2015, 4:59 p.m., Steve Niemitz wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/31338/
 ---
 
 (Updated March 2, 2015, 4:59 p.m.)
 
 
 Review request for Aurora, Jay Buffington and Bill Farner.
 
 
 

Re: Review Request 31338: Added a command line flag, -global_container_mounts, to allow mounting paths from the slaves into the (docker) containers they run.

2015-03-02 Thread Bill Farner


 On Feb. 27, 2015, 5:01 p.m., Bill Farner wrote:
  src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java,
   lines 215-227
  https://reviews.apache.org/r/31338/diff/4/?file=876499#file876499line215
 
  Can this instead be an exact comparison of ListVolume?  Presumably we 
  should fail if other volumes show up.
  
  ```
  assertEquals(
  ImmutableList.of(expected),
  taskInfo.getExecutor().getContainer().getVolumesList());
  ```
 
 Steve Niemitz wrote:
 I think checking only this specifically is a more robust test.  There are 
 other things we may want to mount in by default in the future (for example, 
 we already mount the thermos run directory), and testing for all of them here 
 would make the test to broad.
 
 Bill Farner wrote:
 Hmm...i see your point, but i don't entirely buy it :-)
 
 For example, if the code behind this erroneously inserts two 
 `Protos.Volume` entries for each global mount, the test case should trip.
 
 Steve Niemitz wrote:
 imo it's more likely that we'll be adding more features around volumes 
 (default or otherwise) than introducing breaking changes to the code that 
 adds the default volumes.  Unit tests that are too fragile I think cause more 
 harm than good.

Alrighty, that works for me!


- Bill


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


On March 2, 2015, 4:59 p.m., Steve Niemitz wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/31338/
 ---
 
 (Updated March 2, 2015, 4:59 p.m.)
 
 
 Review request for Aurora, Jay Buffington and Bill Farner.
 
 
 Bugs: AURORA-1107
 https://issues.apache.org/jira/browse/AURORA-1107
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Added a command line flag, -global_container_mounts, to allow mounting paths 
 from the slaves into the (docker) containers they run.
 
 This is the first portion of allowing per-job mounts, however, I wanted to 
 get this out first since more people want it.  I'll implement per-job mounts 
 in a future review.
 
 
 Diffs
 -
 
   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
 6f6124a4c844a1abcf07401d80c3e50eb8b4 
   config/checkstyle/checkstyle.xml 580d9d3737c0f52da2d33f5b99ef9406b55c5807 
   docs/deploying-aurora-scheduler.md d1123359961fd59ddb8c1a07f80f293bdd46019f 
   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
 bacfbfeb237ecddf82f58679e05be012c5214e61 
   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
 24b61c1e4f615295acf28d904588e1512972d3f4 
   src/main/java/org/apache/aurora/scheduler/app/VolumeParser.java 
 PRE-CREATION 
   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
 baacb71403d55c5b90fc11cb2a23f552a32e8ba5 
   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
 5340d651b298ec8aa079e73d6d2f652fdf876293 
   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
 6575b7d420f17ec68d6e2a83e7b380f684577d4f 
   src/test/java/org/apache/aurora/scheduler/app/VolumeParserTest.java 
 PRE-CREATION 
   
 src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
 444d6d3fdaf86eb84612f846eaa326eb75c49898 
   src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
 efe62ceb502ead88a2f0cd6d09a76664e465d9bc 
 
 Diff: https://reviews.apache.org/r/31338/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Steve Niemitz
 




Re: Review Request 31338: Added a command line flag, -global_container_mounts, to allow mounting paths from the slaves into the (docker) containers they run.

2015-03-02 Thread Bill Farner

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

Ship it!


LGTM mod nits.


src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java
https://reviews.apache.org/r/31338/#comment121541

I think you can drop this since it's == `Resources.NONE` (default).



src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java
https://reviews.apache.org/r/31338/#comment121540

remove empty line



src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java
https://reviews.apache.org/r/31338/#comment121537

This can collapse to:

```

assertTrue(taskInfo.getExecutor().getContainer().getVolumesList().contains(expected));
```


- Bill Farner


On March 2, 2015, 4:59 p.m., Steve Niemitz wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/31338/
 ---
 
 (Updated March 2, 2015, 4:59 p.m.)
 
 
 Review request for Aurora, Jay Buffington and Bill Farner.
 
 
 Bugs: AURORA-1107
 https://issues.apache.org/jira/browse/AURORA-1107
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Added a command line flag, -global_container_mounts, to allow mounting paths 
 from the slaves into the (docker) containers they run.
 
 This is the first portion of allowing per-job mounts, however, I wanted to 
 get this out first since more people want it.  I'll implement per-job mounts 
 in a future review.
 
 
 Diffs
 -
 
   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
 6f6124a4c844a1abcf07401d80c3e50eb8b4 
   config/checkstyle/checkstyle.xml 580d9d3737c0f52da2d33f5b99ef9406b55c5807 
   docs/deploying-aurora-scheduler.md d1123359961fd59ddb8c1a07f80f293bdd46019f 
   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
 bacfbfeb237ecddf82f58679e05be012c5214e61 
   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
 24b61c1e4f615295acf28d904588e1512972d3f4 
   src/main/java/org/apache/aurora/scheduler/app/VolumeParser.java 
 PRE-CREATION 
   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
 baacb71403d55c5b90fc11cb2a23f552a32e8ba5 
   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
 5340d651b298ec8aa079e73d6d2f652fdf876293 
   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
 6575b7d420f17ec68d6e2a83e7b380f684577d4f 
   src/test/java/org/apache/aurora/scheduler/app/VolumeParserTest.java 
 PRE-CREATION 
   
 src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
 444d6d3fdaf86eb84612f846eaa326eb75c49898 
   src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
 efe62ceb502ead88a2f0cd6d09a76664e465d9bc 
 
 Diff: https://reviews.apache.org/r/31338/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Steve Niemitz
 




Re: Review Request 31338: Added a command line flag, -global_container_mounts, to allow mounting paths from the slaves into the (docker) containers they run.

2015-02-27 Thread Bill Farner

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


Overall LGTM, all minor issues.


api/src/main/thrift/org/apache/aurora/gen/api.thrift
https://reviews.apache.org/r/31338/#comment121085

The mode for a volume mount



api/src/main/thrift/org/apache/aurora/gen/api.thrift
https://reviews.apache.org/r/31338/#comment121086

s/Describes a/A/



api/src/main/thrift/org/apache/aurora/gen/api.thrift
https://reviews.apache.org/r/31338/#comment121087

s/required //



src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java
https://reviews.apache.org/r/31338/#comment121088

Thanks!  This reads much better.



src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java
https://reviews.apache.org/r/31338/#comment121089

Consider making the 'empty' values defaults in the builder.



src/main/java/org/apache/aurora/scheduler/app/VolumeParser.java
https://reviews.apache.org/r/31338/#comment121090


EnumSet.allOf(Mode.class)
```



src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java
https://reviews.apache.org/r/31338/#comment121091

should this be private?



src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java
https://reviews.apache.org/r/31338/#comment121092

remove extra newline



src/test/java/org/apache/aurora/scheduler/app/VolumeParserTest.java
https://reviews.apache.org/r/31338/#comment121094

s/vp/parser/



src/test/java/org/apache/aurora/scheduler/app/VolumeParserTest.java
https://reviews.apache.org/r/31338/#comment121093

Consider inlining.

```
assertEquals(
  new Volume(...),
  parser.doParse(...));
```



src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java
https://reviews.apache.org/r/31338/#comment121095

Can this instead be an exact comparison of ListVolume?  Presumably we 
should fail if other volumes show up.

```
assertEquals(
ImmutableList.of(expected),
taskInfo.getExecutor().getContainer().getVolumesList());
```


- Bill Farner


On Feb. 25, 2015, 10:35 p.m., Steve Niemitz wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/31338/
 ---
 
 (Updated Feb. 25, 2015, 10:35 p.m.)
 
 
 Review request for Aurora, Jay Buffington and Bill Farner.
 
 
 Bugs: AURORA-1107
 https://issues.apache.org/jira/browse/AURORA-1107
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Added a command line flag, -global_container_mounts, to allow mounting paths 
 from the slaves into the (docker) containers they run.
 
 This is the first portion of allowing per-job mounts, however, I wanted to 
 get this out first since more people want it.  I'll implement per-job mounts 
 in a future review.
 
 
 Diffs
 -
 
   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
 6f6124a4c844a1abcf07401d80c3e50eb8b4 
   config/checkstyle/checkstyle.xml 580d9d3737c0f52da2d33f5b99ef9406b55c5807 
   docs/deploying-aurora-scheduler.md d1123359961fd59ddb8c1a07f80f293bdd46019f 
   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
 bacfbfeb237ecddf82f58679e05be012c5214e61 
   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
 24b61c1e4f615295acf28d904588e1512972d3f4 
   src/main/java/org/apache/aurora/scheduler/app/VolumeParser.java 
 PRE-CREATION 
   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
 baacb71403d55c5b90fc11cb2a23f552a32e8ba5 
   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
 5340d651b298ec8aa079e73d6d2f652fdf876293 
   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
 6575b7d420f17ec68d6e2a83e7b380f684577d4f 
   src/test/java/org/apache/aurora/scheduler/app/VolumeParserTest.java 
 PRE-CREATION 
   
 src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
 444d6d3fdaf86eb84612f846eaa326eb75c49898 
   src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
 efe62ceb502ead88a2f0cd6d09a76664e465d9bc 
 
 Diff: https://reviews.apache.org/r/31338/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Steve Niemitz
 




Re: Review Request 31338: Added a command line flag, -global_container_mounts, to allow mounting paths from the slaves into the (docker) containers they run.

2015-02-27 Thread Steve Niemitz

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

(Updated Feb. 27, 2015, 8:19 p.m.)


Review request for Aurora, Jay Buffington and Bill Farner.


Bugs: AURORA-1107
https://issues.apache.org/jira/browse/AURORA-1107


Repository: aurora


Description
---

Added a command line flag, -global_container_mounts, to allow mounting paths 
from the slaves into the (docker) containers they run.

This is the first portion of allowing per-job mounts, however, I wanted to get 
this out first since more people want it.  I'll implement per-job mounts in a 
future review.


Diffs (updated)
-

  api/src/main/thrift/org/apache/aurora/gen/api.thrift 
6f6124a4c844a1abcf07401d80c3e50eb8b4 
  config/checkstyle/checkstyle.xml 580d9d3737c0f52da2d33f5b99ef9406b55c5807 
  docs/deploying-aurora-scheduler.md d1123359961fd59ddb8c1a07f80f293bdd46019f 
  src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
bacfbfeb237ecddf82f58679e05be012c5214e61 
  src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
24b61c1e4f615295acf28d904588e1512972d3f4 
  src/main/java/org/apache/aurora/scheduler/app/VolumeParser.java PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
baacb71403d55c5b90fc11cb2a23f552a32e8ba5 
  src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
5340d651b298ec8aa079e73d6d2f652fdf876293 
  src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
6575b7d420f17ec68d6e2a83e7b380f684577d4f 
  src/test/java/org/apache/aurora/scheduler/app/VolumeParserTest.java 
PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
444d6d3fdaf86eb84612f846eaa326eb75c49898 
  src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
efe62ceb502ead88a2f0cd6d09a76664e465d9bc 

Diff: https://reviews.apache.org/r/31338/diff/


Testing
---


Thanks,

Steve Niemitz



Re: Review Request 31338: Added a command line flag, -global_container_mounts, to allow mounting paths from the slaves into the (docker) containers they run.

2015-02-27 Thread Steve Niemitz


 On Feb. 27, 2015, 5:01 p.m., Bill Farner wrote:
  src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java, line 
  36
  https://reviews.apache.org/r/31338/diff/4/?file=876495#file876495line36
 
  should this be private?

I wanted it to be, but got style checker violations when making it private.  
Same reason the Builder ctor is public as well.


 On Feb. 27, 2015, 5:01 p.m., Bill Farner wrote:
  src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java,
   lines 215-227
  https://reviews.apache.org/r/31338/diff/4/?file=876499#file876499line215
 
  Can this instead be an exact comparison of ListVolume?  Presumably we 
  should fail if other volumes show up.
  
  ```
  assertEquals(
  ImmutableList.of(expected),
  taskInfo.getExecutor().getContainer().getVolumesList());
  ```

I think checking only this specifically is a more robust test.  There are other 
things we may want to mount in by default in the future (for example, we 
already mount the thermos run directory), and testing for all of them here 
would make the test to broad.


- Steve


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


On Feb. 25, 2015, 10:35 p.m., Steve Niemitz wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/31338/
 ---
 
 (Updated Feb. 25, 2015, 10:35 p.m.)
 
 
 Review request for Aurora, Jay Buffington and Bill Farner.
 
 
 Bugs: AURORA-1107
 https://issues.apache.org/jira/browse/AURORA-1107
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Added a command line flag, -global_container_mounts, to allow mounting paths 
 from the slaves into the (docker) containers they run.
 
 This is the first portion of allowing per-job mounts, however, I wanted to 
 get this out first since more people want it.  I'll implement per-job mounts 
 in a future review.
 
 
 Diffs
 -
 
   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
 6f6124a4c844a1abcf07401d80c3e50eb8b4 
   config/checkstyle/checkstyle.xml 580d9d3737c0f52da2d33f5b99ef9406b55c5807 
   docs/deploying-aurora-scheduler.md d1123359961fd59ddb8c1a07f80f293bdd46019f 
   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
 bacfbfeb237ecddf82f58679e05be012c5214e61 
   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
 24b61c1e4f615295acf28d904588e1512972d3f4 
   src/main/java/org/apache/aurora/scheduler/app/VolumeParser.java 
 PRE-CREATION 
   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
 baacb71403d55c5b90fc11cb2a23f552a32e8ba5 
   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
 5340d651b298ec8aa079e73d6d2f652fdf876293 
   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
 6575b7d420f17ec68d6e2a83e7b380f684577d4f 
   src/test/java/org/apache/aurora/scheduler/app/VolumeParserTest.java 
 PRE-CREATION 
   
 src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
 444d6d3fdaf86eb84612f846eaa326eb75c49898 
   src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
 efe62ceb502ead88a2f0cd6d09a76664e465d9bc 
 
 Diff: https://reviews.apache.org/r/31338/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Steve Niemitz
 




Re: Review Request 31338: Added a command line flag, -global_container_mounts, to allow mounting paths from the slaves into the (docker) containers they run.

2015-02-27 Thread Aurora ReviewBot

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


Master (4cca6a6) is red with this patch.
  ./build-support/jenkins/build.sh

make: Leaving directory 
`/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/thrift'
:api:classesThriftNote: Some input files use unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.

:api:checkPython
:api:generateThriftEntitiesJava
:api:classesThriftEntities
:api:compileJava UP-TO-DATE
:api:generateThriftResources
:api:processResources UP-TO-DATE
:api:classes
:api:jar
:compileJavaNote: Writing 
file:/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/classes/main/com/twitter/common/args/apt/cmdline.arg.info.txt.2

:processResources
:classes
:jar
:assemble
:compileJmhJava
:processJmhResources UP-TO-DATE
:jmhClasses
:checkstyleJmh
:jsHint
:checkstyleMain[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/app/VolumeParser.java:16:8:
 Unused import - java.util.ArrayList.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/app/VolumeParser.java:17:8:
 Unused import - java.util.List.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java:21:
 'org.apache.aurora.gen.Volume' should be separated from previous imports.
 FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':checkstyleMain'.
 Checkstyle rule violations were found. See the report at: 
 file:///home/jenkins/jenkins-slave/workspace/AuroraBot/dist/reports/checkstyle/main.xml

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug 
option to get more log output.

BUILD FAILED

Total time: 1 mins 38.789 secs


I will refresh this build result if you post a review containing @ReviewBot 
retry

- Aurora ReviewBot


On Feb. 27, 2015, 11:16 p.m., Steve Niemitz wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/31338/
 ---
 
 (Updated Feb. 27, 2015, 11:16 p.m.)
 
 
 Review request for Aurora, Jay Buffington and Bill Farner.
 
 
 Bugs: AURORA-1107
 https://issues.apache.org/jira/browse/AURORA-1107
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Added a command line flag, -global_container_mounts, to allow mounting paths 
 from the slaves into the (docker) containers they run.
 
 This is the first portion of allowing per-job mounts, however, I wanted to 
 get this out first since more people want it.  I'll implement per-job mounts 
 in a future review.
 
 
 Diffs
 -
 
   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
 6f6124a4c844a1abcf07401d80c3e50eb8b4 
   config/checkstyle/checkstyle.xml 580d9d3737c0f52da2d33f5b99ef9406b55c5807 
   docs/deploying-aurora-scheduler.md d1123359961fd59ddb8c1a07f80f293bdd46019f 
   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
 bacfbfeb237ecddf82f58679e05be012c5214e61 
   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
 24b61c1e4f615295acf28d904588e1512972d3f4 
   src/main/java/org/apache/aurora/scheduler/app/VolumeParser.java 
 PRE-CREATION 
   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
 baacb71403d55c5b90fc11cb2a23f552a32e8ba5 
   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
 5340d651b298ec8aa079e73d6d2f652fdf876293 
   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
 6575b7d420f17ec68d6e2a83e7b380f684577d4f 
   src/test/java/org/apache/aurora/scheduler/app/VolumeParserTest.java 
 PRE-CREATION 
   
 src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
 444d6d3fdaf86eb84612f846eaa326eb75c49898 
   src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
 efe62ceb502ead88a2f0cd6d09a76664e465d9bc 
 
 Diff: https://reviews.apache.org/r/31338/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Steve Niemitz
 




Re: Review Request 31338: Added a command line flag, -global_container_mounts, to allow mounting paths from the slaves into the (docker) containers they run.

2015-02-27 Thread Bill Farner


 On Feb. 27, 2015, 5:01 p.m., Bill Farner wrote:
  src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java,
   lines 215-227
  https://reviews.apache.org/r/31338/diff/4/?file=876499#file876499line215
 
  Can this instead be an exact comparison of ListVolume?  Presumably we 
  should fail if other volumes show up.
  
  ```
  assertEquals(
  ImmutableList.of(expected),
  taskInfo.getExecutor().getContainer().getVolumesList());
  ```
 
 Steve Niemitz wrote:
 I think checking only this specifically is a more robust test.  There are 
 other things we may want to mount in by default in the future (for example, 
 we already mount the thermos run directory), and testing for all of them here 
 would make the test to broad.

Hmm...i see your point, but i don't entirely buy it :-)

For example, if the code behind this erroneously inserts two `Protos.Volume` 
entries for each global mount, the test case should trip.


- Bill


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


On Feb. 27, 2015, 11:16 p.m., Steve Niemitz wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/31338/
 ---
 
 (Updated Feb. 27, 2015, 11:16 p.m.)
 
 
 Review request for Aurora, Jay Buffington and Bill Farner.
 
 
 Bugs: AURORA-1107
 https://issues.apache.org/jira/browse/AURORA-1107
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Added a command line flag, -global_container_mounts, to allow mounting paths 
 from the slaves into the (docker) containers they run.
 
 This is the first portion of allowing per-job mounts, however, I wanted to 
 get this out first since more people want it.  I'll implement per-job mounts 
 in a future review.
 
 
 Diffs
 -
 
   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
 6f6124a4c844a1abcf07401d80c3e50eb8b4 
   config/checkstyle/checkstyle.xml 580d9d3737c0f52da2d33f5b99ef9406b55c5807 
   docs/deploying-aurora-scheduler.md d1123359961fd59ddb8c1a07f80f293bdd46019f 
   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
 bacfbfeb237ecddf82f58679e05be012c5214e61 
   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
 24b61c1e4f615295acf28d904588e1512972d3f4 
   src/main/java/org/apache/aurora/scheduler/app/VolumeParser.java 
 PRE-CREATION 
   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
 baacb71403d55c5b90fc11cb2a23f552a32e8ba5 
   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
 5340d651b298ec8aa079e73d6d2f652fdf876293 
   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
 6575b7d420f17ec68d6e2a83e7b380f684577d4f 
   src/test/java/org/apache/aurora/scheduler/app/VolumeParserTest.java 
 PRE-CREATION 
   
 src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
 444d6d3fdaf86eb84612f846eaa326eb75c49898 
   src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
 efe62ceb502ead88a2f0cd6d09a76664e465d9bc 
 
 Diff: https://reviews.apache.org/r/31338/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Steve Niemitz
 




Re: Review Request 31338: Added a command line flag, -global_container_mounts, to allow mounting paths from the slaves into the (docker) containers they run.

2015-02-27 Thread Steve Niemitz

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

(Updated Feb. 27, 2015, 11:16 p.m.)


Review request for Aurora, Jay Buffington and Bill Farner.


Bugs: AURORA-1107
https://issues.apache.org/jira/browse/AURORA-1107


Repository: aurora


Description
---

Added a command line flag, -global_container_mounts, to allow mounting paths 
from the slaves into the (docker) containers they run.

This is the first portion of allowing per-job mounts, however, I wanted to get 
this out first since more people want it.  I'll implement per-job mounts in a 
future review.


Diffs (updated)
-

  api/src/main/thrift/org/apache/aurora/gen/api.thrift 
6f6124a4c844a1abcf07401d80c3e50eb8b4 
  config/checkstyle/checkstyle.xml 580d9d3737c0f52da2d33f5b99ef9406b55c5807 
  docs/deploying-aurora-scheduler.md d1123359961fd59ddb8c1a07f80f293bdd46019f 
  src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
bacfbfeb237ecddf82f58679e05be012c5214e61 
  src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
24b61c1e4f615295acf28d904588e1512972d3f4 
  src/main/java/org/apache/aurora/scheduler/app/VolumeParser.java PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
baacb71403d55c5b90fc11cb2a23f552a32e8ba5 
  src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
5340d651b298ec8aa079e73d6d2f652fdf876293 
  src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
6575b7d420f17ec68d6e2a83e7b380f684577d4f 
  src/test/java/org/apache/aurora/scheduler/app/VolumeParserTest.java 
PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
444d6d3fdaf86eb84612f846eaa326eb75c49898 
  src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
efe62ceb502ead88a2f0cd6d09a76664e465d9bc 

Diff: https://reviews.apache.org/r/31338/diff/


Testing
---


Thanks,

Steve Niemitz



Re: Review Request 31338: Added a command line flag, -global_container_mounts, to allow mounting paths from the slaves into the (docker) containers they run.

2015-02-25 Thread Joshua Cohen

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


lgtm minus a few nits...


src/test/java/org/apache/aurora/scheduler/app/VolumeParserTest.java
https://reviews.apache.org/r/31338/#comment120622

this should be unnecessary, we will have thrown on the previous line?

Also can you add a test for too many parts as well (e.g. foo:bar:baz:quux)?



src/test/java/org/apache/aurora/scheduler/app/VolumeParserTest.java
https://reviews.apache.org/r/31338/#comment120623

Again, should be unnecessary.



src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java
https://reviews.apache.org/r/31338/#comment120624

move to next line.

Also is this potentially fragile to assume the global mount will be the 
second mount in the list? It'd be slightly more robust to iterate the mounts 
and ensure the mount we expect is *somewhere* in the list?


- Joshua Cohen


On Feb. 25, 2015, 7:15 p.m., Steve Niemitz wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/31338/
 ---
 
 (Updated Feb. 25, 2015, 7:15 p.m.)
 
 
 Review request for Aurora, Jay Buffington and Bill Farner.
 
 
 Bugs: AURORA-1107
 https://issues.apache.org/jira/browse/AURORA-1107
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Added a command line flag, -global_container_mounts, to allow mounting paths 
 from the slaves into the (docker) containers they run.
 
 This is the first portion of allowing per-job mounts, however, I wanted to 
 get this out first since more people want it.  I'll implement per-job mounts 
 in a future review.
 
 
 Diffs
 -
 
   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
 6f6124a4c844a1abcf07401d80c3e50eb8b4 
   config/checkstyle/checkstyle.xml 580d9d3737c0f52da2d33f5b99ef9406b55c5807 
   docs/deploying-aurora-scheduler.md d1123359961fd59ddb8c1a07f80f293bdd46019f 
   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
 bacfbfeb237ecddf82f58679e05be012c5214e61 
   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
 24b61c1e4f615295acf28d904588e1512972d3f4 
   src/main/java/org/apache/aurora/scheduler/app/VolumeParser.java 
 PRE-CREATION 
   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
 baacb71403d55c5b90fc11cb2a23f552a32e8ba5 
   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
 5340d651b298ec8aa079e73d6d2f652fdf876293 
   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
 6575b7d420f17ec68d6e2a83e7b380f684577d4f 
   src/test/java/org/apache/aurora/scheduler/app/VolumeParserTest.java 
 PRE-CREATION 
   
 src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
 444d6d3fdaf86eb84612f846eaa326eb75c49898 
   src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
 efe62ceb502ead88a2f0cd6d09a76664e465d9bc 
 
 Diff: https://reviews.apache.org/r/31338/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Steve Niemitz
 




Re: Review Request 31338: Added a command line flag, -global_container_mounts, to allow mounting paths from the slaves into the (docker) containers they run.

2015-02-25 Thread Steve Niemitz


 On Feb. 25, 2015, 10:06 p.m., Joshua Cohen wrote:
  src/test/java/org/apache/aurora/scheduler/app/VolumeParserTest.java, lines 
  44-45
  https://reviews.apache.org/r/31338/diff/2/?file=876074#file876074line44
 
  this should be unnecessary, we will have thrown on the previous line?
  
  Also can you add a test for too many parts as well (e.g. 
  foo:bar:baz:quux)?

I added these in to stop some complaints about unused variables, but in 
retrospect I could have just called vp.doParse(...) and not assigned it.  I'll 
change these.


 On Feb. 25, 2015, 10:06 p.m., Joshua Cohen wrote:
  src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java,
   line 214
  https://reviews.apache.org/r/31338/diff/2/?file=876075#file876075line214
 
  move to next line.
  
  Also is this potentially fragile to assume the global mount will be the 
  second mount in the list? It'd be slightly more robust to iterate the 
  mounts and ensure the mount we expect is *somewhere* in the list?

Good point on the ordering, it shouldn't matter.  I'll change it.


- Steve


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


On Feb. 25, 2015, 7:15 p.m., Steve Niemitz wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/31338/
 ---
 
 (Updated Feb. 25, 2015, 7:15 p.m.)
 
 
 Review request for Aurora, Jay Buffington and Bill Farner.
 
 
 Bugs: AURORA-1107
 https://issues.apache.org/jira/browse/AURORA-1107
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Added a command line flag, -global_container_mounts, to allow mounting paths 
 from the slaves into the (docker) containers they run.
 
 This is the first portion of allowing per-job mounts, however, I wanted to 
 get this out first since more people want it.  I'll implement per-job mounts 
 in a future review.
 
 
 Diffs
 -
 
   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
 6f6124a4c844a1abcf07401d80c3e50eb8b4 
   config/checkstyle/checkstyle.xml 580d9d3737c0f52da2d33f5b99ef9406b55c5807 
   docs/deploying-aurora-scheduler.md d1123359961fd59ddb8c1a07f80f293bdd46019f 
   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
 bacfbfeb237ecddf82f58679e05be012c5214e61 
   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
 24b61c1e4f615295acf28d904588e1512972d3f4 
   src/main/java/org/apache/aurora/scheduler/app/VolumeParser.java 
 PRE-CREATION 
   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
 baacb71403d55c5b90fc11cb2a23f552a32e8ba5 
   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
 5340d651b298ec8aa079e73d6d2f652fdf876293 
   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
 6575b7d420f17ec68d6e2a83e7b380f684577d4f 
   src/test/java/org/apache/aurora/scheduler/app/VolumeParserTest.java 
 PRE-CREATION 
   
 src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
 444d6d3fdaf86eb84612f846eaa326eb75c49898 
   src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
 efe62ceb502ead88a2f0cd6d09a76664e465d9bc 
 
 Diff: https://reviews.apache.org/r/31338/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Steve Niemitz
 




Re: Review Request 31338: Added a command line flag, -global_container_mounts, to allow mounting paths from the slaves into the (docker) containers they run.

2015-02-25 Thread Aurora ReviewBot

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

Ship it!


Master (9442e08) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing @ReviewBot 
retry

- Aurora ReviewBot


On Feb. 25, 2015, 10:35 p.m., Steve Niemitz wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/31338/
 ---
 
 (Updated Feb. 25, 2015, 10:35 p.m.)
 
 
 Review request for Aurora, Jay Buffington and Bill Farner.
 
 
 Bugs: AURORA-1107
 https://issues.apache.org/jira/browse/AURORA-1107
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Added a command line flag, -global_container_mounts, to allow mounting paths 
 from the slaves into the (docker) containers they run.
 
 This is the first portion of allowing per-job mounts, however, I wanted to 
 get this out first since more people want it.  I'll implement per-job mounts 
 in a future review.
 
 
 Diffs
 -
 
   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
 6f6124a4c844a1abcf07401d80c3e50eb8b4 
   config/checkstyle/checkstyle.xml 580d9d3737c0f52da2d33f5b99ef9406b55c5807 
   docs/deploying-aurora-scheduler.md d1123359961fd59ddb8c1a07f80f293bdd46019f 
   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
 bacfbfeb237ecddf82f58679e05be012c5214e61 
   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
 24b61c1e4f615295acf28d904588e1512972d3f4 
   src/main/java/org/apache/aurora/scheduler/app/VolumeParser.java 
 PRE-CREATION 
   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
 baacb71403d55c5b90fc11cb2a23f552a32e8ba5 
   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
 5340d651b298ec8aa079e73d6d2f652fdf876293 
   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
 6575b7d420f17ec68d6e2a83e7b380f684577d4f 
   src/test/java/org/apache/aurora/scheduler/app/VolumeParserTest.java 
 PRE-CREATION 
   
 src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
 444d6d3fdaf86eb84612f846eaa326eb75c49898 
   src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
 efe62ceb502ead88a2f0cd6d09a76664e465d9bc 
 
 Diff: https://reviews.apache.org/r/31338/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Steve Niemitz
 




Re: Review Request 31338: Added a command line flag, -global_container_mounts, to allow mounting paths from the slaves into the (docker) containers they run.

2015-02-25 Thread Steve Niemitz

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

(Updated Feb. 25, 2015, 10:35 p.m.)


Review request for Aurora, Jay Buffington and Bill Farner.


Changes
---

That's what I get for not running tests on trivial changes...


Bugs: AURORA-1107
https://issues.apache.org/jira/browse/AURORA-1107


Repository: aurora


Description
---

Added a command line flag, -global_container_mounts, to allow mounting paths 
from the slaves into the (docker) containers they run.

This is the first portion of allowing per-job mounts, however, I wanted to get 
this out first since more people want it.  I'll implement per-job mounts in a 
future review.


Diffs (updated)
-

  api/src/main/thrift/org/apache/aurora/gen/api.thrift 
6f6124a4c844a1abcf07401d80c3e50eb8b4 
  config/checkstyle/checkstyle.xml 580d9d3737c0f52da2d33f5b99ef9406b55c5807 
  docs/deploying-aurora-scheduler.md d1123359961fd59ddb8c1a07f80f293bdd46019f 
  src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
bacfbfeb237ecddf82f58679e05be012c5214e61 
  src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
24b61c1e4f615295acf28d904588e1512972d3f4 
  src/main/java/org/apache/aurora/scheduler/app/VolumeParser.java PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
baacb71403d55c5b90fc11cb2a23f552a32e8ba5 
  src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
5340d651b298ec8aa079e73d6d2f652fdf876293 
  src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
6575b7d420f17ec68d6e2a83e7b380f684577d4f 
  src/test/java/org/apache/aurora/scheduler/app/VolumeParserTest.java 
PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
444d6d3fdaf86eb84612f846eaa326eb75c49898 
  src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
efe62ceb502ead88a2f0cd6d09a76664e465d9bc 

Diff: https://reviews.apache.org/r/31338/diff/


Testing
---


Thanks,

Steve Niemitz



Re: Review Request 31338: Added a command line flag, -global_container_mounts, to allow mounting paths from the slaves into the (docker) containers they run.

2015-02-24 Thread Bill Farner


 On Feb. 24, 2015, 6:01 a.m., Joshua Cohen wrote:
 
 
 Steve Niemitz wrote:
 I'm not a big fan of how the parsing works here either.  I was thinking 
 about this last night, I think I have a better plan here.  Lemme know what 
 you think.
 
 I already want to add volume support per-job at some point, so I propose 
 adding the needed thrift objects to api.thrift (Volume, Mode enum), and 
 converting the command line into those objects in ExecutorSettings.  That 
 would then let me reuse the volume-adding code in MesosTaskFactory for normal 
 volumes in the future.
 
 Re: command line, I chose the format there because it's the same as the 
 docker command line.  What about making a new parser to parse that into the 
 above mentioned thrift objects?
 
 Stephan Erb wrote:
 How about using the same format and argument name as Mesos' 
 `--default_container_info`:
 
 {
   type: MESOS,
   volumes: [
 {
   host_path: ./.private/tmp,
   container_path: /tmp,
   mode: RW
 }
   ]
 }
 
 See https://mesos.apache.org/documentation/latest/configuration/
 
 Steve Niemitz wrote:
 JSON on the command line is an escaping nightmare IMO, and very verbose 
 to specify 2 (or 3) strings for this use case.
 
 Bill Farner wrote:
 JSON on the command line also makes me uneasy, but i don't think we have 
 precedent for command line entities with this many knobs (3 are obvious so 
 far).
 
 How about 2 approaches as straw men:
 
 `-volume_mounts_read_only` and `-volume_mounts_read_write`, both 
 key-value mappings
 
 `-volume_mounts_config`, path to a JSON file with entities similar to 
 what Stephan sketched above
 
 The first approach has the upside that it fits with the status quo, but 
 it cannot be easily extended if we ever want more attributes.  The second 
 would be the first config file we use, but is more future-proof.
 
 Joshua Cohen wrote:
 I think a single arg is easiest to reason about. I think I'm ok with 
 Steve's follow up suggestion to create the thrift types that will be needed 
 for per-job mounts now and use them to aid in arg parsing? That said, I'm not 
 sure we should enforce the docker mount format since in theory these mounts 
 can/will apply to other containers where they may not make as much sense.
 
 Steve Niemitz wrote:
 I think it's simple enough it'd make sense for any container system.  
 host:container:mode is pretty generic.  I'm also not as big of a fan about 
 multiple args, and a default of RO if not specified seems reasonable to me.

Works for me, but -1 to defaulting.  I much prefer explicit over implicit, and 
this way it's easy to see how to turn a read-only mount into read-write.


- Bill


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


On Feb. 24, 2015, 2:07 a.m., Steve Niemitz wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/31338/
 ---
 
 (Updated Feb. 24, 2015, 2:07 a.m.)
 
 
 Review request for Aurora, Jay Buffington and Bill Farner.
 
 
 Bugs: AURORA-1107
 https://issues.apache.org/jira/browse/AURORA-1107
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Added a command line flag, -global_container_mounts, to allow mounting paths 
 from the slaves into the (docker) containers they run.
 
 This is the first portion of allowing per-job mounts, however, I wanted to 
 get this out first since more people want it.  I'll implement per-job mounts 
 in a future review.
 
 
 Diffs
 -
 
   docs/deploying-aurora-scheduler.md d1123359961fd59ddb8c1a07f80f293bdd46019f 
   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
 bacfbfeb237ecddf82f58679e05be012c5214e61 
   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
 24b61c1e4f615295acf28d904588e1512972d3f4 
   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
 baacb71403d55c5b90fc11cb2a23f552a32e8ba5 
   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
 5340d651b298ec8aa079e73d6d2f652fdf876293 
   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
 6575b7d420f17ec68d6e2a83e7b380f684577d4f 
   
 src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
 444d6d3fdaf86eb84612f846eaa326eb75c49898 
   src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
 efe62ceb502ead88a2f0cd6d09a76664e465d9bc 
 
 Diff: https://reviews.apache.org/r/31338/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Steve Niemitz
 




Re: Review Request 31338: Added a command line flag, -global_container_mounts, to allow mounting paths from the slaves into the (docker) containers they run.

2015-02-24 Thread Steve Niemitz


 On Feb. 24, 2015, 6:01 a.m., Joshua Cohen wrote:
 
 
 Steve Niemitz wrote:
 I'm not a big fan of how the parsing works here either.  I was thinking 
 about this last night, I think I have a better plan here.  Lemme know what 
 you think.
 
 I already want to add volume support per-job at some point, so I propose 
 adding the needed thrift objects to api.thrift (Volume, Mode enum), and 
 converting the command line into those objects in ExecutorSettings.  That 
 would then let me reuse the volume-adding code in MesosTaskFactory for normal 
 volumes in the future.
 
 Re: command line, I chose the format there because it's the same as the 
 docker command line.  What about making a new parser to parse that into the 
 above mentioned thrift objects?
 
 Stephan Erb wrote:
 How about using the same format and argument name as Mesos' 
 `--default_container_info`:
 
 {
   type: MESOS,
   volumes: [
 {
   host_path: ./.private/tmp,
   container_path: /tmp,
   mode: RW
 }
   ]
 }
 
 See https://mesos.apache.org/documentation/latest/configuration/

JSON on the command line is an escaping nightmare IMO, and very verbose to 
specify 2 (or 3) strings for this use case.


- Steve


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


On Feb. 24, 2015, 2:07 a.m., Steve Niemitz wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/31338/
 ---
 
 (Updated Feb. 24, 2015, 2:07 a.m.)
 
 
 Review request for Aurora, Jay Buffington and Bill Farner.
 
 
 Bugs: AURORA-1107
 https://issues.apache.org/jira/browse/AURORA-1107
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Added a command line flag, -global_container_mounts, to allow mounting paths 
 from the slaves into the (docker) containers they run.
 
 This is the first portion of allowing per-job mounts, however, I wanted to 
 get this out first since more people want it.  I'll implement per-job mounts 
 in a future review.
 
 
 Diffs
 -
 
   docs/deploying-aurora-scheduler.md d1123359961fd59ddb8c1a07f80f293bdd46019f 
   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
 bacfbfeb237ecddf82f58679e05be012c5214e61 
   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
 24b61c1e4f615295acf28d904588e1512972d3f4 
   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
 baacb71403d55c5b90fc11cb2a23f552a32e8ba5 
   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
 5340d651b298ec8aa079e73d6d2f652fdf876293 
   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
 6575b7d420f17ec68d6e2a83e7b380f684577d4f 
   
 src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
 444d6d3fdaf86eb84612f846eaa326eb75c49898 
   src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
 efe62ceb502ead88a2f0cd6d09a76664e465d9bc 
 
 Diff: https://reviews.apache.org/r/31338/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Steve Niemitz
 




Re: Review Request 31338: Added a command line flag, -global_container_mounts, to allow mounting paths from the slaves into the (docker) containers they run.

2015-02-24 Thread Stephan Erb

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



src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java
https://reviews.apache.org/r/31338/#comment120230

This code will also work for a default Mesos container on a slave with the 
`filesystem/shared` isolator.

Is it worth to generalize here and make mounting work for docker and 
non-docker containers?


- Stephan Erb


On Feb. 24, 2015, 3:07 a.m., Steve Niemitz wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/31338/
 ---
 
 (Updated Feb. 24, 2015, 3:07 a.m.)
 
 
 Review request for Aurora, Jay Buffington and Bill Farner.
 
 
 Bugs: AURORA-1107
 https://issues.apache.org/jira/browse/AURORA-1107
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Added a command line flag, -global_container_mounts, to allow mounting paths 
 from the slaves into the (docker) containers they run.
 
 This is the first portion of allowing per-job mounts, however, I wanted to 
 get this out first since more people want it.  I'll implement per-job mounts 
 in a future review.
 
 
 Diffs
 -
 
   docs/deploying-aurora-scheduler.md d1123359961fd59ddb8c1a07f80f293bdd46019f 
   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
 bacfbfeb237ecddf82f58679e05be012c5214e61 
   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
 24b61c1e4f615295acf28d904588e1512972d3f4 
   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
 baacb71403d55c5b90fc11cb2a23f552a32e8ba5 
   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
 5340d651b298ec8aa079e73d6d2f652fdf876293 
   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
 6575b7d420f17ec68d6e2a83e7b380f684577d4f 
   
 src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
 444d6d3fdaf86eb84612f846eaa326eb75c49898 
   src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
 efe62ceb502ead88a2f0cd6d09a76664e465d9bc 
 
 Diff: https://reviews.apache.org/r/31338/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Steve Niemitz
 




Re: Review Request 31338: Added a command line flag, -global_container_mounts, to allow mounting paths from the slaves into the (docker) containers they run.

2015-02-24 Thread Joshua Cohen


 On Feb. 24, 2015, 6:01 a.m., Joshua Cohen wrote:
 
 
 Steve Niemitz wrote:
 I'm not a big fan of how the parsing works here either.  I was thinking 
 about this last night, I think I have a better plan here.  Lemme know what 
 you think.
 
 I already want to add volume support per-job at some point, so I propose 
 adding the needed thrift objects to api.thrift (Volume, Mode enum), and 
 converting the command line into those objects in ExecutorSettings.  That 
 would then let me reuse the volume-adding code in MesosTaskFactory for normal 
 volumes in the future.
 
 Re: command line, I chose the format there because it's the same as the 
 docker command line.  What about making a new parser to parse that into the 
 above mentioned thrift objects?
 
 Stephan Erb wrote:
 How about using the same format and argument name as Mesos' 
 `--default_container_info`:
 
 {
   type: MESOS,
   volumes: [
 {
   host_path: ./.private/tmp,
   container_path: /tmp,
   mode: RW
 }
   ]
 }
 
 See https://mesos.apache.org/documentation/latest/configuration/
 
 Steve Niemitz wrote:
 JSON on the command line is an escaping nightmare IMO, and very verbose 
 to specify 2 (or 3) strings for this use case.
 
 Bill Farner wrote:
 JSON on the command line also makes me uneasy, but i don't think we have 
 precedent for command line entities with this many knobs (3 are obvious so 
 far).
 
 How about 2 approaches as straw men:
 
 `-volume_mounts_read_only` and `-volume_mounts_read_write`, both 
 key-value mappings
 
 `-volume_mounts_config`, path to a JSON file with entities similar to 
 what Stephan sketched above
 
 The first approach has the upside that it fits with the status quo, but 
 it cannot be easily extended if we ever want more attributes.  The second 
 would be the first config file we use, but is more future-proof.

I think a single arg is easiest to reason about. I think I'm ok with Steve's 
follow up suggestion to create the thrift types that will be needed for per-job 
mounts now and use them to aid in arg parsing? That said, I'm not sure we 
should enforce the docker mount format since in theory these mounts can/will 
apply to other containers where they may not make as much sense.


- Joshua


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


On Feb. 24, 2015, 2:07 a.m., Steve Niemitz wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/31338/
 ---
 
 (Updated Feb. 24, 2015, 2:07 a.m.)
 
 
 Review request for Aurora, Jay Buffington and Bill Farner.
 
 
 Bugs: AURORA-1107
 https://issues.apache.org/jira/browse/AURORA-1107
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Added a command line flag, -global_container_mounts, to allow mounting paths 
 from the slaves into the (docker) containers they run.
 
 This is the first portion of allowing per-job mounts, however, I wanted to 
 get this out first since more people want it.  I'll implement per-job mounts 
 in a future review.
 
 
 Diffs
 -
 
   docs/deploying-aurora-scheduler.md d1123359961fd59ddb8c1a07f80f293bdd46019f 
   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
 bacfbfeb237ecddf82f58679e05be012c5214e61 
   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
 24b61c1e4f615295acf28d904588e1512972d3f4 
   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
 baacb71403d55c5b90fc11cb2a23f552a32e8ba5 
   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
 5340d651b298ec8aa079e73d6d2f652fdf876293 
   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
 6575b7d420f17ec68d6e2a83e7b380f684577d4f 
   
 src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
 444d6d3fdaf86eb84612f846eaa326eb75c49898 
   src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
 efe62ceb502ead88a2f0cd6d09a76664e465d9bc 
 
 Diff: https://reviews.apache.org/r/31338/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Steve Niemitz
 




Re: Review Request 31338: Added a command line flag, -global_container_mounts, to allow mounting paths from the slaves into the (docker) containers they run.

2015-02-24 Thread Bill Farner


 On Feb. 24, 2015, 6:01 a.m., Joshua Cohen wrote:
 
 
 Steve Niemitz wrote:
 I'm not a big fan of how the parsing works here either.  I was thinking 
 about this last night, I think I have a better plan here.  Lemme know what 
 you think.
 
 I already want to add volume support per-job at some point, so I propose 
 adding the needed thrift objects to api.thrift (Volume, Mode enum), and 
 converting the command line into those objects in ExecutorSettings.  That 
 would then let me reuse the volume-adding code in MesosTaskFactory for normal 
 volumes in the future.
 
 Re: command line, I chose the format there because it's the same as the 
 docker command line.  What about making a new parser to parse that into the 
 above mentioned thrift objects?
 
 Stephan Erb wrote:
 How about using the same format and argument name as Mesos' 
 `--default_container_info`:
 
 {
   type: MESOS,
   volumes: [
 {
   host_path: ./.private/tmp,
   container_path: /tmp,
   mode: RW
 }
   ]
 }
 
 See https://mesos.apache.org/documentation/latest/configuration/
 
 Steve Niemitz wrote:
 JSON on the command line is an escaping nightmare IMO, and very verbose 
 to specify 2 (or 3) strings for this use case.

JSON on the command line also makes me uneasy, but i don't think we have 
precedent for command line entities with this many knobs (3 are obvious so far).

How about 2 approaches as straw men:

`-volume_mounts_read_only` and `-volume_mounts_read_write`, both key-value 
mappings

`-volume_mounts_config`, path to a JSON file with entities similar to what 
Stephan sketched above

The first approach has the upside that it fits with the status quo, but it 
cannot be easily extended if we ever want more attributes.  The second would be 
the first config file we use, but is more future-proof.


- Bill


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


On Feb. 24, 2015, 2:07 a.m., Steve Niemitz wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/31338/
 ---
 
 (Updated Feb. 24, 2015, 2:07 a.m.)
 
 
 Review request for Aurora, Jay Buffington and Bill Farner.
 
 
 Bugs: AURORA-1107
 https://issues.apache.org/jira/browse/AURORA-1107
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Added a command line flag, -global_container_mounts, to allow mounting paths 
 from the slaves into the (docker) containers they run.
 
 This is the first portion of allowing per-job mounts, however, I wanted to 
 get this out first since more people want it.  I'll implement per-job mounts 
 in a future review.
 
 
 Diffs
 -
 
   docs/deploying-aurora-scheduler.md d1123359961fd59ddb8c1a07f80f293bdd46019f 
   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
 bacfbfeb237ecddf82f58679e05be012c5214e61 
   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
 24b61c1e4f615295acf28d904588e1512972d3f4 
   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
 baacb71403d55c5b90fc11cb2a23f552a32e8ba5 
   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
 5340d651b298ec8aa079e73d6d2f652fdf876293 
   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
 6575b7d420f17ec68d6e2a83e7b380f684577d4f 
   
 src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
 444d6d3fdaf86eb84612f846eaa326eb75c49898 
   src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
 efe62ceb502ead88a2f0cd6d09a76664e465d9bc 
 
 Diff: https://reviews.apache.org/r/31338/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Steve Niemitz
 




Re: Review Request 31338: Added a command line flag, -global_container_mounts, to allow mounting paths from the slaves into the (docker) containers they run.

2015-02-24 Thread Steve Niemitz


 On Feb. 24, 2015, 6:01 a.m., Joshua Cohen wrote:
 
 
 Steve Niemitz wrote:
 I'm not a big fan of how the parsing works here either.  I was thinking 
 about this last night, I think I have a better plan here.  Lemme know what 
 you think.
 
 I already want to add volume support per-job at some point, so I propose 
 adding the needed thrift objects to api.thrift (Volume, Mode enum), and 
 converting the command line into those objects in ExecutorSettings.  That 
 would then let me reuse the volume-adding code in MesosTaskFactory for normal 
 volumes in the future.
 
 Re: command line, I chose the format there because it's the same as the 
 docker command line.  What about making a new parser to parse that into the 
 above mentioned thrift objects?
 
 Stephan Erb wrote:
 How about using the same format and argument name as Mesos' 
 `--default_container_info`:
 
 {
   type: MESOS,
   volumes: [
 {
   host_path: ./.private/tmp,
   container_path: /tmp,
   mode: RW
 }
   ]
 }
 
 See https://mesos.apache.org/documentation/latest/configuration/
 
 Steve Niemitz wrote:
 JSON on the command line is an escaping nightmare IMO, and very verbose 
 to specify 2 (or 3) strings for this use case.
 
 Bill Farner wrote:
 JSON on the command line also makes me uneasy, but i don't think we have 
 precedent for command line entities with this many knobs (3 are obvious so 
 far).
 
 How about 2 approaches as straw men:
 
 `-volume_mounts_read_only` and `-volume_mounts_read_write`, both 
 key-value mappings
 
 `-volume_mounts_config`, path to a JSON file with entities similar to 
 what Stephan sketched above
 
 The first approach has the upside that it fits with the status quo, but 
 it cannot be easily extended if we ever want more attributes.  The second 
 would be the first config file we use, but is more future-proof.
 
 Joshua Cohen wrote:
 I think a single arg is easiest to reason about. I think I'm ok with 
 Steve's follow up suggestion to create the thrift types that will be needed 
 for per-job mounts now and use them to aid in arg parsing? That said, I'm not 
 sure we should enforce the docker mount format since in theory these mounts 
 can/will apply to other containers where they may not make as much sense.
 
 Steve Niemitz wrote:
 I think it's simple enough it'd make sense for any container system.  
 host:container:mode is pretty generic.  I'm also not as big of a fan about 
 multiple args, and a default of RO if not specified seems reasonable to me.
 
 Bill Farner wrote:
 Works for me, but -1 to defaulting.  I much prefer explicit over 
 implicit, and this way it's easy to see how to turn a read-only mount into 
 read-write.

Cool with me.  Ok I think we're all on the same page here.  I'll submit an 
updated patch in a little bit.


- Steve


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


On Feb. 24, 2015, 2:07 a.m., Steve Niemitz wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/31338/
 ---
 
 (Updated Feb. 24, 2015, 2:07 a.m.)
 
 
 Review request for Aurora, Jay Buffington and Bill Farner.
 
 
 Bugs: AURORA-1107
 https://issues.apache.org/jira/browse/AURORA-1107
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Added a command line flag, -global_container_mounts, to allow mounting paths 
 from the slaves into the (docker) containers they run.
 
 This is the first portion of allowing per-job mounts, however, I wanted to 
 get this out first since more people want it.  I'll implement per-job mounts 
 in a future review.
 
 
 Diffs
 -
 
   docs/deploying-aurora-scheduler.md d1123359961fd59ddb8c1a07f80f293bdd46019f 
   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
 bacfbfeb237ecddf82f58679e05be012c5214e61 
   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
 24b61c1e4f615295acf28d904588e1512972d3f4 
   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
 baacb71403d55c5b90fc11cb2a23f552a32e8ba5 
   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
 5340d651b298ec8aa079e73d6d2f652fdf876293 
   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
 6575b7d420f17ec68d6e2a83e7b380f684577d4f 
   
 src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
 444d6d3fdaf86eb84612f846eaa326eb75c49898 
   src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
 efe62ceb502ead88a2f0cd6d09a76664e465d9bc 
 
 Diff: https://reviews.apache.org/r/31338/diff/
 
 
 

Re: Review Request 31338: Added a command line flag, -global_container_mounts, to allow mounting paths from the slaves into the (docker) containers they run.

2015-02-23 Thread Aurora ReviewBot

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

Ship it!


Master (19378c1) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing @ReviewBot 
retry

- Aurora ReviewBot


On Feb. 24, 2015, 2:07 a.m., Steve Niemitz wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/31338/
 ---
 
 (Updated Feb. 24, 2015, 2:07 a.m.)
 
 
 Review request for Aurora, Jay Buffington and Bill Farner.
 
 
 Bugs: AURORA-1107
 https://issues.apache.org/jira/browse/AURORA-1107
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Added a command line flag, -global_container_mounts, to allow mounting paths 
 from the slaves into the (docker) containers they run.
 
 This is the first portion of allowing per-job mounts, however, I wanted to 
 get this out first since more people want it.  I'll implement per-job mounts 
 in a future review.
 
 
 Diffs
 -
 
   docs/deploying-aurora-scheduler.md d1123359961fd59ddb8c1a07f80f293bdd46019f 
   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
 bacfbfeb237ecddf82f58679e05be012c5214e61 
   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
 24b61c1e4f615295acf28d904588e1512972d3f4 
   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
 baacb71403d55c5b90fc11cb2a23f552a32e8ba5 
   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
 5340d651b298ec8aa079e73d6d2f652fdf876293 
   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
 6575b7d420f17ec68d6e2a83e7b380f684577d4f 
   
 src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
 444d6d3fdaf86eb84612f846eaa326eb75c49898 
   src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
 efe62ceb502ead88a2f0cd6d09a76664e465d9bc 
 
 Diff: https://reviews.apache.org/r/31338/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Steve Niemitz