-
Couldn't load subscription status.
- Fork 5
engineering: Update Host Configuration with sysext/confext paths #276
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: main
Are you sure you want to change the base?
Conversation
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 updates the Host Configuration with the paths of sysext and confext extension images after they are staged during provisioning. The changes introduce an as_any() method to all subsystems to enable downcasting, add a method to update extension paths in the Host Configuration, and integrate this update into both clean install and AB update staging flows.
Key changes:
- Added
as_any()method to all subsystem implementations to support trait object downcasting - Created
update_host_configuration()method inExtensionsSubsystemto populate extension paths - Updated staging flows to call the new method and store the updated Host Configuration
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/trident/src/engine/mod.rs | Added as_any() to Subsystem trait, created helper function to retrieve ExtensionsSubsystem, and registered ExtensionsSubsystem in the lazy_static list |
| crates/trident/src/subsystems/extensions/mod.rs | Implemented as_any() method and added update_host_configuration() to update extension paths in Host Configuration |
| crates/trident/src/engine/update.rs | Integrated Host Configuration update with extension paths during AB update staging |
| crates/trident/src/engine/clean_install.rs | Integrated Host Configuration update with extension paths during clean install staging |
| crates/trident/src/subsystems/storage/mod.rs | Implemented as_any() method for StorageSubsystem |
| crates/trident/src/subsystems/selinux.rs | Implemented as_any() method for SelinuxSubsystem |
| crates/trident/src/subsystems/osconfig/mod.rs | Implemented as_any() method for OsConfigSubsystem and MosConfigSubsystem |
| crates/trident/src/subsystems/network.rs | Implemented as_any() method for NetworkSubsystem |
| crates/trident/src/subsystems/management.rs | Implemented as_any() method for ManagementSubsystem |
| crates/trident/src/subsystems/initrd.rs | Implemented as_any() method for InitrdSubsystem |
| crates/trident/src/subsystems/hooks.rs | Implemented as_any() method for HooksSubsystem |
| crates/trident/src/subsystems/esp.rs | Implemented as_any() method for EspSubsystem |
| crates/trident/src/engine/boot/mod.rs | Implemented as_any() method for BootSubsystem |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
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
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
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
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
/AzurePipelines run [GITHUB]-trident-pr-e2e |
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
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/AzurePipelines run [GITHUB]-trident-pr-e2e |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/AzurePipelines run [GITHUB]-trident-pr-e2e |
|
Azure Pipelines successfully started running 1 pipeline(s). |
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
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
/AzurePipelines run [GITHUB]-trident-pr-e2e |
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
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/AzurePipelines run [GITHUB]-trident-pr-e2e |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| Ok(()) | ||
| } | ||
|
|
||
| fn update_host_configuration(&self, ctx: &mut EngineContext) -> Result<(), TridentError> { |
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.
does this not need to happen for install?
this function only has sysext-related code, perhaps it should be named to reflect that? also add doc 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.
What do you mean? This function is called in clean_install.rs so it should be called for clean install as well.
Since this is a Subsystem method, the doc comments for it are inside crates/trident/src/engine/mod.rs
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
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
🔍 Description
Update the Host Configuration with the paths of sysexts and confexts at the end of Staging. Also add a couple SELinux permissions so Trident can mount sysexts and confexts.
🤔 Rationale
We need to update the Host Configuration with the paths of the sysexts and confexts in the target OS. To do so, we need the "name" of the extension image (information stored in the subsystem) and need to update the Engine Context.
📝 Checks
📌 Follow-ups
TODO:
🗒️ Notes