From 5dc5a55d33941bc30de247fb85b1c03f8b32006a Mon Sep 17 00:00:00 2001 From: Sean McGinnis Date: Mon, 15 Apr 2019 18:30:57 -0500 Subject: [PATCH] Provide setting to ignore lvm descriptor leak warnings For some reason the leaked 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. This was added to the LVM local_dev code in the openstack/cinder repo, but was not also added to os-brick. The long term goal is to have all of this handling in os-brick, so this replicates the changes from cinder done in I85612fa49475beea58d30330c8fe8352a2f91123. Change-Id: I9d5aaad0e6213535afc1ce071f12190cc6aa02d0 Signed-off-by: Sean McGinnis --- os_brick/local_dev/lvm.py | 23 ++++++--- os_brick/tests/local_dev/test_brick_lvm.py | 57 ++++++++++++++-------- 2 files changed, 52 insertions(+), 28 deletions(-) diff --git a/os_brick/local_dev/lvm.py b/os_brick/local_dev/lvm.py index 69c271575..320436c49 100644 --- a/os_brick/local_dev/lvm.py +++ b/os_brick/local_dev/lvm.py @@ -13,9 +13,7 @@ # License for the specific language governing permissions and limitations # under the License. -""" -LVM class for performing LVM operations. -""" +"""LVM class for performing LVM operations.""" import math import os @@ -41,7 +39,8 @@ class LVM(executor.Executor): def __init__(self, vg_name, root_helper, create_vg=False, physical_volumes=None, lvm_type='default', - executor=None, lvm_conf=None): + executor=None, lvm_conf=None, + suppress_fd_warn=False): """Initialize the LVM object. @@ -56,7 +55,7 @@ class LVM(executor.Executor): :param lvm_type: VG and Volume type (default, or thin) :param executor: Execute method to use, None uses oslo_concurrency.processutils - + :param suppress_fd_warn: Add suppress FD Warn to LVM env """ super(LVM, self).__init__(execute=executor, root_helper=root_helper) self.vg_name = vg_name @@ -75,11 +74,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: try: diff --git a/os_brick/tests/local_dev/test_brick_lvm.py b/os_brick/tests/local_dev/test_brick_lvm.py index 70fbf52af..68e4dd5ed 100644 --- a/os_brick/tests/local_dev/test_brick_lvm.py +++ b/os_brick/tests/local_dev/test_brick_lvm.py @@ -25,17 +25,22 @@ class BrickLvmTestCase(base.TestCase): def setUp(self): super(BrickLvmTestCase, self).setUp() + if not hasattr(self, 'configuration'): + self.configuration = mock.Mock() + self.configuration.lvm_suppress_fd_warnings = False self.volume_group_name = 'fake-vg' # Stub processutils.execute for static methods self.mock_object(priv_rootwrap, 'execute', self.fake_execute) - self.vg = brick.LVM(self.volume_group_name, - 'sudo', - create_vg=False, - physical_volumes=None, - lvm_type='default', - executor=self.fake_execute) + self.vg = brick.LVM( + self.volume_group_name, + 'sudo', + create_vg=False, + physical_volumes=None, + lvm_type='default', + executor=self.fake_execute, + suppress_fd_warn=self.configuration.lvm_suppress_fd_warnings) def failed_fake_execute(obj, *cmd, **kwargs): return ("\n", "fake-error") @@ -55,24 +60,29 @@ class BrickLvmTestCase(base.TestCase): def fake_execute(obj, *cmd, **kwargs): # TODO(eharney): remove this and move to per-test mocked execute calls + 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 + 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: @@ -89,17 +99,17 @@ class BrickLvmTestCase(base.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( @@ -118,19 +128,19 @@ class BrickLvmTestCase(base.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-' else: data = ' owi-a-' - 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: @@ -387,3 +397,10 @@ class BrickLvmTestCase(base.TestCase): with mock.patch.object(self.vg, '_execute', return_value=['owi-----', '']): self.assertFalse(self.vg._lv_is_active('test')) + + +class BrickLvmTestCaseIgnoreFDWarnings(BrickLvmTestCase): + def setUp(self): + self.configuration = mock.Mock() + self.configuration.lvm_suppress_fd_warnings = True + super(BrickLvmTestCaseIgnoreFDWarnings, self).setUp()