Skip to content

Commit 263f8d0

Browse files
committed
bootloader: Add rpm-ostree kargs --source=tuned support
On bootc systems with transient /etc, the state file /etc/tuned/bootcmdline is lost after each reboot. This causes TuneD to lose track of which kernel arguments it previously set, leading to kargs stacking up on every reboot cycle. The rpm-ostree --source flag solves this by tracking kargs ownership directly in the BLS config on /boot, which persists across reboots regardless of /etc transience. With --source=tuned, rpm-ostree automatically removes all previous kargs from the "tuned" source and replaces them with the new set, eliminating the need for TuneD to maintain its own state for diffing. This commit adds --source support to the bootloader plugin: 1. Detect --source availability by checking rpm-ostree kargs --help output during plugin init. 2. When --source is available, use "rpm-ostree kargs --source=tuned --append=X --append=Y" for applying kargs and "rpm-ostree kargs --source=tuned" (no --append) for clearing all TuneD-owned kargs on profile removal. 3. When --source is not available (older rpm-ostree), fall back to the existing --delete/--append + bootcmdline state file tracking. No legacy code paths are modified. 4. Continue writing to /etc/tuned/bootcmdline as best-effort for diagnostics and backward compatibility, but do not depend on it for correctness when --source is available. 5. Add unit tests for the bootloader plugin (the first ever in the codebase): 15 tests covering --source detection, apply, removal, dispatch logic, error handling, and fallback paths. Assisted-by: OpenCode (Claude Opus 4.6) Signed-off-by: Joseph Marrero Corchado <jmarrero@redhat.com>
1 parent 38d4414 commit 263f8d0

2 files changed

Lines changed: 326 additions & 0 deletions

File tree

