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

[Pierre Cheynier]

> On March 22, 2016, 1:02 a.m., Joshua Cohen wrote:
> > Sorry for the delay on this. After you filed the pull request, I 
> > investigated a bit what will be required once Mesos 0.30.0 lands: 
> > https://issues.apache.org/jira/browse/AURORA-1632. I think the problem goes 
> > beyond the failure to find `sys.executable` when $PATH is not set. As even 
> > after switching back to chmod+x on the runner, the task failed further down 
> > the stack.
> > 
> > I suspect the fix for Mesos 0.30.0 will be to set our own $PATH which 
> > should allow `sys.executable` to continue working and will allow any tasks 
> > users have running which have come to rely on Thermos setting it for them 
> > to behave as expected. The problem is, I haven't had time to figure out 
> > what we should set $PATH to yet ;) (anyone have any thoughts?).
> > 
> > I know this is probably more info than you bargained for when you opened 
> > what seemed like a simple pull request. I'm not opposed to accepting this 
> > patch (with a TODO to restore `sys.executable` when we figure out what to 
> > do about setting $PATH) if it unblocks your use case, but can you confirm 
> > that you're actually able to run the Mesos agent with 
> > `--executor_environment_variables='{}'` and still launch tasks?

Hi,

Sorry for my delay on opening this review on apache.org.
Actually, I started to use Aurora and faced this issue cause my setup use 
`--executor_environment_variables`.
I first investigated in the Python default setup on CentOS, the pants/pex build 
system and then the differences between the vagrant box provided in the repo 
and mine before discovering that this is the origin of the issue.

I admit this is a simple fix, I just tried to understand the reason of using 
this mechanism and found that it was changed 2 years ago.
In 0.27.2  it works well by using this patch (I'm now able to run something).


- Pierre


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


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

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

[Pierre Cheynier]

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

(Updated March 22, 2016, 9:50 a.m.)


Review request for Aurora, Joshua Cohen and Bill Farner.


Repository: aurora


Description
---

When using `--executor_environment_variables` without explicitely
passing LD_LIBRARY_PATH, `sys.executable` returns an empty string
resulting in a '[Errno 13] Permission denied' error for every launched
task.

Moreover, it seems that this feature is coming in 0.30: "Executors no
longer inherit environment variables from the agent".

This patch partially revert back 07ce21d where chmod_x method was
removed in favor of using sys.executable.


Diffs
-

  src/main/python/apache/aurora/executor/thermos_task_runner.py 
3896e3841562600379705dbf78a6f62728246348 

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


Testing (updated)
---

Make Aurora executor run on CentOS7, while running mesos agent with 
`--executor_environment_variables` option and no excplicit $PATH set.


Thanks,

Pierre Cheynier

Re: Review Request 45135: Descheduling a cron should not fail if the job is not scheduled.

[Zameer Manji]

> On March 21, 2016, 8:48 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java,
> >  line 345
> > 
> >
> > I think we should be consistent and return a message like "No active 
> > cron schedule found" in this case similar the `killTasks` behavior.

+1


- Zameer


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


On March 21, 2016, 5:19 p.m., Stephan Erb wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45135/
> ---
> 
> (Updated March 21, 2016, 5:19 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Bugs: AURORA-1645
> https://issues.apache.org/jira/browse/AURORA-1645
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This allows clients to be more declarative rather than imperative
> when expressing that the given job should not be scheduled.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 6e9364e34db6dbb270468db3ff333b956c6bf9f3 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  d217faf44ab3d6132db3b3c4eed67effd03fb6fa 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  c774ac0f0e2fdda7fe9b64fd9181f107b3fd9eca 
> 
> Diff: https://reviews.apache.org/r/45135/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Stephan Erb
> 
>

Re: Review Request 45167: Fixup install docs.

[John Sirois]

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

(Updated March 22, 2016, 11:48 a.m.)


Review request for Aurora, Benjamin Rice and Stephan Erb.


Changes
---

Fixup "Installing Mesos" code block indents.

 docs/installing.md | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)


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


Repository: aurora


Description
---

This set of fixes eliminates the experimental rpm warnings and updates
the rpm instructions to use the officially released packages.  The deb
instructions are updated as well, in particular to take advantage of the
mesosphere deb repository and with movement of special dep installations
to the appropriate sections requiring them.

This fix RB does not address the new Debian Jessie debs, instead
focusing on getting the existing instructions corrected.

 docs/installing.md | 101 +++---
 1 file changed, 55 insertions(+), 46 deletions(-)


Diffs (updated)
-

  docs/installing.md c3abb332a4a46e62367a39cf73d70f2185657b5a 

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


Testing
---

These changes are rendered here:
  
https://github.com/jsirois/aurora/blob/jsirois/issues/AURORA-1647/docs/installing.md

I ran through the instructions in full in fresh Vagrant vms and was able
to launch sample jobs (used aurora-packaging test jobs with cpu droppped to
`0.5` and s/Service/Job/) and exercise the full UI chain to inspect the
successful one-shot job and its sandbox & logs.


Thanks,

John Sirois

Re: Review Request 45112: Add support for storing and fetching images as properties of task configs.

[Joshua Cohen]

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

(Updated March 22, 2016, 3:49 p.m.)


Review request for Aurora, Maxim Khutornenko and Bill Farner.


Changes
---

Switch to separate tables for docker and appc images.


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


Repository: aurora


Description
---

Add support for storing and fetching images as properties of task configs.


Diffs (updated)
-

  api/src/main/thrift/org/apache/aurora/gen/api.thrift 
d4b8904031e6671a8083cac9b82d934377797fe2 
  
src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
 b3b8ccf868c2a2f18f720a837e90d763072dd3eb 
  src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
364026a8ef2b47cf1beafa3990691d3375516fe6 
  src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java 
12ca16b79a062d9ea15c206ef963fb077ad7ad98 
  src/main/java/org/apache/aurora/scheduler/storage/db/views/DbImage.java 
PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 
eb848add00fba6d3571657bb9080be0599b2756a 
  
src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml 
fd272ccf9b1cfccd9198d1e5e0db37d23f546afa 
  src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
be60c3be0cd51d06ba0ca7f56384281ee71d8c55 
  
src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
 1a520b3cb035a9afc25406d2f313c9f861eee4d6 
  
src/test/java/org/apache/aurora/scheduler/storage/AbstractCronJobStoreTest.java 
c316e497a34a45c7ada2ca83a1115e826c0f572f 
  src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
e56fed2e6c0cdb47737cf1a9b637c44c5e5b9815 
  
src/test/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStoreTest.java 
08530397ff75081bde6f07f9d53317b5486e0da4 

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


Testing
---

./gradlew build -Pq

ran e2e tests.


Thanks,

Joshua Cohen

Re: Review Request 45135: Descheduling a cron should not fail if the job is not scheduled.

[Zameer Manji]

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


Ship it!




LGTM modulo Maxim's comment.

- Zameer Manji


On March 21, 2016, 5:19 p.m., Stephan Erb wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45135/
> ---
> 
> (Updated March 21, 2016, 5:19 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Bugs: AURORA-1645
> https://issues.apache.org/jira/browse/AURORA-1645
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This allows clients to be more declarative rather than imperative
> when expressing that the given job should not be scheduled.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 6e9364e34db6dbb270468db3ff333b956c6bf9f3 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  d217faf44ab3d6132db3b3c4eed67effd03fb6fa 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  c774ac0f0e2fdda7fe9b64fd9181f107b3fd9eca 
> 
> Diff: https://reviews.apache.org/r/45135/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Stephan Erb
> 
>

Review Request 45167: Fixup install docs.

[John Sirois]

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

Review request for Aurora, Benjamin Rice and Stephan Erb.


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


Repository: aurora


Description
---

This set of fixes eliminates the experimental rpm warnings and updates
the rpm instructions to use the officially released packages.  The deb
instructions are updated as well, in particular to take advantage of the
mesosphere deb repository and with movement of special dep installations
to the appropriate sections requiring them.

This fix RB does not address the new Debian Jessie debs, instead
focusing on getting the existing instructions corrected.

 docs/installing.md | 101 +++---
 1 file changed, 55 insertions(+), 46 deletions(-)


Diffs
-

  docs/installing.md c3abb332a4a46e62367a39cf73d70f2185657b5a 

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


Testing
---

These changes are rendered here:
  
https://github.com/jsirois/aurora/blob/jsirois/issues/AURORA-1647/docs/installing.md

I ran through the instructions in full in fresh Vagrant vms and was able
to launch sample jobs (used aurora-packaging test jobs with cpu droppped to
`0.5` and s/Service/Job/) and exercise the full UI chain to inspect the
successful one-shot job and its sandbox & logs.


Thanks,

John Sirois

Review Request 45172: Tweak update-sources script to also update mesos config.

[Joshua Cohen]

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

Review request for Aurora and Bill Farner.


Repository: aurora


Description
---

Tweak update-sources script to also update mesos config.


Diffs
-

  examples/vagrant/provision-dev-cluster.sh 
b1f661ea90ac427621518f653cce1b7630d2a6ed 

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


Testing
---


Thanks,

Joshua Cohen

Re: Review Request 45042: Add ACL support for announcer

[Stephan Erb]

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



Drive-by review: Please also add an entry to the RELEASE-NOTES.md. Thanks! :-)

- Stephan Erb


On March 22, 2016, 6:02 p.m., Kunal Thakar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45042/
> ---
> 
> (Updated March 22, 2016, 6:02 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add ACL support for announcer
> https://issues.apache.org/jira/browse/AURORA-1643
> 
> Adding support for service discovery ZK authentication. ZK authentication 
> secrets should be stored in a file as json (as follows):
> ```json
> {
>   "scheme": "",
>   "credential": "",
>   "permissions": {
> "read": ,
> "write": ,
> "create": ,
> "delete": ,
> "admin": ,
> "all": 
>   }
> }
> ```
> 
> 
> Diffs
> -
> 
>   docs/security.md 32bea426fbceec83187e851a5db11e82df55e962 
>   src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
> 6634506108c346f8c23b2da7cc8d20d09d07d590 
>   src/main/python/apache/aurora/executor/common/announcer.py 
> 79a9cfb6ac3a8444f09fb3658e6e859e06941ba4 
>   src/test/python/apache/aurora/executor/common/test_announcer.py 
> 142b58d5e577c9f4b8e2ae8473cffdea94eba21f 
> 
> Diff: https://reviews.apache.org/r/45042/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kunal Thakar
> 
>

Re: Review Request 45112: Add support for storing and fetching images as properties of task configs.

[Aurora ReviewBot]

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


Ship it!




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

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

- Aurora ReviewBot


On March 22, 2016, 4:46 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45112/
> ---
> 
> (Updated March 22, 2016, 4:46 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
> 
> 
> Bugs: AURORA-1635
> https://issues.apache.org/jira/browse/AURORA-1635
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add support for storing and fetching images as properties of task configs.
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> d4b8904031e6671a8083cac9b82d934377797fe2 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  b3b8ccf868c2a2f18f720a837e90d763072dd3eb 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
> 364026a8ef2b47cf1beafa3990691d3375516fe6 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java 
> 12ca16b79a062d9ea15c206ef963fb077ad7ad98 
>   src/main/java/org/apache/aurora/scheduler/storage/db/views/DbImage.java 
> PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 
> eb848add00fba6d3571657bb9080be0599b2756a 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml
>  fd272ccf9b1cfccd9198d1e5e0db37d23f546afa 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> be60c3be0cd51d06ba0ca7f56384281ee71d8c55 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  1a520b3cb035a9afc25406d2f313c9f861eee4d6 
>   
> src/test/java/org/apache/aurora/scheduler/storage/AbstractCronJobStoreTest.java
>  c316e497a34a45c7ada2ca83a1115e826c0f572f 
>   
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
> e56fed2e6c0cdb47737cf1a9b637c44c5e5b9815 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStoreTest.java
>  08530397ff75081bde6f07f9d53317b5486e0da4 
> 
> Diff: https://reviews.apache.org/r/45112/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build -Pq
> 
> ran e2e tests.
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>

Re: Review Request 45042: Add ACL support for announcer

[Aurora ReviewBot]

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



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

   mock_dump_runner_pex.return_value = Mock()
   mock_options = Mock()
   mock_options.execute_as_user = False
   mock_options.nosetuid = False
   with patch(
   
'apache.aurora.executor.bin.thermos_executor_main.dump_runner_pex',
   return_value=mock_dump_runner_pex):
 with patch(
 
'apache.aurora.executor.bin.thermos_executor_main.DefaultThermosTaskRunnerProvider',
 return_value=mock_runner_provider) as 
mock_provider:
 
   expected_path = 
os.path.join(os.path.abspath('.'), MesosPathDetector.DEFAULT_SANDBOX_PATH)
 > thermos_executor = 
initialize(mock_options)
 
 
src/test/python/apache/aurora/executor/bin/test_thermos_executor_entry_point.py:44:
 
 _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
 
.pants.d/python-setup/chroots/3acd03dbb81b37f2a14a0799a3e0bd882c60c91a/apache/aurora/executor/bin/thermos_executor_main.py:204:
 in initialize
 default_acl = 
make_default_acl(options.announcer_zookeeper_auth_config)
 
.pants.d/python-setup/chroots/3acd03dbb81b37f2a14a0799a3e0bd882c60c91a/apache/aurora/executor/common/announcer.py:107:
 in make_default_acl
 auth_config = parse_config(zk_auth_config)
 _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
 
 config_file = 
 
 def parse_config(config_file):
   try:
 >   with open(config_file) as fp:
 E   TypeError: coercing to Unicode: need 
string or buffer, Mock found
 
 
.pants.d/python-setup/chroots/3acd03dbb81b37f2a14a0799a3e0bd882c60c91a/apache/aurora/executor/common/announcer.py:92:
 TypeError
 -- Captured stderr call --
 WARNING:root:Please remove the deprecated and no-op 
--announcer-enable flag in scheduler config!
  generated xml file: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/test-results/415337499eb72578eab327a6487c1f5c9452b3d6.xml
 
  1 failed, 667 passed, 5 skipped, 1 warnings in 
438.62 seconds 
 
FAILURE


17:31:23 08:55   [complete]
   FAILURE


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

- Aurora ReviewBot


On March 22, 2016, 5:02 p.m., Kunal Thakar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45042/
> ---
> 
> (Updated March 22, 2016, 5:02 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add ACL support for announcer
> https://issues.apache.org/jira/browse/AURORA-1643
> 
> Adding support for service discovery ZK authentication. ZK authentication 
> secrets should be stored in a file as json (as follows):
> ```json
> {
>   "scheme": "",
>   "credential": "",
>   "permissions": {
> "read": ,
> "write": ,
> "create": ,
> "delete": ,
> "admin": ,
> "all": 
>   }
> }
> ```
> 
> 
> Diffs
> -
> 
>   docs/security.md 32bea426fbceec83187e851a5db11e82df55e962 
>   src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
> 6634506108c346f8c23b2da7cc8d20d09d07d590 
>   src/main/python/apache/aurora/executor/common/announcer.py 
> 79a9cfb6ac3a8444f09fb3658e6e859e06941ba4 
>   src/test/python/apache/aurora/executor/common/test_announcer.py 
> 142b58d5e577c9f4b8e2ae8473cffdea94eba21f 
> 
> Diff: https://reviews.apache.org/r/45042/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kunal Thakar
> 
>

Re: Review Request 45042: Add ACL support for announcer

[Kunal Thakar]

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

(Updated March 22, 2016, 6:51 p.m.)


Review request for Aurora, Bill Farner and Zameer Manji.


Changes
---

Add release note and fix test. @ReviewBot retry


Repository: aurora


Description
---

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

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


Diffs (updated)
-

  RELEASE-NOTES.md 6e9364e34db6dbb270468db3ff333b956c6bf9f3 
  docs/security.md 32bea426fbceec83187e851a5db11e82df55e962 
  src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
6634506108c346f8c23b2da7cc8d20d09d07d590 
  src/main/python/apache/aurora/executor/common/announcer.py 
79a9cfb6ac3a8444f09fb3658e6e859e06941ba4 
  
src/test/python/apache/aurora/executor/bin/test_thermos_executor_entry_point.py 
e9f7851292aef3a36da5da9b0fc333a7e7750cf3 
  src/test/python/apache/aurora/executor/common/test_announcer.py 
142b58d5e577c9f4b8e2ae8473cffdea94eba21f 

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


Testing
---


Thanks,

Kunal Thakar

Re: Review Request 45042: Add ACL support for announcer

[Zameer Manji]

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


Ship it!




This change LGTM. I was a bit confused if `default_acl` was supposed to be a 
list or a single instance of `ACL`, but I confirmed that it was a list via 
checking the source of Kazoo 1.3.1.

Thanks for your contribution, this should help operators who want to maintain a 
secure ZK cluster for service discovery.

- Zameer Manji


On March 22, 2016, 10:02 a.m., Kunal Thakar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45042/
> ---
> 
> (Updated March 22, 2016, 10:02 a.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add ACL support for announcer
> https://issues.apache.org/jira/browse/AURORA-1643
> 
> Adding support for service discovery ZK authentication. ZK authentication 
> secrets should be stored in a file as json (as follows):
> ```json
> {
>   "scheme": "",
>   "credential": "",
>   "permissions": {
> "read": ,
> "write": ,
> "create": ,
> "delete": ,
> "admin": ,
> "all": 
>   }
> }
> ```
> 
> 
> Diffs
> -
> 
>   docs/security.md 32bea426fbceec83187e851a5db11e82df55e962 
>   src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
> 6634506108c346f8c23b2da7cc8d20d09d07d590 
>   src/main/python/apache/aurora/executor/common/announcer.py 
> 79a9cfb6ac3a8444f09fb3658e6e859e06941ba4 
>   src/test/python/apache/aurora/executor/common/test_announcer.py 
> 142b58d5e577c9f4b8e2ae8473cffdea94eba21f 
> 
> Diff: https://reviews.apache.org/r/45042/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kunal Thakar
> 
>

Re: Review Request 45167: Fixup install docs.

[Aurora ReviewBot]

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


Ship it!




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

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

- Aurora ReviewBot


On March 22, 2016, 5:48 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45167/
> ---
> 
> (Updated March 22, 2016, 5:48 p.m.)
> 
> 
> Review request for Aurora, Benjamin Rice and Stephan Erb.
> 
> 
> Bugs: AURORA-1647
> https://issues.apache.org/jira/browse/AURORA-1647
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This set of fixes eliminates the experimental rpm warnings and updates
> the rpm instructions to use the officially released packages.  The deb
> instructions are updated as well, in particular to take advantage of the
> mesosphere deb repository and with movement of special dep installations
> to the appropriate sections requiring them.
> 
> This fix RB does not address the new Debian Jessie debs, instead
> focusing on getting the existing instructions corrected.
> 
>  docs/installing.md | 101 +++---
>  1 file changed, 55 insertions(+), 46 deletions(-)
> 
> 
> Diffs
> -
> 
>   docs/installing.md c3abb332a4a46e62367a39cf73d70f2185657b5a 
> 
> Diff: https://reviews.apache.org/r/45167/diff/
> 
> 
> Testing
> ---
> 
> These changes are rendered here:
>   
> https://github.com/jsirois/aurora/blob/jsirois/issues/AURORA-1647/docs/installing.md
> 
> I ran through the instructions in full in fresh Vagrant vms and was able
> to launch sample jobs (used aurora-packaging test jobs with cpu droppped to
> `0.5` and s/Service/Job/) and exercise the full UI chain to inspect the
> successful one-shot job and its sandbox & logs.
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 45112: Add support for storing and fetching images as properties of task configs.

[Maxim Khutornenko]

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


Ship it!




Thanks for following up!

- Maxim Khutornenko


On March 22, 2016, 3:49 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45112/
> ---
> 
> (Updated March 22, 2016, 3:49 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
> 
> 
> Bugs: AURORA-1635
> https://issues.apache.org/jira/browse/AURORA-1635
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add support for storing and fetching images as properties of task configs.
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> d4b8904031e6671a8083cac9b82d934377797fe2 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  b3b8ccf868c2a2f18f720a837e90d763072dd3eb 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
> 364026a8ef2b47cf1beafa3990691d3375516fe6 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java 
> 12ca16b79a062d9ea15c206ef963fb077ad7ad98 
>   src/main/java/org/apache/aurora/scheduler/storage/db/views/DbImage.java 
> PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 
> eb848add00fba6d3571657bb9080be0599b2756a 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml
>  fd272ccf9b1cfccd9198d1e5e0db37d23f546afa 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> be60c3be0cd51d06ba0ca7f56384281ee71d8c55 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  1a520b3cb035a9afc25406d2f313c9f861eee4d6 
>   
> src/test/java/org/apache/aurora/scheduler/storage/AbstractCronJobStoreTest.java
>  c316e497a34a45c7ada2ca83a1115e826c0f572f 
>   
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
> e56fed2e6c0cdb47737cf1a9b637c44c5e5b9815 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStoreTest.java
>  08530397ff75081bde6f07f9d53317b5486e0da4 
> 
> Diff: https://reviews.apache.org/r/45112/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build -Pq
> 
> ran e2e tests.
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>

Re: Review Request 45112: Add support for storing and fetching images as properties of task configs.

[Stephan Erb]

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




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


Shouldn't that rather be formulated as container != `MESOS`? As the Image 
field will only work together with the Mesos containerizer.


- Stephan Erb


On March 22, 2016, 4:49 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45112/
> ---
> 
> (Updated March 22, 2016, 4:49 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
> 
> 
> Bugs: AURORA-1635
> https://issues.apache.org/jira/browse/AURORA-1635
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add support for storing and fetching images as properties of task configs.
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> d4b8904031e6671a8083cac9b82d934377797fe2 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  b3b8ccf868c2a2f18f720a837e90d763072dd3eb 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
> 364026a8ef2b47cf1beafa3990691d3375516fe6 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java 
> 12ca16b79a062d9ea15c206ef963fb077ad7ad98 
>   src/main/java/org/apache/aurora/scheduler/storage/db/views/DbImage.java 
> PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 
> eb848add00fba6d3571657bb9080be0599b2756a 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml
>  fd272ccf9b1cfccd9198d1e5e0db37d23f546afa 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> be60c3be0cd51d06ba0ca7f56384281ee71d8c55 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  1a520b3cb035a9afc25406d2f313c9f861eee4d6 
>   
> src/test/java/org/apache/aurora/scheduler/storage/AbstractCronJobStoreTest.java
>  c316e497a34a45c7ada2ca83a1115e826c0f572f 
>   
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
> e56fed2e6c0cdb47737cf1a9b637c44c5e5b9815 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStoreTest.java
>  08530397ff75081bde6f07f9d53317b5486e0da4 
> 
> Diff: https://reviews.apache.org/r/45112/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build -Pq
> 
> ran e2e tests.
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>

Re: Review Request 45112: Add support for storing and fetching images as properties of task configs.

[Joshua Cohen]

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

(Updated March 22, 2016, 4:25 p.m.)


Review request for Aurora, Maxim Khutornenko and Bill Farner.


Changes
---

checkstyle.


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


Repository: aurora


Description
---

Add support for storing and fetching images as properties of task configs.


Diffs (updated)
-

  api/src/main/thrift/org/apache/aurora/gen/api.thrift 
d4b8904031e6671a8083cac9b82d934377797fe2 
  
src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
 b3b8ccf868c2a2f18f720a837e90d763072dd3eb 
  src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
364026a8ef2b47cf1beafa3990691d3375516fe6 
  src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java 
12ca16b79a062d9ea15c206ef963fb077ad7ad98 
  src/main/java/org/apache/aurora/scheduler/storage/db/views/DbImage.java 
PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 
eb848add00fba6d3571657bb9080be0599b2756a 
  
src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml 
fd272ccf9b1cfccd9198d1e5e0db37d23f546afa 
  src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
be60c3be0cd51d06ba0ca7f56384281ee71d8c55 
  
src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
 1a520b3cb035a9afc25406d2f313c9f861eee4d6 
  
src/test/java/org/apache/aurora/scheduler/storage/AbstractCronJobStoreTest.java 
c316e497a34a45c7ada2ca83a1115e826c0f572f 
  src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
e56fed2e6c0cdb47737cf1a9b637c44c5e5b9815 
  
src/test/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStoreTest.java 
08530397ff75081bde6f07f9d53317b5486e0da4 

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


Testing
---

./gradlew build -Pq

ran e2e tests.


Thanks,

Joshua Cohen

Re: Review Request 45112: Add support for storing and fetching images as properties of task configs.

[Aurora ReviewBot]

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



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

:commons:generateThriftResources
:commons:processResources
:commons:classes
:commons:jar
:compileJava/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java:74:
 Note: Wrote forwarder 
org.apache.aurora.scheduler.storage.log.WriteAheadStorageForwarder
@Forward({
^
Note: Writing 
file:/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/classes/main/org/apache/aurora/common/args/apt/cmdline.arg.info.txt.2
Note: Writing 
file:/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/classes/main/META-INF/compiler/resource-mappings/org.apache.aurora.common.args.apt.CmdLineProcessor

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

:processJmhResources UP-TO-DATE
:jmhClasses
:checkstyleJmh
:jsHint
:checkstyleMain[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/storage/db/views/DbImage.java:33:
 error: 'else' child have incorrect indentation level 8, expected level should 
be 6.
 FAILED

FAILURE: Build failed with an exception.

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

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

BUILD FAILED

Total time: 2 mins 16.503 secs


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

- Aurora ReviewBot


On March 22, 2016, 3:49 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45112/
> ---
> 
> (Updated March 22, 2016, 3:49 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
> 
> 
> Bugs: AURORA-1635
> https://issues.apache.org/jira/browse/AURORA-1635
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add support for storing and fetching images as properties of task configs.
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> d4b8904031e6671a8083cac9b82d934377797fe2 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  b3b8ccf868c2a2f18f720a837e90d763072dd3eb 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
> 364026a8ef2b47cf1beafa3990691d3375516fe6 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java 
> 12ca16b79a062d9ea15c206ef963fb077ad7ad98 
>   src/main/java/org/apache/aurora/scheduler/storage/db/views/DbImage.java 
> PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 
> eb848add00fba6d3571657bb9080be0599b2756a 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml
>  fd272ccf9b1cfccd9198d1e5e0db37d23f546afa 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> be60c3be0cd51d06ba0ca7f56384281ee71d8c55 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  1a520b3cb035a9afc25406d2f313c9f861eee4d6 
>   
> src/test/java/org/apache/aurora/scheduler/storage/AbstractCronJobStoreTest.java
>  c316e497a34a45c7ada2ca83a1115e826c0f572f 
>   
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
> e56fed2e6c0cdb47737cf1a9b637c44c5e5b9815 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStoreTest.java
>  08530397ff75081bde6f07f9d53317b5486e0da4 
> 
> Diff: https://reviews.apache.org/r/45112/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build -Pq
> 
> ran e2e tests.
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>

Re: Review Request 45115: Ensure final processes are executed when ephemeral daemon processes exist.

[Zameer Manji]

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


Ship it!




Ship It!

- Zameer Manji


On March 22, 2016, 2:04 p.m., Amol Deshmukh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45115/
> ---
> 
> (Updated March 22, 2016, 2:04 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Bugs: AURORA-1642
> https://issues.apache.org/jira/browse/AURORA-1642
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Ensure final processes are executed when ephemeral daemon processes exist.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/thermos/core/process.py 
> f147af7d0e84309691c135c8057a597379fa83e7 
>   src/test/python/apache/thermos/core/test_process.py 
> c339c91eb24616c8d640877ef088f659523d2bf5 
>   src/test/sh/org/apache/aurora/e2e/ephemeral_daemon_with_final.aurora 
> PRE-CREATION 
>   src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 
> b469f9bbbdfbf98df947832411bd0cdce97affdc 
> 
> Diff: https://reviews.apache.org/r/45115/diff/
> 
> 
> Testing
> ---
> 
> # End to end tests
> ```
> $ ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> ...
> *** OK (All tests passed) ***
> ```
> 
> # Python unit tests
> ```
> $ ./pants test src/test/python::
> ...
>   663 passed, 5 skipped, 1 warnings in 180.34 seconds
> 
> ```
> 
> # Also tested using a test job in vagrant to ensure that "final" processes 
> are executed as expected using this job definition:
> https://gist.github.com/adeshmukh/697d013dec64498a3942
> 
> 
> Thanks,
> 
> Amol Deshmukh
> 
>

Re: Review Request 45042: Add ACL support for announcer

[Aurora ReviewBot]

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


Ship it!




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

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

- Aurora ReviewBot


On March 22, 2016, 6:51 p.m., Kunal Thakar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45042/
> ---
> 
> (Updated March 22, 2016, 6:51 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add ACL support for announcer
> https://issues.apache.org/jira/browse/AURORA-1643
> 
> Adding support for service discovery ZK authentication. ZK authentication 
> secrets should be stored in a file as json (as follows):
> ```json
> {
>   "scheme": "",
>   "credential": "",
>   "permissions": {
> "read": ,
> "write": ,
> "create": ,
> "delete": ,
> "admin": ,
> "all": 
>   }
> }
> ```
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 6e9364e34db6dbb270468db3ff333b956c6bf9f3 
>   docs/security.md 32bea426fbceec83187e851a5db11e82df55e962 
>   src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
> 6634506108c346f8c23b2da7cc8d20d09d07d590 
>   src/main/python/apache/aurora/executor/common/announcer.py 
> 79a9cfb6ac3a8444f09fb3658e6e859e06941ba4 
>   
> src/test/python/apache/aurora/executor/bin/test_thermos_executor_entry_point.py
>  e9f7851292aef3a36da5da9b0fc333a7e7750cf3 
>   src/test/python/apache/aurora/executor/common/test_announcer.py 
> 142b58d5e577c9f4b8e2ae8473cffdea94eba21f 
> 
> Diff: https://reviews.apache.org/r/45042/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kunal Thakar
> 
>

Re: Review Request 45178: Revert "Add support for storing and fetching images as properties of task configs."

[Maxim Khutornenko]

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


Ship it!




Ship It!

- Maxim Khutornenko


On March 22, 2016, 8:33 p.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45178/
> ---
> 
> (Updated March 22, 2016, 8:33 p.m.)
> 
> 
> Review request for Aurora and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-1648
> https://issues.apache.org/jira/browse/AURORA-1648
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This reverts commit d5d7ec0eb5703d6bda8c43cd0586684a550a575a. As outlined in 
> `AURORA-1648` the schema changes here are not necessarily reflected in H2 
> after a restore from backup.
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 98064bb4607e9fbdff71eaceb2784f407fe91108 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  c6785d0a997dde0783384ccaa7e241d32e1dd898 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
> 25160df1be9539c1ecd4613db5deb99a9606a512 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java 
> e778a39ef61e456ad9d2d2aa6f3e5d69e44bf02c 
>   src/main/java/org/apache/aurora/scheduler/storage/db/views/DbImage.java 
> 5964a2a0b1f602eda0f8f2a8f31a2f15d93dcca7 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 
> cdd1060401a38e26cd726ec95e2bb617ccf4708c 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml
>  cfeb69ba65875831f88ea94b791214391991b3b8 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> 92a0798d4d97920b786c4713f8779a7a32767652 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  52e97080760a70f8e003b1cc7fc0fbeb2ec6d689 
>   
> src/test/java/org/apache/aurora/scheduler/storage/AbstractCronJobStoreTest.java
>  2343394a34541bc266f74b5da512cda6f8e6b56a 
>   
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
> 8a87ff6445ab83aea144867d029537f86c7b61d7 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStoreTest.java
>  e43ec6cb35cbb454b967238dfb9ce006b21f4fb6 
> 
> Diff: https://reviews.apache.org/r/45178/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>

Re: Review Request 45177: Prototype of setting DiscoveryInfo.

[Aurora ReviewBot]

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



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

:compileJava/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java:74:
 Note: Wrote forwarder 
org.apache.aurora.scheduler.storage.log.WriteAheadStorageForwarder
@Forward({
^
Note: Writing 
file:/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/classes/main/org/apache/aurora/common/args/apt/cmdline.arg.info.txt.2
Note: Writing 
file:/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/classes/main/META-INF/compiler/resource-mappings/org.apache.aurora.common.args.apt.CmdLineProcessor

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

:processJmhResources UP-TO-DATE
:jmhClasses
:checkstyleJmh
:jsHint
:checkstyleMain[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java:20:
 error: 'org.apache.aurora.GuavaUtils' should be separated from previous 
imports.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java:31:
 error: Using the '.*' form of import should be avoided - 
org.apache.aurora.scheduler.storage.entities.*.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java:33:
 error: Using the '.*' form of import should be avoided - 
org.apache.mesos.Protos.*.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java:37:
 error: Wrong order for 'javax.inject.Inject' import. Order should be: java, 
javax, scala, com, net, org. Each group should be separated by a single 
blank line.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java:38:
 error: Wrong order for 'java.util.List' import. Order should be: java, javax, 
scala, com, net, org. Each group should be separated by a single blank 
line.
 FAILED

FAILURE: Build failed with an exception.

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

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

BUILD FAILED

Total time: 1 mins 44.437 secs


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

- Aurora ReviewBot


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

Re: Review Request 45115: AURORA-1642: Thermos runner finalization broken.

[Amol Deshmukh]

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

(Updated March 22, 2016, 1:06 p.m.)


Review request for Aurora, Maxim Khutornenko and Zameer Manji.


Changes
---

Added e2e test to ensure that final processes are executed on completion of 
regular processes in the presence of long-running ephemeral_daemon processes.


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


Repository: aurora


Description
---

AURORA-1642: Fix for broken finalization in Thermos runner.


Diffs (updated)
-

  src/main/python/apache/thermos/core/process.py 
f147af7d0e84309691c135c8057a597379fa83e7 
  src/test/python/apache/thermos/core/test_process.py 
c339c91eb24616c8d640877ef088f659523d2bf5 
  src/test/sh/org/apache/aurora/e2e/ephemeral_daemon_with_final.aurora 
PRE-CREATION 
  src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 
b469f9bbbdfbf98df947832411bd0cdce97affdc 

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


Testing
---

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

# Python unit tests
```
$ ./pants test src/test/python::
...
  663 passed, 5 skipped, 1 warnings in 180.34 seconds

```

# Also tested using a test job in vagrant to ensure that "final" processes are 
executed as expected using this job definition:
https://gist.github.com/adeshmukh/697d013dec64498a3942


Thanks,

Amol Deshmukh

Re: Review Request 45115: Ensure final processes are executed when ephemeral daemon processes exist.

[Amol Deshmukh]

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

(Updated March 22, 2016, 2:04 p.m.)


Review request for Aurora, Maxim Khutornenko and Zameer Manji.


Changes
---

Refactored based on Maxim's offline feedback.


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


Repository: aurora


Description
---

Ensure final processes are executed when ephemeral daemon processes exist.


Diffs (updated)
-

  src/main/python/apache/thermos/core/process.py 
f147af7d0e84309691c135c8057a597379fa83e7 
  src/test/python/apache/thermos/core/test_process.py 
c339c91eb24616c8d640877ef088f659523d2bf5 
  src/test/sh/org/apache/aurora/e2e/ephemeral_daemon_with_final.aurora 
PRE-CREATION 
  src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 
b469f9bbbdfbf98df947832411bd0cdce97affdc 

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


Testing
---

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

# Python unit tests
```
$ ./pants test src/test/python::
...
  663 passed, 5 skipped, 1 warnings in 180.34 seconds

```

# Also tested using a test job in vagrant to ensure that "final" processes are 
executed as expected using this job definition:
https://gist.github.com/adeshmukh/697d013dec64498a3942


Thanks,

Amol Deshmukh

Re: Review Request 45115: Ensure final processes are executed when ephemeral daemon processes exist.

[Aurora ReviewBot]

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


Ship it!




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

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

- Aurora ReviewBot


On March 22, 2016, 8:23 p.m., Amol Deshmukh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45115/
> ---
> 
> (Updated March 22, 2016, 8:23 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Bugs: AURORA-1642
> https://issues.apache.org/jira/browse/AURORA-1642
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Ensure final processes are executed when ephemeral daemon processes exist.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/thermos/core/process.py 
> f147af7d0e84309691c135c8057a597379fa83e7 
>   src/test/python/apache/thermos/core/test_process.py 
> c339c91eb24616c8d640877ef088f659523d2bf5 
>   src/test/sh/org/apache/aurora/e2e/ephemeral_daemon_with_final.aurora 
> PRE-CREATION 
>   src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 
> b469f9bbbdfbf98df947832411bd0cdce97affdc 
> 
> Diff: https://reviews.apache.org/r/45115/diff/
> 
> 
> Testing
> ---
> 
> # End to end tests
> ```
> $ ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> ...
> *** OK (All tests passed) ***
> ```
> 
> # Python unit tests
> ```
> $ ./pants test src/test/python::
> ...
>   663 passed, 5 skipped, 1 warnings in 180.34 seconds
> 
> ```
> 
> # Also tested using a test job in vagrant to ensure that "final" processes 
> are executed as expected using this job definition:
> https://gist.github.com/adeshmukh/697d013dec64498a3942
> 
> 
> Thanks,
> 
> Amol Deshmukh
> 
>

Review Request 45178: Revert "Add support for storing and fetching images as properties of task configs."

[Zameer Manji]

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

Review request for Aurora and Maxim Khutornenko.


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


Repository: aurora


Description
---

This reverts commit d5d7ec0eb5703d6bda8c43cd0586684a550a575a. As outlined in 
`AURORA-1648` the schema changes here are not necessarily reflected in H2 after 
a restore from backup.


Diffs
-

  api/src/main/thrift/org/apache/aurora/gen/api.thrift 
98064bb4607e9fbdff71eaceb2784f407fe91108 
  
src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
 c6785d0a997dde0783384ccaa7e241d32e1dd898 
  src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
25160df1be9539c1ecd4613db5deb99a9606a512 
  src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java 
e778a39ef61e456ad9d2d2aa6f3e5d69e44bf02c 
  src/main/java/org/apache/aurora/scheduler/storage/db/views/DbImage.java 
5964a2a0b1f602eda0f8f2a8f31a2f15d93dcca7 
  src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 
cdd1060401a38e26cd726ec95e2bb617ccf4708c 
  
src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml 
cfeb69ba65875831f88ea94b791214391991b3b8 
  src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
92a0798d4d97920b786c4713f8779a7a32767652 
  
src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
 52e97080760a70f8e003b1cc7fc0fbeb2ec6d689 
  
src/test/java/org/apache/aurora/scheduler/storage/AbstractCronJobStoreTest.java 
2343394a34541bc266f74b5da512cda6f8e6b56a 
  src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
8a87ff6445ab83aea144867d029537f86c7b61d7 
  
src/test/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStoreTest.java 
e43ec6cb35cbb454b967238dfb9ce006b21f4fb6 

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


Testing
---


Thanks,

Zameer Manji

Re: Review Request 45179: Support for overriding --mesos-root under upstart.

[John Sirois]

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

(Updated March 22, 2016, 4:49 p.m.)


Review request for Aurora and Stephan Erb.


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


Repository: aurora-packaging


Description
---

Previously only the sysv init script supported overriding `--mesos-root`
via an env var, and thus via `/etc/default/thermos`.

 specs/debian/aurora-executor.thermos.upstart | 1 +
 1 file changed, 1 insertion(+)


Diffs
-

  specs/debian/aurora-executor.thermos.upstart 
7ebe4bea4debc9f558fae92c4d532145156ec43e 

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


Testing (updated)
---

Used this edit in testing done for https://reviews.apache.org/r/45167


Thanks,

John Sirois

Re: Review Request 45182: Improve mname and structdump documentation

[Maxim Khutornenko]

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




src/main/java/org/apache/aurora/scheduler/http/Mname.java (line 59)


?


- Maxim Khutornenko


On March 22, 2016, 11:09 p.m., Stephan Erb wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45182/
> ---
> 
> (Updated March 22, 2016, 11:09 p.m.)
> 
> 
> Review request for Aurora and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Improve mname and structdump documentation
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/http/Mname.java 
> d6e5fc4e886660311228d411cae1bbb42aaffb19 
>   src/main/java/org/apache/aurora/scheduler/http/StructDump.java 
> f84767a2c01bccc23182672123f9ca6701fcd696 
> 
> Diff: https://reviews.apache.org/r/45182/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build 
> 
> Verified rendering in Vagrant
> 
> 
> Thanks,
> 
> Stephan Erb
> 
>

Re: Review Request 45115: Ensure final processes are executed when ephemeral daemon processes exist.

[Maxim Khutornenko]

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


Ship it!





src/main/python/apache/thermos/core/process.py (line 521)


drop extra "the"



src/test/sh/org/apache/aurora/e2e/ephemeral_daemon_with_final.aurora (line 39)


you can drop this line as default `instances` is already set to 1


- Maxim Khutornenko


On March 22, 2016, 9:04 p.m., Amol Deshmukh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45115/
> ---
> 
> (Updated March 22, 2016, 9:04 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Bugs: AURORA-1642
> https://issues.apache.org/jira/browse/AURORA-1642
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Ensure final processes are executed when ephemeral daemon processes exist.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/thermos/core/process.py 
> f147af7d0e84309691c135c8057a597379fa83e7 
>   src/test/python/apache/thermos/core/test_process.py 
> c339c91eb24616c8d640877ef088f659523d2bf5 
>   src/test/sh/org/apache/aurora/e2e/ephemeral_daemon_with_final.aurora 
> PRE-CREATION 
>   src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 
> b469f9bbbdfbf98df947832411bd0cdce97affdc 
> 
> Diff: https://reviews.apache.org/r/45115/diff/
> 
> 
> Testing
> ---
> 
> # End to end tests
> ```
> $ ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> ...
> *** OK (All tests passed) ***
> ```
> 
> # Python unit tests
> ```
> $ ./pants test src/test/python::
> ...
>   663 passed, 5 skipped, 1 warnings in 180.34 seconds
> 
> ```
> 
> # Also tested using a test job in vagrant to ensure that "final" processes 
> are executed as expected using this job definition:
> https://gist.github.com/adeshmukh/697d013dec64498a3942
> 
> 
> Thanks,
> 
> Amol Deshmukh
> 
>

Re: Review Request 45135: Descheduling a cron should not fail if the job is not scheduled.

[Stephan Erb]

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

(Updated March 22, 2016, 11:21 p.m.)


Review request for Aurora, Maxim Khutornenko and Zameer Manji.


Changes
---

Maxim's comment. Message remains at "Job %s is not scheduled with cron"


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


Repository: aurora


Description
---

This allows clients to be more declarative rather than imperative
when expressing that the given job should not be scheduled.


Diffs (updated)
-

  RELEASE-NOTES.md 6e9364e34db6dbb270468db3ff333b956c6bf9f3 
  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
d217faf44ab3d6132db3b3c4eed67effd03fb6fa 
  
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
 c774ac0f0e2fdda7fe9b64fd9181f107b3fd9eca 

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


Testing
---

./gradlew -Pq build


Thanks,

Stephan Erb

Re: Review Request 45182: Improve mname and structdump documentation

[Maxim Khutornenko]

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


Ship it!




Ship It!

- Maxim Khutornenko


On March 22, 2016, 11:33 p.m., Stephan Erb wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45182/
> ---
> 
> (Updated March 22, 2016, 11:33 p.m.)
> 
> 
> Review request for Aurora and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Improve mname and structdump documentation
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/http/Mname.java 
> d6e5fc4e886660311228d411cae1bbb42aaffb19 
>   src/main/java/org/apache/aurora/scheduler/http/StructDump.java 
> f84767a2c01bccc23182672123f9ca6701fcd696 
> 
> Diff: https://reviews.apache.org/r/45182/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build 
> 
> Verified rendering in Vagrant
> 
> 
> Thanks,
> 
> Stephan Erb
> 
>

Re: Review Request 45135: Descheduling a cron should not fail if the job is not scheduled.

[Aurora ReviewBot]

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


Ship it!




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

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

- Aurora ReviewBot


On March 22, 2016, 10:21 p.m., Stephan Erb wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45135/
> ---
> 
> (Updated March 22, 2016, 10:21 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Bugs: AURORA-1645
> https://issues.apache.org/jira/browse/AURORA-1645
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This allows clients to be more declarative rather than imperative
> when expressing that the given job should not be scheduled.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 6e9364e34db6dbb270468db3ff333b956c6bf9f3 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  d217faf44ab3d6132db3b3c4eed67effd03fb6fa 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  c774ac0f0e2fdda7fe9b64fd9181f107b3fd9eca 
> 
> Diff: https://reviews.apache.org/r/45135/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Stephan Erb
> 
>

Re: Review Request 45167: Fixup install docs.

[Stephan Erb]

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



Looks good! A few notes regarding related sections, even though not completely 
correleated with your change.


docs/installing.md (line 154)


That whole section seems outdate.



docs/installing.md (line 246)


Maybe it is more straight forward here to tell where that value is 
configured. Because if it is wrong, the user will have a hard time digging 
where and how our Thermos observer is configured.



docs/installing.md (line 278)


Wow, I am surprised. Is that actually working?



docs/installing.md (line 312)


I believe everything after the "-" is optional and does not need to be 
included.


- Stephan Erb


On March 22, 2016, 6:48 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45167/
> ---
> 
> (Updated March 22, 2016, 6:48 p.m.)
> 
> 
> Review request for Aurora, Benjamin Rice and Stephan Erb.
> 
> 
> Bugs: AURORA-1647
> https://issues.apache.org/jira/browse/AURORA-1647
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This set of fixes eliminates the experimental rpm warnings and updates
> the rpm instructions to use the officially released packages.  The deb
> instructions are updated as well, in particular to take advantage of the
> mesosphere deb repository and with movement of special dep installations
> to the appropriate sections requiring them.
> 
> This fix RB does not address the new Debian Jessie debs, instead
> focusing on getting the existing instructions corrected.
> 
>  docs/installing.md | 101 +++---
>  1 file changed, 55 insertions(+), 46 deletions(-)
> 
> 
> Diffs
> -
> 
>   docs/installing.md c3abb332a4a46e62367a39cf73d70f2185657b5a 
> 
> Diff: https://reviews.apache.org/r/45167/diff/
> 
> 
> Testing
> ---
> 
> These changes are rendered here:
>   
> https://github.com/jsirois/aurora/blob/jsirois/issues/AURORA-1647/docs/installing.md
> 
> I ran through the instructions in full in fresh Vagrant vms and was able
> to launch sample jobs (used aurora-packaging test jobs with cpu droppped to
> `0.5` and s/Service/Job/) and exercise the full UI chain to inspect the
> successful one-shot job and its sandbox & logs.
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 45167: Fixup install docs.

[John Sirois]

> On March 22, 2016, 3:26 p.m., Stephan Erb wrote:
> > Looks good! A few notes regarding related sections, even though not 
> > completely correleated with your change.

Changes forthcoming.


> On March 22, 2016, 3:26 p.m., Stephan Erb wrote:
> > docs/installing.md, line 330
> > 
> >
> > I believe everything after the "-" is optional and does not need to be 
> > included.

I tried and it needs at least  `sudo apt-get -y install mesos=0.25.0*`, which I 
was afraid of so I dropped in the `apt-cache` note. These both failed: 
`mesos=0.25.0` `mesos=0.25.0-0.2.70`


> On March 22, 2016, 3:26 p.m., Stephan Erb wrote:
> > docs/installing.md, line 294
> > 
> >
> > Wow, I am surprised. Is that actually working?

No clue - I run linux on my mac and stay well away from OSX!


> On March 22, 2016, 3:26 p.m., Stephan Erb wrote:
> > docs/installing.md, line 169
> > 
> >
> > That whole section seems outdate.

It is targeted so it is true, but I agree, there is no reason to promote 
installing really old versions!


> On March 22, 2016, 3:26 p.m., Stephan Erb wrote:
> > docs/installing.md, line 261
> > 
> >
> > Maybe it is more straight forward here to tell where that value is 
> > configured. Because if it is wrong, the user will have a hard time digging 
> > where and how our Thermos observer is configured.

Its different on each OS, but we already handle these differences so that makes 
sense to me.


- John


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


On March 22, 2016, 11:48 a.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45167/
> ---
> 
> (Updated March 22, 2016, 11:48 a.m.)
> 
> 
> Review request for Aurora, Benjamin Rice and Stephan Erb.
> 
> 
> Bugs: AURORA-1647
> https://issues.apache.org/jira/browse/AURORA-1647
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This set of fixes eliminates the experimental rpm warnings and updates
> the rpm instructions to use the officially released packages.  The deb
> instructions are updated as well, in particular to take advantage of the
> mesosphere deb repository and with movement of special dep installations
> to the appropriate sections requiring them.
> 
> This fix RB does not address the new Debian Jessie debs, instead
> focusing on getting the existing instructions corrected.
> 
>  docs/installing.md | 101 +++---
>  1 file changed, 55 insertions(+), 46 deletions(-)
> 
> 
> Diffs
> -
> 
>   docs/installing.md c3abb332a4a46e62367a39cf73d70f2185657b5a 
> 
> Diff: https://reviews.apache.org/r/45167/diff/
> 
> 
> Testing
> ---
> 
> These changes are rendered here:
>   
> https://github.com/jsirois/aurora/blob/jsirois/issues/AURORA-1647/docs/installing.md
> 
> I ran through the instructions in full in fresh Vagrant vms and was able
> to launch sample jobs (used aurora-packaging test jobs with cpu droppped to
> `0.5` and s/Service/Job/) and exercise the full UI chain to inspect the
> successful one-shot job and its sandbox & logs.
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 45115: Ensure final processes are executed when ephemeral daemon processes exist.

[Aurora ReviewBot]

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



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

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


21:50:31 04:00   [complete]
   FAILURE


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

- Aurora ReviewBot


On March 22, 2016, 9:04 p.m., Amol Deshmukh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45115/
> ---
> 
> (Updated March 22, 2016, 9:04 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Bugs: AURORA-1642
> https://issues.apache.org/jira/browse/AURORA-1642
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Ensure final processes are executed when ephemeral daemon processes exist.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/thermos/core/process.py 
> f147af7d0e84309691c135c8057a597379fa83e7 
>   src/test/python/apache/thermos/core/test_process.py 
> c339c91eb24616c8d640877ef088f659523d2bf5 
>   src/test/sh/org/apache/aurora/e2e/ephemeral_daemon_with_final.aurora 
> PRE-CREATION 
>   src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 
> b469f9bbbdfbf98df947832411bd0cdce97affdc 
> 
> Diff: https://reviews.apache.org/r/45115/diff/
> 
> 
> Testing
> ---
> 
> # End to end tests
> ```
> $ ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> ...
> *** OK (All tests passed) ***
> ```
> 
> # Python unit tests
> ```
> $ ./pants test src/test/python::
> ...
>   663 passed, 5 skipped, 1 warnings in 180.34 seconds
> 
> ```
> 
> # Also tested using a test job in vagrant to ensure that "final" processes 
> are executed as expected using this job definition:
> https://gist.github.com/adeshmukh/697d013dec64498a3942
> 
> 
> Thanks,
> 
> Amol Deshmukh
> 
>

Re: Review Request 44745: Allow for a pure docker executor.

[John Sirois]

On March 13, 2016, 6:04 a.m., John Sirois wrote:
> > While your patch is rather easy, I am not sure it is the best way to move 
> > forward. It feels like it is crossing streams with 
> > https://issues.apache.org/jira/browse/AURORA-1288. Putting some thought 
> > into this might be helpful in the long run.
> 
> Bill Farner wrote:
> FWIW i don't think it complicates or even diverges from that ticket.  In 
> my opinion it's yet to be seen whether it's feasible to use the same client 
> for a custom executor (at least, without a decent amount of modularization 
> work).  At the very least, that effort has lost momentum and we shouldn't 
> block progress for it.
> 
> Stephan Erb wrote:
> I mostly brought it up because the ticket also repeatedly mentions the 
> default Mesos command executor. Supporting this one does not sound to 
> different from supporting Docker without Thermos. It would also need similar 
> logic at the UI layer to allow use the Mesos sanbox UI instead of the Thermos 
> one.
> 
> I agree that we should not block progress here. I justed wanted to make 
> sure we are not rushing things (i.e., there isn't even a jira ticket right 
> now).
> 
> Maxim Khutornenko wrote:
> +1 to Stephan's concerns. The schema changes in this patch don't 
> necessarily convey enough meaning to paint a clear picture of where this 
> effort leads us. FWICT, nothing in the Task aside from resources is 
> applicable to the Docker case and it feels quite hacky to onboard a new 
> executor case this way.
> 
> > In my opinion it's yet to be seen whether it's feasible to use the same 
> client for a custom executor (at least, without a decent amount of 
> modularization work).
> 
> Bill, would you mind clarifying what this means? Are you expecting this 
> to be a purely experimental (POC) effort or is there a solid production 
> quality future here? If it's the former, would it be more appropriate to have 
> this effort baked in a private branch to avoid possibly unnecessary code 
> churn and review cycles?
> 
> Bill Farner wrote:
> | it feels quite hacky to onboard a new executor case this way
> 
> Suggestions solicited!  Just please don't forget that the intent is to 
> offer real, immediate value - the docker support in Aurora today is quite 
> crippled, and this will address the biggest shortcomings (entrypoints, and 
> zero required deps in images).
> 
> | FWICT, nothing in the Task aside from resources is applicable to the 
> Docker case
> 
> This is a good point.  Perhaps we should create a separate struct and 
> field in `Job` for this case?
> 
> | Bill, would you mind clarifying what this means?
> 
> What i mean is that the client, DSL, and executor all have relatively 
> high coupling.  Adding custom executor support in the client will require 
> non-trivial effort to break that coupling.  I would like to avoid blocking 
> this feature on that goal.
> 
> | Are you expecting this to be a purely experimental (POC) effort or is 
> there a solid production quality future here?
> 
> That is very much dependent on the underlying support in mesos.  Today, i 
> see it as the best support for docker containers in mesos.  It's been 
> available for some time, and the work here is entirely plumbing to enable it 
> in Aurora.
> 
> | would it be more appropriate to have this effort baked in a private 
> branch to avoid possibly unnecessary code churn and review cycles?
> 
> I don't foresee enough churn to warrant that.
> 
> John Sirois wrote:
> Noting that I'm backing off this change until sentiment settles one way 
> or the other.  If it settles in-favor I'll address both Stephan & Joshua's 
> feedback at that point.
> 
> Stephan Erb wrote:
> I have to backoff out of the discussion here, as I don't have the 
> necessary cycles to participate. A couple of closing notes from my side:
> 
> * I agree with Maxim that giving an empty process list a special meaning 
> feels kind of like a hack.
> * I probably wouldn't have complained about this if it was that way from 
> the beginning...
> * Docker support is still considered experimental, so no decision is cast 
> in stone. We can change stuff without to much hassle.
> * It is great that you are improving the current docker support (even 
> though I am a fanboy of the upcoming unified container support :-)
> 
> Maxim Khutornenko wrote:
> Thanks for explaning Bill. I am fine continuing this effort given the 
> above.
> 
> > This is a good point.  Perhaps we should create a separate struct and 
> field in Job for this case?
> 
> I don't have bandwidth to think about the alternatives at the moment but 
> your suggestion about plugging it higher in the chain (e.g. Job struct) 
> sounds logical.
> 
> Joshua Cohen wrote:
> Could we just add name and resources to the Container struct (if we even 
> need name? I think it's 

Re: Review Request 45179: Support for overriding --mesos-root under upstart.

[Stephan Erb]

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


Ship it!




Ship It!

- Stephan Erb


On March 22, 2016, 11:49 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45179/
> ---
> 
> (Updated March 22, 2016, 11:49 p.m.)
> 
> 
> Review request for Aurora and Stephan Erb.
> 
> 
> Bugs: AURORA-1647
> https://issues.apache.org/jira/browse/AURORA-1647
> 
> 
> Repository: aurora-packaging
> 
> 
> Description
> ---
> 
> Previously only the sysv init script supported overriding `--mesos-root`
> via an env var, and thus via `/etc/default/thermos`.
> 
>  specs/debian/aurora-executor.thermos.upstart | 1 +
>  1 file changed, 1 insertion(+)
> 
> 
> Diffs
> -
> 
>   specs/debian/aurora-executor.thermos.upstart 
> 7ebe4bea4debc9f558fae92c4d532145156ec43e 
> 
> Diff: https://reviews.apache.org/r/45179/diff/
> 
> 
> Testing
> ---
> 
> Used this edit in testing done for https://reviews.apache.org/r/45167
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 45115: Ensure final processes are executed when ephemeral daemon processes exist.

[Amol Deshmukh]

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

(Updated March 22, 2016, 3:04 p.m.)


Review request for Aurora, Maxim Khutornenko and Zameer Manji.


Changes
---

- Also applied the fix in case LoggerDestination is CONSOLE (since 
sys.stdout/sys.stderr are file objects).
- Implemented maxim's review comments.


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


Repository: aurora


Description
---

Ensure final processes are executed when ephemeral daemon processes exist.


Diffs (updated)
-

  src/main/python/apache/thermos/core/process.py 
f147af7d0e84309691c135c8057a597379fa83e7 
  src/test/python/apache/thermos/core/test_process.py 
c339c91eb24616c8d640877ef088f659523d2bf5 
  src/test/sh/org/apache/aurora/e2e/ephemeral_daemon_with_final.aurora 
PRE-CREATION 
  src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 
b469f9bbbdfbf98df947832411bd0cdce97affdc 

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


Testing
---

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

# Python unit tests
```
$ ./pants test src/test/python::
...
  663 passed, 5 skipped, 1 warnings in 180.34 seconds

```

# Also tested using a test job in vagrant to ensure that "final" processes are 
executed as expected using this job definition:
https://gist.github.com/adeshmukh/697d013dec64498a3942


Thanks,

Amol Deshmukh

Re: Review Request 45115: Ensure final processes are executed when ephemeral daemon processes exist.

[Amol Deshmukh]

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

(Updated March 22, 2016, 3:35 p.m.)


Review request for Aurora, Maxim Khutornenko and Zameer Manji.


Changes
---

Updated ``test_resolver_console_output``.
@ReviewBot retry


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


Repository: aurora


Description
---

Ensure final processes are executed when ephemeral daemon processes exist.


Diffs (updated)
-

  src/main/python/apache/thermos/core/process.py 
f147af7d0e84309691c135c8057a597379fa83e7 
  src/test/python/apache/thermos/core/test_process.py 
c339c91eb24616c8d640877ef088f659523d2bf5 
  src/test/sh/org/apache/aurora/e2e/ephemeral_daemon_with_final.aurora 
PRE-CREATION 
  src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 
b469f9bbbdfbf98df947832411bd0cdce97affdc 

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


Testing
---

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

# Python unit tests
```
$ ./pants test src/test/python::
...
  663 passed, 5 skipped, 1 warnings in 180.34 seconds

```

# Also tested using a test job in vagrant to ensure that "final" processes are 
executed as expected using this job definition:
https://gist.github.com/adeshmukh/697d013dec64498a3942


Thanks,

Amol Deshmukh

Review Request 45182: Improve mname and structdump documentation

[Stephan Erb]

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

Review request for Aurora and Maxim Khutornenko.


Repository: aurora


Description
---

Improve mname and structdump documentation


Diffs
-

  src/main/java/org/apache/aurora/scheduler/http/Mname.java 
d6e5fc4e886660311228d411cae1bbb42aaffb19 
  src/main/java/org/apache/aurora/scheduler/http/StructDump.java 
f84767a2c01bccc23182672123f9ca6701fcd696 

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


Testing
---

./gradlew -Pq build 

Verified rendering in Vagrant


Thanks,

Stephan Erb

Re: Review Request 45182: Improve mname and structdump documentation

[Stephan Erb]

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

(Updated March 23, 2016, 12:33 a.m.)


Review request for Aurora and Maxim Khutornenko.


Repository: aurora


Description
---

Improve mname and structdump documentation


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/http/Mname.java 
d6e5fc4e886660311228d411cae1bbb42aaffb19 
  src/main/java/org/apache/aurora/scheduler/http/StructDump.java 
f84767a2c01bccc23182672123f9ca6701fcd696 

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


Testing
---

./gradlew -Pq build 

Verified rendering in Vagrant


Thanks,

Stephan Erb

Re: Review Request 45182: Improve mname and structdump documentation

[Aurora ReviewBot]

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



Master (b5c9e1b) 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 March 22, 2016, 11:09 p.m., Stephan Erb wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45182/
> ---
> 
> (Updated March 22, 2016, 11:09 p.m.)
> 
> 
> Review request for Aurora and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Improve mname and structdump documentation
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/http/Mname.java 
> d6e5fc4e886660311228d411cae1bbb42aaffb19 
>   src/main/java/org/apache/aurora/scheduler/http/StructDump.java 
> f84767a2c01bccc23182672123f9ca6701fcd696 
> 
> Diff: https://reviews.apache.org/r/45182/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build 
> 
> Verified rendering in Vagrant
> 
> 
> Thanks,
> 
> Stephan Erb
> 
>

Re: Review Request 45115: Ensure final processes are executed when ephemeral daemon processes exist.

[Maxim Khutornenko]

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



@ReviewBot retry

- Maxim Khutornenko


On March 22, 2016, 10:35 p.m., Amol Deshmukh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45115/
> ---
> 
> (Updated March 22, 2016, 10:35 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Bugs: AURORA-1642
> https://issues.apache.org/jira/browse/AURORA-1642
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Ensure final processes are executed when ephemeral daemon processes exist.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/thermos/core/process.py 
> f147af7d0e84309691c135c8057a597379fa83e7 
>   src/test/python/apache/thermos/core/test_process.py 
> c339c91eb24616c8d640877ef088f659523d2bf5 
>   src/test/sh/org/apache/aurora/e2e/ephemeral_daemon_with_final.aurora 
> PRE-CREATION 
>   src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 
> b469f9bbbdfbf98df947832411bd0cdce97affdc 
> 
> Diff: https://reviews.apache.org/r/45115/diff/
> 
> 
> Testing
> ---
> 
> # End to end tests
> ```
> $ ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> ...
> *** OK (All tests passed) ***
> ```
> 
> # Python unit tests
> ```
> $ ./pants test src/test/python::
> ...
>   663 passed, 5 skipped, 1 warnings in 180.34 seconds
> 
> ```
> 
> # Also tested using a test job in vagrant to ensure that "final" processes 
> are executed as expected using this job definition:
> https://gist.github.com/adeshmukh/697d013dec64498a3942
> 
> 
> Thanks,
> 
> Amol Deshmukh
> 
>

Re: Review Request 45167: Fixup install docs.

[John Sirois]

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

(Updated March 22, 2016, 4:40 p.m.)


Review request for Aurora, Benjamin Rice and Stephan Erb.


Changes
---

Provide explicit advice for adjusting `--mesos-root`.

 docs/installing.md | 40 +---
 1 file changed, 29 insertions(+), 11 deletions(-)


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


Repository: aurora


Description
---

This set of fixes eliminates the experimental rpm warnings and updates
the rpm instructions to use the officially released packages.  The deb
instructions are updated as well, in particular to take advantage of the
mesosphere deb repository and with movement of special dep installations
to the appropriate sections requiring them.

This fix RB does not address the new Debian Jessie debs, instead
focusing on getting the existing instructions corrected.

 docs/installing.md | 101 +++---
 1 file changed, 55 insertions(+), 46 deletions(-)


Diffs (updated)
-

  docs/installing.md c3abb332a4a46e62367a39cf73d70f2185657b5a 

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


Testing
---

These changes are rendered here:
  
https://github.com/jsirois/aurora/blob/jsirois/issues/AURORA-1647/docs/installing.md

I ran through the instructions in full in fresh Vagrant vms and was able
to launch sample jobs (used aurora-packaging test jobs with cpu droppped to
`0.5` and s/Service/Job/) and exercise the full UI chain to inspect the
successful one-shot job and its sandbox & logs.


Thanks,

John Sirois

Re: Review Request 45135: Descheduling a cron should not fail if the job is not scheduled.

[Maxim Khutornenko]

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


Ship it!




Ship It!

- Maxim Khutornenko


On March 22, 2016, 10:21 p.m., Stephan Erb wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45135/
> ---
> 
> (Updated March 22, 2016, 10:21 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Bugs: AURORA-1645
> https://issues.apache.org/jira/browse/AURORA-1645
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This allows clients to be more declarative rather than imperative
> when expressing that the given job should not be scheduled.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 6e9364e34db6dbb270468db3ff333b956c6bf9f3 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  d217faf44ab3d6132db3b3c4eed67effd03fb6fa 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  c774ac0f0e2fdda7fe9b64fd9181f107b3fd9eca 
> 
> Diff: https://reviews.apache.org/r/45135/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Stephan Erb
> 
>

Re: Review Request 45182: Improve mname and structdump documentation

[Stephan Erb]

> On March 23, 2016, 12:12 a.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/http/Mname.java, line 62
> > 
> >
> > ?

Oh no, what whas I thinking :-)


- Stephan


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


On March 23, 2016, 12:09 a.m., Stephan Erb wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45182/
> ---
> 
> (Updated March 23, 2016, 12:09 a.m.)
> 
> 
> Review request for Aurora and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Improve mname and structdump documentation
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/http/Mname.java 
> d6e5fc4e886660311228d411cae1bbb42aaffb19 
>   src/main/java/org/apache/aurora/scheduler/http/StructDump.java 
> f84767a2c01bccc23182672123f9ca6701fcd696 
> 
> Diff: https://reviews.apache.org/r/45182/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build 
> 
> Verified rendering in Vagrant
> 
> 
> Thanks,
> 
> Stephan Erb
> 
>

Re: Review Request 45167: Fixup install docs.

[Aurora ReviewBot]

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


Ship it!




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

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

- Aurora ReviewBot


On March 22, 2016, 10:40 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45167/
> ---
> 
> (Updated March 22, 2016, 10:40 p.m.)
> 
> 
> Review request for Aurora, Benjamin Rice and Stephan Erb.
> 
> 
> Bugs: AURORA-1647
> https://issues.apache.org/jira/browse/AURORA-1647
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This set of fixes eliminates the experimental rpm warnings and updates
> the rpm instructions to use the officially released packages.  The deb
> instructions are updated as well, in particular to take advantage of the
> mesosphere deb repository and with movement of special dep installations
> to the appropriate sections requiring them.
> 
> This fix RB does not address the new Debian Jessie debs, instead
> focusing on getting the existing instructions corrected.
> 
>  docs/installing.md | 101 +++---
>  1 file changed, 55 insertions(+), 46 deletions(-)
> 
> 
> Diffs
> -
> 
>   docs/installing.md c3abb332a4a46e62367a39cf73d70f2185657b5a 
> 
> Diff: https://reviews.apache.org/r/45167/diff/
> 
> 
> Testing
> ---
> 
> These changes are rendered here:
>   
> https://github.com/jsirois/aurora/blob/jsirois/issues/AURORA-1647/docs/installing.md
> 
> I ran through the instructions in full in fresh Vagrant vms and was able
> to launch sample jobs (used aurora-packaging test jobs with cpu droppped to
> `0.5` and s/Service/Job/) and exercise the full UI chain to inspect the
> successful one-shot job and its sandbox & logs.
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 45115: Ensure final processes are executed when ephemeral daemon processes exist.

[Aurora ReviewBot]

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


Ship it!




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

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

- Aurora ReviewBot


On March 22, 2016, 10:35 p.m., Amol Deshmukh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45115/
> ---
> 
> (Updated March 22, 2016, 10:35 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Bugs: AURORA-1642
> https://issues.apache.org/jira/browse/AURORA-1642
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Ensure final processes are executed when ephemeral daemon processes exist.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/thermos/core/process.py 
> f147af7d0e84309691c135c8057a597379fa83e7 
>   src/test/python/apache/thermos/core/test_process.py 
> c339c91eb24616c8d640877ef088f659523d2bf5 
>   src/test/sh/org/apache/aurora/e2e/ephemeral_daemon_with_final.aurora 
> PRE-CREATION 
>   src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 
> b469f9bbbdfbf98df947832411bd0cdce97affdc 
> 
> Diff: https://reviews.apache.org/r/45115/diff/
> 
> 
> Testing
> ---
> 
> # End to end tests
> ```
> $ ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> ...
> *** OK (All tests passed) ***
> ```
> 
> # Python unit tests
> ```
> $ ./pants test src/test/python::
> ...
>   663 passed, 5 skipped, 1 warnings in 180.34 seconds
> 
> ```
> 
> # Also tested using a test job in vagrant to ensure that "final" processes 
> are executed as expected using this job definition:
> https://gist.github.com/adeshmukh/697d013dec64498a3942
> 
> 
> Thanks,
> 
> Amol Deshmukh
> 
>

Review Request 45193: Treat empty and null collections equivalently in task queries.

[John Sirois]

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

Review request for Aurora, David Chung, Bill Farner, and Zameer Manji.


Repository: aurora


Description
---

Previously, `null` was handled differently from an empty collection in
task queries. For the Go thrift bindings, this was problematic since
zero types in Go are useful in almost all cases and in particular in the
case of slices (collections).  In these cases unset `TaskQuery`
collection parameters are represented as empty collections (empty
slices[]) instead of `nil` (`null`), leading to the inability to use the
query API in any natural way.

 src/main/java/org/apache/aurora/scheduler/base/JobKeys.java  | 
 2 +-
 src/main/java/org/apache/aurora/scheduler/base/Query.java| 
 2 +-
 src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java | 
 2 +-
 src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java  | 
 6 --
 src/main/resources/org/apache/aurora/scheduler/storage/db/TaskMapper.xml | 
 6 +++---
 src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java | 
20 +---
 6 files changed, 27 insertions(+), 11 deletions(-)


Diffs
-

  src/main/java/org/apache/aurora/scheduler/base/JobKeys.java 
8f5bf58b963ae5f76aad7dfa34bae5b9e67d6242 
  src/main/java/org/apache/aurora/scheduler/base/Query.java 
ee01eaa4d0230d6bf0909b6460f27a74f03240db 
  src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java 
ac0bb374842741d7ccb7a83c574a90ac156af0f9 
  src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 
231a55615abfbb483667f5f8ef71d2709fc16a88 
  src/main/resources/org/apache/aurora/scheduler/storage/db/TaskMapper.xml 
684614ffc42dd6778c7675a6c2f81cb72c106c0e 
  src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
e56fed2e6c0cdb47737cf1a9b637c44c5e5b9815 

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


Testing
---

NB: This change was broken out of https://reviews.apache.org/r/42756/
since it stands on its own (although its slightly more awkward in the
mutable thrift world) and the case of the Go Aurora API client forces the
issue.

Locally green:
```
./build-support/jenkins/build.sh
./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
```


Thanks,

John Sirois

Re: Review Request 45193: Treat empty and null collections equivalently in task queries.

[John Sirois]

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

(Updated March 22, 2016, 8:32 p.m.)


Review request for Aurora, David Chung, Bill Farner, and Zameer Manji.


Repository: aurora


Description (updated)
---

Previously, `null` was handled differently from an empty collection in
task queries. For the Go thrift bindings, this was problematic since
zero values in Go are useful in almost all cases and in particular in the
case of slices (collections).  In these cases unset `TaskQuery`
collection parameters are represented as empty collections (empty
slices[]) instead of `nil` (`null`), leading to the inability to use the
query API in any natural way.

 src/main/java/org/apache/aurora/scheduler/base/JobKeys.java  | 
 2 +-
 src/main/java/org/apache/aurora/scheduler/base/Query.java| 
 2 +-
 src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java | 
 2 +-
 src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java  | 
 6 --
 src/main/resources/org/apache/aurora/scheduler/storage/db/TaskMapper.xml | 
 6 +++---
 src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java | 
20 +---
 6 files changed, 27 insertions(+), 11 deletions(-)


Diffs
-

  src/main/java/org/apache/aurora/scheduler/base/JobKeys.java 
8f5bf58b963ae5f76aad7dfa34bae5b9e67d6242 
  src/main/java/org/apache/aurora/scheduler/base/Query.java 
ee01eaa4d0230d6bf0909b6460f27a74f03240db 
  src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java 
ac0bb374842741d7ccb7a83c574a90ac156af0f9 
  src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 
231a55615abfbb483667f5f8ef71d2709fc16a88 
  src/main/resources/org/apache/aurora/scheduler/storage/db/TaskMapper.xml 
684614ffc42dd6778c7675a6c2f81cb72c106c0e 
  src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
e56fed2e6c0cdb47737cf1a9b637c44c5e5b9815 

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


Testing
---

NB: This change was broken out of https://reviews.apache.org/r/42756/
since it stands on its own (although its slightly more awkward in the
mutable thrift world) and the case of the Go Aurora API client forces the
issue.

Locally green:
```
./build-support/jenkins/build.sh
./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
```


Thanks,

John Sirois

Re: Review Request 45042: Add ACL support for announcer

[Bill Farner]

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



Thanks for the changes, this is looking great!  My most pressing concern is 
related to not also passing `auth_data`.  Based on the outcome of that topic, 
we may want to change the JSON schema to allow specification of that as well.

Thinking on this more, it would be great for the project and for the 
longetivity of your feature to enable it in end-to-end tests.  (Really, all i 
mean is to configure the stock vagrant environment to use it.)  Happy to help 
on the dev list and/or IRC if it's not clear how to do this.

Meta-review request: please make use of reviewboard's comment feature to 
ack/nack/discuss review comments.  Especially when there's a bunch of comments, 
it can be difficult as a reviewer to keep track of what action was taken on 
each item, if any.


docs/security.md (line 289)


I'd like to propose several changes to this section, which i've made in the 
rewritten block below.

- Use consistent naming and proper nouns for projects (Thermos, ZooKeeper)
- Link to latest version of ZooKeeper docs
- Immediately link to relevant ZooKeeper ACL section
- Describe how to enable the feature before describing the format of the 
ACL file
- Use more accurate requirements level terminology, e.g. must/may/should 
(context reading http://tools.ietf.org/html/rfc2119)

```
# Announcer Authentication
Nodes created by the Thermos executor may include ZooKeeper

[ACLs](https://zookeeper.apache.org/doc/current/zookeeperProgrammers.html#sc_ZooKeeperAccessControl),
which will specify the priviliges of clients to perform different actions 
on these nodes.  This
feature is enabled by specifying an ACL configuration file to the executor 
with the
`--announcer-zookeeper-auth-config` command line argument.

When this feature is _not_ enabled, nodes created by the executor will have 
'world/all' permission
(`ZOO_OPEN_ACL_UNSAFE`).  In most production environments, operators should 
specify ACLs and
limit access.

## ACL configuration format
The configuration file must be formatted as JSON with the following schema:

```json
[
  {
"scheme": "",
"credential": "",
"permissions": {
  "read": ,
  "write": ,
  "create": ,
  "delete": ,
  "admin": ,
  "all": 
}
  }
]
```

The 
[scheme](http://zookeeper.apache.org/doc/r3.1.2/zookeeperProgrammers.html#sc_BuiltinACLSchemes)
defines the encoding of the `credential` field.  Note that these fields are 
passed directly to
ZoooKeeper.  If a scheme is used that requires credential hashing, the 
value of the `credential`
field must be hashed (i.e. the executor will not hash this value).

All properties of the `permissions` object will default to `False` if not 
provided.
```



src/main/python/apache/aurora/executor/bin/thermos_executor_main.py (line 107)


Sorry for not catching this earlier - how about s/auth/acl/?  I think this 
is more specific to what the file defines.



src/main/python/apache/aurora/executor/bin/thermos_executor_main.py (line 111)


Is this more accurate? `Path to ZooKeeper ACL to use for announcer nodes.`



src/main/python/apache/aurora/executor/common/announcer.py (line 84)


I think we should also require that at least one field is specified in 
`permissions`.  Seems like the ACL entry would be meaningliess otherwise.



src/main/python/apache/aurora/executor/common/announcer.py (lines 95 - 96)


I suggest you just have `validate` raise for invalid and catch here, rather 
than returning a `bool`.  The sequence here creates two separate log entries 
that could just as easily be 1, e.g.

```
Expecting a list of ACL objects
ZK authentication config is invalid
```



src/main/python/apache/aurora/executor/common/announcer.py (lines 186 - 187)


Feeling some deja vu here - the double underscore is unconventional - 
please change to single underscore.



src/main/python/apache/aurora/executor/common/announcer.py (line 192)


I just realized something - does it make much sense to specify 
`default_acl` without also specifying `auth_data`?

```
:param auth_data:
  A list of authentication credentials to use for the
  connection. Should be a list of (scheme, credential)
  tuples as 

Re: Review Request 45193: Treat empty and null collections equivalently in task queries.

[Bill Farner]

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


Ship it!




Ship It!

- Bill Farner


On March 22, 2016, 7:32 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45193/
> ---
> 
> (Updated March 22, 2016, 7:32 p.m.)
> 
> 
> Review request for Aurora, David Chung, Bill Farner, and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Previously, `null` was handled differently from an empty collection in
> task queries. For the Go thrift bindings, this was problematic since
> zero values in Go are useful in almost all cases and in particular in the
> case of slices (collections).  In these cases unset `TaskQuery`
> collection parameters are represented as empty collections (empty
> slices[]) instead of `nil` (`null`), leading to the inability to use the
> query API in any natural way.
> 
>  src/main/java/org/apache/aurora/scheduler/base/JobKeys.java  
> |  2 +-
>  src/main/java/org/apache/aurora/scheduler/base/Query.java
> |  2 +-
>  src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java 
> |  2 +-
>  src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java  
> |  6 --
>  src/main/resources/org/apache/aurora/scheduler/storage/db/TaskMapper.xml 
> |  6 +++---
>  src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
> | 20 +---
>  6 files changed, 27 insertions(+), 11 deletions(-)
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/base/JobKeys.java 
> 8f5bf58b963ae5f76aad7dfa34bae5b9e67d6242 
>   src/main/java/org/apache/aurora/scheduler/base/Query.java 
> ee01eaa4d0230d6bf0909b6460f27a74f03240db 
>   src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java 
> ac0bb374842741d7ccb7a83c574a90ac156af0f9 
>   src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 
> 231a55615abfbb483667f5f8ef71d2709fc16a88 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/TaskMapper.xml 
> 684614ffc42dd6778c7675a6c2f81cb72c106c0e 
>   
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
> e56fed2e6c0cdb47737cf1a9b637c44c5e5b9815 
> 
> Diff: https://reviews.apache.org/r/45193/diff/
> 
> 
> Testing
> ---
> 
> NB: This change was broken out of https://reviews.apache.org/r/42756/
> since it stands on its own (although its slightly more awkward in the
> mutable thrift world) and the case of the Go Aurora API client forces the
> issue.
> 
> Locally green:
> ```
> ./build-support/jenkins/build.sh
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> ```
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 45042: Add ACL support for announcer

[Bill Farner]

> On March 22, 2016, 7:23 p.m., Bill Farner wrote:
> > docs/security.md, line 289
> > 
> >
> > I'd like to propose several changes to this section, which i've made in 
> > the rewritten block below.
> > 
> > - Use consistent naming and proper nouns for projects (Thermos, 
> > ZooKeeper)
> > - Link to latest version of ZooKeeper docs
> > - Immediately link to relevant ZooKeeper ACL section
> > - Describe how to enable the feature before describing the format of 
> > the ACL file
> > - Use more accurate requirements level terminology, e.g. 
> > must/may/should (context reading http://tools.ietf.org/html/rfc2119)
> > 
> > ```
> > # Announcer Authentication
> > Nodes created by the Thermos executor may include ZooKeeper
> > 
> > [ACLs](https://zookeeper.apache.org/doc/current/zookeeperProgrammers.html#sc_ZooKeeperAccessControl),
> > which will specify the priviliges of clients to perform different 
> > actions on these nodes.  This
> > feature is enabled by specifying an ACL configuration file to the 
> > executor with the
> > `--announcer-zookeeper-auth-config` command line argument.
> > 
> > When this feature is _not_ enabled, nodes created by the executor will 
> > have 'world/all' permission
> > (`ZOO_OPEN_ACL_UNSAFE`).  In most production environments, operators 
> > should specify ACLs and
> > limit access.
> > 
> > ## ACL configuration format
> > The configuration file must be formatted as JSON with the following 
> > schema:
> > 
> > ```json
> > [
> >   {
> > "scheme": "",
> > "credential": "",
> > "permissions": {
> >   "read": ,
> >   "write": ,
> >   "create": ,
> >   "delete": ,
> >   "admin": ,
> >   "all": 
> > }
> >   }
> > ]
> > ```
> > 
> > The 
> > [scheme](http://zookeeper.apache.org/doc/r3.1.2/zookeeperProgrammers.html#sc_BuiltinACLSchemes)
> > defines the encoding of the `credential` field.  Note that these fields 
> > are passed directly to
> > ZoooKeeper.  If a scheme is used that requires credential hashing, the 
> > value of the `credential`
> > field must be hashed (i.e. the executor will not hash this value).
> > 
> > All properties of the `permissions` object will default to `False` if 
> > not provided.
> > ```

Formatting was broken above due to nested preformatted text, but it should be 
relatively close to being copy/paste-able.


- Bill


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


On March 22, 2016, 11:51 a.m., Kunal Thakar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45042/
> ---
> 
> (Updated March 22, 2016, 11:51 a.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add ACL support for announcer
> https://issues.apache.org/jira/browse/AURORA-1643
> 
> Adding support for service discovery ZK authentication. ZK authentication 
> secrets should be stored in a file as json (as follows):
> ```json
> {
>   "scheme": "",
>   "credential": "",
>   "permissions": {
> "read": ,
> "write": ,
> "create": ,
> "delete": ,
> "admin": ,
> "all": 
>   }
> }
> ```
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 6e9364e34db6dbb270468db3ff333b956c6bf9f3 
>   docs/security.md 32bea426fbceec83187e851a5db11e82df55e962 
>   src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
> 6634506108c346f8c23b2da7cc8d20d09d07d590 
>   src/main/python/apache/aurora/executor/common/announcer.py 
> 79a9cfb6ac3a8444f09fb3658e6e859e06941ba4 
>   
> src/test/python/apache/aurora/executor/bin/test_thermos_executor_entry_point.py
>  e9f7851292aef3a36da5da9b0fc333a7e7750cf3 
>   src/test/python/apache/aurora/executor/common/test_announcer.py 
> 142b58d5e577c9f4b8e2ae8473cffdea94eba21f 
> 
> Diff: https://reviews.apache.org/r/45042/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kunal Thakar
> 
>

Re: Review Request 45172: Tweak update-sources script to also update mesos config.

[Bill Farner]

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




examples/vagrant/provision-dev-cluster.sh (lines 89 - 92)


Echoing IRC discussion: i'm -1 to this change because it copies sources but 
does not effect the change by restarting components.  You rightly point out 
that it's no different from the copy we do for the upstart configs.

My vote is we do one of the following:
1. move the upstart config copy out of `update-sources` and do it only at 
provision time
2. create a helper function that accepts 3 args: `src`, `dest`, 
`on_change_cmd`, which will do an `rsync` and invoke `on_change_cmd` if any 
files were changed.  use this to restart components that were reconfigured

I think there's not much effort in (2), but it's objectively less work than 
(1).  What do you think?


- Bill Farner


On March 22, 2016, 11:53 a.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45172/
> ---
> 
> (Updated March 22, 2016, 11:53 a.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Tweak update-sources script to also update mesos config.
> 
> 
> Diffs
> -
> 
>   examples/vagrant/provision-dev-cluster.sh 
> b1f661ea90ac427621518f653cce1b7630d2a6ed 
> 
> Diff: https://reviews.apache.org/r/45172/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>