engineering: Implement select_servicing_type()

[ayaegashi]

🔍 Description

Implement select_servicing_type() for extensions, osconfig and networks subsystems, enabling runtime updates. Moves the check of whether the OS image has been updated out of storage subsystem so that it is performed before any subsystems' select_servicing_type() functions are called, since if the OS image has been updated no further checks are needed.

Testing can be seen inside Runtime Update and Rollback Testing block.

The base branch was changed.

[ayaegashi]

/AzurePipelines run [GITHUB]-trident-pr-e2e

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[ayaegashi]

This check is performed already inside validate_host_config() for the storage subsystem

[ayaegashi]

/AzurePipelines run [GITHUB]-trident-pr-e2e

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[ayaegashi]

/AzurePipelines run [GITHUB]-trident-pr-e2e

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[ayaegashi]

/AzurePipelines run [GITHUB]-trident-pr-e2e

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

This PR implements select_servicing_type() for multiple subsystems (extensions, osconfig, network, and hooks) to enable runtime updates, and centralizes the servicing type selection logic by moving the OS image check out of the storage subsystem.

• Centralizes servicing type selection in engine/mod.rs, checking for clean install and AB update requirements before delegating to subsystems
• Moves ab_update_required() from subsystems/storage/image.rs to engine/context/image.rs as a method on EngineContext
• Implements subsystem-specific select_servicing_type() methods that compare old vs new configurations to determine if runtime updates or AB updates are needed

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
crates/trident/src/subsystems/storage/mod.rs	Removes select_servicing_type() implementation and image module import from storage subsystem
crates/trident/src/subsystems/osconfig/mod.rs	Adds select_servicing_type() to determine if OS config changes can use runtime update or require AB update
crates/trident/src/subsystems/network.rs	Adds select_servicing_type() to enable runtime updates for netplan configuration changes
crates/trident/src/subsystems/hooks.rs	Adds select_servicing_type() for scripts changes requiring AB updates
crates/trident/src/subsystems/extensions/mod.rs	Adds select_servicing_type() to enable runtime updates for sysext/confext changes
crates/trident/src/engine/update.rs	Refactors to call centralized select_servicing_type() function
crates/trident/src/engine/runtime_update.rs	Removes redundant comment and hardcodes servicing type in tracing
crates/trident/src/engine/mod.rs	Adds centralized select_servicing_type() function, changes default trait implementation, and updates log levels from debug to trace for skip messages
crates/trident/src/engine/context/mod.rs	Makes image submodule public within crate
crates/trident/src/engine/context/image.rs	Refactors ab_update_required() as a method on EngineContext and updates test calls
.pipelines/templates/stages/testing_rollback/vm-testing.yml	Enables runtime update testing for all image flavors
.pipelines/templates/stages/testing_rollback/testing-template.yml	Changes default for skipRuntimeUpdateTesting from true to false and fixes whitespace

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[ndubchak]

nit: Could this be a method of EngineContext like ab_update_required()? They seem somewhat equivalent

[ayaegashi]

I have a preference to keep the logic here so that subsystems that own parts of the HC can individually determine which servicing type is required

[ndubchak]

I didn't mean moving this func; I suggested defining it as a method of EngineContext by using the impl EngineContext key words, just like we're doing with ab_update_required(). I think that is idiomatic Rust

[ayaegashi]

Got it, sorry, based on the GitHub formatting I thought you were suggesting making select_servicing_type() a method of EngineContext, not runtime_update_sufficient.
I think in this case runtime_update_sufficient and the other functions defined lines 34-75 are better as free functions instead of methods since their scope is limited to just this subsystem and file.