From 9b5e4d483247c27d4e46cd6cd3e6a05920e4f6d3 Mon Sep 17 00:00:00 2001 From: Groshev Alexandr Date: Wed, 26 Oct 2022 21:32:14 +0200 Subject: [PATCH] add suggested changes + fix broken test --- .../4028-modprobe-persistent-option.yml | 2 +- plugins/modules/system/modprobe.py | 28 +++++++------- .../plugins/modules/system/test_modprobe.py | 38 +++++++++++-------- 3 files changed, 37 insertions(+), 31 deletions(-) diff --git a/changelogs/fragments/4028-modprobe-persistent-option.yml b/changelogs/fragments/4028-modprobe-persistent-option.yml index 2d073ba933f..78c812bcbd3 100644 --- a/changelogs/fragments/4028-modprobe-persistent-option.yml +++ b/changelogs/fragments/4028-modprobe-persistent-option.yml @@ -1,3 +1,3 @@ --- minor_changes: - - modprobe - add persistent option (https://github.com/ansible-collections/community.general/issues/4028). + - modprobe - add ``persistent`` option (https://github.com/ansible-collections/community.general/issues/4028, https://github.com/ansible-collections/community.general/pull/542). diff --git a/plugins/modules/system/modprobe.py b/plugins/modules/system/modprobe.py index e9d9ae68e40..9723d90ff8b 100644 --- a/plugins/modules/system/modprobe.py +++ b/plugins/modules/system/modprobe.py @@ -37,10 +37,10 @@ default: '' persistent: type: bool - default: False + default: false description: - Persistency between reboots for configured module. - - This option creates files in /etc/modules-load.d/ and /etc/modprobe.d/ that make your module configuration persistent during reboots. + - This option creates files in C(/etc/modules-load.d/) and C(/etc/modprobe.d/) that make your module configuration persistent during reboots. - Note that it is usually a better idea to rely on the automatic module loading by PCI IDs, USB IDs, DMI IDs or similar triggers encoded in the kernel modules themselves instead of configuration like this. - In fact, most modern kernel modules are prepared for automatic loading already. @@ -109,11 +109,11 @@ def load_module(self): @property def module_is_loaded_persistently(self): - pattern = re.compile(r'^ *{0} *(?:[#;].*)?\n?'.format(self.name)) + pattern = re.compile(r'^ *{0} *(?:[#;].*)?\n?\Z'.format(self.name)) for module_file in self.modules_files: with open(module_file) as file: for line in file: - if pattern.fullmatch(line): + if pattern.match(line): return True return False @@ -128,14 +128,14 @@ def params_is_set(self): def permanent_params(self): params = set() - pattern = re.compile(r'^options {0} (\w+=\S+) *(?:[#;].*)?\n?'.format(self.name)) + pattern = re.compile(r'^options {0} (\w+=\S+) *(?:[#;].*)?\n?\Z'.format(self.name)) for modprobe_file in self.modprobe_files: with open(modprobe_file) as file: for line in file: - match = pattern.fullmatch(line) + match = pattern.match(line) if match: - params.add(match[1]) + params.add(match.group(1)) return params @@ -159,7 +159,7 @@ def create_module_options_file(self): def disable_old_params(self): - pattern = re.compile(r'^options {0} \w+=\S+ *(?:[#;].*)?\n?'.format(self.name)) + pattern = re.compile(r'^options {0} \w+=\S+ *(?:[#;].*)?\n?\Z'.format(self.name)) for modprobe_file in self.modprobe_files: with open(modprobe_file) as file: @@ -167,7 +167,7 @@ def disable_old_params(self): content_changed = False for index, line in enumerate(file_content): - if pattern.fullmatch(line): + if pattern.match(line): file_content[index] = '#' + line content_changed = True @@ -177,7 +177,7 @@ def disable_old_params(self): def disable_module_permanent(self): - pattern = re.compile(r'^ *{0} *(?:[#;].*)?\n?'.format(self.name)) + pattern = re.compile(r'^ *{0} *(?:[#;].*)?\n?\Z'.format(self.name)) for module_file in self.modules_files: with open(module_file) as file: @@ -185,7 +185,7 @@ def disable_module_permanent(self): content_changed = False for index, line in enumerate(file_content): - if pattern.fullmatch(line): + if pattern.match(line): file_content[index] = '#' + line content_changed = True @@ -217,13 +217,13 @@ def unload_module_permanent(self): def modules_files(self): modules_paths = [os.path.join(MODULES_LOAD_LOCATION, path) for path in os.listdir(MODULES_LOAD_LOCATION)] - return list(filter(os.path.isfile, modules_paths)) + return [path for path in modules_paths if os.path.isfile(path)] @property def modprobe_files(self): modules_paths = [os.path.join(PARAMETERS_FILES_LOCATION, path) for path in os.listdir(PARAMETERS_FILES_LOCATION)] - return list(filter(os.path.isfile, modules_paths)) + return [path for path in modules_paths if os.path.isfile(path)] def module_loaded(self): is_loaded = False @@ -275,7 +275,7 @@ def main(): name=dict(type='str', required=True), state=dict(type='str', default='present', choices=['absent', 'present']), params=dict(type='str', default=''), - persistent=dict(type='bool', default=False) + persistent=dict(type='bool', default=False), ), supports_check_mode=True, ) diff --git a/tests/unit/plugins/modules/system/test_modprobe.py b/tests/unit/plugins/modules/system/test_modprobe.py index 88ea67cd0d4..0b06e45b7df 100644 --- a/tests/unit/plugins/modules/system/test_modprobe.py +++ b/tests/unit/plugins/modules/system/test_modprobe.py @@ -6,6 +6,7 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type +import sys from ansible_collections.community.general.tests.unit.plugins.modules.utils import ModuleTestCase, set_module_args from ansible_collections.community.general.tests.unit.compat.mock import call, patch from ansible_collections.community.general.tests.unit.compat.mock import Mock @@ -46,7 +47,7 @@ def test_load_module_success(self): name=dict(type='str', required=True), state=dict(type='str', default='present', choices=['absent', 'present']), params=dict(type='str', default=''), - persistent=dict(type='bool', default=False) + persistent=dict(type='bool', default=False), ), supports_check_mode=True, ) @@ -76,7 +77,7 @@ def test_load_module_unchanged(self): name=dict(type='str', required=True), state=dict(type='str', default='present', choices=['absent', 'present']), params=dict(type='str', default=''), - persistent=dict(type='bool', default=False) + persistent=dict(type='bool', default=False), ), supports_check_mode=True, ) @@ -125,7 +126,7 @@ def test_unload_module_success(self): name=dict(type='str', required=True), state=dict(type='str', default='present', choices=['absent', 'present']), params=dict(type='str', default=''), - persistent=dict(type='bool', default=False) + persistent=dict(type='bool', default=False), ), supports_check_mode=True, ) @@ -155,7 +156,7 @@ def test_unload_module_failure(self): name=dict(type='str', required=True), state=dict(type='str', default='present', choices=['absent', 'present']), params=dict(type='str', default=''), - persistent=dict(type='bool', default=False) + persistent=dict(type='bool', default=False), ), supports_check_mode=True, ) @@ -183,6 +184,9 @@ def test_unload_module_failure(self): class TestModuleIsLoadedPersistently(ModuleTestCase): def setUp(self): + if (sys.version_info[0] == 3 and sys.version_info[1] < 7) or (sys.version_info[0] == 2 and sys.version_info[1] < 7): + self.skipTest('open_mock doesnt support readline in earlier python versions') + super(TestModuleIsLoadedPersistently, self).setUp() self.mock_get_bin_path = patch('ansible.module_utils.basic.AnsibleModule.get_bin_path') @@ -208,7 +212,7 @@ def test_module_is_loaded(self): name=dict(type='str', required=True), state=dict(type='str', default='present', choices=['absent', 'present']), params=dict(type='str', default=''), - persistent=dict(type='bool', default=False) + persistent=dict(type='bool', default=False), ), supports_check_mode=True, ) @@ -237,7 +241,7 @@ def test_module_is_not_loaded_empty_file(self): name=dict(type='str', required=True), state=dict(type='str', default='present', choices=['absent', 'present']), params=dict(type='str', default=''), - persistent=dict(type='bool', default=False) + persistent=dict(type='bool', default=False), ), supports_check_mode=True, ) @@ -266,7 +270,7 @@ def test_module_is_not_loaded_no_files(self): name=dict(type='str', required=True), state=dict(type='str', default='present', choices=['absent', 'present']), params=dict(type='str', default=''), - persistent=dict(type='bool', default=False) + persistent=dict(type='bool', default=False), ), supports_check_mode=True, ) @@ -282,6 +286,8 @@ def test_module_is_not_loaded_no_files(self): class TestPermanentParams(ModuleTestCase): def setUp(self): + if (sys.version_info[0] == 3 and sys.version_info[1] < 7) or (sys.version_info[0] == 2 and sys.version_info[1] < 7): + self.skipTest('open_mock doesnt support readline in earlier python versions') super(TestPermanentParams, self).setUp() self.mock_get_bin_path = patch('ansible.module_utils.basic.AnsibleModule.get_bin_path') @@ -313,7 +319,7 @@ def test_module_permanent_params_exist(self): name=dict(type='str', required=True), state=dict(type='str', default='present', choices=['absent', 'present']), params=dict(type='str', default=''), - persistent=dict(type='bool', default=False) + persistent=dict(type='bool', default=False), ), supports_check_mode=True, ) @@ -327,7 +333,7 @@ def test_module_permanent_params_exist(self): with patch('ansible_collections.community.general.plugins.modules.system.modprobe.Modprobe.modprobe_files'): modprobe.modprobe_files = ['/etc/modprobe.d/dummy1.conf', '/etc/modprobe.d/dummy2.conf'] - assert modprobe.permanent_params == set(['dummy_parameter1=6', 'numdummies=4', 'dummy_parameter2=5']) + assert modprobe.permanent_params == set(['numdummies=4', 'dummy_parameter1=6', 'dummy_parameter2=5']) def test_module_permanent_params_empty(self): @@ -348,7 +354,7 @@ def test_module_permanent_params_empty(self): name=dict(type='str', required=True), state=dict(type='str', default='present', choices=['absent', 'present']), params=dict(type='str', default=''), - persistent=dict(type='bool', default=False) + persistent=dict(type='bool', default=False), ), supports_check_mode=True, ) @@ -392,7 +398,7 @@ def test_create_file(self): name=dict(type='str', required=True), state=dict(type='str', default='present', choices=['absent', 'present']), params=dict(type='str', default=''), - persistent=dict(type='bool', default=False) + persistent=dict(type='bool', default=False), ), supports_check_mode=True, ) @@ -435,7 +441,7 @@ def test_create_file(self): name=dict(type='str', required=True), state=dict(type='str', default='present', choices=['absent', 'present']), params=dict(type='str', default=''), - persistent=dict(type='bool', default=False) + persistent=dict(type='bool', default=False), ), supports_check_mode=True, ) @@ -479,7 +485,7 @@ def test_disable_old_params_file_changed(self): name=dict(type='str', required=True), state=dict(type='str', default='present', choices=['absent', 'present']), params=dict(type='str', default=''), - persistent=dict(type='bool', default=False) + persistent=dict(type='bool', default=False), ), supports_check_mode=True, ) @@ -510,7 +516,7 @@ def test_disable_old_params_file_unchanged(self): name=dict(type='str', required=True), state=dict(type='str', default='present', choices=['absent', 'present']), params=dict(type='str', default=''), - persistent=dict(type='bool', default=False) + persistent=dict(type='bool', default=False), ), supports_check_mode=True, ) @@ -554,7 +560,7 @@ def test_disable_module_permanent_file_changed(self): name=dict(type='str', required=True), state=dict(type='str', default='present', choices=['absent', 'present']), params=dict(type='str', default=''), - persistent=dict(type='bool', default=False) + persistent=dict(type='bool', default=False), ), supports_check_mode=True, ) @@ -584,7 +590,7 @@ def test_disable_module_permanent_file_unchanged(self): name=dict(type='str', required=True), state=dict(type='str', default='present', choices=['absent', 'present']), params=dict(type='str', default=''), - persistent=dict(type='bool', default=False) + persistent=dict(type='bool', default=False), ), supports_check_mode=True, )