Re: Review Request 49218: Add support for Mesos Fetcher

2016-06-28 Thread Aurora ReviewBot 

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


Ship it!




Master (102f5f0) 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 June 29, 2016, 4:36 a.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49218/
> ---
> 
> (Updated June 29, 2016, 4:36 a.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Adding a URIs field to TaskConfig inside the ThriftAPI so that users are able 
> to specify resources they wish to download into the sandbox per job.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md af2061c7605c12a066778bd99ec1a3857bee6dec 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 3e6daf53dd563dd7a2d494cc95e9a0aba0b6 
>   docs/features/mesos-fetcher.md PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 6c7c75ac86458884bc767736caf47fb56fc8 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 8103163f5584ed3b0946cd02ec596b5aa75f4475 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  fe18c0f9876a736ea34d4eab92219013cd1e 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> 3b01801d929dd61ee989495bf38af8f03e9f5ad4 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
> c76164292cf62d2181374c09f8bf6d8d3358e982 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java 
> 571201094c1e576e496495a01cb83f6c57decfa8 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/migration/V007_CreateMesosFetcherURIsTable.java
>  PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 
> a90cb00e240df25dce6d55728859768e22d741a6 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml
>  2c8af8b88e41b3b381cac831fd43b1057e4df0aa 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> 5069bedc08bb7111d0e0f101c8a2c81495b97bc9 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  2dff80b5213e98c778d71955517e5f9227d7d0c1 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> 58785bfa37ff214f26e9f94d836e6df40e411c3b 
>   
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
> b1593f682f48ea66339bc2372de3e4f14e40be32 
>   src/test/java/org/apache/aurora/scheduler/thrift/Fixtures.java 
> a883b0e33bfec1d14e6fe4ee8ed2200d93acaeec 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> ed69996593f5556d80a9229063ef5c7d658e2cfb 
> 
> Diff: https://reviews.apache.org/r/49218/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build -Pq
> bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> Tested with a custom client submitting TaskConfigs which included URIs when 
> the -enable_mesos_fetcher_for_jobs flag was on as well as when it was off.
> 
> 
> Thanks,
> 
> Renan DelValle
> 
>

Re: Review Request 49218: Add support for Mesos Fetcher

2016-06-28 Thread Renan DelValle 

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

(Updated June 28, 2016, 9:36 p.m.)


Review request for Aurora.


Changes
---

Added Mesos issue to TODO which tracks output_file attribute for Mesos URI 
fetcher.

Merged in master before submitting.


Repository: aurora


Description
---

Adding a URIs field to TaskConfig inside the ThriftAPI so that users are able 
to specify resources they wish to download into the sandbox per job.


Diffs (updated)
-

  RELEASE-NOTES.md af2061c7605c12a066778bd99ec1a3857bee6dec 
  api/src/main/thrift/org/apache/aurora/gen/api.thrift 
3e6daf53dd563dd7a2d494cc95e9a0aba0b6 
  docs/features/mesos-fetcher.md PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
6c7c75ac86458884bc767736caf47fb56fc8 
  src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
8103163f5584ed3b0946cd02ec596b5aa75f4475 
  
src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
 fe18c0f9876a736ea34d4eab92219013cd1e 
  src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
3b01801d929dd61ee989495bf38af8f03e9f5ad4 
  src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
c76164292cf62d2181374c09f8bf6d8d3358e982 
  src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java 
571201094c1e576e496495a01cb83f6c57decfa8 
  
src/main/java/org/apache/aurora/scheduler/storage/db/migration/V007_CreateMesosFetcherURIsTable.java
 PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 
a90cb00e240df25dce6d55728859768e22d741a6 
  
src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml 
2c8af8b88e41b3b381cac831fd43b1057e4df0aa 
  src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
5069bedc08bb7111d0e0f101c8a2c81495b97bc9 
  
src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
 2dff80b5213e98c778d71955517e5f9227d7d0c1 
  src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
58785bfa37ff214f26e9f94d836e6df40e411c3b 
  src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
b1593f682f48ea66339bc2372de3e4f14e40be32 
  src/test/java/org/apache/aurora/scheduler/thrift/Fixtures.java 
a883b0e33bfec1d14e6fe4ee8ed2200d93acaeec 
  src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
ed69996593f5556d80a9229063ef5c7d658e2cfb 

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


Testing
---

./gradlew build -Pq
bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh

Tested with a custom client submitting TaskConfigs which included URIs when the 
-enable_mesos_fetcher_for_jobs flag was on as well as when it was off.


Thanks,

Renan DelValle

Re: Review Request 49218: Add support for Mesos Fetcher

2016-06-28 Thread Renan DelValle 


> On June 28, 2016, 8:46 p.m., Joshua Cohen wrote:
> > src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java, line 
> > 267
> > 
> >
> > Is there a mesos ticket tracking this? If so, can you add it here?

Done, looks like this was added in Mesos 0.29. This also made me realize I 
forgot an L in my last name. That's what you get when 3/8ths of your last name 
are the same letter :|.


> On June 28, 2016, 8:46 p.m., Joshua Cohen wrote:
> > src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java,
> >  line 301
> > 
> >
> > It's hard to tell from looking at the review, do we have the inverse of 
> > this scenario covered as well (i.e. mesos fetcher is disabled and task 
> > config has no mesos fetcher uris set)? If not, would you mind adding 
> > coverage for that?

This case should be covered in testGpuResourcesNotAllowed. It has the Mesos 
Fetcher disabled and the task config has no fetcher URIs set. The other case 
that is also handled in other tests is when mesos_fetcher is enabled but no 
fetcher URIs have been provided as well.


- Renan


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


On June 28, 2016, 2:39 p.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49218/
> ---
> 
> (Updated June 28, 2016, 2:39 p.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Adding a URIs field to TaskConfig inside the ThriftAPI so that users are able 
> to specify resources they wish to download into the sandbox per job.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md af2061c7605c12a066778bd99ec1a3857bee6dec 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 3e6daf53dd563dd7a2d494cc95e9a0aba0b6 
>   docs/features/mesos-fetcher.md PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 6c7c75ac86458884bc767736caf47fb56fc8 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 4089b79da8079243703eead884e80bcf736f8b29 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  fe18c0f9876a736ea34d4eab92219013cd1e 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> 3b01801d929dd61ee989495bf38af8f03e9f5ad4 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
> c76164292cf62d2181374c09f8bf6d8d3358e982 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java 
> 571201094c1e576e496495a01cb83f6c57decfa8 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/migration/V007_CreateMesosFetcherURIsTable.java
>  PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 
> a90cb00e240df25dce6d55728859768e22d741a6 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml
>  2c8af8b88e41b3b381cac831fd43b1057e4df0aa 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> 5069bedc08bb7111d0e0f101c8a2c81495b97bc9 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  2dff80b5213e98c778d71955517e5f9227d7d0c1 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> 58785bfa37ff214f26e9f94d836e6df40e411c3b 
>   
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
> b1593f682f48ea66339bc2372de3e4f14e40be32 
>   src/test/java/org/apache/aurora/scheduler/thrift/Fixtures.java 
> a883b0e33bfec1d14e6fe4ee8ed2200d93acaeec 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> ed69996593f5556d80a9229063ef5c7d658e2cfb 
> 
> Diff: https://reviews.apache.org/r/49218/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build -Pq
> bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> Tested with a custom client submitting TaskConfigs which included URIs when 
> the -enable_mesos_fetcher_for_jobs flag was on as well as when it was off.
> 
> 
> Thanks,
> 
> Renan DelValle
> 
>

Re: Review Request 49218: Add support for Mesos Fetcher

2016-06-28 Thread Joshua Cohen 

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


Fix it, then Ship it!




lgtm modulo the below. Please fix or drop issues as warranted and then I can 
commit this.


src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java (line 267)


Is there a mesos ticket tracking this? If so, can you add it here?



src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java (lines 
270 - 271)


Duplicate calls to `setExecutable`?



src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
 (line 301)


It's hard to tell from looking at the review, do we have the inverse of 
this scenario covered as well (i.e. mesos fetcher is disabled and task config 
has no mesos fetcher uris set)? If not, would you mind adding coverage for that?


- Joshua Cohen


On June 28, 2016, 9:39 p.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49218/
> ---
> 
> (Updated June 28, 2016, 9:39 p.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Adding a URIs field to TaskConfig inside the ThriftAPI so that users are able 
> to specify resources they wish to download into the sandbox per job.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md af2061c7605c12a066778bd99ec1a3857bee6dec 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 3e6daf53dd563dd7a2d494cc95e9a0aba0b6 
>   docs/features/mesos-fetcher.md PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 6c7c75ac86458884bc767736caf47fb56fc8 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 4089b79da8079243703eead884e80bcf736f8b29 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  fe18c0f9876a736ea34d4eab92219013cd1e 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> 3b01801d929dd61ee989495bf38af8f03e9f5ad4 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
> c76164292cf62d2181374c09f8bf6d8d3358e982 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java 
> 571201094c1e576e496495a01cb83f6c57decfa8 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/migration/V007_CreateMesosFetcherURIsTable.java
>  PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 
> a90cb00e240df25dce6d55728859768e22d741a6 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml
>  2c8af8b88e41b3b381cac831fd43b1057e4df0aa 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> 5069bedc08bb7111d0e0f101c8a2c81495b97bc9 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  2dff80b5213e98c778d71955517e5f9227d7d0c1 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> 58785bfa37ff214f26e9f94d836e6df40e411c3b 
>   
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
> b1593f682f48ea66339bc2372de3e4f14e40be32 
>   src/test/java/org/apache/aurora/scheduler/thrift/Fixtures.java 
> a883b0e33bfec1d14e6fe4ee8ed2200d93acaeec 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> ed69996593f5556d80a9229063ef5c7d658e2cfb 
> 
> Diff: https://reviews.apache.org/r/49218/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build -Pq
> bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> Tested with a custom client submitting TaskConfigs which included URIs when 
> the -enable_mesos_fetcher_for_jobs flag was on as well as when it was off.
> 
> 
> Thanks,
> 
> Renan DelValle
> 
>

Re: Review Request 49218: Add support for Mesos Fetcher

2016-06-28 Thread Maxim Khutornenko 

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


Ship it!




Anyone else wants to review this change? If not I will commit it tomorrow.

- Maxim Khutornenko


On June 28, 2016, 9:39 p.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49218/
> ---
> 
> (Updated June 28, 2016, 9:39 p.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Adding a URIs field to TaskConfig inside the ThriftAPI so that users are able 
> to specify resources they wish to download into the sandbox per job.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md af2061c7605c12a066778bd99ec1a3857bee6dec 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 3e6daf53dd563dd7a2d494cc95e9a0aba0b6 
>   docs/features/mesos-fetcher.md PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 6c7c75ac86458884bc767736caf47fb56fc8 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 4089b79da8079243703eead884e80bcf736f8b29 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  fe18c0f9876a736ea34d4eab92219013cd1e 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> 3b01801d929dd61ee989495bf38af8f03e9f5ad4 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
> c76164292cf62d2181374c09f8bf6d8d3358e982 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java 
> 571201094c1e576e496495a01cb83f6c57decfa8 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/migration/V007_CreateMesosFetcherURIsTable.java
>  PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 
> a90cb00e240df25dce6d55728859768e22d741a6 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml
>  2c8af8b88e41b3b381cac831fd43b1057e4df0aa 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> 5069bedc08bb7111d0e0f101c8a2c81495b97bc9 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  2dff80b5213e98c778d71955517e5f9227d7d0c1 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> 58785bfa37ff214f26e9f94d836e6df40e411c3b 
>   
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
> b1593f682f48ea66339bc2372de3e4f14e40be32 
>   src/test/java/org/apache/aurora/scheduler/thrift/Fixtures.java 
> a883b0e33bfec1d14e6fe4ee8ed2200d93acaeec 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> ed69996593f5556d80a9229063ef5c7d658e2cfb 
> 
> Diff: https://reviews.apache.org/r/49218/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build -Pq
> bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> Tested with a custom client submitting TaskConfigs which included URIs when 
> the -enable_mesos_fetcher_for_jobs flag was on as well as when it was off.
> 
> 
> Thanks,
> 
> Renan DelValle
> 
>

Re: Review Request 49218: Add support for Mesos Fetcher

2016-06-28 Thread Aurora ReviewBot 

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


Ship it!




Master (499b11b) 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 June 28, 2016, 9:39 p.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49218/
> ---
> 
> (Updated June 28, 2016, 9:39 p.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Adding a URIs field to TaskConfig inside the ThriftAPI so that users are able 
> to specify resources they wish to download into the sandbox per job.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md af2061c7605c12a066778bd99ec1a3857bee6dec 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 3e6daf53dd563dd7a2d494cc95e9a0aba0b6 
>   docs/features/mesos-fetcher.md PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 6c7c75ac86458884bc767736caf47fb56fc8 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 4089b79da8079243703eead884e80bcf736f8b29 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  fe18c0f9876a736ea34d4eab92219013cd1e 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> 3b01801d929dd61ee989495bf38af8f03e9f5ad4 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
> c76164292cf62d2181374c09f8bf6d8d3358e982 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java 
> 571201094c1e576e496495a01cb83f6c57decfa8 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/migration/V007_CreateMesosFetcherURIsTable.java
>  PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 
> a90cb00e240df25dce6d55728859768e22d741a6 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml
>  2c8af8b88e41b3b381cac831fd43b1057e4df0aa 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> 5069bedc08bb7111d0e0f101c8a2c81495b97bc9 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  2dff80b5213e98c778d71955517e5f9227d7d0c1 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> 58785bfa37ff214f26e9f94d836e6df40e411c3b 
>   
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
> b1593f682f48ea66339bc2372de3e4f14e40be32 
>   src/test/java/org/apache/aurora/scheduler/thrift/Fixtures.java 
> a883b0e33bfec1d14e6fe4ee8ed2200d93acaeec 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> ed69996593f5556d80a9229063ef5c7d658e2cfb 
> 
> Diff: https://reviews.apache.org/r/49218/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build -Pq
> bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> Tested with a custom client submitting TaskConfigs which included URIs when 
> the -enable_mesos_fetcher_for_jobs flag was on as well as when it was off.
> 
> 
> Thanks,
> 
> Renan DelValle
> 
>

Re: Review Request 49218: Add support for Mesos Fetcher

2016-06-28 Thread Renan DelValle 


> On June 28, 2016, 9:52 a.m., Maxim Khutornenko wrote:
> > LGTM, just a few minor changes left.

Thanks for the quick reviews, really appreciate it.


> On June 28, 2016, 9:52 a.m., Maxim Khutornenko wrote:
> > api/src/main/thrift/org/apache/aurora/gen/api.thrift, lines 148-152
> > 
> >
> > Holes in thrift IDs are legal but since this is a new struct it'd make 
> > sense to avoid them.

Agreed, changed the values so there is no hole.


> On June 28, 2016, 9:52 a.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/app/AppModule.java, line 100
> > 
> >
> > I'd skip `_for_jobs` part in favor of a brief write up under 
> > https://github.com/apache/aurora/tree/master/docs/features describing 
> > custom executors and mesos fetcher.

Added a mesos fetcher document. Custom executors are already documented for the 
most part in docs/opreations/configuration.md. I made a reference to how the 
URIs are different but similar in the Mesos Fetcher document.


- Renan


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


On June 28, 2016, 2:39 p.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49218/
> ---
> 
> (Updated June 28, 2016, 2:39 p.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Adding a URIs field to TaskConfig inside the ThriftAPI so that users are able 
> to specify resources they wish to download into the sandbox per job.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md af2061c7605c12a066778bd99ec1a3857bee6dec 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 3e6daf53dd563dd7a2d494cc95e9a0aba0b6 
>   docs/features/mesos-fetcher.md PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 6c7c75ac86458884bc767736caf47fb56fc8 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 4089b79da8079243703eead884e80bcf736f8b29 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  fe18c0f9876a736ea34d4eab92219013cd1e 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> 3b01801d929dd61ee989495bf38af8f03e9f5ad4 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
> c76164292cf62d2181374c09f8bf6d8d3358e982 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java 
> 571201094c1e576e496495a01cb83f6c57decfa8 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/migration/V007_CreateMesosFetcherURIsTable.java
>  PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 
> a90cb00e240df25dce6d55728859768e22d741a6 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml
>  2c8af8b88e41b3b381cac831fd43b1057e4df0aa 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> 5069bedc08bb7111d0e0f101c8a2c81495b97bc9 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  2dff80b5213e98c778d71955517e5f9227d7d0c1 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> 58785bfa37ff214f26e9f94d836e6df40e411c3b 
>   
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
> b1593f682f48ea66339bc2372de3e4f14e40be32 
>   src/test/java/org/apache/aurora/scheduler/thrift/Fixtures.java 
> a883b0e33bfec1d14e6fe4ee8ed2200d93acaeec 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> ed69996593f5556d80a9229063ef5c7d658e2cfb 
> 
> Diff: https://reviews.apache.org/r/49218/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build -Pq
> bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> Tested with a custom client submitting TaskConfigs which included URIs when 
> the -enable_mesos_fetcher_for_jobs flag was on as well as when it was off.
> 
> 
> Thanks,
> 
> Renan DelValle
> 
>

Re: Review Request 49218: Add support for Mesos Fetcher

2016-06-28 Thread Renan DelValle 

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

(Updated June 28, 2016, 2:39 p.m.)


Review request for Aurora.


Changes
---

Removed hole in Thrift API.
Changed scheduler flag from -enable_mesos_fetcher_for_jobs to 
-enable_mesos_fetcher.
Added brief write up in features concerning Mesos Fetcher (custom executors 
feature is documented in docs/opreations/configuration.md).
Formatting fixes.


Repository: aurora


Description
---

Adding a URIs field to TaskConfig inside the ThriftAPI so that users are able 
to specify resources they wish to download into the sandbox per job.


Diffs (updated)
-

  RELEASE-NOTES.md af2061c7605c12a066778bd99ec1a3857bee6dec 
  api/src/main/thrift/org/apache/aurora/gen/api.thrift 
3e6daf53dd563dd7a2d494cc95e9a0aba0b6 
  docs/features/mesos-fetcher.md PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
6c7c75ac86458884bc767736caf47fb56fc8 
  src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
4089b79da8079243703eead884e80bcf736f8b29 
  
src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
 fe18c0f9876a736ea34d4eab92219013cd1e 
  src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
3b01801d929dd61ee989495bf38af8f03e9f5ad4 
  src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
c76164292cf62d2181374c09f8bf6d8d3358e982 
  src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java 
571201094c1e576e496495a01cb83f6c57decfa8 
  
src/main/java/org/apache/aurora/scheduler/storage/db/migration/V007_CreateMesosFetcherURIsTable.java
 PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 
a90cb00e240df25dce6d55728859768e22d741a6 
  
src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml 
2c8af8b88e41b3b381cac831fd43b1057e4df0aa 
  src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
5069bedc08bb7111d0e0f101c8a2c81495b97bc9 
  
src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
 2dff80b5213e98c778d71955517e5f9227d7d0c1 
  src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
58785bfa37ff214f26e9f94d836e6df40e411c3b 
  src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
b1593f682f48ea66339bc2372de3e4f14e40be32 
  src/test/java/org/apache/aurora/scheduler/thrift/Fixtures.java 
a883b0e33bfec1d14e6fe4ee8ed2200d93acaeec 
  src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
ed69996593f5556d80a9229063ef5c7d658e2cfb 

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


Testing
---

./gradlew build -Pq
bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh

Tested with a custom client submitting TaskConfigs which included URIs when the 
-enable_mesos_fetcher_for_jobs flag was on as well as when it was off.


Thanks,

Renan DelValle

Re: Review Request 49218: Add support for Mesos Fetcher

2016-06-28 Thread Maxim Khutornenko 

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



LGTM, just a few minor changes left.


api/src/main/thrift/org/apache/aurora/gen/api.thrift (lines 148 - 152)


Holes in thrift IDs are legal but since this is a new struct it'd make 
sense to avoid them.



src/main/java/org/apache/aurora/scheduler/app/AppModule.java (line 100)


I'd skip `_for_jobs` part in favor of a brief write up under 
https://github.com/apache/aurora/tree/master/docs/features describing custom 
executors and mesos fetcher.



src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
 (line 225)


indent is off here



src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java (lines 
268 - 272)


Suggest a small reformatting here:
```
List mesosFetcherUris = 
task.getTask().getMesosFetcherUris().stream()
.map(u -> Protos.CommandInfo.URI.newBuilder().setValue(u.getValue())
.setExecutable(false)
.setExecutable(false)
.setExtract(u.isExtract())
.setCache(u.isCache()).build())
.collect(Collectors.toList());
```


- Maxim Khutornenko


On June 28, 2016, 1:18 a.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49218/
> ---
> 
> (Updated June 28, 2016, 1:18 a.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Adding a URIs field to TaskConfig inside the ThriftAPI so that users are able 
> to specify resources they wish to download into the sandbox per job.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md af2061c7605c12a066778bd99ec1a3857bee6dec 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 3e6daf53dd563dd7a2d494cc95e9a0aba0b6 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 6c7c75ac86458884bc767736caf47fb56fc8 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 4089b79da8079243703eead884e80bcf736f8b29 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  fe18c0f9876a736ea34d4eab92219013cd1e 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> 3b01801d929dd61ee989495bf38af8f03e9f5ad4 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
> c76164292cf62d2181374c09f8bf6d8d3358e982 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java 
> 571201094c1e576e496495a01cb83f6c57decfa8 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/migration/V007_CreateMesosFetcherURIsTable.java
>  PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 
> a90cb00e240df25dce6d55728859768e22d741a6 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml
>  2c8af8b88e41b3b381cac831fd43b1057e4df0aa 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> 5069bedc08bb7111d0e0f101c8a2c81495b97bc9 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  2dff80b5213e98c778d71955517e5f9227d7d0c1 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> 58785bfa37ff214f26e9f94d836e6df40e411c3b 
>   
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
> b1593f682f48ea66339bc2372de3e4f14e40be32 
>   src/test/java/org/apache/aurora/scheduler/thrift/Fixtures.java 
> a883b0e33bfec1d14e6fe4ee8ed2200d93acaeec 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> ed69996593f5556d80a9229063ef5c7d658e2cfb 
> 
> Diff: https://reviews.apache.org/r/49218/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build -Pq
> bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> Tested with a custom client submitting TaskConfigs which included URIs when 
> the -enable_mesos_fetcher_for_jobs flag was on as well as when it was off.
> 
> 
> Thanks,
> 
> Renan DelValle
> 
>

Re: Review Request 49218: Add support for Mesos Fetcher

2016-06-27 Thread Aurora ReviewBot 

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



Master (73dd2a8) is red with this patch.
  ./build-support/jenkins/build.sh

   with temporary_dir() as checkpoint_root:
 te = AuroraExecutor(
 >   
runner_provider=make_provider(checkpoint_root),
 
sandbox_provider=DefaultTestSandboxProvider())
 
 
src/test/python/apache/aurora/executor/test_thermos_executor.py:580: 
 _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
 
src/test/python/apache/aurora/executor/test_thermos_executor.py:193: in 
make_provider
 pex_location=thermos_runner_path(),
 _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
 
 build = True
 
 def thermos_runner_path(build=True):
   if not build:
 return getattr(thermos_runner_path, 'value', 
None)
 
   if not hasattr(thermos_runner_path, 'value'):
 pex_dir = safe_mkdtemp()
 >   assert subprocess.call(["./pants", 
"--pants-distdir=%s" % pex_dir, "binary",
   
"src/main/python/apache/thermos/runner:thermos_runner"]) == 0
 E   assert 1 == 0
 E+  where 1 = (['./pants', '--pants-distdir=/tmp/tmpy92fEM', 'binary', 
'src/main/python/apache/thermos/runner:thermos_runner'])
 E+where  = subprocess.call
 
 
src/test/python/apache/aurora/executor/test_thermos_executor.py:185: 
AssertionError
 -- Captured stderr call --
 Traceback (most recent call last):
   File 
"/home/jenkins/.cache/pants/setup/bootstrap-Linux-x86_64/0.0.80/bin/pants", 
line 7, in 
 from pants.bin.pants_exe import main
 ImportError: No module named pants.bin.pants_exe
  generated xml file: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/test-results/415337499eb72578eab327a6487c1f5c9452b3d6.xml
 
  16 failed, 638 passed, 6 skipped, 1 warnings, 8 
error in 108.83 seconds 
 
FAILURE


   Waiting for background workers to finish.
01:29:15 02:31   [complete]
   FAILURE


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

- Aurora ReviewBot


On June 28, 2016, 1:18 a.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49218/
> ---
> 
> (Updated June 28, 2016, 1:18 a.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Adding a URIs field to TaskConfig inside the ThriftAPI so that users are able 
> to specify resources they wish to download into the sandbox per job.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md af2061c7605c12a066778bd99ec1a3857bee6dec 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 3e6daf53dd563dd7a2d494cc95e9a0aba0b6 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 6c7c75ac86458884bc767736caf47fb56fc8 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 4089b79da8079243703eead884e80bcf736f8b29 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  fe18c0f9876a736ea34d4eab92219013cd1e 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> 3b01801d929dd61ee989495bf38af8f03e9f5ad4 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
> c76164292cf62d2181374c09f8bf6d8d3358e982 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java 
> 571201094c1e576e496495a01cb83f6c57decfa8 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/migration/V007_CreateMesosFetcherURIsTable.java
>  PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 
> a90cb00e240df25dce6d55728859768e22d741a6 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml
>  2c8af8b88e41b3b381cac831fd43b1057e4df0aa 
>   src/main/resources/org/apache/aurora/scheduler/stor

Re: Review Request 49218: Add support for Mesos Fetcher

2016-06-27 Thread Renan DelValle 

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

(Updated June 27, 2016, 6:18 p.m.)


Review request for Aurora.


Changes
---

Changed URI in api.thrift to MesosFetcherURI


Repository: aurora


Description
---

Adding a URIs field to TaskConfig inside the ThriftAPI so that users are able 
to specify resources they wish to download into the sandbox per job.


Diffs (updated)
-

  RELEASE-NOTES.md af2061c7605c12a066778bd99ec1a3857bee6dec 
  api/src/main/thrift/org/apache/aurora/gen/api.thrift 
3e6daf53dd563dd7a2d494cc95e9a0aba0b6 
  src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
6c7c75ac86458884bc767736caf47fb56fc8 
  src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
4089b79da8079243703eead884e80bcf736f8b29 
  
src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
 fe18c0f9876a736ea34d4eab92219013cd1e 
  src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
3b01801d929dd61ee989495bf38af8f03e9f5ad4 
  src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
c76164292cf62d2181374c09f8bf6d8d3358e982 
  src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java 
571201094c1e576e496495a01cb83f6c57decfa8 
  
src/main/java/org/apache/aurora/scheduler/storage/db/migration/V007_CreateMesosFetcherURIsTable.java
 PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 
a90cb00e240df25dce6d55728859768e22d741a6 
  
src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml 
2c8af8b88e41b3b381cac831fd43b1057e4df0aa 
  src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
5069bedc08bb7111d0e0f101c8a2c81495b97bc9 
  
src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
 2dff80b5213e98c778d71955517e5f9227d7d0c1 
  src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
58785bfa37ff214f26e9f94d836e6df40e411c3b 
  src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
b1593f682f48ea66339bc2372de3e4f14e40be32 
  src/test/java/org/apache/aurora/scheduler/thrift/Fixtures.java 
a883b0e33bfec1d14e6fe4ee8ed2200d93acaeec 
  src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
ed69996593f5556d80a9229063ef5c7d658e2cfb 

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


Testing (updated)
---

./gradlew build -Pq
bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh

Tested with a custom client submitting TaskConfigs which included URIs when the 
-enable_mesos_fetcher_for_jobs flag was on as well as when it was off.


Thanks,

Renan DelValle

Re: Review Request 49218: Add support for Mesos Fetcher

2016-06-27 Thread Aurora ReviewBot 

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


Ship it!




Master (73dd2a8) 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 June 27, 2016, 11:56 p.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49218/
> ---
> 
> (Updated June 27, 2016, 11:56 p.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Adding a URIs field to TaskConfig inside the ThriftAPI so that users are able 
> to specify resources they wish to download into the sandbox per job.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md af2061c7605c12a066778bd99ec1a3857bee6dec 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 3e6daf53dd563dd7a2d494cc95e9a0aba0b6 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 6c7c75ac86458884bc767736caf47fb56fc8 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 4089b79da8079243703eead884e80bcf736f8b29 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  fe18c0f9876a736ea34d4eab92219013cd1e 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> 3b01801d929dd61ee989495bf38af8f03e9f5ad4 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
> c76164292cf62d2181374c09f8bf6d8d3358e982 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java 
> 571201094c1e576e496495a01cb83f6c57decfa8 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/migration/V007_CreateMesosFetcherURIsTable.java
>  PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 
> a90cb00e240df25dce6d55728859768e22d741a6 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml
>  2c8af8b88e41b3b381cac831fd43b1057e4df0aa 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> 5069bedc08bb7111d0e0f101c8a2c81495b97bc9 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  2dff80b5213e98c778d71955517e5f9227d7d0c1 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> 58785bfa37ff214f26e9f94d836e6df40e411c3b 
>   
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
> b1593f682f48ea66339bc2372de3e4f14e40be32 
>   src/test/java/org/apache/aurora/scheduler/thrift/Fixtures.java 
> a883b0e33bfec1d14e6fe4ee8ed2200d93acaeec 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> ed69996593f5556d80a9229063ef5c7d658e2cfb 
> 
> Diff: https://reviews.apache.org/r/49218/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build -Pq
> ./build-support/jenkins/build.sh
> bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Renan DelValle
> 
>

Re: Review Request 49218: Add support for Mesos Fetcher

2016-06-27 Thread Renan DelValle 


> On June 24, 2016, 4:33 p.m., David McLaughlin wrote:
> > api/src/main/thrift/org/apache/aurora/gen/api.thrift, lines 266-267
> > 
> >
> > Please rename to something like mesosFetcherUris (or just fetcherUris) 
> > to document purpose of these uris in this context.
> 
> Renan DelValle wrote:
> Sounds like a good idea, I'll get this done after all the technical stuff 
> has been cleared up.

Renamed to mesosFetcherUris.


- Renan


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


On June 27, 2016, 4:56 p.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49218/
> ---
> 
> (Updated June 27, 2016, 4:56 p.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Adding a URIs field to TaskConfig inside the ThriftAPI so that users are able 
> to specify resources they wish to download into the sandbox per job.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md af2061c7605c12a066778bd99ec1a3857bee6dec 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 3e6daf53dd563dd7a2d494cc95e9a0aba0b6 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 6c7c75ac86458884bc767736caf47fb56fc8 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 4089b79da8079243703eead884e80bcf736f8b29 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  fe18c0f9876a736ea34d4eab92219013cd1e 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> 3b01801d929dd61ee989495bf38af8f03e9f5ad4 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
> c76164292cf62d2181374c09f8bf6d8d3358e982 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java 
> 571201094c1e576e496495a01cb83f6c57decfa8 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/migration/V007_CreateMesosFetcherURIsTable.java
>  PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 
> a90cb00e240df25dce6d55728859768e22d741a6 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml
>  2c8af8b88e41b3b381cac831fd43b1057e4df0aa 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> 5069bedc08bb7111d0e0f101c8a2c81495b97bc9 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  2dff80b5213e98c778d71955517e5f9227d7d0c1 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> 58785bfa37ff214f26e9f94d836e6df40e411c3b 
>   
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
> b1593f682f48ea66339bc2372de3e4f14e40be32 
>   src/test/java/org/apache/aurora/scheduler/thrift/Fixtures.java 
> a883b0e33bfec1d14e6fe4ee8ed2200d93acaeec 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> ed69996593f5556d80a9229063ef5c7d658e2cfb 
> 
> Diff: https://reviews.apache.org/r/49218/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build -Pq
> ./build-support/jenkins/build.sh
> bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Renan DelValle
> 
>

Re: Review Request 49218: Add support for Mesos Fetcher

2016-06-27 Thread Renan DelValle 


> On June 27, 2016, 9:44 a.m., Maxim Khutornenko wrote:
> > api/src/main/thrift/org/apache/aurora/gen/api.thrift, line 145
> > 
> >
> > typo

Fixed thanks!


> On June 27, 2016, 9:44 a.m., Maxim Khutornenko wrote:
> > api/src/main/thrift/org/apache/aurora/gen/api.thrift, line 146
> > 
> >
> > The name appears to be too generic for its highly specialized use-case. 
> > Perhaps something along the lines of the "MesosFetcherUri" should be a 
> > better fit here?

Since it mirrors the URI in the proto maybe MesosURI would do? I'll change it 
in the next submission if this is fine.


> On June 27, 2016, 9:44 a.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java, line 
> > 260
> > 
> >
> > Please revert, there should be a newline after the args spillover.

Done.


> On June 27, 2016, 9:44 a.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java, line 
> > 266
> > 
> >
> > Favor java8 stream manipulations over Guava's where possible.
> > 
> > Also, formatting is off, should be:
> > ```
> > Iterable uris = Iterables.transform(
> > task.getTask().getUris(),
> > u -> Protos.CommandInfo.URI.newBuilder()
> > .setValue(u.getValue())
> > .setExecutable(false)
> > .setExtract(u.isExtract())
> > .setCache(u.isCache()).build());
> > ```

Changed to Java 8 streams.


> On June 27, 2016, 9:44 a.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java, line 
> > 268
> > 
> >
> > This seems like a candidate for a command line flag documenting the 
> > direct security implications of changing the value here.
> > 
> > Also, along the lines of Joshua's comments, suggest having another 
> > command line flag turning the feature off entirely (default) with a check 
> > in ConfigurationManager rejecting any tasks with fetcher URIs set.

One flag to allow the executable field to be toggled and another to allow URIs 
to be fetched right? I've been playing it safe and not allowing
the user a choice for now to set anything as executable as anything that needs 
to be downloaded to the sandbox and marked executable can be set to be fetched 
in the static server side config (custom executor config). I can add the 
ability to toggle being able to mark URIs downlaoded exectuable otherwise 
though.

I've added the flag to turn this feature off entirely and have set it to be the 
default.


> On June 27, 2016, 9:44 a.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java, 
> > lines 155-157
> > 
> >
> > Fits on a single line.

Changed, thanks!


> On June 27, 2016, 9:44 a.m., Maxim Khutornenko wrote:
> > src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml,
> >  line 406
> > 
> >
> > Please, add/modify AbstractTaskStoreTest to test this codepath.

Added, will iterate if it's not good enough.


- Renan


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


On June 27, 2016, 4:56 p.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49218/
> ---
> 
> (Updated June 27, 2016, 4:56 p.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Adding a URIs field to TaskConfig inside the ThriftAPI so that users are able 
> to specify resources they wish to download into the sandbox per job.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md af2061c7605c12a066778bd99ec1a3857bee6dec 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 3e6daf53dd563dd7a2d494cc95e9a0aba0b6 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 6c7c75ac86458884bc767736caf47fb56fc8 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 4089b79da8079243703eead884e80bcf736f8b29 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  fe18c0f9876a736ea34d4eab92219013cd1e 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskF

Re: Review Request 49218: Add support for Mesos Fetcher

2016-06-27 Thread Renan DelValle 

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

(Updated June 27, 2016, 4:56 p.m.)


Review request for Aurora.


Changes
---

Added flag -enable_mesos_fetcher_for_jobs to control wether arbitrary URIs 
provided by the client are allowed or not.
Renamed uris to mesosFetcherUris. 
Added tests for mesosFetcherUris in AbstractTaskStoreTest. 
Changed from Guava to to Java 8 streams.


Repository: aurora


Description
---

Adding a URIs field to TaskConfig inside the ThriftAPI so that users are able 
to specify resources they wish to download into the sandbox per job.


Diffs (updated)
-

  RELEASE-NOTES.md af2061c7605c12a066778bd99ec1a3857bee6dec 
  api/src/main/thrift/org/apache/aurora/gen/api.thrift 
3e6daf53dd563dd7a2d494cc95e9a0aba0b6 
  src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
6c7c75ac86458884bc767736caf47fb56fc8 
  src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
4089b79da8079243703eead884e80bcf736f8b29 
  
src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
 fe18c0f9876a736ea34d4eab92219013cd1e 
  src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
3b01801d929dd61ee989495bf38af8f03e9f5ad4 
  src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
c76164292cf62d2181374c09f8bf6d8d3358e982 
  src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java 
571201094c1e576e496495a01cb83f6c57decfa8 
  
src/main/java/org/apache/aurora/scheduler/storage/db/migration/V007_CreateMesosFetcherURIsTable.java
 PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 
a90cb00e240df25dce6d55728859768e22d741a6 
  
src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml 
2c8af8b88e41b3b381cac831fd43b1057e4df0aa 
  src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
5069bedc08bb7111d0e0f101c8a2c81495b97bc9 
  
src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
 2dff80b5213e98c778d71955517e5f9227d7d0c1 
  src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
58785bfa37ff214f26e9f94d836e6df40e411c3b 
  src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
b1593f682f48ea66339bc2372de3e4f14e40be32 
  src/test/java/org/apache/aurora/scheduler/thrift/Fixtures.java 
a883b0e33bfec1d14e6fe4ee8ed2200d93acaeec 
  src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
ed69996593f5556d80a9229063ef5c7d658e2cfb 

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


Testing
---

./gradlew build -Pq
./build-support/jenkins/build.sh
bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh


Thanks,

Renan DelValle

Re: Review Request 49218: Add support for Mesos Fetcher

2016-06-27 Thread Maxim Khutornenko 

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




api/src/main/thrift/org/apache/aurora/gen/api.thrift (line 145)


typo



api/src/main/thrift/org/apache/aurora/gen/api.thrift (line 146)


The name appears to be too generic for its highly specialized use-case. 
Perhaps something along the lines of the "MesosFetcherUri" should be a better 
fit here?



src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 


Please revert, there should be a newline after the args spillover.



src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java (line 265)


Favor java8 stream manipulations over Guava's where possible.

Also, formatting is off, should be:
```
Iterable uris = Iterables.transform(
task.getTask().getUris(),
u -> Protos.CommandInfo.URI.newBuilder()
.setValue(u.getValue())
.setExecutable(false)
.setExtract(u.isExtract())
.setCache(u.isCache()).build());
```



src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java (line 267)


This seems like a candidate for a command line flag documenting the direct 
security implications of changing the value here.

Also, along the lines of Joshua's comments, suggest having another command 
line flag turning the feature off entirely (default) with a check in 
ConfigurationManager rejecting any tasks with fetcher URIs set.



src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java 
(lines 155 - 157)


Fits on a single line.



src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml 
(line 406)


Please, add/modify AbstractTaskStoreTest to test this codepath.


- Maxim Khutornenko


On June 24, 2016, 11:01 p.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49218/
> ---
> 
> (Updated June 24, 2016, 11:01 p.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Adding a URIs field to TaskConfig inside the ThriftAPI so that users are able 
> to specify resources they wish to download into the sandbox per job.
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 3e6daf53dd563dd7a2d494cc95e9a0aba0b6 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 4089b79da8079243703eead884e80bcf736f8b29 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> 3b01801d929dd61ee989495bf38af8f03e9f5ad4 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
> c76164292cf62d2181374c09f8bf6d8d3358e982 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java 
> 571201094c1e576e496495a01cb83f6c57decfa8 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/migration/V007_CreateURIsTable.java
>  PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 
> a90cb00e240df25dce6d55728859768e22d741a6 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml
>  2c8af8b88e41b3b381cac831fd43b1057e4df0aa 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> 5069bedc08bb7111d0e0f101c8a2c81495b97bc9 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> 58785bfa37ff214f26e9f94d836e6df40e411c3b 
>   src/test/java/org/apache/aurora/scheduler/thrift/Fixtures.java 
> a883b0e33bfec1d14e6fe4ee8ed2200d93acaeec 
> 
> Diff: https://reviews.apache.org/r/49218/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build -Pq
> ./build-support/jenkins/build.sh
> bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Renan DelValle
> 
>

Re: Review Request 49218: Add support for Mesos Fetcher

2016-06-24 Thread Renan DelValle 


> On June 24, 2016, 4:33 p.m., David McLaughlin wrote:
> > api/src/main/thrift/org/apache/aurora/gen/api.thrift, lines 266-267
> > 
> >
> > Please rename to something like mesosFetcherUris (or just fetcherUris) 
> > to document purpose of these uris in this context.

Sounds like a good idea, I'll get this done after all the technical stuff has 
been cleared up.


- Renan


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


On June 24, 2016, 4:01 p.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49218/
> ---
> 
> (Updated June 24, 2016, 4:01 p.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Adding a URIs field to TaskConfig inside the ThriftAPI so that users are able 
> to specify resources they wish to download into the sandbox per job.
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 3e6daf53dd563dd7a2d494cc95e9a0aba0b6 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 4089b79da8079243703eead884e80bcf736f8b29 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> 3b01801d929dd61ee989495bf38af8f03e9f5ad4 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
> c76164292cf62d2181374c09f8bf6d8d3358e982 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java 
> 571201094c1e576e496495a01cb83f6c57decfa8 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/migration/V007_CreateURIsTable.java
>  PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 
> a90cb00e240df25dce6d55728859768e22d741a6 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml
>  2c8af8b88e41b3b381cac831fd43b1057e4df0aa 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> 5069bedc08bb7111d0e0f101c8a2c81495b97bc9 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> 58785bfa37ff214f26e9f94d836e6df40e411c3b 
>   src/test/java/org/apache/aurora/scheduler/thrift/Fixtures.java 
> a883b0e33bfec1d14e6fe4ee8ed2200d93acaeec 
> 
> Diff: https://reviews.apache.org/r/49218/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build -Pq
> ./build-support/jenkins/build.sh
> bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Renan DelValle
> 
>

Re: Review Request 49218: Add support for Mesos Fetcher

2016-06-24 Thread Renan DelValle 


> On June 24, 2016, 5:57 p.m., Joshua Cohen wrote:
> > I'm not super familiar with the Mesos resource fetcher, but I'm assuming 
> > Mesos does not apply any access control on the uris grabbed by the fetcher 
> > (based on the fact that we already use this to grab the thermos executor 
> > from whatever path is configured via the scheduler command line)?
> > 
> > Am I missing something, or is this potentially a privilege escalation to 
> > just blindly allow user tasks to grab arbitrary URIs into their sandbox? Is 
> > there any way to control this? I think at the very least we should wire 
> > this functionality off by default via a command line flag, rejecting any 
> > tasks that request uris in this fashion if it's not explicitly enabled.

Mesos does not apply any access control and it could lead a security issue. As 
far as I know, the only way of controlling this is by filtering the URIs on the 
serverside (i.e. whitelist/blacklist). Turning this off by default and having 
it be a configuration is a great option while a more refined solution is 
created. For now, the other way I'm combatting this is by never allowing the 
URI grabbed to be set as an executable.


- Renan


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


On June 24, 2016, 4:01 p.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49218/
> ---
> 
> (Updated June 24, 2016, 4:01 p.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Adding a URIs field to TaskConfig inside the ThriftAPI so that users are able 
> to specify resources they wish to download into the sandbox per job.
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 3e6daf53dd563dd7a2d494cc95e9a0aba0b6 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 4089b79da8079243703eead884e80bcf736f8b29 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> 3b01801d929dd61ee989495bf38af8f03e9f5ad4 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
> c76164292cf62d2181374c09f8bf6d8d3358e982 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java 
> 571201094c1e576e496495a01cb83f6c57decfa8 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/migration/V007_CreateURIsTable.java
>  PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 
> a90cb00e240df25dce6d55728859768e22d741a6 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml
>  2c8af8b88e41b3b381cac831fd43b1057e4df0aa 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> 5069bedc08bb7111d0e0f101c8a2c81495b97bc9 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> 58785bfa37ff214f26e9f94d836e6df40e411c3b 
>   src/test/java/org/apache/aurora/scheduler/thrift/Fixtures.java 
> a883b0e33bfec1d14e6fe4ee8ed2200d93acaeec 
> 
> Diff: https://reviews.apache.org/r/49218/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build -Pq
> ./build-support/jenkins/build.sh
> bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Renan DelValle
> 
>

Re: Review Request 49218: Add support for Mesos Fetcher

2016-06-24 Thread Joshua Cohen 

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



I'm not super familiar with the Mesos resource fetcher, but I'm assuming Mesos 
does not apply any access control on the uris grabbed by the fetcher (based on 
the fact that we already use this to grab the thermos executor from whatever 
path is configured via the scheduler command line)?

Am I missing something, or is this potentially a privilege escalation to just 
blindly allow user tasks to grab arbitrary URIs into their sandbox? Is there 
any way to control this? I think at the very least we should wire this 
functionality off by default via a command line flag, rejecting any tasks that 
request uris in this fashion if it's not explicitly enabled.


src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java 
(line 150)


Fix this copy/paste.



src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml 
(line 196)


This should be `u_id` not `m_id`.



src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml 
(lines 408 - 411)


Indent 2


- Joshua Cohen


On June 24, 2016, 11:01 p.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49218/
> ---
> 
> (Updated June 24, 2016, 11:01 p.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Adding a URIs field to TaskConfig inside the ThriftAPI so that users are able 
> to specify resources they wish to download into the sandbox per job.
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 3e6daf53dd563dd7a2d494cc95e9a0aba0b6 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 4089b79da8079243703eead884e80bcf736f8b29 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> 3b01801d929dd61ee989495bf38af8f03e9f5ad4 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
> c76164292cf62d2181374c09f8bf6d8d3358e982 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java 
> 571201094c1e576e496495a01cb83f6c57decfa8 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/migration/V007_CreateURIsTable.java
>  PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 
> a90cb00e240df25dce6d55728859768e22d741a6 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml
>  2c8af8b88e41b3b381cac831fd43b1057e4df0aa 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> 5069bedc08bb7111d0e0f101c8a2c81495b97bc9 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> 58785bfa37ff214f26e9f94d836e6df40e411c3b 
>   src/test/java/org/apache/aurora/scheduler/thrift/Fixtures.java 
> a883b0e33bfec1d14e6fe4ee8ed2200d93acaeec 
> 
> Diff: https://reviews.apache.org/r/49218/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build -Pq
> ./build-support/jenkins/build.sh
> bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Renan DelValle
> 
>

Re: Review Request 49218: Add support for Mesos Fetcher

2016-06-24 Thread David McLaughlin 

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




api/src/main/thrift/org/apache/aurora/gen/api.thrift (lines 266 - 267)


Please rename to something like mesosFetcherUris (or just fetcherUris) to 
document purpose of these uris in this context.


- David McLaughlin


On June 24, 2016, 11:01 p.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49218/
> ---
> 
> (Updated June 24, 2016, 11:01 p.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Adding a URIs field to TaskConfig inside the ThriftAPI so that users are able 
> to specify resources they wish to download into the sandbox per job.
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 3e6daf53dd563dd7a2d494cc95e9a0aba0b6 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 4089b79da8079243703eead884e80bcf736f8b29 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> 3b01801d929dd61ee989495bf38af8f03e9f5ad4 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
> c76164292cf62d2181374c09f8bf6d8d3358e982 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java 
> 571201094c1e576e496495a01cb83f6c57decfa8 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/migration/V007_CreateURIsTable.java
>  PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 
> a90cb00e240df25dce6d55728859768e22d741a6 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml
>  2c8af8b88e41b3b381cac831fd43b1057e4df0aa 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> 5069bedc08bb7111d0e0f101c8a2c81495b97bc9 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> 58785bfa37ff214f26e9f94d836e6df40e411c3b 
>   src/test/java/org/apache/aurora/scheduler/thrift/Fixtures.java 
> a883b0e33bfec1d14e6fe4ee8ed2200d93acaeec 
> 
> Diff: https://reviews.apache.org/r/49218/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build -Pq
> ./build-support/jenkins/build.sh
> bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Renan DelValle
> 
>

Re: Review Request 49218: Add support for Mesos Fetcher

2016-06-24 Thread Aurora ReviewBot 

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


Ship it!




Master (73dd2a8) 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 June 24, 2016, 11:01 p.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49218/
> ---
> 
> (Updated June 24, 2016, 11:01 p.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Adding a URIs field to TaskConfig inside the ThriftAPI so that users are able 
> to specify resources they wish to download into the sandbox per job.
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 3e6daf53dd563dd7a2d494cc95e9a0aba0b6 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 4089b79da8079243703eead884e80bcf736f8b29 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> 3b01801d929dd61ee989495bf38af8f03e9f5ad4 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
> c76164292cf62d2181374c09f8bf6d8d3358e982 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java 
> 571201094c1e576e496495a01cb83f6c57decfa8 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/migration/V007_CreateURIsTable.java
>  PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 
> a90cb00e240df25dce6d55728859768e22d741a6 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml
>  2c8af8b88e41b3b381cac831fd43b1057e4df0aa 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> 5069bedc08bb7111d0e0f101c8a2c81495b97bc9 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> 58785bfa37ff214f26e9f94d836e6df40e411c3b 
>   src/test/java/org/apache/aurora/scheduler/thrift/Fixtures.java 
> a883b0e33bfec1d14e6fe4ee8ed2200d93acaeec 
> 
> Diff: https://reviews.apache.org/r/49218/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build -Pq
> ./build-support/jenkins/build.sh
> bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Renan DelValle
> 
>

Review Request 49218: Add support for Mesos Fetcher

2016-06-24 Thread Renan DelValle 

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

Review request for Aurora.


Repository: aurora


Description
---

Adding a URIs field to TaskConfig inside the ThriftAPI so that users are able 
to specify resources they wish to download into the sandbox per job.


Diffs
-

  api/src/main/thrift/org/apache/aurora/gen/api.thrift 
3e6daf53dd563dd7a2d494cc95e9a0aba0b6 
  src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
4089b79da8079243703eead884e80bcf736f8b29 
  src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
3b01801d929dd61ee989495bf38af8f03e9f5ad4 
  src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
c76164292cf62d2181374c09f8bf6d8d3358e982 
  src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java 
571201094c1e576e496495a01cb83f6c57decfa8 
  
src/main/java/org/apache/aurora/scheduler/storage/db/migration/V007_CreateURIsTable.java
 PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 
a90cb00e240df25dce6d55728859768e22d741a6 
  
src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml 
2c8af8b88e41b3b381cac831fd43b1057e4df0aa 
  src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
5069bedc08bb7111d0e0f101c8a2c81495b97bc9 
  src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
58785bfa37ff214f26e9f94d836e6df40e411c3b 
  src/test/java/org/apache/aurora/scheduler/thrift/Fixtures.java 
a883b0e33bfec1d14e6fe4ee8ed2200d93acaeec 

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


Testing
---

./gradlew build -Pq
./build-support/jenkins/build.sh
bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh


Thanks,

Renan DelValle