Update supervisor immediately on new version #5375
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThe changes include modifications to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Tasks
participant Supervisor
User->>Tasks: Trigger _reload_updater()
Tasks->>Supervisor: Check for new versions
alt New version available
Tasks->>Supervisor: Trigger update
Supervisor-->>Tasks: Update successful
else No new version
Supervisor-->>Tasks: No update needed
end
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: 2
🧹 Outside diff range and nitpick comments (3)
tests/misc/test_tasks.py (1)
172-210: Consider enhancing test coverage.While the current test cases cover the happy path well, consider adding:
- Error scenarios (e.g., network failures during version check)
- Edge cases using pytest.mark.parametrize for different version combinations
Example structure:
@pytest.mark.parametrize( "current_version,new_version,should_update", [ ("2024.10.0", "2024.10.1", True), ("2024.10.0", "2024.10.0", False), ("2024.10.1", "2024.10.0", False), # Downgrade scenario ], ) async def test_reload_updater_version_scenarios( tasks, coresys, current_version, new_version, should_update ): # Test implementationsupervisor/misc/tasks.py (2)
338-345: Consider enhancing the update process with logging and rate limiting.While the implementation correctly checks for updates and triggers immediate supervisor updates, consider these improvements:
- Add logging when a new version is detected to help with troubleshooting
- Consider adding rate limiting to prevent too frequent update checks
- Add error handling for the update process
Here's a suggested enhancement:
@Job(name="tasks_reload_updater") async def _reload_updater(self) -> None: """Check for new versions of Home Assistant, Supervisor, OS, etc.""" await self.sys_updater.reload() # If there's a new version of supervisor, start update immediately if self.sys_supervisor.need_update: + _LOGGER.info( + "New supervisor version %s detected, triggering immediate update", + self.sys_supervisor.latest_version + ) await self._update_supervisor()
337-345: Architecture Review: Well-integrated immediate update feature.The implementation successfully achieves the PR's objective of immediate supervisor updates while maintaining system stability through:
- Thread-safe execution using JobExecutionLimit
- Integration with existing update check mechanisms
- Clear separation of concerns between update checking and execution
Consider documenting this behavior change in the supervisor's documentation to inform users about immediate updates.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- supervisor/misc/tasks.py (3 hunks)
- tests/fixtures/version_stable.json (1 hunks)
- tests/misc/test_tasks.py (2 hunks)
🔇 Additional comments (8)
tests/fixtures/version_stable.json (5)
1-3: LGTM! Basic configuration looks good.The channel and supervisor version format follow semantic versioning (MAJOR.MINOR.PATCH).
50-60: Consider extending the upgrade path coverage.The upgrade paths are defined from version 3 to 11, but there's no clear path for versions 12 and above. This might affect users trying to upgrade from these versions.
#!/bin/bash # Search for any references to HassOS versions 12 and above rg -g '!tests/' 'hassos.*1[2-9]'Consider:
- Adding upgrade paths for versions 12 and above if they exist
- Documenting the upgrade strategy for newer versions
61-66: Review the OTA URL pattern for security considerations.The OTA URL uses HTTPS which is good, but we should verify that the domain is properly secured.
#!/bin/bash # Search for any alternative OTA URL patterns or security configurations rg -g '!tests/' 'os-artifacts\.home-assistant\.io'
67-84: Verify image repository consistency.The image configuration correctly uses GHCR (GitHub Container Registry) for all components. The templating format using {arch} and {machine} is consistent across all entries.
#!/bin/bash # Search for any image references to ensure all repositories are properly configured rg -g '!tests/' 'ghcr\.io/home-assistant/.*-hassio-'
4-28: Verify platform coverage for Home Assistant versions.All platforms are consistently set to version "2024.10.4". However, let's verify that no supported platforms are missing from this list.
tests/misc/test_tasks.py (2)
3-5: LGTM! Well-structured async test setup.The changes to support async testing are well implemented:
- Appropriate imports for async testing capabilities
- Correct use of AsyncGenerator for proper resource management in the fixture
- Clean integration with existing test infrastructure
Also applies to: 17-17, 25-27
172-210: LGTM! Well-structured test implementation.The test thoroughly validates the supervisor update functionality with:
- Proper system state initialization
- Clean mocking of version checks
- Clear test scenarios for both update and no-update cases
supervisor/misc/tasks.py (1)
13-13: LGTM: Import addition is appropriate.The addition of
JobExecutionLimitis necessary for controlling the execution frequency of the supervisor update task.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- supervisor/misc/tasks.py (4 hunks)
🔇 Additional comments (3)
supervisor/misc/tasks.py (3)
13-13: LGTM: Import addition for job execution limit.
The addition of JobExecutionLimit import is appropriate for implementing the concurrent execution control in supervisor updates.
69-69: LGTM: Task registration for updater reload.
The _reload_updater task is properly registered with a 2-hour interval, which is a reasonable frequency for version checks.
139-141: LGTM: Race condition prevention implemented.
The addition of JobExecutionLimit.ONCE effectively prevents concurrent supervisor updates, addressing the previously identified race condition concerns.
Proposed change
Attempt to update supervisor immediately if a new version is detected to prevent users from running into the "supervisor out of date" error on addon updates and such.
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
Bug Fixes
_update_supervisormethod to limit execution to a single instance.Tests