[PR Comments] Feature: Set Installation Status to Warning when all expected packages are installed

src/core/src/CoreMain.py

@@ -93,14 +93,17 @@ def __init__(self, argv):
                 # setting current operation here, to include patch_installer init within installation actions, ensuring any exceptions during patch_installer init are added in installation summary errors object
                 status_handler.set_current_operation(Constants.INSTALLATION)
                 patch_installer = container.get('patch_installer')
-                patch_installation_successful = patch_installer.start_installation()
+                patch_installation_successful, maintenance_window_exceeded = patch_installer.start_installation()
                 patch_assessment_successful = False
                 patch_assessment_successful = patch_assessor.start_assessment()
 
                 # PatchInstallationSummary to be marked as completed successfully only after the implicit (i.e. 2nd) assessment is completed, as per CRP's restrictions
                 if patch_assessment_successful and patch_installation_successful:
                     patch_installer.mark_installation_completed()
                     overall_patch_installation_operation_successful = True
+                elif patch_installer.should_patch_installation_status_be_set_to_warning(patch_installation_successful, maintenance_window_exceeded):
+                    patch_installer.mark_installation_completed_with_warning()
+                    overall_patch_installation_operation_successful = True
                 self.update_patch_substatus_if_pending(patch_operation_requested, overall_patch_installation_operation_successful, patch_assessment_successful, configure_patching_successful, status_handler, composite_logger)
         except Exception as error:
             # Privileged operation handling for non-production use

src/core/src/bootstrap/Constants.py

@@ -308,6 +308,7 @@ class PatchOperationErrorCodes(EnumBackport):
         NEWER_OPERATION_SUPERSEDED = "NEWER_OPERATION_SUPERSEDED"
         UA_ESM_REQUIRED = "UA_ESM_REQUIRED"
         TRUNCATION = "PACKAGE_LIST_TRUNCATED"
+        PACKAGES_RETRY_SUCCEEDED = "PACKAGES_RETRY_SUCCEEDED"
 
     ERROR_ADDED_TO_STATUS = "Error_added_to_status"
 

src/core/src/core_logic/PatchInstaller.py

@@ -22,6 +22,7 @@
 from core.src.bootstrap.Constants import Constants
 from core.src.core_logic.Stopwatch import Stopwatch
 
+
 class PatchInstaller(object):
     """" Wrapper class for a single patch installation operation """
     def __init__(self, env_layer, execution_config, composite_logger, telemetry_writer, status_handler, lifecycle_manager, package_manager, package_filter, maintenance_window, reboot_manager):
@@ -123,7 +124,7 @@
         overall_patch_installation_successful = bool(update_run_successful and not maintenance_window_exceeded)
         # NOTE: Not updating installation substatus at this point because we need to wait for the implicit/second assessment to complete first, as per CRP's instructions
 
-        return overall_patch_installation_successful
+        return overall_patch_installation_successful, maintenance_window_exceeded
 
     def write_installer_perf_logs(self, patch_operation_successful, installed_patch_count, retry_count, maintenance_window, maintenance_window_exceeded, task_status, error_msg):
         perc_maintenance_window_used = -1
@@ -324,7 +325,7 @@
             # package_and_dependencies initially contains only one package. The dependencies are added in the list by method include_dependencies
             package_and_dependencies = [package]
             package_and_dependency_versions = [version]
-            
+
             self.include_dependencies(package_manager, [package], [version], all_packages, all_package_versions, packages, package_versions, package_and_dependencies, package_and_dependency_versions)
 
             # parent package install (+ dependencies) and parent package result management
@@ -379,6 +380,7 @@
 
         self.composite_logger.log_debug("\nPerforming final system state reconciliation...")
         installed_update_count += self.perform_status_reconciliation_conditionally(package_manager, True)
+
         self.log_final_metrics(maintenance_window, patch_installation_successful, maintenance_window_exceeded, installed_update_count)
 
         install_update_count_in_sequential_patching = installed_update_count - install_update_count_in_batch_patching
