date:20151217

Re: Review Request 41201: Preserve env variables for tasks in docker

2015-12-17 Thread Aurora ReviewBot


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


Master (3e1f823) is green with this patch.
  ./build-support/jenkins/build.sh

However, it appears that it might lack test coverage.

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

- Aurora ReviewBot


On Dec. 17, 2015, 12:27 p.m., Kasisnu Singh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41201/
> ---
> 
> (Updated Dec. 17, 2015, 12:27 p.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Preserve env variables for tasks in docker
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
> 0d02dc17da4d496560ea572659e281c736170da2 
>   src/main/python/apache/aurora/executor/thermos_task_runner.py 
> 14e8b4bd539d2c295582d93fa01b5613345c1758 
>   src/main/python/apache/thermos/core/process.py 
> f214bccbd6f3be795e0fa9a259d27b01b81ab655 
>   src/main/python/apache/thermos/core/runner.py 
> f949f279a071c6464b026749f51afc776102f2aa 
>   src/main/python/apache/thermos/runner/thermos_runner.py 
> bd8cf7f4cda54b6be72dad64f9446eedeb132211 
> 
> Diff: https://reviews.apache.org/r/41201/diff/
> 
> 
> Testing
> ---
> 
> Ran ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh with no failures 
> and did manual testing for env variables in containers.
> 
> 
> Thanks,
> 
> Kasisnu Singh
> 
>

Re: Review Request 30695: Implements log rotation in the Thermos runner.

2015-12-17 Thread George Sirois


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

(Updated Dec. 17, 2015, 5:22 p.m.)


Review request for Aurora, Bill Farner and Brian Wickman.


Changes
---

@wfarner this change addresses all of your comments + changes the flags be a 
default behavior that can be overridden by process-level configuration.


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


Repository: aurora


Description
---

Implements log rotation in the Thermos runner.


Diffs (updated)
-

  docs/configuration-reference.md ad2701cadd38bb2fdbbe2acc477038986f8ec733 
  docs/deploying-aurora-scheduler.md 8a1e68e5d54e9b8b66139bfc731563668584fa77 
  src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
b3e8bf1e924999306e0b8a1314273b22c51028e7 
  src/main/python/apache/aurora/executor/thermos_task_runner.py 
14e8b4bd539d2c295582d93fa01b5613345c1758 
  src/main/python/apache/thermos/config/schema_base.py 
f9143cc1b83143d6147f59d90c79435d055d0518 
  src/main/python/apache/thermos/core/process.py 
fe95cb3be01b47616596bd78cb9a919b2e8bd978 
  src/main/python/apache/thermos/core/runner.py 
f949f279a071c6464b026749f51afc776102f2aa 
  src/main/python/apache/thermos/runner/thermos_runner.py 
bd8cf7f4cda54b6be72dad64f9446eedeb132211 
  src/test/python/apache/thermos/core/test_process.py 
5e6ad2fca616b840299bd9ca1614c82c5c39e992 

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


Testing
---

./pants test src/test/python/apache/thermos/core:all


Thanks,

George Sirois

Re: Review Request 41486: Deprecate JobUpdateSettings.maxWaitToInstanceRunning and UpdateConfig.restart_threshold

2015-12-17 Thread Maxim Khutornenko


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

Ship it!


Ship It!

- Maxim Khutornenko


On Dec. 17, 2015, 5:55 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41486/
> ---
> 
> (Updated Dec. 17, 2015, 5:55 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-1252 and AURORA-1253
> https://issues.apache.org/jira/browse/AURORA-1252
> https://issues.apache.org/jira/browse/AURORA-1253
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This pulled a longer thread than i was hoping, since the python class 
> `UpdaterConfig` was overloaded for use by the rolling restart command.
> 
> Within the scheduler, removal of this field has little behavioral difference. 
>  The effect is there are some conditions where the scheduler is event-driven 
> instead of polling (which was more appropriate anyway).
> 
> 
> Diffs
> -
> 
>   NEWS 7a80f32465736de4fbdc7d2c7a3dbf790be31e16 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> d765dd798e6861e3920802db00e0b96a250d1bfa 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  9aef59ac5e969b01004d8d9ede825caacca04674 
>   
> src/main/java/org/apache/aurora/scheduler/updater/InstanceActionHandler.java 
> d8686f1f1b53e5ff2791663489e24c342503831e 
>   
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java
>  0e075607aabdc98a9c8c8c6bfa2929d3428645ff 
>   src/main/java/org/apache/aurora/scheduler/updater/UpdateFactory.java 
> 258151feda2ea0adf0159c24c0e18a7b78b0c390 
>   src/main/python/apache/aurora/client/api/__init__.py 
> a6381585a2a9762202f14f929ab922806e84e8b6 
>   src/main/python/apache/aurora/client/api/restarter.py 
> 1983034ff4e7510f7c2ca35ff9c851be24637c8e 
>   src/main/python/apache/aurora/client/api/updater_util.py 
> b54691eed59854ea5feb4120929cefbf1a95cd24 
>   src/main/python/apache/aurora/client/cli/jobs.py 
> ccc52c8357650b126185eb6100b3e2e0e37e1d45 
>   src/main/python/apache/aurora/client/hooks/hooked_api.py 
> 6410908b48f54f5188c29f247a9f7797ec802e34 
>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
> f7f5275bed4eda5e276e408bf253f5da6a9cacd8 
>   src/test/python/apache/aurora/client/api/test_api.py 
> 7debc7967cf926bc09f8a93cafa5059c0657e620 
>   src/test/python/apache/aurora/client/api/test_restarter.py 
> 7557144ef1eed7ef24e69e82d04494c9f58dc217 
>   src/test/python/apache/aurora/client/cli/test_restart.py 
> 2c51d541c4445f287d8794c0117fb038bb776329 
>   src/test/python/apache/aurora/client/hooks/test_non_hooked_api.py 
> 97ce9732fd9561327c8c4c4f67fefd3866110bcc 
> 
> Diff: https://reviews.apache.org/r/41486/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Review Request 41522: Remove ServerInfo.thriftAPIVersion

2015-12-17 Thread Bill Farner


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

Review request for Aurora and Maxim Khutornenko.


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


Repository: aurora


Description
---

See summary.


Diffs
-

  api/src/main/thrift/org/apache/aurora/gen/api.thrift 
d765dd798e6861e3920802db00e0b96a250d1bfa 
  api/src/main/thrift/org/apache/aurora/gen/storage.thrift 
624076d1acd8d087a857ebdfeec0831e1bd8e89c 
  src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
ff886d9bc6ef44fa17bbad4fde208ab682483809 
  src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 
3ede9a0c3d1718cab07385c86b774efdfccc29f8 
  src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
35a4d627c5049eaec30380699e6005e8da2c10fd 
  src/test/java/org/apache/aurora/scheduler/http/AbstractJettyTest.java 
5768481005ef505d6c397101d8cc005af9d6815f 
  src/test/java/org/apache/aurora/scheduler/storage/backup/RecoveryTest.java 
97647ac3a4e99541484ac14f5796b5dffb01beb0 
  
src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java
 02400f0bbf832fdac610868cff665f4312864b8b 
  
src/test/java/org/apache/aurora/scheduler/thrift/aop/ServerInfoInterceptorTest.java
 3a80305e5169c8440b3a9d595fdaf8a52fdd15e3 
  src/test/python/apache/aurora/client/api/test_restarter.py 
7557144ef1eed7ef24e69e82d04494c9f58dc217 
  src/test/python/apache/aurora/client/cli/util.py 
4b5ef4d1137cc9d545ea2205e4d14e438185a990 
  src/test/python/apache/aurora/common/test_transport.py 
835bb3dfebccd0666f708d0023558aee80c1667d 

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


Testing
---


Thanks,

Bill Farner

Re: Review Request 41486: Deprecate JobUpdateSettings.maxWaitToInstanceRunning and UpdateConfig.restart_threshold

2015-12-17 Thread Bill Farner


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

(Updated Dec. 17, 2015, 10:52 a.m.)


Review request for Aurora, Joshua Cohen and Maxim Khutornenko.


Changes
---

rebase


Bugs: AURORA-1252 and AURORA-1253
https://issues.apache.org/jira/browse/AURORA-1252
https://issues.apache.org/jira/browse/AURORA-1253


Repository: aurora


Description
---

This pulled a longer thread than i was hoping, since the python class 
`UpdaterConfig` was overloaded for use by the rolling restart command.

Within the scheduler, removal of this field has little behavioral difference.  
The effect is there are some conditions where the scheduler is event-driven 
instead of polling (which was more appropriate anyway).


Diffs (updated)
-

  NEWS 7a80f32465736de4fbdc7d2c7a3dbf790be31e16 
  api/src/main/thrift/org/apache/aurora/gen/api.thrift 
d765dd798e6861e3920802db00e0b96a250d1bfa 
  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
9aef59ac5e969b01004d8d9ede825caacca04674 
  src/main/java/org/apache/aurora/scheduler/updater/InstanceActionHandler.java 
0880cf2413063c0d1f28e872737872d6e4710bb7 
  
src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 
0e075607aabdc98a9c8c8c6bfa2929d3428645ff 
  src/main/java/org/apache/aurora/scheduler/updater/UpdateFactory.java 
258151feda2ea0adf0159c24c0e18a7b78b0c390 
  src/main/python/apache/aurora/client/api/__init__.py 
a6381585a2a9762202f14f929ab922806e84e8b6 
  src/main/python/apache/aurora/client/api/restarter.py 
1983034ff4e7510f7c2ca35ff9c851be24637c8e 
  src/main/python/apache/aurora/client/api/updater_util.py 
b54691eed59854ea5feb4120929cefbf1a95cd24 
  src/main/python/apache/aurora/client/cli/jobs.py 
ccc52c8357650b126185eb6100b3e2e0e37e1d45 
  src/main/python/apache/aurora/client/hooks/hooked_api.py 
6410908b48f54f5188c29f247a9f7797ec802e34 
  src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
f7f5275bed4eda5e276e408bf253f5da6a9cacd8 
  src/test/python/apache/aurora/client/api/test_api.py 
7debc7967cf926bc09f8a93cafa5059c0657e620 
  src/test/python/apache/aurora/client/api/test_restarter.py 
7557144ef1eed7ef24e69e82d04494c9f58dc217 
  src/test/python/apache/aurora/client/cli/test_restart.py 
2c51d541c4445f287d8794c0117fb038bb776329 
  src/test/python/apache/aurora/client/hooks/test_non_hooked_api.py 
97ce9732fd9561327c8c4c4f67fefd3866110bcc 

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


Testing
---


Thanks,

Bill Farner

Re: Review Request 41473: Accept a command line argument for an executor configuration via json

2015-12-17 Thread Joshua Cohen


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



src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java
 (line 52)


Maybe change the name to "custom_executor_config" and the help to "Path to 
custom executor settings configuration file."



src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java
 (line 53)


Can you add the following constraint annotations:

@Exists
@CanRead



src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java
 (line 141)


Is there a reason that 
`o.a.aurora.scheduler.configuration.executor.ExecutorSettingsLoader#ExecutorConfig`
 exists in addition to 
`o.a.aurora.scheduler.configuration.executor.ExecutorConfig`? They seem to be 
identical classes, am I missing something?



src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java
 (lines 147 - 148)


nit: Should fit on one line (but probably will just go away if we de-dupe 
these ExecutorConfig classes).



src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java
 (line 150)


Add a message to provide context on the exception? E.g.

throw new IllegalArgumentException("Failed to read executor settings: " 
+ e, e);


- Joshua Cohen


On Dec. 17, 2015, 12:34 a.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41473/
> ---
> 
> (Updated Dec. 17, 2015, 12:34 a.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Patch to allow Aurora to accept an executor config via commandline which 
> overrides the default thermos one.
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java
>  d04763418f55aa1e9f1b537987d21920c05fd950 
> 
> Diff: https://reviews.apache.org/r/41473/diff/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Renan DelValle
> 
>

Review Request 41486: Deprecate JobUpdateSettings.maxWaitToInstanceRunning and UpdateConfig.restart_threshold

2015-12-17 Thread Bill Farner


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

Review request for Aurora, Joshua Cohen and Maxim Khutornenko.


Bugs: AURORA-1252 and AURORA-1253
https://issues.apache.org/jira/browse/AURORA-1252
https://issues.apache.org/jira/browse/AURORA-1253


Repository: aurora


Description
---

This pulled a longer thread than i was hoing, since the python class 
`UpdaterConfig` was overloaded for use by the rolling restart command.

Within the scheduler, removal of this field has little behavioral difference.  
The effect is there are some conditions where the scheduler is event-driven 
instead of polling (which was more appropriate anyway).


Diffs
-

  NEWS 7a80f32465736de4fbdc7d2c7a3dbf790be31e16 
  api/src/main/thrift/org/apache/aurora/gen/api.thrift 
d765dd798e6861e3920802db00e0b96a250d1bfa 
  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
9aef59ac5e969b01004d8d9ede825caacca04674 
  src/main/java/org/apache/aurora/scheduler/updater/InstanceActionHandler.java 
d8686f1f1b53e5ff2791663489e24c342503831e 
  
src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 
0e075607aabdc98a9c8c8c6bfa2929d3428645ff 
  src/main/java/org/apache/aurora/scheduler/updater/UpdateFactory.java 
258151feda2ea0adf0159c24c0e18a7b78b0c390 
  src/main/python/apache/aurora/client/api/__init__.py 
a6381585a2a9762202f14f929ab922806e84e8b6 
  src/main/python/apache/aurora/client/api/restarter.py 
1983034ff4e7510f7c2ca35ff9c851be24637c8e 
  src/main/python/apache/aurora/client/api/updater_util.py 
b54691eed59854ea5feb4120929cefbf1a95cd24 
  src/main/python/apache/aurora/client/cli/jobs.py 
ccc52c8357650b126185eb6100b3e2e0e37e1d45 
  src/main/python/apache/aurora/client/hooks/hooked_api.py 
6410908b48f54f5188c29f247a9f7797ec802e34 
  src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
f7f5275bed4eda5e276e408bf253f5da6a9cacd8 
  src/test/python/apache/aurora/client/api/test_api.py 
7debc7967cf926bc09f8a93cafa5059c0657e620 
  src/test/python/apache/aurora/client/api/test_restarter.py 
7557144ef1eed7ef24e69e82d04494c9f58dc217 
  src/test/python/apache/aurora/client/cli/test_restart.py 
2c51d541c4445f287d8794c0117fb038bb776329 
  src/test/python/apache/aurora/client/hooks/test_non_hooked_api.py 
97ce9732fd9561327c8c4c4f67fefd3866110bcc 

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


Testing
---


Thanks,

Bill Farner

Review Request 41521: Force Windows to always use Unix line endings.

2015-12-17 Thread Joshua Cohen


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

Review request for Aurora and John Sirois.


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


Repository: aurora


Description
---

Since everything is run inside an Ubuntu VM, we want to ensure the line endings 
don't get auto-translated.


Diffs
-

  .gitattributes a5ab8cf17a14d2527e2a765ff94c3a74e719e0f4 

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


Testing
---

Applied settings to a checkout on a windows box and verified I was able to 
provision a VM w/ Vagrant and connect to the scheduler there.


Thanks,

Joshua Cohen

Re: Review Request 41486: Deprecate JobUpdateSettings.maxWaitToInstanceRunning and UpdateConfig.restart_threshold

2015-12-17 Thread Joshua Cohen


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

Ship it!


Ship It!

- Joshua Cohen


On Dec. 17, 2015, 5:55 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41486/
> ---
> 
> (Updated Dec. 17, 2015, 5:55 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-1252 and AURORA-1253
> https://issues.apache.org/jira/browse/AURORA-1252
> https://issues.apache.org/jira/browse/AURORA-1253
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This pulled a longer thread than i was hoping, since the python class 
> `UpdaterConfig` was overloaded for use by the rolling restart command.
> 
> Within the scheduler, removal of this field has little behavioral difference. 
>  The effect is there are some conditions where the scheduler is event-driven 
> instead of polling (which was more appropriate anyway).
> 
> 
> Diffs
> -
> 
>   NEWS 7a80f32465736de4fbdc7d2c7a3dbf790be31e16 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> d765dd798e6861e3920802db00e0b96a250d1bfa 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  9aef59ac5e969b01004d8d9ede825caacca04674 
>   
> src/main/java/org/apache/aurora/scheduler/updater/InstanceActionHandler.java 
> d8686f1f1b53e5ff2791663489e24c342503831e 
>   
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java
>  0e075607aabdc98a9c8c8c6bfa2929d3428645ff 
>   src/main/java/org/apache/aurora/scheduler/updater/UpdateFactory.java 
> 258151feda2ea0adf0159c24c0e18a7b78b0c390 
>   src/main/python/apache/aurora/client/api/__init__.py 
> a6381585a2a9762202f14f929ab922806e84e8b6 
>   src/main/python/apache/aurora/client/api/restarter.py 
> 1983034ff4e7510f7c2ca35ff9c851be24637c8e 
>   src/main/python/apache/aurora/client/api/updater_util.py 
> b54691eed59854ea5feb4120929cefbf1a95cd24 
>   src/main/python/apache/aurora/client/cli/jobs.py 
> ccc52c8357650b126185eb6100b3e2e0e37e1d45 
>   src/main/python/apache/aurora/client/hooks/hooked_api.py 
> 6410908b48f54f5188c29f247a9f7797ec802e34 
>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
> f7f5275bed4eda5e276e408bf253f5da6a9cacd8 
>   src/test/python/apache/aurora/client/api/test_api.py 
> 7debc7967cf926bc09f8a93cafa5059c0657e620 
>   src/test/python/apache/aurora/client/api/test_restarter.py 
> 7557144ef1eed7ef24e69e82d04494c9f58dc217 
>   src/test/python/apache/aurora/client/cli/test_restart.py 
> 2c51d541c4445f287d8794c0117fb038bb776329 
>   src/test/python/apache/aurora/client/hooks/test_non_hooked_api.py 
> 97ce9732fd9561327c8c4c4f67fefd3866110bcc 
> 
> Diff: https://reviews.apache.org/r/41486/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 41525: Add flag to set FrameworkInfo.principal

2015-12-17 Thread Aurora ReviewBot


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

Ship it!


Master (c912c34) 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 Dec. 17, 2015, 6:39 p.m., R.B. Boyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41525/
> ---
> 
> (Updated Dec. 17, 2015, 6:39 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Bill Farner.
> 
> 
> Bugs: AURORA-687
> https://issues.apache.org/jira/browse/AURORA-687
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Bugs closed: AURORA-687
> 
> Copying the intent from one of the trailing comments:
> 
> """
> There are several situations to consider:
> 
> 1. fresh installs that will not have authN/authZ (may want to add authN/authZ 
> later)
> 2. fresh installs that will begin life with authN/authZ (no problems in the 
> future unless the principal is changed)
> 3. existing installs that have authN (may want to add authZ later)
> 
> The current defaults are friendly for (1) above, where the typical 
> test-driver won't be turning any flavor of Auth on in their mesos cluster. 
> These defaults are good.
> 
> If someone elects to go down (2) above, the defaults you get from 
> -framework_authentication_file are not awesome, as this is where the misstep 
> lies.
> 
> If someone went down (3) above already, and used the 
> -framework_authentication_file non-awesome setting, you also don't want 
> _them_ to break their cluster.
> 
> The bare minimum change to be the least breaking would be to introduce a new 
> setting like -framework_announce_principal or something so that (2) can be 
> happy without breaking (3).
> 
> Similarly to the mesos checkpoint breaking change, you could roll this out as 
> opt-in and then in a future release make it opt-out when setting 
> -framework_authentication_file and announce it as a breaking change in the 
> changelogs for the poor folks in group (3).
> """
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java
>  68aeda1692271841d10e5f29d439806576bd691c 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java
>  513391ff82c3e985bf9f673d35414e9245807b4a 
> 
> Diff: https://reviews.apache.org/r/41525/diff/
> 
> 
> Testing
> ---
> 
> Ran normal unit tests.
> 
> Built the patched scheduler and deployed it in an authN+authZ mesos cluster 
> (3 control plane boxes, 2 execution plane boxes) and the framework could 
> actually register with mesos.
> 
> 
> Thanks,
> 
> R.B. Boyer
> 
>

Re: Review Request 41522: Remove ServerInfo.thriftAPIVersion

2015-12-17 Thread Maxim Khutornenko


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

Ship it!


Ship It!

- Maxim Khutornenko


On Dec. 17, 2015, 6:23 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41522/
> ---
> 
> (Updated Dec. 17, 2015, 6:23 p.m.)
> 
> 
> Review request for Aurora and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-1553
> https://issues.apache.org/jira/browse/AURORA-1553
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> d765dd798e6861e3920802db00e0b96a250d1bfa 
>   api/src/main/thrift/org/apache/aurora/gen/storage.thrift 
> 624076d1acd8d087a857ebdfeec0831e1bd8e89c 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> ff886d9bc6ef44fa17bbad4fde208ab682483809 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 
> 3ede9a0c3d1718cab07385c86b774efdfccc29f8 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 35a4d627c5049eaec30380699e6005e8da2c10fd 
>   src/test/java/org/apache/aurora/scheduler/http/AbstractJettyTest.java 
> 5768481005ef505d6c397101d8cc005af9d6815f 
>   src/test/java/org/apache/aurora/scheduler/storage/backup/RecoveryTest.java 
> 97647ac3a4e99541484ac14f5796b5dffb01beb0 
>   
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java
>  02400f0bbf832fdac610868cff665f4312864b8b 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/aop/ServerInfoInterceptorTest.java
>  3a80305e5169c8440b3a9d595fdaf8a52fdd15e3 
>   src/test/python/apache/aurora/client/api/test_restarter.py 
> 7557144ef1eed7ef24e69e82d04494c9f58dc217 
>   src/test/python/apache/aurora/client/cli/util.py 
> 4b5ef4d1137cc9d545ea2205e4d14e438185a990 
>   src/test/python/apache/aurora/common/test_transport.py 
> 835bb3dfebccd0666f708d0023558aee80c1667d 
> 
> Diff: https://reviews.apache.org/r/41522/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 41428: Refactoring HealthCheckConfig into separate structs

2015-12-17 Thread Dmitriy Shirchenko



> On Dec. 16, 2015, 10:14 p.m., Joshua Cohen wrote:
> > src/test/python/apache/aurora/client/test_config.py, line 187
> > 
> >
> > Kill this comment? It seems superfluous.
> 
> Dmitriy Shirchenko wrote:
> I added because I had a hard time finding my tests. I like comments to 
> logically separate tests into sections. If you feel like they should be 
> separated into individual classes, then I can do that as well but comments 
> seemed good enough for tests. Lmk.
> 
> Joshua Cohen wrote:
> I generally prefer to rely on explicitly named tests for this. Adding a 
> comment to delineate a group of tests is vulnerable to someone unknowingly 
> sneaking an unrelated test in the middle of that section. If you feel that 
> there are tests that should be logically grouped, adding them to a class is 
> the way to go.

Deleted (under protest)! I don't want to separate them into classes as I don't 
want to drag out this diff even further and get blocked on comments in tests. :)


> On Dec. 16, 2015, 10:14 p.m., Joshua Cohen wrote:
> > src/test/python/apache/aurora/client/test_config.py, lines 240-243
> > 
> >
> > I'm somewhat uncomfortable with using monkeypatch to accomplish this. 
> > We had generally been trending towards adding injection points for mocks, 
> > rather than monkeypatching globals. How would you feel about adding a 
> > logger arg to `_validate_health_check_config` that just defaults to `log`? 
> > and passing in a mock from this test case?
> 
> Dmitriy Shirchenko wrote:
> I personally use mock.patch decorators myself but I try not to add 
> dependencies as well so I had to figure out how to make this work with 
> pytest. I feel like mock.patch is the way to go but I also feel like we 
> should be consistent and swapping to mock is a separate chunk of work as it 
> may be required affect other tests as well. Like it would be weird to just 
> have one test using mock.
> 
> In any case: is this a nit or you feel like this is a blocker?
> 
> Joshua Cohen wrote:
> We use mock.patch elsewhere throughout the codebase, so adding a 
> dependency on that is not a problem.
> 
> That said, I think using mock.patch is essentially the same thing as 
> using pytest monkeypatch. Perhaps it's due to the fact that this is a python 
> codebase maintained primarily by a bunch of Java developers, but we've tended 
> to prefer injection over patching. The way we accomplish these sort of 
> assertions elsewhere in the client is through the use of the 
> [FakeAuroraCommandContext](https://github.com/apache/aurora/blob/master/src/test/python/apache/aurora/client/cli/util.py#L71)
>  which abstracts away the log statements in a way that makes them easy to 
> mock. However config is at a higher level and `AuroraCommandContext` is not 
> used, but we can still follow a similar pattern with regards to asserting on 
> cli logging.
> 
> So... long story short, this is more than a nit but less than a blocker, 
> hopefully the above makes a compelling case for "injecting" a mock?

If this is not a blocker, then I would prefer to leave this as is.

I am totally onboard with `mock.patch` and you can see test_base.py where I 
used injections all around.


- Dmitriy


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


On Dec. 16, 2015, 10:43 p.m., Dmitriy Shirchenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41428/
> ---
> 
> (Updated Dec. 16, 2015, 10:43 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-1562
> https://issues.apache.org/jira/browse/AURORA-1562
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Refactoring HealthCheckConfig into separate structs
> 
> 
> Diffs
> -
> 
>   docs/configuration-reference.md 07149c7f49371e39b70e8744d60affeffd73c77f 
>   src/main/python/apache/aurora/client/config.py 
> 161c3627db0afa8fdd8afff5abf6a94556f53180 
>   src/main/python/apache/aurora/config/schema/base.py 
> e752482b95ae6377d0cf12cf0ecf0fa60e2a5d46 
>   src/main/python/apache/aurora/executor/common/health_checker.py 
> cba4e8c879fae2b9f56a36bf8fa4b702163667d1 
>   src/test/python/apache/aurora/client/test_config.py 
> 8fd112fd8a10120f57324d8efec22a009b93404b 
>   src/test/python/apache/aurora/executor/common/test_health_checker.py 
> 8561abc3e7df8803785b70064bb76553d3210399 
>   src/test/sh/org/apache/aurora/e2e/http/http_example_bad_healthcheck.aurora 
> 37f2e9c6ebc4a4bbd6089e821420e5a02179c6b9 
> 
>

Re: Review Request 30695: Implements log rotation in the Thermos runner.

2015-12-17 Thread Aurora ReviewBot


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


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

virtualenv-12.1.1/virtualenv_support/__init__.py
virtualenv-12.1.1/virtualenv_support/pip-6.1.1-py2.py3-none-any.whl
virtualenv-12.1.1/virtualenv_support/setuptools-15.0-py2.py3-none-any.whl
+ touch virtualenv-12.1.1/BOOTSTRAPPED
+ popd
~/jenkins-slave/workspace/AuroraBot
+ exec /usr/bin/python2.7 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/virtualenv-12.1.1/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...done.
You are using pip version 6.1.1, however version 7.1.2 is available.
You should consider upgrading via the 'pip install --upgrade pip' command.
Collecting isort==4.0.0
  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/main/python/apache/thermos/config/schema_base.py
 Imports are incorrectly sorted.
--- 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/thermos/config/schema_base.py:before
 2015-12-17 19:35:33.439546
+++ 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/thermos/config/schema_base.py:after
  2015-12-17 19:40:34.485465
@@ -14,7 +14,19 @@
 
 # checkstyle: noqa
 
-from pystachio import Boolean, Default, Empty, Enum, Float, Integer, List, 
Map, Required, String, Struct
+from pystachio import (
+Boolean,
+Default,
+Empty,
+Enum,
+Float,
+Integer,
+List,
+Map,
+Required,
+String,
+Struct
+)
 
 # Define constants for resources
 BYTES = 1


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

- Aurora ReviewBot


On Dec. 17, 2015, 5:22 p.m., George Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30695/
> ---
> 
> (Updated Dec. 17, 2015, 5:22 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Brian Wickman.
> 
> 
> Bugs: AURORA-95
> https://issues.apache.org/jira/browse/AURORA-95
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Implements log rotation in the Thermos runner.
> 
> 
> Diffs
> -
> 
>   docs/configuration-reference.md ad2701cadd38bb2fdbbe2acc477038986f8ec733 
>   docs/deploying-aurora-scheduler.md 8a1e68e5d54e9b8b66139bfc731563668584fa77 
>   src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
> b3e8bf1e924999306e0b8a1314273b22c51028e7 
>   src/main/python/apache/aurora/executor/thermos_task_runner.py 
> 14e8b4bd539d2c295582d93fa01b5613345c1758 
>   src/main/python/apache/thermos/config/schema_base.py 
> f9143cc1b83143d6147f59d90c79435d055d0518 
>   src/main/python/apache/thermos/core/process.py 
> fe95cb3be01b47616596bd78cb9a919b2e8bd978 
>   src/main/python/apache/thermos/core/runner.py 
> f949f279a071c6464b026749f51afc776102f2aa 
>   src/main/python/apache/thermos/runner/thermos_runner.py 
> bd8cf7f4cda54b6be72dad64f9446eedeb132211 
>   src/test/python/apache/thermos/core/test_process.py 
> 5e6ad2fca616b840299bd9ca1614c82c5c39e992 
> 
> Diff: https://reviews.apache.org/r/30695/diff/
> 
> 
> Testing
> ---
> 
> ./pants test src/test/python/apache/thermos/core:all
> 
> 
> Thanks,
> 
> George Sirois
> 
>

Re: Review Request 41428: Refactoring HealthCheckConfig into separate structs

2015-12-17 Thread Bill Farner



> On Dec. 17, 2015, 12:31 p.m., Bill Farner wrote:
> > Shape of the API LGTM, end-to-end tests pass.

I'm going to commit this.  Maxim - if you have any remaining issues, i'm happy 
to address follow-up comments on Dmitriy's behalf


- Bill


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


On Dec. 17, 2015, 11:21 a.m., Dmitriy Shirchenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41428/
> ---
> 
> (Updated Dec. 17, 2015, 11:21 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-1562
> https://issues.apache.org/jira/browse/AURORA-1562
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Refactoring HealthCheckConfig into separate structs
> 
> 
> Diffs
> -
> 
>   docs/configuration-reference.md 07149c7f49371e39b70e8744d60affeffd73c77f 
>   src/main/python/apache/aurora/client/config.py 
> 161c3627db0afa8fdd8afff5abf6a94556f53180 
>   src/main/python/apache/aurora/config/schema/base.py 
> e752482b95ae6377d0cf12cf0ecf0fa60e2a5d46 
>   src/main/python/apache/aurora/executor/common/health_checker.py 
> cba4e8c879fae2b9f56a36bf8fa4b702163667d1 
>   src/test/python/apache/aurora/client/test_config.py 
> 8fd112fd8a10120f57324d8efec22a009b93404b 
>   src/test/python/apache/aurora/executor/common/test_health_checker.py 
> 8561abc3e7df8803785b70064bb76553d3210399 
>   src/test/sh/org/apache/aurora/e2e/http/http_example_bad_healthcheck.aurora 
> 37f2e9c6ebc4a4bbd6089e821420e5a02179c6b9 
> 
> Diff: https://reviews.apache.org/r/41428/diff/
> 
> 
> Testing
> ---
> 
> Added unit tests.
> Changed end to end test.
> 
> 
> Thanks,
> 
> Dmitriy Shirchenko
> 
>

Re: Review Request 41528: Fixup `getJobSummary` for cron jobs with invalid next run dates.

2015-12-17 Thread John Sirois


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

(Updated Dec. 17, 2015, 1:34 p.m.)


Review request for Aurora, Stephan Erb, Bill Farner, and Zameer Manji.


Changes
---

Fix several typos.

 src/main/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImpl.java   
  | 2 +-
 
src/test/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImplTest.java
 | 2 +-
 
src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java 
 | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)


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


Repository: aurora


Description
---

Previously,  the fact `null` could be returned by Quartz when
calculating the next run was not taken into account.  Now The
CronPredictor interface makes this possibility manifest with an
Optional result.

Code that uses the CronPredictor is adjusted and tests are added.

NB: This code is as first proposed here by Brice Arnould with small
changes: https://reviews.apache.org/r/39170/

 src/main/java/org/apache/aurora/scheduler/cron/CronPredictor.java  
  | 10 --
 src/main/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImpl.java   
  |  8 +---
 src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java
  | 13 -
 
src/test/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImplTest.java
 | 23 +++
 
src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java 
 | 35 ---
 5 files changed, 72 insertions(+), 17 deletions(-)


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/cron/CronPredictor.java 
043ba7e6858db28001dfb07ea0c2ddf274a1c755 
  src/main/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImpl.java 
1ddc4e1946910de798f7f423dd1b19ed56dece15 
  src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 
c0e8a201400338a8cb6bc24b2c21d0abb0d01e41 
  
src/test/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImplTest.java
 b85f4abd59ef64264fb089527ad42b9ceee7f8d6 
  
src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java 
6efe03fe4841cf1275e2ee0c7cc1b9576540f34e 

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


Testing
---

Green locally: `./gradlew -Pq build`


Thanks,

John Sirois

Re: Review Request 41523: Remove duplicate ExecutorConfig class.

2015-12-17 Thread Aurora ReviewBot


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

Ship it!


Master (c912c34) 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 Dec. 17, 2015, 6:31 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41523/
> ---
> 
> (Updated Dec. 17, 2015, 6:31 p.m.)
> 
> 
> Review request for Aurora and Joshua Cohen.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Remove duplicate ExecutorConfig class.
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorSettingsLoader.java
>  fafd90dbb4e628a213a5437aad8a50e7ec64ab79 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/executor/ExecutorSettingsLoaderTest.java
>  90986a5b1ed88d3f71fb8414688878aea4a312cc 
> 
> Diff: https://reviews.apache.org/r/41523/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 41428: Refactoring HealthCheckConfig into separate structs

2015-12-17 Thread Dmitriy Shirchenko



> On Dec. 17, 2015, 1:16 a.m., Joshua Cohen wrote:
> > src/main/python/apache/aurora/executor/common/health_checker.py, line 214
> > 
> >
> > Rename this var too? `_end` was never great, but now it's lost all 
> > relevance ;).

Done.


- Dmitriy


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


On Dec. 16, 2015, 10:43 p.m., Dmitriy Shirchenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41428/
> ---
> 
> (Updated Dec. 16, 2015, 10:43 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-1562
> https://issues.apache.org/jira/browse/AURORA-1562
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Refactoring HealthCheckConfig into separate structs
> 
> 
> Diffs
> -
> 
>   docs/configuration-reference.md 07149c7f49371e39b70e8744d60affeffd73c77f 
>   src/main/python/apache/aurora/client/config.py 
> 161c3627db0afa8fdd8afff5abf6a94556f53180 
>   src/main/python/apache/aurora/config/schema/base.py 
> e752482b95ae6377d0cf12cf0ecf0fa60e2a5d46 
>   src/main/python/apache/aurora/executor/common/health_checker.py 
> cba4e8c879fae2b9f56a36bf8fa4b702163667d1 
>   src/test/python/apache/aurora/client/test_config.py 
> 8fd112fd8a10120f57324d8efec22a009b93404b 
>   src/test/python/apache/aurora/executor/common/test_health_checker.py 
> 8561abc3e7df8803785b70064bb76553d3210399 
>   src/test/sh/org/apache/aurora/e2e/http/http_example_bad_healthcheck.aurora 
> 37f2e9c6ebc4a4bbd6089e821420e5a02179c6b9 
> 
> Diff: https://reviews.apache.org/r/41428/diff/
> 
> 
> Testing
> ---
> 
> Added unit tests.
> Changed end to end test.
> 
> 
> Thanks,
> 
> Dmitriy Shirchenko
> 
>

Re: Review Request 41428: Refactoring HealthCheckConfig into separate structs

2015-12-17 Thread Bill Farner


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

Ship it!


Shape of the API LGTM, end-to-end tests pass.

- Bill Farner


On Dec. 17, 2015, 11:21 a.m., Dmitriy Shirchenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41428/
> ---
> 
> (Updated Dec. 17, 2015, 11:21 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-1562
> https://issues.apache.org/jira/browse/AURORA-1562
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Refactoring HealthCheckConfig into separate structs
> 
> 
> Diffs
> -
> 
>   docs/configuration-reference.md 07149c7f49371e39b70e8744d60affeffd73c77f 
>   src/main/python/apache/aurora/client/config.py 
> 161c3627db0afa8fdd8afff5abf6a94556f53180 
>   src/main/python/apache/aurora/config/schema/base.py 
> e752482b95ae6377d0cf12cf0ecf0fa60e2a5d46 
>   src/main/python/apache/aurora/executor/common/health_checker.py 
> cba4e8c879fae2b9f56a36bf8fa4b702163667d1 
>   src/test/python/apache/aurora/client/test_config.py 
> 8fd112fd8a10120f57324d8efec22a009b93404b 
>   src/test/python/apache/aurora/executor/common/test_health_checker.py 
> 8561abc3e7df8803785b70064bb76553d3210399 
>   src/test/sh/org/apache/aurora/e2e/http/http_example_bad_healthcheck.aurora 
> 37f2e9c6ebc4a4bbd6089e821420e5a02179c6b9 
> 
> Diff: https://reviews.apache.org/r/41428/diff/
> 
> 
> Testing
> ---
> 
> Added unit tests.
> Changed end to end test.
> 
> 
> Thanks,
> 
> Dmitriy Shirchenko
> 
>

Re: Review Request 41521: Force Windows to always use Unix line endings.

2015-12-17 Thread John Sirois


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

Ship it!


Ship It!

- John Sirois


On Dec. 17, 2015, 11:18 a.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41521/
> ---
> 
> (Updated Dec. 17, 2015, 11:18 a.m.)
> 
> 
> Review request for Aurora and John Sirois.
> 
> 
> Bugs: AURORA-1354
> https://issues.apache.org/jira/browse/AURORA-1354
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Since everything is run inside an Ubuntu VM, we want to ensure the line 
> endings don't get auto-translated.
> 
> 
> Diffs
> -
> 
>   .gitattributes a5ab8cf17a14d2527e2a765ff94c3a74e719e0f4 
> 
> Diff: https://reviews.apache.org/r/41521/diff/
> 
> 
> Testing
> ---
> 
> Applied settings to a checkout on a windows box and verified I was able to 
> provision a VM w/ Vagrant and connect to the scheduler there.
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>

Re: Review Request 41428: Refactoring HealthCheckConfig into separate structs

2015-12-17 Thread Dmitriy Shirchenko


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

(Updated Dec. 17, 2015, 7:21 p.m.)


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


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


Repository: aurora


Description
---

Refactoring HealthCheckConfig into separate structs


Diffs (updated)
-

  docs/configuration-reference.md 07149c7f49371e39b70e8744d60affeffd73c77f 
  src/main/python/apache/aurora/client/config.py 
161c3627db0afa8fdd8afff5abf6a94556f53180 
  src/main/python/apache/aurora/config/schema/base.py 
e752482b95ae6377d0cf12cf0ecf0fa60e2a5d46 
  src/main/python/apache/aurora/executor/common/health_checker.py 
cba4e8c879fae2b9f56a36bf8fa4b702163667d1 
  src/test/python/apache/aurora/client/test_config.py 
8fd112fd8a10120f57324d8efec22a009b93404b 
  src/test/python/apache/aurora/executor/common/test_health_checker.py 
8561abc3e7df8803785b70064bb76553d3210399 
  src/test/sh/org/apache/aurora/e2e/http/http_example_bad_healthcheck.aurora 
37f2e9c6ebc4a4bbd6089e821420e5a02179c6b9 

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


Testing
---

Added unit tests.
Changed end to end test.


Thanks,

Dmitriy Shirchenko

Review Request 41528: Fixup `getJobSummary` for cron jobs with invalid next run dates.

2015-12-17 Thread John Sirois


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

Review request for Aurora, Bill Farner and Zameer Manji.


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


Repository: aurora


Description
---

Previously,  the fact `null` could be returned by Quartz when
calculating the next run was not taken into account.  Now The
CronPredictor interface makes this possibility manifest with an
Optional result.

Code that uses the CronPredictor is adjusted and tests are added.

NB: This code is as first proposed here by Brice Arnould with small
changes: https://reviews.apache.org/r/39170/

 src/main/java/org/apache/aurora/scheduler/cron/CronPredictor.java  
  | 10 --
 src/main/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImpl.java   
  |  8 +---
 src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java
  | 13 -
 
src/test/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImplTest.java
 | 23 +++
 
src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java 
 | 35 ---
 5 files changed, 72 insertions(+), 17 deletions(-)


Diffs
-

  src/main/java/org/apache/aurora/scheduler/cron/CronPredictor.java 
043ba7e6858db28001dfb07ea0c2ddf274a1c755 
  src/main/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImpl.java 
1ddc4e1946910de798f7f423dd1b19ed56dece15 
  src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 
c0e8a201400338a8cb6bc24b2c21d0abb0d01e41 
  
src/test/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImplTest.java
 b85f4abd59ef64264fb089527ad42b9ceee7f8d6 
  
src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java 
6efe03fe4841cf1275e2ee0c7cc1b9576540f34e 

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


Testing
---

Green locally: `./gradlew -Pq build`


Thanks,

John Sirois

Re: Review Request 39170: Fix NPE on accessing crons set at impossible dates

2015-12-17 Thread John Sirois



> On Nov. 15, 2015, 11:34 a.m., John Sirois wrote:
> > Brice - this would be nice to land.  Are you able to put time towards the 
> > cleanups suggested by Kevin and style fixes failing the build currently?  
> > If so - great.  If not, I can brush this up and send out a new RB.

Brice, I've taken your patch up over here: https://reviews.apache.org/r/41528/
Thanks for this fix!


- John


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


On Oct. 9, 2015, 6:41 a.m., Brice Arnould wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39170/
> ---
> 
> (Updated Oct. 9, 2015, 6:41 a.m.)
> 
> 
> Review request for Aurora and Kevin Sweeney.
> 
> 
> Bugs: AURORA-1385
> https://issues.apache.org/jira/browse/AURORA-1385
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is for AURORA-1385
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/cron/CronPredictor.java 
> 043ba7e6858db28001dfb07ea0c2ddf274a1c755 
>   
> src/main/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImpl.java 
> 1ddc4e1946910de798f7f423dd1b19ed56dece15 
>   src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 
> 13b5c222a0d7b9dd347990e6c09aac09ee566315 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java
>  64cbd5b58d9254bb741e20e47165732e52569f70 
> 
> Diff: https://reviews.apache.org/r/39170/diff/
> 
> 
> Testing
> ---
> 
> Added a specific unit test
> 
> 
> Thanks,
> 
> Brice Arnould
> 
>

Re: Review Request 41528: Fixup `getJobSummary` for cron jobs with invalid next run dates.

2015-12-17 Thread Bill Farner


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

Ship it!


Ship It!

- Bill Farner


On Dec. 17, 2015, 12:34 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41528/
> ---
> 
> (Updated Dec. 17, 2015, 12:34 p.m.)
> 
> 
> Review request for Aurora, Stephan Erb, Bill Farner, and Zameer Manji.
> 
> 
> Bugs: AURORA-1385
> https://issues.apache.org/jira/browse/AURORA-1385
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Previously,  the fact `null` could be returned by Quartz when
> calculating the next run was not taken into account.  Now The
> CronPredictor interface makes this possibility manifest with an
> Optional result.
> 
> Code that uses the CronPredictor is adjusted and tests are added.
> 
> NB: This code is as first proposed here by Brice Arnould with small
> changes: https://reviews.apache.org/r/39170/
> 
>  src/main/java/org/apache/aurora/scheduler/cron/CronPredictor.java
> | 10 --
>  src/main/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImpl.java 
> |  8 +---
>  src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java  
> | 13 -
>  
> src/test/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImplTest.java
>  | 23 +++
>  
> src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java
>   | 35 ---
>  5 files changed, 72 insertions(+), 17 deletions(-)
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/cron/CronPredictor.java 
> 043ba7e6858db28001dfb07ea0c2ddf274a1c755 
>   
> src/main/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImpl.java 
> 1ddc4e1946910de798f7f423dd1b19ed56dece15 
>   src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 
> c0e8a201400338a8cb6bc24b2c21d0abb0d01e41 
>   
> src/test/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImplTest.java
>  b85f4abd59ef64264fb089527ad42b9ceee7f8d6 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java
>  6efe03fe4841cf1275e2ee0c7cc1b9576540f34e 
> 
> Diff: https://reviews.apache.org/r/41528/diff/
> 
> 
> Testing
> ---
> 
> Green locally: `./gradlew -Pq build`
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 41473: Accept a command line argument for an executor configuration via json

2015-12-17 Thread Renan DelValle


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

(Updated Dec. 17, 2015, 8:57 p.m.)


Review request for Aurora.


Changes
---

Updated code with suggestions and taking into account Bill's patch to remove 
ExecutorConfig dup.


Repository: aurora


Description
---

Patch to allow Aurora to accept an executor config via commandline which 
overrides the default thermos one.


Diffs (updated)
-

  .gitattributes a5ab8cf17a14d2527e2a765ff94c3a74e719e0f4 
  NEWS 7a80f32465736de4fbdc7d2c7a3dbf790be31e16 
  api/src/main/thrift/org/apache/aurora/gen/api.thrift 
d765dd798e6861e3920802db00e0b96a250d1bfa 
  api/src/main/thrift/org/apache/aurora/gen/storage.thrift 
624076d1acd8d087a857ebdfeec0831e1bd8e89c 
  src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
ff886d9bc6ef44fa17bbad4fde208ab682483809 
  
src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java
 d04763418f55aa1e9f1b537987d21920c05fd950 
  
src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorSettingsLoader.java
 fafd90dbb4e628a213a5437aad8a50e7ec64ab79 
  src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
838bfc9ed6242e777d81a17b337f342b7bea72ec 
  src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 
3ede9a0c3d1718cab07385c86b774efdfccc29f8 
  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
9aef59ac5e969b01004d8d9ede825caacca04674 
  src/main/java/org/apache/aurora/scheduler/updater/InstanceActionHandler.java 
d8686f1f1b53e5ff2791663489e24c342503831e 
  
src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 
0e075607aabdc98a9c8c8c6bfa2929d3428645ff 
  src/main/java/org/apache/aurora/scheduler/updater/UpdateFactory.java 
258151feda2ea0adf0159c24c0e18a7b78b0c390 
  src/main/python/apache/aurora/client/api/__init__.py 
a6381585a2a9762202f14f929ab922806e84e8b6 
  src/main/python/apache/aurora/client/api/restarter.py 
1983034ff4e7510f7c2ca35ff9c851be24637c8e 
  src/main/python/apache/aurora/client/api/updater_util.py 
b54691eed59854ea5feb4120929cefbf1a95cd24 
  src/main/python/apache/aurora/client/cli/jobs.py 
ccc52c8357650b126185eb6100b3e2e0e37e1d45 
  src/main/python/apache/aurora/client/hooks/hooked_api.py 
6410908b48f54f5188c29f247a9f7797ec802e34 
  src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
35a4d627c5049eaec30380699e6005e8da2c10fd 
  
src/test/java/org/apache/aurora/scheduler/configuration/executor/ExecutorSettingsLoaderTest.java
 90986a5b1ed88d3f71fb8414688878aea4a312cc 
  src/test/java/org/apache/aurora/scheduler/http/AbstractJettyTest.java 
5768481005ef505d6c397101d8cc005af9d6815f 
  src/test/java/org/apache/aurora/scheduler/storage/backup/RecoveryTest.java 
97647ac3a4e99541484ac14f5796b5dffb01beb0 
  
src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java
 02400f0bbf832fdac610868cff665f4312864b8b 
  
src/test/java/org/apache/aurora/scheduler/thrift/aop/ServerInfoInterceptorTest.java
 3a80305e5169c8440b3a9d595fdaf8a52fdd15e3 
  src/test/java/org/apache/aurora/scheduler/updater/AddTaskTest.java 
0583a63a175880355a7296ebd1c6e6fb5dc99f38 
  src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
f7f5275bed4eda5e276e408bf253f5da6a9cacd8 
  src/test/java/org/apache/aurora/scheduler/updater/KillTaskTest.java 
PRE-CREATION 
  src/test/python/apache/aurora/client/api/test_api.py 
7debc7967cf926bc09f8a93cafa5059c0657e620 
  src/test/python/apache/aurora/client/api/test_restarter.py 
7557144ef1eed7ef24e69e82d04494c9f58dc217 
  src/test/python/apache/aurora/client/cli/test_restart.py 
2c51d541c4445f287d8794c0117fb038bb776329 
  src/test/python/apache/aurora/client/cli/util.py 
4b5ef4d1137cc9d545ea2205e4d14e438185a990 
  src/test/python/apache/aurora/client/hooks/test_non_hooked_api.py 
97ce9732fd9561327c8c4c4f67fefd3866110bcc 
  src/test/python/apache/aurora/common/test_transport.py 
835bb3dfebccd0666f708d0023558aee80c1667d 

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


Testing (updated)
---

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

Tested with the following config files:
CommandExec.json
```json
{
  "executor": {
"name": "commandExec",
"command": {
  "value": "echo 'Hello World from Aurora!'"
},
"resources": [
  {
"name": "cpus",
"type": "SCALAR",
"scalar": {
  "value": 0.25
}
  },
  {
"name": "mem",
"type": "SCALAR",
"scalar": {
  "value": 128
}
  }
]
  }
}
```
Thermos.json:
```json
{
  "executor": {
"name": "thermos",
"command": {
  "value": "./thermos_executor.pex",
  "arguments": [
"--announcer-enable",
"--announcer-ensemble",

Re: Review Request 41525: Add flag to set FrameworkInfo.principal

2015-12-17 Thread R.B. Boyer



> On Dec. 17, 2015, 2:53 p.m., Bill Farner wrote:
> > Thanks, overall this looks great!
> > 
> > Can you also add a line in the NEWS file to call out the new flag?  Doesn't 
> > need formal documentation, just a brief blurb is fine.

Under 0.11.0 or in an unspecified area above that?


- R.B.


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


On Dec. 17, 2015, 12:39 p.m., R.B. Boyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41525/
> ---
> 
> (Updated Dec. 17, 2015, 12:39 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Bill Farner.
> 
> 
> Bugs: AURORA-687
> https://issues.apache.org/jira/browse/AURORA-687
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Bugs closed: AURORA-687
> 
> Copying the intent from one of the trailing comments:
> 
> """
> There are several situations to consider:
> 
> 1. fresh installs that will not have authN/authZ (may want to add authN/authZ 
> later)
> 2. fresh installs that will begin life with authN/authZ (no problems in the 
> future unless the principal is changed)
> 3. existing installs that have authN (may want to add authZ later)
> 
> The current defaults are friendly for (1) above, where the typical 
> test-driver won't be turning any flavor of Auth on in their mesos cluster. 
> These defaults are good.
> 
> If someone elects to go down (2) above, the defaults you get from 
> -framework_authentication_file are not awesome, as this is where the misstep 
> lies.
> 
> If someone went down (3) above already, and used the 
> -framework_authentication_file non-awesome setting, you also don't want 
> _them_ to break their cluster.
> 
> The bare minimum change to be the least breaking would be to introduce a new 
> setting like -framework_announce_principal or something so that (2) can be 
> happy without breaking (3).
> 
> Similarly to the mesos checkpoint breaking change, you could roll this out as 
> opt-in and then in a future release make it opt-out when setting 
> -framework_authentication_file and announce it as a breaking change in the 
> changelogs for the poor folks in group (3).
> """
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java
>  68aeda1692271841d10e5f29d439806576bd691c 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java
>  513391ff82c3e985bf9f673d35414e9245807b4a 
> 
> Diff: https://reviews.apache.org/r/41525/diff/
> 
> 
> Testing
> ---
> 
> Ran normal unit tests.
> 
> Built the patched scheduler and deployed it in an authN+authZ mesos cluster 
> (3 control plane boxes, 2 execution plane boxes) and the framework could 
> actually register with mesos.
> 
> 
> Thanks,
> 
> R.B. Boyer
> 
>

Re: Review Request 41473: Accept a command line argument for an executor configuration via json

2015-12-17 Thread Bill Farner



> On Dec. 17, 2015, 1:16 p.m., Bill Farner wrote:
> > LGTM once Joshua's final comments are addressed.

Oh, actually - can you add a line to the NEWS file about the new argument?


- Bill


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


On Dec. 17, 2015, 1:10 p.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41473/
> ---
> 
> (Updated Dec. 17, 2015, 1:10 p.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Patch to allow Aurora to accept an executor config via commandline which 
> overrides the default thermos one.
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java
>  d04763418f55aa1e9f1b537987d21920c05fd950 
> 
> Diff: https://reviews.apache.org/r/41473/diff/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> Tested with the following config files:
> CommandExec.json
> ```json
> {
>   "executor": {
> "name": "commandExec",
> "command": {
>   "value": "echo 'Hello World from Aurora!'"
> },
> "resources": [
>   {
> "name": "cpus",
> "type": "SCALAR",
> "scalar": {
>   "value": 0.25
> }
>   },
>   {
> "name": "mem",
> "type": "SCALAR",
> "scalar": {
>   "value": 128
> }
>   }
> ]
>   }
> }
> ```
> Thermos.json:
> ```json
> {
>   "executor": {
> "name": "thermos",
> "command": {
>   "value": "./thermos_executor.pex",
>   "arguments": [
> "--announcer-enable",
> "--announcer-ensemble",
> "localhost:2181"
>   ],
>   "uris": [
> {
>   "value": "/home/vagrant/aurora/dist/thermos_executor.pex",
>   "executable": true,
>   "extract": false,
>   "cache": false
> }
>   ]
> },
> "resources": [
>   {
> "name": "cpus",
> "type": "SCALAR",
> "scalar": {
>   "value": 0.25
> }
>   },
>   {
> "name": "mem",
> "type": "SCALAR",
> "scalar": {
>   "value": 128
> }
>   }
> ]
>   }
> }
> ```
> 
> 
> Thanks,
> 
> Renan DelValle
> 
>

Re: Review Request 41525: Add flag to set FrameworkInfo.principal

2015-12-17 Thread R.B. Boyer


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

(Updated Dec. 17, 2015, 3:24 p.m.)


Review request for Aurora, Joshua Cohen and Bill Farner.


Changes
---

Switching to Optional for the principal


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


Repository: aurora


Description
---

Bugs closed: AURORA-687

Copying the intent from one of the trailing comments:

"""
There are several situations to consider:

1. fresh installs that will not have authN/authZ (may want to add authN/authZ 
later)
2. fresh installs that will begin life with authN/authZ (no problems in the 
future unless the principal is changed)
3. existing installs that have authN (may want to add authZ later)

The current defaults are friendly for (1) above, where the typical test-driver 
won't be turning any flavor of Auth on in their mesos cluster. These defaults 
are good.

If someone elects to go down (2) above, the defaults you get from 
-framework_authentication_file are not awesome, as this is where the misstep 
lies.

If someone went down (3) above already, and used the 
-framework_authentication_file non-awesome setting, you also don't want _them_ 
to break their cluster.

The bare minimum change to be the least breaking would be to introduce a new 
setting like -framework_announce_principal or something so that (2) can be 
happy without breaking (3).

Similarly to the mesos checkpoint breaking change, you could roll this out as 
opt-in and then in a future release make it opt-out when setting 
-framework_authentication_file and announce it as a breaking change in the 
changelogs for the poor folks in group (3).
"""


Diffs (updated)
-

  
src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java
 68aeda1692271841d10e5f29d439806576bd691c 
  
src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java
 513391ff82c3e985bf9f673d35414e9245807b4a 

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


Testing
---

Ran normal unit tests.

Built the patched scheduler and deployed it in an authN+authZ mesos cluster (3 
control plane boxes, 2 execution plane boxes) and the framework could actually 
register with mesos.


Thanks,

R.B. Boyer

Re: Review Request 30695: Implements log rotation in the Thermos runner.

2015-12-17 Thread George Sirois


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

(Updated Dec. 17, 2015, 9:45 p.m.)


Review request for Aurora, Bill Farner and Brian Wickman.


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


Repository: aurora


Description
---

Implements log rotation in the Thermos runner.


Diffs (updated)
-

  NEWS 83a1213e2ea4ab6b3706cd70c015ba9f16735520 
  docs/configuration-reference.md ad2701cadd38bb2fdbbe2acc477038986f8ec733 
  docs/deploying-aurora-scheduler.md 8a1e68e5d54e9b8b66139bfc731563668584fa77 
  src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
b3e8bf1e924999306e0b8a1314273b22c51028e7 
  src/main/python/apache/aurora/executor/thermos_task_runner.py 
14e8b4bd539d2c295582d93fa01b5613345c1758 
  src/main/python/apache/thermos/config/schema_base.py 
f9143cc1b83143d6147f59d90c79435d055d0518 
  src/main/python/apache/thermos/core/process.py 
fe95cb3be01b47616596bd78cb9a919b2e8bd978 
  src/main/python/apache/thermos/core/runner.py 
f949f279a071c6464b026749f51afc776102f2aa 
  src/main/python/apache/thermos/runner/thermos_runner.py 
bd8cf7f4cda54b6be72dad64f9446eedeb132211 
  src/test/python/apache/thermos/core/test_process.py 
5e6ad2fca616b840299bd9ca1614c82c5c39e992 

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


Testing
---

./pants test src/test/python/apache/thermos/core:all


Thanks,

George Sirois

Re: Review Request 41201: Preserve env variables for tasks in docker

2015-12-17 Thread Bill Farner


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


Thanks!  This patch LGTM overall, but it would be really nice to exercise the 
behavior in unit tests.  In particular, i'd love to see an added test case in 
each of these:
```
src/test/python/apache/aurora/executor/test_thermos_task_runner.py
src/test/python/apache/thermos/core/test_process.py
```

Please let me know if you hit any hurdles or could use pointers.

- Bill Farner


On Dec. 17, 2015, 4:27 a.m., Kasisnu Singh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41201/
> ---
> 
> (Updated Dec. 17, 2015, 4:27 a.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Preserve env variables for tasks in docker
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
> 0d02dc17da4d496560ea572659e281c736170da2 
>   src/main/python/apache/aurora/executor/thermos_task_runner.py 
> 14e8b4bd539d2c295582d93fa01b5613345c1758 
>   src/main/python/apache/thermos/core/process.py 
> f214bccbd6f3be795e0fa9a259d27b01b81ab655 
>   src/main/python/apache/thermos/core/runner.py 
> f949f279a071c6464b026749f51afc776102f2aa 
>   src/main/python/apache/thermos/runner/thermos_runner.py 
> bd8cf7f4cda54b6be72dad64f9446eedeb132211 
> 
> Diff: https://reviews.apache.org/r/41201/diff/
> 
> 
> Testing
> ---
> 
> Ran ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh with no failures 
> and did manual testing for env variables in containers.
> 
> 
> Thanks,
> 
> Kasisnu Singh
> 
>

Re: Review Request 41528: Fixup `getJobSummary` for cron jobs with invalid next run dates.

2015-12-17 Thread John Sirois



> On Dec. 17, 2015, 2:17 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/cron/CronPredictor.java, line 29
> > 
> >
> > nit:  is enough to split paragraphs.

Thanks - fixed.  I could have sworn I got -Xdoclint:all errors for this style 
recently over in pants land - but I just experimented and definitely am 
remembering wrong.


- John


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


On Dec. 17, 2015, 2:27 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41528/
> ---
> 
> (Updated Dec. 17, 2015, 2:27 p.m.)
> 
> 
> Review request for Aurora, Stephan Erb, Bill Farner, and Zameer Manji.
> 
> 
> Bugs: AURORA-1385
> https://issues.apache.org/jira/browse/AURORA-1385
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Previously,  the fact `null` could be returned by Quartz when
> calculating the next run was not taken into account.  Now The
> CronPredictor interface makes this possibility manifest with an
> Optional result.
> 
> Code that uses the CronPredictor is adjusted and tests are added.
> 
> NB: This code is as first proposed here by Brice Arnould with small
> changes: https://reviews.apache.org/r/39170/
> 
>  src/main/java/org/apache/aurora/scheduler/cron/CronPredictor.java
> | 10 --
>  src/main/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImpl.java 
> |  8 +---
>  src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java  
> | 13 -
>  
> src/test/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImplTest.java
>  | 23 +++
>  
> src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java
>   | 35 ---
>  5 files changed, 72 insertions(+), 17 deletions(-)
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/cron/CronPredictor.java 
> 043ba7e6858db28001dfb07ea0c2ddf274a1c755 
>   
> src/main/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImpl.java 
> 1ddc4e1946910de798f7f423dd1b19ed56dece15 
>   src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 
> c0e8a201400338a8cb6bc24b2c21d0abb0d01e41 
>   
> src/test/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImplTest.java
>  b85f4abd59ef64264fb089527ad42b9ceee7f8d6 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java
>  6efe03fe4841cf1275e2ee0c7cc1b9576540f34e 
> 
> Diff: https://reviews.apache.org/r/41528/diff/
> 
> 
> Testing
> ---
> 
> Green locally: `./gradlew -Pq build`
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Review Request 41531: Add a NEWS entry about shell health checks and HealthCheckConfig deprecations.

2015-12-17 Thread Joshua Cohen


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

Review request for Aurora and Bill Farner.


Repository: aurora


Description
---

Add a NEWS entry about shell health checks and HealthCheckConfig deprecations.


Diffs
-

  NEWS 89ebe0d0aac7e751722b526ec4d984a9da74a069 
  docs/configuration-reference.md ba486d83b9a86a0fc584d8e0553c8f90660cd45a 

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


Testing
---


Thanks,

Joshua Cohen

Re: Review Request 41473: Accept a command line argument for an executor configuration via json

2015-12-17 Thread Joshua Cohen


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

Ship it!


lgtm modulo a couple of style nits.


src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java
 (lines 54 - 55)


style nit: one arg per line when lines exceed 100 chars:

@CmdLine(
name = "...",
help = "...")



src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java
 (line 150)


Same here, one arg per line.


- Joshua Cohen


On Dec. 17, 2015, 9:10 p.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41473/
> ---
> 
> (Updated Dec. 17, 2015, 9:10 p.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Patch to allow Aurora to accept an executor config via commandline which 
> overrides the default thermos one.
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java
>  d04763418f55aa1e9f1b537987d21920c05fd950 
> 
> Diff: https://reviews.apache.org/r/41473/diff/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> Tested with the following config files:
> CommandExec.json
> ```json
> {
>   "executor": {
> "name": "commandExec",
> "command": {
>   "value": "echo 'Hello World from Aurora!'"
> },
> "resources": [
>   {
> "name": "cpus",
> "type": "SCALAR",
> "scalar": {
>   "value": 0.25
> }
>   },
>   {
> "name": "mem",
> "type": "SCALAR",
> "scalar": {
>   "value": 128
> }
>   }
> ]
>   }
> }
> ```
> Thermos.json:
> ```json
> {
>   "executor": {
> "name": "thermos",
> "command": {
>   "value": "./thermos_executor.pex",
>   "arguments": [
> "--announcer-enable",
> "--announcer-ensemble",
> "localhost:2181"
>   ],
>   "uris": [
> {
>   "value": "/home/vagrant/aurora/dist/thermos_executor.pex",
>   "executable": true,
>   "extract": false,
>   "cache": false
> }
>   ]
> },
> "resources": [
>   {
> "name": "cpus",
> "type": "SCALAR",
> "scalar": {
>   "value": 0.25
> }
>   },
>   {
> "name": "mem",
> "type": "SCALAR",
> "scalar": {
>   "value": 128
> }
>   }
> ]
>   }
> }
> ```
> 
> 
> Thanks,
> 
> Renan DelValle
> 
>

Re: Review Request 41473: Accept a command line argument for an executor configuration via json

2015-12-17 Thread Renan DelValle


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

(Updated Dec. 17, 2015, 9:10 p.m.)


Review request for Aurora.


Changes
---

Apologies, need to fix my other repo, this is from a clean one.


Repository: aurora


Description
---

Patch to allow Aurora to accept an executor config via commandline which 
overrides the default thermos one.


Diffs (updated)
-

  
src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java
 d04763418f55aa1e9f1b537987d21920c05fd950 

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


Testing
---

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

Tested with the following config files:
CommandExec.json
```json
{
  "executor": {
"name": "commandExec",
"command": {
  "value": "echo 'Hello World from Aurora!'"
},
"resources": [
  {
"name": "cpus",
"type": "SCALAR",
"scalar": {
  "value": 0.25
}
  },
  {
"name": "mem",
"type": "SCALAR",
"scalar": {
  "value": 128
}
  }
]
  }
}
```
Thermos.json:
```json
{
  "executor": {
"name": "thermos",
"command": {
  "value": "./thermos_executor.pex",
  "arguments": [
"--announcer-enable",
"--announcer-ensemble",
"localhost:2181"
  ],
  "uris": [
{
  "value": "/home/vagrant/aurora/dist/thermos_executor.pex",
  "executable": true,
  "extract": false,
  "cache": false
}
  ]
},
"resources": [
  {
"name": "cpus",
"type": "SCALAR",
"scalar": {
  "value": 0.25
}
  },
  {
"name": "mem",
"type": "SCALAR",
"scalar": {
  "value": 128
}
  }
]
  }
}
```


Thanks,

Renan DelValle

Re: Review Request 41531: Add a NEWS entry about shell health checks and HealthCheckConfig deprecations.

2015-12-17 Thread Bill Farner


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

Ship it!


Ship It!

- Bill Farner


On Dec. 17, 2015, 1:09 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41531/
> ---
> 
> (Updated Dec. 17, 2015, 1:09 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add a NEWS entry about shell health checks and HealthCheckConfig deprecations.
> 
> 
> Diffs
> -
> 
>   NEWS 89ebe0d0aac7e751722b526ec4d984a9da74a069 
>   docs/configuration-reference.md ba486d83b9a86a0fc584d8e0553c8f90660cd45a 
> 
> Diff: https://reviews.apache.org/r/41531/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>

Re: Review Request 41525: Add flag to set FrameworkInfo.principal

2015-12-17 Thread Bill Farner



> On Dec. 17, 2015, 12:53 p.m., Bill Farner wrote:
> > Thanks, overall this looks great!
> > 
> > Can you also add a line in the NEWS file to call out the new flag?  Doesn't 
> > need formal documentation, just a brief blurb is fine.
> 
> R.B. Boyer wrote:
> Under 0.11.0 or in an unspecified area above that?

0.11.0 please!


- Bill


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


On Dec. 17, 2015, 10:39 a.m., R.B. Boyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41525/
> ---
> 
> (Updated Dec. 17, 2015, 10:39 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Bill Farner.
> 
> 
> Bugs: AURORA-687
> https://issues.apache.org/jira/browse/AURORA-687
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Bugs closed: AURORA-687
> 
> Copying the intent from one of the trailing comments:
> 
> """
> There are several situations to consider:
> 
> 1. fresh installs that will not have authN/authZ (may want to add authN/authZ 
> later)
> 2. fresh installs that will begin life with authN/authZ (no problems in the 
> future unless the principal is changed)
> 3. existing installs that have authN (may want to add authZ later)
> 
> The current defaults are friendly for (1) above, where the typical 
> test-driver won't be turning any flavor of Auth on in their mesos cluster. 
> These defaults are good.
> 
> If someone elects to go down (2) above, the defaults you get from 
> -framework_authentication_file are not awesome, as this is where the misstep 
> lies.
> 
> If someone went down (3) above already, and used the 
> -framework_authentication_file non-awesome setting, you also don't want 
> _them_ to break their cluster.
> 
> The bare minimum change to be the least breaking would be to introduce a new 
> setting like -framework_announce_principal or something so that (2) can be 
> happy without breaking (3).
> 
> Similarly to the mesos checkpoint breaking change, you could roll this out as 
> opt-in and then in a future release make it opt-out when setting 
> -framework_authentication_file and announce it as a breaking change in the 
> changelogs for the poor folks in group (3).
> """
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java
>  68aeda1692271841d10e5f29d439806576bd691c 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java
>  513391ff82c3e985bf9f673d35414e9245807b4a 
> 
> Diff: https://reviews.apache.org/r/41525/diff/
> 
> 
> Testing
> ---
> 
> Ran normal unit tests.
> 
> Built the patched scheduler and deployed it in an authN+authZ mesos cluster 
> (3 control plane boxes, 2 execution plane boxes) and the framework could 
> actually register with mesos.
> 
> 
> Thanks,
> 
> R.B. Boyer
> 
>

Re: Review Request 41525: Add flag to set FrameworkInfo.principal

2015-12-17 Thread Joshua Cohen


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

Ship it!


lgtm pending the changes Bill requested.

- Joshua Cohen


On Dec. 17, 2015, 6:39 p.m., R.B. Boyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41525/
> ---
> 
> (Updated Dec. 17, 2015, 6:39 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Bill Farner.
> 
> 
> Bugs: AURORA-687
> https://issues.apache.org/jira/browse/AURORA-687
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Bugs closed: AURORA-687
> 
> Copying the intent from one of the trailing comments:
> 
> """
> There are several situations to consider:
> 
> 1. fresh installs that will not have authN/authZ (may want to add authN/authZ 
> later)
> 2. fresh installs that will begin life with authN/authZ (no problems in the 
> future unless the principal is changed)
> 3. existing installs that have authN (may want to add authZ later)
> 
> The current defaults are friendly for (1) above, where the typical 
> test-driver won't be turning any flavor of Auth on in their mesos cluster. 
> These defaults are good.
> 
> If someone elects to go down (2) above, the defaults you get from 
> -framework_authentication_file are not awesome, as this is where the misstep 
> lies.
> 
> If someone went down (3) above already, and used the 
> -framework_authentication_file non-awesome setting, you also don't want 
> _them_ to break their cluster.
> 
> The bare minimum change to be the least breaking would be to introduce a new 
> setting like -framework_announce_principal or something so that (2) can be 
> happy without breaking (3).
> 
> Similarly to the mesos checkpoint breaking change, you could roll this out as 
> opt-in and then in a future release make it opt-out when setting 
> -framework_authentication_file and announce it as a breaking change in the 
> changelogs for the poor folks in group (3).
> """
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java
>  68aeda1692271841d10e5f29d439806576bd691c 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java
>  513391ff82c3e985bf9f673d35414e9245807b4a 
> 
> Diff: https://reviews.apache.org/r/41525/diff/
> 
> 
> Testing
> ---
> 
> Ran normal unit tests.
> 
> Built the patched scheduler and deployed it in an authN+authZ mesos cluster 
> (3 control plane boxes, 2 execution plane boxes) and the framework could 
> actually register with mesos.
> 
> 
> Thanks,
> 
> R.B. Boyer
> 
>

Re: Review Request 41473: Accept a command line argument for an executor configuration via json

2015-12-17 Thread Bill Farner


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

Ship it!


LGTM once Joshua's final comments are addressed.

- Bill Farner


On Dec. 17, 2015, 1:10 p.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41473/
> ---
> 
> (Updated Dec. 17, 2015, 1:10 p.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Patch to allow Aurora to accept an executor config via commandline which 
> overrides the default thermos one.
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java
>  d04763418f55aa1e9f1b537987d21920c05fd950 
> 
> Diff: https://reviews.apache.org/r/41473/diff/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> Tested with the following config files:
> CommandExec.json
> ```json
> {
>   "executor": {
> "name": "commandExec",
> "command": {
>   "value": "echo 'Hello World from Aurora!'"
> },
> "resources": [
>   {
> "name": "cpus",
> "type": "SCALAR",
> "scalar": {
>   "value": 0.25
> }
>   },
>   {
> "name": "mem",
> "type": "SCALAR",
> "scalar": {
>   "value": 128
> }
>   }
> ]
>   }
> }
> ```
> Thermos.json:
> ```json
> {
>   "executor": {
> "name": "thermos",
> "command": {
>   "value": "./thermos_executor.pex",
>   "arguments": [
> "--announcer-enable",
> "--announcer-ensemble",
> "localhost:2181"
>   ],
>   "uris": [
> {
>   "value": "/home/vagrant/aurora/dist/thermos_executor.pex",
>   "executable": true,
>   "extract": false,
>   "cache": false
> }
>   ]
> },
> "resources": [
>   {
> "name": "cpus",
> "type": "SCALAR",
> "scalar": {
>   "value": 0.25
> }
>   },
>   {
> "name": "mem",
> "type": "SCALAR",
> "scalar": {
>   "value": 128
> }
>   }
> ]
>   }
> }
> ```
> 
> 
> Thanks,
> 
> Renan DelValle
> 
>

Re: Review Request 41525: Add flag to set FrameworkInfo.principal

2015-12-17 Thread R.B. Boyer


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

(Updated Dec. 17, 2015, 3:26 p.m.)


Review request for Aurora, Joshua Cohen and Bill Farner.


Changes
---

Amend NEWS


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


Repository: aurora


Description
---

Bugs closed: AURORA-687

Copying the intent from one of the trailing comments:

"""
There are several situations to consider:

1. fresh installs that will not have authN/authZ (may want to add authN/authZ 
later)
2. fresh installs that will begin life with authN/authZ (no problems in the 
future unless the principal is changed)
3. existing installs that have authN (may want to add authZ later)

The current defaults are friendly for (1) above, where the typical test-driver 
won't be turning any flavor of Auth on in their mesos cluster. These defaults 
are good.

If someone elects to go down (2) above, the defaults you get from 
-framework_authentication_file are not awesome, as this is where the misstep 
lies.

If someone went down (3) above already, and used the 
-framework_authentication_file non-awesome setting, you also don't want _them_ 
to break their cluster.

The bare minimum change to be the least breaking would be to introduce a new 
setting like -framework_announce_principal or something so that (2) can be 
happy without breaking (3).

Similarly to the mesos checkpoint breaking change, you could roll this out as 
opt-in and then in a future release make it opt-out when setting 
-framework_authentication_file and announce it as a breaking change in the 
changelogs for the poor folks in group (3).
"""


Diffs (updated)
-

  NEWS 7a80f32465736de4fbdc7d2c7a3dbf790be31e16 
  
src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java
 68aeda1692271841d10e5f29d439806576bd691c 
  
src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java
 513391ff82c3e985bf9f673d35414e9245807b4a 

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


Testing
---

Ran normal unit tests.

Built the patched scheduler and deployed it in an authN+authZ mesos cluster (3 
control plane boxes, 2 execution plane boxes) and the framework could actually 
register with mesos.


Thanks,

R.B. Boyer

Re: Review Request 41525: Add flag to set FrameworkInfo.principal

2015-12-17 Thread Bill Farner


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



src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java
 (line 73)


FYI since JDK8 the type witness should be unnecessary, so this can be 
`Optional.absent()`.

Ditto below.


- Bill Farner


On Dec. 17, 2015, 1:26 p.m., R.B. Boyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41525/
> ---
> 
> (Updated Dec. 17, 2015, 1:26 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Bill Farner.
> 
> 
> Bugs: AURORA-687
> https://issues.apache.org/jira/browse/AURORA-687
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Bugs closed: AURORA-687
> 
> Copying the intent from one of the trailing comments:
> 
> """
> There are several situations to consider:
> 
> 1. fresh installs that will not have authN/authZ (may want to add authN/authZ 
> later)
> 2. fresh installs that will begin life with authN/authZ (no problems in the 
> future unless the principal is changed)
> 3. existing installs that have authN (may want to add authZ later)
> 
> The current defaults are friendly for (1) above, where the typical 
> test-driver won't be turning any flavor of Auth on in their mesos cluster. 
> These defaults are good.
> 
> If someone elects to go down (2) above, the defaults you get from 
> -framework_authentication_file are not awesome, as this is where the misstep 
> lies.
> 
> If someone went down (3) above already, and used the 
> -framework_authentication_file non-awesome setting, you also don't want 
> _them_ to break their cluster.
> 
> The bare minimum change to be the least breaking would be to introduce a new 
> setting like -framework_announce_principal or something so that (2) can be 
> happy without breaking (3).
> 
> Similarly to the mesos checkpoint breaking change, you could roll this out as 
> opt-in and then in a future release make it opt-out when setting 
> -framework_authentication_file and announce it as a breaking change in the 
> changelogs for the poor folks in group (3).
> """
> 
> 
> Diffs
> -
> 
>   NEWS 7a80f32465736de4fbdc7d2c7a3dbf790be31e16 
>   
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java
>  68aeda1692271841d10e5f29d439806576bd691c 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java
>  513391ff82c3e985bf9f673d35414e9245807b4a 
> 
> Diff: https://reviews.apache.org/r/41525/diff/
> 
> 
> Testing
> ---
> 
> Ran normal unit tests.
> 
> Built the patched scheduler and deployed it in an authN+authZ mesos cluster 
> (3 control plane boxes, 2 execution plane boxes) and the framework could 
> actually register with mesos.
> 
> 
> Thanks,
> 
> R.B. Boyer
> 
>

Re: Review Request 30695: Implements log rotation in the Thermos runner.

2015-12-17 Thread Bill Farner


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

Ship it!


Ship It!

- Bill Farner


On Dec. 17, 2015, 12:38 p.m., George Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30695/
> ---
> 
> (Updated Dec. 17, 2015, 12:38 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Brian Wickman.
> 
> 
> Bugs: AURORA-95
> https://issues.apache.org/jira/browse/AURORA-95
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Implements log rotation in the Thermos runner.
> 
> 
> Diffs
> -
> 
>   docs/configuration-reference.md ad2701cadd38bb2fdbbe2acc477038986f8ec733 
>   docs/deploying-aurora-scheduler.md 8a1e68e5d54e9b8b66139bfc731563668584fa77 
>   src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
> b3e8bf1e924999306e0b8a1314273b22c51028e7 
>   src/main/python/apache/aurora/executor/thermos_task_runner.py 
> 14e8b4bd539d2c295582d93fa01b5613345c1758 
>   src/main/python/apache/thermos/config/schema_base.py 
> f9143cc1b83143d6147f59d90c79435d055d0518 
>   src/main/python/apache/thermos/core/process.py 
> fe95cb3be01b47616596bd78cb9a919b2e8bd978 
>   src/main/python/apache/thermos/core/runner.py 
> f949f279a071c6464b026749f51afc776102f2aa 
>   src/main/python/apache/thermos/runner/thermos_runner.py 
> bd8cf7f4cda54b6be72dad64f9446eedeb132211 
>   src/test/python/apache/thermos/core/test_process.py 
> 5e6ad2fca616b840299bd9ca1614c82c5c39e992 
> 
> Diff: https://reviews.apache.org/r/30695/diff/
> 
> 
> Testing
> ---
> 
> ./pants test src/test/python/apache/thermos/core:all
> 
> 
> Thanks,
> 
> George Sirois
> 
>

Review Request 41534: Update leader redirection logic to return an error page if there is no leading scheduler.

2015-12-17 Thread Joshua Cohen


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

Review request for Aurora, Maxim Khutornenko and Bill Farner.


Repository: aurora


Description
---

We'd like to serve Aurora through DNS rather than against the singleton 
serverset so that when there is no leader users get a friendly error page 
rather than a generic error (which inevitably leads to them coming to ask for 
support). In order to accomplish this, I've updated the `LeaderRedirectFilter` 
to return a 503 with an html body in the event there is no elected leader.


Diffs
-

  src/main/java/org/apache/aurora/scheduler/http/LeaderRedirect.java 
2c86ddb0a44bcec03cd9305fa96201c214fc731c 
  src/main/java/org/apache/aurora/scheduler/http/LeaderRedirectFilter.java 
51566e9ccddfd4921ba33f0ecfbf303fdab31549 
  src/main/resources/org/apache/aurora/scheduler/http/no-leader.html 
PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/http/AbstractJettyTest.java 
152af0d511fdc26906a3f7428024e5af08daab6d 
  src/test/java/org/apache/aurora/scheduler/http/LeaderRedirectTest.java 
98b6ef6304521c5f99a187e1c6d83a135e4cfabd 
  src/test/java/org/apache/aurora/scheduler/http/ServletFilterTest.java 
af1e5f1c4c82ab2450c1487af67bc8907675822c 

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


Testing
---

./gradlew build -Pq

Also verified in Vagrant (by preventing the scheduler from leading) that 
accessing the scheduler in that state returns the expected page.
Additionally verified that client error message is still usable:

WARN] Connection error with scheduler: Unknown error talking to 
http://192.168.33.7:8081/api: 503 Server Error: Service Unavailable, 
reconnecting...


File Attachments


No Leader page
  
https://reviews.apache.org/media/uploaded/files/2015/12/17/2759a88b-a2c4-4c45-a45f-037126e30f1e__Screen_Shot_2015-12-17_at_5.02.51_PM.png


Thanks,

Joshua Cohen

Re: Review Request 41201: Preserve env variables for tasks in docker

2015-12-17 Thread Kasisnu Singh


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

(Updated Dec. 17, 2015, 12:27 p.m.)


Review request for Aurora.


Repository: aurora


Description
---

Preserve env variables for tasks in docker


Diffs (updated)
-

  src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
0d02dc17da4d496560ea572659e281c736170da2 
  src/main/python/apache/aurora/executor/thermos_task_runner.py 
14e8b4bd539d2c295582d93fa01b5613345c1758 
  src/main/python/apache/thermos/core/process.py 
f214bccbd6f3be795e0fa9a259d27b01b81ab655 
  src/main/python/apache/thermos/core/runner.py 
f949f279a071c6464b026749f51afc776102f2aa 
  src/main/python/apache/thermos/runner/thermos_runner.py 
bd8cf7f4cda54b6be72dad64f9446eedeb132211 

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


Testing
---

Ran ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh with no failures and 
did manual testing for env variables in containers.


Thanks,

Kasisnu Singh

Re: Review Request 41486: Deprecate JobUpdateSettings.maxWaitToInstanceRunning and UpdateConfig.restart_threshold

2015-12-17 Thread Aurora ReviewBot


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

Ship it!


Master (c912c34) 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 Dec. 17, 2015, 6:52 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41486/
> ---
> 
> (Updated Dec. 17, 2015, 6:52 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-1252 and AURORA-1253
> https://issues.apache.org/jira/browse/AURORA-1252
> https://issues.apache.org/jira/browse/AURORA-1253
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This pulled a longer thread than i was hoping, since the python class 
> `UpdaterConfig` was overloaded for use by the rolling restart command.
> 
> Within the scheduler, removal of this field has little behavioral difference. 
>  The effect is there are some conditions where the scheduler is event-driven 
> instead of polling (which was more appropriate anyway).
> 
> 
> Diffs
> -
> 
>   NEWS 7a80f32465736de4fbdc7d2c7a3dbf790be31e16 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> d765dd798e6861e3920802db00e0b96a250d1bfa 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  9aef59ac5e969b01004d8d9ede825caacca04674 
>   
> src/main/java/org/apache/aurora/scheduler/updater/InstanceActionHandler.java 
> 0880cf2413063c0d1f28e872737872d6e4710bb7 
>   
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java
>  0e075607aabdc98a9c8c8c6bfa2929d3428645ff 
>   src/main/java/org/apache/aurora/scheduler/updater/UpdateFactory.java 
> 258151feda2ea0adf0159c24c0e18a7b78b0c390 
>   src/main/python/apache/aurora/client/api/__init__.py 
> a6381585a2a9762202f14f929ab922806e84e8b6 
>   src/main/python/apache/aurora/client/api/restarter.py 
> 1983034ff4e7510f7c2ca35ff9c851be24637c8e 
>   src/main/python/apache/aurora/client/api/updater_util.py 
> b54691eed59854ea5feb4120929cefbf1a95cd24 
>   src/main/python/apache/aurora/client/cli/jobs.py 
> ccc52c8357650b126185eb6100b3e2e0e37e1d45 
>   src/main/python/apache/aurora/client/hooks/hooked_api.py 
> 6410908b48f54f5188c29f247a9f7797ec802e34 
>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
> f7f5275bed4eda5e276e408bf253f5da6a9cacd8 
>   src/test/python/apache/aurora/client/api/test_api.py 
> 7debc7967cf926bc09f8a93cafa5059c0657e620 
>   src/test/python/apache/aurora/client/api/test_restarter.py 
> 7557144ef1eed7ef24e69e82d04494c9f58dc217 
>   src/test/python/apache/aurora/client/cli/test_restart.py 
> 2c51d541c4445f287d8794c0117fb038bb776329 
>   src/test/python/apache/aurora/client/hooks/test_non_hooked_api.py 
> 97ce9732fd9561327c8c4c4f67fefd3866110bcc 
> 
> Diff: https://reviews.apache.org/r/41486/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 41534: Update leader redirection logic to return an error page if there is no leading scheduler.

2015-12-17 Thread Aurora ReviewBot


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

Ship it!


Master (93fb2c7) 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 Dec. 17, 2015, 11:03 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41534/
> ---
> 
> (Updated Dec. 17, 2015, 11:03 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> We'd like to serve Aurora through DNS rather than against the singleton 
> serverset so that when there is no leader users get a friendly error page 
> rather than a generic error (which inevitably leads to them coming to ask for 
> support). In order to accomplish this, I've updated the 
> `LeaderRedirectFilter` to return a 503 with an html body in the event there 
> is no elected leader.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/http/LeaderRedirect.java 
> 2c86ddb0a44bcec03cd9305fa96201c214fc731c 
>   src/main/java/org/apache/aurora/scheduler/http/LeaderRedirectFilter.java 
> 51566e9ccddfd4921ba33f0ecfbf303fdab31549 
>   src/main/resources/org/apache/aurora/scheduler/http/no-leader.html 
> PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/http/AbstractJettyTest.java 
> 152af0d511fdc26906a3f7428024e5af08daab6d 
>   src/test/java/org/apache/aurora/scheduler/http/LeaderRedirectTest.java 
> 98b6ef6304521c5f99a187e1c6d83a135e4cfabd 
>   src/test/java/org/apache/aurora/scheduler/http/ServletFilterTest.java 
> af1e5f1c4c82ab2450c1487af67bc8907675822c 
> 
> Diff: https://reviews.apache.org/r/41534/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build -Pq
> 
> Also verified in Vagrant (by preventing the scheduler from leading) that 
> accessing the scheduler in that state returns the expected page.
> Additionally verified that client error message is still usable:
> 
> WARN] Connection error with scheduler: Unknown error talking to 
> http://192.168.33.7:8081/api: 503 Server Error: Service Unavailable, 
> reconnecting...
> 
> 
> File Attachments
> 
> 
> No Leader page
>   
> https://reviews.apache.org/media/uploaded/files/2015/12/17/2759a88b-a2c4-4c45-a45f-037126e30f1e__Screen_Shot_2015-12-17_at_5.02.51_PM.png
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>

Re: Review Request 41528: Fixup `getJobSummary` for cron jobs with invalid next run dates.

2015-12-17 Thread Aurora ReviewBot


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

Ship it!


Master (93fb2c7) 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 Dec. 17, 2015, 9:27 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41528/
> ---
> 
> (Updated Dec. 17, 2015, 9:27 p.m.)
> 
> 
> Review request for Aurora, Stephan Erb, Bill Farner, and Zameer Manji.
> 
> 
> Bugs: AURORA-1385
> https://issues.apache.org/jira/browse/AURORA-1385
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Previously,  the fact `null` could be returned by Quartz when
> calculating the next run was not taken into account.  Now The
> CronPredictor interface makes this possibility manifest with an
> Optional result.
> 
> Code that uses the CronPredictor is adjusted and tests are added.
> 
> NB: This code is as first proposed here by Brice Arnould with small
> changes: https://reviews.apache.org/r/39170/
> 
>  src/main/java/org/apache/aurora/scheduler/cron/CronPredictor.java
> | 10 --
>  src/main/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImpl.java 
> |  8 +---
>  src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java  
> | 13 -
>  
> src/test/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImplTest.java
>  | 23 +++
>  
> src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java
>   | 35 ---
>  5 files changed, 72 insertions(+), 17 deletions(-)
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/cron/CronPredictor.java 
> 043ba7e6858db28001dfb07ea0c2ddf274a1c755 
>   
> src/main/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImpl.java 
> 1ddc4e1946910de798f7f423dd1b19ed56dece15 
>   src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 
> c0e8a201400338a8cb6bc24b2c21d0abb0d01e41 
>   
> src/test/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImplTest.java
>  b85f4abd59ef64264fb089527ad42b9ceee7f8d6 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java
>  6efe03fe4841cf1275e2ee0c7cc1b9576540f34e 
> 
> Diff: https://reviews.apache.org/r/41528/diff/
> 
> 
> Testing
> ---
> 
> Green locally: `./gradlew -Pq build`
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 41534: Update leader redirection logic to return an error page if there is no leading scheduler.

2015-12-17 Thread Maxim Khutornenko


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

Ship it!


Ship It!

- Maxim Khutornenko


On Dec. 17, 2015, 11:03 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41534/
> ---
> 
> (Updated Dec. 17, 2015, 11:03 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> We'd like to serve Aurora through DNS rather than against the singleton 
> serverset so that when there is no leader users get a friendly error page 
> rather than a generic error (which inevitably leads to them coming to ask for 
> support). In order to accomplish this, I've updated the 
> `LeaderRedirectFilter` to return a 503 with an html body in the event there 
> is no elected leader.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/http/LeaderRedirect.java 
> 2c86ddb0a44bcec03cd9305fa96201c214fc731c 
>   src/main/java/org/apache/aurora/scheduler/http/LeaderRedirectFilter.java 
> 51566e9ccddfd4921ba33f0ecfbf303fdab31549 
>   src/main/resources/org/apache/aurora/scheduler/http/no-leader.html 
> PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/http/AbstractJettyTest.java 
> 152af0d511fdc26906a3f7428024e5af08daab6d 
>   src/test/java/org/apache/aurora/scheduler/http/LeaderRedirectTest.java 
> 98b6ef6304521c5f99a187e1c6d83a135e4cfabd 
>   src/test/java/org/apache/aurora/scheduler/http/ServletFilterTest.java 
> af1e5f1c4c82ab2450c1487af67bc8907675822c 
> 
> Diff: https://reviews.apache.org/r/41534/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build -Pq
> 
> Also verified in Vagrant (by preventing the scheduler from leading) that 
> accessing the scheduler in that state returns the expected page.
> Additionally verified that client error message is still usable:
> 
> WARN] Connection error with scheduler: Unknown error talking to 
> http://192.168.33.7:8081/api: 503 Server Error: Service Unavailable, 
> reconnecting...
> 
> 
> File Attachments
> 
> 
> No Leader page
>   
> https://reviews.apache.org/media/uploaded/files/2015/12/17/2759a88b-a2c4-4c45-a45f-037126e30f1e__Screen_Shot_2015-12-17_at_5.02.51_PM.png
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>

46 matches

Mail list logo