Fix Linux4 Auto OS issue observed in Canary(/etc/dnf/automatic.conf absent by design in Linux4) - #368
Open
yashnap wants to merge 5 commits into
Open
Fix Linux4 Auto OS issue observed in Canary(/etc/dnf/automatic.conf absent by design in Linux4) #368yashnap wants to merge 5 commits into
yashnap wants to merge 5 commits into
Conversation
yashnap
changed the title
yashnap
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the DNF5 auto OS update handling to support Azure Linux 4’s behavior where /etc/dnf/automatic.conf may be absent by design, by separating “default” vs “override” configuration sources and making disable/revert flows resilient to a missing override file.
Changes:
- Treat DNF5 automatic configuration as two layers: default (
/usr/share/.../automatic.conf) + override (/etc/dnf/automatic.conf), and compute effective values accordingly. - Create an override config from the default config when disabling auto OS updates if the override file doesn’t exist; on revert, remove the override file when backup indicates it was absent.
- Update unit tests and legacy env mocks to reflect the new default/override split and dependency simulation changes.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/core/src/package_managers/Dnf5PackageManager.py | Implements default+override config handling, new backup keys, and override-file creation/removal during disable/revert. |
| src/core/tests/Test_Dnf5PackageManager.py | Updates tests for new backup schema and config paths; adds coverage for override removal failure. |
| src/core/tests/library/LegacyEnvLayerExtensions.py | Extends command-output mocking for dnf5 install --assumeno --skip-broken openssl used by tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # restore sys.stdout output | ||
| sys.stdout = original_stdout | ||
| self.__assert_std_io(captured_output=captured_output,expected_output=testcase["stdio"]["expected_output"]) | ||
| print("packagemanager" ,dir(package_manager)) |
Comment on lines
638
to
+642
| def is_backup_valid_for_dnf5_automatic(self, image_default_patch_configuration_backup): | ||
| if self.dnf5_auto_os_update_service in image_default_patch_configuration_backup \ | ||
| and self.dnf5_automatic_download_updates_identifier_text in image_default_patch_configuration_backup[self.dnf5_auto_os_update_service] \ | ||
| and self.dnf5_automatic_apply_updates_identifier_text in image_default_patch_configuration_backup[self.dnf5_auto_os_update_service] \ | ||
| and self.dnf5_automatic_enable_on_reboot_identifier_text in image_default_patch_configuration_backup[self.dnf5_auto_os_update_service] \ | ||
| and self.dnf5_automatic_installation_state_identifier_text in image_default_patch_configuration_backup[self.dnf5_auto_os_update_service]: | ||
| self.composite_logger.log_debug("[DNF5] Extension has a valid backup for default dnf5-automatic configuration settings") | ||
| return True | ||
| else: | ||
| self.composite_logger.log_debug("[DNF5] Extension does not have a valid backup for default dnf5-automatic configuration settings") | ||
| default_backup_valid = self.__is_backup_valid(image_default_patch_configuration_backup, self.os_patch_default_configuration_backup_key) | ||
| override_backup_valid = self.__is_backup_valid(image_default_patch_configuration_backup, self.os_patch_override_configuration_backup_key) | ||
|
|
||
| if default_backup_valid and override_backup_valid: |
Comment on lines
+734
to
+743
| self.composite_logger.log_debug("[DNF5] Removing override configuration file to restore machine default.[Path={0}]".format(self.os_patch_override_configuration_settings_file_path)) | ||
| code, out = self.env_layer.run_command_output(self.dnf5_automatic_remove_override_configuration_file_cmd, False, False) | ||
|
|
||
| if code != 0: | ||
| error_msg = "[DNF5] Error removing override configuration file. [Command={0}][Code={1}][Output={2}]".format(self.dnf5_automatic_remove_override_configuration_file_cmd, str(code), out) | ||
| self.composite_logger.log_error(error_msg) | ||
| self.status_handler.add_error_to_status(error_msg, Constants.PatchOperationErrorCodes.OPERATION_FAILED) | ||
| raise Exception(error_msg, "[{0}]".format(Constants.ERROR_ADDED_TO_STATUS)) | ||
|
|
||
| self.composite_logger.log_debug("[DNF5] Removed override configuration file. [Command={0}][Code={1}][Output={2}]".format(self.dnf5_automatic_remove_override_configuration_file_cmd, str(code), out)) |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #368 +/- ##
==========================================
+ Coverage 94.10% 94.17% +0.07%
==========================================
Files 109 109
Lines 20642 20770 +128
==========================================
+ Hits 19425 19560 +135
+ Misses 1217 1210 -7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (2)
src/core/src/package_managers/Dnf5PackageManager.py:642
- The body of is_backup_valid_for_dnf5_automatic is over-indented, which will raise an IndentationError (or change the block structure) at runtime and break the package manager module import.
def is_backup_valid_for_dnf5_automatic(self, image_default_patch_configuration_backup):
default_backup_valid = self.__is_backup_valid(image_default_patch_configuration_backup, self.os_patch_default_configuration_backup_key)
override_backup_valid = self.__is_backup_valid(image_default_patch_configuration_backup, self.os_patch_override_configuration_backup_key)
if default_backup_valid and override_backup_valid:
src/core/tests/Test_Dnf5PackageManager.py:487
- Leftover debug print in the test will add noise to test output and can cause brittle assertions when stdout is captured.
print("packagemanager" ,dir(package_manager))
|
|
||
| def __setup_current_auto_os_update_config(self, package_manager, config_value='', | ||
| config_file_name="automatic.conf"): | ||
| def __setup_current_auto_os_update_config(self, package_manager, config_value='', config_file_name=""): |
Comment on lines
+725
to
+727
| def __remove_override_configuration_if_exists(self): | ||
| """Removes dnf5-automatic override configuration file if it exists.Missing override file is valid by design, so this method must not throw | ||
| when the file is absent.""" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
REPRO STEPS:
Provision an Azure Linux 4.0 (AzL4) VM with DNF5
Install or verify DNF5 automatic plugin: rpm -qa | grep dnf5-plugin-automatic
Enable machine default auto updates using timer:
sudo systemctl enable --now dnf5-automatic.timer
Verify timer is enabled:
systemctl is-enabled dnf5-automatic.timer -> ENABLED
No configuration file exist yet :
/etc/dnf/automatic.conf
Run Assess/Install Patch from Azure Portal.
Expected: LPE should read current state which is enabled, disable it, perform the task and leave it in disabled state
Actual: LPE correctly Detects the current state of the system but when trying to disable, it runs into File not found error since /etc/dnf/automatic.conf is not automatically created when service is enabled in Azl4 (DNF5).
SOLUTION
According to their DNF5 doc (Automatic Command — dnf5 documentation), it uses 2 configuration files.
/usr/share/dnf5/dnf5-plugins/automatic.conf contains the default values and should be available once dnf5 is installed
/etc/dnf/automatic.conf contains Host-specific overrides and may not be available/created during dnf5 installation. Has to be explicitly created
They advise using /etc/dnf/automatic.conf to customize configurations for the automatic service.
Sequence of steps for disabling machine default OS updates should be:
3.Check if service is installed
If not, do nothing
If installed, pre-emptively disable it
For this, if the override file does not exist, create a copy from default file and set apply_updates and download_updates to false in the override. DO NOT modify the default file
Now to revert auto OS update to machine default OS update on when AutoPatching is disabled:
If installed, log current state
Read backup file and revert to the state it contains. i.e. if override file did not exist before, remove it. If values in default config were different to what we had logged in backup, modify default config to backup values.
Since override config does not always exist, code does not throw an exception for something that is by design
TESTING
ARM ID : /subscriptions/6acc8a91-e2b0-4041-a069-c2932ab42fd9/resourceGroups/azl4-rg-canary/providers/Microsoft.Compute/virtualMachines/linux4-auto-os
No service Installed
no_service_installed.log
Service instal
service_installed_no_timer.log
led but timer not enabled
Service inst
service_installed_yes_timer.log
alled and timer enabled
Idempotent call checking if backup
revert_idempotent.log
is not overriden/recreated
Revert to Default( Offboarding from GuestPatching) and service installed
revert_service_installed.log
Idempotent call after revert
revert_idempotent.log
Revert back ( Onboard again to Guest Patching)
revert_to_Azure.log