Re: Review Request 45222: Introduce "preemptible" flag in TierInfo with backward compatible support for "production" flag in TaskConfig.

2016-03-25 Thread Aurora ReviewBot

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


Ship it!




Master (b352e8b) 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 26, 2016, 12:08 a.m., Amol Deshmukh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45222/
> ---
> 
> (Updated March 26, 2016, 12:08 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-1616
> https://issues.apache.org/jira/browse/AURORA-1616
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Introduce "preemptible" flag in TierInfo with backward compatible support for 
> "production" flag in TaskConfig.
> 
> # Summary of changes
> - TierInfo extended to manage a new property: "preemptible"
> - If TaskConfig does not specify a tier, ``TierManager.getTier(..)`` falls 
> back to selecting a non-revocable tier matching ``TaskConfig.isProduction()``.
> - Removed ``TierInfo.DEFAULT`` and replaced its in test/benchmark code by 
> references to ``TaskTestUtil.DEV_TIER``.
> - Eager validation of tier specified in TaskConfig in 
> ``ConfigurationManager.validateAndPopulate(..)``
> 
> # Note on backward incompatible change
> TaskConfigs with a tier name not matching any tiers defined in ``tiers.json`` 
> were silently assigned a default tier. With this change, attempting to 
> schedule tasks that specify non-existent tier names will result in an error 
> (see ``ConfigurationManager.validateAndPopulate(ITaskConfig config)``).
> 
> 
> Diffs
> -
> 
>   src/jmh/java/org/apache/aurora/benchmark/Offers.java 
> 055a2ffcb13c643a3086343e3fbf71545c5fb0a6 
>   src/main/java/org/apache/aurora/scheduler/TierInfo.java 
> b4611b9cf99ca236cd1ea4610d01c0d293bf2e87 
>   src/main/java/org/apache/aurora/scheduler/TierManager.java 
> 480c4853a6c87dd63a9810ae013e5cfacb29336d 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 97d87ff1b789625f2c07baf7a74a076f07742596 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 9cf8002e932d562e93fb8d17b4c56f564eb54ed5 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  b3b8ccf868c2a2f18f720a837e90d763072dd3eb 
>   src/main/java/org/apache/aurora/scheduler/mesos/TestExecutorSettings.java 
> 16fa2d35cdf7ecdf82dd1ec7739e685ec1ff1aa2 
>   
> src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java
>  7f84e90774193b0d31adb7dafcab0a249167cdba 
>   src/main/resources/org/apache/aurora/scheduler/tiers.json 
> 18701058076bedc5d1b667e2b97ad09ce84a9bb9 
>   src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java 
> 39096af816864ada32a9c07958975740e805f6b0 
>   src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java 
> 52113b80d91ecaf0cc2aeaad77e5fbc0ea4d1216 
>   src/test/java/org/apache/aurora/scheduler/ResourcesTest.java 
> 4fe8c518c580418078275b8056a5c195a765681e 
>   src/test/java/org/apache/aurora/scheduler/TierManagerTest.java 
> a662100ba726cff0e47b4f9650753db9cecdef51 
>   src/test/java/org/apache/aurora/scheduler/TierModuleTest.java 
> e0601c83486671596e412f022ffae78b01c81c9d 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  1a520b3cb035a9afc25406d2f313c9f861eee4d6 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> 5103bd0f43d53079976b0f1596e299f2d91aa860 
>   
> src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java
>  b6f5e4632ac1e028fdf93da1735463373e2d2788 
>   src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 
> b00add0b2fd4277e196505fffba4440e2e94207e 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> b1c71a6f1847d205c378d0b5a7269ea9d1165be5 
> 
> Diff: https://reviews.apache.org/r/45222/diff/
> 
> 
> Testing
> ---
> 
> # End to end tests
> ```
> $ ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> ...
> *** OK (All tests passed) ***
> ```
> 
> # Jenkins build.sh
> ```
> $ ./build-support/jenkins/build.sh
> ...
>SUCCESS
> ```
> 
> # Local Scheduler
> ```
> $ ./gradlew run
> ...
> I0325 16:00:55.374 [pool-9-thread-1, FakeMaster:139] All offers consumed, 
> suppressing offer cycle.
> I0325 16:01:00.371 [pool-9-thread-1, FakeMaster:139] All offers consumed, 
> suppressing offer cycle.
> ```
> 
> # Attempting to schedule job with invalid tier-name
> ```
> vagrant@aurora:~/workspace$ aurora job create devcluster/vagrant/devel/test 
> job.aurora
>  INFO] Creating job test
> Job creation failed due to error:
>   Invalid 

Re: Review Request 45222: Introduce "preemptible" flag in TierInfo with backward compatible support for "production" flag in TaskConfig.

2016-03-25 Thread Amol Deshmukh

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

(Updated March 25, 2016, 5:08 p.m.)


Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Bill Farner.


Changes
---

Linked AURORA-1616


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


Repository: aurora


Description
---

Introduce "preemptible" flag in TierInfo with backward compatible support for 
"production" flag in TaskConfig.

# Summary of changes
- TierInfo extended to manage a new property: "preemptible"
- If TaskConfig does not specify a tier, ``TierManager.getTier(..)`` falls back 
to selecting a non-revocable tier matching ``TaskConfig.isProduction()``.
- Removed ``TierInfo.DEFAULT`` and replaced its in test/benchmark code by 
references to ``TaskTestUtil.DEV_TIER``.
- Eager validation of tier specified in TaskConfig in 
``ConfigurationManager.validateAndPopulate(..)``

# Note on backward incompatible change
TaskConfigs with a tier name not matching any tiers defined in ``tiers.json`` 
were silently assigned a default tier. With this change, attempting to schedule 
tasks that specify non-existent tier names will result in an error (see 
``ConfigurationManager.validateAndPopulate(ITaskConfig config)``).


Diffs
-

  src/jmh/java/org/apache/aurora/benchmark/Offers.java 
055a2ffcb13c643a3086343e3fbf71545c5fb0a6 
  src/main/java/org/apache/aurora/scheduler/TierInfo.java 
b4611b9cf99ca236cd1ea4610d01c0d293bf2e87 
  src/main/java/org/apache/aurora/scheduler/TierManager.java 
480c4853a6c87dd63a9810ae013e5cfacb29336d 
  src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
97d87ff1b789625f2c07baf7a74a076f07742596 
  src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
9cf8002e932d562e93fb8d17b4c56f564eb54ed5 
  
src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
 b3b8ccf868c2a2f18f720a837e90d763072dd3eb 
  src/main/java/org/apache/aurora/scheduler/mesos/TestExecutorSettings.java 
16fa2d35cdf7ecdf82dd1ec7739e685ec1ff1aa2 
  
src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java 
7f84e90774193b0d31adb7dafcab0a249167cdba 
  src/main/resources/org/apache/aurora/scheduler/tiers.json 
18701058076bedc5d1b667e2b97ad09ce84a9bb9 
  src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java 
39096af816864ada32a9c07958975740e805f6b0 
  src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java 
52113b80d91ecaf0cc2aeaad77e5fbc0ea4d1216 
  src/test/java/org/apache/aurora/scheduler/ResourcesTest.java 
4fe8c518c580418078275b8056a5c195a765681e 
  src/test/java/org/apache/aurora/scheduler/TierManagerTest.java 
a662100ba726cff0e47b4f9650753db9cecdef51 
  src/test/java/org/apache/aurora/scheduler/TierModuleTest.java 
e0601c83486671596e412f022ffae78b01c81c9d 
  
src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
 1a520b3cb035a9afc25406d2f313c9f861eee4d6 
  src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
5103bd0f43d53079976b0f1596e299f2d91aa860 
  
src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java
 b6f5e4632ac1e028fdf93da1735463373e2d2788 
  src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 
b00add0b2fd4277e196505fffba4440e2e94207e 
  src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
b1c71a6f1847d205c378d0b5a7269ea9d1165be5 

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


Testing
---

# End to end tests
```
$ ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
...
*** OK (All tests passed) ***
```

# Jenkins build.sh
```
$ ./build-support/jenkins/build.sh
...
   SUCCESS
```

# Local Scheduler
```
$ ./gradlew run
...
I0325 16:00:55.374 [pool-9-thread-1, FakeMaster:139] All offers consumed, 
suppressing offer cycle.
I0325 16:01:00.371 [pool-9-thread-1, FakeMaster:139] All offers consumed, 
suppressing offer cycle.
```

# Attempting to schedule job with invalid tier-name
```
vagrant@aurora:~/workspace$ aurora job create devcluster/vagrant/devel/test 
job.aurora
 INFO] Creating job test
