You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Renan DelValle <rd...@binghamton.edu> on 2017/02/01 20:44:37 UTC
Re: Review Request 52437: Adding support for Ubuntu Xenial packages
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52437/
-----------------------------------------------------------
(Updated Feb. 1, 2017, 12:44 p.m.)
Review request for Aurora, Dmitriy Shirchenko and Zameer Manji.
Changes
-------
Merged ubuntu-xenial spec into debian spec.
Bugs: AURORA-1872
https://issues.apache.org/jira/browse/AURORA-1872
Repository: aurora-packaging
Description (updated)
-------
Added builder and test environment for Xenial as well as updated instructions on how to test it.
Added distribution to release-candidate script.
Diffs (updated)
-----
build-support/release/release-candidate b3ebe916102d0a6ef2c1997b6c4110008eafff1a
builder/deb/debian-jessie/Dockerfile 54cb2e072a0d465840899e00e541cec34130993d
builder/deb/debian-jessie/build.sh e0aeaf52a29483e65172f29792638dfbca100079
builder/deb/ubuntu-trusty/Dockerfile 802049b6b2a53666c153525f2fb97bfbf0d4bab4
builder/deb/ubuntu-trusty/build.sh e0aeaf52a29483e65172f29792638dfbca100079
builder/deb/ubuntu-xenial/Dockerfile PRE-CREATION
builder/deb/ubuntu-xenial/build.sh PRE-CREATION
specs/debian/aurora-executor.thermos.default 82ab0c8ab585acf253f8fecc0ed88a3e19511ffb
specs/debian/aurora-executor.thermos.service da351164298bd2b5a1802a945211647c99193ae6
specs/debian/aurora-pants.ini e4f2c54df45f746ffb2a4e3f786fc4105323bc0b
specs/debian/aurora-scheduler.service ae33d26ac10518985eacea6bf015f81a02c53f66
specs/debian/aurora-scheduler.startup.sh PRE-CREATION
specs/debian/rules 4effc3ecf3208f82986d15f6000d18abc11652b1
test/deb/ubuntu-xenial/README.md PRE-CREATION
test/deb/ubuntu-xenial/Vagrantfile PRE-CREATION
test/deb/ubuntu-xenial/provision.sh PRE-CREATION
Diff: https://reviews.apache.org/r/52437/diff/
Testing (updated)
-------
Created artifacts using the build-artifacts script.
Brought a vagrant image up, installed all deb files created by the artifacts script, started both aurora-scheduler and thermos services, and launched a sample job.
Made sure all debian systems were unaffected by the changes by bringing a Vagrant image up for each distribution and installing the packages.
Thanks,
Renan DelValle
Re: Review Request 52437: Adding support for Ubuntu Xenial packages
Posted by Stephan Erb <se...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52437/#review164643
-----------------------------------------------------------
Ship it!
Ship It!
- Stephan Erb
On Feb. 8, 2017, 1:48 a.m., Renan DelValle wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52437/
> -----------------------------------------------------------
>
> (Updated Feb. 8, 2017, 1:48 a.m.)
>
>
> Review request for Aurora, Dmitriy Shirchenko and Zameer Manji.
>
>
> Bugs: AURORA-1872
> https://issues.apache.org/jira/browse/AURORA-1872
>
>
> Repository: aurora-packaging
>
>
> Description
> -------
>
> Added builder and test environment for Xenial as well as updated instructions on how to test it.
>
> Added distribution to release-candidate script.
>
>
> Diffs
> -----
>
> build-support/release/release-candidate b3ebe916102d0a6ef2c1997b6c4110008eafff1a
> builder/deb/debian-jessie/Dockerfile 54cb2e072a0d465840899e00e541cec34130993d
> builder/deb/debian-jessie/build.sh e0aeaf52a29483e65172f29792638dfbca100079
> builder/deb/ubuntu-trusty/Dockerfile 802049b6b2a53666c153525f2fb97bfbf0d4bab4
> builder/deb/ubuntu-trusty/build.sh e0aeaf52a29483e65172f29792638dfbca100079
> builder/deb/ubuntu-xenial/Dockerfile PRE-CREATION
> builder/deb/ubuntu-xenial/build.sh PRE-CREATION
> specs/debian/aurora-executor.thermos.default 82ab0c8ab585acf253f8fecc0ed88a3e19511ffb
> specs/debian/aurora-executor.thermos.service da351164298bd2b5a1802a945211647c99193ae6
> specs/debian/aurora-pants.ini e4f2c54df45f746ffb2a4e3f786fc4105323bc0b
> specs/debian/aurora-scheduler.service ae33d26ac10518985eacea6bf015f81a02c53f66
> specs/debian/aurora-scheduler.startup.sh PRE-CREATION
> specs/debian/rules 4effc3ecf3208f82986d15f6000d18abc11652b1
> test/deb/ubuntu-xenial/README.md PRE-CREATION
> test/deb/ubuntu-xenial/Vagrantfile PRE-CREATION
> test/deb/ubuntu-xenial/provision.sh PRE-CREATION
>
> Diff: https://reviews.apache.org/r/52437/diff/
>
>
> Testing
> -------
>
> Created artifacts using the build-artifacts script.
>
> Brought a vagrant image up, installed all deb files created by the artifacts script, started both aurora-scheduler and thermos services, and launched a sample job.
>
> Made sure all debian systems were unaffected by the changes by bringing a Vagrant image up for each distribution and installing the packages.
>
>
> Thanks,
>
> Renan DelValle
>
>
Re: Review Request 52437: Adding support for Ubuntu Xenial packages
Posted by Renan DelValle <rd...@binghamton.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52437/
-----------------------------------------------------------
(Updated Feb. 7, 2017, 4:48 p.m.)
Review request for Aurora, Dmitriy Shirchenko and Zameer Manji.
Changes
-------
Moved python repos header for aurora-pants into the bash script for all debian builders. Removed Envirnoment values set in executor unit file and made EnvironmentFile error out if the file inside of /etc/default/aurora-scheduler is not found.
Bugs: AURORA-1872
https://issues.apache.org/jira/browse/AURORA-1872
Repository: aurora-packaging
Description
-------
Added builder and test environment for Xenial as well as updated instructions on how to test it.
Added distribution to release-candidate script.
Diffs (updated)
-----
build-support/release/release-candidate b3ebe916102d0a6ef2c1997b6c4110008eafff1a
builder/deb/debian-jessie/Dockerfile 54cb2e072a0d465840899e00e541cec34130993d
builder/deb/debian-jessie/build.sh e0aeaf52a29483e65172f29792638dfbca100079
builder/deb/ubuntu-trusty/Dockerfile 802049b6b2a53666c153525f2fb97bfbf0d4bab4
builder/deb/ubuntu-trusty/build.sh e0aeaf52a29483e65172f29792638dfbca100079
builder/deb/ubuntu-xenial/Dockerfile PRE-CREATION
builder/deb/ubuntu-xenial/build.sh PRE-CREATION
specs/debian/aurora-executor.thermos.default 82ab0c8ab585acf253f8fecc0ed88a3e19511ffb
specs/debian/aurora-executor.thermos.service da351164298bd2b5a1802a945211647c99193ae6
specs/debian/aurora-pants.ini e4f2c54df45f746ffb2a4e3f786fc4105323bc0b
specs/debian/aurora-scheduler.service ae33d26ac10518985eacea6bf015f81a02c53f66
specs/debian/aurora-scheduler.startup.sh PRE-CREATION
specs/debian/rules 4effc3ecf3208f82986d15f6000d18abc11652b1
test/deb/ubuntu-xenial/README.md PRE-CREATION
test/deb/ubuntu-xenial/Vagrantfile PRE-CREATION
test/deb/ubuntu-xenial/provision.sh PRE-CREATION
Diff: https://reviews.apache.org/r/52437/diff/
Testing
-------
Created artifacts using the build-artifacts script.
Brought a vagrant image up, installed all deb files created by the artifacts script, started both aurora-scheduler and thermos services, and launched a sample job.
Made sure all debian systems were unaffected by the changes by bringing a Vagrant image up for each distribution and installing the packages.
Thanks,
Renan DelValle
Re: Review Request 52437: Adding support for Ubuntu Xenial packages
Posted by Renan DelValle <rd...@binghamton.edu>.
> On Feb. 6, 2017, 1:01 p.m., Stephan Erb wrote:
> >
Thanks for the review!
> On Feb. 6, 2017, 1:01 p.m., Stephan Erb wrote:
> > builder/deb/debian-jessie/build.sh, line 28
> > <https://reviews.apache.org/r/52437/diff/4/?file=1621611#file1621611line28>
> >
> > That indentation seems unnecessary.
Fixed!
> On Feb. 6, 2017, 1:01 p.m., Stephan Erb wrote:
> > builder/deb/debian-jessie/build.sh, lines 30-31
> > <https://reviews.apache.org/r/52437/diff/4/?file=1621611#file1621611line30>
> >
> > It is more robust if we inject the whole section here (including the section header).
Yep, that makes sense. I'll go ahead and fix that.
> On Feb. 6, 2017, 1:01 p.m., Stephan Erb wrote:
> > specs/debian/aurora-executor.thermos.service, lines 22-23
> > <https://reviews.apache.org/r/52437/diff/4/?file=1621617#file1621617line22>
> >
> > Can the corresponding init scripts be dropped?
We can definitely drop them but we'd lose (unofficial) compatibility for those who chose to init sysv init system (debian wheezy).
> On Feb. 6, 2017, 1:01 p.m., Stephan Erb wrote:
> > specs/debian/aurora-executor.thermos.service, lines 19-21
> > <https://reviews.apache.org/r/52437/diff/4/?file=1621617#file1621617line19>
> >
> > Why have you specified both `Environment` and `EnvironmentFile`. I suppose the latter has precedence?
Great catch, I'd forgotten I'd done this. Yup, EnvironmentFile has preedence. In this case EnvironmentFile also has a negative sign after the equal sign to ignore errors (e.g. file doesn't exist). I was experimenting with having a fallback option if the config file wasn't found, but since the deb package places one there, we can assume it'll be there. I'll get rid of lines 19 and 20.
- Renan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52437/#review164402
-----------------------------------------------------------
On Feb. 1, 2017, 12:44 p.m., Renan DelValle wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52437/
> -----------------------------------------------------------
>
> (Updated Feb. 1, 2017, 12:44 p.m.)
>
>
> Review request for Aurora, Dmitriy Shirchenko and Zameer Manji.
>
>
> Bugs: AURORA-1872
> https://issues.apache.org/jira/browse/AURORA-1872
>
>
> Repository: aurora-packaging
>
>
> Description
> -------
>
> Added builder and test environment for Xenial as well as updated instructions on how to test it.
>
> Added distribution to release-candidate script.
>
>
> Diffs
> -----
>
> build-support/release/release-candidate b3ebe916102d0a6ef2c1997b6c4110008eafff1a
> builder/deb/debian-jessie/Dockerfile 54cb2e072a0d465840899e00e541cec34130993d
> builder/deb/debian-jessie/build.sh e0aeaf52a29483e65172f29792638dfbca100079
> builder/deb/ubuntu-trusty/Dockerfile 802049b6b2a53666c153525f2fb97bfbf0d4bab4
> builder/deb/ubuntu-trusty/build.sh e0aeaf52a29483e65172f29792638dfbca100079
> builder/deb/ubuntu-xenial/Dockerfile PRE-CREATION
> builder/deb/ubuntu-xenial/build.sh PRE-CREATION
> specs/debian/aurora-executor.thermos.default 82ab0c8ab585acf253f8fecc0ed88a3e19511ffb
> specs/debian/aurora-executor.thermos.service da351164298bd2b5a1802a945211647c99193ae6
> specs/debian/aurora-pants.ini e4f2c54df45f746ffb2a4e3f786fc4105323bc0b
> specs/debian/aurora-scheduler.service ae33d26ac10518985eacea6bf015f81a02c53f66
> specs/debian/aurora-scheduler.startup.sh PRE-CREATION
> specs/debian/rules 4effc3ecf3208f82986d15f6000d18abc11652b1
> test/deb/ubuntu-xenial/README.md PRE-CREATION
> test/deb/ubuntu-xenial/Vagrantfile PRE-CREATION
> test/deb/ubuntu-xenial/provision.sh PRE-CREATION
>
> Diff: https://reviews.apache.org/r/52437/diff/
>
>
> Testing
> -------
>
> Created artifacts using the build-artifacts script.
>
> Brought a vagrant image up, installed all deb files created by the artifacts script, started both aurora-scheduler and thermos services, and launched a sample job.
>
> Made sure all debian systems were unaffected by the changes by bringing a Vagrant image up for each distribution and installing the packages.
>
>
> Thanks,
>
> Renan DelValle
>
>
Re: Review Request 52437: Adding support for Ubuntu Xenial packages
Posted by Stephan Erb <se...@apache.org>.
> On Feb. 6, 2017, 10:01 p.m., Stephan Erb wrote:
> > specs/debian/aurora-executor.thermos.service, lines 22-23
> > <https://reviews.apache.org/r/52437/diff/4/?file=1621617#file1621617line22>
> >
> > Can the corresponding init scripts be dropped?
>
> Renan DelValle wrote:
> We can definitely drop them but we'd lose (unofficial) compatibility for those who chose to init sysv init system (debian wheezy).
Let's keep them for one more version then.
- Stephan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52437/#review164402
-----------------------------------------------------------
On Feb. 1, 2017, 9:44 p.m., Renan DelValle wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52437/
> -----------------------------------------------------------
>
> (Updated Feb. 1, 2017, 9:44 p.m.)
>
>
> Review request for Aurora, Dmitriy Shirchenko and Zameer Manji.
>
>
> Bugs: AURORA-1872
> https://issues.apache.org/jira/browse/AURORA-1872
>
>
> Repository: aurora-packaging
>
>
> Description
> -------
>
> Added builder and test environment for Xenial as well as updated instructions on how to test it.
>
> Added distribution to release-candidate script.
>
>
> Diffs
> -----
>
> build-support/release/release-candidate b3ebe916102d0a6ef2c1997b6c4110008eafff1a
> builder/deb/debian-jessie/Dockerfile 54cb2e072a0d465840899e00e541cec34130993d
> builder/deb/debian-jessie/build.sh e0aeaf52a29483e65172f29792638dfbca100079
> builder/deb/ubuntu-trusty/Dockerfile 802049b6b2a53666c153525f2fb97bfbf0d4bab4
> builder/deb/ubuntu-trusty/build.sh e0aeaf52a29483e65172f29792638dfbca100079
> builder/deb/ubuntu-xenial/Dockerfile PRE-CREATION
> builder/deb/ubuntu-xenial/build.sh PRE-CREATION
> specs/debian/aurora-executor.thermos.default 82ab0c8ab585acf253f8fecc0ed88a3e19511ffb
> specs/debian/aurora-executor.thermos.service da351164298bd2b5a1802a945211647c99193ae6
> specs/debian/aurora-pants.ini e4f2c54df45f746ffb2a4e3f786fc4105323bc0b
> specs/debian/aurora-scheduler.service ae33d26ac10518985eacea6bf015f81a02c53f66
> specs/debian/aurora-scheduler.startup.sh PRE-CREATION
> specs/debian/rules 4effc3ecf3208f82986d15f6000d18abc11652b1
> test/deb/ubuntu-xenial/README.md PRE-CREATION
> test/deb/ubuntu-xenial/Vagrantfile PRE-CREATION
> test/deb/ubuntu-xenial/provision.sh PRE-CREATION
>
> Diff: https://reviews.apache.org/r/52437/diff/
>
>
> Testing
> -------
>
> Created artifacts using the build-artifacts script.
>
> Brought a vagrant image up, installed all deb files created by the artifacts script, started both aurora-scheduler and thermos services, and launched a sample job.
>
> Made sure all debian systems were unaffected by the changes by bringing a Vagrant image up for each distribution and installing the packages.
>
>
> Thanks,
>
> Renan DelValle
>
>
Re: Review Request 52437: Adding support for Ubuntu Xenial packages
Posted by Stephan Erb <se...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52437/#review164402
-----------------------------------------------------------
builder/deb/debian-jessie/build.sh (line 28)
<https://reviews.apache.org/r/52437/#comment236093>
That indentation seems unnecessary.
builder/deb/debian-jessie/build.sh (lines 30 - 31)
<https://reviews.apache.org/r/52437/#comment236094>
It is more robust if we inject the whole section here (including the section header).
specs/debian/aurora-executor.thermos.service (lines 19 - 21)
<https://reviews.apache.org/r/52437/#comment236095>
Why have you specified both `Environment` and `EnvironmentFile`. I suppose the latter has precedence?
specs/debian/aurora-executor.thermos.service (lines 22 - 23)
<https://reviews.apache.org/r/52437/#comment236098>
Can the corresponding init scripts be dropped?
specs/debian/aurora-scheduler.service
<https://reviews.apache.org/r/52437/#comment236097>
Same here, can the corresponding init scripts be dropped?
- Stephan Erb
On Feb. 1, 2017, 9:44 p.m., Renan DelValle wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52437/
> -----------------------------------------------------------
>
> (Updated Feb. 1, 2017, 9:44 p.m.)
>
>
> Review request for Aurora, Dmitriy Shirchenko and Zameer Manji.
>
>
> Bugs: AURORA-1872
> https://issues.apache.org/jira/browse/AURORA-1872
>
>
> Repository: aurora-packaging
>
>
> Description
> -------
>
> Added builder and test environment for Xenial as well as updated instructions on how to test it.
>
> Added distribution to release-candidate script.
>
>
> Diffs
> -----
>
> build-support/release/release-candidate b3ebe916102d0a6ef2c1997b6c4110008eafff1a
> builder/deb/debian-jessie/Dockerfile 54cb2e072a0d465840899e00e541cec34130993d
> builder/deb/debian-jessie/build.sh e0aeaf52a29483e65172f29792638dfbca100079
> builder/deb/ubuntu-trusty/Dockerfile 802049b6b2a53666c153525f2fb97bfbf0d4bab4
> builder/deb/ubuntu-trusty/build.sh e0aeaf52a29483e65172f29792638dfbca100079
> builder/deb/ubuntu-xenial/Dockerfile PRE-CREATION
> builder/deb/ubuntu-xenial/build.sh PRE-CREATION
> specs/debian/aurora-executor.thermos.default 82ab0c8ab585acf253f8fecc0ed88a3e19511ffb
> specs/debian/aurora-executor.thermos.service da351164298bd2b5a1802a945211647c99193ae6
> specs/debian/aurora-pants.ini e4f2c54df45f746ffb2a4e3f786fc4105323bc0b
> specs/debian/aurora-scheduler.service ae33d26ac10518985eacea6bf015f81a02c53f66
> specs/debian/aurora-scheduler.startup.sh PRE-CREATION
> specs/debian/rules 4effc3ecf3208f82986d15f6000d18abc11652b1
> test/deb/ubuntu-xenial/README.md PRE-CREATION
> test/deb/ubuntu-xenial/Vagrantfile PRE-CREATION
> test/deb/ubuntu-xenial/provision.sh PRE-CREATION
>
> Diff: https://reviews.apache.org/r/52437/diff/
>
>
> Testing
> -------
>
> Created artifacts using the build-artifacts script.
>
> Brought a vagrant image up, installed all deb files created by the artifacts script, started both aurora-scheduler and thermos services, and launched a sample job.
>
> Made sure all debian systems were unaffected by the changes by bringing a Vagrant image up for each distribution and installing the packages.
>
>
> Thanks,
>
> Renan DelValle
>
>