Re: Review Request 46596: Command line flag to change FrameworkInfo.name

2016-04-28 Thread Stephan Erb

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



@ReviewBot retry

- Stephan Erb


On April 28, 2016, 12:44 a.m., Stephan Erb wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46596/
> ---
> 
> (Updated April 28, 2016, 12:44 a.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Bugs: AURORA-945
> https://issues.apache.org/jira/browse/AURORA-945
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This commit introduces a new command line flag `-framework_name` and changes 
> the value from the hardcoded 'TwitterScheduler' to 'aurora'.
> 
> This change is non-intrusive as 
> https://issues.apache.org/jira/browse/MESOS-2614 has been fixed. The new 
> value will be shown on the UI and used for Mesos DNS. I have skipped a 
> deprecation cycle for this change, as if an operator wants to maintain the 
> old value, he can simply use the new option when deploying the next Aurora 
> version.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 7a37d0d69f688bece624628fe5b98efc85d506a2 
>   docs/features/service-discovery.md f242730fc16103903167182372ff6903127d73f5 
>   docs/reference/scheduler-configuration.md 
> d2262f79edfde23eccd87bae7f1cf319b63b1103 
>   
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java
>  5c6cdd2b4c7ee92eb7d8e1649a95cad3035efec0 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java
>  dc964b8dac1b8e21e86ad331216c57fa11abeb6d 
> 
> Diff: https://reviews.apache.org/r/46596/diff/
> 
> 
> Testing
> ---
> 
> * ./gradlew -Pq build
> * Changed the framework name in vagrant. Aurora happily re-registered without 
> a fallout.
> 
> 
> Thanks,
> 
> Stephan Erb
> 
>



Re: Review Request 46786: Add missing Mesos dependency to packer build script.

2016-04-28 Thread Joshua Cohen


> On April 28, 2016, 3:54 p.m., Maxim Khutornenko wrote:
> > build-support/packer/build.sh, line 37
> > 
> >
> > What about the `java2-runtime-headless` proposed in that pull request? 
> > Did you find it redundant?

I found that java2-runtime-headless did not work at all:

...
virtualbox-ovf: Package java2-runtime-headless is a virtual package 
provided by:
virtualbox-ovf: openjdk-8-jre-headless 8u91-b14-0ubuntu4~14.04
virtualbox-ovf: openjdk-6-jre-headless 6b38-1.13.10-0ubuntu0.14.04.1
virtualbox-ovf: openjdk-7-jre-headless 7u95-2.6.4-0ubuntu0.14.04.2
virtualbox-ovf: gcj-4.8-jre-headless 4.8.4-2ubuntu1~14.04.1
virtualbox-ovf: gcj-jre-headless 4:4.8.2-1ubuntu6
virtualbox-ovf: default-jre-headless 2:1.7-51
virtualbox-ovf:
virtualbox-ovf: E: Package 'java2-runtime-headless' has no installation 
candidate
==> virtualbox-ovf: Unregistering and deleting imported VM...
==> virtualbox-ovf: Deleting output directory...
Build 'virtualbox-ovf' errored: Script exited with non-zero exit status: 100

But perhaps I missed something?


- Joshua


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


On April 28, 2016, 3:48 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46786/
> ---
> 
> (Updated April 28, 2016, 3:48 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> See http://markmail.org/message/rofuiaclgkesfx3o for full details.
> 
> 
> Diffs
> -
> 
>   build-support/packer/build.sh 658dbc4bc5ae0dd1cfa69abd8fdf6119c3ccf8c9 
> 
> Diff: https://reviews.apache.org/r/46786/diff/
> 
> 
> Testing
> ---
> 
> Successfully built base box with these changes.
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>



Re: Review Request 46786: Add missing Mesos dependency to packer build script.

2016-04-28 Thread John Sirois

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




build-support/packer/build.sh (line 37)


Try default-jre-headless?: 
https://launchpad.net/ubuntu/trusty/+package/default-jre-headless



build-support/packer/build.sh (line 44)


I'm missing how this is needed too.


- John Sirois


On April 28, 2016, 9:48 a.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46786/
> ---
> 
> (Updated April 28, 2016, 9:48 a.m.)
> 
> 
> Review request for Aurora, John Sirois and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> See http://markmail.org/message/rofuiaclgkesfx3o for full details.
> 
> 
> Diffs
> -
> 
>   build-support/packer/build.sh 658dbc4bc5ae0dd1cfa69abd8fdf6119c3ccf8c9 
> 
> Diff: https://reviews.apache.org/r/46786/diff/
> 
> 
> Testing
> ---
> 
> Successfully built base box with these changes.
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>



Re: Review Request 46786: Add missing Mesos dependency to packer build script.

2016-04-28 Thread Stephan Erb

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


Ship it!




Ship It!

- Stephan Erb


On April 28, 2016, 5:48 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46786/
> ---
> 
> (Updated April 28, 2016, 5:48 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> See http://markmail.org/message/rofuiaclgkesfx3o for full details.
> 
> 
> Diffs
> -
> 
>   build-support/packer/build.sh 658dbc4bc5ae0dd1cfa69abd8fdf6119c3ccf8c9 
> 
> Diff: https://reviews.apache.org/r/46786/diff/
> 
> 
> Testing
> ---
> 
> Successfully built base box with these changes.
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>



Re: Review Request 46786: Add missing Mesos dependency to packer build script.

2016-04-28 Thread Aurora ReviewBot

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


Ship it!




Master (e817eb1) 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 April 28, 2016, 3:48 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46786/
> ---
> 
> (Updated April 28, 2016, 3:48 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> See http://markmail.org/message/rofuiaclgkesfx3o for full details.
> 
> 
> Diffs
> -
> 
>   build-support/packer/build.sh 658dbc4bc5ae0dd1cfa69abd8fdf6119c3ccf8c9 
> 
> Diff: https://reviews.apache.org/r/46786/diff/
> 
> 
> Testing
> ---
> 
> Successfully built base box with these changes.
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>



Review Request 46786: Add missing Mesos dependency to packer build script.

2016-04-28 Thread Joshua Cohen

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

Review request for Aurora, John Sirois and Stephan Erb.


Repository: aurora


Description
---

See http://markmail.org/message/rofuiaclgkesfx3o for full details.


Diffs
-

  build-support/packer/build.sh 658dbc4bc5ae0dd1cfa69abd8fdf6119c3ccf8c9 

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


Testing
---

Successfully built base box with these changes.


Thanks,

Joshua Cohen



Re: Review Request 46786: Add missing Mesos dependency to packer build script.

2016-04-28 Thread Maxim Khutornenko

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




build-support/packer/build.sh (line 37)


What about the `java2-runtime-headless` proposed in that pull request? Did 
you find it redundant?


- Maxim Khutornenko


On April 28, 2016, 3:48 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46786/
> ---
> 
> (Updated April 28, 2016, 3:48 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> See http://markmail.org/message/rofuiaclgkesfx3o for full details.
> 
> 
> Diffs
> -
> 
>   build-support/packer/build.sh 658dbc4bc5ae0dd1cfa69abd8fdf6119c3ccf8c9 
> 
> Diff: https://reviews.apache.org/r/46786/diff/
> 
> 
> Testing
> ---
> 
> Successfully built base box with these changes.
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>



Re: Review Request 46786: Add missing Mesos dependency to packer build script.

2016-04-28 Thread John Sirois


> On April 28, 2016, 10:02 a.m., John Sirois wrote:
> > build-support/packer/build.sh, line 44
> > 
> >
> > I'm missing how this is needed too.
> 
> Joshua Cohen wrote:
> If I remove this we don't have a jdk in the vagrant image and therefore 
> can't build the scheduler. I.e., I guess default-jre-headless brings in only 
> the JRE but not the JDK?
> 
> John Sirois wrote:
> Backing up - were you anticipating the mesos upgrade issue linked or did 
> you hit a different real issue that prompted this change?  If the former, 
> JDKs include JREs in their jre subdir and the fact we have a direct 
> dependency on a jdk means no upgrade will magically remove it.

TLDR; the mesos thread could be said to be warning for all non-JRE-using 
frameworks only.


- John


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


On April 28, 2016, 11:08 a.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46786/
> ---
> 
> (Updated April 28, 2016, 11:08 a.m.)
> 
> 
> Review request for Aurora, John Sirois and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> See http://markmail.org/message/rofuiaclgkesfx3o for full details.
> 
> 
> Diffs
> -
> 
>   build-support/packer/build.sh 658dbc4bc5ae0dd1cfa69abd8fdf6119c3ccf8c9 
> 
> Diff: https://reviews.apache.org/r/46786/diff/
> 
> 
> Testing
> ---
> 
> Successfully built base box with these changes.
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>



Re: Review Request 46786: Add missing Mesos dependency to packer build script.

2016-04-28 Thread John Sirois


> On April 28, 2016, 10:02 a.m., John Sirois wrote:
> > build-support/packer/build.sh, line 44
> > 
> >
> > I'm missing how this is needed too.
> 
> Joshua Cohen wrote:
> If I remove this we don't have a jdk in the vagrant image and therefore 
> can't build the scheduler. I.e., I guess default-jre-headless brings in only 
> the JRE but not the JDK?

Backing up - were you anticipating the mesos upgrade issue linked or did you 
hit a different real issue that prompted this change?  If the former, JDKs 
include JREs in their jre subdir and the fact we have a direct dependency on a 
jdk means no upgrade will magically remove it.


- John


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


On April 28, 2016, 11:08 a.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46786/
> ---
> 
> (Updated April 28, 2016, 11:08 a.m.)
> 
> 
> Review request for Aurora, John Sirois and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> See http://markmail.org/message/rofuiaclgkesfx3o for full details.
> 
> 
> Diffs
> -
> 
>   build-support/packer/build.sh 658dbc4bc5ae0dd1cfa69abd8fdf6119c3ccf8c9 
> 
> Diff: https://reviews.apache.org/r/46786/diff/
> 
> 
> Testing
> ---
> 
> Successfully built base box with these changes.
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>



Re: Review Request 46786: Add missing Mesos dependency to packer build script.

2016-04-28 Thread John Sirois


> On April 28, 2016, 10:02 a.m., John Sirois wrote:
> > build-support/packer/build.sh, line 44
> > 
> >
> > I'm missing how this is needed too.
> 
> Joshua Cohen wrote:
> If I remove this we don't have a jdk in the vagrant image and therefore 
> can't build the scheduler. I.e., I guess default-jre-headless brings in only 
> the JRE but not the JDK?
> 
> John Sirois wrote:
> Backing up - were you anticipating the mesos upgrade issue linked or did 
> you hit a different real issue that prompted this change?  If the former, 
> JDKs include JREs in their jre subdir and the fact we have a direct 
> dependency on a jdk means no upgrade will magically remove it.
> 
> John Sirois wrote:
> TLDR; the mesos thread could be said to be warning for all non-JRE-using 
> frameworks only.
> 
> Joshua Cohen wrote:
> I'm trying to build a new base box in another branch for the unified 
> container changes and it was failing on the `install_mesos` step due to 
> java-runtime-headless not being found. After ruling out my changes by running 
> the script on master and getting the same results, I happened across that 
> thread on mesos-users that described the problem. I'll confess to being 
> unfamiliar with the internals of all this packaging.
> 
> You should be able to replicate the original problem fairly trivially by 
> running the packer build command on master. If there's a better/cleaner 
> solution than adding this dependency (or getting mesosphere to publish a new 
> deb), I'm open to it!

I think 1 dep on https://launchpad.net/ubuntu/+source/openjdk-8 with a note why 
- ie we need its transitive dep on jdk for aurora builds + jre for mesos deb 
package constraints.
Or else - preferrable I think for being explicit - the existing dep on 
openjdk-8-jdk-headless paired with a dep on openjdk-8-jre-headless and a 
comment on why the latter.  The mesos thread comment could be removed since its 
not at play here fwict - ie we are not worried about magic removal, just trying 
to satisfy a deb logical dep fwict.

These take me a while on my slow link, but I'm trying this out now.


- John


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


On April 28, 2016, 11:08 a.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46786/
> ---
> 
> (Updated April 28, 2016, 11:08 a.m.)
> 
> 
> Review request for Aurora, John Sirois and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> See http://markmail.org/message/rofuiaclgkesfx3o for full details.
> 
> 
> Diffs
> -
> 
>   build-support/packer/build.sh 658dbc4bc5ae0dd1cfa69abd8fdf6119c3ccf8c9 
> 
> Diff: https://reviews.apache.org/r/46786/diff/
> 
> 
> Testing
> ---
> 
> Successfully built base box with these changes.
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>



Re: Review Request 46786: Add missing Mesos dependency to packer build script.

2016-04-28 Thread Joshua Cohen

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

(Updated April 28, 2016, 5:08 p.m.)


Review request for Aurora, John Sirois and Stephan Erb.


Changes
---

Switch to default-jre-headless


Repository: aurora


Description
---

See http://markmail.org/message/rofuiaclgkesfx3o for full details.


Diffs (updated)
-

  build-support/packer/build.sh 658dbc4bc5ae0dd1cfa69abd8fdf6119c3ccf8c9 

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


Testing
---

Successfully built base box with these changes.


Thanks,

Joshua Cohen



Re: Review Request 46786: Add missing Mesos dependency to packer build script.

2016-04-28 Thread Joshua Cohen


> On April 28, 2016, 4:02 p.m., John Sirois wrote:
> > build-support/packer/build.sh, line 44
> > 
> >
> > I'm missing how this is needed too.
> 
> Joshua Cohen wrote:
> If I remove this we don't have a jdk in the vagrant image and therefore 
> can't build the scheduler. I.e., I guess default-jre-headless brings in only 
> the JRE but not the JDK?
> 
> John Sirois wrote:
> Backing up - were you anticipating the mesos upgrade issue linked or did 
> you hit a different real issue that prompted this change?  If the former, 
> JDKs include JREs in their jre subdir and the fact we have a direct 
> dependency on a jdk means no upgrade will magically remove it.
> 
> John Sirois wrote:
> TLDR; the mesos thread could be said to be warning for all non-JRE-using 
> frameworks only.

I'm trying to build a new base box in another branch for the unified container 
changes and it was failing on the `install_mesos` step due to 
java-runtime-headless not being found. After ruling out my changes by running 
the script on master and getting the same results, I happened across that 
thread on mesos-users that described the problem. I'll confess to being 
unfamiliar with the internals of all this packaging.

You should be able to replicate the original problem fairly trivially by 
running the packer build command on master. If there's a better/cleaner 
solution than adding this dependency (or getting mesosphere to publish a new 
deb), I'm open to it!


- Joshua


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


On April 28, 2016, 5:08 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46786/
> ---
> 
> (Updated April 28, 2016, 5:08 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> See http://markmail.org/message/rofuiaclgkesfx3o for full details.
> 
> 
> Diffs
> -
> 
>   build-support/packer/build.sh 658dbc4bc5ae0dd1cfa69abd8fdf6119c3ccf8c9 
> 
> Diff: https://reviews.apache.org/r/46786/diff/
> 
> 
> Testing
> ---
> 
> Successfully built base box with these changes.
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>



Re: Review Request 46786: Add missing Mesos dependency to packer build script.

2016-04-28 Thread Aurora ReviewBot

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


Ship it!




Master (e817eb1) 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 April 28, 2016, 5:08 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46786/
> ---
> 
> (Updated April 28, 2016, 5:08 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> See http://markmail.org/message/rofuiaclgkesfx3o for full details.
> 
> 
> Diffs
> -
> 
>   build-support/packer/build.sh 658dbc4bc5ae0dd1cfa69abd8fdf6119c3ccf8c9 
> 
> Diff: https://reviews.apache.org/r/46786/diff/
> 
> 
> Testing
> ---
> 
> Successfully built base box with these changes.
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>



Re: Review Request 46803: AURORA-1458: Add tier into the UI "show config" summary.

2016-04-28 Thread Aurora ReviewBot

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



Master (450d881) 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 April 28, 2016, 11:09 p.m., Amol Deshmukh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46803/
> ---
> 
> (Updated April 28, 2016, 11:09 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> AURORA-1458: Add tier into the UI "show config" summary.
> 
> 
> Diffs
> -
> 
>   src/main/resources/scheduler/assets/configSummary.html 
> 1af7511de0e8a143c8ea88377aad756b44e3ac30 
>   src/main/resources/scheduler/assets/js/controllers.js 
> 84417ebeadfae57d55b9f12e8a985825bd620fc8 
>   src/main/resources/scheduler/assets/js/services.js 
> d9ce52065f9573b0aa68a95da7da7c50fb14310a 
>   src/main/resources/scheduler/assets/schedulingDetail.html 
> eb88c1e6dec7a26643e8b13ffcf8e90df70a67f7 
> 
> Diff: https://reviews.apache.org/r/46803/diff/
> 
> 
> Testing
> ---
> 
> Ensured the changes appear in the UI after launching the scheduler using 
> vagrant.
> 
> 
> File Attachments
> 
> 
> Jobs by Role
>   
> https://reviews.apache.org/media/uploaded/files/2016/04/28/7cbec002-e213-4c24-92d9-16b45efb839c__Jobs_by_Role.png
> Per Job Config Summary
>   
> https://reviews.apache.org/media/uploaded/files/2016/04/28/5b785fa7-1053-4b8b-ba0d-438a9170c756__Per_Job_Config_Summary.png
> 
> 
> Thanks,
> 
> Amol Deshmukh
> 
>



Re: Review Request 46795: Fixing e2e tests.

2016-04-28 Thread Joshua Cohen


> On April 28, 2016, 6:31 p.m., Joshua Cohen wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/db/migration/V005_CreateQuotaResourceTable.java,
> >  line 39
> > 
> >
> > What is value in the case of ports? I thought it would be the port 
> > name? In which case this constraint should still be valid?
> 
> Maxim Khutornenko wrote:
> Ports are not supported in ResourceAggregates, hence the ThriftBackfill 
> changes in this patch to only allow CPUS, RAM and disk. This constraint was 
> too permissive as it would allow something like 'CPUS, 1.0' and 'CPUS, 2.0', 
> which is no longer accepted.

Ahhh, gotcha, thanks for clarifying!


- Joshua


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


On April 28, 2016, 6:14 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46795/
> ---
> 
> (Updated April 28, 2016, 6:14 p.m.)
> 
> 
> Review request for Aurora and Joshua Cohen.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Got too greedy converting pairs to maps during the last round of 
> https://reviews.apache.org/r/46716. Task resources must be handled via a list 
> of Pairs as there could be duplicate resource types there (e.g. ports).
> 
> Also, added more validation into ResourceAggregate backfilling to account for 
> invalid/missing resource types.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbQuotaStore.java 
> e7afbad33a92936f2949f00e60ade3703b97befe 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
> 07bdf22dcf90f6ca11a1d1800c59032ffad824ce 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java 
> 6e06af47c89ccb3b0dec1bda3683a76cd1effb81 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/migration/V005_CreateQuotaResourceTable.java
>  e9e5de10418d42035268c593541814705d145d26 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/views/DBResourceAggregate.java
>  4f5c07e722ba580c1c3b45dcf83321694625bbaa 
>   src/main/java/org/apache/aurora/scheduler/storage/log/ThriftBackfill.java 
> ad45085e907d5386fb6d96f1297276c0a413ef23 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml
>  6b5e1304d7213bd3a1483b4981e2558512b64a4e 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> 5d905db17ca478e789dd22c9c49e5c1e0d554a51 
>   
> src/test/java/org/apache/aurora/scheduler/storage/log/ThriftBackfillTest.java 
> f213b7f9a35a490e3d62283bb3b9e6b32ed4d624 
> 
> Diff: https://reviews.apache.org/r/46795/diff/
> 
> 
> Testing
> ---
> 
> unit and e2e tests
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>



Re: Review Request 46786: Add missing Mesos dependency to packer build script.

2016-04-28 Thread John Sirois


> On April 28, 2016, 10:02 a.m., John Sirois wrote:
> > build-support/packer/build.sh, line 44
> > 
> >
> > I'm missing how this is needed too.
> 
> Joshua Cohen wrote:
> If I remove this we don't have a jdk in the vagrant image and therefore 
> can't build the scheduler. I.e., I guess default-jre-headless brings in only 
> the JRE but not the JDK?
> 
> John Sirois wrote:
> Backing up - were you anticipating the mesos upgrade issue linked or did 
> you hit a different real issue that prompted this change?  If the former, 
> JDKs include JREs in their jre subdir and the fact we have a direct 
> dependency on a jdk means no upgrade will magically remove it.
> 
> John Sirois wrote:
> TLDR; the mesos thread could be said to be warning for all non-JRE-using 
> frameworks only.
> 
> Joshua Cohen wrote:
> I'm trying to build a new base box in another branch for the unified 
> container changes and it was failing on the `install_mesos` step due to 
> java-runtime-headless not being found. After ruling out my changes by running 
> the script on master and getting the same results, I happened across that 
> thread on mesos-users that described the problem. I'll confess to being 
> unfamiliar with the internals of all this packaging.
> 
> You should be able to replicate the original problem fairly trivially by 
> running the packer build command on master. If there's a better/cleaner 
> solution than adding this dependency (or getting mesosphere to publish a new 
> deb), I'm open to it!
> 
> John Sirois wrote:
> I think 1 dep on https://launchpad.net/ubuntu/+source/openjdk-8 with a 
> note why - ie we need its transitive dep on jdk for aurora builds + jre for 
> mesos deb package constraints.
> Or else - preferrable I think for being explicit - the existing dep on 
> openjdk-8-jdk-headless paired with a dep on openjdk-8-jre-headless and a 
> comment on why the latter.  The mesos thread comment could be removed since 
> its not at play here fwict - ie we are not worried about magic removal, just 
> trying to satisfy a deb logical dep fwict.
> 
> These take me a while on my slow link, but I'm trying this out now.

This works for me:
```diff
$ git diff -U1
diff --git a/build-support/packer/build.sh b/build-support/packer/build.sh
index 658dbc4..57279c8 100644
--- a/build-support/packer/build.sh
+++ b/build-support/packer/build.sh
@@ -19,3 +19,3 @@ set -o verbose
 
-readonly MESOS_VERSION=0.27.2
+readonly MESOS_VERSION=0.28.0
 
@@ -29,3 +29,3 @@ function remove_unused {
 function install_base_packages {
-  add-apt-repository ppa:openjdk-r/ppa -y
+  add-apt-repository -y ppa:openjdk-r/ppa
   apt-get update
@@ -40,3 +40,3 @@ function install_base_packages {
   libsvn-dev \
-  openjdk-8-jdk \
+  openjdk-8-jdk-headless \
   python-dev
@@ -44,3 +44,3 @@ function install_base_packages {
   # Installing zookeeperd as a separate command, as otherwise openjdk-7-jdk is 
also installed.
-  apt-get install -y zookeeperd
+  apt-get -y install zookeeperd
 }
@@ -49,3 +49,3 @@ function install_docker {
   # Instructions from 
https://docs.docker.com/engine/installation/linux/ubuntulinux/
-  apt-get install -y apt-transport-https ca-certificates
+  apt-get -y install apt-transport-https ca-certificates
   apt-key adv --keyserver hkp://p80.pool.sks-keyservers.net:80 \
@@ -62,8 +62,9 @@ function install_docker {
 function install_mesos {
-  URL_BASE='http://repos.mesosphere.com/ubuntu/pool/main/m/mesos'
-  DEB_URL="$URL_BASE/mesos_${MESOS_VERSION}-2.0.15.ubuntu1404_amd64.deb"
-  deb=$(basename $DEB_URL)
-  wget -c "$DEB_URL"
-  dpkg --install $deb
-  rm $deb
+  sudo apt-key adv --keyserver keyserver.ubuntu.com --recv E56151BF
+  DISTRO=$(lsb_release -is | tr '[:upper:]' '[:lower:]')
+  CODENAME=$(lsb_release -cs)
+  echo "deb http://repos.mesosphere.io/${DISTRO} ${CODENAME} main" \
+> /etc/apt/sources.list.d/mesosphere.list
+  apt-get update
+  apt-get -y install mesos=${MESOS_VERSION}*
 }
@@ -77,3 +78,3 @@ function install_thrift {
   apt-get update
-  apt-get install thrift-compiler=0.9.1
+  apt-get -y install thrift-compiler=0.9.1
 }
```

The jdk is slimmed to headless, no explicit jre added - and the lynchpin is the 
use of apt-get to install mesos.


- John


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


On April 28, 2016, 11:08 a.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46786/
> ---
> 
> (Updated April 28, 2016, 11:08 a.m.)
> 
> 
> Review request for Aurora, John Sirois and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 

Re: Review Request 46795: Fixing e2e tests.

2016-04-28 Thread Joshua Cohen

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


Ship it!




lgtm assuming the question below is not an issue.


src/main/java/org/apache/aurora/scheduler/storage/db/migration/V005_CreateQuotaResourceTable.java
 (line 39)


What is value in the case of ports? I thought it would be the port name? In 
which case this constraint should still be valid?


- Joshua Cohen


On April 28, 2016, 6:14 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46795/
> ---
> 
> (Updated April 28, 2016, 6:14 p.m.)
> 
> 
> Review request for Aurora and Joshua Cohen.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Got too greedy converting pairs to maps during the last round of 
> https://reviews.apache.org/r/46716. Task resources must be handled via a list 
> of Pairs as there could be duplicate resource types there (e.g. ports).
> 
> Also, added more validation into ResourceAggregate backfilling to account for 
> invalid/missing resource types.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbQuotaStore.java 
> e7afbad33a92936f2949f00e60ade3703b97befe 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
> 07bdf22dcf90f6ca11a1d1800c59032ffad824ce 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java 
> 6e06af47c89ccb3b0dec1bda3683a76cd1effb81 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/migration/V005_CreateQuotaResourceTable.java
>  e9e5de10418d42035268c593541814705d145d26 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/views/DBResourceAggregate.java
>  4f5c07e722ba580c1c3b45dcf83321694625bbaa 
>   src/main/java/org/apache/aurora/scheduler/storage/log/ThriftBackfill.java 
> ad45085e907d5386fb6d96f1297276c0a413ef23 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml
>  6b5e1304d7213bd3a1483b4981e2558512b64a4e 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> 5d905db17ca478e789dd22c9c49e5c1e0d554a51 
>   
> src/test/java/org/apache/aurora/scheduler/storage/log/ThriftBackfillTest.java 
> f213b7f9a35a490e3d62283bb3b9e6b32ed4d624 
> 
> Diff: https://reviews.apache.org/r/46795/diff/
> 
> 
> Testing
> ---
> 
> unit and e2e tests
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>



Re: Review Request 46795: Fixing e2e tests.

2016-04-28 Thread Aurora ReviewBot

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


Ship it!




Master (e817eb1) 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 April 28, 2016, 6:14 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46795/
> ---
> 
> (Updated April 28, 2016, 6:14 p.m.)
> 
> 
> Review request for Aurora and Joshua Cohen.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Got too greedy converting pairs to maps during the last round of 
> https://reviews.apache.org/r/46716. Task resources must be handled via a list 
> of Pairs as there could be duplicate resource types there (e.g. ports).
> 
> Also, added more validation into ResourceAggregate backfilling to account for 
> invalid/missing resource types.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbQuotaStore.java 
> e7afbad33a92936f2949f00e60ade3703b97befe 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
> 07bdf22dcf90f6ca11a1d1800c59032ffad824ce 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java 
> 6e06af47c89ccb3b0dec1bda3683a76cd1effb81 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/migration/V005_CreateQuotaResourceTable.java
>  e9e5de10418d42035268c593541814705d145d26 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/views/DBResourceAggregate.java
>  4f5c07e722ba580c1c3b45dcf83321694625bbaa 
>   src/main/java/org/apache/aurora/scheduler/storage/log/ThriftBackfill.java 
> ad45085e907d5386fb6d96f1297276c0a413ef23 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml
>  6b5e1304d7213bd3a1483b4981e2558512b64a4e 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> 5d905db17ca478e789dd22c9c49e5c1e0d554a51 
>   
> src/test/java/org/apache/aurora/scheduler/storage/log/ThriftBackfillTest.java 
> f213b7f9a35a490e3d62283bb3b9e6b32ed4d624 
> 
> Diff: https://reviews.apache.org/r/46795/diff/
> 
> 
> Testing
> ---
> 
> unit and e2e tests
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>



Review Request 46810: Generalizing port resource management.

2016-04-28 Thread Maxim Khutornenko

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

Review request for Aurora, Joshua Cohen, Bill Farner, and Zameer Manji.


Repository: aurora


Description
---

This patch effectively gets rid of `ITaskConfig.getRequestedPorts()` usage. A 
few implementation notes:
- `ResourceType` gets an optional `ResourceMapper` property applicable to any 
resources types that require mapping and assignment of matched values
- All port specific matching/assigning logic is now encapsulated within 
`PortMapper` implementing `ResourceMapper` interface
- `TaskAssigner` now passes a callback into the `StateManager` to map and 
assign port values
- The newly introduced `ResourceManager` will eventually replace both 
`Resources` and `ResourceSlot` classes.


Diffs
-

  
src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
 9a15a4b5c1cdc3efbf0588e8bd81636b66e938ef 
  src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 
625e6d5e1db1c7327154823f5a604efd3a4ff690 
  src/main/java/org/apache/aurora/scheduler/resources/ResourceManager.java 
PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/resources/ResourceMapper.java 
PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/resources/ResourceSlot.java 
1df2c11a84db8d46158c87e7c3709fc08016a107 
  src/main/java/org/apache/aurora/scheduler/resources/ResourceType.java 
6e4d694cc6449a211297919d7834996382b21b21 
  src/main/java/org/apache/aurora/scheduler/resources/Resources.java 
894c6a66545ac336d594d83c595ee83b7c91e21d 
  src/main/java/org/apache/aurora/scheduler/state/StateManager.java 
5d34fe38492cae3f50ea9ed0baca11472295af60 
  src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java 
e5b2f41f55aec161840c3fb17f2ff73161b77482 
  src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 
7d43d4ad2de60f76183c91b860b00e7022d79bcb 
  
src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
 731327950099f2567430f3af75633854910443ff 
  
src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 
6370a126340dd4312ae0f2da52fee6ccdbced595 
  src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
bf18d5d53f7eda62120299146a956fa0a0985f71 
  src/test/java/org/apache/aurora/scheduler/resources/AcceptedOfferTest.java 
d5f2172131ce4b765a1f85d63aeddf1daaa0b606 
  src/test/java/org/apache/aurora/scheduler/resources/PortMapperTest.java 
PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/resources/ResourceManagerTest.java 
PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/resources/ResourceTestUtil.java 
04a82382e8535e1a233d48370bd0650f5abcd787 
  src/test/java/org/apache/aurora/scheduler/resources/ResourcesTest.java 
ceb81e42e558fadb372cf4de862c6d3bcd293099 
  src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java 
498da78169be42fd5edeb9963d8262af02895f0e 
  src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 
a2df3114a299218e7fb04a21cdedc8d911029bb0 

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


Testing
---

unit and e2e tests


Thanks,

Maxim Khutornenko



Re: Review Request 46803: AURORA-1458: Add tier into the UI "show config" summary.

2016-04-28 Thread Joshua Cohen

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



Is there ever a case where we wouldn't want to show the tier? I.e., can someone 
configure the scheduler to *not* use tiers, or are they always on now?

I'm concerned that this is fairly prominent UI placement for a concept that 
will be completely unknown to users. It's hard for me to judge what the best UI 
is to represent tiers when no one's actually using them yet (e.g. will users 
think in terms of tier names, or will they think in terms of the properties 
associated with tiers, in which case, would it make more sense to break it down 
to the level of preemptible and revocable).

I guess for now, given the unknowns this is fine and we can iterate as we get 
feedback from users.

- Joshua Cohen


On April 28, 2016, 11:09 p.m., Amol Deshmukh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46803/
> ---
> 
> (Updated April 28, 2016, 11:09 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> AURORA-1458: Add tier into the UI "show config" summary.
> 
> 
> Diffs
> -
> 
>   src/main/resources/scheduler/assets/configSummary.html 
> 1af7511de0e8a143c8ea88377aad756b44e3ac30 
>   src/main/resources/scheduler/assets/js/controllers.js 
> 84417ebeadfae57d55b9f12e8a985825bd620fc8 
>   src/main/resources/scheduler/assets/js/services.js 
> d9ce52065f9573b0aa68a95da7da7c50fb14310a 
>   src/main/resources/scheduler/assets/schedulingDetail.html 
> eb88c1e6dec7a26643e8b13ffcf8e90df70a67f7 
> 
> Diff: https://reviews.apache.org/r/46803/diff/
> 
> 
> Testing
> ---
> 
> Ensured the changes appear in the UI after launching the scheduler using 
> vagrant.
> 
> 
> File Attachments
> 
> 
> Jobs by Role
>   
> https://reviews.apache.org/media/uploaded/files/2016/04/28/7cbec002-e213-4c24-92d9-16b45efb839c__Jobs_by_Role.png
> Per Job Config Summary
>   
> https://reviews.apache.org/media/uploaded/files/2016/04/28/5b785fa7-1053-4b8b-ba0d-438a9170c756__Per_Job_Config_Summary.png
> 
> 
> Thanks,
> 
> Amol Deshmukh
> 
>



Re: Review Request 46803: AURORA-1458: Add tier into the UI "show config" summary.

2016-04-28 Thread Joshua Cohen

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


Ship it!




Ship It!

- Joshua Cohen


On April 28, 2016, 11:09 p.m., Amol Deshmukh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46803/
> ---
> 
> (Updated April 28, 2016, 11:09 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> AURORA-1458: Add tier into the UI "show config" summary.
> 
> 
> Diffs
> -
> 
>   src/main/resources/scheduler/assets/configSummary.html 
> 1af7511de0e8a143c8ea88377aad756b44e3ac30 
>   src/main/resources/scheduler/assets/js/controllers.js 
> 84417ebeadfae57d55b9f12e8a985825bd620fc8 
>   src/main/resources/scheduler/assets/js/services.js 
> d9ce52065f9573b0aa68a95da7da7c50fb14310a 
>   src/main/resources/scheduler/assets/schedulingDetail.html 
> eb88c1e6dec7a26643e8b13ffcf8e90df70a67f7 
> 
> Diff: https://reviews.apache.org/r/46803/diff/
> 
> 
> Testing
> ---
> 
> Ensured the changes appear in the UI after launching the scheduler using 
> vagrant.
> 
> 
> File Attachments
> 
> 
> Jobs by Role
>   
> https://reviews.apache.org/media/uploaded/files/2016/04/28/7cbec002-e213-4c24-92d9-16b45efb839c__Jobs_by_Role.png
> Per Job Config Summary
>   
> https://reviews.apache.org/media/uploaded/files/2016/04/28/5b785fa7-1053-4b8b-ba0d-438a9170c756__Per_Job_Config_Summary.png
> 
> 
> Thanks,
> 
> Amol Deshmukh
> 
>



Re: Review Request 46810: Generalizing port resource management.

2016-04-28 Thread Aurora ReviewBot

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


Ship it!




Master (450d881) 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 April 29, 2016, 1:08 a.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46810/
> ---
> 
> (Updated April 29, 2016, 1:08 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Bill Farner, and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This patch effectively gets rid of `ITaskConfig.getRequestedPorts()` usage. A 
> few implementation notes:
> - `ResourceType` gets an optional `ResourceMapper` property applicable to any 
> resources types that require mapping and assignment of matched values
> - All port specific matching/assigning logic is now encapsulated within 
> `PortMapper` implementing `ResourceMapper` interface
> - `TaskAssigner` now passes a callback into the `StateManager` to map and 
> assign port values
> - The newly introduced `ResourceManager` will eventually replace both 
> `Resources` and `ResourceSlot` classes.
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  9a15a4b5c1cdc3efbf0588e8bd81636b66e938ef 
>   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 
> 625e6d5e1db1c7327154823f5a604efd3a4ff690 
>   src/main/java/org/apache/aurora/scheduler/resources/ResourceManager.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/resources/ResourceMapper.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/resources/ResourceSlot.java 
> 1df2c11a84db8d46158c87e7c3709fc08016a107 
>   src/main/java/org/apache/aurora/scheduler/resources/ResourceType.java 
> 6e4d694cc6449a211297919d7834996382b21b21 
>   src/main/java/org/apache/aurora/scheduler/resources/Resources.java 
> 894c6a66545ac336d594d83c595ee83b7c91e21d 
>   src/main/java/org/apache/aurora/scheduler/state/StateManager.java 
> 5d34fe38492cae3f50ea9ed0baca11472295af60 
>   src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java 
> e5b2f41f55aec161840c3fb17f2ff73161b77482 
>   src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 
> 7d43d4ad2de60f76183c91b860b00e7022d79bcb 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  731327950099f2567430f3af75633854910443ff 
>   
> src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java
>  6370a126340dd4312ae0f2da52fee6ccdbced595 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> bf18d5d53f7eda62120299146a956fa0a0985f71 
>   src/test/java/org/apache/aurora/scheduler/resources/AcceptedOfferTest.java 
> d5f2172131ce4b765a1f85d63aeddf1daaa0b606 
>   src/test/java/org/apache/aurora/scheduler/resources/PortMapperTest.java 
> PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/resources/ResourceManagerTest.java 
> PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/resources/ResourceTestUtil.java 
> 04a82382e8535e1a233d48370bd0650f5abcd787 
>   src/test/java/org/apache/aurora/scheduler/resources/ResourcesTest.java 
> ceb81e42e558fadb372cf4de862c6d3bcd293099 
>   src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java 
> 498da78169be42fd5edeb9963d8262af02895f0e 
>   src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 
> a2df3114a299218e7fb04a21cdedc8d911029bb0 
> 
> Diff: https://reviews.apache.org/r/46810/diff/
> 
> 
> Testing
> ---
> 
> unit and e2e tests
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>



Re: Review Request 46803: AURORA-1458: Add tier into the UI "show config" summary.

2016-04-28 Thread Maxim Khutornenko

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




src/main/resources/scheduler/assets/js/controllers.js (line 139)


I am not too keen on having a column for an optional field that will most 
likely read 'default' for all jobs. Perhaps postpone its introduction until 
TaskConfig.tier is required?


- Maxim Khutornenko


On April 28, 2016, 11:09 p.m., Amol Deshmukh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46803/
> ---
> 
> (Updated April 28, 2016, 11:09 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> AURORA-1458: Add tier into the UI "show config" summary.
> 
> 
> Diffs
> -
> 
>   src/main/resources/scheduler/assets/configSummary.html 
> 1af7511de0e8a143c8ea88377aad756b44e3ac30 
>   src/main/resources/scheduler/assets/js/controllers.js 
> 84417ebeadfae57d55b9f12e8a985825bd620fc8 
>   src/main/resources/scheduler/assets/js/services.js 
> d9ce52065f9573b0aa68a95da7da7c50fb14310a 
>   src/main/resources/scheduler/assets/schedulingDetail.html 
> eb88c1e6dec7a26643e8b13ffcf8e90df70a67f7 
> 
> Diff: https://reviews.apache.org/r/46803/diff/
> 
> 
> Testing
> ---
> 
> Ensured the changes appear in the UI after launching the scheduler using 
> vagrant.
> 
> 
> File Attachments
> 
> 
> Jobs by Role
>   
> https://reviews.apache.org/media/uploaded/files/2016/04/28/7cbec002-e213-4c24-92d9-16b45efb839c__Jobs_by_Role.png
> Per Job Config Summary
>   
> https://reviews.apache.org/media/uploaded/files/2016/04/28/5b785fa7-1053-4b8b-ba0d-438a9170c756__Per_Job_Config_Summary.png
> 
> 
> Thanks,
> 
> Amol Deshmukh
> 
>



Re: Review Request 46803: AURORA-1458: Add tier into the UI "show config" summary.

2016-04-28 Thread Amol Deshmukh


> On April 28, 2016, 6:20 p.m., Maxim Khutornenko wrote:
> > src/main/resources/scheduler/assets/js/controllers.js, line 139
> > 
> >
> > I am not too keen on having a column for an optional field that will 
> > most likely read 'default' for all jobs. Perhaps postpone its introduction 
> > until TaskConfig.tier is required?

I thought that leaving it blank would raise more questions for most users who 
did not use tiers at all (basically the same point that Joshua raised above).

I'll leave it blank and we can address that if the need arises.


- Amol


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


On April 28, 2016, 4:09 p.m., Amol Deshmukh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46803/
> ---
> 
> (Updated April 28, 2016, 4:09 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> AURORA-1458: Add tier into the UI "show config" summary.
> 
> 
> Diffs
> -
> 
>   src/main/resources/scheduler/assets/configSummary.html 
> 1af7511de0e8a143c8ea88377aad756b44e3ac30 
>   src/main/resources/scheduler/assets/js/controllers.js 
> 84417ebeadfae57d55b9f12e8a985825bd620fc8 
>   src/main/resources/scheduler/assets/js/services.js 
> d9ce52065f9573b0aa68a95da7da7c50fb14310a 
>   src/main/resources/scheduler/assets/schedulingDetail.html 
> eb88c1e6dec7a26643e8b13ffcf8e90df70a67f7 
> 
> Diff: https://reviews.apache.org/r/46803/diff/
> 
> 
> Testing
> ---
> 
> Ensured the changes appear in the UI after launching the scheduler using 
> vagrant.
> 
> 
> File Attachments
> 
> 
> Jobs by Role
>   
> https://reviews.apache.org/media/uploaded/files/2016/04/28/7cbec002-e213-4c24-92d9-16b45efb839c__Jobs_by_Role.png
> Per Job Config Summary
>   
> https://reviews.apache.org/media/uploaded/files/2016/04/28/5b785fa7-1053-4b8b-ba0d-438a9170c756__Per_Job_Config_Summary.png
> 
> 
> Thanks,
> 
> Amol Deshmukh
> 
>



Re: Review Request 46803: AURORA-1458: Add tier into the UI "show config" summary.

2016-04-28 Thread Amol Deshmukh

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

(Updated April 28, 2016, 7:54 p.m.)


Review request for Aurora, Joshua Cohen and Maxim Khutornenko.


Changes
---

Removed the 'default' text replacement.


Repository: aurora


Description
---

AURORA-1458: Add tier into the UI "show config" summary.


Diffs (updated)
-

  src/main/resources/scheduler/assets/configSummary.html 
1af7511de0e8a143c8ea88377aad756b44e3ac30 
  src/main/resources/scheduler/assets/js/controllers.js 
84417ebeadfae57d55b9f12e8a985825bd620fc8 
  src/main/resources/scheduler/assets/js/services.js 
d9ce52065f9573b0aa68a95da7da7c50fb14310a 
  src/main/resources/scheduler/assets/schedulingDetail.html 
eb88c1e6dec7a26643e8b13ffcf8e90df70a67f7 

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


Testing
---

Ensured the changes appear in the UI after launching the scheduler using 
vagrant.


File Attachments (updated)


Jobs by Role
  
https://reviews.apache.org/media/uploaded/files/2016/04/29/6e58d26d-e77c-49fe-85e0-ee1acae3efe0__Jobs_by_Role.png
Per Job Config Summary
  
https://reviews.apache.org/media/uploaded/files/2016/04/29/8a666fe7-9a35-427f-b1ee-f41e5413059d__Per_Job_Config_Summary.png


Thanks,

Amol Deshmukh



Re: Review Request 46786: Add missing Mesos dependency to packer build script.

2016-04-28 Thread Joshua Cohen

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

(Updated April 28, 2016, 7:37 p.m.)


Review request for Aurora, John Sirois and Stephan Erb.


Changes
---

Update based on John's diff.


Repository: aurora


Description
---

See http://markmail.org/message/rofuiaclgkesfx3o for full details.


Diffs (updated)
-

  build-support/packer/build.sh 658dbc4bc5ae0dd1cfa69abd8fdf6119c3ccf8c9 

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


Testing (updated)
---

Successfully built base box with these changes and then ran e2e tests.


Thanks,

Joshua Cohen



Re: Review Request 46786: Add missing Mesos dependency to packer build script.

2016-04-28 Thread John Sirois

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


Ship it!




Ship It!

- John Sirois


On April 28, 2016, 1:37 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46786/
> ---
> 
> (Updated April 28, 2016, 1:37 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> See http://markmail.org/message/rofuiaclgkesfx3o for full details.
> 
> 
> Diffs
> -
> 
>   build-support/packer/build.sh 658dbc4bc5ae0dd1cfa69abd8fdf6119c3ccf8c9 
> 
> Diff: https://reviews.apache.org/r/46786/diff/
> 
> 
> Testing
> ---
> 
> Successfully built base box with these changes and then ran e2e tests.
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>



Re: Review Request 46803: AURORA-1458: Add tier into the UI "show config" summary.

2016-04-28 Thread Aurora ReviewBot

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



Master (450d881) 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 April 29, 2016, 2:54 a.m., Amol Deshmukh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46803/
> ---
> 
> (Updated April 29, 2016, 2:54 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> AURORA-1458: Add tier into the UI "show config" summary.
> 
> 
> Diffs
> -
> 
>   src/main/resources/scheduler/assets/configSummary.html 
> 1af7511de0e8a143c8ea88377aad756b44e3ac30 
>   src/main/resources/scheduler/assets/js/controllers.js 
> 84417ebeadfae57d55b9f12e8a985825bd620fc8 
>   src/main/resources/scheduler/assets/js/services.js 
> d9ce52065f9573b0aa68a95da7da7c50fb14310a 
>   src/main/resources/scheduler/assets/schedulingDetail.html 
> eb88c1e6dec7a26643e8b13ffcf8e90df70a67f7 
> 
> Diff: https://reviews.apache.org/r/46803/diff/
> 
> 
> Testing
> ---
> 
> Ensured the changes appear in the UI after launching the scheduler using 
> vagrant.
> 
> 
> File Attachments
> 
> 
> Jobs by Role
>   
> https://reviews.apache.org/media/uploaded/files/2016/04/29/6e58d26d-e77c-49fe-85e0-ee1acae3efe0__Jobs_by_Role.png
> Per Job Config Summary
>   
> https://reviews.apache.org/media/uploaded/files/2016/04/29/8a666fe7-9a35-427f-b1ee-f41e5413059d__Per_Job_Config_Summary.png
> 
> 
> Thanks,
> 
> Amol Deshmukh
> 
>



Re: Review Request 46810: Generalizing port resource management.

2016-04-28 Thread Joshua Cohen

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




src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
 (lines 351 - 356)


These two cases are now mutually exclusive, use `else if` for the second?



src/main/java/org/apache/aurora/scheduler/resources/ResourceManager.java (line 
45)


nit: use a more appropriate variable name than `e` here to give some 
context (`r` or `resource`?).



src/main/java/org/apache/aurora/scheduler/resources/ResourceManager.java (line 
67)


Same here and below?

(I notice this is fairly common, presumably `e` is short for "element" in 
these cases, but I find that giving it a more contextual name helps grok what's 
going on in the absence of explicit types, feel free to punt on this though if 
it's a major pain ;)).



src/main/java/org/apache/aurora/scheduler/resources/ResourceMapper.java (line 
28)


nit: just import `Protos.Offer`?



src/main/java/org/apache/aurora/scheduler/state/StateManager.java (line 18)


Use `java.util.Function`?



src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java (line 111)


this should be passing `assigned` to `mapAndAssign` not `task`, right? In 
case there are multiple resource types with mappers we don't want to discard 
any changes to the task from previous mappers.


- Joshua Cohen


On April 29, 2016, 1:08 a.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46810/
> ---
> 
> (Updated April 29, 2016, 1:08 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Bill Farner, and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This patch effectively gets rid of `ITaskConfig.getRequestedPorts()` usage. A 
> few implementation notes:
> - `ResourceType` gets an optional `ResourceMapper` property applicable to any 
> resources types that require mapping and assignment of matched values
> - All port specific matching/assigning logic is now encapsulated within 
> `PortMapper` implementing `ResourceMapper` interface
> - `TaskAssigner` now passes a callback into the `StateManager` to map and 
> assign port values
> - The newly introduced `ResourceManager` will eventually replace both 
> `Resources` and `ResourceSlot` classes.
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  9a15a4b5c1cdc3efbf0588e8bd81636b66e938ef 
>   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 
> 625e6d5e1db1c7327154823f5a604efd3a4ff690 
>   src/main/java/org/apache/aurora/scheduler/resources/ResourceManager.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/resources/ResourceMapper.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/resources/ResourceSlot.java 
> 1df2c11a84db8d46158c87e7c3709fc08016a107 
>   src/main/java/org/apache/aurora/scheduler/resources/ResourceType.java 
> 6e4d694cc6449a211297919d7834996382b21b21 
>   src/main/java/org/apache/aurora/scheduler/resources/Resources.java 
> 894c6a66545ac336d594d83c595ee83b7c91e21d 
>   src/main/java/org/apache/aurora/scheduler/state/StateManager.java 
> 5d34fe38492cae3f50ea9ed0baca11472295af60 
>   src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java 
> e5b2f41f55aec161840c3fb17f2ff73161b77482 
>   src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 
> 7d43d4ad2de60f76183c91b860b00e7022d79bcb 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  731327950099f2567430f3af75633854910443ff 
>   
> src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java
>  6370a126340dd4312ae0f2da52fee6ccdbced595 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> bf18d5d53f7eda62120299146a956fa0a0985f71 
>   src/test/java/org/apache/aurora/scheduler/resources/AcceptedOfferTest.java 
> d5f2172131ce4b765a1f85d63aeddf1daaa0b606 
>   src/test/java/org/apache/aurora/scheduler/resources/PortMapperTest.java 
> PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/resources/ResourceManagerTest.java 
> PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/resources/ResourceTestUtil.java 
> 04a82382e8535e1a233d48370bd0650f5abcd787 
>   src/test/java/org/apache/aurora/scheduler/resources/ResourcesTest.java 
> 

Re: Review Request 46803: AURORA-1458: Add tier into the UI "show config" summary.

2016-04-28 Thread Joshua Cohen


> On April 29, 2016, 1:20 a.m., Maxim Khutornenko wrote:
> > src/main/resources/scheduler/assets/js/controllers.js, line 139
> > 
> >
> > I am not too keen on having a column for an optional field that will 
> > most likely read 'default' for all jobs. Perhaps postpone its introduction 
> > until TaskConfig.tier is required?
> 
> Amol Deshmukh wrote:
> I thought that leaving it blank would raise more questions for most users 
> who did not use tiers at all (basically the same point that Joshua raised 
> above).
> 
> I'll leave it blank and we can address that if the need arises.

I'd be ok with just not shipping this change until TaskConfig.tier is required 
and users are actually aware of it.


- Joshua


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


On April 29, 2016, 2:54 a.m., Amol Deshmukh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46803/
> ---
> 
> (Updated April 29, 2016, 2:54 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> AURORA-1458: Add tier into the UI "show config" summary.
> 
> 
> Diffs
> -
> 
>   src/main/resources/scheduler/assets/configSummary.html 
> 1af7511de0e8a143c8ea88377aad756b44e3ac30 
>   src/main/resources/scheduler/assets/js/controllers.js 
> 84417ebeadfae57d55b9f12e8a985825bd620fc8 
>   src/main/resources/scheduler/assets/js/services.js 
> d9ce52065f9573b0aa68a95da7da7c50fb14310a 
>   src/main/resources/scheduler/assets/schedulingDetail.html 
> eb88c1e6dec7a26643e8b13ffcf8e90df70a67f7 
> 
> Diff: https://reviews.apache.org/r/46803/diff/
> 
> 
> Testing
> ---
> 
> Ensured the changes appear in the UI after launching the scheduler using 
> vagrant.
> 
> 
> File Attachments
> 
> 
> Jobs by Role
>   
> https://reviews.apache.org/media/uploaded/files/2016/04/29/6e58d26d-e77c-49fe-85e0-ee1acae3efe0__Jobs_by_Role.png
> Per Job Config Summary
>   
> https://reviews.apache.org/media/uploaded/files/2016/04/29/8a666fe7-9a35-427f-b1ee-f41e5413059d__Per_Job_Config_Summary.png
> 
> 
> Thanks,
> 
> Amol Deshmukh
> 
>



Re: Review Request 46803: AURORA-1458: Add tier into the UI "show config" summary.

2016-04-28 Thread Bill Farner


> On April 28, 2016, 6:20 p.m., Maxim Khutornenko wrote:
> > src/main/resources/scheduler/assets/js/controllers.js, line 139
> > 
> >
> > I am not too keen on having a column for an optional field that will 
> > most likely read 'default' for all jobs. Perhaps postpone its introduction 
> > until TaskConfig.tier is required?
> 
> Amol Deshmukh wrote:
> I thought that leaving it blank would raise more questions for most users 
> who did not use tiers at all (basically the same point that Joshua raised 
> above).
> 
> I'll leave it blank and we can address that if the need arises.
> 
> Joshua Cohen wrote:
> I'd be ok with just not shipping this change until TaskConfig.tier is 
> required and users are actually aware of it.

I thought we had concluded that tier would not become a required field in user 
configurations?


- Bill


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


On April 28, 2016, 7:54 p.m., Amol Deshmukh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46803/
> ---
> 
> (Updated April 28, 2016, 7:54 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> AURORA-1458: Add tier into the UI "show config" summary.
> 
> 
> Diffs
> -
> 
>   src/main/resources/scheduler/assets/configSummary.html 
> 1af7511de0e8a143c8ea88377aad756b44e3ac30 
>   src/main/resources/scheduler/assets/js/controllers.js 
> 84417ebeadfae57d55b9f12e8a985825bd620fc8 
>   src/main/resources/scheduler/assets/js/services.js 
> d9ce52065f9573b0aa68a95da7da7c50fb14310a 
>   src/main/resources/scheduler/assets/schedulingDetail.html 
> eb88c1e6dec7a26643e8b13ffcf8e90df70a67f7 
> 
> Diff: https://reviews.apache.org/r/46803/diff/
> 
> 
> Testing
> ---
> 
> Ensured the changes appear in the UI after launching the scheduler using 
> vagrant.
> 
> 
> File Attachments
> 
> 
> Jobs by Role
>   
> https://reviews.apache.org/media/uploaded/files/2016/04/29/6e58d26d-e77c-49fe-85e0-ee1acae3efe0__Jobs_by_Role.png
> Per Job Config Summary
>   
> https://reviews.apache.org/media/uploaded/files/2016/04/29/8a666fe7-9a35-427f-b1ee-f41e5413059d__Per_Job_Config_Summary.png
> 
> 
> Thanks,
> 
> Amol Deshmukh
> 
>