Code Review comments

Author: yashnap

src/core/src/bootstrap/EnvLayer.py

@@ -71,11 +71,6 @@ def is_distro_azure_linux_3(self, distro_name):
         """ Checks if the current distro is Azure Linux 3 """
         return self.__is_matching_distro_and_version(distro_name, Constants.AZURE_LINUX, version_to_match=3)
 
-    def is_distro_azure_linux_4(self, distro_name):
-        # type: (str) -> bool
-        """ Checks if the current distro is Azure Linux 4 """
-        return self.__is_matching_distro_and_version(distro_name, Constants.AZURE_LINUX, version_to_match=4)
-
     def is_distro_rhel_10(self, distro_name):
         # type: (str) -> bool
         """ Checks if the current distro is RHEL 10 """
@@ -92,7 +87,7 @@ def get_package_manager(self):
         os_name, os_version, os_code = self.platform.linux_distribution()
 
         # Check for unsupported distros
-        if self.is_distro_azure_linux_4(os_name) or self.is_distro_rhel_10(os_name):
+        if self.is_distro_rhel_10(os_name):
             error_msg = "This distro is not yet supported in your region. Please review https://aka.ms/VMGuestPatchingCompatibility for more information. [Distro={0}][Version={1}][Code={2}]".format(str(os_name), os_version, os_code)
             print("Error: {0}".format(error_msg))
             return str()

src/core/src/package_managers/Dnf5PackageManager.py

@@ -269,7 +269,9 @@ def get_dependent_list(self, packages):
         self.composite_logger.log_verbose("[DNF5] Dependency simulation. [Command={0}][Code={1}]".format(cmd, str(code)))
         if code not in self.dnf5_simulation_valid_exit_codes:
             self.composite_logger.log_error("[DNF5] Unexpected failure. [Command={0}][Code={1}][Output={2}]".format(cmd, str(code), output))
-            raise Exception("DNF5 dependency simulation failed")
+            error_msg = "DNF5 dependency simulation failed. Investigate and resolve unexpected return code({0}) from package manager on command: {1} ".format(str(code), cmd)
+            self.status_handler.add_error_to_status(error_msg, Constants.PatchOperationErrorCodes.DEFAULT_ERROR)
+            raise Exception(error_msg, "[{0}]".format(Constants.ERROR_ADDED_TO_STATUS))
 
         dependencies = self.extract_dependencies(output, packages)
         self.composite_logger.log_verbose("[DNF5] Resolved dependencies. [Command={0}][Packages={1}][DependencyCount={2}]".format(str(cmd), str(packages), len(dependencies)))
@@ -311,8 +313,6 @@ def extract_dependencies(self, output, packages):
                 continue
 
             #  Remove input packages (support both pkg and pkg.arch)
-            base_pkg = dependent_package_name.rsplit('.', 1)[0] if '.' in dependent_package_name else dependent_package_name
-
             if len(dependent_package_name) != 0 and dependent_package_name not in packages and dependent_package_name not in dependencies:
                 self.composite_logger.log_verbose("[DNF5] > Dependency detected: " + dependent_package_name)
                 dependencies.append(dependent_package_name)
@@ -479,10 +479,8 @@ def is_service_set_to_enable_on_reboot(self, command):
         """ Checking if auto update is set to enable on reboot on the machine. An enable_on_reboot service will be activated (if currently inactive) on machine reboot """
         self.composite_logger.log_verbose("[DNF5] Checking if auto update service is set to enable on reboot. [Command={0}]".format(command))
         code, out = self.env_layer.run_command_output(command, False, False)
-        #[inGuestLinux@vm-yashna-linux4 ~]$ systemctl is-enabled dnf5-automatic.timer
-        # enabled
-        is_enable_on_reboot = len(out.strip()) > 0 and code == 0 and out.strip() == "enabled"
-        self.composite_logger.log_debug("[DNF5] Auto update service enable on reboot check completed. [Command={0}][Code={1}][EnabledOnReboot={2}]".format(command,str(code),str(is_enable_on_reboot)))
+        is_enable_on_reboot = len(out.strip()) > 0 and code == 0 and out.strip().lower() == "enabled"
+        self.composite_logger.log_debug("[DNF5] Auto update service enable on reboot check completed. [Command={0}][Code={1}][IsServiceSetToEnableOnReboot={2}]".format(command, str(code), str(is_enable_on_reboot)))
         return is_enable_on_reboot
 
     def __get_extension_standard_value_for_apply_updates(self, apply_updates_value):
