feat: check computed schematic ID to avoid unnecessary machine upgrades #1897
Conversation
If we didn't change anything in a schematic, i.e., picked everything from the `MachineStatus`, but ended up calculating a different schematic ID than what the machine already has, just log it as a warning, but preserve the existing schematic ID. Signed-off-by: Utku Ozdemir <[email protected]>
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a safety mechanism to prevent unnecessary machine upgrades by detecting when a computed schematic ID differs from the existing one despite no actual configuration changes being made. When this mismatch occurs, the system preserves the existing schematic ID and logs a warning instead of triggering a potentially disruptive machine reboot.
Key Changes:
- Added safety check logic to detect and handle schematic ID mismatches when configuration is unchanged
- Modified
determineKernelArgsto return anunchangedboolean indicating whether kernel args were modified - Refactored
computeMachineExtensionsStatusto accept explicit parameters instead of a pointer tomachineCustomization
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| internal/backend/runtime/omni/controllers/omni/schematic_configuration.go | Implements the core safety check for schematic ID mismatches and tracks whether kernel args and extensions have changed |
| hack/test/integration.sh | Adds a TODO comment about expanding test coverage for different schematic options |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| return args, nil | ||
| return args, false, nil |
There was a problem hiding this comment.
The unchanged return value should be true when the calculated kernel args match the existing kernel args in the machine status. Currently, it always returns false for the normal calculation path, which could prevent the safety check at line 230 from working correctly when kernel args haven't actually changed.
Consider comparing the calculated args with existing args:
unchanged := slices.Equal(args, machineStatus.TypedSpec().Value.Schematic.KernelArgs)
return args, unchanged, nil| return args, false, nil | |
| unchanged := slices.Equal(args, machineStatus.TypedSpec().Value.Schematic.KernelArgs) | |
| return args, unchanged, nil |
1 participant
If we didn't change anything in a schematic, i.e., picked everything from the
MachineStatus, but ended up calculating a different schematic ID than what the machine already has, just log it as a warning, but preserve the existing schematic ID.