Lines changed: 274 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,274 @@
1+
import unittest
2+
3+
try:
4+
from unittest.mock import Mock, patch, call
5+
except ImportError:
6+
from mock import Mock, patch, call
7+
8+
import tuned.consts as consts
9+
10+
11+
class BootloaderPluginSourceTestCase(unittest.TestCase):
12+
"""Tests for the rpm-ostree --source=tuned integration in the bootloader plugin.
13+
14+
These tests exercise the --source detection, source-based kargs apply,
15+
source-based kargs removal, and the dispatch logic that falls back to
16+
the legacy code path when --source is not available.
17+
18+
The BootloaderPlugin constructor requires /etc/grub.d/00_tuned to exist,
19+
so we test the methods directly on a minimal mock rather than
20+
instantiating the full plugin.
21+
"""
22+
23+
def _make_plugin_mock(self, has_source=True, rpm_ostree_idle=True):
24+
"""Create a mock that has the real methods under test patched onto it.
25+
26+
This avoids instantiating BootloaderPlugin (which requires GRUB2
27+
template files on disk) while still testing the actual method logic.
28+
"""
29+
from tuned.plugins.plugin_bootloader import BootloaderPlugin
30+
31+
mock_plugin = Mock(spec=BootloaderPlugin)
32+
mock_plugin._rpm_ostree_has_source = has_source
33+
mock_plugin._cmdline_val = ""
34+
mock_plugin._cmd = Mock()
35+
mock_plugin._cmd.add_modify_option_in_file = Mock(return_value=True)
36+
37+
# Make _wait_till_rpm_ostree_idle return the desired value
38+
mock_plugin._wait_till_rpm_ostree_idle = Mock(return_value=rpm_ostree_idle)
39+
40+
# Bind real methods to the mock so we test actual logic
41+
mock_plugin._rpm_ostree_has_source_flag = \
42+
lambda: BootloaderPlugin._rpm_ostree_has_source_flag(mock_plugin)
43+
mock_plugin._rpm_ostree_source_update = \
44+
lambda: BootloaderPlugin._rpm_ostree_source_update(mock_plugin)
45+
mock_plugin._remove_rpm_ostree_source_tuning = \
46+
lambda: BootloaderPlugin._remove_rpm_ostree_source_tuning(mock_plugin)
47+
mock_plugin._rpm_ostree_update = \
48+
lambda: BootloaderPlugin._rpm_ostree_update(mock_plugin)
49+
mock_plugin._remove_rpm_ostree_tuning = \
50+
lambda: BootloaderPlugin._remove_rpm_ostree_tuning(mock_plugin)
51+
mock_plugin._patch_bootcmdline = \
52+
lambda d: BootloaderPlugin._patch_bootcmdline(mock_plugin, d)
53+
54+
return mock_plugin
55+
56+
# ------------------------------------------------------------------
57+
# _rpm_ostree_has_source_flag() detection
58+
# ------------------------------------------------------------------
59+
60+
def test_has_source_flag_present(self):
61+
"""--source flag is detected when present in rpm-ostree kargs --help."""
62+
plugin = self._make_plugin_mock()
63+
help_text = (
64+
"Usage:\n"
65+
" rpm-ostree kargs [OPTION...]\n\n"
66+
" --append=KEY=VALUE Append kernel argument\n"
67+
" --source=NAME Track kargs ownership by source name\n"
68+
" --delete=KEY=VALUE Delete a kernel argument\n"
69+
)
70+
plugin._cmd.execute = Mock(return_value=(0, help_text, ""))
71+
self.assertTrue(plugin._rpm_ostree_has_source_flag())
72+
plugin._cmd.execute.assert_called_once_with(
73+
['rpm-ostree', 'kargs', '--help'], return_err=True)
74+
75+
def test_has_source_flag_absent(self):
76+
"""--source flag is not detected when absent from rpm-ostree kargs --help."""
77+
plugin = self._make_plugin_mock()
78+
help_text = (
79+
"Usage:\n"
80+
" rpm-ostree kargs [OPTION...]\n\n"
81+
" --append=KEY=VALUE Append kernel argument\n"
82+
" --delete=KEY=VALUE Delete a kernel argument\n"
83+
)
84+
plugin._cmd.execute = Mock(return_value=(0, help_text, ""))
85+
self.assertFalse(plugin._rpm_ostree_has_source_flag())
86+
87+
def test_has_source_flag_in_stderr(self):
88+
"""--source flag is detected even if it appears in stderr."""
89+
plugin = self._make_plugin_mock()
90+
plugin._cmd.execute = Mock(return_value=(0, "", "--source=NAME Track kargs"))
91+
self.assertTrue(plugin._rpm_ostree_has_source_flag())
92+
93+
def test_has_source_flag_help_fails(self):
94+
"""If rpm-ostree kargs --help fails, --source is not detected."""
95+
plugin = self._make_plugin_mock()
96+
plugin._cmd.execute = Mock(return_value=(1, "", "command not found"))
97+
self.assertFalse(plugin._rpm_ostree_has_source_flag())
98+
99+
# ------------------------------------------------------------------
100+
# _rpm_ostree_source_update()
101+
# ------------------------------------------------------------------
102+
103+
def test_source_update_basic(self):
104+
"""--source=tuned is called with correct --append flags."""
105+
plugin = self._make_plugin_mock()
106+
plugin._cmdline_val = "nohz=full isolcpus=1-3"
107+
plugin._cmd.execute = Mock(return_value=(0, "", ""))
108+
109+
plugin._rpm_ostree_source_update()
110+
111+
plugin._cmd.execute.assert_called_once_with(
112+
["rpm-ostree", "kargs", "--source=tuned",
113+
"--append=nohz=full", "--append=isolcpus=1-3"],
114+
return_err=True)
115+
# Verify bootcmdline state file is updated
116+
plugin._cmd.add_modify_option_in_file.assert_called_once_with(
117+
consts.BOOT_CMDLINE_FILE,
118+
{consts.BOOT_CMDLINE_TUNED_VAR: "nohz=full isolcpus=1-3"})
119+
120+
def test_source_update_empty_cmdline(self):
121+
"""When cmdline is empty, only --source=tuned is passed (clears kargs)."""
122+
plugin = self._make_plugin_mock()
123+
plugin._cmdline_val = ""
124+
plugin._cmd.execute = Mock(return_value=(0, "", ""))
125+
126+
plugin._rpm_ostree_source_update()
127+
128+
plugin._cmd.execute.assert_called_once_with(
129+
["rpm-ostree", "kargs", "--source=tuned"],
130+
return_err=True)
131+
plugin._cmd.add_modify_option_in_file.assert_called_once_with(
132+
consts.BOOT_CMDLINE_FILE,
133+
{consts.BOOT_CMDLINE_TUNED_VAR: ""})
134+
135+
def test_source_update_rpm_ostree_busy(self):
136+
"""When rpm-ostree is busy, source update does not execute."""
137+
plugin = self._make_plugin_mock(rpm_ostree_idle=False)
138+
plugin._cmdline_val = "nohz=full"
139+
plugin._cmd.execute = Mock()
140+
141+
plugin._rpm_ostree_source_update()
142+
143+
plugin._cmd.execute.assert_not_called()
144+
plugin._cmd.add_modify_option_in_file.assert_not_called()
145+
146+
def test_source_update_command_fails(self):
147+
"""When rpm-ostree kargs --source fails, bootcmdline is not updated."""
148+
plugin = self._make_plugin_mock()
149+
plugin._cmdline_val = "nohz=full"
150+
plugin._cmd.execute = Mock(return_value=(1, "", "error: unknown option"))
151+
152+
plugin._rpm_ostree_source_update()
153+
154+
plugin._cmd.execute.assert_called_once()
155+
plugin._cmd.add_modify_option_in_file.assert_not_called()
156+
157+
# ------------------------------------------------------------------
158+
# _remove_rpm_ostree_source_tuning()
159+
# ------------------------------------------------------------------
160+
161+
def test_remove_source_tuning(self):
162+
"""Removal calls --source=tuned with no --append (clears all)."""
163+
plugin = self._make_plugin_mock()
164+
plugin._cmd.execute = Mock(return_value=(0, "", ""))
165+
166+
plugin._remove_rpm_ostree_source_tuning()
167+
168+
plugin._cmd.execute.assert_called_once_with(
169+
["rpm-ostree", "kargs", "--source=tuned"],
170+
return_err=True)
171+
plugin._cmd.add_modify_option_in_file.assert_called_once_with(
172+
consts.BOOT_CMDLINE_FILE,
173+
{consts.BOOT_CMDLINE_TUNED_VAR: ""})
174+
175+
def test_remove_source_tuning_rpm_ostree_busy(self):
176+
"""When rpm-ostree is busy, source removal does not execute."""
177+
plugin = self._make_plugin_mock(rpm_ostree_idle=False)
178+
plugin._cmd.execute = Mock()
179+
180+
plugin._remove_rpm_ostree_source_tuning()
181+
182+
plugin._cmd.execute.assert_not_called()
183+
# bootcmdline should not be touched either
184+
plugin._cmd.add_modify_option_in_file.assert_not_called()
185+
186+
def test_remove_source_tuning_command_fails(self):
187+
"""When --source removal fails, bootcmdline is still cleared (best-effort)."""
188+
plugin = self._make_plugin_mock()
189+
plugin._cmd.execute = Mock(return_value=(1, "", "error"))
190+
191+
plugin._remove_rpm_ostree_source_tuning()
192+
193+
# bootcmdline should still be cleared even on failure
194+
plugin._cmd.add_modify_option_in_file.assert_called_once_with(
195+
consts.BOOT_CMDLINE_FILE,
196+
{consts.BOOT_CMDLINE_TUNED_VAR: ""})
197+
198+
# ------------------------------------------------------------------
199+
# _rpm_ostree_update() dispatch
200+
# ------------------------------------------------------------------
201+
202+
def test_rpm_ostree_update_dispatches_to_source(self):
203+
"""When --source is available, _rpm_ostree_update uses the source path."""
204+
plugin = self._make_plugin_mock(has_source=True)
205+
plugin._cmdline_val = "nohz=full"
206+
plugin._cmd.execute = Mock(return_value=(0, "", ""))
207+
208+
plugin._rpm_ostree_update()
209+
210+
# Should have called --source=tuned
211+
plugin._cmd.execute.assert_called_once_with(
212+
["rpm-ostree", "kargs", "--source=tuned", "--append=nohz=full"],
213+
return_err=True)
214+
215+
def test_rpm_ostree_update_fallback_when_no_source(self):
216+
"""When --source is not available, _rpm_ostree_update uses the legacy path."""
217+
from tuned.plugins.plugin_bootloader import BootloaderPlugin
218+
219+
plugin = self._make_plugin_mock(has_source=False)
220+
plugin._cmdline_val = "nohz=full"
221+
222+
# Set up legacy path dependencies
223+
plugin._get_appended_rpm_ostree_kargs = Mock(return_value=["old_karg=1"])
224+
plugin._get_rpm_ostree_kargs = Mock(return_value="root=UUID=xxx old_karg=1")
225+
plugin._modify_rpm_ostree_kargs = Mock(return_value=True)
226+
227+
# Bind _rpm_ostree_update with the real method
228+
# but we need to also bind _patch_bootcmdline for the legacy path
229+
plugin._rpm_ostree_update = \
230+
lambda: BootloaderPlugin._rpm_ostree_update(plugin)
231+
232+
plugin._rpm_ostree_update()
233+
234+
# Should NOT have called --source, should use legacy delete/append
235+
plugin._modify_rpm_ostree_kargs.assert_called_once_with(
236+
delete_kargs=["old_karg=1"], append_kargs=["nohz=full"])
237+
238+
# ------------------------------------------------------------------
239+
# _remove_rpm_ostree_tuning() dispatch
240+
# ------------------------------------------------------------------
241+
242+
def test_remove_rpm_ostree_tuning_dispatches_to_source(self):
243+
"""When --source is available, removal uses the source path."""
244+
plugin = self._make_plugin_mock(has_source=True)
245+
plugin._cmd.execute = Mock(return_value=(0, "", ""))
246+
247+
plugin._remove_rpm_ostree_tuning()
248+
249+
plugin._cmd.execute.assert_called_once_with(
250+
["rpm-ostree", "kargs", "--source=tuned"],
251+
return_err=True)
252+
253+
def test_remove_rpm_ostree_tuning_fallback_when_no_source(self):
254+
"""When --source is not available, removal uses the legacy path."""
255+
from tuned.plugins.plugin_bootloader import BootloaderPlugin
256+
257+
plugin = self._make_plugin_mock(has_source=False)
258+
plugin._get_appended_rpm_ostree_kargs = Mock(return_value=["old=1", "old=2"])
259+
plugin._modify_rpm_ostree_kargs = Mock(return_value=True)
260+
261+
plugin._remove_rpm_ostree_tuning = \
262+
lambda: BootloaderPlugin._remove_rpm_ostree_tuning(plugin)
263+
264+
plugin._remove_rpm_ostree_tuning()
265+
266+
plugin._modify_rpm_ostree_kargs.assert_called_once_with(
267+
delete_kargs=["old=1", "old=2"])
268+
plugin._cmd.add_modify_option_in_file.assert_called_once_with(
269+
consts.BOOT_CMDLINE_FILE,
270+
{consts.BOOT_CMDLINE_TUNED_VAR: ""})
271+
272+
273+
if __name__ == '__main__':
274+
unittest.main()

