Address code review comments.

config.yaml

@@ -106,8 +106,8 @@ options:
     type: boolean
     description: |
       If True, charm will attempt to remove missing physical volumes from
-      volume group, even when logical volumes are allocated on them. The
-      'remove-missing' option must also be set to True.
+      volume group, even when logical volumes are allocated on them. This
+      option overrides 'remove-missing' when set.
   database-user:
     default: cinder
     type: string

hooks/cinder_utils.py

@@ -333,24 +333,18 @@ def services():
     return list(set(_services))
 
 
-def reduce_lvm_volume_group_missing(volume_group):
+def reduce_lvm_volume_group_missing(volume_group, extra_args=None):
     '''
     Remove all missing physical volumes from the volume group, if there
     are no logical volumes allocated on them.
 
     :param volume_group: str: Name of volume group to reduce.
+    :param extra_args: list: List of extra args to pass to vgreduce
     '''
-    subprocess.check_call(['vgreduce', '--removemissing', volume_group])
+    if extra_args is None:
+        extra_args = []
 
-
-def force_reduce_lvm_volume_group_missing(volume_group):
-    '''
-    Remove all missing physical volumes from the volume group, even if
-    logical volumes are allocated on them.
-
-    :param volume_group: str: Name of volume group to reduce.
-    '''
-    command = ['vgreduce', '--removemissing', '--force', volume_group]
+    command = ['vgreduce', '--removemissing'] + extra_args + [volume_group]
     subprocess.check_call(command)
 
 
@@ -419,18 +413,17 @@ def configure_lvm_storage(block_devices, volume_group, overwrite=False,
             vg_found = True
 
     log_lvm_info()
-    
+
     if vg_found is False and len(new_devices) > 0:
         # Create new volume group from first device
         create_lvm_volume_group(volume_group, new_devices[0])
         new_devices.remove(new_devices[0])
 
     # Remove missing physical volumes from volume group
-    if remove_missing:
-        if remove_missing_force:
-            force_reduce_lvm_volume_group_missing(volume_group)
-        else:
-            reduce_lvm_volume_group_missing(volume_group)
+    if remove_missing_force:
+        reduce_lvm_volume_group_missing(volume_group, extra_args=['--force'])
+    elif remove_missing:
+        reduce_lvm_volume_group_missing(volume_group)
 
     if len(new_devices) > 0:
         # Extend the volume group as required

unit_tests/test_cinder_utils.py

@@ -411,20 +411,20 @@ class TestCinderUtils(CharmTestCase):
 
     @patch('cinder_utils.log_lvm_info', Mock())
     @patch.object(cinder_utils, 'reduce_lvm_volume_group_missing')
-    def test_configure_lvm_storage_used_dev(self, reduce_lvm):
+    def test_configure_lvm_storage_unforced_remove_default(self, reduce_lvm):
         """It doesn't force remove missing by default."""
         devices = ['/dev/vdb']
         cinder_utils.configure_lvm_storage(devices, 'test', False, True)
         reduce_lvm.assert_called_with('test')
 
     @patch('cinder_utils.log_lvm_info', Mock())
-    @patch.object(cinder_utils, 'force_reduce_lvm_volume_group_missing')
-    def test_configure_lvm_storage_used_dev(self, force_reduce_lvm):
+    @patch.object(cinder_utils, 'reduce_lvm_volume_group_missing')
+    def test_configure_lvm_storage_force_removemissing(self, reduce_lvm):
         """It forces remove missing when asked to."""
         devices = ['/dev/vdb']
         cinder_utils.configure_lvm_storage(
             devices, 'test', False, True, remove_missing_force=True)
-        force_reduce_lvm.assert_called_with('test')
+        reduce_lvm.assert_called_with('test', extra_args=['--force'])
 
     @patch('subprocess.check_call')
     def test_reduce_lvm_volume_group_missing(self, _call):
@@ -432,9 +432,10 @@ class TestCinderUtils(CharmTestCase):
         _call.assert_called_with(['vgreduce', '--removemissing', 'test'])
 
     @patch('subprocess.check_call')
-    def test_force_reduce_lvm_volume_group_missing(self, _call):
-        cinder_utils.force_reduce_lvm_volume_group_missing('test')
-        expected_call_args = ['vgreduce', '--removemissing', '--force', 'test']
+    def test_reduce_lvm_volume_group_missing_extra_args(self, _call):
+        cinder_utils.reduce_lvm_volume_group_missing(
+            'test', extra_args=['--arg'])
+        expected_call_args = ['vgreduce', '--removemissing', '--arg', 'test']
         _call.assert_called_with(expected_call_args)
 
     @patch('subprocess.check_call')