Juju Charms Collection
odl-controller package

Merge lp://staging/~ack/charms/trusty/odl-controller/odl-package-from-repo into lp://staging/~sdn-charmers/charms/trusty/odl-controller/trunk

  1. Trusty Tahr (14.04)
  2. odl-package-from-repo
  3. Merge into trunk
Proposed by Alberto Donato
Status: Merged
Merged at revision: 10
Proposed branch: lp://staging/~ack/charms/trusty/odl-controller/odl-package-from-repo
Merge into: lp://staging/~sdn-charmers/charms/trusty/odl-controller/trunk
Diff against target: 90 lines (+36/-12)
2 files modified
config.yaml (+13/-1)
hooks/odl_controller_hooks.py (+23/-11)
To merge this branch: bzr merge lp://staging/~ack/charms/trusty/odl-controller/odl-package-from-repo
Reviewer Review Type Date Requested Status
Robert Ayres (community) Approve
Chris Glass Pending
Review via email: mp+276421@code.staging.launchpad.net

Description of the change

This adds an "install-origin" option to the charm to allow installing the distribution-karaf tarball via package, through the specified repo.

If the option is not specified (as by default), the "install-url" is used instead.

I built a sample package here: https://launchpad.net/~ack/+archive/ubuntu/odl-test
so a deploy can be tested with the following config:

   install-origin: ppa:ack/odl-test

To post a comment you must log in.
Revision history for this message
Robert Ayres (robert-ayres) wrote :

Minor points:

*I think 'install-sources' is better than 'install-origin' and is consistent with other charms.

*Using 'configure_sources(True, "install-sources")' is better than just 'add_source(install_origin)' so that you can use a list of PPAs or other methods for installation.

*Remove 'install-origin' empty default.

*I think the logic of the install hook could be cleaner. It may be easier to express as:

if config.get("install-origin"):
    # configure sources

# install dependencies
apt_install(PACKAGES)

url = config.get("install-url")
if url:
    # install opendaylight from url
else:
    # install opendaylight using apt

Then 'install-origin' is more like a apt repo override and 'install-url' a package override where both options can be used with or without each other.

Revision history for this message
Robert Ayres (robert-ayres) :
review: Needs Fixing
lp://staging/~ack/charms/trusty/odl-controller/odl-package-from-repo updated
12. By Alberto Donato

Address review comments.

13. By Alberto Donato

Fix call.

14. By Alberto Donato

Handle empty case.

Revision history for this message
Alberto Donato (ack) wrote :

> Minor points:
>
> *I think 'install-sources' is better than 'install-origin' and is consistent
> with other charms.
>
> *Using 'configure_sources(True, "install-sources")' is better than just
> 'add_source(install_origin)' so that you can use a list of PPAs or other
> methods for installation.
>
> *Remove 'install-origin' empty default.
>
> *I think the logic of the install hook could be cleaner. It may be easier to
> express as:
>
> if config.get("install-origin"):
> # configure sources
>
> # install dependencies
> apt_install(PACKAGES)
>
> url = config.get("install-url")
> if url:
> # install opendaylight from url
> else:
> # install opendaylight using apt
>
> Then 'install-origin' is more like a apt repo override and 'install-url' a
> package override where both options can be used with or without each other.

Thanks for the review. All points addressed as suggested.

Revision history for this message
Robert Ayres (robert-ayres) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: