-
-
Notifications
You must be signed in to change notification settings - Fork 36.5k
Add Unraid integration #160581
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Add Unraid integration #160581
Conversation
This adds a new integration for Unraid servers using the unraid-api library. Features: - System monitoring (CPU, RAM, temperature, uptime) - Storage monitoring (array state, disk usage, parity status) - Docker container control (start/stop switches) - VM control (start/stop switches) - UPS monitoring (battery, load, runtime) - Binary sensors for disk health, array status, parity validity - Config flow with options for polling intervals and UPS configuration - Full diagnostics support - Platinum quality scale compliance
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems you haven't yet signed a CLA. Please do so here.
Once you do that we will be able to review and accept this pull request.
Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds a comprehensive new integration for Unraid servers using the unraid-api library (v1.2.0). The integration provides extensive monitoring and control capabilities for Unraid systems through sensors, binary sensors, switches, and buttons.
Key changes:
- New Unraid integration with config flow, coordinators, and multiple entity platforms
- Comprehensive test suite with 352 tests achieving ~98% coverage
- Support for Docker container and VM control, array management, parity checks, UPS monitoring, and disk operations
Reviewed changes
Copilot reviewed 38 out of 40 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/components/unraid/test_switch.py | Tests for Docker container and VM switches with error handling |
| tests/components/unraid/test_sensor.py | Extensive sensor tests covering system metrics, storage, and UPS |
| tests/components/unraid/test_models.py | Data model validation and parsing tests |
| tests/components/unraid/test_init.py | Integration setup, error handling, and teardown tests |
| tests/components/unraid/test_diagnostics.py | Diagnostics data collection tests |
| tests/components/unraid/test_coordinator*.py | Coordinator data fetching and error handling tests |
| tests/components/unraid/test_config_flow.py | Config/reauth/options/reconfigure flow tests |
| tests/components/unraid/test_button.py | Button entity and action tests |
| tests/components/unraid/fixtures/*.json | Test fixture data |
| tests/components/unraid/conftest.py | Shared test fixtures and helpers |
| homeassistant/components/unraid/switch.py | Switch entity implementations |
| requirements files | Added unraid-api==1.2.0 dependency |
| Generated files | Updated integrations.json, config_flows.py |
jpbede
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please limit the initial PR to one platform (sensor, button, switch, ..). Which makes reviewing the PR easier.
Per Home Assistant review process, new integrations should start with a single platform. This reduces the PR to sensor platform only. Follow-up PRs will add: - binary_sensor platform - button platform - switch platform
zweckj
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, thanks for your contribution. Please further reduce the size of this PR by
- removing reauthentication and diagnostics. You can add that in follow up PRs
- pydantic models and py.typed belong in the lib not into HA
- for the sensors return raw values (e.g. bytes), we can change those into the required unit through the sensor, if you really need to do larger conversions move that into the library
- use sensor entity descriptions for the sensors. Check other integrations in core how that is done. One example: lamarzocco
Thanks!
Per reviewer jpbede's feedback: - Remove reauthentication flow and related strings - Remove diagnostics module - Remove pydantic models (belong in library, not HA) - Remove py.typed (belongs in library) - Return raw values (bytes/kilobytes) instead of formatted strings - Use sensor entity descriptions pattern (like lamarzocco) - Remove binary_sensor, button, switch platforms (sensor only) - Mark diagnostics and reauthentication-flow as exempt in quality_scale.yaml - Update all tests to match new patterns The integration now uses: - DataUpdateCoordinator with dataclass-based data containers - Separate SensorEntityDescription classes for system vs storage sensors - suggested_unit_of_measurement for display conversion - Simplified config flow without reauth
- Remove EntityCategory.DIAGNOSTIC from sensors (moved to regular sensors) - Add UPS Power sensor (calculates watts from load% × nominal power) - Add CPU Power sensor from packages.totalPower - Add Flash device usage/used sensors - Fix string-to-number conversion for API values - Fix CPU temp/power extraction from packages dict structure - Fix share usage calculation (API returns size=0, use used+free) - Format uptime and UPS runtime as human-readable duration strings - Remove unused pydantic models (belong in library, not HA) - Update tests for all changes
Thanks for the feedback. I have updated the code. |
zweckj
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I started a first pass, but you'll need to move a lot of things to the library. HA should not do graphQL requests and shouldn't know about the request structure it should only work with the return (ideally data)classes. Data wrangling and preparation (unless HA specific) should also go into the library
| # ============================================================================= | ||
| # Sensor Types | ||
| # ============================================================================= | ||
| SENSOR_CPU_USAGE: Final = "cpu_usage" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if it's only used in one file (e.g. sensors), better to define it there, but I don't think it's used at all, so remove it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
| # Icons - Material Design Icons (mdi:) | ||
| # ============================================================================= | ||
| # System | ||
| ICON_CPU: Final = "mdi:cpu-64-bit" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
icons go into icon translations file (icons.json)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
| # ============================================================================= | ||
| # Configuration Keys | ||
| # ============================================================================= | ||
| CONF_SYSTEM_INTERVAL: Final = "system_interval" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we don't allow user definable poll intervals
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Updated
| # Integration Info | ||
| # ============================================================================= | ||
| DOMAIN: Final = "unraid" | ||
| INTEGRATION_VERSION: Final = "2026.01.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not a thing in core
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Updated
| return True | ||
| return current >= minimum | ||
|
|
||
| async def async_step_reconfigure( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove that as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
| def _handle_http_error(self, err: aiohttp.ClientResponseError, host: str) -> None: | ||
| """Handle HTTP errors from API client.""" | ||
| if err.status in (401, 403): | ||
| msg = "Invalid API key or insufficient permissions" | ||
| raise InvalidAuthError(msg) from err | ||
| msg = f"HTTP error {err.status}: {err.message}" | ||
| raise CannotConnectError(msg) from err | ||
|
|
||
| def _handle_generic_error(self, err: Exception) -> None: | ||
| """Handle generic errors, mapping to appropriate exception types.""" | ||
| error_str = str(err).lower() | ||
| if "401" in error_str or "unauthorized" in error_str: | ||
| msg = "Invalid API key or insufficient permissions" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we shouldn't be hanlding those kinds of errors in HA (like aiohttp) the lib should handle that and raise purpuseful errors
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should now be fixed and handled by unraid-api library
| info_query = """ | ||
| query { | ||
| info { | ||
| system { uuid } | ||
| os { hostname } | ||
| } | ||
| } | ||
| """ | ||
| info_data = await api_client.query(info_query) | ||
| info = info_data.get("info", {}) | ||
| system = info.get("system", {}) | ||
| os_info = info.get("os", {}) | ||
|
|
||
| self._server_uuid = system.get("uuid") | ||
| self._server_hostname = os_info.get("hostname") or host |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's also library code (in general). Build a get_server_infoor similar method there and ideaally return a dataclass
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should now be fixed and handled by unraid-api library
| query SystemInfo { | ||
| info { | ||
| system { uuid manufacturer model serial } | ||
| baseboard { manufacturer model serial } | ||
| os { hostname distro release kernel arch } | ||
| cpu { manufacturer brand cores threads } | ||
| versions { core { unraid api } } | ||
| } | ||
| server { lanip localurl remoteurl } | ||
| registration { type state } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
libary code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should now be fixed and handled by unraid-api library
| __all__ = ["DOMAIN", "UnraidConfigEntry", "UnraidRuntimeData"] | ||
|
|
||
|
|
||
| def _build_server_info(info: dict, host: str, verify_ssl: bool) -> dict: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also rather belongs into the library (at least main parts of it)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should now be fixed and handled by unraid-api library
- Add CPU temperature and CPU power sensors (unraid-api 1.3.1) - Add RAM, swap, Docker, VM, notification, array, parity sensors - Add UPS voltage and disk sensors - Update quality_scale to bronze per reviewer feedback - Mark reauthentication-flow and reconfiguration-flow as todo - Fix parity sensors to show 0 instead of Unknown - Update flash device to show in GiB
Reduced it to a sensor for now and made additional updates as per @zweckj comments |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two small notes from a translator's perspective.
| CONF_HTTP_PORT = "http_port" | ||
| CONF_HTTPS_PORT = "https_port" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
those should be in conf, because they are use in more files
| errors[CONF_HOST] = "cannot_connect" | ||
| errors[CONF_HTTPS_PORT] = "check_port" | ||
| except UnsupportedVersionError: | ||
| errors["base"] = "unsupported_version" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in those cases where the user can't do something to fix it through changing info/retrying we should abort the flow
|
|
||
|
|
||
| # System sensor descriptions - using Pydantic model attributes | ||
| SYSTEM_SENSORS: tuple[UnraidSystemSensorEntityDescription, ...] = ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please do two things here:
- create a base entity containing the device info and the has_entity_name
- limit to a selection (on type of sesnors) for now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I followed the La Marzocco integration pattern and created the base entity and then reduced the sensors.
| if not self._server_uuid: | ||
| errors["base"] = "no_server_uuid" | ||
| else: | ||
| await self.async_set_unique_id(self._server_uuid) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how would a user fix that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be fixed now
| error_str = str(err).lower() | ||
| if "ssl" in error_str or "certificate" in error_str: | ||
| msg = f"SSL error: {err}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this seems a bit hacky tbh, can't you define a custom exceptions in the lib for that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The library handles it and I have updated the code
| api_client=api_client, | ||
| system_coordinator=system_coordinator, | ||
| storage_coordinator=storage_coordinator, | ||
| server_info=server_info, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need to store the api client? I think the server info should be passed into the coordinators instead, you already pass the name, why not pass the entire server info? What actually is contained in the info and how often does that change?
| unload_ok = await hass.config_entries.async_unload_platforms(entry, PLATFORMS) | ||
|
|
||
| if unload_ok: | ||
| await entry.runtime_data.api_client.close() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is IO happening during close?
| if TYPE_CHECKING: | ||
| from homeassistant.config_entries import ConfigFlowResult |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you shouldn't need type_checking for that
| vol.Coerce(int), vol.Range(min=1, max=65535) | ||
| ), | ||
| vol.Optional(CONF_HTTPS_PORT, default=DEFAULT_HTTPS_PORT): vol.All( | ||
| vol.Coerce(int), vol.Range(min=1, max=65535) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why are you splitting the port into two settings? Either we use HTTPs or we don't? Even though you can have both on unraid, I think we should let the user choose one for the communication.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The separate ports are needed to match Unraid's actual configuration options where users can have non-standard ports for both protocols.
The two-port design is necessary because:
- Unraid allows users to configure different ports for HTTP and HTTPS
- Unraid has three SSL modes: No, Yes, and Strict.
- The library's "SSL auto-discovery" feature handles this by trying HTTPS first, then falling back to HTTP. This is for accessing the api on http/s://unraidservername/graphql
Co-authored-by: Norbert Rittel <[email protected]>
Co-authored-by: Josef Zweck <[email protected]>
Co-authored-by: Norbert Rittel <[email protected]>
Co-authored-by: Josef Zweck <[email protected]>
Co-authored-by: Josef Zweck <[email protected]>
Reviewer feedback implementation: - Move port constants (CONF_HTTP_PORT, CONF_HTTPS_PORT, defaults) to const.py - Change UnsupportedVersionError and no_server_uuid from errors to async_abort() - Simplify exception handling in _validate_connection - Remove api_client.close() in __init__.py (HA manages session lifecycle) - Create base entity class (entity.py) with has_entity_name=True - Drastically reduce sensors to 5 core system metrics: - CPU usage (%) - CPU temperature - RAM usage (%) - RAM used (bytes) - Uptime - Remove storage coordinator entirely - Remove UPS, Docker, VM, storage, disk, share, parity sensors - Simplify quality_scale.yaml to Bronze requirements - Update strings.json for simplified sensors - Simplify test files to match new structure - Remove unused fixture files
- Update to unraid-api 1.4.0 with UnraidSSLError exception - Use AwesomeVersion instead of packaging.version - Remove TYPE_CHECKING for directly imported types - Refactor config flow to use errors dict pattern (no custom exceptions) - Remove api_client from UnraidRuntimeData (coordinator stores it) - Remove connection recovery logging from coordinator - Simplify __init__.py (inline variables, remove test_connection) - Pass server_info to coordinator - Remove all unused code (_LOGGER, MIN_UNRAID_VERSION) - Clean up imports and remove string type annotations - Follow La Marzocco integration patterns All review comments from PR home-assistant#160581 addressed. Tests: 46 passed, all linters passing.
What does this implement/fix?
This PR adds a new integration for Unraid servers, using the unraid-api library.
Features
Sensors
Type of change
Checklist
pytestruff formatpre-commit run --all-files)Test Coverage
All modules have >95% test coverage:
__init__.py,config_flow.py,coordinator.py,sensor.pyLibrary
This integration uses the unraid-api library which provides: