Juju Charms Collection
ubuntu package

Merge lp://qastaging/~cbjchen/charms/trusty/ubuntu/lxc-network-config into lp://qastaging/charms/trusty/ubuntu

Proposed by Liang Chen on 2015-02-23

Status:

Merged

Merged at revision:

Proposed branch:

lp://qastaging/~cbjchen/charms/trusty/ubuntu/lxc-network-config

Merge into:

lp://qastaging/charms/trusty/ubuntu

Diff against target:

3472 lines (+3356/-0)

22 files modified

charm-helpers-hooks.yaml (+5/-0)
config.yaml (+8/-0)
hooks/charmhelpers/core/__init__.py (+15/-0)
hooks/charmhelpers/core/decorators.py (+57/-0)
hooks/charmhelpers/core/fstab.py (+134/-0)
hooks/charmhelpers/core/hookenv.py (+568/-0)
hooks/charmhelpers/core/host.py (+446/-0)
hooks/charmhelpers/core/services/__init__.py (+18/-0)
hooks/charmhelpers/core/services/base.py (+329/-0)
hooks/charmhelpers/core/services/helpers.py (+259/-0)
hooks/charmhelpers/core/strutils.py (+42/-0)
hooks/charmhelpers/core/sysctl.py (+56/-0)
hooks/charmhelpers/core/templating.py (+68/-0)
hooks/charmhelpers/core/unitdata.py (+477/-0)
hooks/charmhelpers/fetch/__init__.py (+439/-0)
hooks/charmhelpers/fetch/archiveurl.py (+161/-0)
hooks/charmhelpers/fetch/bzrurl.py (+78/-0)
hooks/charmhelpers/fetch/giturl.py (+71/-0)
hooks/config-changed (+43/-0)
hooks/hooks.py (+45/-0)
hooks/utils.py (+27/-0)
templates/lxc-bridge.conf (+10/-0)

To merge this branch:

bzr merge lp://qastaging/~cbjchen/charms/trusty/ubuntu/lxc-network-config

Undecided

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
Charles Butler (community)		2015-02-23	Approve on 2015-03-27
Antonio Rosales (community)	community		Approve on 2015-03-24
Adam Israel (community)			Needs Fixing on 2015-03-24
Review Queue (community)	automated testing		Needs Fixing on 2015-03-03
Review via email: mp+250666@code.qastaging.launchpad.net

Description of the change

This patch provides a config option for the ubuntu
charm, so that users can choose whether to create
a different subnet or just bridge to the exist
network that the ubuntu instance is in for lxc
containers running in the ubuntu instance.

Revision history for this message

Review Queue (review-queue) wrote on 2015-03-03:

This items has failed automated testing! Results available here http://reports.vapour.ws/charm-tests/charm-bundle-test-11062-results

review: Needs Fixing (automated testing)

Revision history for this message

Adam Israel (aisrael) wrote on 2015-03-24:

Hi Liang,

Thanks for your work on improving this charm's functionality. It's passing tests for me, but it would be really useful to update the README to reflect the new config option and how to use it. Once that is done, I'm happy to retest.

review: Needs Fixing

Revision history for this message

Antonio Rosales (arosales) wrote on 2015-03-24:

The failed automated testing was due to HP Cloud not standing up in time[0] all other tests on the other substrates passed:

[0] http://reports.vapour.ws/all-bundle-and-charm-results/charm-bundle-test-11062-results/charm/charm-testing-hp/2

I'll work on a branch to update the README.md per Adam's comments.

-thanks,
Antonio

review: Approve (community)

Revision history for this message

Antonio Rosales (arosales) wrote on 2015-03-25:

I have an MP against this branch to:
  -Add documenetation to the readme about the new new-lxc-network network option for LXC.
  -Add Amulet tests to ensure the new-lxc-network config option is operating correctly.
  -Update the default to be false for new-lxc-network. Reason is the goal of the Ubuntu charm is to be a very simple charm, and is a good indicator juju functions are working correctly.

-thanks,
Antonio

Revision history for this message

Antonio Rosales (arosales) wrote on 2015-03-25:

For reference that MP is at:
https://code.launchpad.net/~a.rosales/charms/trusty/ubuntu/update-readme-test-for-lxc-network/+merge/254147

-thanks,
Antonio

Revision history for this message

Liang Chen (cbjchen) wrote on 2015-03-26:

Hi Adam and Antonio,

Thanks for the review, and thanks for the help to supply the the documentation and tests. It makes sense to me to set the default value to false. I was just trying to make sure the behavior is the same as the previous version by default. I took a look at the patch, and it looks good to me.

Revision history for this message

Charles Butler (lazypower) wrote on 2015-03-27:

Liang,

Thank you for the contribution to the Ubuntu charm. I've taken some time to review your changes, and the follow up documentation + test from Antonio.

I've taken the liberty of merging his additions to your branch, and have pushed them to the store.

Thank you for the high quality submission, tests, and documentation. This is exactly the type of merge that makes me happy to perform a final review on. Keep up the great work!

review: Approve