@@ -398,16 +400,14 @@
     def log_final_metrics(self, maintenance_window, patch_installation_successful, maintenance_window_exceeded, installed_update_count):
         """
         logs the final metrics.
-
         Parameters:
         maintenance_window (MaintenanceWindow): Maintenance window for the job.
         patch_installation_successful (bool): Whether patch installation succeeded.
         maintenance_window_exceeded (bool): Whether maintenance window exceeded.
         installed_update_count (int): Number of updates installed.
         """
-        progress_status = self.progress_template.format(str(datetime.timedelta(minutes=maintenance_window.get_remaining_time_in_minutes())), str(self.attempted_parent_package_install_count), str(self.successful_parent_package_install_count), str(self.failed_parent_package_install_count), str(installed_update_count - self.successful_parent_package_install_count),
-                                                        "Completed processing packages!")
-        self.composite_logger.log(progress_status)
+
+        self.__log_progress_status(maintenance_window, installed_update_count)
 
         if not patch_installation_successful or maintenance_window_exceeded:
             message = "\n\nOperation status was marked as failed because: "
@@ -416,10 +416,16 @@
             self.status_handler.add_error_to_status(message, Constants.PatchOperationErrorCodes.OPERATION_FAILED)
             self.composite_logger.log_error(message)
 
+    def log_final_warning_metric(self, maintenance_window, installed_update_count):
+        """ Log the final metrics for warning installation status. """
+        self.__log_progress_status(maintenance_window, installed_update_count)
+        message = "All requested package(s) are installed. Any patch errors marked are from previous attempts."
+        self.status_handler.add_error_to_status(message, Constants.PatchOperationErrorCodes.PACKAGES_RETRY_SUCCEEDED)
+        self.composite_logger.log_error(message)
+
     def include_dependencies(self, package_manager, packages_in_batch, package_versions_in_batch, all_packages, all_package_versions, packages, package_versions, package_and_dependencies, package_and_dependency_versions):
         """
         Add dependent packages in the list of packages to install i.e. package_and_dependencies.
-
         Parameters:
         package_manager (PackageManager): Package manager used.
         packages_in_batch (List of strings): List of packages to be installed in the current batch.
@@ -509,9 +515,7 @@
     def install_packages_in_batches(self, all_packages, all_package_versions, packages, package_versions, maintenance_window, package_manager, max_batch_size_for_packages, simulate=False):
         """
         Install packages in batches.
-
         Parameters:
-
         all_packages (List of strings): List of all available packages to install.
         all_package_versions (List of strings): Versions of the packages in the list all_packages.
         packages (List of strings): List of all packages selected by user to install.
@@ -520,7 +524,6 @@
         package_manager (PackageManager): Package manager used.
         max_batch_size_for_packages (Integer): Maximum batch size.
         simulate (bool): Whether this function is called from a test run.
-
         Returns:
         installed_update_count (int): Number of packages installed through installing packages in batches.
         patch_installation_successful (bool): Whether package installation succeeded for all attempted packages.
@@ -529,7 +532,6 @@
         not_attempted_and_failed_packages (List of strings): List of packages which are (a) Not attempted due to not enough time in maintenance window to install in batch. 
                                                              (b) Failed to install in batch patching.
         not_attempted_and_failed_package_versions (List of strings): Versions of packages in the list not_attempted_and_failed_packages.
-
         """
         number_of_batches = int(math.ceil(len(packages) / float(max_batch_size_for_packages)))
         self.composite_logger.log("\nDividing package install in batches. \nNumber of packages to be installed: " + str(len(packages)) + "\nBatch Size: " + str(max_batch_size_for_packages) + "\nNumber of batches: " + str(number_of_batches))
@@ -683,12 +685,20 @@
             self.status_handler.set_installation_substatus_json(status=Constants.STATUS_WARNING)
 
         # Update patch metadata in status for auto patching request, to be reported to healthStore
-        self.composite_logger.log_debug("[PI] Reviewing final healthstore record write. [HealthStoreId={0}][MaintenanceRunId={1}]".format(str(self.execution_config.health_store_id), str(self.execution_config.maintenance_run_id)))
-        if self.execution_config.health_store_id is not None:
-            self.status_handler.set_patch_metadata_for_healthstore_substatus_json(
-                patch_version=self.execution_config.health_store_id,
-                report_to_healthstore=True,
-                wait_after_update=False)
+        self.__send_metadata_to_health_store()
+
+    def mark_installation_completed_with_warning(self):
+        """ Marks Installation operation as warning by updating the status of PatchInstallationSummary as warning and patch metadata to be sent to healthstore.
+        This is set outside of start_installation function due to a restriction in CRP, where installation substatus should be marked as warning only after the implicit (2nd) assessment operation
+        and all customer requested packages are installed. """
+
+        message = "All requested package(s) are installed. Any patch errors marked are from previous attempts."
+        self.composite_logger.log_error(message)
+        self.status_handler.add_error_to_status(message, Constants.PatchOperationErrorCodes.PACKAGES_RETRY_SUCCEEDED)
+        self.status_handler.set_installation_substatus_json(status=Constants.STATUS_WARNING)
+
+        # Update patch metadata in status for auto patching request, to be reported to healthStore
+        self.__send_metadata_to_health_store()
 
     # region Installation Progress support
     def perform_status_reconciliation_conditionally(self, package_manager, condition=True):
@@ -801,3 +811,22 @@
 
         return max_batch_size_for_packages
 
+    def __log_progress_status(self, maintenance_window, installed_update_count):
+        progress_status = self.progress_template.format(str(datetime.timedelta(minutes=maintenance_window.get_remaining_time_in_minutes())),
+                                                        str(self.attempted_parent_package_install_count),
+                                                        str(self.successful_parent_package_install_count),
+                                                        str(self.failed_parent_package_install_count),
+                                                        str(installed_update_count - self.successful_parent_package_install_count),
+                                                        "Completed processing packages!")
+        self.composite_logger.log(progress_status)
+
+    def __send_metadata_to_health_store(self):
+        self.composite_logger.log_debug("[PI] Reviewing final healthstore record write. [HealthStoreId={0}][MaintenanceRunId={1}]".format(str(self.execution_config.health_store_id), str(self.execution_config.maintenance_run_id)))
+        if self.execution_config.health_store_id is not None:
+            self.status_handler.set_patch_metadata_for_healthstore_substatus_json(
+                patch_version=self.execution_config.health_store_id,
+                report_to_healthstore=True,
+                wait_after_update=False)
+
+    def should_patch_installation_status_be_set_to_warning(self, patch_installation_successful, maintenance_window_exceeded):
+        return not patch_installation_successful and not maintenance_window_exceeded and self.status_handler.are_all_requested_packgaes_installed()

src/core/src/service_interfaces/StatusHandler.py

@@ -262,6 +262,17 @@ def get_os_name_and_version(self):
         except Exception as error:
             self.composite_logger.log_error("Unable to determine platform information: {0}".format(repr(error)))
             return "unknownDist_unknownVer"
+
+    def are_all_requested_packgaes_installed(self):
+        # type (none) -> bool
+        """ Check if all requested package(s) are installed. """
+
+        for package in self.__installation_packages:
+            if package['patchInstallationState'] != Constants.INSTALLED:
+                return False
+
+        # All requested package(s) are installed
+        return True
     # endregion
 
     # region - Installation Reboot Status

src/core/tests/Test_CoreMain.py

@@ -56,6 +56,17 @@
     def mock_os_path_exists(self, patch_to_validate):
         return False
 
+    def mock_batch_patching_with_packages(self, all_packages, all_package_versions, packages, package_versions, maintenance_window, package_manager):
+        """Mock batch_patching to simulate package installation failure, return some packages """
+        return packages, package_versions, 0, False
+
+    def mock_batch_patching_with_no_packages(self, all_packages, all_package_versions, packages, package_versions, maintenance_window, package_manager):
+        """Mock batch_patching to simulate package installation failure, return no packages"""
+        return [], [], 0, False
+
+    def mock_check_all_requested_packages_install_state(self):
+        return True
+
     def test_operation_fail_for_non_autopatching_request(self):
         # Test for non auto patching request
         argument_composer = ArgumentComposer()
@@ -1275,6 +1286,69 @@
         os.remove = self.backup_os_remove
         runtime.stop()
 
+    def test_warning_status_when_packages_initially_fail_but_succeed_on_retry(self):
+        """
+        Tests installation status set warning when:
+        1. Packages initially fail installation
+        2. Package manager indicates retry is needed
+        3. On retry, all supposed packages are installed successfully
+        4. Batch_patching returns some packages
+        """
+        argument_composer = ArgumentComposer()
+        argument_composer.operation = Constants.INSTALLATION
+        runtime = RuntimeCompositor(argument_composer.get_composed_arguments(), True, Constants.ZYPPER)
+
+        runtime.set_legacy_test_type('PackageRetrySuccessPath')
+
+        # Store original methods
+        original_batch_patching = runtime.patch_installer.batch_patching
+
+        # Mock batch_patching with packages to return false
+        runtime.patch_installer.batch_patching = self.mock_batch_patching_with_packages
+
+        # Run CoreMain to execute the installation
+        try:
+            CoreMain(argument_composer.get_composed_arguments())
+            self.__assertion_pkg_succeed_on_retry(runtime)
+
+        finally:
+            # reset mock
+            runtime.patch_installer.batch_patching = original_batch_patching
+            runtime.stop()
+
+    def test_warning_status_when_packages_initially_fail_but_succeed_on_retry_no_batch_packages(self):
+        """
+        Tests installation status set warning when:
+        1. Packages initially fail installation
+        2. Package manager indicates retry is needed
+        3. On retry, all supposed packages are installed successfully
+        4. Batch_patching returns no packages
+        """
+        argument_composer = ArgumentComposer()
+        argument_composer.operation = Constants.INSTALLATION
+        runtime = RuntimeCompositor(argument_composer.get_composed_arguments(), True, Constants.ZYPPER)
+
+        runtime.set_legacy_test_type('PackageRetrySuccessPath')
+
+        # Store original methods
+        original_batch_patching = runtime.patch_installer.batch_patching
+        original_check_all_requested_packages_install_state= runtime.status_handler.check_all_requested_packages_install_state
+
+        # Mock batch_patching with packages to return [], [], false
+        runtime.patch_installer.batch_patching = self.mock_batch_patching_with_no_packages
+        runtime.status_handler.check_all_requested_packages_install_state = self.mock_check_all_requested_packages_install_state
+
+        # Run CoreMain to execute the installation
+        try:
+            CoreMain(argument_composer.get_composed_arguments())
+            self.__assertion_pkg_succeed_on_retry(runtime)
+
+        finally:
+            # reset mock
+            runtime.patch_installer.batch_patching = original_batch_patching
+            runtime.status_handler.get_installation_packages_list = original_check_all_requested_packages_install_state
+            runtime.stop()
+
     def __check_telemetry_events(self, runtime):
         all_events = os.listdir(runtime.telemetry_writer.events_folder_path)
         self.assertTrue(len(all_events) > 0)
@@ -1285,6 +1359,29 @@
             self.assertTrue('Core' in events[0]['TaskName'])
             f.close()
 
+    def __assertion_pkg_succeed_on_retry(self, runtime):
+        # Check telemetry events
+        self.__check_telemetry_events(runtime)
+
+        # If true, set installation status to warning
+        self.assertTrue(runtime.patch_installer.set_patch_installation_status_to_warning_from_failed())
+
+        # Check status file
+        with runtime.env_layer.file_system.open(runtime.execution_config.status_file_path, 'r') as file_handle:
+            substatus_file_data = json.load(file_handle)[0]["status"]["substatus"]
+
+        self.assertEqual(len(substatus_file_data), 3)
+        self.assertTrue(substatus_file_data[0]["name"] == Constants.PATCH_ASSESSMENT_SUMMARY)
+        self.assertTrue(substatus_file_data[0]["status"].lower() == Constants.STATUS_SUCCESS.lower())
+
+        # Check installation status is WARNING
+        self.assertTrue(substatus_file_data[1]["name"] == Constants.PATCH_INSTALLATION_SUMMARY)
+        self.assertTrue(substatus_file_data[1]["status"].lower() == Constants.STATUS_WARNING.lower())
+
+        # Verify at least one error detail about package retry
+        error_details = json.loads(substatus_file_data[1]["formattedMessage"]["message"])["errors"]["details"]
+        self.assertTrue(any("package(s) are installed" in detail["message"] for detail in error_details))
+
 
 if __name__ == '__main__':
     unittest.main()