Merge lp://staging/~jason-hobbs/charms/trusty/cinder/force-vgreduce into lp://staging/~openstack-charmers-archive/charms/trusty/cinder/next

Proposed by Jason Hobbs
Status: Merged
Approved by: Billy Olsen
Approved revision: no longer in the source branch.
Merged at revision: 108
Proposed branch: lp://staging/~jason-hobbs/charms/trusty/cinder/force-vgreduce
Merge into: lp://staging/~openstack-charmers-archive/charms/trusty/cinder/next
Prerequisite: lp://staging/~jason-hobbs/charms/trusty/cinder/log-lvm-info
Diff against target: 165 lines (+64/-7)
5 files modified
config.yaml (+7/-0)
hooks/cinder_hooks.py (+2/-1)
hooks/cinder_utils.py (+14/-4)
unit_tests/test_cinder_hooks.py (+17/-2)
unit_tests/test_cinder_utils.py (+24/-0)
To merge this branch: bzr merge lp://staging/~jason-hobbs/charms/trusty/cinder/force-vgreduce
Reviewer Review Type Date Requested Status
Billy Olsen Approve
Corey Bryant Approve
Review via email: mp+265448@code.staging.launchpad.net

Commit message

Add 'remove-missing-force' config option.

To post a comment you must log in.
Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

charm_lint_check #6566 cinder-next for jason-hobbs mp265448
    LINT FAIL: lint-test failed

LINT Results (max last 2 lines):
make: *** [lint] Error 1
ERROR:root:Make target returned non-zero.

Full lint test output: http://paste.ubuntu.com/11916404/
Build: http://10.245.162.77:8080/job/charm_lint_check/6566/

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

charm_unit_test #6198 cinder-next for jason-hobbs mp265448
    UNIT OK: passed

Build: http://10.245.162.77:8080/job/charm_unit_test/6198/

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

charm_amulet_test #5243 cinder-next for jason-hobbs mp265448
    AMULET OK: passed

Build: http://10.245.162.77:8080/job/charm_amulet_test/5243/

Revision history for this message
Billy Olsen (billy-olsen) wrote :

Jason, thanks for the code submission! There were some lint errors which need to be corrected and I've also added a few comments inline.

review: Needs Fixing
Revision history for this message
Corey Bryant (corey.bryant) wrote :

Hey Jason, One comment from me below.

107. By Corey Bryant

[jason-hobbs,r=corey.bryant] Add LVM logging when configuring LVM.

Revision history for this message
Jason Hobbs (jason-hobbs) wrote :

> Jason, thanks for the code submission! There were some lint errors which need
> to be corrected and I've also added a few comments inline.

Thanks for the review. I fixed up the lint issues and got rid of the force_reduce_lvm_volume_group_missing(). reduce_lvm_volume_group_missing() now takes an optional list of extra args to pass to vgreduce.

Revision history for this message
Jason Hobbs (jason-hobbs) wrote :

> Hey Jason, One comment from me below.

Thanks Corey. I changed it so that force-remove-missing overrides remove-missing.

Revision history for this message
Jason Hobbs (jason-hobbs) wrote :

All of the review comments have been addressed - this is ready for another look.

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

charm_lint_check #6634 cinder-next for jason-hobbs mp265448
    LINT OK: passed

Build: http://10.245.162.77:8080/job/charm_lint_check/6634/

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

charm_unit_test #6261 cinder-next for jason-hobbs mp265448
    UNIT OK: passed

Build: http://10.245.162.77:8080/job/charm_unit_test/6261/

Revision history for this message
Corey Bryant (corey.bryant) wrote :

LGTM. Just want to wait on amulet results to be posted.

review: Approve
Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

charm_amulet_test #5264 cinder-next for jason-hobbs mp265448
    AMULET OK: passed

Build: http://10.245.162.77:8080/job/charm_amulet_test/5264/

Revision history for this message
Corey Bryant (corey.bryant) wrote :

Ok Amulet passes. Billy, look ok to you?

Revision history for this message
Billy Olsen (billy-olsen) wrote :

Comment below, not a big enough of an issue to hold up this MP, but I'd like to revisit this in the near future.

review: Approve
108. By Billy Olsen <wolsen@blackwell>

[jason-hobbs,r=corecb,billy-olsen] Add remove-missing-force config option.

Adds an option to force remove physical volumes from a volume group even
when a logical volume is allocated on top of the physical volume.

Revision history for this message
Jason Hobbs (jason-hobbs) wrote :

Thanks Billy - I responded to your comment inline blow.

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