Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds a --runtime flag to the trident update command as a safety mechanism to prevent unexpected system reboots. When the flag is set, Trident will error out if it determines that an A/B update (which requires a reboot) is necessary, allowing users to ensure they're performing only runtime updates.
Key Changes
- Added
--runtimeboolean CLI flag to theupdatecommand - Introduced
AbUpdateRuntimeFlagMismatcherror type for when A/B update is determined with runtime flag set - Threaded the flag through the call chain from CLI → main.rs → lib.rs → engine/update.rs
- Added documentation for the new flag in the CLI reference
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
docs/Reference/Trident-CLI.md |
Added documentation for the --runtime flag in the update command reference |
crates/trident_api/src/error.rs |
Added AbUpdateRuntimeFlagMismatch error variant with descriptive error message |
crates/trident/src/cli.rs |
Added runtime: bool field to the Update command struct |
crates/trident/src/main.rs |
Extracted and passed the runtime flag to the update function |
crates/trident/src/lib.rs |
Updated update function signature and hardcoded false for gRPC call |
crates/trident/src/engine/update.rs |
Implemented logic to check runtime flag and return error when A/B update is determined |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| host_config: &HostConfiguration, | ||
| state: &mut DataStore, | ||
| allowed_operations: &Operations, | ||
| expect_runtime: bool, |
There was a problem hiding this comment.
The parameter name expect_runtime is inconsistent with the CLI flag name runtime used elsewhere in the codebase. For clarity and consistency, consider renaming this parameter to runtime to match the CLI flag name used in cli.rs, main.rs, and lib.rs.
| expect_runtime: bool, | |
| runtime: bool, |
| if expect_runtime { | ||
| return Err(TridentError::new( | ||
| InvalidInputError::AbUpdateRuntimeFlagMismatch, | ||
| )); | ||
| } |
There was a problem hiding this comment.
The new AbUpdateRuntimeFlagMismatch error behavior lacks test coverage. Given that this repository has comprehensive test coverage for other error cases and the update function handles critical update logic, this new error path should be tested to ensure it correctly prevents A/B updates when the --runtime flag is set.
| Default: `stage,finalize` | ||
|
|
||
|
|
||
| #### <span>--runtime <RUNTIME></span> |
There was a problem hiding this comment.
The documentation header incorrectly shows --runtime <RUNTIME> for a boolean flag. Since this is a boolean flag (defined with #[clap(long)] without a value parser), it should not show a parameter placeholder in the header. It should be --runtime instead of --runtime <RUNTIME>.
| #### <span>--runtime <RUNTIME></span> | |
| #### <span>--runtime</span> |
ayaegashi
changed the title
--runtime flag--runtime flag
🔍 Description
--runtimeflag ontrident updatecommand indicates that a runtime update is expected. If Trident determines than an A/B update is necessary, Trident will error out.🤔 Rationale
Serves as a safety check so that users can prevent unexpected reboots.