-
Notifications
You must be signed in to change notification settings - Fork 162
add handling for LVM configuration #1460
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| from leapp.actors import Actor | ||
| from leapp.libraries.actor.checklvm import check_lvm | ||
| from leapp.models import DistributionSignedRPM, LVMConfig, TargetUserSpaceUpgradeTasks, UpgradeInitramfsTasks | ||
| from leapp.reporting import Report | ||
| from leapp.tags import ChecksPhaseTag, IPUWorkflowTag | ||
|
|
||
|
|
||
| class CheckLVM(Actor): | ||
| """ | ||
| Check if the LVM is installed and ensure the target userspace container | ||
| and initramfs are prepared to support it. | ||
| The LVM configuration files are copied into the target userspace container | ||
| so that the dracut is able to use them while creating the initramfs. | ||
| The dracut LVM module is enabled by this actor as well. | ||
| """ | ||
|
|
||
| name = 'check_lvm' | ||
| consumes = (DistributionSignedRPM, LVMConfig) | ||
| produces = (Report, TargetUserSpaceUpgradeTasks, UpgradeInitramfsTasks) | ||
| tags = (ChecksPhaseTag, IPUWorkflowTag) | ||
|
|
||
| def process(self): | ||
| check_lvm() |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,74 @@ | ||||||||||||||||||
| import os | ||||||||||||||||||
|
|
||||||||||||||||||
| from leapp import reporting | ||||||||||||||||||
| from leapp.libraries.common.rpms import has_package | ||||||||||||||||||
| from leapp.libraries.stdlib import api | ||||||||||||||||||
| from leapp.models import ( | ||||||||||||||||||
| CopyFile, | ||||||||||||||||||
| DistributionSignedRPM, | ||||||||||||||||||
| DracutModule, | ||||||||||||||||||
| LVMConfig, | ||||||||||||||||||
| TargetUserSpaceUpgradeTasks, | ||||||||||||||||||
| UpgradeInitramfsTasks | ||||||||||||||||||
| ) | ||||||||||||||||||
|
|
||||||||||||||||||
| LVM_CONFIG_PATH = '/etc/lvm/lvm.conf' | ||||||||||||||||||
| LVM_DEVICES_FILE_PATH_PREFIX = '/etc/lvm/devices' | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
||||||||||||||||||
| def _report_filter_detection(): | ||||||||||||||||||
| title = 'LVM filter definition detected.' | ||||||||||||||||||
| summary = ( | ||||||||||||||||||
| 'RHEL 9 and above uses the LVM devices file by default to select devices used by LVM. ' | ||||||||||||||||||
| f'Since this system has LVM filter defined in the {LVM_CONFIG_PATH}, it will be ' | ||||||||||||||||||
| 'used after the upgrade as well.' | ||||||||||||||||||
| ) | ||||||||||||||||||
|
|
||||||||||||||||||
| remediation_hint = ( | ||||||||||||||||||
| 'While not mandatory, switching to the LVM devices file from the LVM filter is possible ' | ||||||||||||||||||
| 'using the following command. It uses the existing LVM filter to create the system.devices ' | ||||||||||||||||||
| 'file which is then used instead of the LVM filter as long as it exists. Before running the command, ' | ||||||||||||||||||
| 'make sure that the use_devicesfile=1 (default for RHEL 9 and above).' | ||||||||||||||||||
|
Comment on lines
+28
to
+31
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||
| ) | ||||||||||||||||||
| remediation_command = ['vgimportdevices', '-a'] | ||||||||||||||||||
|
|
||||||||||||||||||
| reporting.create_report([ | ||||||||||||||||||
| reporting.Title(title), | ||||||||||||||||||
| reporting.Summary(summary), | ||||||||||||||||||
| reporting.Remediation(hint=remediation_hint, commands=[remediation_command]), | ||||||||||||||||||
| reporting.ExternalLink( | ||||||||||||||||||
| title='Limiting LVM device visibility and usage', | ||||||||||||||||||
| url='https://red.ht/3MfgK7c', | ||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should create a nicer shortened URL for this one if it doesn't exist already, we can sync about that.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe we could leave that for a followup though |
||||||||||||||||||
| ), | ||||||||||||||||||
| reporting.Severity(reporting.Severity.INFO), | ||||||||||||||||||
| ]) | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
||||||||||||||||||
| def check_lvm(): | ||||||||||||||||||
| if not has_package(DistributionSignedRPM, 'lvm2'): | ||||||||||||||||||
| return | ||||||||||||||||||
|
|
||||||||||||||||||
| lvm_config = next(api.consume(LVMConfig), None) | ||||||||||||||||||
| if not lvm_config: | ||||||||||||||||||
| return | ||||||||||||||||||
|
|
||||||||||||||||||
| lvm_devices_file_path = os.path.join(LVM_DEVICES_FILE_PATH_PREFIX, lvm_config.devices.devicesfile) | ||||||||||||||||||
| lvm_devices_file_exists = os.path.isfile(lvm_devices_file_path) | ||||||||||||||||||
|
|
||||||||||||||||||
| filters_used = not lvm_config.devices.use_devicesfile or not lvm_devices_file_exists | ||||||||||||||||||
| if filters_used: | ||||||||||||||||||
| _report_filter_detection() | ||||||||||||||||||
|
|
||||||||||||||||||
| api.current_logger().debug('Including lvm dracut module.') | ||||||||||||||||||
| api.produce(UpgradeInitramfsTasks(include_dracut_modules=[DracutModule(name='lvm')])) | ||||||||||||||||||
| # TODO: decide if we need to install lvm2 package in the container as well | ||||||||||||||||||
|
|
||||||||||||||||||
| copy_files = [] | ||||||||||||||||||
| api.current_logger().debug('Copying "{}" to the target userspace.'.format(LVM_CONFIG_PATH)) | ||||||||||||||||||
| copy_files.append(CopyFile(src=LVM_CONFIG_PATH)) | ||||||||||||||||||
|
|
||||||||||||||||||
| if lvm_devices_file_exists and lvm_config.devices.use_devicesfile: | ||||||||||||||||||
| api.current_logger().debug('Copying "{}" to the target userspace.'.format(lvm_devices_file_path)) | ||||||||||||||||||
| copy_files.append(CopyFile(src=lvm_devices_file_path)) | ||||||||||||||||||
|
|
||||||||||||||||||
| api.produce(TargetUserSpaceUpgradeTasks(copy_files=copy_files)) | ||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
| import os | ||
|
|
||
| import pytest | ||
|
|
||
| from leapp.libraries.actor import checklvm | ||
| from leapp.libraries.common.testutils import produce_mocked | ||
| from leapp.libraries.stdlib import api | ||
| from leapp.models import ( | ||
| DistributionSignedRPM, | ||
| LVMConfig, | ||
| LVMConfigDevicesSection, | ||
| RPM, | ||
| TargetUserSpaceUpgradeTasks, | ||
| UpgradeInitramfsTasks | ||
| ) | ||
|
|
||
|
|
||
| def test_check_lvm_when_lvm_not_installed(monkeypatch): | ||
| def consume_mocked(model): | ||
| if model == LVMConfig: | ||
| assert False | ||
| if model == DistributionSignedRPM: | ||
| yield DistributionSignedRPM(items=[]) | ||
|
|
||
| monkeypatch.setattr(api, 'produce', produce_mocked()) | ||
| monkeypatch.setattr(api, 'consume', consume_mocked) | ||
|
|
||
| checklvm.check_lvm() | ||
|
|
||
| assert not api.produce.called | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| ('config', 'create_report', 'devices_file_exists'), | ||
| [ | ||
| (LVMConfig(devices=LVMConfigDevicesSection(use_devicesfile=False)), True, False), | ||
| (LVMConfig(devices=LVMConfigDevicesSection(use_devicesfile=True)), False, True), | ||
| (LVMConfig(devices=LVMConfigDevicesSection(use_devicesfile=True)), True, False), | ||
| (LVMConfig(devices=LVMConfigDevicesSection(use_devicesfile=False, devicesfile="test.devices")), True, False), | ||
| (LVMConfig(devices=LVMConfigDevicesSection(use_devicesfile=True, devicesfile="test.devices")), False, True), | ||
| (LVMConfig(devices=LVMConfigDevicesSection(use_devicesfile=True, devicesfile="test.devices")), True, False), | ||
| ] | ||
| ) | ||
| def test_scan_when_lvm_installed(monkeypatch, config, create_report, devices_file_exists): | ||
| lvm_package = RPM( | ||
| name='lvm2', | ||
| version='2', | ||
| release='1', | ||
| epoch='1', | ||
| packager='', | ||
| arch='x86_64', | ||
| pgpsig='RSA/SHA256, Mon 01 Jan 1970 00:00:00 AM -03, Key ID 199e2f91fd431d51' | ||
| ) | ||
|
|
||
| def isfile_mocked(_): | ||
| return devices_file_exists | ||
|
|
||
| def consume_mocked(model): | ||
| if model == LVMConfig: | ||
| yield config | ||
| if model == DistributionSignedRPM: | ||
| yield DistributionSignedRPM(items=[lvm_package]) | ||
|
|
||
| def report_filter_detection_mocked(): | ||
| assert create_report | ||
|
|
||
| monkeypatch.setattr(api, 'produce', produce_mocked()) | ||
| monkeypatch.setattr(api, 'consume', consume_mocked) | ||
| monkeypatch.setattr(os.path, 'isfile', isfile_mocked) | ||
| monkeypatch.setattr(checklvm, '_report_filter_detection', report_filter_detection_mocked) | ||
|
|
||
| checklvm.check_lvm() | ||
|
|
||
| # The lvm is installed, thus the dracut module is enabled and at least the lvm.conf is copied | ||
| assert api.produce.called == 2 | ||
| assert len(api.produce.model_instances) == 2 | ||
|
|
||
| expected_copied_files = [checklvm.LVM_CONFIG_PATH] | ||
| if devices_file_exists and config.devices.use_devicesfile: | ||
| devices_file_path = os.path.join(checklvm.LVM_DEVICES_FILE_PATH_PREFIX, config.devices.devicesfile) | ||
| expected_copied_files.append(devices_file_path) | ||
|
|
||
| for produced_model in api.produce.model_instances: | ||
| assert isinstance(produced_model, (UpgradeInitramfsTasks, TargetUserSpaceUpgradeTasks)) | ||
|
|
||
| if isinstance(produced_model, UpgradeInitramfsTasks): | ||
| assert len(produced_model.include_dracut_modules) == 1 | ||
| assert produced_model.include_dracut_modules[0].name == 'lvm' | ||
| else: | ||
| assert len(produced_model.copy_files) == len(expected_copied_files) | ||
| for file in produced_model.copy_files: | ||
| assert file.src in expected_copied_files |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,6 +12,7 @@ | |
| from leapp.models import ( | ||
| BootContent, | ||
| LiveModeConfig, | ||
| LVMConfig, | ||
| TargetOSInstallationImage, | ||
| TargetUserSpaceInfo, | ||
| TargetUserSpaceUpgradeTasks, | ||
|
|
@@ -363,20 +364,29 @@ def generate_initram_disk(context): | |
| def fmt_module_list(module_list): | ||
| return ','.join(mod.name for mod in module_list) | ||
|
|
||
| env_variables = [ | ||
| 'LEAPP_KERNEL_VERSION={kernel_version}', | ||
| 'LEAPP_ADD_DRACUT_MODULES="{dracut_modules}"', | ||
| 'LEAPP_KERNEL_ARCH={arch}', | ||
| 'LEAPP_ADD_KERNEL_MODULES="{kernel_modules}"', | ||
| 'LEAPP_DRACUT_INSTALL_FILES="{files}"' | ||
| ] | ||
|
|
||
| if next(api.consume(LVMConfig), None): | ||
| env_variables.append('LEAPP_DRACUT_LVMCONF="1"') | ||
|
|
||
| env_variables = ' '.join(env_variables) | ||
| env_variables = env_variables.format( | ||
| kernel_version=_get_target_kernel_version(context), | ||
| dracut_modules=fmt_module_list(initramfs_includes.dracut_modules), | ||
| kernel_modules=fmt_module_list(initramfs_includes.kernel_modules), | ||
| arch=api.current_actor().configuration.architecture, | ||
| files=' '.join(initramfs_includes.files) | ||
| ) | ||
| cmd = os.path.join('/', INITRAM_GEN_SCRIPT_NAME) | ||
|
Comment on lines
+367
to
+386
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not very clean solution. I'd suggest expanding on the tasks mechanism with the addition of enabling environment variables (similarly to copying files for example). I didn't try to implement this yet, so it could be an improvement for later.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We might improve the solution greatly by rewriting it in Python instead of this hacky Python-bash interface based on env variables -- something that should be discussed on Cabal. I would implement the minimum that is required to get LVM working properly, like you did, and do refactoring in a separate PR.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree I think for now this is ok. |
||
|
|
||
| # FIXME: issue #376 | ||
| context.call([ | ||
| '/bin/sh', '-c', | ||
| 'LEAPP_KERNEL_VERSION={kernel_version} ' | ||
| 'LEAPP_ADD_DRACUT_MODULES="{dracut_modules}" LEAPP_KERNEL_ARCH={arch} ' | ||
| 'LEAPP_ADD_KERNEL_MODULES="{kernel_modules}" ' | ||
| 'LEAPP_DRACUT_INSTALL_FILES="{files}" {cmd}'.format( | ||
| kernel_version=_get_target_kernel_version(context), | ||
| dracut_modules=fmt_module_list(initramfs_includes.dracut_modules), | ||
| kernel_modules=fmt_module_list(initramfs_includes.kernel_modules), | ||
| arch=api.current_actor().configuration.architecture, | ||
| files=' '.join(initramfs_includes.files), | ||
| cmd=os.path.join('/', INITRAM_GEN_SCRIPT_NAME)) | ||
| ], env=env) | ||
| context.call(['/bin/sh', '-c', f'{env_variables} {cmd}'], env=env) | ||
|
|
||
| boot_files_info = copy_boot_files(context) | ||
| return boot_files_info | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| from leapp.actors import Actor | ||
| from leapp.libraries.actor import scanlvmconfig | ||
| from leapp.models import DistributionSignedRPM, LVMConfig | ||
| from leapp.tags import FactsPhaseTag, IPUWorkflowTag | ||
|
|
||
|
|
||
| class ScanLVMConfig(Actor): | ||
| """ | ||
| Scan LVM configuration. | ||
| """ | ||
|
|
||
| name = 'scan_lvm_config' | ||
| consumes = (DistributionSignedRPM,) | ||
| produces = (LVMConfig,) | ||
| tags = (FactsPhaseTag, IPUWorkflowTag) | ||
|
|
||
| def process(self): | ||
| scanlvmconfig.scan() |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| import os | ||
|
|
||
| from leapp.libraries.common.config import version | ||
| from leapp.libraries.common.rpms import has_package | ||
| from leapp.libraries.stdlib import api | ||
| from leapp.models import DistributionSignedRPM, LVMConfig, LVMConfigDevicesSection | ||
|
|
||
| LVM_CONFIG_PATH = '/etc/lvm/lvm.conf' | ||
|
|
||
|
|
||
| def _lvm_config_devices_parser(lvm_config_lines): | ||
| in_section = False | ||
| config = {} | ||
| for line in lvm_config_lines: | ||
| line = line.split("#", 1)[0].strip() | ||
| if not line: | ||
| continue | ||
| if "devices {" in line: | ||
| in_section = True | ||
| continue | ||
| if in_section and "}" in line: | ||
| in_section = False | ||
| if in_section: | ||
| value = line.split("=", 1) | ||
| config[value[0].strip()] = value[1].strip().strip('"') | ||
| return config | ||
|
|
||
|
|
||
| def _read_config_lines(path): | ||
| with open(path) as lvm_conf_file: | ||
| return lvm_conf_file.readlines() | ||
|
|
||
|
|
||
| def scan(): | ||
| if not has_package(DistributionSignedRPM, 'lvm2'): | ||
| return | ||
|
|
||
| if not os.path.isfile(LVM_CONFIG_PATH): | ||
| api.current_logger().debug('The "{}" is not present on the system.'.format(LVM_CONFIG_PATH)) | ||
| return | ||
|
|
||
| lvm_config_lines = _read_config_lines(LVM_CONFIG_PATH) | ||
| devices_section = _lvm_config_devices_parser(lvm_config_lines) | ||
|
|
||
| lvm_config_devices = LVMConfigDevicesSection(use_devicesfile=int(version.get_source_major_version()) > 8) | ||
| if 'devicesfile' in devices_section: | ||
| lvm_config_devices.devicesfile = devices_section['devicesfile'] | ||
PeterMocary marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| if 'use_devicesfile' in devices_section and devices_section['use_devicesfile'] in ['0', '1']: | ||
| lvm_config_devices.use_devicesfile = devices_section['use_devicesfile'] == '1' | ||
|
|
||
| api.produce(LVMConfig(devices=lvm_config_devices)) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My suggestion (emphasize that we wont modify the config):
'Beginning with RHEL 9, LVM devicesfile is used by default to select devices used by LVM. Since leapp detected the use of LVM filter in the {LVM_CONFIG_PATH} configuration file, the configuration won't be modified to use devicesfile during the upgrade and the LVM filter will remain in use.'