Skip to content

Allow dynamic instances of non-hotplug plugins#735

Open
adriaan42 wants to merge 1 commit into
redhat-performance:masterfrom
adriaan42:adriaan/dynamic-non-device-plugin-instances
Open

Allow dynamic instances of non-hotplug plugins#735
adriaan42 wants to merge 1 commit into
redhat-performance:masterfrom
adriaan42:adriaan/dynamic-non-device-plugin-instances

Conversation

@adriaan42

Copy link
Copy Markdown
Contributor

Split off #628 for easier review and discussion.

This removes the checks for "is the plugin of type hotplug.Plugin" from instance_create and instance_destroy, while keeping the rest of the code in controller.py as-is. To do this, some of the device related methods need noop-implementations in base.Plugin.

Notes:

  • base.Plugin seems to implement some device methods with actual logic (assign_free_devices, release_devices, _run_for_each_device, _call_device_script, _*_device_commands). Why are those not in hotplug.Plugin?
  • In create_instance, we could add checking and warnings if the user creates a base.Plugin instance and sets the devices= or devices_udev_regex= options. At the moment these options would be ignored silently (which is also ok I guess).

@adriaan42 adriaan42 force-pushed the adriaan/dynamic-non-device-plugin-instances branch from b4c28f6 to 0c9446e Compare February 28, 2025 07:45
@adriaan42 adriaan42 force-pushed the adriaan/dynamic-non-device-plugin-instances branch from 4ccee1f to 1c81bc0 Compare March 10, 2025 07:24
@adriaan42 adriaan42 force-pushed the adriaan/dynamic-non-device-plugin-instances branch from 1c81bc0 to 21bc0b4 Compare May 27, 2025 12:17
@adriaan42 adriaan42 force-pushed the adriaan/dynamic-non-device-plugin-instances branch from 21bc0b4 to eece4fe Compare February 23, 2026 17:15
Signed-off-by: Adriaan Schmidt <adriaan.schmidt@siemens.com>
@adriaan42 adriaan42 force-pushed the adriaan/dynamic-non-device-plugin-instances branch from eece4fe to 2909323 Compare June 5, 2026 13:20
@coderabbitai

coderabbitai Bot commented Jun 5, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Enterprise

Run ID: 1c6a3013-a732-43e0-8434-f9f61453d8cf

📥 Commits

Reviewing files that changed from the base of the PR and between 0eb28ac and 2909323.

📒 Files selected for processing (3)
  • tests/unit/plugins/test_base.py
  • tuned/daemon/controller.py
  • tuned/plugins/base.py
💤 Files with no reviewable changes (1)
  • tuned/daemon/controller.py

📝 Walkthrough

Summary by CodeRabbit

  • Tests

    • Improved test coverage for plugin device initialization and matching logic.
  • Refactor

    • Simplified plugin controller logic by consolidating error handling.
    • Enhanced plugin base architecture for consistent device management support.

Walkthrough

The PR establishes a device management abstraction pattern for the tuned plugin system. The base Plugin class gains placeholder device-handling methods and a guard to safely return empty results when device support is disabled. The Controller simplifies instance lifecycle methods by removing explicit type checks, delegating error handling to exceptions. Tests are updated to explicitly initialize device state in the DummyPlugin test double and call init_devices() before device-matching operations.

Changes

Device Support Infrastructure and Controller Adaptation

Layer / File(s) Summary
Base Plugin device support infrastructure
tuned/plugins/base.py
Plugin class adds placeholder no-op methods for device management (_add_device, _add_devices_nocheck, _remove_device, _remove_devices_nocheck) and guards _get_matching_devices to return an empty set when _devices_supported is false.
Controller hotplug.Plugin type check removal
tuned/daemon/controller.py
instance_create and instance_destroy no longer explicitly validate that the plugin is a hotplug.Plugin; they proceed directly to operations and rely on existing exception handlers to report failures.
Test double device initialization
tests/unit/plugins/test_base.py
DummyPlugin now implements _init_devices to enable device behavior and mark _devices_supported = True; test cases explicitly call instance.plugin.init_devices() before _get_matching_devices in both udev-regex and non-udev-regex paths.

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 18.18% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: allowing dynamic instances of non-hotplug plugins by removing type-checking restrictions.
Description check ✅ Passed The description is related to the changeset, explaining the removal of hotplug.Plugin type checks and the addition of noop device methods in base.Plugin.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant