diff --git a/cinder/brick/local_dev/lvm.py b/cinder/brick/local_dev/lvm.py index 871047da608..664883b6a10 100644 --- a/cinder/brick/local_dev/lvm.py +++ b/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', - 'LC_ALL=C', - 'LVM_SYSTEM_DIR=' + lvm_sys_dir] + _lvm_cmd_prefix.append('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 diff --git a/cinder/tests/unit/brick/test_brick_lvm.py b/cinder/tests/unit/brick/test_brick_lvm.py index 864e1c55195..71ae1c3de95 100644 --- a/cinder/tests/unit/brick/test_brick_lvm.py +++ b/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): - self.configuration = mock.Mock(conf.Configuration) + 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, - 'sudo', - False, None, - 'default', - self.fake_execute) + self.vg = brick.LVM( + self.configuration.volume_group_name, + 'sudo', + False, None, + 'default', + 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 ('env, LC_ALL=C, vgs, --noheadings, --unit=g, -o, name' == + if (_lvm_prefix + '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 - cmd_string): + 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, ' \ - '-o, name,size,free,lv_count,uuid, ' \ - '--separator, :, --nosuffix' in cmd_string: + elif(_lvm_prefix + 'vgs, --noheadings, --unit=g, ' + '-o, name,size,free,lv_count,uuid, ' + '--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,19 +129,19 @@ 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' \ - ', -o, size,data_percent, --separator, :' in cmd_string: + 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" else: @@ -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() diff --git a/cinder/volume/drivers/lvm.py b/cinder/volume/drivers/lvm.py index 3fa80600470..1fdf76b1f03 100644 --- a/cinder/volume/drivers/lvm.py +++ b/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, - root_helper, - lvm_type=self.configuration.lvm_type, - executor=self._execute, - lvm_conf=lvm_conf_file) + self.vg = lvm.LVM( + self.configuration.volume_group, + root_helper, + lvm_type=self.configuration.lvm_type, + executor=self._execute, + lvm_conf=lvm_conf_file, + suppress_fd_warn=( + self.configuration.lvm_suppress_fd_warnings)) except exception.VolumeGroupNotFound: message = (_("Volume Group %s does not exist") % diff --git a/etc/cinder/rootwrap.d/volume.filters b/etc/cinder/rootwrap.d/volume.filters index db642f3a00e..f7810c46f72 100644 --- a/etc/cinder/rootwrap.d/volume.filters +++ b/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 ' lvchange: CommandFilter, lvchange, root diff --git a/releasenotes/notes/add-suppress-lvm-fd-warnings-option.402bebc03b0a9f00.yaml b/releasenotes/notes/add-suppress-lvm-fd-warnings-option.402bebc03b0a9f00.yaml new file mode 100644 index 00000000000..50acdf89d2b --- /dev/null +++ b/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. +