add persistent option for modprobe

[haddystuff]

SUMMARY

Add persistent option as was requested in #4028

ISSUE TYPE

• Feature Pull Request

COMPONENT NAME

plugins/modules/system/modprobe.py

ADDITIONAL INFORMATION

If kernel module doesn't load after reboot(by PCI IDs, USB IDs, DMI IDs or similar triggers) you can use static files in /etc/modules-load.d to load this module. This is what I implemented. If you add option permanent and set it as true ansible will add module file which should enable module on startup. If you set permanent option as false then ansible will try to find line where you enabled this module previously and comment it out. Same works with kernel module options in /etc/modprobe.d directory.
Example:

 - name: Add the dummy module
    community.general.modprobe:
      name: dummy
      state: present
      params: 'numdummies=4'
      persistent: true

[ansibullbot]

cc @jdauphant @mattjeffery
click here for bot help

[github-actions]

Docs Build 📝

Thank you for contribution!✨

The docsite for this PR is available for download as an artifact from this run:
https://github.com/ansible-collections/community.general/actions/runs/3615033372

File changes:

• M collections/community/general/modprobe_module.html

Click to see the diff comparison.

NOTE: only file modifications are shown here. New and deleted files are excluded.
See the file list and check the published docs to see those files.

diff --git a/home/runner/work/community.general/community.general/docsbuild/base/collections/community/general/modprobe_module.html b/home/runner/work/community.general/community.general/docsbuild/head/collections/community/general/modprobe_module.html
index 9c6fcf8..3b348b9 100644
--- a/home/runner/work/community.general/community.general/docsbuild/base/collections/community/general/modprobe_module.html
+++ b/home/runner/work/community.general/community.general/docsbuild/head/collections/community/general/modprobe_module.html
@@ -177,6 +177,22 @@
 </div></td>
 </tr>
 <tr class="row-even"><td><div class="ansible-option-cell">
+<div class="ansibleOptionAnchor" id="parameter-persistent"></div><p class="ansible-option-title" id="ansible-collections-community-general-modprobe-module-parameter-persistent"><strong>persistent</strong></p>
+<a class="ansibleOptionLink" href="#parameter-persistent" title="Permalink to this option"></a><p class="ansible-option-type-line"><span class="ansible-option-type">boolean</span></p>
+</div></td>
+<td><div class="ansible-option-cell"><p>Persistency between reboots for configured module.</p>
+<p>This option creates files in <code class="docutils literal notranslate"><span class="pre">/etc/modules-load.d/</span></code> and <code class="docutils literal notranslate"><span class="pre">/etc/modprobe.d/</span></code> that make your module configuration persistent during reboots.</p>
+<p>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.</p>
+<p>In fact, most modern kernel modules are prepared for automatic loading already.</p>
+<p>Also this options work only with distributives that uses systemd.</p>
+<p class="ansible-option-line"><span class="ansible-option-choices">Choices:</span></p>
+<ul class="simple">
+<li><p><code class="ansible-option-default-bold docutils literal notranslate"><span class="pre">false</span></code> <span class="ansible-option-choices-default-mark">← (default)</span></p></li>
+<li><p><code class="ansible-option-choices-entry docutils literal notranslate"><span class="pre">true</span></code></p></li>
+</ul>
+</div></td>
+</tr>
+<tr class="row-odd"><td><div class="ansible-option-cell">
 <div class="ansibleOptionAnchor" id="parameter-state"></div><p class="ansible-option-title" id="ansible-collections-community-general-modprobe-module-parameter-state"><strong>state</strong></p>
 <a class="ansibleOptionLink" href="#parameter-state" title="Permalink to this option"></a><p class="ansible-option-type-line"><span class="ansible-option-type">string</span></p>
 </div></td>

[felixfontein]

Thanks for your contribution! I've added some first comments.

[haddystuff]

Made all the suggested changes

[felixfontein]

I've been looking at this PR for some time now, and I'm not sure whether this option (as a boolean toggle with a default value) is a good idea. How can you for example say "the module should be still loaded now, but not persistently anymore" with this toggle?

I first thought it might be better to update state to have two more values, presistent and not_persistent, so that present means "I don't care whether it is persistent", and absent implies "not persistent", but then I know too little about this to be sure this is an accurate description of what can be done / makes sense. I guess you could declare a module as persistent without having it loaded now? (Which would not be possible with these states.) Also since persistent has stronger requirements than the remainder of the module, coupling it to state=absent is a big problem.

[felixfontein]

Suggested change

	- Also this options work only with distributives that uses systemd.
	- Also this options work only with distributions that uses systemd.

[haddystuff]

Answering your first question - yes. That's the exact idea behind this feature. You need to set status to present and persistent to false. Basically if persistent set to false then this module behavior would be the same as it was before. One problem with this approach - if you previously added module persistently and want to disable persistency but module should still be activated - that's not gonna work in one task. I can fix this and check if module activated persistently with persistent option set to false and then disable it if needed. Also I had an idea of using persistent as multiple choice parameter with options like: loaded, unloaded, ignore(or some sort of value that you can use if you don't care about persistency). This could also be handy in future if we want to add more states like blacklist.

Second question - No, you cannot declare module as persistent without having it loaded now. As i already mentioned persistency is connected to state. You can just take a look at main function to understand how it works.

[felixfontein]

Please note that in #5461 the collection repository was restructured to remove the directory tree in plugins/modules/, and the corresponding tree in tests/unit/plugins/modules/. Your PR modifies files in this directory structure, and unfortunately now has some conflicts, potentially because of this. Please rebase with the current main branch to remove the conflicts.

[haddystuff]

Sorry that it took me so long, but I've made this rebase.
BTW what about my previous message? Should I change something?

[haddystuff]

Hi @felixfontein, can you please help me with understanding tests result: I don't quite get why this one test is failed. What should I change?

[felixfontein]

@haddystuff please simply ignore it, it will fail until you rebase with current main (please don't do that just for this test).

[haddystuff]

Thank you @felixfontein

[russoz]

Just one small improvement when compiling the regexps, other than that it LGTM

[haddystuff]

@felixfontein, all issues you and @russoz have mentioned are fixed

[felixfontein]

Looks good to me. I'll merge tomorrow evening if nobody objects.

[patchback]

Backport to stable-6: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-6/29f5033737a7fd86349ff3daab7d7ee7db66ad00/pr-5424

Backported as #6102

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

[felixfontein]

@haddystuff thanks for your contribution, and sorry it took so long to get this done!
@russoz thanks for reviewing this!