Job creation failed due to error:
Invalid tier 'badtier' in TaskConfig.
```


Thanks,

Amol Deshmukh



Review Request 45222: Introduce "preemptible" flag in TierInfo with backward compatible support for "production" flag in TaskConfig.

2016-03-25 Thread Amol Deshmukh

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

Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Bill Farner.


Repository: aurora


Description
---

Introduce "preemptible" flag in TierInfo with backward compatible support for 
"production" flag in TaskConfig.

# Summary of changes
- TierInfo extended to manage a new property: "preemptible"
- If TaskConfig does not specify a tier, ``TierManager.getTier(..)`` falls back 
to selecting a non-revocable tier matching ``TaskConfig.isProduction()``.
- Removed ``TierInfo.DEFAULT`` and replaced its in test/benchmark code by 
references to ``TaskTestUtil.DEV_TIER``.
- Eager validation of tier specified in TaskConfig in 
``ConfigurationManager.validateAndPopulate(..)``

# Note on backward incompatible change
TaskConfigs with a tier name not matching any tiers defined in ``tiers.json`` 
were silently assigned a default tier. With this change, attempting to schedule 
tasks that specify non-existent tier names will result in an error (see 
``ConfigurationManager.validateAndPopulate(ITaskConfig config)``).


Diffs
-

  src/jmh/java/org/apache/aurora/benchmark/Offers.java 
055a2ffcb13c643a3086343e3fbf71545c5fb0a6 
  src/main/java/org/apache/aurora/scheduler/TierInfo.java 
b4611b9cf99ca236cd1ea4610d01c0d293bf2e87 
  src/main/java/org/apache/aurora/scheduler/TierManager.java 
480c4853a6c87dd63a9810ae013e5cfacb29336d 
  src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
97d87ff1b789625f2c07baf7a74a076f07742596 
  src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
9cf8002e932d562e93fb8d17b4c56f564eb54ed5 
  
src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
 b3b8ccf868c2a2f18f720a837e90d763072dd3eb 
  src/main/java/org/apache/aurora/scheduler/mesos/TestExecutorSettings.java 
16fa2d35cdf7ecdf82dd1ec7739e685ec1ff1aa2 
  
src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java 
7f84e90774193b0d31adb7dafcab0a249167cdba 
  src/main/resources/org/apache/aurora/scheduler/tiers.json 
18701058076bedc5d1b667e2b97ad09ce84a9bb9 
  src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java 
39096af816864ada32a9c07958975740e805f6b0 
  src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java 
52113b80d91ecaf0cc2aeaad77e5fbc0ea4d1216 
  src/test/java/org/apache/aurora/scheduler/ResourcesTest.java 
4fe8c518c580418078275b8056a5c195a765681e 
  src/test/java/org/apache/aurora/scheduler/TierManagerTest.java 
a662100ba726cff0e47b4f9650753db9cecdef51 
  src/test/java/org/apache/aurora/scheduler/TierModuleTest.java 
e0601c83486671596e412f022ffae78b01c81c9d 
  
src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
 1a520b3cb035a9afc25406d2f313c9f861eee4d6 
  src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
5103bd0f43d53079976b0f1596e299f2d91aa860 
  
src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java
 b6f5e4632ac1e028fdf93da1735463373e2d2788 
  src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 
b00add0b2fd4277e196505fffba4440e2e94207e 
  src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
b1c71a6f1847d205c378d0b5a7269ea9d1165be5 

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


Testing
---

# End to end tests
```
$ ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
...
*** OK (All tests passed) ***
```

# Jenkins build.sh
```
$ ./build-support/jenkins/build.sh
...
   SUCCESS