@@ -575,8 +573,8 @@ def backup_image_default_patch_configuration_if_not_exists(self):
                                                 .format(str(image_default_patch_configuration_backup),self.image_default_patch_configuration_backup_path))
                 self.env_layer.file_system.write_with_retry(self.image_default_patch_configuration_backup_path,'{0}'.format(json.dumps(image_default_patch_configuration_backup)),mode='w+')
         except Exception as error:
-            self.composite_logger.log_error( "[DNF5] Exception during fetching and logging default auto update settings on the machine. [Exception={0}]".format(repr(error)))
-            self.status_handler.add_error_to_status( "[DNF5] Exception during fetching and logging default auto update settings on the machine. [Exception={0}]".format(repr(error)), Constants.PatchOperationErrorCodes.DEFAULT_ERROR)
+            self.composite_logger.log_error("[DNF5] Exception during fetching and logging default auto update settings on the machine. [Exception={0}]".format(repr(error)))
+            self.status_handler.add_error_to_status("[DNF5] Exception during fetching and logging default auto update settings on the machine. [Exception={0}]".format(repr(error)), Constants.PatchOperationErrorCodes.DEFAULT_ERROR)
             raise
 
     def is_image_default_patch_configuration_backup_valid(self, image_default_patch_configuration_backup):

src/core/tests/Test_Dnf5PackageManager.py

@@ -187,6 +187,8 @@ def test_get_current_auto_os_patch_state_with_installed_services_and_state_enabl
         dnf5_automatic_os_patch_configuration_settings = 'apply_updates = yes\ndownload_updates = yes\n'
         self.runtime.write_to_file(package_manager.dnf5_automatic_configuration_file_path, dnf5_automatic_os_patch_configuration_settings)
 
+        is_enabled = package_manager.is_service_set_to_enable_on_reboot(package_manager.enable_on_reboot_check_cmd)
+        self.assertFalse(is_enabled)
         current_auto_os_patch_state = package_manager.get_current_auto_os_patch_state()
 
         self.assertFalse(package_manager.image_default_patch_configuration_backup_exists())
@@ -631,7 +633,6 @@ def test_disable_auto_os_update_on_reboot(self):
         command = "systemctl disable --now dnf5-automatic.timer"
 
         package_manager.disable_auto_update_on_reboot(command)
-        self.assertTrue(True)  # method should NOT throw
 
         self.runtime.set_legacy_test_type('AnotherSadPath')
         package_manager = self.container.get('package_manager')
@@ -722,8 +723,6 @@ def test_no_op_methods(self):
         package_manager.set_security_esm_package_status("op", [])
         package_manager.separate_out_esm_packages([], [])
 
-        self.assertTrue(True)
-
 if __name__ == '__main__':
     unittest.main()
 

src/core/tests/Test_EnvLayer.py

@@ -80,12 +80,6 @@ def mock_distro_os_release_attr_return_azure_linux_2(self, attribute):
     def mock_distro_os_release_attr_return_none(self, attribute):
         return None
 
-    def mock_linux_distribution_to_return_azure_linux_4(self):
-        return ['Microsoft Azure Linux', '4.0', '']
-
-    def mock_distro_os_release_attr_return_azure_linux_4(self, attribute):
-        return '4.0.2'
-
     def mock_linux_distribution_to_return_rhel_10(self):
         return ['Red Hat', '10.0', 'abc']
 
@@ -159,17 +153,15 @@ def test_platform(self):
         self.envlayer.platform.vm_name()
 
     def test_get_package_manager_azure_linux_4_and_rhel10_not_supported(self):
-        """Test that Azure Linux 4 and RHEL 10 log unsupported message"""
+        """Test for RHEL 10 log unsupported message"""
         self.backup_platform_system = platform.system
         self.backup_linux_distribution = self.envlayer.platform.linux_distribution
         self.backup_distro_os_release_attr = distro.os_release_attr
 
         platform.system = self.mock_platform_system
         test_input_output_table = [
-            [self.mock_linux_distribution_to_return_azure_linux_4, self.mock_distro_os_release_attr_return_azure_linux_4, "Error: This distro is not yet supported in your region. Please review https://aka.ms/VMGuestPatchingCompatibility for more information. [Distro=Microsoft Azure Linux][Version=4.0][Code=]\n"],
             [self.mock_linux_distribution_to_return_rhel_10, self.mock_distro_os_release_attr_return_rhel_10, "Error: This distro is not yet supported in your region. Please review https://aka.ms/VMGuestPatchingCompatibility for more information. [Distro=Red Hat][Version=10.0][Code=abc]\n"],
         ]
-
         for row in test_input_output_table:
             captured_output = StringIO()
             original_output = sys.stdout