Re: Review Request 45521: Remove client-side validation of environment names

2016-03-30 Thread Aurora ReviewBot 

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



Master (193f17e) is red with this patch.
  ./build-support/jenkins/build.sh

   
self._clock.converge(threads=[hct.threaded_health_checker])
   
self._clock.assert_waiting(hct.threaded_health_checker, amount=1)
 
   assert hct._total_latency == 0
   assert 
hct.metrics.sample()['total_latency_secs'] == 0
 
   # start the health check (during health check it 
is still 0)
   epsilon = 0.001
   self._clock.tick(1.0 + epsilon)
   
self._clock.converge(threads=[hct.threaded_health_checker])
   
self._clock.assert_waiting(hct.threaded_health_checker, amount=0.5)
   assert hct._total_latency == 0
   assert 
hct.metrics.sample()['total_latency_secs'] == 0
   assert hct.metrics.sample()['checks'] == 0
 
   # finish the health check
   self._clock.tick(0.5 + epsilon)
   
self._clock.converge(threads=[hct.threaded_health_checker])
   
self._clock.assert_waiting(hct.threaded_health_checker, amount=1)  # 
interval_secs
 > assert hct._total_latency == 0.5
 E AssertionError: assert 0.5009 
== 0.5
 E  +  where 0.5009 = 
._total_latency
 
 
src/test/python/apache/aurora/executor/common/test_health_checker.py:174: 
AssertionError
 -- Captured stderr call --
 [] Time now: 0.0
 [] Time now: 0.0
 [] Time now: 1.0
 [] Time now: 1.001
 [] Time now: 1.001
 [] Time now: 1.501
 [] Time now: 1.502
  generated xml file: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/test-results/415337499eb72578eab327a6487c1f5c9452b3d6.xml
 
  2 failed, 667 passed, 5 skipped, 1 warnings in 
209.54 seconds 
 
FAILURE


06:07:16 04:23   [complete]
   FAILURE


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

- Aurora ReviewBot


On March 31, 2016, 5:55 a.m., Benjamin Staffin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45521/
> ---
> 
> (Updated March 31, 2016, 5:55 a.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Bugs: AURORA-319
> https://issues.apache.org/jira/browse/AURORA-319
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This should be done in the scheduler, if anywhere.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/config.py 
> 2fc12559016d406c347adb416a5166cca31c961e 
> 
> Diff: https://reviews.apache.org/r/45521/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benjamin Staffin
> 
>

Re: Review Request 45521: Remove client-side validation of environment names

2016-03-30 Thread Benjamin Staffin 

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

(Updated March 30, 2016, 10:55 p.m.)


Review request for Aurora.


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


Repository: aurora


Description
---

This should be done in the scheduler, if anywhere.


Diffs (updated)
-

  src/main/python/apache/aurora/client/config.py 
2fc12559016d406c347adb416a5166cca31c961e 

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


Testing
---


Thanks,

Benjamin Staffin

Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to detect leading scheduler

2016-03-30 Thread Aurora ReviewBot 

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


Ship it!




Master (193f17e) 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 31, 2016, 5:06 a.m., Ashwin Murthy wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45511/
> ---
> 
> (Updated March 31, 2016, 5:06 a.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The 
> fix is to add a new endpoint - "/leaderhealth" which returns http status code 
> 200 (OK) if the instance is the leader. If the instance is not the leader but 
> a leading exists, returns 500 (Internal server error). If there is no leader 
> at all, returns 503 (Service unavailable)
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java 
> PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45511/diff/
> 
> 
> Testing
> ---
> 
> Added new unit test
> 
> 
> Thanks,
> 
> Ashwin Murthy
> 
>

Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to detect leading scheduler

2016-03-30 Thread Ashwin Murthy 

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

(Updated March 31, 2016, 5:06 a.m.)


Review request for Aurora and Bill Farner.


Repository: aurora


Description
---

AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The fix 
is to add a new endpoint - "/leaderhealth" which returns http status code 200 
(OK) if the instance is the leader. If the instance is not the leader but a 
leading exists, returns 500 (Internal server error). If there is no leader at 
all, returns 503 (Service unavailable)


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java 
PRE-CREATION 

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


Testing
---

Added new unit test


Thanks,

Ashwin Murthy

Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to detect leading scheduler

2016-03-30 Thread Aurora ReviewBot 

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



Master (193f17e) is red with this patch.
  ./build-support/jenkins/build.sh

:jar
:startScripts
:distTar
:distZip
:assemble
:compileJmhJavaNote: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/jmh/java/org/apache/aurora/benchmark/fakes/FakeSchedulerDriver.java
 uses or overrides a deprecated API.
Note: Recompile with -Xlint:deprecation for details.

:processJmhResources UP-TO-DATE
:jmhClasses
:checkstyleJmh
:jsHint
:checkstyleMain
:compileTestJava/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/thrift/aop/MockDecoratedThrift.java:38:
 Note: Wrote forwarder 
org.apache.aurora.scheduler.thrift.aop.MockDecoratedThriftForwarder
@Forward(AnnotatedAuroraAdmin.class)
^
Note: Some input files use or override a deprecated API.
Note: Recompile with -Xlint:deprecation for details.

:processTestResources
:testClasses
:checkstyleTest[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java:22:
 error: Wrong order for 'java.util.Arrays' import. Order should be: java, 
javax, scala, com, net, org. Each group should be separated by a single 
blank line.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java:33:
 error: File contains a sequence of empty lines.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java:41:51:
 error: '{' is not preceded with whitespace.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java:44:
 error: 'lambda arguments' have incorrect indentation level 8, expected level 
should be 6.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java:65:
 error: 'method def rcurly' have incorrect indentation level 0, expected level 
should be 2.
 FAILED

FAILURE: Build failed with an exception.

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

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

BUILD FAILED

Total time: 2 mins 39.143 secs


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

- Aurora ReviewBot


On March 31, 2016, 3:36 a.m., Ashwin Murthy wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45511/
> ---
> 
> (Updated March 31, 2016, 3:36 a.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The 
> fix is to add a new endpoint - "/leaderhealth" which returns http status code 
> 200 (OK) if the instance is the leader. If the instance is not the leader but 
> a leading exists, returns 500 (Internal server error). If there is no leader 
> at all, returns 503 (Service unavailable)
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java 
> PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45511/diff/
> 
> 
> Testing
> ---
> 
> Added new unit test
> 
> 
> Thanks,
> 
> Ashwin Murthy
> 
>

Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to detect leading scheduler

2016-03-30 Thread Ashwin Murthy 

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

(Updated March 31, 2016, 3:36 a.m.)


Review request for Aurora and Bill Farner.


Repository: aurora


Description
---

AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The fix 
is to add a new endpoint - "/leaderhealth" which returns http status code 200 
(OK) if the instance is the leader. If the instance is not the leader but a 
leading exists, returns 500 (Internal server error). If there is no leader at 
all, returns 503 (Service unavailable)


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java 
PRE-CREATION 

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


Testing
---

Added new unit test


Thanks,

Ashwin Murthy

Re: Review Request 45521: Remove client-side validation of environment names

2016-03-30 Thread Aurora ReviewBot 

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



Master (193f17e) is red with this patch.
  ./build-support/jenkins/build.sh

03:06:08 00:01   [imports]
03:06:08 00:01 [ivy-imports]
03:06:08 00:01   [unpack-jars]
03:06:08 00:01 [unpack-jars]
03:06:08 00:01   [deferred-sources]
03:06:08 00:01 [deferred-sources]
03:06:08 00:01   [gen]
03:06:08 00:01 [thrift]
03:06:08 00:01 [protoc]
03:06:08 00:01 [antlr]
03:06:08 00:01 [ragel]
03:06:08 00:01 [jaxb]
03:06:08 00:01 [wire]
03:06:08 00:01   [jvm-platform-validate]
03:06:08 00:01 [jvm-platform-validate]
03:06:08 00:01   [resolve]
03:06:08 00:01 [ivy]
03:06:08 00:01   [bootstrap-nailgun-server]
03:06:08 00:01   [compile]
03:06:08 00:01 [compile-jvm-prep-command]
03:06:08 00:01   [jvm_prep_command]
03:06:08 00:01 [compile-prep-command]
03:06:08 00:01 [python-eval]
03:06:08 00:01 [pythonstyle]
03:06:09 00:02   [cache]  
   No cached artifacts for 42 targets.
   Invalidated 42 targets.
F821:ERROR   src/main/python/apache/aurora/common/aurora_job_key.py:031 
undefined name 're'
 |  VALID_IDENTIFIER = re.compile(GOOD_IDENTIFIER_PATTERN_PYTHON)


F401:ERROR   src/main/python/apache/aurora/client/config.py:022 're' imported 
but unused
 |import re


FAILURE: Python Style issues found


03:06:25 00:18   [complete]
   FAILURE


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

- Aurora ReviewBot


On March 31, 2016, 2:58 a.m., Benjamin Staffin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45521/
> ---
> 
> (Updated March 31, 2016, 2:58 a.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Bugs: AURORA-319
> https://issues.apache.org/jira/browse/AURORA-319
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This should be done in the scheduler, if anywhere.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/config.py 
> 2fc12559016d406c347adb416a5166cca31c961e 
>   src/main/python/apache/aurora/common/aurora_job_key.py 
> 21cdca6074c6fa58d68a0818805144662044841f 
> 
> Diff: https://reviews.apache.org/r/45521/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benjamin Staffin
> 
>

Re: Review Request 45521: Remove client-side validation of environment names

2016-03-30 Thread Benjamin Staffin 

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

(Updated March 30, 2016, 7:58 p.m.)


Review request for Aurora.


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


Repository: aurora


Description
---

This should be done in the scheduler, if anywhere.


Diffs (updated)
-

  src/main/python/apache/aurora/client/config.py 
2fc12559016d406c347adb416a5166cca31c961e 
  src/main/python/apache/aurora/common/aurora_job_key.py 
21cdca6074c6fa58d68a0818805144662044841f 

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


Testing
---


Thanks,

Benjamin Staffin

Re: Review Request 45521: Remove client-side validation of environment names

2016-03-30 Thread Aurora ReviewBot 

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



Master (193f17e) is red with this patch.
  ./build-support/jenkins/build.sh

   Executing tasks in goals: bootstrap -> imports -> unpack-jars -> 
deferred-sources -> gen -> jvm-platform-validate -> resolve -> compile -> 
resources -> test
02:47:45 00:00   [bootstrap]
02:47:45 00:00 [jar-dependency-management]
02:47:45 00:00 [bootstrap-jvm-tools]
02:47:45 00:00   [imports]
02:47:45 00:00 [ivy-imports]
02:47:45 00:00   [unpack-jars]
02:47:45 00:00 [unpack-jars]
02:47:45 00:00   [deferred-sources]
02:47:45 00:00 [deferred-sources]
02:47:45 00:00   [gen]
02:47:45 00:00 [thrift]
02:47:45 00:00 [protoc]
02:47:45 00:00 [antlr]
02:47:45 00:00 [ragel]
02:47:45 00:00 [jaxb]
02:47:45 00:00 [wire]
02:47:45 00:00   [jvm-platform-validate]
02:47:45 00:00 [jvm-platform-validate]
02:47:45 00:00   [resolve]
02:47:45 00:00 [ivy]
02:47:45 00:00   [bootstrap-nailgun-server]
02:47:46 00:01   [compile]
02:47:46 00:01 [compile-jvm-prep-command]
02:47:46 00:01   [jvm_prep_command]
02:47:46 00:01 [compile-prep-command]
02:47:46 00:01 [python-eval]
02:47:46 00:01 [pythonstyle]
02:47:46 00:01   [cache]  
   No cached artifacts for 42 targets.
   Invalidated 42 targets.
F401:ERROR   src/main/python/apache/aurora/client/config.py:022 're' imported 
but unused
 |import re


FAILURE: Python Style issues found


02:48:02 00:17   [complete]
   FAILURE


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

- Aurora ReviewBot


On March 31, 2016, 2:34 a.m., Benjamin Staffin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45521/
> ---
> 
> (Updated March 31, 2016, 2:34 a.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Bugs: AURORA-319
> https://issues.apache.org/jira/browse/AURORA-319
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This should be done in the scheduler, if anywhere.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/config.py 
> 2fc12559016d406c347adb416a5166cca31c961e 
> 
> Diff: https://reviews.apache.org/r/45521/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benjamin Staffin
> 
>

Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to detect leading scheduler

2016-03-30 Thread Aurora ReviewBot 

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



Master (193f17e) is red with this patch.
  ./build-support/jenkins/build.sh


:generateBuildProperties
:processResources
:classes
:jar
:startScripts
:distTar
:distZip
:assemble
:compileJmhJavaNote: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/jmh/java/org/apache/aurora/benchmark/fakes/FakeSchedulerDriver.java
 uses or overrides a deprecated API.
Note: Recompile with -Xlint:deprecation for details.

:processJmhResources UP-TO-DATE
:jmhClasses
:checkstyleJmh
:jsHint
:checkstyleMain[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:29:
 error: 'class def modifier' have incorrect indentation level 2, expected level 
should be 0.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:30:
 error: 'class def modifier' have incorrect indentation level 2, expected level 
should be 0.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:32:
 error: 'member def modifier' have incorrect indentation level 4, expected 
level should be 2.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:34:
 error: 'ctor def modifier' have incorrect indentation level 4, expected level 
should be 2.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:36:
 error: 'ctor def' child have incorrect indentation level 6, expected level 
should be 4.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:37:
 error: 'ctor def rcurly' have incorrect indentation level 4, expected level 
should be 2.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:49:7:
 error: 'if' is not followed by whitespace.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:51:5:
 error: '}' at column 5 should be on the same line as the next part of a 
multi-block statement.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:52:12:
 error: 'if' is not followed by whitespace.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:54:5:
 error: '}' at column 5 should be on the same line as the next part of a 
multi-block statement.
 FAILED

FAILURE: Build failed with an exception.

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

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

BUILD FAILED

Total time: 1 mins 36.345 secs


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

- Aurora ReviewBot


On March 31, 2016, 2:35 a.m., Ashwin Murthy wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45511/
> ---
> 
> (Updated March 31, 2016, 2:35 a.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The 
> fix is to add a new endpoint - "/leaderhealth" which returns http status code 
> 200 (OK) if the instance is the leader. If the instance is not the leader but 
> a leading exists, returns 500 (Internal server error). If there is no leader 
> at all, returns 503 (Service unavailable)
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java 
> PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45511/diff/
> 
> 
> Testing
> ---
> 
> Added new unit test
> 
> 
> Thanks,
> 
> Ashwin Murthy
> 
>

Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to detect leading scheduler

2016-03-30 Thread Ashwin Murthy 


> On March 30, 2016, 11:30 p.m., Ashwin Murthy wrote:
> >

LeaderHealth

Element Missed Instructions Cov.Missed Branches Cov.Missed  Cxty
Missed  Lines   Missed  Methods
Total   0 of 30 100%0 of 4  100%0   4   0   9   0   
2
get()   22  100%4   100%0   3   0   6   0   
1
LeaderHealth(LeaderRedirect)8   100%n/a 0   1   
0   3   0   1


Code coverage is 100% for the new code


- Ashwin


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


On March 31, 2016, 2:35 a.m., Ashwin Murthy wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45511/
> ---
> 
> (Updated March 31, 2016, 2:35 a.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The 
> fix is to add a new endpoint - "/leaderhealth" which returns http status code 
> 200 (OK) if the instance is the leader. If the instance is not the leader but 
> a leading exists, returns 500 (Internal server error). If there is no leader 
> at all, returns 503 (Service unavailable)
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java 
> PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45511/diff/
> 
> 
> Testing
> ---
> 
> Added new unit test
> 
> 
> Thanks,
> 
> Ashwin Murthy
> 
>

Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to detect leading scheduler

2016-03-30 Thread Ashwin Murthy 

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

(Updated March 31, 2016, 2:35 a.m.)


Review request for Aurora and Bill Farner.


Repository: aurora


Description
---

AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The fix 
is to add a new endpoint - "/leaderhealth" which returns http status code 200 
(OK) if the instance is the leader. If the instance is not the leader but a 
leading exists, returns 500 (Internal server error). If there is no leader at 
all, returns 503 (Service unavailable)


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java 
PRE-CREATION 

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


Testing
---

Added new unit test


Thanks,

Ashwin Murthy

Review Request 45521: Remove client-side validation of environment names

2016-03-30 Thread Benjamin Staffin 

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

Review request for Aurora.


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


Repository: aurora


Description
---

This should be done in the scheduler, if anywhere.


Diffs
-

  src/main/python/apache/aurora/client/config.py 
2fc12559016d406c347adb416a5166cca31c961e 

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


Testing
---


Thanks,

Benjamin Staffin

Re: Review Request 45506: Execute shell-based health checks as the task user.

2016-03-30 Thread Aurora ReviewBot 

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


Ship it!




Master (193f17e) 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 30, 2016, 11:41 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45506/
> ---
> 
> (Updated March 30, 2016, 11:41 p.m.)
> 
> 
> Review request for Aurora, Dmitriy Shirchenko and Zameer Manji.
> 
> 
> Bugs: AURORA-1641
> https://issues.apache.org/jira/browse/AURORA-1641
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Here's a stab at this using `os` and `pwd` modules directly to demote health 
> checks to the target user.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/common/health_check/shell.py 
> 6cb7dfc164f4e16143fc974d50c19a5887d32015 
>   src/main/python/apache/aurora/executor/common/health_checker.py 
> 28fd3ec3ef7d0b66621c0295804af0eb72c64b4a 
>   src/test/python/apache/aurora/common/health_check/test_shell.py 
> 7026af8c4671a40f4b517ecf12149eac34a552c8 
>   src/test/python/apache/aurora/executor/common/test_health_checker.py 
> 19c4f76347e34374c29974c182d1f4c118bcb18d 
> 
> Diff: https://reviews.apache.org/r/45506/diff/
> 
> 
> Testing
> ---
> 
> I haven't spent any time thinking of a test strategy for this, but i don't 
> think we should proceed without end-to-end validation.  I'm open to ideas 
> here.
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 45506: Execute shell-based health checks as the task user.

2016-03-30 Thread Bill Farner 

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

(Updated March 30, 2016, 4:41 p.m.)


Review request for Aurora, Dmitriy Shirchenko and Zameer Manji.


Changes
---

green build, moved uid/gid awareness up a level from `ShellHealthCheck`.


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


Repository: aurora


Description
---

Here's a stab at this using `os` and `pwd` modules directly to demote health 
checks to the target user.


Diffs (updated)
-

  src/main/python/apache/aurora/common/health_check/shell.py 
6cb7dfc164f4e16143fc974d50c19a5887d32015 
  src/main/python/apache/aurora/executor/common/health_checker.py 
28fd3ec3ef7d0b66621c0295804af0eb72c64b4a 
  src/test/python/apache/aurora/common/health_check/test_shell.py 
7026af8c4671a40f4b517ecf12149eac34a552c8 
  src/test/python/apache/aurora/executor/common/test_health_checker.py 
19c4f76347e34374c29974c182d1f4c118bcb18d 

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


Testing
---

I haven't spent any time thinking of a test strategy for this, but i don't 
think we should proceed without end-to-end validation.  I'm open to ideas here.


Thanks,

Bill Farner

Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to detect leading scheduler

2016-03-30 Thread Ashwin Murthy 

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




src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java (line 25)


Need to fix this comment. It should be 200, 500 or 503. Will udpate in next 
rev


- Ashwin Murthy


On March 30, 2016, 11:19 p.m., Ashwin Murthy wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45511/
> ---
> 
> (Updated March 30, 2016, 11:19 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The 
> fix is to add a new endpoint - "/leaderhealth" which returns http status code 
> 200 (OK) if the instance is the leader. If the instance is not the leader but 
> a leading exists, returns 500 (Internal server error). If there is no leader 
> at all, returns 503 (Service unavailable)
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java 
> PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45511/diff/
> 
> 
> Testing
> ---
> 
> Added new unit test
> 
> 
> Thanks,
> 
> Ashwin Murthy
> 
>

Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to detect leading scheduler

2016-03-30 Thread Aurora ReviewBot 

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



Master (193f17e) is red with this patch.
  ./build-support/jenkins/build.sh

:checkstyleJmh
:jsHint
:checkstyleMain[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:22:
 error: 'org.apache.aurora.scheduler.http.LeaderRedirect.LeaderStatus' should 
be separated from previous imports.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:25:
 error: Line is longer than 100 characters (found 163).
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:30:
 error: 'member def modifier' have incorrect indentation level 4, expected 
level should be 2.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:32:
 error: 'ctor def modifier' have incorrect indentation level 4, expected level 
should be 2.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:34:
 error: 'ctor def' child have incorrect indentation level 8, expected level 
should be 4.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:35:
 error: 'ctor def rcurly' have incorrect indentation level 4, expected level 
should be 2.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:39:
 error: Line is longer than 100 characters (found 111).
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:44:
 error: 'method def modifier' have incorrect indentation level 4, expected 
level should be 2.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:47:
 error: 'member def type' have incorrect indentation level 8, expected level 
should be 4.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:47:
 error: 'method def' child have incorrect indentation level 8, expected level 
should be 4.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:49:
 error: 'if' have incorrect indentation level 8, expected level should be 4.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:49:11:
 error: 'if' is not followed by whitespace.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:50:
 error: 'if' child have incorrect indentation level 12, expected level should 
be 6.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:51:
 error: 'if rcurly' have incorrect indentation level 8, expected level should 
be 4.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:51:9:
 error: '}' at column 9 should be on the same line as the next part of a 
multi-block statement.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:52:
 error: 'else' have incorrect indentation level 8, expected level should be 4.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:52:16:
 error: 'if' is not followed by whitespace.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:53:
 error: 'if' child have incorrect indentation level 12, expected level should 
be 6.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:54:
 error: 'if rcurly' have incorrect indentation level 8, expected level should 
be 4.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:54:9:
 error: '}' at column 9 should be on the same line as the next part of a 
multi-block statement.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:55:
 error: 'else' have incorrect indentation level 8, expected level should be 4.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/

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

2016-03-30 Thread Aurora ReviewBot 

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



Master (55a2422) is red with this patch.
  ./build-support/jenkins/build.sh

   proxy_driver = ProxyDriver()
   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/user/10021/tmpjUDijX', 
'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.75/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
 
  17 failed, 644 passed, 5 skipped, 1 warnings, 8 
error in 252.03 seconds 
 
FAILURE


23:21:11 05:37   [complete]
   FAILURE


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

- Aurora ReviewBot


On March 30, 2016, 11 p.m., Amol Deshmukh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45222/
> ---
> 
> (Updated March 30, 2016, 11 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
> 
> 
> 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
> -
> 
>   RELEASE-NOTES.md 4eebc9df16c7222fe2c40df2237f77c5d4e76e3a 
>   docs/reference/configuration.md c9459fe9d655ac0668af87b62dd5e8258bdb0f35 
>   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 
> 48

Review Request 45511: AURORA-1493: create ELB-friendly endpoint to detect leading scheduler

2016-03-30 Thread Ashwin Murthy 

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

Review request for Aurora and Bill Farner.


Repository: aurora


Description
---

AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The fix 
is to add a new endpoint - "/leaderhealth" which returns http status code 200 
(OK) if the instance is the leader. If the instance is not the leader but a 
leading exists, returns 500 (Internal server error). If there is no leader at 
all, returns 503 (Service unavailable)


Diffs
-

  src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java 
PRE-CREATION 

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


Testing
---

Added new unit test


Thanks,

Ashwin Murthy

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

2016-03-30 Thread Amol Deshmukh 

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

(Updated March 30, 2016, 4 p.m.)


Review request for Aurora, Joshua Cohen and Maxim Khutornenko.


Changes
---

- Merged remote HEAD and resolved conflict.


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 (updated)
-

  RELEASE-NOTES.md 4eebc9df16c7222fe2c40df2237f77c5d4e76e3a 
  docs/reference/configuration.md c9459fe9d655ac0668af87b62dd5e8258bdb0f35 
  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
 9d2bc825edef7fabeccd2334db48acc1bf622eb0 
  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 45506: Execute shell-based health checks as the task user.

2016-03-30 Thread Bill Farner 


> On March 30, 2016, 3:07 p.m., John Sirois wrote:
> > There is code to do this in apache.thermos.core.process.Process and its 
> > tested here: 
> > https://github.com/apache/aurora/blob/master/src/test/python/apache/thermos/core/test_process.py#L103
> > Process (ProcessBase) does look a bit fat for this, but it would be nice if 
> > there was eventually 1 tested piece of code for executing an un-priviledged 
> > process in the task sandbox.

I agree that it would be preferable.  I admit i only did a quick pass through 
`Process`, but backed away because it does not support a timeout, and wants to 
write several files to disk (including the checkpoint stream).  I could 
certainly retrofit it to do that, but it was hard to justify.


- Bill


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


On March 30, 2016, 2:58 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45506/
> ---
> 
> (Updated March 30, 2016, 2:58 p.m.)
> 
> 
> Review request for Aurora, Dmitriy Shirchenko and Zameer Manji.
> 
> 
> Bugs: AURORA-1641
> https://issues.apache.org/jira/browse/AURORA-1641
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Here's a stab at this using `os` and `pwd` modules directly to demote health 
> checks to the target user.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/common/health_check/shell.py 
> 6cb7dfc164f4e16143fc974d50c19a5887d32015 
>   src/main/python/apache/aurora/executor/common/health_checker.py 
> 28fd3ec3ef7d0b66621c0295804af0eb72c64b4a 
>   src/test/python/apache/aurora/executor/common/test_health_checker.py 
> 19c4f76347e34374c29974c182d1f4c118bcb18d 
> 
> Diff: https://reviews.apache.org/r/45506/diff/
> 
> 
> Testing
> ---
> 
> I haven't spent any time thinking of a test strategy for this, but i don't 
> think we should proceed without end-to-end validation.  I'm open to ideas 
> here.
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 45506: Execute shell-based health checks as the task user.

2016-03-30 Thread Aurora ReviewBot 

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



Master (55a2422) is red with this patch.
  ./build-support/jenkins/build.sh

virtualenv-14.0.6/virtualenv_embedded/activate.sh
virtualenv-14.0.6/virtualenv_embedded/activate_this.py
virtualenv-14.0.6/virtualenv_embedded/deactivate.bat
virtualenv-14.0.6/virtualenv_embedded/distutils-init.py
virtualenv-14.0.6/virtualenv_embedded/distutils.cfg
virtualenv-14.0.6/virtualenv_embedded/python-config
virtualenv-14.0.6/virtualenv_embedded/site.py
virtualenv-14.0.6/virtualenv_support/
virtualenv-14.0.6/virtualenv_support/__init__.py
virtualenv-14.0.6/virtualenv_support/argparse-1.4.0-py2.py3-none-any.whl
virtualenv-14.0.6/virtualenv_support/pip-8.0.2-py2.py3-none-any.whl
virtualenv-14.0.6/virtualenv_support/setuptools-20.0-py2.py3-none-any.whl
virtualenv-14.0.6/virtualenv_support/wheel-0.29.0-py2.py3-none-any.whl
+ touch virtualenv-14.0.6/BOOTSTRAPPED
+ popd
~/jenkins-slave/workspace/AuroraBot
+ exec /usr/bin/python2.7 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/virtualenv-14.0.6/virtualenv.py
 /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/isort.venv
New python executable in 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/isort.venv/bin/python2.7
Also creating executable in 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/isort.venv/bin/python
Installing setuptools, pip, wheel...done.
Collecting isort==4.0.0
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/isort.venv/local/lib/python2.7/site-packages/pip/_vendor/requests/packages/urllib3/util/ssl_.py:315:
 SNIMissingWarning: An HTTPS request has been made, but the SNI (Subject Name 
Indication) extension to TLS is not available on this platform. This may cause 
the server to present an incorrect TLS certificate, which can cause validation 
failures. For more information, see 
https://urllib3.readthedocs.org/en/latest/security.html#snimissingwarning.
  SNIMissingWarning
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/isort.venv/local/lib/python2.7/site-packages/pip/_vendor/requests/packages/urllib3/util/ssl_.py:120:
 InsecurePlatformWarning: A true SSLContext object is not available. This 
prevents urllib3 from configuring SSL appropriately and may cause certain SSL 
connections to fail. For more information, see 
https://urllib3.readthedocs.org/en/latest/security.html#insecureplatformwarning.
  InsecurePlatformWarning
  Using cached isort-4.0.0-py2.py3-none-any.whl
Installing collected packages: isort
Successfully installed isort-4.0.0
ERROR: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/executor/common/test_health_checker.py
 Imports are incorrectly sorted.
--- 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/executor/common/test_health_checker.py:before
 2016-03-30 22:01:12.247552
+++ 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/executor/common/test_health_checker.py:after
  2016-03-30 22:13:40.741774
@@ -39,7 +39,7 @@
 
 from .fixtures import HELLO_WORLD, MESOS_JOB
 
-from gen.apache.aurora.api.ttypes import AssignedTask, ExecutorConfig, 
TaskConfig, JobKey
+from gen.apache.aurora.api.ttypes import AssignedTask, ExecutorConfig, JobKey, 
TaskConfig
 
 
 class TestHealthChecker(unittest.TestCase):


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

- Aurora ReviewBot


On March 30, 2016, 9:58 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45506/
> ---
> 
> (Updated March 30, 2016, 9:58 p.m.)
> 
> 
> Review request for Aurora, Dmitriy Shirchenko and Zameer Manji.
> 
> 
> Bugs: AURORA-1641
> https://issues.apache.org/jira/browse/AURORA-1641
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Here's a stab at this using `os` and `pwd` modules directly to demote health 
> checks to the target user.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/common/health_check/shell.py 
> 6cb7dfc164f4e16143fc974d50c19a5887d32015 
>   src/main/python/apache/aurora/executor/common/health_checker.py 
> 28fd3ec3ef7d0b66621c0295804af0eb72c64b4a 
>   src/test/python/apache/aurora/executor/common/test_health_checker.py 
> 19c4f76347e34374c29974c182d1f4c118bcb18d 
> 
> Diff: https://reviews.apache.org/r/45506/diff/
> 
> 
> Testing
> ---
> 
> I haven't spent any time thinking of a test strategy for this, but i don't 
> think we should proceed without end-to-end validation.  I'm open to ideas 
> here.
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 45506: Execute shell-based health checks as the task user.

2016-03-30 Thread John Sirois 

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



There is code to do this in apache.thermos.core.process.Process and its tested 
here: 
https://github.com/apache/aurora/blob/master/src/test/python/apache/thermos/core/test_process.py#L103
Process (ProcessBase) does look a bit fat for this, but it would be nice if 
there was eventually 1 tested piece of code for executing an un-priviledged 
process in the task sandbox.

- John Sirois


On March 30, 2016, 3:58 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45506/
> ---
> 
> (Updated March 30, 2016, 3:58 p.m.)
> 
> 
> Review request for Aurora, Dmitriy Shirchenko and Zameer Manji.
> 
> 
> Bugs: AURORA-1641
> https://issues.apache.org/jira/browse/AURORA-1641
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Here's a stab at this using `os` and `pwd` modules directly to demote health 
> checks to the target user.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/common/health_check/shell.py 
> 6cb7dfc164f4e16143fc974d50c19a5887d32015 
>   src/main/python/apache/aurora/executor/common/health_checker.py 
> 28fd3ec3ef7d0b66621c0295804af0eb72c64b4a 
>   src/test/python/apache/aurora/executor/common/test_health_checker.py 
> 19c4f76347e34374c29974c182d1f4c118bcb18d 
> 
> Diff: https://reviews.apache.org/r/45506/diff/
> 
> 
> Testing
> ---
> 
> I haven't spent any time thinking of a test strategy for this, but i don't 
> think we should proceed without end-to-end validation.  I'm open to ideas 
> here.
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Review Request 45506: Execute shell-based health checks as the task user.

2016-03-30 Thread Bill Farner 

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

Review request for Aurora, Dmitriy Shirchenko and Zameer Manji.


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


Repository: aurora


Description
---

Here's a stab at this using `os` and `pwd` modules directly to demote health 
checks to the target user.


Diffs
-

  src/main/python/apache/aurora/common/health_check/shell.py 
6cb7dfc164f4e16143fc974d50c19a5887d32015 
  src/main/python/apache/aurora/executor/common/health_checker.py 
28fd3ec3ef7d0b66621c0295804af0eb72c64b4a 
  src/test/python/apache/aurora/executor/common/test_health_checker.py 
19c4f76347e34374c29974c182d1f4c118bcb18d 

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


Testing
---

I haven't spent any time thinking of a test strategy for this, but i don't 
think we should proceed without end-to-end validation.  I'm open to ideas here.


Thanks,

Bill Farner

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

2016-03-30 Thread Joshua Cohen 


> On March 30, 2016, 8:47 p.m., Joshua Cohen wrote:
> > Ship It!

Can you rebase and fix the conflicts in RELEASE-NOTES.md? Then I'll commit this.


- Joshua


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


On March 29, 2016, 5:51 p.m., Amol Deshmukh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45222/
> ---
> 
> (Updated March 29, 2016, 5:51 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
> 
> 
> 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
> -
> 
>   RELEASE-NOTES.md 3a83961f5133fc0021b7c61f1117488703a9849d 
>   docs/reference/configuration.md fa09fd47b7637c035a5a23aaf6e3bc1c2c7fc143 
>   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
>  9d2bc825edef7fabeccd2334db48acc1bf622eb0 
>   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.a

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

2016-03-30 Thread Joshua Cohen 

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


Ship it!




Ship It!

- Joshua Cohen


On March 29, 2016, 5:51 p.m., Amol Deshmukh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45222/
> ---
> 
> (Updated March 29, 2016, 5:51 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
> 
> 
> 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
> -
> 
>   RELEASE-NOTES.md 3a83961f5133fc0021b7c61f1117488703a9849d 
>   docs/reference/configuration.md fa09fd47b7637c035a5a23aaf6e3bc1c2c7fc143 
>   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
>  9d2bc825edef7fabeccd2334db48acc1bf622eb0 
>   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.
> ```

Re: Review Request 45042: Add ACL support for announcer

2016-03-30 Thread Bill Farner 

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


Ship it!




Thanks for sticking it out through the review, nice patch!

- Bill Farner


On March 30, 2016, 12:50 p.m., Kunal Thakar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45042/
> ---
> 
> (Updated March 30, 2016, 12:50 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add ACL support for announcer
> https://issues.apache.org/jira/browse/AURORA-1643
> 
> Adding support for service discovery ZK authentication. ZK authentication 
> secrets should be stored in a file as json (as follows):
> (Updated JSON format for config file)
> ```json
> {
>   "auth": [
> {
>   "scheme": "",
>   "credential": ""
> }
>   ],
>   "acls": [
> {
>   "scheme": "",
>   "credential": "",
>   "permissions": {
> "read": ,
> "write": ,
> "create": ,
> "delete": ,
> "admin": ,
> "all": 
>   }
> }
>   ]
> }
> ```
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 3a83961f5133fc0021b7c61f1117488703a9849d 
>   docs/operations/security.md 1a3d9b7e7ba4ec1952dc886d5fbeb6b85d994fb9 
>   examples/vagrant/announcer-auth.json PRE-CREATION 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 120b89a1dc10a259940cb9527eb2517f19d04471 
>   src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
> 6634506108c346f8c23b2da7cc8d20d09d07d590 
>   src/main/python/apache/aurora/executor/common/announcer.py 
> 79a9cfb6ac3a8444f09fb3658e6e859e06941ba4 
>   src/main/python/apache/aurora/executor/common/announcer_zkauth_schema.py 
> PRE-CREATION 
>   
> src/test/python/apache/aurora/executor/bin/test_thermos_executor_entry_point.py
>  e9f7851292aef3a36da5da9b0fc333a7e7750cf3 
>   src/test/python/apache/aurora/executor/common/test_announcer.py 
> 142b58d5e577c9f4b8e2ae8473cffdea94eba21f 
> 
> Diff: https://reviews.apache.org/r/45042/diff/
> 
> 
> Testing
> ---
> 
> /vagrant/src/test/sh/org/apache/aurora/e2e/test_announcer_auth_end_to_end.sh
> /vagrant/src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Kunal Thakar
> 
>

Re: Review Request 45042: Add ACL support for announcer

2016-03-30 Thread Aurora ReviewBot 

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


Ship it!




Master (bc4649e) 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 30, 2016, 7:50 p.m., Kunal Thakar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45042/
> ---
> 
> (Updated March 30, 2016, 7:50 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add ACL support for announcer
> https://issues.apache.org/jira/browse/AURORA-1643
> 
> Adding support for service discovery ZK authentication. ZK authentication 
> secrets should be stored in a file as json (as follows):
> (Updated JSON format for config file)
> ```json
> {
>   "auth": [
> {
>   "scheme": "",
>   "credential": ""
> }
>   ],
>   "acls": [
> {
>   "scheme": "",
>   "credential": "",
>   "permissions": {
> "read": ,
> "write": ,
> "create": ,
> "delete": ,
> "admin": ,
> "all": 
>   }
> }
>   ]
> }
> ```
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 3a83961f5133fc0021b7c61f1117488703a9849d 
>   docs/operations/security.md 1a3d9b7e7ba4ec1952dc886d5fbeb6b85d994fb9 
>   examples/vagrant/announcer-auth.json PRE-CREATION 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 120b89a1dc10a259940cb9527eb2517f19d04471 
>   src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
> 6634506108c346f8c23b2da7cc8d20d09d07d590 
>   src/main/python/apache/aurora/executor/common/announcer.py 
> 79a9cfb6ac3a8444f09fb3658e6e859e06941ba4 
>   src/main/python/apache/aurora/executor/common/announcer_zkauth_schema.py 
> PRE-CREATION 
>   
> src/test/python/apache/aurora/executor/bin/test_thermos_executor_entry_point.py
>  e9f7851292aef3a36da5da9b0fc333a7e7750cf3 
>   src/test/python/apache/aurora/executor/common/test_announcer.py 
> 142b58d5e577c9f4b8e2ae8473cffdea94eba21f 
> 
> Diff: https://reviews.apache.org/r/45042/diff/
> 
> 
> Testing
> ---
> 
> /vagrant/src/test/sh/org/apache/aurora/e2e/test_announcer_auth_end_to_end.sh
> /vagrant/src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Kunal Thakar
> 
>

Re: Review Request 45042: Add ACL support for announcer

2016-03-30 Thread Kunal Thakar 

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

(Updated March 30, 2016, 7:50 p.m.)


Review request for Aurora, Bill Farner and Zameer Manji.


Changes
---

Updated documenation and announcer-auth.json


Repository: aurora


Description
---

Add ACL support for announcer
https://issues.apache.org/jira/browse/AURORA-1643

Adding support for service discovery ZK authentication. ZK authentication 
secrets should be stored in a file as json (as follows):
(Updated JSON format for config file)
```json
{
  "auth": [
{
  "scheme": "",
  "credential": ""
}
  ],
  "acls": [
{
  "scheme": "",
  "credential": "",
  "permissions": {
"read": ,
"write": ,
"create": ,
"delete": ,
"admin": ,
"all": 
  }
}
  ]
}
```


Diffs (updated)
-

  RELEASE-NOTES.md 3a83961f5133fc0021b7c61f1117488703a9849d 
  docs/operations/security.md 1a3d9b7e7ba4ec1952dc886d5fbeb6b85d994fb9 
  examples/vagrant/announcer-auth.json PRE-CREATION 
  examples/vagrant/upstart/aurora-scheduler.conf 
120b89a1dc10a259940cb9527eb2517f19d04471 
  src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
6634506108c346f8c23b2da7cc8d20d09d07d590 
  src/main/python/apache/aurora/executor/common/announcer.py 
79a9cfb6ac3a8444f09fb3658e6e859e06941ba4 
  src/main/python/apache/aurora/executor/common/announcer_zkauth_schema.py 
PRE-CREATION 
  
src/test/python/apache/aurora/executor/bin/test_thermos_executor_entry_point.py 
e9f7851292aef3a36da5da9b0fc333a7e7750cf3 
  src/test/python/apache/aurora/executor/common/test_announcer.py 
142b58d5e577c9f4b8e2ae8473cffdea94eba21f 

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


Testing
---

/vagrant/src/test/sh/org/apache/aurora/e2e/test_announcer_auth_end_to_end.sh
/vagrant/src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh


Thanks,

Kunal Thakar

Re: Review Request 45042: Add ACL support for announcer

2016-03-30 Thread Kunal Thakar 


> On March 28, 2016, 11:46 p.m., Bill Farner wrote:
> > examples/vagrant/announcer-auth.json, line 11
> > 
> >
> > I now have to backpedal on my advice to store the encrypted credentials 
> > here.  Since our hand is forced to store plaintext for the auth section, we 
> > might as well make this part plaintext too.  That leaves us with the burden 
> > of handling the digest step, but that shouldn't be too bad.
> 
> Kunal Thakar wrote:
> I'd prefer to keep the burden on the configuration provider to keep it 
> simple.
> 
> Bill Farner wrote:
> I'm still a -1 to that, but willing to be out-voted by Zameer.
> 
> In my opinion, requiring the user to configure the same data (passwords) 
> in 2 different ways (encrypted and plaintext) introduces unnecessary burden 
> and a class of misconfiguration that mere mortals should not be subjected to 
> :-)

Okay. I have special cased 'digest' scheme to generate the credential.


- Kunal


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


On March 30, 2016, 12:17 a.m., Kunal Thakar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45042/
> ---
> 
> (Updated March 30, 2016, 12:17 a.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add ACL support for announcer
> https://issues.apache.org/jira/browse/AURORA-1643
> 
> Adding support for service discovery ZK authentication. ZK authentication 
> secrets should be stored in a file as json (as follows):
> (Updated JSON format for config file)
> ```json
> {
>   "auth": [
> {
>   "scheme": "",
>   "credential": ""
> }
>   ],
>   "acls": [
> {
>   "scheme": "",
>   "credential": "",
>   "permissions": {
> "read": ,
> "write": ,
> "create": ,
> "delete": ,
> "admin": ,
> "all": 
>   }
> }
>   ]
> }
> ```
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 34f28a165aae4ae24fa95ef19b4972e088fd63a0 
>   docs/operations/security.md 1a3d9b7e7ba4ec1952dc886d5fbeb6b85d994fb9 
>   examples/vagrant/announcer-auth.json PRE-CREATION 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 120b89a1dc10a259940cb9527eb2517f19d04471 
>   src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
> 6634506108c346f8c23b2da7cc8d20d09d07d590 
>   src/main/python/apache/aurora/executor/common/announcer.py 
> 79a9cfb6ac3a8444f09fb3658e6e859e06941ba4 
>   src/main/python/apache/aurora/executor/common/announcer_zkauth_schema.py 
> PRE-CREATION 
>   
> src/test/python/apache/aurora/executor/bin/test_thermos_executor_entry_point.py
>  e9f7851292aef3a36da5da9b0fc333a7e7750cf3 
>   src/test/python/apache/aurora/executor/common/test_announcer.py 
> 142b58d5e577c9f4b8e2ae8473cffdea94eba21f 
>   src/test/sh/org/apache/aurora/e2e/validate_serverset.py 
> fca1137bd2e7b1306a03dc2a54d2ef15b59af6a8 
> 
> Diff: https://reviews.apache.org/r/45042/diff/
> 
> 
> Testing
> ---
> 
> /vagrant/src/test/sh/org/apache/aurora/e2e/test_announcer_auth_end_to_end.sh
> /vagrant/src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Kunal Thakar
> 
>

Re: Review Request 45042: Add ACL support for announcer

2016-03-30 Thread Kunal Thakar 


> On March 30, 2016, 2:20 a.m., Bill Farner wrote:
> > RELEASE-NOTES.md, line 14
> > 
> >
> > s/Support/Added support/
> > s/ZK/ZooKeeper/

Done


> On March 30, 2016, 2:20 a.m., Bill Farner wrote:
> > docs/operations/security.md, line 26
> > 
> >
> > s/Zookeeper/ZooKeeper/ (capital K)

Done


> On March 30, 2016, 2:20 a.m., Bill Farner wrote:
> > docs/operations/security.md, line 34
> > 
> >
> > "To enable authentication for the announcer, see..."

Done


> On March 30, 2016, 2:20 a.m., Bill Farner wrote:
> > docs/operations/security.md, line 292
> > 
> >
> > "The Thermos executor can be configured to authenticate with ZooKeeper 
> > and include an [ACL] on the nodes it creates, which will specify..."

Done


> On March 30, 2016, 2:20 a.m., Bill Farner wrote:
> > docs/operations/security.md, line 299
> > 
> >
> > s/ACL/an ACL/

Done


> On March 30, 2016, 2:20 a.m., Bill Farner wrote:
> > examples/vagrant/announcer-auth.json, line 1
> > 
> >
> > Whoops, in the last round i intended for you to only _add_ world read 
> > access.  We should not remove the auth and restricted write/create access - 
> > these are valuable to exercise in the vagrant environment.

Done. I have a added read and delete permissions for world:anyone as 
validate_serverset.py also needs delete perms.


> On March 30, 2016, 2:20 a.m., Bill Farner wrote:
> > src/main/python/apache/aurora/executor/bin/thermos_executor_main.py, line 
> > 201
> > 
> >
> > feel free to inline this below to avoid the extra var declaration

Done


> On March 30, 2016, 2:20 a.m., Bill Farner wrote:
> > src/main/python/apache/aurora/executor/common/announcer.py, lines 154-164
> > 
> >
> > Use a list comprehension instead:
> > 
> > ```
> > def to_acl(access):
> >   return make_acl(...)
> > 
> > default_acl = [to_acl(a) for a in self._zk_auth.acl()]
> > ```

Done


> On March 30, 2016, 2:20 a.m., Bill Farner wrote:
> > src/main/python/apache/aurora/executor/common/announcer.py, lines 168-169
> > 
> >
> > I assume you do the `[]` -> `None` conversion because the behavior is 
> > different for these args?
> > 
> > If so, you can make the code more concise and avoid these variable 
> > reassignments:
> > ```
> > auth_data = auth_data if auth_data else None
> > ...
> > default_acl = default_acl if default_acl else None
> > ```
> > 
> > By instead doing this:
> > ```python
> > return KazooClient(self._ensemble,
> >connection_retry=self.DEFAULT_RETRY_POLICY,
> >default_acl=default_acl or None,
> >auth_data=auth_data or None)
> > ```

Done


> On March 30, 2016, 2:20 a.m., Bill Farner wrote:
> > src/test/sh/org/apache/aurora/e2e/validate_serverset.py, line 50
> > 
> >
> > Is this needed?  If so, please include a comment explaining why the 
> > timeout needs to be at 30.

This was an artifact of my separate e2e test. I have reverted the timeout to 10.


- Kunal


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


On March 30, 2016, 12:17 a.m., Kunal Thakar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45042/
> ---
> 
> (Updated March 30, 2016, 12:17 a.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add ACL support for announcer
> https://issues.apache.org/jira/browse/AURORA-1643
> 
> Adding support for service discovery ZK authentication. ZK authentication 
> secrets should be stored in a file as json (as follows):
> (Updated JSON format for config file)
> ```json
> {
>   "auth": [
> {
>   "scheme": "",
>   "credential": ""
> }
>   ],
>   "acls": [
> {
>   "scheme": "",
>   "credential": "",
>   "permissions": {
> "read": ,
> "write": ,
> "create": ,
>

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

2016-03-30 Thread Bill Farner 


> On March 30, 2016, 9:49 a.m., Joshua Cohen wrote:
> > Code-wise this looks fine to me, however, I have some reservations about 
> > making tier required. I think that throwing a `TaskDescriptionError` when 
> > tier is defined, but not valid is fine, but can we/should we continue to 
> > default to some known value if tier is undefined?
> 
> Bill Farner wrote:
> Emphatic +1 - tier should never be required.
> 
> Amol Deshmukh wrote:
> # Scope of this change
> To clarify, this change does not require specifying a tier in the job 
> configuration. With this change, the **default out-of-box behavior** w.r.t. 
> tiers will remain largely unchanged:
> 1. If job configuration specifies a valid tier, that tier will be used.
> 2. If job configuration does not specify any tier, a tier will be 
> selected as follows:
> ```
> TaskConfig.production == true => "preferred"
> TaskConfig.production == false => "preemptible"
> ```
> 3. [Backward Incompatibility] If a job configuration specifies an invalid 
> tier, the scheduling attempt will fail (rather than select a default tier).
> 
> If the cluster administrator chooses to customize the tier configuration 
> so that:
> 1. **One of the ``preemptible`` or ``preferred`` tiers is removed** 
> (expected likelihood: low), attempts to schedule jobs with ``production = 
> false`` or ``production = true`` respectively, will fail.
> 2. **The ``revocable`` tier is removed** (expected likelihood: medium), 
> only attempts to schedule jobs that explicitly request ``tier = "revocable"`` 
> will fail.
> 3. **The provided tiers are renamed** (expected likelihood: very low), 
> client configurations that reference the tiers explicitly by the old names 
> will fail to schedule.
> 
> # Looking ahead
> The plan going forward as described in the proposal at 
> https://docs.google.com/document/d/1erszT-HsWf1zCIfhbqHlsotHxWUvDyI2xUwNQQQxLgs/edit#heading=h.5htko8wan7dm,
>  was indeed to make tiers required in client configuration. The immediate 
> driver for this was to deprecate the ``production`` flag in TaskConfig.
> 
> As an alternative to making tiers mandatory, I'd like to propose a change 
> to the tier configuration file to allow specifying a ``default`` tier. This 
> will yield control to the cluster administrator in determining the default 
> tier rather than bake the default into the code as a constant. If this 
> approach seems reasonable:
> 1. I will update the proposal accordingly.
> 2. Implement the support for default tier in tier configuration.

| To clarify, this change does not require specifying a tier in the job 
configuration.

Understood, i'm stating a preference on the direction; and was tipped off by a 
comment i spotted in this review.  I will continue this line of thought in 
[AURORA-1624](https://issues.apache.org/jira/browse/AURORA-1624) instead.


- Bill


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


On March 29, 2016, 10:51 a.m., Amol Deshmukh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45222/
> ---
> 
> (Updated March 29, 2016, 10:51 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
> 
> 
> 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
> -
> 
>   RELEASE-NOTES.md 3a83961f5133fc0021b7c61f1117488703a9849d 
>   docs/reference/configuration.md fa09fd47b7637c035a5a23aaf6e3bc1c2c7fc143 
>   src/jmh/java/org/apache/aurora/benchmark/Offers.java 
> 055a2ffcb13c643a3086343e3fbf71545c5fb0a6 
>   src/main/java/org/apache/aurora/scheduler/TierInfo.java 
> b4611b9cf99ca236cd

Re: Review Request 45467: [PROTOTYPE] Add support for DB migrations and rollbacks.

2016-03-30 Thread Bill Farner 


> On March 30, 2016, 10:59 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java,
> >  line 180
> > 
> >
> > As i read this, i found myself wishing for something that appears to be 
> > [`FileMigrationLoader`](https://github.com/mybatis/migrations/blob/master/src/main/java/org/apache/ibatis/migration/FileMigrationLoader.java)
> >  but with the ability to load resources from the classpath.
> > 
> > Have you given any thought to using that instead?
> 
> Joshua Cohen wrote:
> Are you saying you'd prefer migrations be stored as sql scripts rather 
> than classes? It think it would be possible to write something on top of 
> `FileMigrationLoader` that did that, but I actually kind of prefer the 
> class-based representation since we want to define both up and down scripts. 
> I don't love the way the file-based scripts represent both in the same file 
> with an `@UNDO` annotation; this seems like it could potentially be brittle.
> 
> Bill Farner wrote:
> Yes, my preference was to keep them in files.  I agree that the `@UNDO` 
> is a bit odd, but ultimately we should have a test plan for this effort.  
> Without proof of brittleness, i don't think the argument holds water.

To clarify more - i'm not firm on this position, more of a "if it's more 
concise and readable, i'd take it".  Unless this is obviously better from where 
you sit, i don't intend to get in the way.


- Bill


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


On March 29, 2016, 8:41 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45467/
> ---
> 
> (Updated March 29, 2016, 8:41 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> THIS CODE IS NOT INTENDED TO BE COMMITTED.
> 
> This is just a spike to show a proof of concept for how we can effect 
> automatic migrations and rollbacks of the H2 schema. The code is very sloppy, 
> please use this to further the discussion on the mailing list about 
> migrations. If we agree this methodology is acceptable, I'll clean this up 
> and send out an actual review.
> 
> That said...
> 
> The general gist here is:
> 
> 1. Use MyBatis Migrations which has built in support for specifying an up and 
> a down script for db changes and also makes it easy to locate all existing 
> migrations on the classpath.
> 2. When applying a migration, write the downgrade script to the changelog 
> table in the database.
> 3. Before actually applying migrations, check all changes in the changelog 
> table. If the corresponding migration does not exist on the classpath, we 
> assume this is a rollback and run the downgrade script from the changelog 
> table and delete the corresponding changelog row.
> 
> 
> Diffs
> -
> 
>   build.gradle ad5ec5c4afe4dd5c2189d0680692a7409f0417a9 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 120b89a1dc10a259940cb9527eb2517f19d04471 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityModule.java
>  e32862034a1ad47dae8fff89cb04deb34ccd90e2 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
> baf460e987d0a6ba2810507695fe118b6b502f87 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/migration/V001_CreateUnifiedContainerTables.java
>  PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 
> 6fee2510d044515e0704cf20ec0ba77231050ec4 
> 
> Diff: https://reviews.apache.org/r/45467/diff/
> 
> 
> Testing
> ---
> 
> I manually verified in vagrant that this works as expected for upgrades with 
> migrations, upgrades without migrations and rollbacks.
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>

Re: Review Request 45467: [PROTOTYPE] Add support for DB migrations and rollbacks.

2016-03-30 Thread Bill Farner 


> On March 30, 2016, 10:59 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java,
> >  line 180
> > 
> >
> > As i read this, i found myself wishing for something that appears to be 
> > [`FileMigrationLoader`](https://github.com/mybatis/migrations/blob/master/src/main/java/org/apache/ibatis/migration/FileMigrationLoader.java)
> >  but with the ability to load resources from the classpath.
> > 
> > Have you given any thought to using that instead?
> 
> Joshua Cohen wrote:
> Are you saying you'd prefer migrations be stored as sql scripts rather 
> than classes? It think it would be possible to write something on top of 
> `FileMigrationLoader` that did that, but I actually kind of prefer the 
> class-based representation since we want to define both up and down scripts. 
> I don't love the way the file-based scripts represent both in the same file 
> with an `@UNDO` annotation; this seems like it could potentially be brittle.

Yes, my preference was to keep them in files.  I agree that the `@UNDO` is a 
bit odd, but ultimately we should have a test plan for this effort.  Without 
proof of brittleness, i don't think the argument holds water.


- Bill


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


On March 29, 2016, 8:41 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45467/
> ---
> 
> (Updated March 29, 2016, 8:41 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> THIS CODE IS NOT INTENDED TO BE COMMITTED.
> 
> This is just a spike to show a proof of concept for how we can effect 
> automatic migrations and rollbacks of the H2 schema. The code is very sloppy, 
> please use this to further the discussion on the mailing list about 
> migrations. If we agree this methodology is acceptable, I'll clean this up 
> and send out an actual review.
> 
> That said...
> 
> The general gist here is:
> 
> 1. Use MyBatis Migrations which has built in support for specifying an up and 
> a down script for db changes and also makes it easy to locate all existing 
> migrations on the classpath.
> 2. When applying a migration, write the downgrade script to the changelog 
> table in the database.
> 3. Before actually applying migrations, check all changes in the changelog 
> table. If the corresponding migration does not exist on the classpath, we 
> assume this is a rollback and run the downgrade script from the changelog 
> table and delete the corresponding changelog row.
> 
> 
> Diffs
> -
> 
>   build.gradle ad5ec5c4afe4dd5c2189d0680692a7409f0417a9 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 120b89a1dc10a259940cb9527eb2517f19d04471 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityModule.java
>  e32862034a1ad47dae8fff89cb04deb34ccd90e2 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
> baf460e987d0a6ba2810507695fe118b6b502f87 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/migration/V001_CreateUnifiedContainerTables.java
>  PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 
> 6fee2510d044515e0704cf20ec0ba77231050ec4 
> 
> Diff: https://reviews.apache.org/r/45467/diff/
> 
> 
> Testing
> ---
> 
> I manually verified in vagrant that this works as expected for upgrades with 
> migrations, upgrades without migrations and rollbacks.
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>

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

2016-03-30 Thread Amol Deshmukh 


> On March 30, 2016, 9:49 a.m., Joshua Cohen wrote:
> > Code-wise this looks fine to me, however, I have some reservations about 
> > making tier required. I think that throwing a `TaskDescriptionError` when 
> > tier is defined, but not valid is fine, but can we/should we continue to 
> > default to some known value if tier is undefined?
> 
> Bill Farner wrote:
> Emphatic +1 - tier should never be required.

# Scope of this change
To clarify, this change does not require specifying a tier in the job 
configuration. With this change, the **default out-of-box behavior** w.r.t. 
tiers will remain largely unchanged:
1. If job configuration specifies a valid tier, that tier will be used.
2. If job configuration does not specify any tier, a tier will be selected as 
follows:
```
TaskConfig.production == true => "preferred"
TaskConfig.production == false => "preemptible"
```
3. [Backward Incompatibility] If a job configuration specifies an invalid tier, 
the scheduling attempt will fail (rather than select a default tier).

If the cluster administrator chooses to customize the tier configuration so 
that:
1. **One of the ``preemptible`` or ``preferred`` tiers is removed** (expected 
likelihood: low), attempts to schedule jobs with ``production = false`` or 
``production = true`` respectively, will fail.
2. **The ``revocable`` tier is removed** (expected likelihood: medium), only 
attempts to schedule jobs that explicitly request ``tier = "revocable"`` will 
fail.
3. **The provided tiers are renamed** (expected likelihood: very low), client 
configurations that reference the tiers explicitly by the old names will fail 
to schedule.

# Looking ahead
The plan going forward as described in the proposal at 
https://docs.google.com/document/d/1erszT-HsWf1zCIfhbqHlsotHxWUvDyI2xUwNQQQxLgs/edit#heading=h.5htko8wan7dm,
 was indeed to make tiers required in client configuration. The immediate 
driver for this was to deprecate the ``production`` flag in TaskConfig.

As an alternative to making tiers mandatory, I'd like to propose a change to 
the tier configuration file to allow specifying a ``default`` tier. This will 
yield control to the cluster administrator in determining the default tier 
rather than bake the default into the code as a constant. If this approach 
seems reasonable:
1. I will update the proposal accordingly.
2. Implement the support for default tier in tier configuration.


- Amol


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


On March 29, 2016, 10:51 a.m., Amol Deshmukh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45222/
> ---
> 
> (Updated March 29, 2016, 10:51 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
> 
> 
> 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
> -
> 
>   RELEASE-NOTES.md 3a83961f5133fc0021b7c61f1117488703a9849d 
>   docs/reference/configuration.md fa09fd47b7637c035a5a23aaf6e3bc1c2c7fc143 
>   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
>  9d2bc825edef7fabeccd2334db48acc1bf622eb0 
>   src/main/java

Re: Review Request 45467: [PROTOTYPE] Add support for DB migrations and rollbacks.

2016-03-30 Thread Joshua Cohen 


> On March 30, 2016, 5:59 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java,
> >  line 180
> > 
> >
> > As i read this, i found myself wishing for something that appears to be 
> > [`FileMigrationLoader`](https://github.com/mybatis/migrations/blob/master/src/main/java/org/apache/ibatis/migration/FileMigrationLoader.java)
> >  but with the ability to load resources from the classpath.
> > 
> > Have you given any thought to using that instead?

Are you saying you'd prefer migrations be stored as sql scripts rather than 
classes? It think it would be possible to write something on top of 
`FileMigrationLoader` that did that, but I actually kind of prefer the 
class-based representation since we want to define both up and down scripts. I 
don't love the way the file-based scripts represent both in the same file with 
an `@UNDO` annotation; this seems like it could potentially be brittle.


- Joshua


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


On March 30, 2016, 3:41 a.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45467/
> ---
> 
> (Updated March 30, 2016, 3:41 a.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> THIS CODE IS NOT INTENDED TO BE COMMITTED.
> 
> This is just a spike to show a proof of concept for how we can effect 
> automatic migrations and rollbacks of the H2 schema. The code is very sloppy, 
> please use this to further the discussion on the mailing list about 
> migrations. If we agree this methodology is acceptable, I'll clean this up 
> and send out an actual review.
> 
> That said...
> 
> The general gist here is:
> 
> 1. Use MyBatis Migrations which has built in support for specifying an up and 
> a down script for db changes and also makes it easy to locate all existing 
> migrations on the classpath.
> 2. When applying a migration, write the downgrade script to the changelog 
> table in the database.
> 3. Before actually applying migrations, check all changes in the changelog 
> table. If the corresponding migration does not exist on the classpath, we 
> assume this is a rollback and run the downgrade script from the changelog 
> table and delete the corresponding changelog row.
> 
> 
> Diffs
> -
> 
>   build.gradle ad5ec5c4afe4dd5c2189d0680692a7409f0417a9 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 120b89a1dc10a259940cb9527eb2517f19d04471 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityModule.java
>  e32862034a1ad47dae8fff89cb04deb34ccd90e2 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
> baf460e987d0a6ba2810507695fe118b6b502f87 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/migration/V001_CreateUnifiedContainerTables.java
>  PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 
> 6fee2510d044515e0704cf20ec0ba77231050ec4 
> 
> Diff: https://reviews.apache.org/r/45467/diff/
> 
> 
> Testing
> ---
> 
> I manually verified in vagrant that this works as expected for upgrades with 
> migrations, upgrades without migrations and rollbacks.
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>

Re: Review Request 45456: Use correct query to serve /maintenance.

2016-03-30 Thread Aurora ReviewBot 

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


Ship it!




Master (ec29ac1) 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 29, 2016, 10:46 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45456/
> ---
> 
> (Updated March 29, 2016, 10:46 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Zameer Manji.
> 
> 
> Bugs: AURORA-1652
> https://issues.apache.org/jira/browse/AURORA-1652
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Fixes the 2 issues identified in AURORA-1652.
> 
> I will submit a separate patch to mitigate this issue by preventing 
> error-prone calls to conveniences like `Query.slaveScoped()` with an empty 
> `Iterable` (which has the effect of not filtering).
> 
> 
> Diffs
> -
> 
>   config/legacy_untested_classes.txt afe954fa962b1590e9e6d0c44d62a0f923cb5727 
>   src/main/java/org/apache/aurora/scheduler/http/Maintenance.java 
> 72c8c3e52599ac98bd730e6be6b7b15dffe57cdd 
>   src/test/java/org/apache/aurora/scheduler/http/MaintenanceTest.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45456/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 45467: [PROTOTYPE] Add support for DB migrations and rollbacks.

2016-03-30 Thread Bill Farner 

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




src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 
(line 179)


As i read this, i found myself wishing for something that appears to be 
[`FileMigrationLoader`](https://github.com/mybatis/migrations/blob/master/src/main/java/org/apache/ibatis/migration/FileMigrationLoader.java)
 but with the ability to load resources from the classpath.

Have you given any thought to using that instead?


- Bill Farner


On March 29, 2016, 8:41 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45467/
> ---
> 
> (Updated March 29, 2016, 8:41 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> THIS CODE IS NOT INTENDED TO BE COMMITTED.
> 
> This is just a spike to show a proof of concept for how we can effect 
> automatic migrations and rollbacks of the H2 schema. The code is very sloppy, 
> please use this to further the discussion on the mailing list about 
> migrations. If we agree this methodology is acceptable, I'll clean this up 
> and send out an actual review.
> 
> That said...
> 
> The general gist here is:
> 
> 1. Use MyBatis Migrations which has built in support for specifying an up and 
> a down script for db changes and also makes it easy to locate all existing 
> migrations on the classpath.
> 2. When applying a migration, write the downgrade script to the changelog 
> table in the database.
> 3. Before actually applying migrations, check all changes in the changelog 
> table. If the corresponding migration does not exist on the classpath, we 
> assume this is a rollback and run the downgrade script from the changelog 
> table and delete the corresponding changelog row.
> 
> 
> Diffs
> -
> 
>   build.gradle ad5ec5c4afe4dd5c2189d0680692a7409f0417a9 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 120b89a1dc10a259940cb9527eb2517f19d04471 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityModule.java
>  e32862034a1ad47dae8fff89cb04deb34ccd90e2 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
> baf460e987d0a6ba2810507695fe118b6b502f87 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/migration/V001_CreateUnifiedContainerTables.java
>  PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 
> 6fee2510d044515e0704cf20ec0ba77231050ec4 
> 
> Diff: https://reviews.apache.org/r/45467/diff/
> 
> 
> Testing
> ---
> 
> I manually verified in vagrant that this works as expected for upgrades with 
> migrations, upgrades without migrations and rollbacks.
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>

Re: Review Request 45456: Use correct query to serve /maintenance.

2016-03-30 Thread Bill Farner 

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



@ReviewBot retry

- Bill Farner


On March 29, 2016, 3:46 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45456/
> ---
> 
> (Updated March 29, 2016, 3:46 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Zameer Manji.
> 
> 
> Bugs: AURORA-1652
> https://issues.apache.org/jira/browse/AURORA-1652
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Fixes the 2 issues identified in AURORA-1652.
> 
> I will submit a separate patch to mitigate this issue by preventing 
> error-prone calls to conveniences like `Query.slaveScoped()` with an empty 
> `Iterable` (which has the effect of not filtering).
> 
> 
> Diffs
> -
> 
>   config/legacy_untested_classes.txt afe954fa962b1590e9e6d0c44d62a0f923cb5727 
>   src/main/java/org/apache/aurora/scheduler/http/Maintenance.java 
> 72c8c3e52599ac98bd730e6be6b7b15dffe57cdd 
>   src/test/java/org/apache/aurora/scheduler/http/MaintenanceTest.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45456/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

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

2016-03-30 Thread Bill Farner 


> On March 30, 2016, 9:49 a.m., Joshua Cohen wrote:
> > Code-wise this looks fine to me, however, I have some reservations about 
> > making tier required. I think that throwing a `TaskDescriptionError` when 
> > tier is defined, but not valid is fine, but can we/should we continue to 
> > default to some known value if tier is undefined?

Emphatic +1 - tier should never be required.


- Bill


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


On March 29, 2016, 10:51 a.m., Amol Deshmukh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45222/
> ---
> 
> (Updated March 29, 2016, 10:51 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
> 
> 
> 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
> -
> 
>   RELEASE-NOTES.md 3a83961f5133fc0021b7c61f1117488703a9849d 
>   docs/reference/configuration.md fa09fd47b7637c035a5a23aaf6e3bc1c2c7fc143 
>   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
>  9d2bc825edef7fabeccd2334db48acc1bf622eb0 
>   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.37

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

2016-03-30 Thread Joshua Cohen 

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



Code-wise this looks fine to me, however, I have some reservations about making 
tier required. I think that throwing a `TaskDescriptionError` when tier is 
defined, but not valid is fine, but can we/should we continue to default to 
some known value if tier is undefined?

- Joshua Cohen


On March 29, 2016, 5:51 p.m., Amol Deshmukh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45222/
> ---
> 
> (Updated March 29, 2016, 5:51 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
> 
> 
> 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
> -
> 
>   RELEASE-NOTES.md 3a83961f5133fc0021b7c61f1117488703a9849d 
>   docs/reference/configuration.md fa09fd47b7637c035a5a23aaf6e3bc1c2c7fc143 
>   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
>  9d2bc825edef7fabeccd2334db48acc1bf622eb0 
>   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