```

# Local Scheduler
```
$ ./gradlew run
...
I0325 16:00:55.374 [pool-9-thread-1, FakeMaster:139] All offers consumed, 
suppressing offer cycle.
I0325 16:01:00.371 [pool-9-thread-1, FakeMaster:139] All offers consumed, 
suppressing offer cycle.
```

# Attempting to schedule job with invalid tier-name
```
vagrant@aurora:~/workspace$ aurora job create devcluster/vagrant/devel/test 
job.aurora
 INFO] Creating job test
Job creation failed due to error:
Invalid tier 'badtier' in TaskConfig.
```


Thanks,

Amol Deshmukh



Re: Review Request 45177: Prototype of setting DiscoveryInfo.

2016-03-25 Thread Stephan Erb


> On March 25, 2016, 9:30 p.m., Stephan Erb wrote:
> > src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java, line 
> > 237
> > 
> >
> > That can lead to non-DNS compatible names. I guess there is nothering 
> > that we can really do about it. It's just something consumers have to be 
> > aware of.

Looks like we don't have to worry: 
https://github.com/apache/aurora/blob/26efe5517fc0cb471101fdcb072e5dbf5d20bc56/api/src/main/thrift/org/apache/aurora/gen/api.thrift#L426


- Stephan


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


On March 22, 2016, 9:10 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45177/
> ---
> 
> (Updated March 22, 2016, 9:10 p.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Prototype of setting DiscoveryInfo.
> 
> Disclaimer: this just shows how easy it is to pipe this information, not 
> seeking to commit the code as is.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> 20cbd419dcb988f2c7ba72c8acbe6fee90d02248 
> 
> Diff: https://reviews.apache.org/r/45177/diff/
> 
> 
> Testing
> ---
> 
> Use the vagrant environment to start the example in http_example.aurora, and 
> observe the following in master's state.json:
> 
> ```
> curl 192.168.33.7:5050/state | jq . | less
> 
> "tasks": [
> {
> "discovery": {
> "environment": "test",
> "name": "vagrant.test.http_example.1",
> "ports": {
> "ports": [
> {
> "name": "http",
> "number": 31648
> }
> ]
> },
> "visibility": "CLUSTER"
> },
> "executor_id": 
> "thermos-vagrant-test-http_example-1-69c3908a-e8cd-44a7-a824-dd87ef5920f0",
> "framework_id": "770e194c-6366-41f2-8a2d-e2c616aa9490-",
> "id": "vagrant-test-http_example-1-69c3908a-e8cd-44a7-a824-dd87ef5920f0",
> "name": "vagrant/test/http_example",
> "resources": {
> "cpus": 0.4,
> "disk": 64,
> "mem": 32,
> "ports": "[31648-31648]"
> },
> ...
> 
> ```
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>



Re: Review Request 45177: Prototype of setting DiscoveryInfo.

2016-03-25 Thread Stephan Erb

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




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


Sounds OK as hard coded value for now.



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


That can lead to non-DNS compatible names. I guess there is nothering that 
we can really do about it. It's just something consumers have to be aware of.



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


That sounds like a sane choice, but we could also leave it out for now 
until there is a proper usecase for it.



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


I guess this refers to the API version of the task? The Mesos documentation 
is not clear here. I'd vote to keep it out for now.



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


We could probably do this via the `Announcer` struct. Unfortunately, the 
documentation is rather unclear if we should pass L4 or L7 protocol names: 
https://github.com/apache/mesos/blob/master/include/mesos/mesos.proto#L1799

However, looks like Marathon is saying this should by only `TCP` or `UDP`: 
https://github.com/mesosphere/marathon/blob/master/src/main/scala/mesosphere/marathon/state/DiscoveryInfo.scala#L25
 Hardcoding this value to `TCP` should therefore probably work for us for now, 
right?


- Stephan Erb


On March 22, 2016, 9:10 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45177/
> ---
> 
> (Updated March 22, 2016, 9:10 p.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Prototype of setting DiscoveryInfo.
> 
> Disclaimer: this just shows how easy it is to pipe this information, not 
> seeking to commit the code as is.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> 20cbd419dcb988f2c7ba72c8acbe6fee90d02248 
> 
> Diff: https://reviews.apache.org/r/45177/diff/
> 
> 
> Testing
> ---
> 
> Use the vagrant environment to start the example in http_example.aurora, and 
> observe the following in master's state.json:
> 
> ```
> curl 192.168.33.7:5050/state | jq . | less
> 
> "tasks": [
> {
> "discovery": {
> "environment": "test",
> "name": "vagrant.test.http_example.1",
> "ports": {
> "ports": [
> {
> "name": "http",
> "number": 31648
> }
> ]
> },
> "visibility": "CLUSTER"
> },
> "executor_id": 
> "thermos-vagrant-test-http_example-1-69c3908a-e8cd-44a7-a824-dd87ef5920f0",
> "framework_id": "770e194c-6366-41f2-8a2d-e2c616aa9490-",
> "id": "vagrant-test-http_example-1-69c3908a-e8cd-44a7-a824-dd87ef5920f0",
> "name": "vagrant/test/http_example",
> "resources": {
> "cpus": 0.4,
> "disk": 64,
> "mem": 32,
> "ports": "[31648-31648]"
> },
> ...
> 
> ```
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>



Re: Review Request 45212: Remove hard dependency on a specific mesos-version

2016-03-25 Thread John Sirois

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


Ship it!




Ship It!

- John Sirois


On March 23, 2016, 8:56 a.m., Pierre Cheynier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45212/
> ---
> 
> (Updated March 23, 2016, 8:56 a.m.)
> 
> 
> Review request for Aurora, Jake Farrell, John Sirois, Stephan Erb, Bill 
> Farner, and Zameer Manji.
> 
> 
> Repository: aurora-packaging
> 
> 
> Description
> ---
> 
> We should consider MESOS_VERSION as the minimal requirement to install
> the current Aurora version instead of enforce a specific Mesos version.
> 
> 
> Diffs
> -
> 
>   specs/rpm/aurora.spec 61e7d146108ae7dd5e129d8288a05773c2659d25 
> 
> Diff: https://reviews.apache.org/r/45212/diff/
> 
> 
> Testing
> ---
> 
> Install Aurora through the RPM built with aurora-packaging on a Mesos 0.27
> running install.
> 
> 
> Thanks,
> 
> Pierre Cheynier
> 
>



Re: Review Request 45212: Remove hard dependency on a specific mesos-version

2016-03-25 Thread Bill Farner

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


Ship it!




Ship It!

- Bill Farner


On March 23, 2016, 7:56 a.m., Pierre Cheynier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45212/
> ---
> 
> (Updated March 23, 2016, 7:56 a.m.)
> 
> 
> Review request for Aurora, Jake Farrell, John Sirois, Stephan Erb, Bill 
> Farner, and Zameer Manji.
> 
> 
> Repository: aurora-packaging
> 
> 
> Description
> ---
> 
> We should consider MESOS_VERSION as the minimal requirement to install
> the current Aurora version instead of enforce a specific Mesos version.
> 
> 
> Diffs
> -
> 
>   specs/rpm/aurora.spec 61e7d146108ae7dd5e129d8288a05773c2659d25 
> 
> Diff: https://reviews.apache.org/r/45212/diff/
> 
> 
> Testing
> ---
> 
> Install Aurora through the RPM built with aurora-packaging on a Mesos 0.27
> running install.
> 
> 
> Thanks,
> 
> Pierre Cheynier
> 
>



Re: Review Request 45212: Remove hard dependency on a specific mesos-version

2016-03-25 Thread Stephan Erb

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


Ship it!




Ship It!

- Stephan Erb


On March 23, 2016, 3:56 p.m., Pierre Cheynier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45212/
> ---
> 
> (Updated March 23, 2016, 3:56 p.m.)
> 
> 
> Review request for Aurora, Jake Farrell, John Sirois, Stephan Erb, Bill 
> Farner, and Zameer Manji.
> 
> 
> Repository: aurora-packaging
> 
> 
> Description
> ---
> 
> We should consider MESOS_VERSION as the minimal requirement to install
> the current Aurora version instead of enforce a specific Mesos version.
> 
> 
> Diffs
> -
> 
>   specs/rpm/aurora.spec 61e7d146108ae7dd5e129d8288a05773c2659d25 
> 
> Diff: https://reviews.apache.org/r/45212/diff/
> 
> 
> Testing
> ---
> 
> Install Aurora through the RPM built with aurora-packaging on a Mesos 0.27
> running install.
> 
> 
> Thanks,
> 
> Pierre Cheynier
> 
>



Re: Review Request 45212: Remove hard dependency on a specific mesos-version

2016-03-25 Thread Stephan Erb


> On March 23, 2016, 3:31 p.m., Stephan Erb wrote:
> > Thinking out loud here, so please comment: 
> > 
> > We could move to a mode where we build against a specific Mesos version, 
> > and recommend that version for deployment, but leave it up to the cluster 
> > operator to select and deploy a compatible Mesos version. 
> > 
> > This would enable the following usecase:
> > 
> > * Aurora 0.12 currently depends on Mesos 0.25
> > * By the given compatability, cluster operators can safely update to Mesos 
> > 0.26.
> > * Instead of releasing Aurora with Mesos 0.26 as currently planned, we 
> > release a version build against 0.27. This one will be backwards 
> > compatible, and will therefore work with the deployed Mesos 0.26.
> > * Cluster operators can then safely update to Mesos 0.27 and Mesos 0.28
> > 
> > This should make it easier for us to keep up with the Mesos release train...
> 
> John Sirois wrote:
> Leaving off the package dependency (which we already do by mistake for 
> the aurora-executor deb) certainly has a maintenance appeal, since we already 
> need to maintain the install guide in the aurora repo, which could contain or 
> point to a compatibility matrix we maintain.  If we do go with no explicit 
> mesos dependency in our binary packages (or a floating one), I think its 
> important a compatibility matrix be prominent in the install docs since the 
> questions and install problems will happen.  But if we do maintain a 
> compatibility matrix we could ~just as easily be adding the compatibility 
> constraints into the package dependencies too and avoiding a wider swath of 
> bug reports / questions.
> 
> I've widened the reviewer scope a bit to gather more opions here.  This 
> may need a dev@ thread.
> 
> Stephan Erb wrote:
> It's not a real bug for the executor. For the executor we boundle the 
> `mesos.native` wheel in the pex-file.
> 
> Bill Farner wrote:
> I'm pro min-version.  Without true semantic versioning in mesos, i'm 
> doubtful we could be accurate with an upper bound.
> 
> Zameer Manji wrote:
> I am pro min and max version. I believe the range that John proposes 
> above is the best way to go. Mesos only guarantees -1/+1 and we should 
> reflect that in the packaging. In my experience I have been bit by 
> incompatabilities that can exist beyond +1/-1 and they were very difficult to 
> debug.
> 
> A more sophisticated cluster operator that knows what they are doing can 
> use the facilities of rpm/deb to force a version of the package beyond our 
> constraints if neeed.
> 
> I'm not in favor of a compatability matrix, it seems like it would be a 
> lot of work to maintain and test out, I suggest just rolling with what the 
> Mesos project recomends until a better story comes out.
> 
> Maxim Khutornenko wrote:
> -1 to this patch. I don't see any real benefits here and certainly would 
> not want to _guess_ the compat matrix.
> 
> Pierre Cheynier wrote:
> From my point of view, this compatibility (or at least approval) matrix 
> could be simply written in the release note at each release.
> For ex: "0.12.0 supports only 0.25+ (enforced by constraint) and was 
> tested on: 0.25/0.26/0.27".
> 
> No guarantee on the newer versions, but it avoid everyone that want to 
> test it to patch the aurora-packaging project.
> 
> Operators on such big perimeters should be aware of the possible impacts 
> in upgrading one of the building block of their infrastructure.
> 
> John Sirois wrote:
> I'll point out that Maxim's -1 implies a -1 to the existing deb packaging 
> which uses >= instead of pinning like the rpms.
> 
> OK - so we have absolutely no consensus fwict, but to move things 
> forward, I'd like to see consistency in the version constraints across our 
> binary packages and I think Pierre's matrix proposal is good enough to allay 
> my fears of folks trying really new versions of mesos with older versions of 
> aurora and expecting things to work.  We could either use a matrix in the 
> installing docs - which I still favor in Pierre's suggested format - or, if 
> Zameer or others still find that  to be too much overhead - I'd also be fine 
> with static boilerplate text in installing docs that say tested with X, X+1 
> should work, for the rest you are on your own and extensive testing is 
> reccomended before using the combination in prod.
> 
> So the tally is:
> +3 >=  (Pierre, Bill, John)
> +1 [no constraint] (Stephan)
> +1 >= <=   (Zameer)
> +1 ==  (Maxim)
> 
> Anyone else willing to compromise here?
> 
> Zameer Manji wrote:
> I'm willing to move to the `>=` camp to be inline with the existing debs 
> and for the documentation to suggest X and X+1 are the suggested versions of 
> Mesos.

I am fine with `>=` as well.


- Stephan


---
This is an automatically generated 

Re: Review Request 45298: Remove some dependencies linked to docker usage

2016-03-25 Thread John Sirois


> On March 24, 2016, 12:04 p.m., John Sirois wrote:
> > specs/rpm/aurora.spec, line 146
> > 
> >
> > This breaks the release script, you can try something like this - it 
> > needs to run green: `./build-artifact.sh /tmp/apache-aurora-0.12.0.tar.gz 
> > 0.12.0`
> 
> Pierre Cheynier wrote:
> Hi John.
> This is why I edited the README (AFAIK, this is a pure naming convention 
> introduced by the README). 
> 
> I changed it to be able to make the aurora.spec work (see Source0 : it 
> fetch Aurora sources from github).
> 
> Pierre Cheynier wrote:
> Let me know, maybe it was not the good approach. I was initially 
> wondering why I was not able to simply build the RPM with spectool + mock (= 
> fetch sources + rpmbuild in a chrooted context).

Gotcha.  The failure I got was from using the official released sources, ie: 
this bit of the README:
> Fetch a source distribution, such as an [official 
> one](https://aurora.apache.org/downloads/).

So that portion of the README needscan update.


- John


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


On March 24, 2016, 12:04 p.m., Pierre Cheynier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45298/
> ---
> 
> (Updated March 24, 2016, 12:04 p.m.)
> 
> 
> Review request for Aurora and John Sirois.
> 
> 
> Repository: aurora-packaging
> 
> 
> Description
> ---
> 
> An operator should be able to build Aurora on his platform without
> relying on Docker.
> 
> Even if this is an interesting way of doing it, it sometimes introduce missed
> (build-)dependencies (in this case openssl) and context-specific behaviour
> (here, the way pants.ini was added to the build context)
> 
> 
> Diffs
> -
> 
>   README.md 3a7cf45034b7896c23588fed83176468ca627ebc 
>   builder/deb/debian-jessie/Dockerfile 
> 63e89deed9a411b1859ba28ea2572ef4a210da1f 
>   builder/deb/debian-jessie/pants.ini 
> 2ff2d5eb6128d4be3e15c0488d8d23bed81a8d5b 
>   builder/deb/ubuntu-trusty/Dockerfile 
> 52739100c9593292a7aea2459412c8e49f0155a4 
>   builder/deb/ubuntu-trusty/pants.ini  
>   builder/rpm/centos-7/Dockerfile a4d9dfcee5637d2c31d0c2b63a9ebcf04e8d 
>   builder/rpm/centos-7/pants.ini  
>   specs/debian/control a30e987e4423c611c86518a8f5e3e68ee996982a 
>   specs/debian/rules 974fddfda6da04686daae691fa32c5b4c11a0b5d 
>   specs/rpm/aurora.spec 61e7d146108ae7dd5e129d8288a05773c2659d25 
> 
> Diff: https://reviews.apache.org/r/45298/diff/
> 
> 
> Testing
> ---
> 
> Able to build on CentOS 6 using mock.
> Able to build on Debian. Limitation = need the patched version of Gradle.
> Still able to build using the current Docker build system
> 
> 
> Thanks,
> 
> Pierre Cheynier
> 
>



Re: Review Request 45104: Use chmod+x to make termos_runner.pex executable

2016-03-25 Thread Joshua Cohen


> On March 22, 2016, 1:02 a.m., Joshua Cohen wrote:
> > Sorry for the delay on this. After you filed the pull request, I 
> > investigated a bit what will be required once Mesos 0.30.0 lands: 
> > https://issues.apache.org/jira/browse/AURORA-1632. I think the problem goes 
> > beyond the failure to find `sys.executable` when $PATH is not set. As even 
> > after switching back to chmod+x on the runner, the task failed further down 
> > the stack.
> > 
> > I suspect the fix for Mesos 0.30.0 will be to set our own $PATH which 
> > should allow `sys.executable` to continue working and will allow any tasks 
> > users have running which have come to rely on Thermos setting it for them 
> > to behave as expected. The problem is, I haven't had time to figure out 
> > what we should set $PATH to yet ;) (anyone have any thoughts?).
> > 
> > I know this is probably more info than you bargained for when you opened 
> > what seemed like a simple pull request. I'm not opposed to accepting this 
> > patch (with a TODO to restore `sys.executable` when we figure out what to 
> > do about setting $PATH) if it unblocks your use case, but can you confirm 
> > that you're actually able to run the Mesos agent with 
> > `--executor_environment_variables='{}'` and still launch tasks?
> 
> Pierre Cheynier wrote:
> Hi,
> 
> Sorry for my delay on opening this review on apache.org.
> Actually, I started to use Aurora and faced this issue cause my setup use 
> `--executor_environment_variables`.
> I first investigated in the Python default setup on CentOS, the pants/pex 
> build system and then the differences between the vagrant box provided in the 
> repo and mine before discovering that this is the origin of the issue.
> 
> I admit this is a simple fix, I just tried to understand the reason of 
> using this mechanism and found that it was changed 2 years ago.
> In 0.27.2  it works well by using this patch (I'm now able to run 
> something).
> 
> Joshua Cohen wrote:
> So he's the behavior I see when I run the Mesos agent with 
> `--executor_environment_variables='{}'`. Without this patch, as you note, 
> tasks fail to start with a permission denied error (due to `sys.executable` 
> being unset). After applying this patch, tasks start up, but processes fail 
> to fork: 
> https://www.dropbox.com/s/7zzy574xdy02ukq/Screen%20Shot%202016-03-24%20at%203.49.24%20PM.png?dl=0.
> 
> Are you by chance running Docker tasks?
> 
> Pierre Cheynier wrote:
> I'm running tasks on the Mesos containerizer for now.
> Your problem is not purely a side-effect of not passing any environment 
> variables ? (your tasks themselves maybe need PATH or LD_LIBRARY_PATH ?)

It's not the task that requires PATH to be set, it's the thermos runner itself: 
https://github.com/apache/aurora/blob/master/src/main/python/apache/thermos/core/process.py#L397-L402

This is why I'm confused as to how this works for you unless PATH is somehow 
being set elsewhere (and if PATH is set elsewhere, then `sys.executable` should 
work as well).


- Joshua


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


On March 22, 2016, 9:50 a.m., Pierre Cheynier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45104/
> ---
> 
> (Updated March 22, 2016, 9:50 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> When using `--executor_environment_variables` without explicitely
> passing LD_LIBRARY_PATH, `sys.executable` returns an empty string
> resulting in a '[Errno 13] Permission denied' error for every launched
> task.
> 
> Moreover, it seems that this feature is coming in 0.30: "Executors no
> longer inherit environment variables from the agent".
> 
> This patch partially revert back 07ce21d where chmod_x method was
> removed in favor of using sys.executable.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/executor/thermos_task_runner.py 
> 3896e3841562600379705dbf78a6f62728246348 
> 
> Diff: https://reviews.apache.org/r/45104/diff/
> 
> 
> Testing
> ---
> 
> Make Aurora executor run on CentOS7, while running mesos agent with 
> `--executor_environment_variables` option and no excplicit $PATH set.
> 
> 
> Thanks,
> 
> Pierre Cheynier
> 
>



Re: Review Request 45298: Remove some dependencies linked to docker usage

2016-03-25 Thread Pierre Cheynier


> On mars 24, 2016, 6:04 après-midi, John Sirois wrote:
> > specs/rpm/aurora.spec, line 146
> > 
> >
> > This breaks the release script, you can try something like this - it 
> > needs to run green: `./build-artifact.sh /tmp/apache-aurora-0.12.0.tar.gz 
> > 0.12.0`
> 
> Pierre Cheynier wrote:
> Hi John.
> This is why I edited the README (AFAIK, this is a pure naming convention 
> introduced by the README). 
> 
> I changed it to be able to make the aurora.spec work (see Source0 : it 
> fetch Aurora sources from github).

Let me know, maybe it was not the good approach. I was initially wondering why 
I was not able to simply build the RPM with spectool + mock (= fetch sources + 
rpmbuild in a chrooted context).


- Pierre


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


On mars 24, 2016, 6:04 après-midi, Pierre Cheynier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45298/
> ---
> 
> (Updated mars 24, 2016, 6:04 après-midi)
> 
> 
> Review request for Aurora and John Sirois.
> 
> 
> Repository: aurora-packaging
> 
> 
> Description
> ---
> 
> An operator should be able to build Aurora on his platform without
> relying on Docker.
> 
> Even if this is an interesting way of doing it, it sometimes introduce missed
> (build-)dependencies (in this case openssl) and context-specific behaviour
> (here, the way pants.ini was added to the build context)
> 
> 
> Diffs
> -
> 
>   README.md 3a7cf45034b7896c23588fed83176468ca627ebc 
>   builder/deb/debian-jessie/Dockerfile 
> 63e89deed9a411b1859ba28ea2572ef4a210da1f 
>   builder/deb/debian-jessie/pants.ini 
> 2ff2d5eb6128d4be3e15c0488d8d23bed81a8d5b 
>   builder/deb/ubuntu-trusty/Dockerfile 
> 52739100c9593292a7aea2459412c8e49f0155a4 
>   builder/deb/ubuntu-trusty/pants.ini  
>   builder/rpm/centos-7/Dockerfile a4d9dfcee5637d2c31d0c2b63a9ebcf04e8d 
>   builder/rpm/centos-7/pants.ini  
>   specs/debian/control a30e987e4423c611c86518a8f5e3e68ee996982a 
>   specs/debian/rules 974fddfda6da04686daae691fa32c5b4c11a0b5d 
>   specs/rpm/aurora.spec 61e7d146108ae7dd5e129d8288a05773c2659d25 
> 
> Diff: https://reviews.apache.org/r/45298/diff/
> 
> 
> Testing
> ---
> 
> Able to build on CentOS 6 using mock.
> Able to build on Debian. Limitation = need the patched version of Gradle.
> Still able to build using the current Docker build system
> 
> 
> Thanks,
> 
> Pierre Cheynier
> 
>



Re: Review Request 45298: Remove some dependencies linked to docker usage

2016-03-25 Thread Pierre Cheynier


> On mars 24, 2016, 6:04 après-midi, John Sirois wrote:
> > specs/rpm/aurora.spec, line 146
> > 
> >
> > This breaks the release script, you can try something like this - it 
> > needs to run green: `./build-artifact.sh /tmp/apache-aurora-0.12.0.tar.gz 
> > 0.12.0`

Hi John.
This is why I edited the README (AFAIK, this is a pure naming convention 
introduced by the README). 

I changed it to be able to make the aurora.spec work (see Source0 : it fetch 
Aurora sources from github).


- Pierre


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


On mars 24, 2016, 6:04 après-midi, Pierre Cheynier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45298/
> ---
> 
> (Updated mars 24, 2016, 6:04 après-midi)
> 
> 
> Review request for Aurora and John Sirois.
> 
> 
> Repository: aurora-packaging
> 
> 
> Description
> ---
> 
> An operator should be able to build Aurora on his platform without
> relying on Docker.
> 
> Even if this is an interesting way of doing it, it sometimes introduce missed
> (build-)dependencies (in this case openssl) and context-specific behaviour
> (here, the way pants.ini was added to the build context)
> 
> 
> Diffs
> -
> 
>   README.md 3a7cf45034b7896c23588fed83176468ca627ebc 
>   builder/deb/debian-jessie/Dockerfile 
> 63e89deed9a411b1859ba28ea2572ef4a210da1f 
>   builder/deb/debian-jessie/pants.ini 
> 2ff2d5eb6128d4be3e15c0488d8d23bed81a8d5b 
>   builder/deb/ubuntu-trusty/Dockerfile 
> 52739100c9593292a7aea2459412c8e49f0155a4 
>   builder/deb/ubuntu-trusty/pants.ini  
>   builder/rpm/centos-7/Dockerfile a4d9dfcee5637d2c31d0c2b63a9ebcf04e8d 
>   builder/rpm/centos-7/pants.ini  
>   specs/debian/control a30e987e4423c611c86518a8f5e3e68ee996982a 
>   specs/debian/rules 974fddfda6da04686daae691fa32c5b4c11a0b5d 
>   specs/rpm/aurora.spec 61e7d146108ae7dd5e129d8288a05773c2659d25 
> 
> Diff: https://reviews.apache.org/r/45298/diff/
> 
> 
> Testing
> ---
> 
> Able to build on CentOS 6 using mock.
> Able to build on Debian. Limitation = need the patched version of Gradle.
> Still able to build using the current Docker build system
> 
> 
> Thanks,
> 
> Pierre Cheynier
> 
>



Re: Review Request 45104: Use chmod+x to make termos_runner.pex executable

2016-03-25 Thread Pierre Cheynier


> On mars 22, 2016, 1:02 matin, Joshua Cohen wrote:
> > Sorry for the delay on this. After you filed the pull request, I 
> > investigated a bit what will be required once Mesos 0.30.0 lands: 
> > https://issues.apache.org/jira/browse/AURORA-1632. I think the problem goes 
> > beyond the failure to find `sys.executable` when $PATH is not set. As even 
> > after switching back to chmod+x on the runner, the task failed further down 
> > the stack.
> > 
> > I suspect the fix for Mesos 0.30.0 will be to set our own $PATH which 
> > should allow `sys.executable` to continue working and will allow any tasks 
> > users have running which have come to rely on Thermos setting it for them 
> > to behave as expected. The problem is, I haven't had time to figure out 
> > what we should set $PATH to yet ;) (anyone have any thoughts?).
> > 
> > I know this is probably more info than you bargained for when you opened 
> > what seemed like a simple pull request. I'm not opposed to accepting this 
> > patch (with a TODO to restore `sys.executable` when we figure out what to 
> > do about setting $PATH) if it unblocks your use case, but can you confirm 
> > that you're actually able to run the Mesos agent with 
> > `--executor_environment_variables='{}'` and still launch tasks?
> 
> Pierre Cheynier wrote:
> Hi,
> 
> Sorry for my delay on opening this review on apache.org.
> Actually, I started to use Aurora and faced this issue cause my setup use 
> `--executor_environment_variables`.
> I first investigated in the Python default setup on CentOS, the pants/pex 
> build system and then the differences between the vagrant box provided in the 
> repo and mine before discovering that this is the origin of the issue.
> 
> I admit this is a simple fix, I just tried to understand the reason of 
> using this mechanism and found that it was changed 2 years ago.
> In 0.27.2  it works well by using this patch (I'm now able to run 
> something).
> 
> Joshua Cohen wrote:
> So he's the behavior I see when I run the Mesos agent with 
> `--executor_environment_variables='{}'`. Without this patch, as you note, 
> tasks fail to start with a permission denied error (due to `sys.executable` 
> being unset). After applying this patch, tasks start up, but processes fail 
> to fork: 
> https://www.dropbox.com/s/7zzy574xdy02ukq/Screen%20Shot%202016-03-24%20at%203.49.24%20PM.png?dl=0.
> 
> Are you by chance running Docker tasks?

I'm running tasks on the Mesos containerizer for now.
Your problem is not purely a side-effect of not passing any environment 
variables ? (your tasks themselves maybe need PATH or LD_LIBRARY_PATH ?)


- Pierre


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


On mars 22, 2016, 9:50 matin, Pierre Cheynier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45104/
> ---
> 
> (Updated mars 22, 2016, 9:50 matin)
> 
> 
> Review request for Aurora, Joshua Cohen and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> When using `--executor_environment_variables` without explicitely
> passing LD_LIBRARY_PATH, `sys.executable` returns an empty string
> resulting in a '[Errno 13] Permission denied' error for every launched
> task.
> 
> Moreover, it seems that this feature is coming in 0.30: "Executors no
> longer inherit environment variables from the agent".
> 
> This patch partially revert back 07ce21d where chmod_x method was
> removed in favor of using sys.executable.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/executor/thermos_task_runner.py 
> 3896e3841562600379705dbf78a6f62728246348 
> 
> Diff: https://reviews.apache.org/r/45104/diff/
> 
> 
> Testing
> ---
> 
> Make Aurora executor run on CentOS7, while running mesos agent with 
> `--executor_environment_variables` option and no excplicit $PATH set.
> 
> 
> Thanks,
> 
> Pierre Cheynier
> 
>