Merge lp:~ahasenack/charm-helpers/charm-helpers-ceph-lvm-1317980 into lp:charm-helpers

[Andreas Hasenack]

Fix list_lvm_volume_group() to handle PVs that are not part of a volume group.

list_lvm_volume_group() would incorrectly return "Name" if an LVM physical volume is detected, but not assigned to a volume group. This is what such a PV looks like:

# pvdisplay /dev/sdb
  "/dev/sdb" is a new physical volume of "465.00 GiB"
  --- NEW Physical volume ---
  PV Name /dev/sdb
  VG Name
  PV Size 465.00 GiB
  Allocatable NO
  PE Size 0
  Total PE 0
  Free PE 0
  Allocated PE 0
  PV UUID 7RDJz8-pUqi-9RAz-J8pK-CDKs-JbAa-AO603b

[Marco Ceppi (marcoceppi)]

LGTM! Thanks for this fix

review: Approve

=== modified file 'README.test'
--- README.test	2013-10-21 08:34:03 +0000
+++ README.test	2014-05-10 20:06:51 +0000
@@ -6,3 +6,6 @@
 python-mock
 python-testtools
 python-jinja2
+python-coverage
+python-netifaces
+python-netaddr
 
=== modified file 'charmhelpers/contrib/storage/linux/lvm.py'
--- charmhelpers/contrib/storage/linux/lvm.py	2013-06-24 22:39:29 +0000
+++ charmhelpers/contrib/storage/linux/lvm.py	2014-05-10 20:06:51 +0000
@@ -62,7 +62,7 @@
     pvd = check_output(['pvdisplay', block_device]).splitlines()
     for l in pvd:
         if l.strip().startswith('VG Name'):
-            vg = ' '.join(l.split()).split(' ').pop()
+            vg = ' '.join(l.strip().split()[2:])
     return vg
 
 
 
=== modified file 'tests/contrib/storage/test_linux_storage_lvm.py'
--- tests/contrib/storage/test_linux_storage_lvm.py	2013-06-24 22:39:29 +0000
+++ tests/contrib/storage/test_linux_storage_lvm.py	2014-05-10 20:06:51 +0000
@@ -19,6 +19,20 @@
 
 """
 
+EMPTY_VG_IN_PVDISPLAY = """
+  --- Physical volume ---
+  PV Name               /dev/loop0
+  VG Name   
+  PV Size               10.00 MiB / not usable 2.00 MiB
+  Allocatable           yes
+  PE Size               4.00 MiB
+  Total PE              2
+  Free PE               2
+  Allocated PE          0
+  PV UUID               fyVqlr-pyrL-89On-f6MD-U91T-dEfc-SL0V2V
+
+"""
+
 # It's a mouthful.
 STORAGE_LINUX_LVM = 'charmhelpers.contrib.storage.linux.lvm'
 
@@ -31,6 +45,13 @@
             vg = lvm.list_lvm_volume_group('/dev/loop0')
             self.assertEquals(vg, 'foo')
 
+    def test_find_empty_volume_group_on_pv(self):
+        '''Return empty string when no volume group is assigned to the PV'''
+        with patch(STORAGE_LINUX_LVM + '.check_output') as check_output:
+            check_output.return_value = EMPTY_VG_IN_PVDISPLAY
+            vg = lvm.list_lvm_volume_group('/dev/loop0')
+            self.assertEquals(vg, '')
+
     @patch(STORAGE_LINUX_LVM + '.list_lvm_volume_group')
     def test_deactivate_lvm_volume_groups(self, ls_vg):
         '''It deactivates active volume groups on LVM PV'''