Add tun to static device list in hardware manager #5547
Conversation
📝 WalkthroughWalkthroughThe changes introduce a new static device node mechanism for the "tun" device in the Changes
Sequence DiagramsequenceDiagram
participant HM as HardwareManager
participant Udev as Udev Database
participant StaticNodes as Static Nodes
HM->>Udev: Attempt to import devices
alt Devices imported successfully
Udev-->>HM: Return device list
else Specific device not found
HM->>StaticNodes: Check static nodes
StaticNodes-->>HM: Add missing device
end
The sequence diagram illustrates the enhanced device import process, showing how static nodes are integrated when udev fails to provide a specific device. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
supervisor/hardware/manager.py (3)
19-26: Fix typo in comment.There's a typo in the word "expoesed" on line 24.
-# Which nodes are expoesed by module is system specific, so ideally Supervisor +# Which nodes are exposed by module is system specific, so ideally Supervisor
27-44: Consider adding a type hint for device attributes.The implementation looks good! For better code maintainability, consider adding a type hint for the device attributes dictionary.
_STATIC_NODES: list[Device] = [ Device( "tun", Path("/dev/net/tun"), Path("/sys/devices/virtual/misc/tun"), "misc", None, [], - { + dict[str, str]({ "DEVNAME": "/dev/net/tun", "DEVPATH": "/devices/virtual/misc/tun", "MAJOR": "10", "MINOR": "200", "SUBSYSTEM": "misc", - }, + }), [], ) ]
144-147: Consider adding debug logging for static node addition.The implementation looks good! For better observability, consider adding debug logging when a static node is added.
# Add static nodes if not found through udev (e.g. module not yet loaded) for device in _STATIC_NODES: if device.name not in self._devices: + _LOGGER.debug("Adding static device node: %s", device.name) self._devices[device.name] = device
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
supervisor/hardware/manager.py(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Run tests Python 3.12.8
🔇 Additional comments (1)
supervisor/hardware/manager.py (1)
27-44: Verify the integration with existing device handling.Let's verify the usage and impact of the tun device in the codebase:
Also applies to: 144-147
✅ Verification successful
Static tun device implementation follows established patterns
The implementation correctly follows the existing device handling patterns in the codebase. The Device class usage and misc subsystem handling are consistent with other devices, and the static definition won't impact existing functionality.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for tun device usage and potential impacts # Check for tun device references in the codebase echo "=== Checking for tun device references ===" rg -l "tun" --type python # Check for other static device handling echo -e "\n=== Checking for similar device handling patterns ===" rg -l "static.*device" --type python # Check for Device class usage patterns echo -e "\n=== Checking Device class usage patterns ===" ast-grep --pattern 'Device($$$)' --lang pythonLength of output: 26506
agners
force-pushed
the
add-static-device-list-to-hardware-manager
branch
from
9a0d536 to
cf9d55e
Compare
Some devices are provided by kernel modules which potentially get loaded later at startup. Those are not listed by udev, and hence add-ons do not get permissions for these types of devices as long as the kernel module is not loaded. Typically, such devices are created by the kmod-static-nodes.service systemd service. Ideally, we would read the output of that service and add those specifically. However, there are very few devices which use static nodes, and we actually only really interested in tun. So let's simply add this static node in case udev does not list it already.
agners
force-pushed
the
add-static-device-list-to-hardware-manager
branch
from
cf9d55e to
b04d5e7
Compare
Some devices are provided by kernel modules which potentially get loaded later at startup. Those are not listed by udev, and hence add-ons do not get permissions for these types of devices as long as the kernel module is not loaded.
Typically, such devices are created by the kmod-static-nodes.service systemd service. Ideally, we would read the output of that service and add those specifically. However, there are very few devices which use static nodes, and we actually only really interested in tun. So let's simply add this static node in case udev does not list it already.
Proposed change
Type of change
Additional information
Checklist
ruff format supervisor tests)If API endpoints or add-on configuration are added/changed:
Summary by CodeRabbit
New Features
Improvements