Merge "Provide cfg to ignore lvm descriptor leak warnings"

cinder/brick/local_dev/lvm.py

@@ -41,7 +41,8 @@ class LVM(executor.Executor):
 
     def __init__(self, vg_name, root_helper, create_vg=False,
                  physical_volumes=None, lvm_type='default',
-                 executor=putils.execute, lvm_conf=None):
+                 executor=putils.execute, lvm_conf=None,
+                 suppress_fd_warn=False):
 
         """Initialize the LVM object.
 
@@ -55,6 +56,7 @@ class LVM(executor.Executor):
         :param physical_volumes: List of PVs to build VG on
         :param lvm_type: VG and Volume type (default, or thin)
         :param executor: Execute method to use, None uses common/processutils
+        :param suppress_fd_warn: Add suppress FD Warn to LVM env
 
         """
         super(LVM, self).__init__(execute=executor, root_helper=root_helper)
@@ -74,11 +76,19 @@ class LVM(executor.Executor):
         # Ensure LVM_SYSTEM_DIR has been added to LVM.LVM_CMD_PREFIX
         # before the first LVM command is executed, and use the directory
         # where the specified lvm_conf file is located as the value.
+
+        # NOTE(jdg): We use the temp var here becuase LVM_CMD_PREFIX is a
+        # class global and if you use append here, you'll literally just keep
+        # appending values to the global.
+        _lvm_cmd_prefix = ['env', 'LC_ALL=C']
+
         if lvm_conf and os.path.isfile(lvm_conf):
             lvm_sys_dir = os.path.dirname(lvm_conf)
-            LVM.LVM_CMD_PREFIX = ['env',
+            _lvm_cmd_prefix.append('LVM_SYSTEM_DIR=' + lvm_sys_dir)
-                                  'LC_ALL=C',
+
-                                  'LVM_SYSTEM_DIR=' + lvm_sys_dir]
+        if suppress_fd_warn:
+            _lvm_cmd_prefix.append('LVM_SUPPRESS_FD_WARNINGS=1')
+        LVM.LVM_CMD_PREFIX = _lvm_cmd_prefix
 
         if create_vg and physical_volumes is not None:
             self.pv_list = physical_volumes

cinder/tests/unit/brick/test_brick_lvm.py

@@ -23,16 +23,20 @@ from cinder.volume import configuration as conf
 
 class BrickLvmTestCase(test.TestCase):
     def setUp(self):
+        if not hasattr(self, 'configuration'):
             self.configuration = mock.Mock(conf.Configuration)
+            self.configuration.lvm_suppress_fd_warnings = False
         self.configuration.volume_group_name = 'fake-vg'
         super(BrickLvmTestCase, self).setUp()
 
         self.mock_object(processutils, 'execute', self.fake_execute)
-        self.vg = brick.LVM(self.configuration.volume_group_name,
+        self.vg = brick.LVM(
+            self.configuration.volume_group_name,
             'sudo',
             False, None,
             'default',
-                            self.fake_execute)
+            self.fake_execute,
+            suppress_fd_warn=self.configuration.lvm_suppress_fd_warnings)
 
     def failed_fake_execute(obj, *cmd, **kwargs):
         return ("\n", "fake-error")
@@ -48,24 +52,28 @@ class BrickLvmTestCase(test.TestCase):
         return ("  LVM version:     2.02.100(2)-RHEL6 (2013-09-12)\n", "")
 
     def fake_execute(obj, *cmd, **kwargs):  # noqa
+        if obj.configuration.lvm_suppress_fd_warnings:
+            _lvm_prefix = 'env, LC_ALL=C, LVM_SUPPRESS_FD_WARNINGS=1, '
+        else:
+            _lvm_prefix = 'env, LC_ALL=C, '
+
         cmd_string = ', '.join(cmd)
         data = "\n"
