Re: Review Request 30768: Reject None values for TaskPath

2015-02-09 Thread Brian Wickman 

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

Ship it!


Ship It!

- Brian Wickman


On Feb. 8, 2015, 9:34 p.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30768/
> ---
> 
> (Updated Feb. 8, 2015, 9:34 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Brian Wickman.
> 
> 
> Bugs: AURORA-1115
> https://issues.apache.org/jira/browse/AURORA-1115
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This patch modifies TaskPath to reject None values.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/executor/thermos_task_runner.py 
> 7b346e253677ee9b42c57782f7f67ff63b6a0083 
>   src/main/python/apache/thermos/common/path.py 
> 9e617051f16f4270b3958f48e0cc43706d245eec 
>   
> src/test/python/apache/aurora/executor/common/test_resource_manager_integration.py
>  PRE-CREATION 
>   src/test/python/apache/thermos/common/test_pathspec.py 
> 3437b196d33d7c2ff6ba292ff99b6881954e7ecb 
>   src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 
> 989801cfcbd19109ac140b01cd3024d70c78c829 
> 
> Diff: https://reviews.apache.org/r/30768/diff/
> 
> 
> Testing
> ---
> 
> ./pants test src/test/python/apache/thermos::
> ./pants test src/test/python/apache/aurora/executor::
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>

Re: Review Request 30749: Port GC executor to PathDetector interface

2015-02-09 Thread Brian Wickman 


> On Feb. 8, 2015, 2:20 a.m., Zameer Manji wrote:
> > src/main/python/apache/aurora/executor/gc_executor.py, line 73
> > 
> >
> > What's the benefit of using a namedtuple here?

As opposed to?  This means we don't have to change every function signature in 
this file.  (I suppose on the other hand we get less "type safety" because 
functions called w/o the correct number of parameters would at least TypeError.)


> On Feb. 8, 2015, 2:20 a.m., Zameer Manji wrote:
> > src/main/python/apache/aurora/executor/gc_executor.py, line 411
> > 
> >
> > Shouldn't this log messsage be inside the collector implementation?

The gc executor is on its deathbed -- I propose not changing anything not 
completely necessary for its deprecation.


- Brian


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


On Feb. 7, 2015, 2:27 a.m., Brian Wickman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30749/
> ---
> 
> (Updated Feb. 7, 2015, 2:27 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Zameer Manji.
> 
> 
> Bugs: AURORA-1025
> https://issues.apache.org/jira/browse/AURORA-1025
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This makes the GC executor detect checkpoint roots via the PathDetector 
> interface.  This paves the way to detecting checkpoint roots both from 
> /var/run/thermos and from /var/lib/mesos/**
> 
> 
> Diffs
> -
> 
>   pants.ini 99648e46ca755c3c8e22d90c7b682107d8dee333 
>   src/main/python/apache/aurora/executor/bin/BUILD 
> 1fa1e9e839bf28093b4e9ded403a761cf4bf5f44 
>   src/main/python/apache/aurora/executor/bin/gc_executor_main.py 
> b903bcb3630a8a8d50a2008bfae532b2eb947356 
>   src/main/python/apache/aurora/executor/gc_executor.py 
> 43b415bc6c5177be24ec036cc32ae7cbd87fc70f 
>   src/main/python/apache/thermos/bin/thermos.py 
> 161bbdbc4de95c82e2b596e77b0eac7b920eae66 
>   src/main/python/apache/thermos/monitoring/garbage.py 
> 69bf8e4c2e2e5f85f6b822fbe45f828d61814d7f 
>   src/test/python/apache/aurora/executor/test_gc_executor.py 
> b1bbc89a822302d8ea12324eb767631326639ebb 
> 
> Diff: https://reviews.apache.org/r/30749/diff/
> 
> 
> Testing
> ---
> 
> ./pants test.pytest --no-fast src/main/python::
> 
> 
> Thanks,
> 
> Brian Wickman
> 
>

Re: Review Request 30749: Port GC executor to PathDetector interface

2015-02-09 Thread Brian Wickman 

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

(Updated Feb. 9, 2015, 6:45 p.m.)


Review request for Aurora, Joshua Cohen and Zameer Manji.


Changes
---

Revert unintended pants.ini change.  Pull in a slice of r/30768 to pass e2e.


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


Repository: aurora


Description
---

This makes the GC executor detect checkpoint roots via the PathDetector 
interface.  This paves the way to detecting checkpoint roots both from 
/var/run/thermos and from /var/lib/mesos/**


Diffs (updated)
-

  src/main/python/apache/aurora/executor/bin/BUILD 
1fa1e9e839bf28093b4e9ded403a761cf4bf5f44 
  src/main/python/apache/aurora/executor/bin/gc_executor_main.py 
b903bcb3630a8a8d50a2008bfae532b2eb947356 
  src/main/python/apache/aurora/executor/gc_executor.py 
43b415bc6c5177be24ec036cc32ae7cbd87fc70f 
  src/main/python/apache/aurora/executor/thermos_task_runner.py 
9ff8c5379aad7ac05e44b1f5a2b74f759f26 
  src/main/python/apache/thermos/bin/thermos.py 
161bbdbc4de95c82e2b596e77b0eac7b920eae66 
  src/main/python/apache/thermos/monitoring/garbage.py 
69bf8e4c2e2e5f85f6b822fbe45f828d61814d7f 
  src/test/python/apache/aurora/executor/test_gc_executor.py 
b1bbc89a822302d8ea12324eb767631326639ebb 

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


Testing
---

./pants test.pytest --no-fast src/main/python::


Thanks,

Brian Wickman

Re: Review Request 30768: Reject None values for TaskPath

2015-02-09 Thread Zameer Manji 

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

(Updated Feb. 9, 2015, 10:47 a.m.)


Review request for Aurora, Bill Farner and Brian Wickman.


Changes
---

Style Fixes.


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


Repository: aurora


Description
---

This patch modifies TaskPath to reject None values.


Diffs (updated)
-

  src/main/python/apache/aurora/executor/thermos_task_runner.py 
7b346e253677ee9b42c57782f7f67ff63b6a0083 
  src/main/python/apache/thermos/common/path.py 
9e617051f16f4270b3958f48e0cc43706d245eec 
  
src/test/python/apache/aurora/executor/common/test_resource_manager_integration.py
 PRE-CREATION 
  src/test/python/apache/thermos/common/test_pathspec.py 
3437b196d33d7c2ff6ba292ff99b6881954e7ecb 
  src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 
989801cfcbd19109ac140b01cd3024d70c78c829 

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


Testing
---

./pants test src/test/python/apache/thermos::
./pants test src/test/python/apache/aurora/executor::
./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh


Thanks,

Zameer Manji

Re: Review Request 30768: Reject None values for TaskPath

2015-02-09 Thread Zameer Manji 


> On Feb. 8, 2015, 3:20 p.m., Bill Farner wrote:
> > src/test/python/apache/aurora/executor/common/test_resource_manager_integration.py,
> >  line 42
> > 
> >
> > This source now has 3 styles for continued lines:
> > 
> > - indent matching the first argument
> > - +2 indent
> > - +4 indent
> > 
> > AFAIK +4 indent is the agreed-upon convention.  Can you fix the whole 
> > file to match that?

Done.


- Zameer


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


On Feb. 9, 2015, 10:47 a.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30768/
> ---
> 
> (Updated Feb. 9, 2015, 10:47 a.m.)
> 
> 
> Review request for Aurora, Bill Farner and Brian Wickman.
> 
> 
> Bugs: AURORA-1115
> https://issues.apache.org/jira/browse/AURORA-1115
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This patch modifies TaskPath to reject None values.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/executor/thermos_task_runner.py 
> 7b346e253677ee9b42c57782f7f67ff63b6a0083 
>   src/main/python/apache/thermos/common/path.py 
> 9e617051f16f4270b3958f48e0cc43706d245eec 
>   
> src/test/python/apache/aurora/executor/common/test_resource_manager_integration.py
>  PRE-CREATION 
>   src/test/python/apache/thermos/common/test_pathspec.py 
> 3437b196d33d7c2ff6ba292ff99b6881954e7ecb 
>   src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 
> 989801cfcbd19109ac140b01cd3024d70c78c829 
> 
> Diff: https://reviews.apache.org/r/30768/diff/
> 
> 
> Testing
> ---
> 
> ./pants test src/test/python/apache/thermos::
> ./pants test src/test/python/apache/aurora/executor::
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>

Re: Review Request 30749: Port GC executor to PathDetector interface

2015-02-09 Thread Aurora ReviewBot 

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


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

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

- Aurora ReviewBot


On Feb. 9, 2015, 6:45 p.m., Brian Wickman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30749/
> ---
> 
> (Updated Feb. 9, 2015, 6:45 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Zameer Manji.
> 
> 
> Bugs: AURORA-1025
> https://issues.apache.org/jira/browse/AURORA-1025
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This makes the GC executor detect checkpoint roots via the PathDetector 
> interface.  This paves the way to detecting checkpoint roots both from 
> /var/run/thermos and from /var/lib/mesos/**
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/executor/bin/BUILD 
> 1fa1e9e839bf28093b4e9ded403a761cf4bf5f44 
>   src/main/python/apache/aurora/executor/bin/gc_executor_main.py 
> b903bcb3630a8a8d50a2008bfae532b2eb947356 
>   src/main/python/apache/aurora/executor/gc_executor.py 
> 43b415bc6c5177be24ec036cc32ae7cbd87fc70f 
>   src/main/python/apache/aurora/executor/thermos_task_runner.py 
> 9ff8c5379aad7ac05e44b1f5a2b74f759f26 
>   src/main/python/apache/thermos/bin/thermos.py 
> 161bbdbc4de95c82e2b596e77b0eac7b920eae66 
>   src/main/python/apache/thermos/monitoring/garbage.py 
> 69bf8e4c2e2e5f85f6b822fbe45f828d61814d7f 
>   src/test/python/apache/aurora/executor/test_gc_executor.py 
> b1bbc89a822302d8ea12324eb767631326639ebb 
> 
> Diff: https://reviews.apache.org/r/30749/diff/
> 
> 
> Testing
> ---
> 
> ./pants test.pytest --no-fast src/main/python::
> 
> 
> Thanks,
> 
> Brian Wickman
> 
>

Re: Review Request 30749: Port GC executor to PathDetector interface

2015-02-09 Thread Brian Wickman 

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

(Updated Feb. 9, 2015, 6:56 p.m.)


Review request for Aurora, Joshua Cohen and Zameer Manji.


Changes
---

Post master merge.


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


Repository: aurora


Description
---

This makes the GC executor detect checkpoint roots via the PathDetector 
interface.  This paves the way to detecting checkpoint roots both from 
/var/run/thermos and from /var/lib/mesos/**


Diffs (updated)
-

  src/main/python/apache/aurora/executor/bin/BUILD 
1fa1e9e839bf28093b4e9ded403a761cf4bf5f44 
  src/main/python/apache/aurora/executor/bin/gc_executor_main.py 
b903bcb3630a8a8d50a2008bfae532b2eb947356 
  src/main/python/apache/aurora/executor/gc_executor.py 
43b415bc6c5177be24ec036cc32ae7cbd87fc70f 
  src/main/python/apache/thermos/bin/thermos.py 
161bbdbc4de95c82e2b596e77b0eac7b920eae66 
  src/main/python/apache/thermos/monitoring/garbage.py 
69bf8e4c2e2e5f85f6b822fbe45f828d61814d7f 
  src/test/python/apache/aurora/executor/test_gc_executor.py 
b1bbc89a822302d8ea12324eb767631326639ebb 

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


Testing
---

./pants test.pytest --no-fast src/main/python::


Thanks,

Brian Wickman

Re: Review Request 30749: Port GC executor to PathDetector interface

2015-02-09 Thread Aurora ReviewBot 

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

Ship it!


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

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

- Aurora ReviewBot


On Feb. 9, 2015, 6:56 p.m., Brian Wickman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30749/
> ---
> 
> (Updated Feb. 9, 2015, 6:56 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Zameer Manji.
> 
> 
> Bugs: AURORA-1025
> https://issues.apache.org/jira/browse/AURORA-1025
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This makes the GC executor detect checkpoint roots via the PathDetector 
> interface.  This paves the way to detecting checkpoint roots both from 
> /var/run/thermos and from /var/lib/mesos/**
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/executor/bin/BUILD 
> 1fa1e9e839bf28093b4e9ded403a761cf4bf5f44 
>   src/main/python/apache/aurora/executor/bin/gc_executor_main.py 
> b903bcb3630a8a8d50a2008bfae532b2eb947356 
>   src/main/python/apache/aurora/executor/gc_executor.py 
> 43b415bc6c5177be24ec036cc32ae7cbd87fc70f 
>   src/main/python/apache/thermos/bin/thermos.py 
> 161bbdbc4de95c82e2b596e77b0eac7b920eae66 
>   src/main/python/apache/thermos/monitoring/garbage.py 
> 69bf8e4c2e2e5f85f6b822fbe45f828d61814d7f 
>   src/test/python/apache/aurora/executor/test_gc_executor.py 
> b1bbc89a822302d8ea12324eb767631326639ebb 
> 
> Diff: https://reviews.apache.org/r/30749/diff/
> 
> 
> Testing
> ---
> 
> ./pants test.pytest --no-fast src/main/python::
> 
> 
> Thanks,
> 
> Brian Wickman
> 
>

Re: Review Request 30749: Port GC executor to PathDetector interface

2015-02-09 Thread Zameer Manji 

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

Ship it!


There is a lot of churn because of interface changes but this change LGTM.

- Zameer Manji


On Feb. 9, 2015, 10:56 a.m., Brian Wickman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30749/
> ---
> 
> (Updated Feb. 9, 2015, 10:56 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Zameer Manji.
> 
> 
> Bugs: AURORA-1025
> https://issues.apache.org/jira/browse/AURORA-1025
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This makes the GC executor detect checkpoint roots via the PathDetector 
> interface.  This paves the way to detecting checkpoint roots both from 
> /var/run/thermos and from /var/lib/mesos/**
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/executor/bin/BUILD 
> 1fa1e9e839bf28093b4e9ded403a761cf4bf5f44 
>   src/main/python/apache/aurora/executor/bin/gc_executor_main.py 
> b903bcb3630a8a8d50a2008bfae532b2eb947356 
>   src/main/python/apache/aurora/executor/gc_executor.py 
> 43b415bc6c5177be24ec036cc32ae7cbd87fc70f 
>   src/main/python/apache/thermos/bin/thermos.py 
> 161bbdbc4de95c82e2b596e77b0eac7b920eae66 
>   src/main/python/apache/thermos/monitoring/garbage.py 
> 69bf8e4c2e2e5f85f6b822fbe45f828d61814d7f 
>   src/test/python/apache/aurora/executor/test_gc_executor.py 
> b1bbc89a822302d8ea12324eb767631326639ebb 
> 
> Diff: https://reviews.apache.org/r/30749/diff/
> 
> 
> Testing
> ---
> 
> ./pants test.pytest --no-fast src/main/python::
> 
> 
> Thanks,
> 
> Brian Wickman
> 
>

Review Request 30804: Fixing test coverage in startJobUpdate RPC

2015-02-09 Thread Maxim Khutornenko 

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

Review request for Aurora and Bill Farner.


Repository: aurora


Description
---

The startJobUpdate RPC test coverage got broken in 
9fe6d5408d4aed113a239f22fa5c43aa4f9ae338 as all settings tests short-circuited 
at `isIsService()` validation.


Diffs
-

  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
2b5383f3c56927bc42ab47c0a5c7c4b1d41af6dd 
  
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
 c6fe218c709c73dbd4c3aeeb94e211ee6cdf9a0d 

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


Testing
---

./gradlew -Pq build


Thanks,

Maxim Khutornenko

Re: Review Request 30804: Fixing test coverage in startJobUpdate RPC

2015-02-09 Thread Aurora ReviewBot 

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

Ship it!


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

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

- Aurora ReviewBot


On Feb. 9, 2015, 7:57 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30804/
> ---
> 
> (Updated Feb. 9, 2015, 7:57 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The startJobUpdate RPC test coverage got broken in 
> 9fe6d5408d4aed113a239f22fa5c43aa4f9ae338 as all settings tests 
> short-circuited at `isIsService()` validation.
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  2b5383f3c56927bc42ab47c0a5c7c4b1d41af6dd 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  c6fe218c709c73dbd4c3aeeb94e211ee6cdf9a0d 
> 
> Diff: https://reviews.apache.org/r/30804/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 30749: Port GC executor to PathDetector interface

2015-02-09 Thread Brian Wickman 

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

(Updated Feb. 9, 2015, 8:22 p.m.)


Review request for Aurora, Joshua Cohen and Zameer Manji.


Changes
---

Adds entry point tests for gc/thermos executors, since I got some of the import 
wiring wrong:

vagrant@192:/var/lib/mesos/slaves/20150209-182237-119646400-5050-15827-S0/frameworks/20150209-182237-119646400-5050-15827-/executors/aurora.gc/runs/latest$
 cat stderr
WARNING: Logging before InitGoogleLogging() is written to STDERR
I0209 19:16:58.301468 22286 fetcher.cpp:76] Fetching URI 
'/home/vagrant/aurora/dist/gc_executor.pex'
I0209 19:16:58.301730 22286 fetcher.cpp:179] Copying resource from 
'/home/vagrant/aurora/dist/gc_executor.pex' to 
'/var/lib/mesos/slaves/20150209-182237-119646400-5050-15827-S0/frameworks/20150209-182237-119646400-5050-15827-/executors/aurora.gc/runs/6f1f2749-66a9-41e7-bb68-5e31c08cac6d'
Traceback (most recent call last):
  File 
"/var/lib/mesos/slaves/20150209-182237-119646400-5050-15827-S0/frameworks/20150209-182237-119646400-5050-15827-/executors/aurora.gc/runs/6f1f2749-66a9-41e7-bb68-5e31c08cac6d/gc_executor.pex/.bootstrap/_pex/pex.py",
 line 272, in execute
self.execute_entry(entry_point, args)
  File 
"/var/lib/mesos/slaves/20150209-182237-119646400-5050-15827-S0/frameworks/20150209-182237-119646400-5050-15827-/executors/aurora.gc/runs/6f1f2749-66a9-41e7-bb68-5e31c08cac6d/gc_executor.pex/.bootstrap/_pex/pex.py",
 line 320, in execute_entry
runner(entry_point)
  File 
"/var/lib/mesos/slaves/20150209-182237-119646400-5050-15827-S0/frameworks/20150209-182237-119646400-5050-15827-/executors/aurora.gc/runs/6f1f2749-66a9-41e7-bb68-5e31c08cac6d/gc_executor.pex/.bootstrap/_pex/pex.py",
 line 342, in execute_pkg_resources
runner = entry.load(require=False)  # trust that the environment is sane
  File 
"/var/lib/mesos/slaves/20150209-182237-119646400-5050-15827-S0/frameworks/20150209-182237-119646400-5050-15827-/executors/aurora.gc/runs/6f1f2749-66a9-41e7-bb68-5e31c08cac6d/gc_executor.pex/.bootstrap/pkg_resources.py",
 line 2048, in load
entry = __import__(self.module_name, globals(),globals(), ['__name__'])
  File 
"/var/lib/mesos/slaves/20150209-182237-119646400-5050-15827-S0/frameworks/20150209-182237-119646400-5050-15827-/executors/aurora.gc/runs/6f1f2749-66a9-41e7-bb68-5e31c08cac6d/gc_executor.pex/apache/aurora/executor/bin/gc_executor_main.py",
 line 27, in 
ImportError: No module named executor_detector
vagrant@192:/var/lib/mesos/slaves/20150209-182237-119646400-5050-15827-S0/frameworks/20150209-182237-119646400-5050-15827-/executors/aurora.gc/runs/latest$


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


Repository: aurora


Description
---

This makes the GC executor detect checkpoint roots via the PathDetector 
interface.  This paves the way to detecting checkpoint roots both from 
/var/run/thermos and from /var/lib/mesos/**


Diffs (updated)
-

  src/main/python/apache/aurora/executor/bin/BUILD 
1fa1e9e839bf28093b4e9ded403a761cf4bf5f44 
  src/main/python/apache/aurora/executor/bin/gc_executor_main.py 
b903bcb3630a8a8d50a2008bfae532b2eb947356 
  src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
0752d50015b2ff936f079c4a9f2777172dc00a93 
  src/main/python/apache/aurora/executor/gc_executor.py 
43b415bc6c5177be24ec036cc32ae7cbd87fc70f 
  src/main/python/apache/thermos/bin/thermos.py 
161bbdbc4de95c82e2b596e77b0eac7b920eae66 
  src/main/python/apache/thermos/monitoring/garbage.py 
69bf8e4c2e2e5f85f6b822fbe45f828d61814d7f 
  src/test/python/apache/aurora/executor/bin/BUILD PRE-CREATION 
  src/test/python/apache/aurora/executor/bin/test_gc_executor_entry_point.py 
PRE-CREATION 
  
src/test/python/apache/aurora/executor/bin/test_thermos_executor_entry_point.py 
PRE-CREATION 
  src/test/python/apache/aurora/executor/test_gc_executor.py 
b1bbc89a822302d8ea12324eb767631326639ebb 

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


Testing
---

./pants test.pytest --no-fast src/main/python::


Thanks,

Brian Wickman

Re: Review Request 30749: Port GC executor to PathDetector interface

2015-02-09 Thread Brian Wickman 

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

(Updated Feb. 9, 2015, 8:24 p.m.)


Review request for Aurora, Joshua Cohen and Zameer Manji.


Changes
---

Argh, can't catch a break.  Fixes build typo.


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


Repository: aurora


Description
---

This makes the GC executor detect checkpoint roots via the PathDetector 
interface.  This paves the way to detecting checkpoint roots both from 
/var/run/thermos and from /var/lib/mesos/**


Diffs (updated)
-

  src/main/python/apache/aurora/executor/bin/BUILD 
1fa1e9e839bf28093b4e9ded403a761cf4bf5f44 
  src/main/python/apache/aurora/executor/bin/gc_executor_main.py 
b903bcb3630a8a8d50a2008bfae532b2eb947356 
  src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
0752d50015b2ff936f079c4a9f2777172dc00a93 
  src/main/python/apache/aurora/executor/gc_executor.py 
43b415bc6c5177be24ec036cc32ae7cbd87fc70f 
  src/main/python/apache/thermos/bin/thermos.py 
161bbdbc4de95c82e2b596e77b0eac7b920eae66 
  src/main/python/apache/thermos/monitoring/garbage.py 
69bf8e4c2e2e5f85f6b822fbe45f828d61814d7f 
  src/test/python/apache/aurora/executor/bin/BUILD PRE-CREATION 
  src/test/python/apache/aurora/executor/bin/test_gc_executor_entry_point.py 
PRE-CREATION 
  
src/test/python/apache/aurora/executor/bin/test_thermos_executor_entry_point.py 
PRE-CREATION 
  src/test/python/apache/aurora/executor/test_gc_executor.py 
b1bbc89a822302d8ea12324eb767631326639ebb 

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


Testing
---

./pants test.pytest --no-fast src/main/python::


Thanks,

Brian Wickman

Re: Review Request 30749: Port GC executor to PathDetector interface

2015-02-09 Thread Aurora ReviewBot 

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


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

  Running setup.py install for twitter.common.collections
Skipping installation of 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/__init__.py
 (namespace package)
Skipping installation of 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/common/__init__.py
 (namespace package)
Installing 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter.common.collections-0.3.0-py2.7-nspkg.pth
  Running setup.py install for twitter.common.util
Skipping installation of 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/__init__.py
 (namespace package)
Skipping installation of 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/common/__init__.py
 (namespace package)
Installing 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter.common.util-0.3.0-py2.7-nspkg.pth
  Running setup.py install for twitter.common.log
Skipping installation of 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/__init__.py
 (namespace package)
Skipping installation of 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/common/__init__.py
 (namespace package)
Installing 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter.common.log-0.3.0-py2.7-nspkg.pth
  Running setup.py install for twitter.common.process
Skipping installation of 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/__init__.py
 (namespace package)
Skipping installation of 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/common/__init__.py
 (namespace package)
Installing 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter.common.process-0.3.0-py2.7-nspkg.pth
  Running setup.py install for gitdb
building 'gitdb._perf' extension
x86_64-linux-gnu-gcc -pthread -fno-strict-aliasing -DNDEBUG -g -fwrapv -O2 
-Wall -Wstrict-prototypes -fPIC -Igitdb -I/usr/include/python2.7 -c 
gitdb/_fun.c -o build/temp.linux-x86_64-2.7/gitdb/_fun.o
x86_64-linux-gnu-gcc -pthread -fno-strict-aliasing -DNDEBUG -g -fwrapv -O2 
-Wall -Wstrict-prototypes -fPIC -Igitdb -I/usr/include/python2.7 -c 
gitdb/_delta_apply.c -o build/temp.linux-x86_64-2.7/gitdb/_delta_apply.o
x86_64-linux-gnu-gcc -pthread -shared -Wl,-O1 -Wl,-Bsymbolic-functions 
-Wl,-Bsymbolic-functions -Wl,-z,relro -fno-strict-aliasing -DNDEBUG -g -fwrapv 
-O2 -Wall -Wstrict-prototypes -D_FORTIFY_SOURCE=2 -g -fstack-protector 
--param=ssp-buffer-size=4 -Wformat -Werror=format-security 
build/temp.linux-x86_64-2.7/gitdb/_fun.o 
build/temp.linux-x86_64-2.7/gitdb/_delta_apply.o -o 
build/lib.linux-x86_64-2.7/gitdb/_perf.so
  Running setup.py install for twitter.common.app
Skipping installation of 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/__init__.py
 (namespace package)
Skipping installation of 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/common/__init__.py
 (namespace package)
Installing 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter.common.app-0.3.0-py2.7-nspkg.pth
  Running setup.py install for GitPython

/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/local/lib/python2.7/site-packages/setuptools/dist.py:292:
 UserWarning: The version specified ('0.3.2 RC1') is an invalid version, this 
may not work as expected with newer versions of setuptools, pip, and PyPI. 
Please see PEP 440 for more details.
  "details." % self.metadata.version
  Running setup.py install for pep8
Installing pep8 script to 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/bin
  Running setup.py install for pyflakes
Installing pyflakes script to 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/bin
  Running setup.py in

Re: Review Request 30804: Fixing test coverage in startJobUpdate RPC

2015-02-09 Thread Bill Farner 

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

Ship it!



src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java


Consider collapsing these all to one test case related to input validation.


- Bill Farner


On Feb. 9, 2015, 7:57 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30804/
> ---
> 
> (Updated Feb. 9, 2015, 7:57 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The startJobUpdate RPC test coverage got broken in 
> 9fe6d5408d4aed113a239f22fa5c43aa4f9ae338 as all settings tests 
> short-circuited at `isIsService()` validation.
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  2b5383f3c56927bc42ab47c0a5c7c4b1d41af6dd 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  c6fe218c709c73dbd4c3aeeb94e211ee6cdf9a0d 
> 
> Diff: https://reviews.apache.org/r/30804/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 30804: Fixing test coverage in startJobUpdate RPC

2015-02-09 Thread Maxim Khutornenko 


> On Feb. 9, 2015, 9:54 p.m., Bill Farner wrote:
> > src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java,
> >  line 2690
> > 
> >
> > Consider collapsing these all to one test case related to input 
> > validation.

I'd rather not. This goes against general unit testing guideline of "one issue 
- one test" and isn't saving much in setup.


- Maxim


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


On Feb. 9, 2015, 7:57 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30804/
> ---
> 
> (Updated Feb. 9, 2015, 7:57 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The startJobUpdate RPC test coverage got broken in 
> 9fe6d5408d4aed113a239f22fa5c43aa4f9ae338 as all settings tests 
> short-circuited at `isIsService()` validation.
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  2b5383f3c56927bc42ab47c0a5c7c4b1d41af6dd 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  c6fe218c709c73dbd4c3aeeb94e211ee6cdf9a0d 
> 
> Diff: https://reviews.apache.org/r/30804/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 30647: Instrument the HealthChecker to export stats.

2015-02-09 Thread Brian Wickman 

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


ping.

- Brian Wickman


On Feb. 6, 2015, 11:13 p.m., Brian Wickman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30647/
> ---
> 
> (Updated Feb. 6, 2015, 11:13 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Bill Farner.
> 
> 
> Bugs: AURORA-1062
> https://issues.apache.org/jira/browse/AURORA-1062
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Instrument the HealthChecker to export stats.
> 
> HealthChecker plugin now should export three stats:
>   consecutive_failures: number of consecutive failures experienced (resets on 
> success)
>   latency: how long health checks are taking in practice
>   snoozed: whether or not the health checker is snoozed
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/executor/common/health_checker.py 
> 60676ba0fbd8a218fe4309f07de28e2c66d54530 
>   src/main/python/apache/aurora/executor/common/status_checker.py 
> 624921d68199df098ea51ee8a10815403bf58984 
>   src/test/python/apache/aurora/executor/common/test_health_checker.py 
> def249c2509a28f7145380f250f79202b653dc83 
>   src/test/python/apache/aurora/executor/test_thermos_executor.py 
> c8fab307d17949a8157659c4b3944ec7520feb9d 
> 
> Diff: https://reviews.apache.org/r/30647/diff/
> 
> 
> Testing
> ---
> 
> ./pants test.pytest --no-fast src/test/python/apache/aurora/executor/common::
> 
> 
> Thanks,
> 
> Brian Wickman
> 
>

Re: Review Request 30749: Port GC executor to PathDetector interface

2015-02-09 Thread Brian Wickman 

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

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


Review request for Aurora, Joshua Cohen and Zameer Manji.


Changes
---

Checkstyle


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


Repository: aurora


Description
---

This makes the GC executor detect checkpoint roots via the PathDetector 
interface.  This paves the way to detecting checkpoint roots both from 
/var/run/thermos and from /var/lib/mesos/**


Diffs (updated)
-

  src/main/python/apache/aurora/executor/bin/BUILD 
1fa1e9e839bf28093b4e9ded403a761cf4bf5f44 
  src/main/python/apache/aurora/executor/bin/gc_executor_main.py 
b903bcb3630a8a8d50a2008bfae532b2eb947356 
  src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
0752d50015b2ff936f079c4a9f2777172dc00a93 
  src/main/python/apache/aurora/executor/gc_executor.py 
43b415bc6c5177be24ec036cc32ae7cbd87fc70f 
  src/main/python/apache/thermos/bin/thermos.py 
161bbdbc4de95c82e2b596e77b0eac7b920eae66 
  src/main/python/apache/thermos/monitoring/garbage.py 
69bf8e4c2e2e5f85f6b822fbe45f828d61814d7f 
  src/test/python/apache/aurora/executor/bin/BUILD PRE-CREATION 
  src/test/python/apache/aurora/executor/bin/test_gc_executor_entry_point.py 
PRE-CREATION 
  
src/test/python/apache/aurora/executor/bin/test_thermos_executor_entry_point.py 
PRE-CREATION 
  src/test/python/apache/aurora/executor/test_gc_executor.py 
b1bbc89a822302d8ea12324eb767631326639ebb 

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


Testing
---

./pants test.pytest --no-fast src/main/python::


Thanks,

Brian Wickman

Re: Review Request 30749: Port GC executor to PathDetector interface

2015-02-09 Thread Aurora ReviewBot 

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

Ship it!


Master (68aa285) is green with this patch.
  ./build-support/jenkins/build.sh

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

- Aurora ReviewBot


On Feb. 9, 2015, 11:16 p.m., Brian Wickman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30749/
> ---
> 
> (Updated Feb. 9, 2015, 11:16 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Zameer Manji.
> 
> 
> Bugs: AURORA-1025
> https://issues.apache.org/jira/browse/AURORA-1025
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This makes the GC executor detect checkpoint roots via the PathDetector 
> interface.  This paves the way to detecting checkpoint roots both from 
> /var/run/thermos and from /var/lib/mesos/**
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/executor/bin/BUILD 
> 1fa1e9e839bf28093b4e9ded403a761cf4bf5f44 
>   src/main/python/apache/aurora/executor/bin/gc_executor_main.py 
> b903bcb3630a8a8d50a2008bfae532b2eb947356 
>   src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
> 0752d50015b2ff936f079c4a9f2777172dc00a93 
>   src/main/python/apache/aurora/executor/gc_executor.py 
> 43b415bc6c5177be24ec036cc32ae7cbd87fc70f 
>   src/main/python/apache/thermos/bin/thermos.py 
> 161bbdbc4de95c82e2b596e77b0eac7b920eae66 
>   src/main/python/apache/thermos/monitoring/garbage.py 
> 69bf8e4c2e2e5f85f6b822fbe45f828d61814d7f 
>   src/test/python/apache/aurora/executor/bin/BUILD PRE-CREATION 
>   src/test/python/apache/aurora/executor/bin/test_gc_executor_entry_point.py 
> PRE-CREATION 
>   
> src/test/python/apache/aurora/executor/bin/test_thermos_executor_entry_point.py
>  PRE-CREATION 
>   src/test/python/apache/aurora/executor/test_gc_executor.py 
> b1bbc89a822302d8ea12324eb767631326639ebb 
> 
> Diff: https://reviews.apache.org/r/30749/diff/
> 
> 
> Testing
> ---
> 
> ./pants test.pytest --no-fast src/main/python::
> 
> 
> Thanks,
> 
> Brian Wickman
> 
>

Re: Review Request 30325: Implementing pulseJobUpdate RPC.

2015-02-09 Thread Maxim Khutornenko 

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

(Updated Feb. 10, 2015, 12:53 a.m.)


Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner.


Changes
---

CR comments.


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


Repository: aurora


Description
---

Implemented the `pulseJobUpdate` RPC. Also, moved it into AuroraAdmin interface 
to support AOP capability validation. 

The RB is diffed against https://reviews.apache.org/r/30225/


Diffs (updated)
-

  api/src/main/thrift/org/apache/aurora/gen/api.thrift 
4f603f9e7ed004e53937a45ea8edf7241e15f5cf 
  src/main/java/org/apache/aurora/auth/CapabilityValidator.java 
45ef643ebe57c1517cdae373574331ea302a8b74 
  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
fd4d6908fe7680808cfdee9e78c37506af280068 
  
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
 a9966a823e9616d0bf9bd19fd62e632d931ddf0a 
  src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 
4bf63921f8aa4be943b2b9a7b0be9fb33f7762db 

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


Testing
---

./gradlew -Pq build


Thanks,

Maxim Khutornenko

Re: Review Request 30325: Implementing pulseJobUpdate RPC.

2015-02-09 Thread Maxim Khutornenko 


> On Feb. 3, 2015, 12:16 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java,
> >  line 1388
> > 
> >
> > You can check whether the primitive field is set, which will 
> > distinguish between default int value and an explicitly set value.
> > 
> > I think it makes sense to reject zero at this layer.

Done.


> On Feb. 3, 2015, 12:16 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java,
> >  line 1535
> > 
> >
> > This is too restrictive.  It means that _only_ the update coordinator 
> > role may call this RPC, and that a user cannot build something to pulse 
> > their own updates.
> > 
> > You should instead do what `isAdmin` does and fall back to the 
> > coordinator role if direct auth fails.
> > 
> > This is an unfortunate state of affairs, and hopefully the move to 
> > shiro dramatically improves all this.
> 
> Maxim Khutornenko wrote:
> I don't see how it's necessarily better. Pulsing can always be done under 
> UPDATE_COORDINATOR membership, which is specifically covering heartbeats 
> only. The isAdmin() requires ROOT that opens up everything else, including 
> the ability to kill anyone's tasks in the claster. Isn't that more 
> restrictive in real life? We don't expect regular users having ROOT access, 
> meaning they will unlikely to get to write their own pulse routine due to 
> that. 
> 
> Or maybe you suggesting an approach similar to killTasks(), where an 
> admin check followed by a role validation? The problem here is that we don't 
> have a job key to extract the role for authorization. Perhaps we can change 
> the RPC to accept a job key instead but that would open up for a race we 
> don't want to see (e.g. late pulse arrives for a wrong update ID). We could 
> probably get away with both updateId and jobKey in the API to avoid 
> ambiguity, or perhpas just updateId and role? I am open to suggestions.
> 
> Kevin Sweeney wrote:
> In Shiro this will look something like
> 
> subject.checkPermission("update:resume:mesos:prod:labrat");
> 
> With role update_coordinator having:
> ```
> update:*
> ```
> 
> role root having:
> ```
> *
> ```
> 
> and user mesos having:
> ```
> *:*:mesos
> ```
> 
> So they'd all be allowed.
> 
> Bill Farner wrote:
> > Or maybe you suggesting an approach similar to killTasks(), where an 
> admin check followed by a role validation?
> 
> Yeah, this.  I'm not asking for update heartbeats to require ROOT.
> 
> > The problem here is that we don't have a job key to extract the role 
> for authorization.
> 
> We have an association between {{updateId}} and role owning the job, 
> right?
> 
> /**
>  * Fetches a read-only view of a job update.
>  *
>  * @param updateId Update ID to fetch.
>  * @return A read-only view of job update.
>  */
> Optional fetchJobUpdate(String updateId);
> 
> Maxim Khutornenko wrote:
> We do. However, it would require accessing the store in order to 
> authenticate the call. This is a new pattern we have not tried before and it 
> may potentially interfere with moving to pure AOP auth.
> 
> Kevin Sweeney wrote:
> Operations scoped to an object owned by a role will not be enforcable 
> with pure AOP, as many will typically require a storage lookup (usually by 
> ID) to determine the owner of the object being operated on.
> 
> Operations that happen on "global" like `"snapshot:create"` will be 
> enforceable with pure AOP.

Refactored to use UPDATE_COORDINATOR role with a user auth as fallback. The 
authorization is matching the provided credentials (SessionKey) against the 
operational intent (JobUpdateKey). The validation of JobUpdateKey matching the 
actual stored data is deferred to application layer where a missing content 
will generate an INVALID_REQUEST response (or JobUpdatePulseStatus.FINISHED as 
in the current case).


- Maxim


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


On Jan. 30, 2015, 5:23 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30325/
> ---
> 
> (Updated Jan. 30, 2015, 5:23 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner.
> 
> 
> Bugs: AURORA-1009
> https://issues.apache.org/jira/browse/AURORA-1009
> 
> 
> Repository: a

Re: Review Request 30325: Implementing pulseJobUpdate RPC.

2015-02-09 Thread Maxim Khutornenko 

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

(Updated Feb. 10, 2015, 12:53 a.m.)


Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner.


Changes
---

Updating description.


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


Repository: aurora


Description (updated)
---

Implemented the `pulseJobUpdate` RPC.

The RB is diffed against https://reviews.apache.org/r/30225/


Diffs
-

  api/src/main/thrift/org/apache/aurora/gen/api.thrift 
4f603f9e7ed004e53937a45ea8edf7241e15f5cf 
  src/main/java/org/apache/aurora/auth/CapabilityValidator.java 
45ef643ebe57c1517cdae373574331ea302a8b74 
  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
fd4d6908fe7680808cfdee9e78c37506af280068 
  
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
 a9966a823e9616d0bf9bd19fd62e632d931ddf0a 
  src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 
4bf63921f8aa4be943b2b9a7b0be9fb33f7762db 

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


Testing
---

./gradlew -Pq build


Thanks,

Maxim Khutornenko

Re: Review Request 30710: add mesos role feature

2015-02-09 Thread Zameer Manji 

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



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


I don't think we can safely do this right now. I know the default value is 
'*' 
(https://github.com/apache/mesos/blob/master/include/mesos/mesos.proto#L128), 
but we have hit bugs in the past where Mesos will reject protobufs as changed 
when we start setting a previously unset field to a default value.

See https://issues.apache.org/jira/browse/MESOS-2309 for details.

Currently as this stands, I think this will cause existing Aurora instances 
to be rejected by the Master.


- Zameer Manji


On Feb. 9, 2015, 4:49 p.m., lozh...@ebay.com zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30710/
> ---
> 
> (Updated Feb. 9, 2015, 4:49 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Bill Farner.
> 
> 
> Bugs: AURORA-1109
> https://issues.apache.org/jira/browse/AURORA-1109
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> ## Problems
> 
> We are from eBay platform team. Previously, we used marathon to generate 
> Jenkins master instance in dedicated vms and recieve resource offer from same 
> dedicated vms. For the details, please refer to
> http://www.ebaytechblog.com/2014/04/04/delivering-ebays-ci-solution-with-apache-mesos-part-i/#.VNQUuC6_SPU
> 
> Now, we found Aurora is more stable and powerful. We are moving from Marathon 
> to Aurora. During the move, we found there is no mesos role in Aurora now. 
> But we need use mesos role way to solve the problem in section "Frameworks 
> stopped receiving offers after a while" of the given url.
> 
> Here is a snippet of the problem description:
> 
> *We noticed occurred after we used Marathon to create the initial set of CI 
> masters. As those CI masters started registering themselves as frameworks, 
> Marathon stopped receiving any offers from Mesos; essentially, no new CI 
> masters could be launched. Let’s start with Marathon. In the DRF model, it 
> was unfair to treat Marathon in the same bucket/role alongside hundreds of 
> connected Jenkins frameworks. After launching all these Jenkins frameworks, 
> Marathon had a large resource share and Mesos would aggressively offer 
> resources to frameworks that were using little or no resources. Marathon was 
> placed last in priority and got starved out.*
> 
> *We decided to define a dedicated Mesos role for Marathon and to have all of 
> the Mesos slaves that were reserved for Jenkins master instances support that 
> Mesos role. Jenkins frameworks were left with the default role “*”.*This 
> solved the problem – Mesos offered resources per role and hence Marathon 
> never got starved out. A framework with a special role will get resource 
> offers from both slaves supporting that special role and also from the 
> default role “*”.* However, since we were using placement constraints, 
> Marathon accepted resource offers only from slaves that supported both the 
> role and the placement constraints.*
> ## Solution
> 
> So we add role feature is the source code to solve the problem in same way: 
> When accept a resource offer, Aurora will send back the needed resources to 
> Mesos with the mesos role in resource offer.
> 
> How to configure the Mesos role:
> 1.Add cmd option --mesos_role=${Mesos role name} when start Aurora scheduler.
> 
> We change the test cases according code change. Each changed test case is 
> green
> Merge https://github.com/zhanglong2015/incubator-aurora
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 
> 1a158b4e0be94762ad0480e8ce74b19bacc90c97 
>   src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 
> 31aa2bbaab3d97875493ad75c4d2c7c82ac7fa58 
>   src/main/java/org/apache/aurora/scheduler/configuration/Resources.java 
> b5a3140e3560f790d1db496dca3c2ee0dc96a195 
>   
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java
>  d0994203b5650f44ca2eb32e1e2aa61875163854 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> 5340d651b298ec8aa079e73d6d2f652fdf876293 
>   src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 
> e1c29747c9854cf75bf63f6f085cf40ca68989af 
>   src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 
> 422d5a9a42310979752eb7282658316c2b772419 
>   src/test/java/org/apache/aurora/scheduler/configuration/ResourcesTest.java 
> d6febb8998e05257cabe8d193cefa0b6c79f197e 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.jav

Re: Review Request 30225: Modifying update controller to support heartbeats.

2015-02-09 Thread David McLaughlin 

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

Ship it!


Pulse logic LGTM.

- David McLaughlin


On Feb. 7, 2015, 2:43 a.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30225/
> ---
> 
> (Updated Feb. 7, 2015, 2:43 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner.
> 
> 
> Bugs: AURORA-1010
> https://issues.apache.org/jira/browse/AURORA-1010
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Added pulsing support into the JobUpdateController. The qualified coordinated 
> updates get blocked until a pulse arrives. An update then becomes active and 
> proceeds until `blockIfNoPulsesAfterMs` expires or the update reaches a 
> terminal state (whichever comes first).
> 
> UPDATE:
> - Added explicit states to capture "blocked" updates
> - Refactored pulseUpdate() to not rely on DB state
> - Dropped JobUpdatePulseState.PAUSED as it does not seem necessary.
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 4f603f9e7ed004e53937a45ea8edf7241e15f5cf 
>   src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 
> 7f2ec71d744d5fafac84291cc79feba3398d0e1e 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> b7d8d524e15f101416889c00efc3ecf2c8d9c1a1 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
> d479d20259f284528b2291e2e486b36d8e47fe5e 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
>  60f535988a20638fb16515d5027bfa65f1afb73c 
>   src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java 
> d3b30d48b76d8d7c64cda006a34f7ed3296526f2 
>   
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java
>  ec9d1e07abca1bf30262e1c0f741a34741e96f5d 
>   
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachine.java 
> 76460f95e058181b711fb6869f5a34c1d5bdfe31 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  f9c9ceddc559b43b4a5c45c745d54ff47484edde 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  ca7c0c2675477cc727ca006697665f997972dfde 
>   
> src/test/java/org/apache/aurora/scheduler/testing/FakeScheduledExecutor.java 
> 92cfa2b30c1c4daa3ae225fc8609fbeaecdaff7a 
>   
> src/test/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachineTest.java
>  89765ac3d34a827d3748fb96a275d78e9d1b8b72 
>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
> 41d422939209d0808235128e4242c11e8ef25969 
> 
> Diff: https://reviews.apache.org/r/30225/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 30325: Implementing pulseJobUpdate RPC.

2015-02-09 Thread Aurora ReviewBot 

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


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

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

- Aurora ReviewBot


On Feb. 10, 2015, 12:53 a.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30325/
> ---
> 
> (Updated Feb. 10, 2015, 12:53 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner.
> 
> 
> Bugs: AURORA-1009
> https://issues.apache.org/jira/browse/AURORA-1009
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Implemented the `pulseJobUpdate` RPC.
> 
> The RB is diffed against https://reviews.apache.org/r/30225/
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 4f603f9e7ed004e53937a45ea8edf7241e15f5cf 
>   src/main/java/org/apache/aurora/auth/CapabilityValidator.java 
> 45ef643ebe57c1517cdae373574331ea302a8b74 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  fd4d6908fe7680808cfdee9e78c37506af280068 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  a9966a823e9616d0bf9bd19fd62e632d931ddf0a 
>   src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 
> 4bf63921f8aa4be943b2b9a7b0be9fb33f7762db 
> 
> Diff: https://reviews.apache.org/r/30325/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 30749: Port GC executor to PathDetector interface

2015-02-09 Thread Zameer Manji 

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

Ship it!


Ship It!

- Zameer Manji


On Feb. 9, 2015, 3:16 p.m., Brian Wickman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30749/
> ---
> 
> (Updated Feb. 9, 2015, 3:16 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Zameer Manji.
> 
> 
> Bugs: AURORA-1025
> https://issues.apache.org/jira/browse/AURORA-1025
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This makes the GC executor detect checkpoint roots via the PathDetector 
> interface.  This paves the way to detecting checkpoint roots both from 
> /var/run/thermos and from /var/lib/mesos/**
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/executor/bin/BUILD 
> 1fa1e9e839bf28093b4e9ded403a761cf4bf5f44 
>   src/main/python/apache/aurora/executor/bin/gc_executor_main.py 
> b903bcb3630a8a8d50a2008bfae532b2eb947356 
>   src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
> 0752d50015b2ff936f079c4a9f2777172dc00a93 
>   src/main/python/apache/aurora/executor/gc_executor.py 
> 43b415bc6c5177be24ec036cc32ae7cbd87fc70f 
>   src/main/python/apache/thermos/bin/thermos.py 
> 161bbdbc4de95c82e2b596e77b0eac7b920eae66 
>   src/main/python/apache/thermos/monitoring/garbage.py 
> 69bf8e4c2e2e5f85f6b822fbe45f828d61814d7f 
>   src/test/python/apache/aurora/executor/bin/BUILD PRE-CREATION 
>   src/test/python/apache/aurora/executor/bin/test_gc_executor_entry_point.py 
> PRE-CREATION 
>   
> src/test/python/apache/aurora/executor/bin/test_thermos_executor_entry_point.py
>  PRE-CREATION 
>   src/test/python/apache/aurora/executor/test_gc_executor.py 
> b1bbc89a822302d8ea12324eb767631326639ebb 
> 
> Diff: https://reviews.apache.org/r/30749/diff/
> 
> 
> Testing
> ---
> 
> ./pants test.pytest --no-fast src/main/python::
> 
> 
> Thanks,
> 
> Brian Wickman
> 
>

Re: Review Request 30710: add mesos role feature

2015-02-09 Thread lozh...@ebay.com zhang 


> On Feb. 10, 2015, 12:56 a.m., Zameer Manji wrote:
> > src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java,
> >  line 117
> > 
> >
> > I don't think we can safely do this right now. I know the default value 
> > is '*' 
> > (https://github.com/apache/mesos/blob/master/include/mesos/mesos.proto#L128),
> >  but we have hit bugs in the past where Mesos will reject protobufs as 
> > changed when we start setting a previously unset field to a default value.
> > 
> > See https://issues.apache.org/jira/browse/MESOS-2309 for details.
> > 
> > Currently as this stands, I think this will cause existing Aurora 
> > instances to be rejected by the Master.

How about if we left the role unset in register framework message if no role 
specified in command line?


- lozh...@ebay.com


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


On Feb. 10, 2015, 12:49 a.m., lozh...@ebay.com zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30710/
> ---
> 
> (Updated Feb. 10, 2015, 12:49 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Bill Farner.
> 
> 
> Bugs: AURORA-1109
> https://issues.apache.org/jira/browse/AURORA-1109
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> ## Problems
> 
> We are from eBay platform team. Previously, we used marathon to generate 
> Jenkins master instance in dedicated vms and recieve resource offer from same 
> dedicated vms. For the details, please refer to
> http://www.ebaytechblog.com/2014/04/04/delivering-ebays-ci-solution-with-apache-mesos-part-i/#.VNQUuC6_SPU
> 
> Now, we found Aurora is more stable and powerful. We are moving from Marathon 
> to Aurora. During the move, we found there is no mesos role in Aurora now. 
> But we need use mesos role way to solve the problem in section "Frameworks 
> stopped receiving offers after a while" of the given url.
> 
> Here is a snippet of the problem description:
> 
> *We noticed occurred after we used Marathon to create the initial set of CI 
> masters. As those CI masters started registering themselves as frameworks, 
> Marathon stopped receiving any offers from Mesos; essentially, no new CI 
> masters could be launched. Let’s start with Marathon. In the DRF model, it 
> was unfair to treat Marathon in the same bucket/role alongside hundreds of 
> connected Jenkins frameworks. After launching all these Jenkins frameworks, 
> Marathon had a large resource share and Mesos would aggressively offer 
> resources to frameworks that were using little or no resources. Marathon was 
> placed last in priority and got starved out.*
> 
> *We decided to define a dedicated Mesos role for Marathon and to have all of 
> the Mesos slaves that were reserved for Jenkins master instances support that 
> Mesos role. Jenkins frameworks were left with the default role “*”.*This 
> solved the problem – Mesos offered resources per role and hence Marathon 
> never got starved out. A framework with a special role will get resource 
> offers from both slaves supporting that special role and also from the 
> default role “*”.* However, since we were using placement constraints, 
> Marathon accepted resource offers only from slaves that supported both the 
> role and the placement constraints.*
> ## Solution
> 
> So we add role feature is the source code to solve the problem in same way: 
> When accept a resource offer, Aurora will send back the needed resources to 
> Mesos with the mesos role in resource offer.
> 
> How to configure the Mesos role:
> 1.Add cmd option --mesos_role=${Mesos role name} when start Aurora scheduler.
> 
> We change the test cases according code change. Each changed test case is 
> green
> Merge https://github.com/zhanglong2015/incubator-aurora
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 
> 1a158b4e0be94762ad0480e8ce74b19bacc90c97 
>   src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 
> 31aa2bbaab3d97875493ad75c4d2c7c82ac7fa58 
>   src/main/java/org/apache/aurora/scheduler/configuration/Resources.java 
> b5a3140e3560f790d1db496dca3c2ee0dc96a195 
>   
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java
>  d0994203b5650f44ca2eb32e1e2aa61875163854 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> 5340d651b298ec8aa079e73d6d2f652fdf876293 
>   src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 
> e1c29747c9854cf75bf63f6f085cf40ca68989af 
>   src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.ja

Re: Review Request 30710: add mesos role feature

2015-02-09 Thread Zameer Manji 


> On Feb. 9, 2015, 4:56 p.m., Zameer Manji wrote:
> > src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java,
> >  line 117
> > 
> >
> > I don't think we can safely do this right now. I know the default value 
> > is '*' 
> > (https://github.com/apache/mesos/blob/master/include/mesos/mesos.proto#L128),
> >  but we have hit bugs in the past where Mesos will reject protobufs as 
> > changed when we start setting a previously unset field to a default value.
> > 
> > See https://issues.apache.org/jira/browse/MESOS-2309 for details.
> > 
> > Currently as this stands, I think this will cause existing Aurora 
> > instances to be rejected by the Master.
> 
> lozh...@ebay.com zhang wrote:
> How about if we left the role unset in register framework message if no 
> role specified in command line?

I think that's a good step to take. When you make this change can you reference 
MESOS-2309 as the reason why in a comment?


- Zameer


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


On Feb. 9, 2015, 4:49 p.m., lozh...@ebay.com zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30710/
> ---
> 
> (Updated Feb. 9, 2015, 4:49 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Bill Farner.
> 
> 
> Bugs: AURORA-1109
> https://issues.apache.org/jira/browse/AURORA-1109
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> ## Problems
> 
> We are from eBay platform team. Previously, we used marathon to generate 
> Jenkins master instance in dedicated vms and recieve resource offer from same 
> dedicated vms. For the details, please refer to
> http://www.ebaytechblog.com/2014/04/04/delivering-ebays-ci-solution-with-apache-mesos-part-i/#.VNQUuC6_SPU
> 
> Now, we found Aurora is more stable and powerful. We are moving from Marathon 
> to Aurora. During the move, we found there is no mesos role in Aurora now. 
> But we need use mesos role way to solve the problem in section "Frameworks 
> stopped receiving offers after a while" of the given url.
> 
> Here is a snippet of the problem description:
> 
> *We noticed occurred after we used Marathon to create the initial set of CI 
> masters. As those CI masters started registering themselves as frameworks, 
> Marathon stopped receiving any offers from Mesos; essentially, no new CI 
> masters could be launched. Let’s start with Marathon. In the DRF model, it 
> was unfair to treat Marathon in the same bucket/role alongside hundreds of 
> connected Jenkins frameworks. After launching all these Jenkins frameworks, 
> Marathon had a large resource share and Mesos would aggressively offer 
> resources to frameworks that were using little or no resources. Marathon was 
> placed last in priority and got starved out.*
> 
> *We decided to define a dedicated Mesos role for Marathon and to have all of 
> the Mesos slaves that were reserved for Jenkins master instances support that 
> Mesos role. Jenkins frameworks were left with the default role “*”.*This 
> solved the problem – Mesos offered resources per role and hence Marathon 
> never got starved out. A framework with a special role will get resource 
> offers from both slaves supporting that special role and also from the 
> default role “*”.* However, since we were using placement constraints, 
> Marathon accepted resource offers only from slaves that supported both the 
> role and the placement constraints.*
> ## Solution
> 
> So we add role feature is the source code to solve the problem in same way: 
> When accept a resource offer, Aurora will send back the needed resources to 
> Mesos with the mesos role in resource offer.
> 
> How to configure the Mesos role:
> 1.Add cmd option --mesos_role=${Mesos role name} when start Aurora scheduler.
> 
> We change the test cases according code change. Each changed test case is 
> green
> Merge https://github.com/zhanglong2015/incubator-aurora
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 
> 1a158b4e0be94762ad0480e8ce74b19bacc90c97 
>   src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 
> 31aa2bbaab3d97875493ad75c4d2c7c82ac7fa58 
>   src/main/java/org/apache/aurora/scheduler/configuration/Resources.java 
> b5a3140e3560f790d1db496dca3c2ee0dc96a195 
>   
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java
>  d0994203b5650f44ca2eb32e1e2aa61875163854 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> 5340d651b298ec8aa079e73d6d2f652fdf876293 
>   src/main/java/org/apache/aurora/sched

Re: Review Request 30710: add mesos role feature

2015-02-09 Thread Maxim Khutornenko 

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



src/main/java/org/apache/aurora/scheduler/configuration/Resources.java


I am afraid the current approach is not going to work in a cluster with 
mesos slaves configured to offer multi-role resources. Consider an example 
where a slave is configured to offer both role-specific and general pool CPU:
```
--resources=cpus(aurora):4;cpus(*):2
```

A correspondent offer resources section could be:
```
...
  "resources" :
  [
{
  "name": "cpus",
  "type": SCALAR,
  "scalar"  : { "value" : 4.0 },
  "role": "aurora",
},
{
  "name": "cpus",
  "type": SCALAR,
  "scalar"  : { "value" : 2.0 },
  "role": "*",
}
  ]
...
```
Given the above, a matching task with 6.0 CPU would require a TaskInfo with 
the same resources section (4.0-aurora, 2.0-*) or mesos would reject the task 
launch.

The current state of Aurora where a framework role is not configurable 
gives us the "luxury" of not supporting multi-role allocations. However, if we 
are going to support role-based allocations I think we should do it the proper 
way and fully support single and multi-role assignments. This is going to be 
especially critical when the mesos dynamic reservation [1] lands. 

[1] - 
https://docs.google.com/document/d/1e3j69pfBgtc8xM00DhcuiMl6ImkEB5na0TzOMyzrg8A/edit#


- Maxim Khutornenko


On Feb. 10, 2015, 12:49 a.m., lozh...@ebay.com zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30710/
> ---
> 
> (Updated Feb. 10, 2015, 12:49 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Bill Farner.
> 
> 
> Bugs: AURORA-1109
> https://issues.apache.org/jira/browse/AURORA-1109
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> ## Problems
> 
> We are from eBay platform team. Previously, we used marathon to generate 
> Jenkins master instance in dedicated vms and recieve resource offer from same 
> dedicated vms. For the details, please refer to
> http://www.ebaytechblog.com/2014/04/04/delivering-ebays-ci-solution-with-apache-mesos-part-i/#.VNQUuC6_SPU
> 
> Now, we found Aurora is more stable and powerful. We are moving from Marathon 
> to Aurora. During the move, we found there is no mesos role in Aurora now. 
> But we need use mesos role way to solve the problem in section "Frameworks 
> stopped receiving offers after a while" of the given url.
> 
> Here is a snippet of the problem description:
> 
> *We noticed occurred after we used Marathon to create the initial set of CI 
> masters. As those CI masters started registering themselves as frameworks, 
> Marathon stopped receiving any offers from Mesos; essentially, no new CI 
> masters could be launched. Let’s start with Marathon. In the DRF model, it 
> was unfair to treat Marathon in the same bucket/role alongside hundreds of 
> connected Jenkins frameworks. After launching all these Jenkins frameworks, 
> Marathon had a large resource share and Mesos would aggressively offer 
> resources to frameworks that were using little or no resources. Marathon was 
> placed last in priority and got starved out.*
> 
> *We decided to define a dedicated Mesos role for Marathon and to have all of 
> the Mesos slaves that were reserved for Jenkins master instances support that 
> Mesos role. Jenkins frameworks were left with the default role “*”.*This 
> solved the problem – Mesos offered resources per role and hence Marathon 
> never got starved out. A framework with a special role will get resource 
> offers from both slaves supporting that special role and also from the 
> default role “*”.* However, since we were using placement constraints, 
> Marathon accepted resource offers only from slaves that supported both the 
> role and the placement constraints.*
> ## Solution
> 
> So we add role feature is the source code to solve the problem in same way: 
> When accept a resource offer, Aurora will send back the needed resources to 
> Mesos with the mesos role in resource offer.
> 
> How to configure the Mesos role:
> 1.Add cmd option --mesos_role=${Mesos role name} when start Aurora scheduler.
> 
> We change the test cases according code change. Each changed test case is 
> green
> Merge https://github.com/zhanglong2015/incubator-aurora
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 
> 1a158b4e0be94762ad0480e8ce74b19bacc90c97 
>   src/main/java/org/apache/aurora/scheduler

Review Request 30818: Support separate routes for job controller tabs.

2015-02-09 Thread Joshua Cohen 

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

Review request for Aurora and David McLaughlin.


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


Repository: aurora


Description
---

Previously navigating between the active/completed/all tabs on the
job controller page did not alter the browser history. Now it does,
meaning that you can navigate with the back button as well as link
directly to a tab.


Diffs
-

  src/main/resources/scheduler/assets/job.html 
4a00a8863d676f168fbfce9f45ec4bad0244199f 
  src/main/resources/scheduler/assets/js/app.js 
b66409f628046c67b62c92a75cbeed20c09b4ec1 
  src/main/resources/scheduler/assets/js/controllers.js 
092e7d5df2121f45f99f5a788187d52bebb7e5dd 

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


Testing
---

./gradlew jshint

Verified push state worked in vagrant.


Thanks,

Joshua Cohen

Re: Review Request 30818: Support separate routes for job controller tabs.

2015-02-09 Thread Aurora ReviewBot 

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


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

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

:processResources
:classes
:jar
:assemble
:compileJmhJavawarning: Supported source version 'RELEASE_6' from annotation 
processor 'org.openjdk.jmh.generators.BenchmarkProcessor' less than -source 
'1.7'
1 warning

:processJmhResources UP-TO-DATE
:jmhClasses
:checkstyleJmh
:jsHint
Line is too long. 
(/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/resources/scheduler/assets/js/controllers.js:340:102)
> var selectedTab = (length === JOB_CONTROLLER_PATH_MAX_PARTS ? 
> pathParts[length - 1] : 'active');

Missing space after 'function'. 
(/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/resources/scheduler/assets/js/controllers.js:345:37)
> $scope.pushTabState = function(tab) {

Missing semicolon. 
(/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/resources/scheduler/assets/js/controllers.js:355:8)
> }

:jsHint FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':jsHint'.
> Process 'command '/home/jenkins/tools/java/jdk1.7.0_25-32/bin/java'' finished 
> with non-zero exit value 2

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

BUILD FAILED

Total time: 1 mins 33.219 secs


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

- Aurora ReviewBot


On Feb. 10, 2015, 5:24 a.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30818/
> ---
> 
> (Updated Feb. 10, 2015, 5:24 a.m.)
> 
> 
> Review request for Aurora and David McLaughlin.
> 
> 
> Bugs: AURORA-696
> https://issues.apache.org/jira/browse/AURORA-696
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Previously navigating between the active/completed/all tabs on the
> job controller page did not alter the browser history. Now it does,
> meaning that you can navigate with the back button as well as link
> directly to a tab.
> 
> 
> Diffs
> -
> 
>   src/main/resources/scheduler/assets/job.html 
> 4a00a8863d676f168fbfce9f45ec4bad0244199f 
>   src/main/resources/scheduler/assets/js/app.js 
> b66409f628046c67b62c92a75cbeed20c09b4ec1 
>   src/main/resources/scheduler/assets/js/controllers.js 
> 092e7d5df2121f45f99f5a788187d52bebb7e5dd 
> 
> Diff: https://reviews.apache.org/r/30818/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew jshint
> 
> Verified push state worked in vagrant.
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>

Re: Review Request 30818: Support separate routes for job controller tabs.

2015-02-09 Thread Joshua Cohen 

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

(Updated Feb. 10, 2015, 6:01 a.m.)


Review request for Aurora and David McLaughlin.


Changes
---

Checkstyle. Apparently it's jsHint, not jshint. Gradle didn't complain about 
the latter being an invalid task and reported everything as ok.


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


Repository: aurora


Description
---

Previously navigating between the active/completed/all tabs on the
job controller page did not alter the browser history. Now it does,
meaning that you can navigate with the back button as well as link
directly to a tab.


Diffs (updated)
-

  src/main/resources/scheduler/assets/job.html 
4a00a8863d676f168fbfce9f45ec4bad0244199f 
  src/main/resources/scheduler/assets/js/app.js 
b66409f628046c67b62c92a75cbeed20c09b4ec1 
  src/main/resources/scheduler/assets/js/controllers.js 
092e7d5df2121f45f99f5a788187d52bebb7e5dd 

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


Testing
---

./gradlew jshint

Verified push state worked in vagrant.


Thanks,

Joshua Cohen

Re: Review Request 30818: Support separate routes for job controller tabs.

2015-02-09 Thread Joshua Cohen 

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

(Updated Feb. 10, 2015, 6:01 a.m.)


Review request for Aurora and David McLaughlin.


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


Repository: aurora


Description
---

Previously navigating between the active/completed/all tabs on the
job controller page did not alter the browser history. Now it does,
meaning that you can navigate with the back button as well as link
directly to a tab.


Diffs
-

  src/main/resources/scheduler/assets/job.html 
4a00a8863d676f168fbfce9f45ec4bad0244199f 
  src/main/resources/scheduler/assets/js/app.js 
b66409f628046c67b62c92a75cbeed20c09b4ec1 
  src/main/resources/scheduler/assets/js/controllers.js 
092e7d5df2121f45f99f5a788187d52bebb7e5dd 

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


Testing (updated)
---

./gradlew jsHint

Verified push state worked in vagrant.


Thanks,

Joshua Cohen

Re: Review Request 30818: Support separate routes for job controller tabs.

2015-02-09 Thread Aurora ReviewBot 

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


Master (68aa285) 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 Feb. 10, 2015, 6:01 a.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30818/
> ---
> 
> (Updated Feb. 10, 2015, 6:01 a.m.)
> 
> 
> Review request for Aurora and David McLaughlin.
> 
> 
> Bugs: AURORA-696
> https://issues.apache.org/jira/browse/AURORA-696
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Previously navigating between the active/completed/all tabs on the
> job controller page did not alter the browser history. Now it does,
> meaning that you can navigate with the back button as well as link
> directly to a tab.
> 
> 
> Diffs
> -
> 
>   src/main/resources/scheduler/assets/job.html 
> 4a00a8863d676f168fbfce9f45ec4bad0244199f 
>   src/main/resources/scheduler/assets/js/app.js 
> b66409f628046c67b62c92a75cbeed20c09b4ec1 
>   src/main/resources/scheduler/assets/js/controllers.js 
> 092e7d5df2121f45f99f5a788187d52bebb7e5dd 
> 
> Diff: https://reviews.apache.org/r/30818/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew jsHint
> 
> Verified push state worked in vagrant.
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>