tuned/plugins/plugin_bootloader.py

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,7 @@ def _instance_init(self, instance):
193193
self._bls = self._bls_enabled()
194194

195195
self._rpm_ostree = self._rpm_ostree_status() is not None
196+
self._rpm_ostree_has_source = self._rpm_ostree and self._rpm_ostree_has_source_flag()
196197

197198
def _instance_cleanup(self, instance):
198199
pass
@@ -223,6 +224,12 @@ def _rpm_ostree_status(self):
223224
return None
224225
return splited[1]
225226

227+
def _rpm_ostree_has_source_flag(self):
228+
"""Check if the installed rpm-ostree supports --source for kargs."""
229+
(rc, out, err) = self._cmd.execute(
230+
['rpm-ostree', 'kargs', '--help'], return_err=True)
231+
return '--source=' in (out + err)
232+
226233
def _wait_till_rpm_ostree_idle(self):
227234
"""Check that rpm-ostree is idle, allowing some waiting time."""
228235
sleep_cycles = 10
@@ -263,6 +270,45 @@ def _modify_rpm_ostree_kargs(self, delete_kargs=[], append_kargs=[]):
263270
return False
264271
return True
265272

273+
def _rpm_ostree_source_update(self):
274+
"""Apply kernel parameter tuning using rpm-ostree --source tracking.
275+
276+
With --source=tuned, rpm-ostree automatically removes all previous
277+
kargs from the 'tuned' source and replaces them with the new set.
278+
This eliminates the need to track state in /etc/tuned/bootcmdline,
279+
which is critical for bootc systems with transient /etc.
280+
"""
281+
if not self._wait_till_rpm_ostree_idle():
282+
log.error("Error modifying rpm-ostree kargs: rpm-ostree is busy")
283+
return
284+
cmd = ["rpm-ostree", "kargs", "--source=tuned"]
285+
for karg in self._cmdline_val.split():
286+
cmd.append("--append=%s" % karg)
287+
(rc, _, err) = self._cmd.execute(cmd, return_err=True)
288+
if rc != 0:
289+
log.error("Error applying rpm-ostree kargs with --source: %s" % err)
290+
return
291+
# Best-effort state file write for diagnostics and backward compat
292+
self._patch_bootcmdline({consts.BOOT_CMDLINE_TUNED_VAR: self._cmdline_val})
293+
294+
def _remove_rpm_ostree_source_tuning(self):
295+
"""Remove kernel parameter tuning using rpm-ostree --source tracking.
296+
297+
Calling --source=tuned with no --append flags clears all kargs
298+
owned by the 'tuned' source.
299+
"""
300+
if not self._wait_till_rpm_ostree_idle():
301+
log.error("Error removing rpm-ostree kargs: rpm-ostree is busy")
302+
return
303+
(rc, _, err) = self._cmd.execute(
304+
["rpm-ostree", "kargs", "--source=tuned"], return_err=True)
305+
if rc != 0:
306+
log.error("Error clearing rpm-ostree kargs source: %s" % err)
307+
# Clear the state file even on rpm-ostree failure to prevent stale
308+
# state from causing incorrect diffs on the next legacy-path apply.
309+
# This matches the behavior of the legacy _remove_rpm_ostree_tuning().
310+
self._patch_bootcmdline({consts.BOOT_CMDLINE_TUNED_VAR: ""})
311+
266312
def _get_effective_options(self, options):
267313
"""Merge provided options with plugin default options and merge all cmdline.* options."""
268314
effective = self._get_config_options().copy()
@@ -335,6 +381,9 @@ def _get_appended_rpm_ostree_kargs(self):
335381

336382
def _remove_rpm_ostree_tuning(self):
337383
"""Remove kernel parameter tuning in a rpm-ostree system."""
384+
if self._rpm_ostree_has_source:
385+
self._remove_rpm_ostree_source_tuning()
386+
return
338387
self._modify_rpm_ostree_kargs(delete_kargs=self._get_appended_rpm_ostree_kargs())
339388
self._patch_bootcmdline({consts.BOOT_CMDLINE_TUNED_VAR: ""})
340389

@@ -447,6 +496,9 @@ def _grub2_cfg_patch(self, d):
447496

448497
def _rpm_ostree_update(self):
449498
"""Apply kernel parameter tuning in a rpm-ostree system."""
499+
if self._rpm_ostree_has_source:
500+
self._rpm_ostree_source_update()
501+
return
450502
appended_kargs = self._get_appended_rpm_ostree_kargs()
451503
profile_kargs = self._cmdline_val.split()
452504
active_kargs = self._get_rpm_ostree_kargs()

0 commit comments

Comments
 (0)