-
+        if (_lvm_prefix + 'vgs, --noheadings, --unit=g, -o, name' ==
-        if ('env, LC_ALL=C, vgs, --noheadings, --unit=g, -o, name' ==
                 cmd_string):
             data = "  fake-vg\n"
             data += "  some-other-vg\n"
-        elif ('env, LC_ALL=C, vgs, --noheadings, -o, name, fake-vg' ==
+        elif (_lvm_prefix + 'vgs, --noheadings, -o, name, fake-vg' ==
                 cmd_string):
             data = "  fake-vg\n"
-        elif 'env, LC_ALL=C, vgs, --version' in cmd_string:
+        elif _lvm_prefix + 'vgs, --version' in cmd_string:
             data = "  LVM version:     2.02.95(2) (2012-03-06)\n"
-        elif ('env, LC_ALL=C, vgs, --noheadings, -o, uuid, fake-vg' in
+        elif(_lvm_prefix + 'vgs, --noheadings, -o, uuid, fake-vg' in
              cmd_string):
             data = "  kVxztV-dKpG-Rz7E-xtKY-jeju-QsYU-SLG6Z1\n"
-        elif 'env, LC_ALL=C, vgs, --noheadings, --unit=g, ' \
+        elif(_lvm_prefix + 'vgs, --noheadings, --unit=g, '
-             '-o, name,size,free,lv_count,uuid, ' \
+             '-o, name,size,free,lv_count,uuid, '
-             '--separator, :, --nosuffix' in cmd_string:
+             '--separator, :, --nosuffix' in cmd_string):
             data = ("  test-prov-cap-vg-unit:10.00:10.00:0:"
                     "mXzbuX-dKpG-Rz7E-xtKY-jeju-QsYU-SLG8Z4\n")
             if 'test-prov-cap-vg-unit' in cmd_string:
@@ -82,17 +90,17 @@ class BrickLvmTestCase(test.TestCase):
                     "lWyauW-dKpG-Rz7E-xtKY-jeju-QsYU-SLG7Z2\n"
             data += "  fake-vg-3:10.00:10.00:0:"\
                     "mXzbuX-dKpG-Rz7E-xtKY-jeju-QsYU-SLG8Z3\n"
-        elif ('env, LC_ALL=C, lvs, --noheadings, '
+        elif (_lvm_prefix + 'lvs, --noheadings, '
               '--unit=g, -o, vg_name,name,size, --nosuffix, '
               'fake-vg/lv-nothere' in cmd_string):
             raise processutils.ProcessExecutionError(
                 stderr="One or more specified logical volume(s) not found.")
-        elif ('env, LC_ALL=C, lvs, --noheadings, '
+        elif (_lvm_prefix + 'lvs, --noheadings, '
               '--unit=g, -o, vg_name,name,size, --nosuffix, '
               'fake-vg/lv-newerror' in cmd_string):
             raise processutils.ProcessExecutionError(
                 stderr="Failed to find logical volume \"fake-vg/lv-newerror\"")
-        elif ('env, LC_ALL=C, lvs, --noheadings, '
+        elif (_lvm_prefix + 'lvs, --noheadings, '
               '--unit=g, -o, vg_name,name,size' in cmd_string):
             if 'fake-unknown' in cmd_string:
                 raise processutils.ProcessExecutionError(
@@ -111,7 +119,7 @@ class BrickLvmTestCase(test.TestCase):
             else:
                 data = "  fake-vg fake-1 1.00g\n"
                 data += "  fake-vg fake-2 1.00g\n"
-        elif ('env, LC_ALL=C, lvdisplay, --noheading, -C, -o, Attr' in
+        elif (_lvm_prefix + 'lvdisplay, --noheading, -C, -o, Attr' in
               cmd_string):
             if 'test-volumes' in cmd_string:
                 data = '  wi-a-'
@@ -121,18 +129,18 @@ class BrickLvmTestCase(test.TestCase):
                 data = '  -wi-ao---'
             else:
                 data = '  owi-a-'
-        elif ('env, LC_ALL=C, lvdisplay, --noheading, -C, -o, Origin' in
+        elif (_lvm_prefix + 'lvdisplay, --noheading, -C, -o, Origin' in
               cmd_string):
             if 'snapshot' in cmd_string:
                 data = '  fake-volume-1'
             else:
                 data = '       '
-        elif 'env, LC_ALL=C, pvs, --noheadings' in cmd_string:
+        elif _lvm_prefix + 'pvs, --noheadings' in cmd_string:
             data = "  fake-vg|/dev/sda|10.00|1.00\n"
             data += "  fake-vg|/dev/sdb|10.00|1.00\n"
             data += "  fake-vg|/dev/sdc|10.00|8.99\n"
             data += "  fake-vg-2|/dev/sdd|10.00|9.99\n"
-        elif 'env, LC_ALL=C, lvs, --noheadings, --unit=g' \
+        elif _lvm_prefix + 'lvs, --noheadings, --unit=g' \
                 ', -o, size,data_percent, --separator, :' in cmd_string:
             if 'test-prov-cap-pool' in cmd_string:
                 data = "  9.5:20\n"
@@ -389,3 +397,10 @@ class BrickLvmTestCase(test.TestCase):
             self.vg.create_volume('test', '1G')
             self.assertRaises(exception.VolumeNotDeactivated,
                               self.vg.deactivate_lv, 'test')
+
+
+class BrickLvmTestCaseIgnoreFDWarnings(BrickLvmTestCase):
+    def setUp(self):
+        self.configuration = mock.Mock(conf.Configuration)
+        self.configuration.lvm_suppress_fd_warnings = True
+        super(BrickLvmTestCaseIgnoreFDWarnings, self).setUp()

cinder/volume/drivers/lvm.py

@@ -69,7 +69,11 @@ volume_opts = [
                  help='max_over_subscription_ratio setting for the LVM '
                       'driver.  If set, this takes precedence over the '
                       'general max_over_subscription_ratio option.  If '
-                      'None, the general option is used.')
+                      'None, the general option is used.'),
+    cfg.BoolOpt('lvm_suppress_fd_warnings',
+                default=False,
+                help='Suppress leaked file descriptor warnings in LVM '
+                     'commands.')
 ]
 
 CONF = cfg.CONF
@@ -287,11 +291,14 @@ class LVMVolumeDriver(driver.VolumeDriver):
                 lvm_conf_file = None
 
             try:
-                self.vg = lvm.LVM(self.configuration.volume_group,
+                self.vg = lvm.LVM(
+                    self.configuration.volume_group,
                     root_helper,
                     lvm_type=self.configuration.lvm_type,
                     executor=self._execute,
-                                  lvm_conf=lvm_conf_file)
+                    lvm_conf=lvm_conf_file,
+                    suppress_fd_warn=(
+                        self.configuration.lvm_suppress_fd_warnings))
 
             except exception.VolumeGroupNotFound:
                 message = (_("Volume Group %s does not exist") %

etc/cinder/rootwrap.d/volume.filters

@@ -16,12 +16,25 @@ vgs: EnvFilter, env, root, LC_ALL=C, vgs
 lvs: EnvFilter, env, root, LC_ALL=C, lvs
 lvdisplay: EnvFilter, env, root, LC_ALL=C, lvdisplay
 
-# LVM conf var
+# -LVM related show commands with suppress fd warnings
+pvs_fdwarn: EnvFilter, env, root, LC_ALL=C, LVM_SUPPRESS_FD_WARNINGS=, pvs
+vgs_fdwarn: EnvFilter, env, root, LC_ALL=C, LVM_SUPPRESS_FD_WARNINGS=, vgs
+lvs_fdwarn: EnvFilter, env, root, LC_ALL=C, LVM_SUPPRESS_FD_WARNINGS=, lvs
+lvdisplay_fdwarn: EnvFilter, env, root, LC_ALL=C, LVM_SUPPRESS_FD_WARNINGS=, lvdisplay
+
+
+# -LVM related show commands conf var
 pvs_lvmconf: EnvFilter, env, root, LVM_SYSTEM_DIR=, LC_ALL=C, pvs
 vgs_lvmconf: EnvFilter, env, root, LVM_SYSTEM_DIR=, LC_ALL=C, vgs
 lvs_lvmconf: EnvFilter, env, root, LVM_SYSTEM_DIR=, LC_ALL=C, lvs
 lvdisplay_lvmconf: EnvFilter, env, root, LVM_SYSTEM_DIR=, LC_ALL=C, lvdisplay
 
+# -LVM conf var with suppress fd_warnings
+pvs_lvmconf: EnvFilter, env, root, LVM_SYSTEM_DIR=, LC_ALL=C, LVM_SUPPRESS_FD_WARNINGS=, pvs
+vgs_lvmconf: EnvFilter, env, root, LVM_SYSTEM_DIR=, LC_ALL=C, LVM_SUPPRESS_FD_WARNINGS=, vgs
+lvs_lvmconf: EnvFilter, env, root, LVM_SYSTEM_DIR=, LC_ALL=C, LVM_SUPPRESS_FD_WARNINGS=, lvs
+lvdisplay_lvmconf: EnvFilter, env, root, LVM_SYSTEM_DIR=, LC_ALL=C, LVM_SUPPRESS_FD_WARNINGS=, lvdisplay
+
 # os-brick library commands
 # os_brick.privileged.run_as_root oslo.privsep context
 # This line ties the superuser privs with the config files, context name,
@@ -40,6 +53,8 @@ vgcreate: CommandFilter, vgcreate, root
 # cinder/brick/local_dev/lvm.py: 'lvcreate', '-L', ...
 lvcreate: EnvFilter, env, root, LC_ALL=C, lvcreate
 lvcreate_lvmconf: EnvFilter, env, root, LVM_SYSTEM_DIR=, LC_ALL=C, lvcreate
+lvcreate_fdwarn: EnvFilter, env, root, LC_ALL=C, LVM_SUPPRESS_FD_WARNINGS=, lvcreate
+lvcreate_lvmconf_fdwarn: EnvFilter, env, root, LVM_SYSTEM_DIR=, LVM_SUPPRESS_FD_WARNINGS=, LC_ALL=C, lvcreate
 
 # cinder/volume/driver.py: 'dd', 'if=%s' % srcstr, 'of=%s' % deststr,...
 dd: CommandFilter, dd, root
@@ -54,6 +69,8 @@ lvrename: CommandFilter, lvrename, root
 # cinder/brick/local_dev/lvm.py: 'lvextend', '-L' '%(new_size)s', '%(thin_pool)s' ...
 lvextend: EnvFilter, env, root, LC_ALL=C, lvextend
 lvextend_lvmconf: EnvFilter, env, root, LVM_SYSTEM_DIR=, LC_ALL=C, lvextend
+lvextend_fdwarn: EnvFilter, env, root, LC_ALL=C, LVM_SUPPRESS_FD_WARNINGS=, lvextend
+lvextend_lvmconf_fdwarn: EnvFilter, env, root, LVM_SYSTEM_DIR=, LC_ALL=C, LVM_SUPPRESS_FD_WARNINGS=, lvextend
 
 # cinder/brick/local_dev/lvm.py: 'lvchange -a y -K <lv>'
 lvchange: CommandFilter, lvchange, root

releasenotes/notes/add-suppress-lvm-fd-warnings-option.402bebc03b0a9f00.yaml

@@ -0,0 +1,17 @@
+---
+upgrade:
+    - |
+      In certain environments (Kubernetes for example) indirect calls to the LVM
+      commands result in file descriptor leak warning messages which in turn cause
+      the process_execution method to raise and exception.
+
+      To accommodate these environments, and to maintain backward compatibility
+      in Newton we add a ``lvm_suppress_fd_warnings`` bool config to the LVM driver.
+      Setting this to True will append the LVM env vars to include the variable
+      ``LVM_SUPPRESS_FD_WARNINGS=1``.
+
+      This is made an optional configuration because it only applies to very specific
+      environments.  If we were to make this global that would require a rootwrap/privsep
+      update that could break compatibility when trying to do rolling upgrades of the
+      volume service.
+