From 844aa0ac3e8068e25193a680ac0c63d68682de4b Mon Sep 17 00:00:00 2001 From: John Griffith Date: Fri, 2 Sep 2016 23:00:32 +0000 Subject: [PATCH] Provide cfg to ignore lvm descriptor leak warnings For some reason the leaded descriptor warning message coming from LVM is causing Cinder to fail startup and it appears to be masking out the vg response in vgs calls. We typically don't hit this, but due to the nature of Kolla and I guess going through the different processes via the containers this gets logged every time vgs is called. Eric Harney rightly pointed out that rather than use exception handling and such that we should use the LVM env variable mechanism we already have in place in Cinder. For now this patch added a new config option to the LVM driver: lvm_suppress_fd_warnings=True|False This is useful for K8 deployments that have an indirect call to the LVM cmds which results in failures. For those that are interested, this can also be done outside of cinder by setting the silence_logs variable in lvm.conf This is made optional as a config flag to avoid any breakage for existing deployments during upgrade. DocImpact: Need to add info regarding config update to docs Change-Id: I85612fa49475beea58d30330c8fe8352a2f91123 Closes-Bug: #1619701 (cherry picked from commit 055ec1ce73ca55c463481c349b42dee66e5e86d6) --- cinder/brick/local_dev/lvm.py | 18 ++++-- cinder/tests/unit/brick/test_brick_lvm.py | 61 ++++++++++++------- cinder/volume/drivers/lvm.py | 19 ++++-- etc/cinder/rootwrap.d/volume.filters | 19 +++++- ...m-fd-warnings-option.402bebc03b0a9f00.yaml | 17 ++++++ 5 files changed, 100 insertions(+), 34 deletions(-) create mode 100644 releasenotes/notes/add-suppress-lvm-fd-warnings-option.402bebc03b0a9f00.yaml diff --git a/cinder/brick/local_dev/lvm.py b/cinder/brick/local_dev/lvm.py index 871047da6..664883b6a 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 864e1c551..71ae1c3de 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 3fa806004..1fdf76b1f 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 db642f3a0..f7810c46f 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 000000000..50acdf89d --- /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. +