Skip to content

Commit

Permalink
add suggested changes + fix broken test
Browse files Browse the repository at this point in the history
  • Loading branch information
@haddystuff
haddystuff committed Oct 26, 2022
1 parent f1b6f12 commit 9b5e4d4
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 31 deletions.
2 changes: 1 addition & 1 deletion changelogs/fragments/4028-modprobe-persistent-option.yml
View file
Original file line number Diff line number Diff line change
@@ -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).
28 changes: 14 additions & 14 deletions plugins/modules/system/modprobe.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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

Expand All @@ -159,15 +159,15 @@ 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:
file_content = file.readlines()

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

Expand All @@ -177,15 +177,15 @@ 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:
file_content = file.readlines()

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

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
)
Expand Down
38 changes: 22 additions & 16 deletions tests/unit/plugins/modules/system/test_modprobe.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
)
Expand Down Expand Up @@ -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,
)
Expand Down Expand Up @@ -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,
)
Expand Down Expand Up @@ -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,
)
Expand Down Expand Up @@ -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')
Expand All @@ -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,
)
Expand Down Expand Up @@ -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,
)
Expand Down Expand Up @@ -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,
)
Expand All @@ -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')
Expand Down Expand Up @@ -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,
)
Expand All @@ -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):

Expand All @@ -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,
)
Expand Down Expand Up @@ -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,
)
Expand Down Expand Up @@ -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,
)
Expand Down Expand Up @@ -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,
)
Expand Down Expand Up @@ -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,
)
Expand Down Expand Up @@ -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,
)
Expand Down Expand Up @@ -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,
)
Expand Down

0 comments on commit 9b5e4d4

Please sign in to comment.