From 9a9b48a6c408be255eea02d31bb48f71ad48d704 Mon Sep 17 00:00:00 2001 From: john feng Date: Sun, 23 Feb 2025 23:36:06 -0800 Subject: [PATCH 01/38] add logic to set installation warning when all pkgs are installed --- src/core/src/CoreMain.py | 3 + src/core/src/bootstrap/Constants.py | 1 + src/core/src/core_logic/PatchInstaller.py | 123 ++++++++++++++---- .../src/service_interfaces/StatusHandler.py | 4 + 4 files changed, 106 insertions(+), 25 deletions(-) diff --git a/src/core/src/CoreMain.py b/src/core/src/CoreMain.py index a98160c79..400efad03 100644 --- a/src/core/src/CoreMain.py +++ b/src/core/src/CoreMain.py @@ -101,6 +101,9 @@ def __init__(self, argv): if patch_assessment_successful and patch_installation_successful: patch_installer.mark_installation_completed() overall_patch_installation_operation_successful = True + elif patch_installer.get_enabled_installation_warning_status(): + patch_installer.mark_installation_warning_completed() + 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 diff --git a/src/core/src/bootstrap/Constants.py b/src/core/src/bootstrap/Constants.py index c742495f5..7a341be84 100644 --- a/src/core/src/bootstrap/Constants.py +++ b/src/core/src/bootstrap/Constants.py @@ -305,6 +305,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" diff --git a/src/core/src/core_logic/PatchInstaller.py b/src/core/src/core_logic/PatchInstaller.py index 63f8771c2..93f52e59d 100644 --- a/src/core/src/core_logic/PatchInstaller.py +++ b/src/core/src/core_logic/PatchInstaller.py @@ -19,6 +19,7 @@ import math import sys import time + from core.src.bootstrap.Constants import Constants from core.src.core_logic.Stopwatch import Stopwatch @@ -52,6 +53,10 @@ def __init__(self, env_layer, execution_config, composite_logger, telemetry_writ self.stopwatch = Stopwatch(self.env_layer, self.telemetry_writer, self.composite_logger) + self.__org_expected_install_packages_list = None + self.__org_expected_install_packages_version_list = None + self.__enable_installation_warning_status = False + def start_installation(self, simulate=False): """ Kick off a patch installation run """ self.status_handler.set_current_operation(Constants.INSTALLATION) @@ -123,6 +128,9 @@ def start_installation(self, simulate=False): 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 + # Reset expected packages and versions list: + self.__reset_expected_packages_and_version_list() + return overall_patch_installation_successful def write_installer_perf_logs(self, patch_operation_successful, installed_patch_count, retry_count, maintenance_window, maintenance_window_exceeded, task_status, error_msg): @@ -252,6 +260,10 @@ def install_updates(self, maintenance_window, package_manager, simulate=False): self.status_handler.set_package_install_status(self.skipped_esm_packages, self.skipped_esm_package_versions, Constants.FAILED) self.composite_logger.log("\nList of packages to be updated: \n" + str(packages)) + # Store the original list of packages and versions that were intended to be installed + self.__org_expected_install_packages_list = list(packages) + self.__org_expected_install_packages_version_list = list(package_versions) + sec_packages, sec_package_versions = self.package_manager.get_security_updates() self.telemetry_writer.write_event("Security packages out of the final package list: " + str(sec_packages), Constants.TelemetryEventLevel.Verbose) self.status_handler.set_package_install_status_classification(sec_packages, sec_package_versions, classification="Security") @@ -281,7 +293,12 @@ def install_updates(self, maintenance_window, package_manager, simulate=False): successful_parent_package_install_count_in_batch_patching = self.successful_parent_package_install_count if len(packages) == 0: - self.log_final_metrics(maintenance_window, patch_installation_successful, maintenance_window_exceeded, installed_update_count) + if not patch_installation_successful and not maintenance_window_exceeded and self.__check_if_all_packages_installed: + self.log_final_warning_metric(maintenance_window, installed_update_count) + self.__enable_installation_warning_status = True + else: + self.log_final_metrics(maintenance_window, patch_installation_successful, maintenance_window_exceeded, installed_update_count) + return installed_update_count, patch_installation_successful, maintenance_window_exceeded else: 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), @@ -324,7 +341,7 @@ def install_updates(self, maintenance_window, package_manager, simulate=False): # 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,7 +396,12 @@ def install_updates(self, maintenance_window, package_manager, simulate=False): 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) + + if not patch_installation_successful and not maintenance_window_exceeded and self.__check_if_all_packages_installed(): + self.log_final_warning_metric(maintenance_window, installed_update_count) + self.__enable_installation_warning_status = True + else: + 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 attempted_parent_package_install_count_in_sequential_patching = self.attempted_parent_package_install_count - attempted_parent_package_install_count_in_batch_patching @@ -388,7 +410,7 @@ def install_updates(self, maintenance_window, package_manager, simulate=False): sequential_processing_perf_log = "[{0}={1}][{2}={3}][{4}={5}][{6}={7}][{8}={9}]".format(Constants.PerfLogTrackerParams.TASK, "InstallPackagesSequentially", "InstalledPackagesCountInSequentialProcessing", install_update_count_in_sequential_patching, "AttemptedParentPackageInstallCount", attempted_parent_package_install_count_in_sequential_patching, - "SuccessfulParentPackageInstallCount", successful_parent_package_install_count_in_sequential_patching, "FailedParentPackageInstallCount", + "SuccessfulParentPackageInstallCount", successful_parent_package_install_count_in_sequential_patching, "FailedParentPackageInstallCount", failed_parent_package_install_count_after_sequential_patching) stopwatch_for_sequential_install_process.stop_and_write_telemetry(sequential_processing_perf_log) @@ -398,16 +420,15 @@ def install_updates(self, maintenance_window, package_manager, simulate=False): 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 +437,21 @@ def log_final_metrics(self, maintenance_window, patch_installation_successful, m 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): + """ + logs the final metrics for warning installatoin status. + """ + + self.__log_progress_status(maintenance_window, installed_update_count) + + message = "\n\nAll supposed packages are installed." + 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. @@ -427,7 +459,7 @@ def include_dependencies(self, package_manager, packages_in_batch, package_versi all_package_versions (List of strings): Versions of packages in all_packages. packages (List of strings): List of all packages selected by user to install. package_versions (List of strings): Versions of packages in packages list. - package_and_dependencies (List of strings): List of packages selected by user along with packages they are dependent on. The input package_and_dependencies + package_and_dependencies (List of strings): List of packages selected by user along with packages they are dependent on. The input package_and_dependencies does not contain dependent packages. The dependent packages are added in the list in this function. package_and_dependency_versions (List of strings): Versions of packages in package_and_dependencies. Input list does not contain versions of the dependent packages. The version of dependent packages are added in the list in this function. @@ -479,7 +511,7 @@ def batch_patching(self, all_packages, all_package_versions, packages, package_v stopwatch_for_phase.stop() - batch_phase_processing_perf_log = "[{0}={1}][{2}={3}][{4}={5}][{6}={7}][{8}={9}][{10}={11}][{12}={13}][{14}={15}]".format(Constants.PerfLogTrackerParams.TASK, "InstallPackagesInBatchInDifferentPhases", + batch_phase_processing_perf_log = "[{0}={1}][{2}={3}][{4}={5}][{6}={7}][{8}={9}][{10}={11}][{12}={13}][{14}={15}]".format(Constants.PerfLogTrackerParams.TASK, "InstallPackagesInBatchInDifferentPhases", "Phase", str(phase), "InstalledPackagesCount", str(installed_update_count), "SuccessfulParentPackageInstallCount", self.successful_parent_package_install_count, "FailedParentPackageInstallCount", self.failed_parent_package_install_count, "RemainingPackagesToInstall", str(len(packages)), Constants.PerfLogTrackerParams.PATCH_OPERATION_SUCCESSFUL, str(patch_installation_successful), "IsMaintenanceWindowBatchCutoffReached", str(maintenance_window_batch_cutoff_reached)) @@ -496,7 +528,7 @@ def batch_patching(self, all_packages, all_package_versions, packages, package_v stopwatch_for_batch_install_process.stop() - batch_processing_perf_log = "[{0}={1}][{2}={3}][{4}={5}][{6}={7}][{8}={9}][{10}={11}][{12}={13}][{14}={15}]".format(Constants.PerfLogTrackerParams.TASK, "InstallPackagesInBatches", + batch_processing_perf_log = "[{0}={1}][{2}={3}][{4}={5}][{6}={7}][{8}={9}][{10}={11}][{12}={13}][{14}={15}]".format(Constants.PerfLogTrackerParams.TASK, "InstallPackagesInBatches", "InstalledPackagesCountInBatchProcessing", str(installed_update_count_in_batch_patching), "AttemptedParentPackageInstallCount", self.attempted_parent_package_install_count, "SuccessfulParentPackageInstallCount", self.successful_parent_package_install_count, "FailedParentPackageInstallCount", self.failed_parent_package_install_count, "RemainingPackagesToInstall", str(len(packages)), Constants.PerfLogTrackerParams.PATCH_OPERATION_SUCCESSFUL, str(patch_installation_successful_in_batch_patching), @@ -509,9 +541,9 @@ def batch_patching(self, all_packages, all_package_versions, packages, package_v 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,16 +552,16 @@ def install_packages_in_batches(self, all_packages, all_package_versions, packag 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. maintenance_window_batch_cutoff_reached (bool): Whether process of installing packages in batches stopped due to not enough time in maintenance window to install packages in batch. - 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. + 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)) @@ -541,8 +573,8 @@ def install_packages_in_batches(self, all_packages, all_package_versions, packag # These packages will be attempted in sequential installation if there is enough time in maintenance window to install package sequentially. remaining_packages = [] remaining_package_versions = [] - - # failed_packages are the packages which are failed to install in batch patching. These packages will be attempted again in sequential patching if there is + + # failed_packages are the packages which are failed to install in batch patching. These packages will be attempted again in sequential patching if there is # enough time remaining in maintenance window. failed_packages = [] failed_package_versions = [] @@ -683,12 +715,17 @@ def mark_installation_completed(self): 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.__sent_metadata_health_store() + + def mark_installation_warning_completed(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 to a restriction in CRP, where installation substatus should be marked as warning only after the implicit (2nd) assessment operation + and all supposed packages are installed as expected + """ + 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.__sent_metadata_health_store() # region Installation Progress support def perform_status_reconciliation_conditionally(self, package_manager, condition=True): @@ -801,3 +838,39 @@ def get_max_batch_size(self, maintenance_window, package_manager): 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) + + # region - Failed packages retry succeeded + def __reset_expected_packages_and_version_list(self): + self.__org_expected_install_packages_list = None + self.__org_expected_install_packages_version_list = None + + def __check_if_all_packages_installed(self): + #type (none) -> bool + """ Check if all supposed packages are installed """ + # Get the list of installed packages + installed_packages_list = self.status_handler.get_installation_packages_list() + # Create a list to store uninstalled packages + uninstalled_packages_list = [ + (pkg, ver) for pkg, ver in zip(self.__org_expected_install_packages_list, self.__org_expected_install_packages_version_list) + if not any(installed_pkg['name'] == pkg and installed_pkg['version'] == ver and installed_pkg['patchInstallationState'] == Constants.INSTALLED for + installed_pkg in installed_packages_list) + ] + # Return True if all packages are installed, otherwise return False + return len(uninstalled_packages_list) == 0 + + def __sent_metadata_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 get_enabled_installation_warning_status(self): + """Access enable_installation_warning_status value""" + return self.__enable_installation_warning_status + # endregion diff --git a/src/core/src/service_interfaces/StatusHandler.py b/src/core/src/service_interfaces/StatusHandler.py index 94c8286d1..a749bc68c 100644 --- a/src/core/src/service_interfaces/StatusHandler.py +++ b/src/core/src/service_interfaces/StatusHandler.py @@ -262,6 +262,10 @@ 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 get_installation_packages_list(self): + """Access the installation packages data""" + return self.__installation_packages # endregion # region - Installation Reboot Status From b6e7003a4d35bcf79e4667d95621ed2df8958607 Mon Sep 17 00:00:00 2001 From: john feng Date: Wed, 26 Feb 2025 10:18:55 -0800 Subject: [PATCH 02/38] add print in statushandler to help with debug --- src/core/src/service_interfaces/StatusHandler.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/core/src/service_interfaces/StatusHandler.py b/src/core/src/service_interfaces/StatusHandler.py index a749bc68c..a27f3a9d1 100644 --- a/src/core/src/service_interfaces/StatusHandler.py +++ b/src/core/src/service_interfaces/StatusHandler.py @@ -210,6 +210,7 @@ def set_package_install_status(self, package_names, package_versions, status="Pe self.composite_logger.log_debug("Package install status summary [Status= " + status + "] : " + package_install_status_summary) self.__installation_packages = list(self.__installation_packages_map.values()) + print('what is __installation_packages', self.__installation_packages) self.__installation_packages = self.sort_packages_by_classification_and_state(self.__installation_packages) self.set_installation_substatus_json() From 091af986fc02bf89fc4d9790b7c2e881dca47ecf Mon Sep 17 00:00:00 2001 From: john feng Date: Wed, 26 Feb 2025 10:19:36 -0800 Subject: [PATCH 03/38] add print in patchinstaller --- src/core/src/core_logic/PatchInstaller.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/core/src/core_logic/PatchInstaller.py b/src/core/src/core_logic/PatchInstaller.py index 93f52e59d..884f20bfb 100644 --- a/src/core/src/core_logic/PatchInstaller.py +++ b/src/core/src/core_logic/PatchInstaller.py @@ -294,9 +294,11 @@ def install_updates(self, maintenance_window, package_manager, simulate=False): if len(packages) == 0: if not patch_installation_successful and not maintenance_window_exceeded and self.__check_if_all_packages_installed: + print('did this get called1', patch_installation_successful) self.log_final_warning_metric(maintenance_window, installed_update_count) self.__enable_installation_warning_status = True else: + print('did this get called2', patch_installation_successful) self.log_final_metrics(maintenance_window, patch_installation_successful, maintenance_window_exceeded, installed_update_count) return installed_update_count, patch_installation_successful, maintenance_window_exceeded @@ -398,9 +400,11 @@ def install_updates(self, maintenance_window, package_manager, simulate=False): installed_update_count += self.perform_status_reconciliation_conditionally(package_manager, True) if not patch_installation_successful and not maintenance_window_exceeded and self.__check_if_all_packages_installed(): + print('did this get called3', patch_installation_successful) self.log_final_warning_metric(maintenance_window, installed_update_count) self.__enable_installation_warning_status = True else: + print('did this get called4', patch_installation_successful) 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 @@ -853,6 +857,7 @@ def __check_if_all_packages_installed(self): """ Check if all supposed packages are installed """ # Get the list of installed packages installed_packages_list = self.status_handler.get_installation_packages_list() + print('what is installed_packages_list', installed_packages_list) # Create a list to store uninstalled packages uninstalled_packages_list = [ (pkg, ver) for pkg, ver in zip(self.__org_expected_install_packages_list, self.__org_expected_install_packages_version_list) @@ -860,6 +865,7 @@ def __check_if_all_packages_installed(self): installed_pkg in installed_packages_list) ] # Return True if all packages are installed, otherwise return False + print('what is uninstalled_packages_list', uninstalled_packages_list) return len(uninstalled_packages_list) == 0 def __sent_metadata_health_store(self): From 3f77ecd9ba81fcb5586713f1d065b8aca340e2f3 Mon Sep 17 00:00:00 2001 From: john feng Date: Wed, 26 Feb 2025 14:05:20 -0800 Subject: [PATCH 04/38] add unit test to test install successful on retry --- src/core/src/CoreMain.py | 3 +- src/core/src/core_logic/PatchInstaller.py | 6 -- src/core/tests/Test_CoreMain.py | 98 ++++++++++++++++++- .../tests/library/LegacyEnvLayerExtensions.py | 54 ++++++++++ 4 files changed, 152 insertions(+), 9 deletions(-) diff --git a/src/core/src/CoreMain.py b/src/core/src/CoreMain.py index 400efad03..072f3cf68 100644 --- a/src/core/src/CoreMain.py +++ b/src/core/src/CoreMain.py @@ -96,7 +96,6 @@ def __init__(self, argv): patch_installation_successful = 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() @@ -159,7 +158,7 @@ def update_patch_substatus_if_pending(patch_operation_requested, overall_patch_i if patch_operation_requested == Constants.INSTALLATION.lower() and not overall_patch_installation_operation_successful: status_handler.set_current_operation(Constants.INSTALLATION) if not patch_assessment_successful: - status_handler.add_error_to_status("Installation failed due to assessment failure. Please refer the error details in assessment substatus") + status_handler.add_error_to_status(f"Installation failed due to assessment failure. Please refer the error details in assessment substatus") status_handler.set_installation_substatus_json(status=Constants.STATUS_ERROR) # NOTE: For auto patching requests, no need to report patch metadata to health store in case of failure composite_logger.log_debug(' -- Persisted failed installation substatus.') diff --git a/src/core/src/core_logic/PatchInstaller.py b/src/core/src/core_logic/PatchInstaller.py index 884f20bfb..93f52e59d 100644 --- a/src/core/src/core_logic/PatchInstaller.py +++ b/src/core/src/core_logic/PatchInstaller.py @@ -294,11 +294,9 @@ def install_updates(self, maintenance_window, package_manager, simulate=False): if len(packages) == 0: if not patch_installation_successful and not maintenance_window_exceeded and self.__check_if_all_packages_installed: - print('did this get called1', patch_installation_successful) self.log_final_warning_metric(maintenance_window, installed_update_count) self.__enable_installation_warning_status = True else: - print('did this get called2', patch_installation_successful) self.log_final_metrics(maintenance_window, patch_installation_successful, maintenance_window_exceeded, installed_update_count) return installed_update_count, patch_installation_successful, maintenance_window_exceeded @@ -400,11 +398,9 @@ def install_updates(self, maintenance_window, package_manager, simulate=False): installed_update_count += self.perform_status_reconciliation_conditionally(package_manager, True) if not patch_installation_successful and not maintenance_window_exceeded and self.__check_if_all_packages_installed(): - print('did this get called3', patch_installation_successful) self.log_final_warning_metric(maintenance_window, installed_update_count) self.__enable_installation_warning_status = True else: - print('did this get called4', patch_installation_successful) 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 @@ -857,7 +853,6 @@ def __check_if_all_packages_installed(self): """ Check if all supposed packages are installed """ # Get the list of installed packages installed_packages_list = self.status_handler.get_installation_packages_list() - print('what is installed_packages_list', installed_packages_list) # Create a list to store uninstalled packages uninstalled_packages_list = [ (pkg, ver) for pkg, ver in zip(self.__org_expected_install_packages_list, self.__org_expected_install_packages_version_list) @@ -865,7 +860,6 @@ def __check_if_all_packages_installed(self): installed_pkg in installed_packages_list) ] # Return True if all packages are installed, otherwise return False - print('what is uninstalled_packages_list', uninstalled_packages_list) return len(uninstalled_packages_list) == 0 def __sent_metadata_health_store(self): diff --git a/src/core/tests/Test_CoreMain.py b/src/core/tests/Test_CoreMain.py index 06d149658..0c2bbb5e4 100644 --- a/src/core/tests/Test_CoreMain.py +++ b/src/core/tests/Test_CoreMain.py @@ -53,6 +53,18 @@ def mock_os_remove(self, file_to_remove): 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 """ + # This simulates the first batch attempt failing, returning original packages unchanged + # When the retry path kicks in, the package list should be processed and succeed there + 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, with no packages""" + # This simulates the first batch attempt failing, returning original packages unchanged + # When the retry path kicks in, the package list should be processed and succeed there + return [], [], 0, False + def test_operation_fail_for_non_autopatching_request(self): # Test for non auto patching request argument_composer = ArgumentComposer() @@ -1215,6 +1227,66 @@ def test_delete_temp_folder_contents_failure(self): 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 + + # Mock batch_patching with packages to return false + runtime.patch_installer.batch_patching = self.mock_batch_patching_with_no_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 __check_telemetry_events(self, runtime): all_events = os.listdir(runtime.telemetry_writer.events_folder_path) self.assertTrue(len(all_events) > 0) @@ -1225,6 +1297,30 @@ def __check_telemetry_events(self, runtime): 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.get_enabled_installation_warning_status()) + + # 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("packages are installed" in detail["message"] for detail in error_details)) + if __name__ == '__main__': - unittest.main() \ No newline at end of file + unittest.main() + diff --git a/src/core/tests/library/LegacyEnvLayerExtensions.py b/src/core/tests/library/LegacyEnvLayerExtensions.py index b24488987..7aad33e73 100644 --- a/src/core/tests/library/LegacyEnvLayerExtensions.py +++ b/src/core/tests/library/LegacyEnvLayerExtensions.py @@ -1307,6 +1307,60 @@ def run_command_output(self, cmd, no_output=False, chk_err=True): "\n" + \ "Total download size: 231 k\n" + \ "Operation aborted.\n" + elif self.legacy_test_type == 'PackageRetrySuccessPath': + # Track the current call using a tmp file to simulate first failure, then success + is_update_cmd = False + + if isinstance(cmd, str) and 'zypper' in cmd.lower() and ('update' in cmd.lower() or 'install' in cmd.lower()) and not ('--dry-run' in cmd.lower() or 'search' in cmd.lower() or 'list-updates' in cmd.lower()): + is_update_cmd = True + + if is_update_cmd: + tracking_file = os.path.join(self.temp_folder_path, 'package_retry_tracking') + + if not os.path.exists(tracking_file): + # First attempt - simulate failure + with open(tracking_file, 'w') as f: + f.write('failed_once') + code = 1 + output = "Package update failed, but can be retried" + return code, output + else: + # Second attempt - simulate success + code = 0 + output = "Successfully installed/updated kernel-default libgoa-1_0-0\nExecution complete." + return code, output + + # For non-update commands, return appropriate responses + if self.legacy_package_manager_name is Constants.ZYPPER: + if cmd.find("list-updates") > -1: + code = 0 + output = " Refreshing service 'cloud_update'.\n" + \ + " Loading repository data...\n" + \ + " Reading installed packages..\n" + \ + "S | Repository | Name | Current Version | Available Version | Arch\n" + \ + "--+--------------------+--------------------+-----------------+-------------------+-------#\n" + \ + "v | SLES12-SP2-Updates | kernel-default | 4.4.38-93.1 | 4.4.49-92.11.1 | x86_64\n" + \ + "v | SLES12-SP2-Updates | libgoa-1_0-0 | 3.20.4-7.2 | 3.20.5-9.6 | x86_64\n" + elif cmd.find("LANG=en_US.UTF8 zypper search -s") > -1: + code = 0 + output = "Loading repository data...\n" + \ + "Reading installed packages...\n" + \ + "\n" + \ + "S | Name | Type | Version | Arch | Repository\n" + \ + "---+------------------------+------------+---------------------+--------+-------------------\n" + \ + " i | selinux-policy | package | 3.13.1-102.el7_3.16 | noarch | SLES12-SP2-Updates\n" + \ + " i | libgoa-1_0-0 | package | 3.20.5-9.6 | noarch | SLES12-SP2-Updates\n" + \ + " i | kernel-default | package | 4.4.49-92.11.1 | noarch | SLES12-SP2-Updates\n" + elif cmd.find("--dry-run") > -1: + code = 0 + output = " Refreshing service 'SMT-http_smt-azure_susecloud_net'.\n" + \ + " Refreshing service 'cloud_update'.\n" + \ + " Loading repository data...\n" + \ + " Reading installed packages...\n" + \ + " Resolving package dependencies...\n" + \ + " The following package is going to be upgraded:\n" + \ + " kernel-default libgoa-1_0-0\n" + major_version = self.get_python_major_version() if major_version == 2: return code, output.decode('utf8', 'ignore').encode('ascii', 'ignore') From a9b23f855c2c1cc4e28c54ca5f0584fa4cf10740 Mon Sep 17 00:00:00 2001 From: john feng Date: Wed, 26 Feb 2025 14:19:42 -0800 Subject: [PATCH 05/38] remove print, revert linebreak --- src/core/src/CoreMain.py | 2 +- src/core/src/core_logic/PatchInstaller.py | 4 ++-- src/core/src/service_interfaces/StatusHandler.py | 4 ++-- src/core/tests/Test_CoreMain.py | 7 +++---- 4 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/core/src/CoreMain.py b/src/core/src/CoreMain.py index 072f3cf68..d420a7cfa 100644 --- a/src/core/src/CoreMain.py +++ b/src/core/src/CoreMain.py @@ -158,7 +158,7 @@ def update_patch_substatus_if_pending(patch_operation_requested, overall_patch_i if patch_operation_requested == Constants.INSTALLATION.lower() and not overall_patch_installation_operation_successful: status_handler.set_current_operation(Constants.INSTALLATION) if not patch_assessment_successful: - status_handler.add_error_to_status(f"Installation failed due to assessment failure. Please refer the error details in assessment substatus") + status_handler.add_error_to_status("Installation failed due to assessment failure. Please refer the error details in assessment substatus") status_handler.set_installation_substatus_json(status=Constants.STATUS_ERROR) # NOTE: For auto patching requests, no need to report patch metadata to health store in case of failure composite_logger.log_debug(' -- Persisted failed installation substatus.') diff --git a/src/core/src/core_logic/PatchInstaller.py b/src/core/src/core_logic/PatchInstaller.py index 93f52e59d..9a0059c3c 100644 --- a/src/core/src/core_logic/PatchInstaller.py +++ b/src/core/src/core_logic/PatchInstaller.py @@ -439,12 +439,12 @@ def log_final_metrics(self, maintenance_window, patch_installation_successful, m def log_final_warning_metric(self, maintenance_window, installed_update_count): """ - logs the final metrics for warning installatoin status. + logs the final metrics for warning installation status. """ self.__log_progress_status(maintenance_window, installed_update_count) - message = "\n\nAll supposed packages are installed." + message = "\n\nAll supposed package(s) are installed." self.status_handler.add_error_to_status(message, Constants.PatchOperationErrorCodes.PACKAGES_RETRY_SUCCEEDED) self.composite_logger.log_error(message) diff --git a/src/core/src/service_interfaces/StatusHandler.py b/src/core/src/service_interfaces/StatusHandler.py index a27f3a9d1..0ee2f2271 100644 --- a/src/core/src/service_interfaces/StatusHandler.py +++ b/src/core/src/service_interfaces/StatusHandler.py @@ -210,7 +210,6 @@ def set_package_install_status(self, package_names, package_versions, status="Pe self.composite_logger.log_debug("Package install status summary [Status= " + status + "] : " + package_install_status_summary) self.__installation_packages = list(self.__installation_packages_map.values()) - print('what is __installation_packages', self.__installation_packages) self.__installation_packages = self.sort_packages_by_classification_and_state(self.__installation_packages) self.set_installation_substatus_json() @@ -265,7 +264,8 @@ def get_os_name_and_version(self): return "unknownDist_unknownVer" def get_installation_packages_list(self): - """Access the installation packages data""" + #type (none) -> list + """Access installation packages data""" return self.__installation_packages # endregion diff --git a/src/core/tests/Test_CoreMain.py b/src/core/tests/Test_CoreMain.py index 0c2bbb5e4..bb46f2ef5 100644 --- a/src/core/tests/Test_CoreMain.py +++ b/src/core/tests/Test_CoreMain.py @@ -54,13 +54,13 @@ 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 """ + """Mock batch_patching to simulate package installation failure, return some packages """ # This simulates the first batch attempt failing, returning original packages unchanged # When the retry path kicks in, the package list should be processed and succeed there 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, with no packages""" + """Mock batch_patching to simulate package installation failure, return no packages""" # This simulates the first batch attempt failing, returning original packages unchanged # When the retry path kicks in, the package list should be processed and succeed there return [], [], 0, False @@ -1322,5 +1322,4 @@ def __assertion_pkg_succeed_on_retry(self, runtime): if __name__ == '__main__': - unittest.main() - + unittest.main() \ No newline at end of file From 68421aa0ba7d915d7a0a92399f8afb3c55b8c95b Mon Sep 17 00:00:00 2001 From: john feng Date: Wed, 26 Feb 2025 14:39:18 -0800 Subject: [PATCH 06/38] correct spelling --- src/core/tests/Test_CoreMain.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/tests/Test_CoreMain.py b/src/core/tests/Test_CoreMain.py index bb46f2ef5..9327d2a15 100644 --- a/src/core/tests/Test_CoreMain.py +++ b/src/core/tests/Test_CoreMain.py @@ -1318,7 +1318,7 @@ def __assertion_pkg_succeed_on_retry(self, runtime): # Verify at least one error detail about package retry error_details = json.loads(substatus_file_data[1]["formattedMessage"]["message"])["errors"]["details"] - self.assertTrue(any("packages are installed" in detail["message"] for detail in error_details)) + self.assertTrue(any("package(s) are installed" in detail["message"] for detail in error_details)) if __name__ == '__main__': From 1f757ef6007218dc837f70368d9df1825cb34ed9 Mon Sep 17 00:00:00 2001 From: john feng Date: Wed, 26 Feb 2025 22:43:15 -0800 Subject: [PATCH 07/38] remove comments --- src/core/src/core_logic/PatchInstaller.py | 41 +++++++++++------------ src/core/tests/Test_CoreMain.py | 4 --- 2 files changed, 19 insertions(+), 26 deletions(-) diff --git a/src/core/src/core_logic/PatchInstaller.py b/src/core/src/core_logic/PatchInstaller.py index 9a0059c3c..ec7dea07b 100644 --- a/src/core/src/core_logic/PatchInstaller.py +++ b/src/core/src/core_logic/PatchInstaller.py @@ -53,8 +53,6 @@ def __init__(self, env_layer, execution_config, composite_logger, telemetry_writ self.stopwatch = Stopwatch(self.env_layer, self.telemetry_writer, self.composite_logger) - self.__org_expected_install_packages_list = None - self.__org_expected_install_packages_version_list = None self.__enable_installation_warning_status = False def start_installation(self, simulate=False): @@ -128,9 +126,6 @@ def start_installation(self, simulate=False): 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 - # Reset expected packages and versions list: - self.__reset_expected_packages_and_version_list() - return overall_patch_installation_successful def write_installer_perf_logs(self, patch_operation_successful, installed_patch_count, retry_count, maintenance_window, maintenance_window_exceeded, task_status, error_msg): @@ -260,10 +255,6 @@ def install_updates(self, maintenance_window, package_manager, simulate=False): self.status_handler.set_package_install_status(self.skipped_esm_packages, self.skipped_esm_package_versions, Constants.FAILED) self.composite_logger.log("\nList of packages to be updated: \n" + str(packages)) - # Store the original list of packages and versions that were intended to be installed - self.__org_expected_install_packages_list = list(packages) - self.__org_expected_install_packages_version_list = list(package_versions) - sec_packages, sec_package_versions = self.package_manager.get_security_updates() self.telemetry_writer.write_event("Security packages out of the final package list: " + str(sec_packages), Constants.TelemetryEventLevel.Verbose) self.status_handler.set_package_install_status_classification(sec_packages, sec_package_versions, classification="Security") @@ -844,23 +835,29 @@ def __log_progress_status(self, maintenance_window, installed_update_count): self.composite_logger.log(progress_status) # region - Failed packages retry succeeded - def __reset_expected_packages_and_version_list(self): - self.__org_expected_install_packages_list = None - self.__org_expected_install_packages_version_list = None - def __check_if_all_packages_installed(self): #type (none) -> bool - """ Check if all supposed packages are installed """ + """ Check if all supposed security and critical packages are installed """ # Get the list of installed packages installed_packages_list = self.status_handler.get_installation_packages_list() - # Create a list to store uninstalled packages - uninstalled_packages_list = [ - (pkg, ver) for pkg, ver in zip(self.__org_expected_install_packages_list, self.__org_expected_install_packages_version_list) - if not any(installed_pkg['name'] == pkg and installed_pkg['version'] == ver and installed_pkg['patchInstallationState'] == Constants.INSTALLED for - installed_pkg in installed_packages_list) - ] - # Return True if all packages are installed, otherwise return False - return len(uninstalled_packages_list) == 0 + print('what is installed_packages_list', installed_packages_list) + # Get security and critical packages + security_critical_packages = [] + for package in installed_packages_list: + if 'classifications' in package and any(classification in ['Security', 'Critical'] for classification in package['classifications']): + security_critical_packages.append(package) + + # Return false there's no security/critical packages + if len(security_critical_packages) == 0: + return False + + # Check if any security/critical package are not installed + for package in security_critical_packages: + if package['patchInstallationState'] != Constants.INSTALLED: + return False + + # All security/critical packages are installed + return True def __sent_metadata_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))) diff --git a/src/core/tests/Test_CoreMain.py b/src/core/tests/Test_CoreMain.py index 9327d2a15..0ba46dbda 100644 --- a/src/core/tests/Test_CoreMain.py +++ b/src/core/tests/Test_CoreMain.py @@ -55,14 +55,10 @@ def mock_os_path_exists(self, patch_to_validate): 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 """ - # This simulates the first batch attempt failing, returning original packages unchanged - # When the retry path kicks in, the package list should be processed and succeed there 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""" - # This simulates the first batch attempt failing, returning original packages unchanged - # When the retry path kicks in, the package list should be processed and succeed there return [], [], 0, False def test_operation_fail_for_non_autopatching_request(self): From 4be150a0d22183f383a6aeda1e94c4da60aa6afe Mon Sep 17 00:00:00 2001 From: johnfeng Date: Wed, 12 Mar 2025 23:05:43 -0700 Subject: [PATCH 08/38] revert format changes --- src/core/src/CoreMain.py | 1 + src/core/src/core_logic/PatchInstaller.py | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/src/CoreMain.py b/src/core/src/CoreMain.py index d420a7cfa..400efad03 100644 --- a/src/core/src/CoreMain.py +++ b/src/core/src/CoreMain.py @@ -96,6 +96,7 @@ def __init__(self, argv): patch_installation_successful = 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() diff --git a/src/core/src/core_logic/PatchInstaller.py b/src/core/src/core_logic/PatchInstaller.py index ec7dea07b..410f571cf 100644 --- a/src/core/src/core_logic/PatchInstaller.py +++ b/src/core/src/core_logic/PatchInstaller.py @@ -19,7 +19,6 @@ import math import sys import time - from core.src.bootstrap.Constants import Constants from core.src.core_logic.Stopwatch import Stopwatch From 34bd06aea86000724f975348b767ed34b67444ed Mon Sep 17 00:00:00 2001 From: john feng Date: Wed, 19 Mar 2025 11:54:08 -0700 Subject: [PATCH 09/38] create fake comment to reset file format --- src/core/src/core_logic/PatchInstaller.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/src/core_logic/PatchInstaller.py b/src/core/src/core_logic/PatchInstaller.py index 63f8771c2..91366dc61 100644 --- a/src/core/src/core_logic/PatchInstaller.py +++ b/src/core/src/core_logic/PatchInstaller.py @@ -141,7 +141,7 @@ def write_installer_perf_logs(self, patch_operation_successful, installed_patch_ Constants.PerfLogTrackerParams.MAINTENANCE_WINDOW_EXCEEDED, str(maintenance_window_exceeded), Constants.PerfLogTrackerParams.MACHINE_INFO, self.telemetry_writer.machine_info) self.stopwatch.stop_and_write_telemetry(patch_installation_perf_log) return True - + "this is a testing" def raise_if_telemetry_unsupported(self): if self.lifecycle_manager.get_vm_cloud_type() == Constants.VMCloudType.ARC and self.execution_config.operation not in [Constants.ASSESSMENT, Constants.INSTALLATION]: self.composite_logger.log("Skipping telemetry compatibility check for Arc cloud type when operation is not manual") From 0fdab660f317fa6aa245f6d1c7214ff53ac485ec Mon Sep 17 00:00:00 2001 From: john feng Date: Wed, 19 Mar 2025 12:09:50 -0700 Subject: [PATCH 10/38] create conflict --- src/core/src/core_logic/PatchInstaller.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/src/core_logic/PatchInstaller.py b/src/core/src/core_logic/PatchInstaller.py index 63f8771c2..3179266b9 100644 --- a/src/core/src/core_logic/PatchInstaller.py +++ b/src/core/src/core_logic/PatchInstaller.py @@ -133,7 +133,7 @@ def write_installer_perf_logs(self, patch_operation_successful, installed_patch_ except Exception as error: self.composite_logger.log_debug("Error in writing patch installation performance logs. Error is: " + repr(error)) - patch_installation_perf_log = "[{0}={1}][{2}={3}][{4}={5}][{6}={7}][{8}={9}][{10}={11}][{12}={13}][{14}={15}][{16}={17}][{18}={19}][{20}={21}]".format( + patch_installation_perf_log = "testtest[{0}={1}][{2}={3}][{4}={5}][{6}={7}][{8}={9}][{10}={11}][{12}={13}][{14}={15}][{16}={17}][{18}={19}][{20}={21}]".format( Constants.PerfLogTrackerParams.TASK, Constants.INSTALLATION, Constants.PerfLogTrackerParams.TASK_STATUS, str(task_status), Constants.PerfLogTrackerParams.ERROR_MSG, error_msg, Constants.PerfLogTrackerParams.PACKAGE_MANAGER, self.package_manager_name, Constants.PerfLogTrackerParams.PATCH_OPERATION_SUCCESSFUL, str(patch_operation_successful), Constants.PerfLogTrackerParams.INSTALLED_PATCH_COUNT, str(installed_patch_count), Constants.PerfLogTrackerParams.RETRY_COUNT, str(retry_count), From 000dba1c9270b726753a9471f0c5045bf1bb79ee Mon Sep 17 00:00:00 2001 From: john feng Date: Wed, 19 Mar 2025 12:10:52 -0700 Subject: [PATCH 11/38] create conflict --- src/core/src/core_logic/PatchInstaller.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/src/core_logic/PatchInstaller.py b/src/core/src/core_logic/PatchInstaller.py index 9eb5d257e..410f571cf 100644 --- a/src/core/src/core_logic/PatchInstaller.py +++ b/src/core/src/core_logic/PatchInstaller.py @@ -143,7 +143,7 @@ def write_installer_perf_logs(self, patch_operation_successful, installed_patch_ Constants.PerfLogTrackerParams.MAINTENANCE_WINDOW_EXCEEDED, str(maintenance_window_exceeded), Constants.PerfLogTrackerParams.MACHINE_INFO, self.telemetry_writer.machine_info) self.stopwatch.stop_and_write_telemetry(patch_installation_perf_log) return True - "this is a testing" + def raise_if_telemetry_unsupported(self): if self.lifecycle_manager.get_vm_cloud_type() == Constants.VMCloudType.ARC and self.execution_config.operation not in [Constants.ASSESSMENT, Constants.INSTALLATION]: self.composite_logger.log("Skipping telemetry compatibility check for Arc cloud type when operation is not manual") From f2bc6d8a5eb1903d15fe5c290e85f622dc6998a1 Mon Sep 17 00:00:00 2001 From: john feng Date: Wed, 19 Mar 2025 12:13:10 -0700 Subject: [PATCH 12/38] remove print --- src/core/src/core_logic/PatchInstaller.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/core/src/core_logic/PatchInstaller.py b/src/core/src/core_logic/PatchInstaller.py index d4120e11d..82d85eeb5 100644 --- a/src/core/src/core_logic/PatchInstaller.py +++ b/src/core/src/core_logic/PatchInstaller.py @@ -135,7 +135,7 @@ def write_installer_perf_logs(self, patch_operation_successful, installed_patch_ except Exception as error: self.composite_logger.log_debug("Error in writing patch installation performance logs. Error is: " + repr(error)) - patch_installation_perf_log = "testtest[{0}={1}][{2}={3}][{4}={5}][{6}={7}][{8}={9}][{10}={11}][{12}={13}][{14}={15}][{16}={17}][{18}={19}][{20}={21}]".format( + patch_installation_perf_log = "[{0}={1}][{2}={3}][{4}={5}][{6}={7}][{8}={9}][{10}={11}][{12}={13}][{14}={15}][{16}={17}][{18}={19}][{20}={21}]".format( Constants.PerfLogTrackerParams.TASK, Constants.INSTALLATION, Constants.PerfLogTrackerParams.TASK_STATUS, str(task_status), Constants.PerfLogTrackerParams.ERROR_MSG, error_msg, Constants.PerfLogTrackerParams.PACKAGE_MANAGER, self.package_manager_name, Constants.PerfLogTrackerParams.PATCH_OPERATION_SUCCESSFUL, str(patch_operation_successful), Constants.PerfLogTrackerParams.INSTALLED_PATCH_COUNT, str(installed_patch_count), Constants.PerfLogTrackerParams.RETRY_COUNT, str(retry_count), @@ -839,7 +839,6 @@ def __check_if_all_packages_installed(self): """ Check if all supposed security and critical packages are installed """ # Get the list of installed packages installed_packages_list = self.status_handler.get_installation_packages_list() - print('what is installed_packages_list', installed_packages_list) # Get security and critical packages security_critical_packages = [] for package in installed_packages_list: From ead52965b4940ef4ceec705f19fa655ef8e86128 Mon Sep 17 00:00:00 2001 From: john feng Date: Wed, 19 Mar 2025 12:32:27 -0700 Subject: [PATCH 13/38] add fake comment --- src/core/src/core_logic/PatchInstaller.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/src/core_logic/PatchInstaller.py b/src/core/src/core_logic/PatchInstaller.py index 63f8771c2..6a53e8230 100644 --- a/src/core/src/core_logic/PatchInstaller.py +++ b/src/core/src/core_logic/PatchInstaller.py @@ -386,7 +386,7 @@ def install_updates(self, maintenance_window, package_manager, simulate=False): successful_parent_package_install_count_in_sequential_patching = self.successful_parent_package_install_count - successful_parent_package_install_count_in_batch_patching failed_parent_package_install_count_after_sequential_patching = self.failed_parent_package_install_count - sequential_processing_perf_log = "[{0}={1}][{2}={3}][{4}={5}][{6}={7}][{8}={9}]".format(Constants.PerfLogTrackerParams.TASK, "InstallPackagesSequentially", "InstalledPackagesCountInSequentialProcessing", + sequential_processing_perf_log = "testtesttest[{0}={1}][{2}={3}][{4}={5}][{6}={7}][{8}={9}]".format(Constants.PerfLogTrackerParams.TASK, "InstallPackagesSequentially", "InstalledPackagesCountInSequentialProcessing", install_update_count_in_sequential_patching, "AttemptedParentPackageInstallCount", attempted_parent_package_install_count_in_sequential_patching, "SuccessfulParentPackageInstallCount", successful_parent_package_install_count_in_sequential_patching, "FailedParentPackageInstallCount", failed_parent_package_install_count_after_sequential_patching) @@ -479,7 +479,7 @@ def batch_patching(self, all_packages, all_package_versions, packages, package_v stopwatch_for_phase.stop() - batch_phase_processing_perf_log = "[{0}={1}][{2}={3}][{4}={5}][{6}={7}][{8}={9}][{10}={11}][{12}={13}][{14}={15}]".format(Constants.PerfLogTrackerParams.TASK, "InstallPackagesInBatchInDifferentPhases", + batch_phase_processing_perf_log = "testtestste[{0}={1}][{2}={3}][{4}={5}][{6}={7}][{8}={9}][{10}={11}][{12}={13}][{14}={15}]".format(Constants.PerfLogTrackerParams.TASK, "InstallPackagesInBatchInDifferentPhases", "Phase", str(phase), "InstalledPackagesCount", str(installed_update_count), "SuccessfulParentPackageInstallCount", self.successful_parent_package_install_count, "FailedParentPackageInstallCount", self.failed_parent_package_install_count, "RemainingPackagesToInstall", str(len(packages)), Constants.PerfLogTrackerParams.PATCH_OPERATION_SUCCESSFUL, str(patch_installation_successful), "IsMaintenanceWindowBatchCutoffReached", str(maintenance_window_batch_cutoff_reached)) From 7e57abcc600cb5b9e739c0df468fcabb6ff007dc Mon Sep 17 00:00:00 2001 From: john feng Date: Wed, 19 Mar 2025 13:11:28 -0700 Subject: [PATCH 14/38] reset format --- src/core/src/core_logic/PatchInstaller.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/src/core_logic/PatchInstaller.py b/src/core/src/core_logic/PatchInstaller.py index 6a53e8230..50d8deaf3 100644 --- a/src/core/src/core_logic/PatchInstaller.py +++ b/src/core/src/core_logic/PatchInstaller.py @@ -496,7 +496,7 @@ def batch_patching(self, all_packages, all_package_versions, packages, package_v stopwatch_for_batch_install_process.stop() - batch_processing_perf_log = "[{0}={1}][{2}={3}][{4}={5}][{6}={7}][{8}={9}][{10}={11}][{12}={13}][{14}={15}]".format(Constants.PerfLogTrackerParams.TASK, "InstallPackagesInBatches", + batch_processing_perf_log = "testtes12[{0}={1}][{2}={3}][{4}={5}][{6}={7}][{8}={9}][{10}={11}][{12}={13}][{14}={15}]".format(Constants.PerfLogTrackerParams.TASK, "InstallPackagesInBatches", "InstalledPackagesCountInBatchProcessing", str(installed_update_count_in_batch_patching), "AttemptedParentPackageInstallCount", self.attempted_parent_package_install_count, "SuccessfulParentPackageInstallCount", self.successful_parent_package_install_count, "FailedParentPackageInstallCount", self.failed_parent_package_install_count, "RemainingPackagesToInstall", str(len(packages)), Constants.PerfLogTrackerParams.PATCH_OPERATION_SUCCESSFUL, str(patch_installation_successful_in_batch_patching), @@ -646,7 +646,7 @@ def install_packages_in_batches(self, all_packages, all_package_versions, packag # dependency package result management fallback (not reliable enough to be used as primary, and will be removed; remember to retain last_still_needed refresh when you do that) installed_update_count += self.perform_status_reconciliation_conditionally(package_manager, condition=(self.attempted_parent_package_install_count % Constants.PACKAGE_STATUS_REFRESH_RATE_IN_SECONDS == 0)) # reconcile status after every 10 attempted installs - per_batch_install_perf_log = "[{0}={1}][{2}={3}][{4}={5}][{6}={7}][{8}={9}][{10}={11}][{12}={13}][{14}={15}]".format(Constants.PerfLogTrackerParams.TASK, "InstallBatchOfPackages", + per_batch_install_perf_log = "testtest[{0}={1}][{2}={3}][{4}={5}][{6}={7}][{8}={9}][{10}={11}][{12}={13}][{14}={15}]".format(Constants.PerfLogTrackerParams.TASK, "InstallBatchOfPackages", "PackagesInBatch", str(packages_in_batch), "PackageAndDependencies", str(package_and_dependencies), "PackageAndDependencyVersions", str(package_and_dependency_versions), "NumberOfParentPackagesInstalled", str(parent_packages_installed_in_batch_count), "NumberOfParentPackagesFailed", str(parent_packages_failed_in_batch_count), "NumberOfDependenciesInstalled", str(number_of_dependencies_installed), "NumberOfDependenciesFailed", str(number_of_dependencies_failed)) From 2171a610badc1b4552b0fd9d010d1508be38370e Mon Sep 17 00:00:00 2001 From: john feng Date: Wed, 19 Mar 2025 13:31:20 -0700 Subject: [PATCH 15/38] remove add trailing space --- src/core/src/core_logic/PatchInstaller.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/src/core_logic/PatchInstaller.py b/src/core/src/core_logic/PatchInstaller.py index 0037ce053..adb6ee1b9 100644 --- a/src/core/src/core_logic/PatchInstaller.py +++ b/src/core/src/core_logic/PatchInstaller.py @@ -501,7 +501,7 @@ def batch_patching(self, all_packages, all_package_versions, packages, package_v stopwatch_for_phase.stop() - batch_phase_processing_perf_log = "[{0}={1}][{2}={3}][{4}={5}][{6}={7}][{8}={9}][{10}={11}][{12}={13}][{14}={15}]".format(Constants.PerfLogTrackerParams.TASK, "InstallPackagesInBatchInDifferentPhases", + batch_phase_processing_perf_log = "[{0}={1}][{2}={3}][{4}={5}][{6}={7}][{8}={9}][{10}={11}][{12}={13}][{14}={15}]".format(Constants.PerfLogTrackerParams.TASK, "InstallPackagesInBatchInDifferentPhases", "Phase", str(phase), "InstalledPackagesCount", str(installed_update_count), "SuccessfulParentPackageInstallCount", self.successful_parent_package_install_count, "FailedParentPackageInstallCount", self.failed_parent_package_install_count, "RemainingPackagesToInstall", str(len(packages)), Constants.PerfLogTrackerParams.PATCH_OPERATION_SUCCESSFUL, str(patch_installation_successful), "IsMaintenanceWindowBatchCutoffReached", str(maintenance_window_batch_cutoff_reached)) @@ -518,7 +518,7 @@ def batch_patching(self, all_packages, all_package_versions, packages, package_v stopwatch_for_batch_install_process.stop() - batch_processing_perf_log = "[{0}={1}][{2}={3}][{4}={5}][{6}={7}][{8}={9}][{10}={11}][{12}={13}][{14}={15}]".format(Constants.PerfLogTrackerParams.TASK, "InstallPackagesInBatches", + batch_processing_perf_log = "[{0}={1}][{2}={3}][{4}={5}][{6}={7}][{8}={9}][{10}={11}][{12}={13}][{14}={15}]".format(Constants.PerfLogTrackerParams.TASK, "InstallPackagesInBatches", "InstalledPackagesCountInBatchProcessing", str(installed_update_count_in_batch_patching), "AttemptedParentPackageInstallCount", self.attempted_parent_package_install_count, "SuccessfulParentPackageInstallCount", self.successful_parent_package_install_count, "FailedParentPackageInstallCount", self.failed_parent_package_install_count, "RemainingPackagesToInstall", str(len(packages)), Constants.PerfLogTrackerParams.PATCH_OPERATION_SUCCESSFUL, str(patch_installation_successful_in_batch_patching), From e64f078d87178bf329ed52af7f4a9342dc0809d3 Mon Sep 17 00:00:00 2001 From: john feng Date: Wed, 26 Mar 2025 10:49:30 -0700 Subject: [PATCH 16/38] refactor functions and comments --- src/core/src/CoreMain.py | 2 +- src/core/src/core_logic/PatchInstaller.py | 27 +++++++++++------------ 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/src/core/src/CoreMain.py b/src/core/src/CoreMain.py index e4e35fc83..430ee5332 100644 --- a/src/core/src/CoreMain.py +++ b/src/core/src/CoreMain.py @@ -102,7 +102,7 @@ def __init__(self, argv): patch_installer.mark_installation_completed() overall_patch_installation_operation_successful = True elif patch_installer.get_enabled_installation_warning_status(): - patch_installer.mark_installation_warning_completed() + 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: diff --git a/src/core/src/core_logic/PatchInstaller.py b/src/core/src/core_logic/PatchInstaller.py index adb6ee1b9..1a8ff561c 100644 --- a/src/core/src/core_logic/PatchInstaller.py +++ b/src/core/src/core_logic/PatchInstaller.py @@ -283,7 +283,7 @@ def install_updates(self, maintenance_window, package_manager, simulate=False): successful_parent_package_install_count_in_batch_patching = self.successful_parent_package_install_count if len(packages) == 0: - if not patch_installation_successful and not maintenance_window_exceeded and self.__check_if_all_packages_installed: + if not patch_installation_successful and not maintenance_window_exceeded and self.__check_all_requested_packages_install_state: self.log_final_warning_metric(maintenance_window, installed_update_count) self.__enable_installation_warning_status = True else: @@ -387,7 +387,7 @@ def install_updates(self, maintenance_window, package_manager, simulate=False): self.composite_logger.log_debug("\nPerforming final system state reconciliation...") installed_update_count += self.perform_status_reconciliation_conditionally(package_manager, True) - if not patch_installation_successful and not maintenance_window_exceeded and self.__check_if_all_packages_installed(): + if not patch_installation_successful and not maintenance_window_exceeded and self.__check_all_requested_packages_install_state(): self.log_final_warning_metric(maintenance_window, installed_update_count) self.__enable_installation_warning_status = True else: @@ -428,13 +428,11 @@ def log_final_metrics(self, maintenance_window, patch_installation_successful, m self.composite_logger.log_error(message) def log_final_warning_metric(self, maintenance_window, installed_update_count): - """ - logs the final metrics for warning installation status. - """ + """ Log the final metrics for warning installation status. """ self.__log_progress_status(maintenance_window, installed_update_count) - message = "\n\nAll supposed package(s) are installed." + message = "\n\nAll 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) @@ -705,17 +703,17 @@ def mark_installation_completed(self): 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.__sent_metadata_health_store() + self.__send_metadata_health_store() - def mark_installation_warning_completed(self): + 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 to a restriction in CRP, where installation substatus should be marked as warning only after the implicit (2nd) assessment operation - and all supposed packages are installed as expected - """ + and all supposed packages are installed as expected. """ + 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.__sent_metadata_health_store() + self.__send_metadata_health_store() # region Installation Progress support def perform_status_reconciliation_conditionally(self, package_manager, condition=True): @@ -834,9 +832,10 @@ def __log_progress_status(self, maintenance_window, installed_update_count): self.composite_logger.log(progress_status) # region - Failed packages retry succeeded - def __check_if_all_packages_installed(self): + def __check_all_requested_packages_install_state(self): #type (none) -> bool - """ Check if all supposed security and critical packages are installed """ + """ Check if all requested security and critical packages are installed. """ + # Get the list of installed packages installed_packages_list = self.status_handler.get_installation_packages_list() # Get security and critical packages @@ -857,7 +856,7 @@ def __check_if_all_packages_installed(self): # All security/critical packages are installed return True - def __sent_metadata_health_store(self): + def __send_metadata_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( From c3f80b34bd9b688b181c68848fa7fbd6df6df6ab Mon Sep 17 00:00:00 2001 From: john feng Date: Wed, 26 Mar 2025 12:35:23 -0700 Subject: [PATCH 17/38] add extra line btw imports and class --- src/core/src/core_logic/PatchInstaller.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/core/src/core_logic/PatchInstaller.py b/src/core/src/core_logic/PatchInstaller.py index 1a8ff561c..d0e817963 100644 --- a/src/core/src/core_logic/PatchInstaller.py +++ b/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): From 008e919b33ee59aa9ad4900607c164be19d937bc Mon Sep 17 00:00:00 2001 From: john feng Date: Thu, 27 Mar 2025 10:01:48 -0700 Subject: [PATCH 18/38] remove extra spaces --- src/core/src/core_logic/PatchInstaller.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/core/src/core_logic/PatchInstaller.py b/src/core/src/core_logic/PatchInstaller.py index d0e817963..55b9e62e1 100644 --- a/src/core/src/core_logic/PatchInstaller.py +++ b/src/core/src/core_logic/PatchInstaller.py @@ -332,7 +332,7 @@ def install_updates(self, maintenance_window, package_manager, simulate=False): # 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 @@ -411,7 +411,7 @@ def install_updates(self, maintenance_window, package_manager, simulate=False): 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. @@ -440,7 +440,7 @@ def log_final_warning_metric(self, maintenance_window, installed_update_count): 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. @@ -530,9 +530,9 @@ def batch_patching(self, all_packages, all_package_versions, packages, package_v 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. @@ -541,7 +541,7 @@ def install_packages_in_batches(self, all_packages, all_package_versions, packag 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. @@ -550,7 +550,7 @@ def install_packages_in_batches(self, all_packages, all_package_versions, packag 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)) @@ -562,7 +562,7 @@ def install_packages_in_batches(self, all_packages, all_package_versions, packag # These packages will be attempted in sequential installation if there is enough time in maintenance window to install package sequentially. remaining_packages = [] remaining_package_versions = [] - + # failed_packages are the packages which are failed to install in batch patching. These packages will be attempted again in sequential patching if there is # enough time remaining in maintenance window. failed_packages = [] From 4a4dcc3abf744e2992d3f3a0623647c2b61d9727 Mon Sep 17 00:00:00 2001 From: john feng Date: Thu, 27 Mar 2025 17:02:19 -0700 Subject: [PATCH 19/38] extract log metric into a function --- src/core/src/core_logic/PatchInstaller.py | 19 +++++++++---------- src/core/tests/Test_CoreMain.py | 23 ++++++++++++++++++++++- 2 files changed, 31 insertions(+), 11 deletions(-) diff --git a/src/core/src/core_logic/PatchInstaller.py b/src/core/src/core_logic/PatchInstaller.py index 55b9e62e1..5ac7053e2 100644 --- a/src/core/src/core_logic/PatchInstaller.py +++ b/src/core/src/core_logic/PatchInstaller.py @@ -284,11 +284,7 @@ def install_updates(self, maintenance_window, package_manager, simulate=False): successful_parent_package_install_count_in_batch_patching = self.successful_parent_package_install_count if len(packages) == 0: - if not patch_installation_successful and not maintenance_window_exceeded and self.__check_all_requested_packages_install_state: - self.log_final_warning_metric(maintenance_window, installed_update_count) - self.__enable_installation_warning_status = True - else: - self.log_final_metrics(maintenance_window, patch_installation_successful, maintenance_window_exceeded, installed_update_count) + self.log_final_error_status_msg(patch_installation_successful, maintenance_window_exceeded, maintenance_window, installed_update_count) return installed_update_count, patch_installation_successful, maintenance_window_exceeded else: @@ -388,11 +384,7 @@ def install_updates(self, maintenance_window, package_manager, simulate=False): self.composite_logger.log_debug("\nPerforming final system state reconciliation...") installed_update_count += self.perform_status_reconciliation_conditionally(package_manager, True) - if not patch_installation_successful and not maintenance_window_exceeded and self.__check_all_requested_packages_install_state(): - self.log_final_warning_metric(maintenance_window, installed_update_count) - self.__enable_installation_warning_status = True - else: - self.log_final_metrics(maintenance_window, patch_installation_successful, maintenance_window_exceeded, installed_update_count) + self.log_final_error_status_msg(patch_installation_successful, maintenance_window_exceeded, maintenance_window, installed_update_count) install_update_count_in_sequential_patching = installed_update_count - install_update_count_in_batch_patching attempted_parent_package_install_count_in_sequential_patching = self.attempted_parent_package_install_count - attempted_parent_package_install_count_in_batch_patching @@ -833,6 +825,13 @@ def __log_progress_status(self, maintenance_window, installed_update_count): self.composite_logger.log(progress_status) # region - Failed packages retry succeeded + def log_final_error_status_msg(self, patch_installation_successful, maintenance_window_exceeded, maintenance_window, installed_update_count): + if not patch_installation_successful and not maintenance_window_exceeded and self.__check_all_requested_packages_install_state(): + self.log_final_warning_metric(maintenance_window, installed_update_count) + self.__enable_installation_warning_status = True + else: + self.log_final_metrics(maintenance_window, patch_installation_successful, maintenance_window_exceeded, installed_update_count) + def __check_all_requested_packages_install_state(self): #type (none) -> bool """ Check if all requested security and critical packages are installed. """ diff --git a/src/core/tests/Test_CoreMain.py b/src/core/tests/Test_CoreMain.py index df571fe94..708be943b 100644 --- a/src/core/tests/Test_CoreMain.py +++ b/src/core/tests/Test_CoreMain.py @@ -61,6 +61,25 @@ def mock_batch_patching_with_no_packages(self, all_packages, all_package_version """Mock batch_patching to simulate package installation failure, return no packages""" return [], [], 0, False + def mock_installed_pkg_list(self): + """ Mock list of security and critical installed packages. """ + return [ + { + 'patchId': 'kernel-default_4.4.49-92.11.1_Ubuntu_16.04', + 'name': 'kernel-default', + 'version': '4.4.49-92.11.1', + 'classifications': ['Security'], + 'patchInstallationState': 'Installed' + }, + { + 'patchId': 'libgoa-1_0-0_3.20.5-9.6_Ubuntu_16.04', + 'name': 'libgoa-1_0-0', + 'version': '3.20.5-9.6', + 'classifications': ['Security'], + 'patchInstallationState': 'Installed' + } + ] + def test_operation_fail_for_non_autopatching_request(self): # Test for non auto patching request argument_composer = ArgumentComposer() @@ -1276,9 +1295,10 @@ def test_warning_status_when_packages_initially_fail_but_succeed_on_retry_no_bat # Store original methods original_batch_patching = runtime.patch_installer.batch_patching - + original_installation_pkg_list = runtime.status_handler.get_installation_packages_list # Mock batch_patching with packages to return false runtime.patch_installer.batch_patching = self.mock_batch_patching_with_no_packages + runtime.status_handler.get_installation_packages_list = self.mock_installed_pkg_list # Run CoreMain to execute the installation try: @@ -1288,6 +1308,7 @@ def test_warning_status_when_packages_initially_fail_but_succeed_on_retry_no_bat finally: # reset mock runtime.patch_installer.batch_patching = original_batch_patching + runtime.status_handler.get_installation_packages_list = original_installation_pkg_list runtime.stop() def __check_telemetry_events(self, runtime): From 8922159a42e6253107c5b82f16860342d5fd9d38 Mon Sep 17 00:00:00 2001 From: john feng Date: Mon, 7 Apr 2025 12:52:08 -0700 Subject: [PATCH 20/38] apply code refactor --- src/core/src/CoreMain.py | 2 +- src/core/src/core_logic/PatchInstaller.py | 36 +++++++++---------- .../src/service_interfaces/StatusHandler.py | 2 +- src/core/tests/Test_CoreMain.py | 2 +- 4 files changed, 20 insertions(+), 22 deletions(-) diff --git a/src/core/src/CoreMain.py b/src/core/src/CoreMain.py index 430ee5332..a52fecb41 100644 --- a/src/core/src/CoreMain.py +++ b/src/core/src/CoreMain.py @@ -101,7 +101,7 @@ def __init__(self, argv): if patch_assessment_successful and patch_installation_successful: patch_installer.mark_installation_completed() overall_patch_installation_operation_successful = True - elif patch_installer.get_enabled_installation_warning_status(): + elif patch_installer.set_patch_installation_status_to_warning_from_failed(): 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) diff --git a/src/core/src/core_logic/PatchInstaller.py b/src/core/src/core_logic/PatchInstaller.py index 5ac7053e2..b710489aa 100644 --- a/src/core/src/core_logic/PatchInstaller.py +++ b/src/core/src/core_logic/PatchInstaller.py @@ -53,7 +53,7 @@ def __init__(self, env_layer, execution_config, composite_logger, telemetry_writ self.stopwatch = Stopwatch(self.env_layer, self.telemetry_writer, self.composite_logger) - self.__enable_installation_warning_status = False + self.__enable_installation_status_to_warning_flag = False def start_installation(self, simulate=False): """ Kick off a patch installation run """ @@ -284,7 +284,8 @@ def install_updates(self, maintenance_window, package_manager, simulate=False): successful_parent_package_install_count_in_batch_patching = self.successful_parent_package_install_count if len(packages) == 0: - self.log_final_error_status_msg(patch_installation_successful, maintenance_window_exceeded, maintenance_window, installed_update_count) + self.__enable_installation_status_to_warning_flag = self.__check_installation_status_can_set_to_warning(patch_installation_successful, maintenance_window_exceeded) + self.log_final_installation_metric(patch_installation_successful, maintenance_window_exceeded, maintenance_window, installed_update_count) return installed_update_count, patch_installation_successful, maintenance_window_exceeded else: @@ -384,7 +385,8 @@ def install_updates(self, maintenance_window, package_manager, simulate=False): self.composite_logger.log_debug("\nPerforming final system state reconciliation...") installed_update_count += self.perform_status_reconciliation_conditionally(package_manager, True) - self.log_final_error_status_msg(patch_installation_successful, maintenance_window_exceeded, maintenance_window, installed_update_count) + self.__enable_installation_status_to_warning_flag = self.__check_installation_status_can_set_to_warning(patch_installation_successful, maintenance_window_exceeded) + self.log_final_installation_metric(patch_installation_successful, maintenance_window_exceeded, maintenance_window, installed_update_count) install_update_count_in_sequential_patching = installed_update_count - install_update_count_in_batch_patching attempted_parent_package_install_count_in_sequential_patching = self.attempted_parent_package_install_count - attempted_parent_package_install_count_in_batch_patching @@ -424,8 +426,7 @@ 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 = "\n\nAll requested package(s) are installed. Any patch errors marked are from previous attempts." + 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) @@ -522,9 +523,7 @@ def batch_patching(self, all_packages, all_package_versions, packages, package_v 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. @@ -533,7 +532,6 @@ def install_packages_in_batches(self, all_packages, all_package_versions, packag 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. @@ -542,7 +540,6 @@ def install_packages_in_batches(self, all_packages, all_package_versions, packag 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)) @@ -554,7 +551,6 @@ def install_packages_in_batches(self, all_packages, all_package_versions, packag # These packages will be attempted in sequential installation if there is enough time in maintenance window to install package sequentially. remaining_packages = [] remaining_package_versions = [] - # failed_packages are the packages which are failed to install in batch patching. These packages will be attempted again in sequential patching if there is # enough time remaining in maintenance window. failed_packages = [] @@ -696,7 +692,7 @@ def mark_installation_completed(self): 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_health_store() + 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. @@ -706,7 +702,7 @@ def mark_installation_completed_with_warning(self): 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_health_store() + self.__send_metadata_to_health_store() # region Installation Progress support def perform_status_reconciliation_conditionally(self, package_manager, condition=True): @@ -825,15 +821,17 @@ def __log_progress_status(self, maintenance_window, installed_update_count): self.composite_logger.log(progress_status) # region - Failed packages retry succeeded - def log_final_error_status_msg(self, patch_installation_successful, maintenance_window_exceeded, maintenance_window, installed_update_count): - if not patch_installation_successful and not maintenance_window_exceeded and self.__check_all_requested_packages_install_state(): + def log_final_installation_metric(self, patch_installation_successful, maintenance_window_exceeded, maintenance_window, installed_update_count): + if self.__enable_installation_status_to_warning_flag: self.log_final_warning_metric(maintenance_window, installed_update_count) - self.__enable_installation_warning_status = True else: self.log_final_metrics(maintenance_window, patch_installation_successful, maintenance_window_exceeded, installed_update_count) + def __check_installation_status_can_set_to_warning(self, patch_installation_successful, maintenance_window_exceeded): + return not patch_installation_successful and not maintenance_window_exceeded and self.__check_all_requested_packages_install_state() + def __check_all_requested_packages_install_state(self): - #type (none) -> bool + # type (none) -> bool """ Check if all requested security and critical packages are installed. """ # Get the list of installed packages @@ -856,7 +854,7 @@ def __check_all_requested_packages_install_state(self): # All security/critical packages are installed return True - def __send_metadata_health_store(self): + 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( @@ -864,7 +862,7 @@ def __send_metadata_health_store(self): report_to_healthstore=True, wait_after_update=False) - def get_enabled_installation_warning_status(self): + def set_patch_installation_status_to_warning_from_failed(self): """Access enable_installation_warning_status value""" - return self.__enable_installation_warning_status + return self.__enable_installation_status_to_warning_flag # endregion diff --git a/src/core/src/service_interfaces/StatusHandler.py b/src/core/src/service_interfaces/StatusHandler.py index 0ee2f2271..e94a9d219 100644 --- a/src/core/src/service_interfaces/StatusHandler.py +++ b/src/core/src/service_interfaces/StatusHandler.py @@ -264,7 +264,7 @@ def get_os_name_and_version(self): return "unknownDist_unknownVer" def get_installation_packages_list(self): - #type (none) -> list + # type (none) -> list """Access installation packages data""" return self.__installation_packages # endregion diff --git a/src/core/tests/Test_CoreMain.py b/src/core/tests/Test_CoreMain.py index 708be943b..4ed854f2b 100644 --- a/src/core/tests/Test_CoreMain.py +++ b/src/core/tests/Test_CoreMain.py @@ -1326,7 +1326,7 @@ def __assertion_pkg_succeed_on_retry(self, runtime): self.__check_telemetry_events(runtime) # If true, set installation status to warning - self.assertTrue(runtime.patch_installer.get_enabled_installation_warning_status()) + 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: From e12f2d4fe69462e7c882114919554139a06edc26 Mon Sep 17 00:00:00 2001 From: john feng Date: Mon, 7 Apr 2025 13:31:30 -0700 Subject: [PATCH 21/38] alter logic to check all packages are installed instead critical/securit --- src/core/src/core_logic/PatchInstaller.py | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/src/core/src/core_logic/PatchInstaller.py b/src/core/src/core_logic/PatchInstaller.py index b710489aa..467ca038a 100644 --- a/src/core/src/core_logic/PatchInstaller.py +++ b/src/core/src/core_logic/PatchInstaller.py @@ -822,12 +822,15 @@ def __log_progress_status(self, maintenance_window, installed_update_count): # region - Failed packages retry succeeded def log_final_installation_metric(self, patch_installation_successful, maintenance_window_exceeded, maintenance_window, installed_update_count): + """ log final installation operation status. """ if self.__enable_installation_status_to_warning_flag: self.log_final_warning_metric(maintenance_window, installed_update_count) else: self.log_final_metrics(maintenance_window, patch_installation_successful, maintenance_window_exceeded, installed_update_count) def __check_installation_status_can_set_to_warning(self, patch_installation_successful, maintenance_window_exceeded): + """ Verify patch installation status can be set to warning from failed. """ + # type (bool, bool) -> bool return not patch_installation_successful and not maintenance_window_exceeded and self.__check_all_requested_packages_install_state() def __check_all_requested_packages_install_state(self): @@ -836,22 +839,13 @@ def __check_all_requested_packages_install_state(self): # Get the list of installed packages installed_packages_list = self.status_handler.get_installation_packages_list() - # Get security and critical packages - security_critical_packages = [] - for package in installed_packages_list: - if 'classifications' in package and any(classification in ['Security', 'Critical'] for classification in package['classifications']): - security_critical_packages.append(package) - - # Return false there's no security/critical packages - if len(security_critical_packages) == 0: - return False - # Check if any security/critical package are not installed - for package in security_critical_packages: + # Check if any package(s) are not installed + for package in installed_packages_list: if package['patchInstallationState'] != Constants.INSTALLED: return False - # All security/critical packages are installed + # All package(s) are installed return True def __send_metadata_to_health_store(self): From 30f867e6802ecd373e33dc39e6a320e3d0dc8985 Mon Sep 17 00:00:00 2001 From: john feng Date: Mon, 7 Apr 2025 13:39:10 -0700 Subject: [PATCH 22/38] refactor extra lines --- src/core/src/core_logic/PatchInstaller.py | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/core/src/core_logic/PatchInstaller.py b/src/core/src/core_logic/PatchInstaller.py index 467ca038a..cd918fde9 100644 --- a/src/core/src/core_logic/PatchInstaller.py +++ b/src/core/src/core_logic/PatchInstaller.py @@ -405,7 +405,6 @@ def install_updates(self, maintenance_window, package_manager, simulate=False): 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. @@ -433,7 +432,6 @@ def log_final_warning_metric(self, maintenance_window, installed_update_count): 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. @@ -835,13 +833,10 @@ def __check_installation_status_can_set_to_warning(self, patch_installation_succ def __check_all_requested_packages_install_state(self): # type (none) -> bool - """ Check if all requested security and critical packages are installed. """ - - # Get the list of installed packages - installed_packages_list = self.status_handler.get_installation_packages_list() + """ Check if any package(s) are all installed. """ # Check if any package(s) are not installed - for package in installed_packages_list: + for package in self.status_handler.get_installation_packages_list(): if package['patchInstallationState'] != Constants.INSTALLED: return False From c57be7754f15594fad2952362ef849d059251dca Mon Sep 17 00:00:00 2001 From: john feng Date: Tue, 15 Apr 2025 15:37:48 -0700 Subject: [PATCH 23/38] refactor __check_installation_status_can_set_to_warning --- src/core/src/core_logic/PatchInstaller.py | 23 +++----------- .../src/service_interfaces/StatusHandler.py | 14 ++++++--- src/core/tests/Test_CoreMain.py | 31 +++++-------------- 3 files changed, 23 insertions(+), 45 deletions(-) diff --git a/src/core/src/core_logic/PatchInstaller.py b/src/core/src/core_logic/PatchInstaller.py index cd918fde9..ef9d27dfa 100644 --- a/src/core/src/core_logic/PatchInstaller.py +++ b/src/core/src/core_logic/PatchInstaller.py @@ -284,9 +284,7 @@ def install_updates(self, maintenance_window, package_manager, simulate=False): successful_parent_package_install_count_in_batch_patching = self.successful_parent_package_install_count if len(packages) == 0: - self.__enable_installation_status_to_warning_flag = self.__check_installation_status_can_set_to_warning(patch_installation_successful, maintenance_window_exceeded) self.log_final_installation_metric(patch_installation_successful, maintenance_window_exceeded, maintenance_window, installed_update_count) - return installed_update_count, patch_installation_successful, maintenance_window_exceeded else: 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), @@ -384,8 +382,7 @@ def install_updates(self, maintenance_window, package_manager, simulate=False): self.composite_logger.log_debug("\nPerforming final system state reconciliation...") installed_update_count += self.perform_status_reconciliation_conditionally(package_manager, True) - - self.__enable_installation_status_to_warning_flag = self.__check_installation_status_can_set_to_warning(patch_installation_successful, maintenance_window_exceeded) + self.log_final_installation_metric(patch_installation_successful, maintenance_window_exceeded, maintenance_window, installed_update_count) install_update_count_in_sequential_patching = installed_update_count - install_update_count_in_batch_patching @@ -821,7 +818,8 @@ def __log_progress_status(self, maintenance_window, installed_update_count): # region - Failed packages retry succeeded def log_final_installation_metric(self, patch_installation_successful, maintenance_window_exceeded, maintenance_window, installed_update_count): """ log final installation operation status. """ - if self.__enable_installation_status_to_warning_flag: + warning_status = self.__check_installation_status_can_set_to_warning(patch_installation_successful, maintenance_window_exceeded) + if warning_status: self.log_final_warning_metric(maintenance_window, installed_update_count) else: self.log_final_metrics(maintenance_window, patch_installation_successful, maintenance_window_exceeded, installed_update_count) @@ -829,19 +827,8 @@ def log_final_installation_metric(self, patch_installation_successful, maintenan def __check_installation_status_can_set_to_warning(self, patch_installation_successful, maintenance_window_exceeded): """ Verify patch installation status can be set to warning from failed. """ # type (bool, bool) -> bool - return not patch_installation_successful and not maintenance_window_exceeded and self.__check_all_requested_packages_install_state() - - def __check_all_requested_packages_install_state(self): - # type (none) -> bool - """ Check if any package(s) are all installed. """ - - # Check if any package(s) are not installed - for package in self.status_handler.get_installation_packages_list(): - if package['patchInstallationState'] != Constants.INSTALLED: - return False - - # All package(s) are installed - return True + self.__enable_installation_status_to_warning_flag = not patch_installation_successful and not maintenance_window_exceeded and self.status_handler.check_all_requested_packages_install_state() + return self.__enable_installation_status_to_warning_flag 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))) diff --git a/src/core/src/service_interfaces/StatusHandler.py b/src/core/src/service_interfaces/StatusHandler.py index e94a9d219..a373dae72 100644 --- a/src/core/src/service_interfaces/StatusHandler.py +++ b/src/core/src/service_interfaces/StatusHandler.py @@ -263,10 +263,16 @@ def get_os_name_and_version(self): self.composite_logger.log_error("Unable to determine platform information: {0}".format(repr(error))) return "unknownDist_unknownVer" - def get_installation_packages_list(self): - # type (none) -> list - """Access installation packages data""" - return self.__installation_packages + def check_all_requested_packages_install_state(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 diff --git a/src/core/tests/Test_CoreMain.py b/src/core/tests/Test_CoreMain.py index 71efad5d0..8b224442b 100644 --- a/src/core/tests/Test_CoreMain.py +++ b/src/core/tests/Test_CoreMain.py @@ -63,25 +63,9 @@ def mock_batch_patching_with_packages(self, all_packages, all_package_versions, 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_installed_pkg_list(self): - """ Mock list of security and critical installed packages. """ - return [ - { - 'patchId': 'kernel-default_4.4.49-92.11.1_Ubuntu_16.04', - 'name': 'kernel-default', - 'version': '4.4.49-92.11.1', - 'classifications': ['Security'], - 'patchInstallationState': 'Installed' - }, - { - 'patchId': 'libgoa-1_0-0_3.20.5-9.6_Ubuntu_16.04', - 'name': 'libgoa-1_0-0', - 'version': '3.20.5-9.6', - 'classifications': ['Security'], - 'patchInstallationState': 'Installed' - } - ] + + 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 @@ -1348,10 +1332,11 @@ def test_warning_status_when_packages_initially_fail_but_succeed_on_retry_no_bat # Store original methods original_batch_patching = runtime.patch_installer.batch_patching - original_installation_pkg_list = runtime.status_handler.get_installation_packages_list - # Mock batch_patching with packages to return false + 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.get_installation_packages_list = self.mock_installed_pkg_list + runtime.status_handler.check_all_requested_packages_install_state = self.mock_check_all_requested_packages_install_state # Run CoreMain to execute the installation try: @@ -1361,7 +1346,7 @@ def test_warning_status_when_packages_initially_fail_but_succeed_on_retry_no_bat finally: # reset mock runtime.patch_installer.batch_patching = original_batch_patching - runtime.status_handler.get_installation_packages_list = original_installation_pkg_list + runtime.status_handler.get_installation_packages_list = original_check_all_requested_packages_install_state runtime.stop() def __check_telemetry_events(self, runtime): From 12a1c2bcea3c4e8491321ab9f6f73dd6f3b3a933 Mon Sep 17 00:00:00 2001 From: john feng Date: Tue, 15 Apr 2025 15:51:19 -0700 Subject: [PATCH 24/38] add extra lines --- src/core/src/core_logic/PatchInstaller.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/core/src/core_logic/PatchInstaller.py b/src/core/src/core_logic/PatchInstaller.py index ef9d27dfa..7b421f49a 100644 --- a/src/core/src/core_logic/PatchInstaller.py +++ b/src/core/src/core_logic/PatchInstaller.py @@ -546,6 +546,7 @@ def install_packages_in_batches(self, all_packages, all_package_versions, packag # These packages will be attempted in sequential installation if there is enough time in maintenance window to install package sequentially. remaining_packages = [] remaining_package_versions = [] + # failed_packages are the packages which are failed to install in batch patching. These packages will be attempted again in sequential patching if there is # enough time remaining in maintenance window. failed_packages = [] From 2de4b077f4100cffbc8ea7986af0df7a7522f5bc Mon Sep 17 00:00:00 2001 From: john feng Date: Thu, 17 Apr 2025 13:28:33 -0700 Subject: [PATCH 25/38] refactor __send_metadata_to_health_store --- src/core/src/core_logic/PatchInstaller.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/core/src/core_logic/PatchInstaller.py b/src/core/src/core_logic/PatchInstaller.py index 7b421f49a..8dc9932a8 100644 --- a/src/core/src/core_logic/PatchInstaller.py +++ b/src/core/src/core_logic/PatchInstaller.py @@ -815,6 +815,14 @@ 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) # region - Failed packages retry succeeded def log_final_installation_metric(self, patch_installation_successful, maintenance_window_exceeded, maintenance_window, installed_update_count): @@ -831,14 +839,6 @@ def __check_installation_status_can_set_to_warning(self, patch_installation_succ self.__enable_installation_status_to_warning_flag = not patch_installation_successful and not maintenance_window_exceeded and self.status_handler.check_all_requested_packages_install_state() return self.__enable_installation_status_to_warning_flag - 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 set_patch_installation_status_to_warning_from_failed(self): """Access enable_installation_warning_status value""" return self.__enable_installation_status_to_warning_flag From f9ae07e6bd1df3701e6c0b6f6b0d54ee394b0cc9 Mon Sep 17 00:00:00 2001 From: Rajasi Rane Date: Fri, 18 Apr 2025 16:15:39 -0700 Subject: [PATCH 26/38] PR feedback explanation --- src/core/src/CoreMain.py | 4 +- src/core/src/core_logic/PatchInstaller.py | 43 +++++++------------ .../src/service_interfaces/StatusHandler.py | 2 +- src/core/tests/Test_PatchInstaller.py | 12 ++++-- 4 files changed, 26 insertions(+), 35 deletions(-) diff --git a/src/core/src/CoreMain.py b/src/core/src/CoreMain.py index a52fecb41..61de40415 100644 --- a/src/core/src/CoreMain.py +++ b/src/core/src/CoreMain.py @@ -93,7 +93,7 @@ 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() @@ -101,7 +101,7 @@ def __init__(self, argv): if patch_assessment_successful and patch_installation_successful: patch_installer.mark_installation_completed() overall_patch_installation_operation_successful = True - elif patch_installer.set_patch_installation_status_to_warning_from_failed(): + 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) diff --git a/src/core/src/core_logic/PatchInstaller.py b/src/core/src/core_logic/PatchInstaller.py index 8dc9932a8..3910a4aeb 100644 --- a/src/core/src/core_logic/PatchInstaller.py +++ b/src/core/src/core_logic/PatchInstaller.py @@ -53,8 +53,6 @@ def __init__(self, env_layer, execution_config, composite_logger, telemetry_writ self.stopwatch = Stopwatch(self.env_layer, self.telemetry_writer, self.composite_logger) - self.__enable_installation_status_to_warning_flag = False - def start_installation(self, simulate=False): """ Kick off a patch installation run """ self.status_handler.set_current_operation(Constants.INSTALLATION) @@ -126,7 +124,7 @@ def start_installation(self, simulate=False): 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 @@ -284,7 +282,7 @@ def install_updates(self, maintenance_window, package_manager, simulate=False): successful_parent_package_install_count_in_batch_patching = self.successful_parent_package_install_count if len(packages) == 0: - self.log_final_installation_metric(patch_installation_successful, maintenance_window_exceeded, maintenance_window, installed_update_count) + self.log_final_metrics(maintenance_window, patch_installation_successful, maintenance_window_exceeded, installed_update_count) return installed_update_count, patch_installation_successful, maintenance_window_exceeded else: 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), @@ -383,7 +381,7 @@ def install_updates(self, maintenance_window, package_manager, simulate=False): self.composite_logger.log_debug("\nPerforming final system state reconciliation...") installed_update_count += self.perform_status_reconciliation_conditionally(package_manager, True) - self.log_final_installation_metric(patch_installation_successful, maintenance_window_exceeded, maintenance_window, installed_update_count) + 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 attempted_parent_package_install_count_in_sequential_patching = self.attempted_parent_package_install_count - attempted_parent_package_install_count_in_batch_patching @@ -420,7 +418,6 @@ def log_final_metrics(self, maintenance_window, patch_installation_successful, m 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) @@ -692,9 +689,12 @@ def mark_installation_completed(self): 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 to a restriction in CRP, where installation substatus should be marked as warning only after the implicit (2nd) assessment operation - and all supposed packages are installed as expected. """ + 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 @@ -812,7 +812,11 @@ def get_max_batch_size(self, maintenance_window, package_manager): 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), + 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) @@ -824,22 +828,5 @@ def __send_metadata_to_health_store(self): report_to_healthstore=True, wait_after_update=False) - # region - Failed packages retry succeeded - def log_final_installation_metric(self, patch_installation_successful, maintenance_window_exceeded, maintenance_window, installed_update_count): - """ log final installation operation status. """ - warning_status = self.__check_installation_status_can_set_to_warning(patch_installation_successful, maintenance_window_exceeded) - if warning_status: - self.log_final_warning_metric(maintenance_window, installed_update_count) - else: - self.log_final_metrics(maintenance_window, patch_installation_successful, maintenance_window_exceeded, installed_update_count) - - def __check_installation_status_can_set_to_warning(self, patch_installation_successful, maintenance_window_exceeded): - """ Verify patch installation status can be set to warning from failed. """ - # type (bool, bool) -> bool - self.__enable_installation_status_to_warning_flag = not patch_installation_successful and not maintenance_window_exceeded and self.status_handler.check_all_requested_packages_install_state() - return self.__enable_installation_status_to_warning_flag - - def set_patch_installation_status_to_warning_from_failed(self): - """Access enable_installation_warning_status value""" - return self.__enable_installation_status_to_warning_flag - # endregion + 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() diff --git a/src/core/src/service_interfaces/StatusHandler.py b/src/core/src/service_interfaces/StatusHandler.py index a373dae72..a38d1acb7 100644 --- a/src/core/src/service_interfaces/StatusHandler.py +++ b/src/core/src/service_interfaces/StatusHandler.py @@ -263,7 +263,7 @@ def get_os_name_and_version(self): self.composite_logger.log_error("Unable to determine platform information: {0}".format(repr(error))) return "unknownDist_unknownVer" - def check_all_requested_packages_install_state(self): + def are_all_requested_packgaes_installed(self): # type (none) -> bool """ Check if all requested package(s) are installed. """ diff --git a/src/core/tests/Test_PatchInstaller.py b/src/core/tests/Test_PatchInstaller.py index c6282ad89..b431872c2 100644 --- a/src/core/tests/Test_PatchInstaller.py +++ b/src/core/tests/Test_PatchInstaller.py @@ -241,7 +241,8 @@ def test_patch_installer_for_azgps_coordinated(self): argument_composer.maximum_duration = "PT30M" runtime = RuntimeCompositor(argument_composer.get_composed_arguments(), True, Constants.APT) runtime.set_legacy_test_type('HappyPath') - self.assertFalse(runtime.patch_installer.start_installation()) # failure is in unrelated patch installation batch processing + patch_installation_successful, maintenance_window_exceeded = runtime.patch_installer.start_installation() + self.assertFalse(patch_installation_successful) # failure is in unrelated patch installation batch processing self.assertEqual(runtime.execution_config.max_patch_publish_date, "20240401T000000Z") self.assertEqual(runtime.package_manager.max_patch_publish_date, "") # reason: not enough time to use @@ -253,21 +254,24 @@ def test_patch_installer_for_azgps_coordinated(self): runtime = RuntimeCompositor(argument_composer.get_composed_arguments(), True, Constants.APT) runtime.set_legacy_test_type('HappyPath') runtime.package_manager.install_security_updates_azgps_coordinated = lambda: (1, "Failed") - self.assertFalse(runtime.patch_installer.start_installation()) + patch_installation_successful, maintenance_window_exceeded = runtime.patch_installer.start_installation() + self.assertFalse(patch_installation_successful) self.assertEqual(runtime.execution_config.max_patch_publish_date, "20240401T000000Z") self.assertEqual(runtime.package_manager.max_patch_publish_date, "") # reason: the strict SDP is forced to fail with the lambda above runtime.stop() runtime = RuntimeCompositor(argument_composer.get_composed_arguments(), True, Constants.YUM) runtime.set_legacy_test_type('HappyPath') - self.assertTrue(runtime.patch_installer.start_installation()) + patch_installation_successful, maintenance_window_exceeded = runtime.patch_installer.start_installation() + self.assertTrue(patch_installation_successful) self.assertEqual(runtime.execution_config.max_patch_publish_date, "20240401T000000Z") self.assertEqual(runtime.package_manager.max_patch_publish_date, "") # unsupported in Yum runtime.stop() runtime = RuntimeCompositor(argument_composer.get_composed_arguments(), True, Constants.ZYPPER) runtime.set_legacy_test_type('HappyPath') - self.assertFalse(runtime.patch_installer.start_installation()) # failure is in unrelated patch installation batch processing + patch_installation_successful, maintenance_window_exceeded = runtime.patch_installer.start_installation() + self.assertFalse(patch_installation_successful) # failure is in unrelated patch installation batch processing self.assertEqual(runtime.execution_config.max_patch_publish_date, "20240401T000000Z") self.assertEqual(runtime.package_manager.max_patch_publish_date, "") # unsupported in Zypper runtime.stop() From 48453dac606ab8b003eef954b7ff5e0a1b29eec4 Mon Sep 17 00:00:00 2001 From: Rajasi Rane Date: Fri, 18 Apr 2025 16:33:33 -0700 Subject: [PATCH 27/38] Revert "PR feedback explanation" This reverts commit f9ae07e6bd1df3701e6c0b6f6b0d54ee394b0cc9. --- src/core/src/CoreMain.py | 4 +- src/core/src/core_logic/PatchInstaller.py | 43 ++++++++++++------- .../src/service_interfaces/StatusHandler.py | 2 +- src/core/tests/Test_PatchInstaller.py | 12 ++---- 4 files changed, 35 insertions(+), 26 deletions(-) diff --git a/src/core/src/CoreMain.py b/src/core/src/CoreMain.py index 61de40415..a52fecb41 100644 --- a/src/core/src/CoreMain.py +++ b/src/core/src/CoreMain.py @@ -93,7 +93,7 @@ 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, maintenance_window_exceeded = patch_installer.start_installation() + patch_installation_successful = patch_installer.start_installation() patch_assessment_successful = False patch_assessment_successful = patch_assessor.start_assessment() @@ -101,7 +101,7 @@ def __init__(self, argv): 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): + elif patch_installer.set_patch_installation_status_to_warning_from_failed(): 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) diff --git a/src/core/src/core_logic/PatchInstaller.py b/src/core/src/core_logic/PatchInstaller.py index 3910a4aeb..8dc9932a8 100644 --- a/src/core/src/core_logic/PatchInstaller.py +++ b/src/core/src/core_logic/PatchInstaller.py @@ -53,6 +53,8 @@ def __init__(self, env_layer, execution_config, composite_logger, telemetry_writ self.stopwatch = Stopwatch(self.env_layer, self.telemetry_writer, self.composite_logger) + self.__enable_installation_status_to_warning_flag = False + def start_installation(self, simulate=False): """ Kick off a patch installation run """ self.status_handler.set_current_operation(Constants.INSTALLATION) @@ -124,7 +126,7 @@ def start_installation(self, simulate=False): 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, maintenance_window_exceeded + return overall_patch_installation_successful 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 @@ -282,7 +284,7 @@ def install_updates(self, maintenance_window, package_manager, simulate=False): successful_parent_package_install_count_in_batch_patching = self.successful_parent_package_install_count if len(packages) == 0: - self.log_final_metrics(maintenance_window, patch_installation_successful, maintenance_window_exceeded, installed_update_count) + self.log_final_installation_metric(patch_installation_successful, maintenance_window_exceeded, maintenance_window, installed_update_count) return installed_update_count, patch_installation_successful, maintenance_window_exceeded else: 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), @@ -381,7 +383,7 @@ def install_updates(self, maintenance_window, package_manager, simulate=False): 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) + self.log_final_installation_metric(patch_installation_successful, maintenance_window_exceeded, maintenance_window, installed_update_count) install_update_count_in_sequential_patching = installed_update_count - install_update_count_in_batch_patching attempted_parent_package_install_count_in_sequential_patching = self.attempted_parent_package_install_count - attempted_parent_package_install_count_in_batch_patching @@ -418,6 +420,7 @@ def log_final_metrics(self, maintenance_window, patch_installation_successful, m 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) @@ -689,12 +692,9 @@ def mark_installation_completed(self): 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. """ + This is set outside of start_installation function to a restriction in CRP, where installation substatus should be marked as warning only after the implicit (2nd) assessment operation + and all supposed packages are installed as expected. """ - 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 @@ -812,11 +812,7 @@ def get_max_batch_size(self, maintenance_window, package_manager): 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), + 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) @@ -828,5 +824,22 @@ def __send_metadata_to_health_store(self): 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() + # region - Failed packages retry succeeded + def log_final_installation_metric(self, patch_installation_successful, maintenance_window_exceeded, maintenance_window, installed_update_count): + """ log final installation operation status. """ + warning_status = self.__check_installation_status_can_set_to_warning(patch_installation_successful, maintenance_window_exceeded) + if warning_status: + self.log_final_warning_metric(maintenance_window, installed_update_count) + else: + self.log_final_metrics(maintenance_window, patch_installation_successful, maintenance_window_exceeded, installed_update_count) + + def __check_installation_status_can_set_to_warning(self, patch_installation_successful, maintenance_window_exceeded): + """ Verify patch installation status can be set to warning from failed. """ + # type (bool, bool) -> bool + self.__enable_installation_status_to_warning_flag = not patch_installation_successful and not maintenance_window_exceeded and self.status_handler.check_all_requested_packages_install_state() + return self.__enable_installation_status_to_warning_flag + + def set_patch_installation_status_to_warning_from_failed(self): + """Access enable_installation_warning_status value""" + return self.__enable_installation_status_to_warning_flag + # endregion diff --git a/src/core/src/service_interfaces/StatusHandler.py b/src/core/src/service_interfaces/StatusHandler.py index a38d1acb7..a373dae72 100644 --- a/src/core/src/service_interfaces/StatusHandler.py +++ b/src/core/src/service_interfaces/StatusHandler.py @@ -263,7 +263,7 @@ def get_os_name_and_version(self): self.composite_logger.log_error("Unable to determine platform information: {0}".format(repr(error))) return "unknownDist_unknownVer" - def are_all_requested_packgaes_installed(self): + def check_all_requested_packages_install_state(self): # type (none) -> bool """ Check if all requested package(s) are installed. """ diff --git a/src/core/tests/Test_PatchInstaller.py b/src/core/tests/Test_PatchInstaller.py index b431872c2..c6282ad89 100644 --- a/src/core/tests/Test_PatchInstaller.py +++ b/src/core/tests/Test_PatchInstaller.py @@ -241,8 +241,7 @@ def test_patch_installer_for_azgps_coordinated(self): argument_composer.maximum_duration = "PT30M" runtime = RuntimeCompositor(argument_composer.get_composed_arguments(), True, Constants.APT) runtime.set_legacy_test_type('HappyPath') - patch_installation_successful, maintenance_window_exceeded = runtime.patch_installer.start_installation() - self.assertFalse(patch_installation_successful) # failure is in unrelated patch installation batch processing + self.assertFalse(runtime.patch_installer.start_installation()) # failure is in unrelated patch installation batch processing self.assertEqual(runtime.execution_config.max_patch_publish_date, "20240401T000000Z") self.assertEqual(runtime.package_manager.max_patch_publish_date, "") # reason: not enough time to use @@ -254,24 +253,21 @@ def test_patch_installer_for_azgps_coordinated(self): runtime = RuntimeCompositor(argument_composer.get_composed_arguments(), True, Constants.APT) runtime.set_legacy_test_type('HappyPath') runtime.package_manager.install_security_updates_azgps_coordinated = lambda: (1, "Failed") - patch_installation_successful, maintenance_window_exceeded = runtime.patch_installer.start_installation() - self.assertFalse(patch_installation_successful) + self.assertFalse(runtime.patch_installer.start_installation()) self.assertEqual(runtime.execution_config.max_patch_publish_date, "20240401T000000Z") self.assertEqual(runtime.package_manager.max_patch_publish_date, "") # reason: the strict SDP is forced to fail with the lambda above runtime.stop() runtime = RuntimeCompositor(argument_composer.get_composed_arguments(), True, Constants.YUM) runtime.set_legacy_test_type('HappyPath') - patch_installation_successful, maintenance_window_exceeded = runtime.patch_installer.start_installation() - self.assertTrue(patch_installation_successful) + self.assertTrue(runtime.patch_installer.start_installation()) self.assertEqual(runtime.execution_config.max_patch_publish_date, "20240401T000000Z") self.assertEqual(runtime.package_manager.max_patch_publish_date, "") # unsupported in Yum runtime.stop() runtime = RuntimeCompositor(argument_composer.get_composed_arguments(), True, Constants.ZYPPER) runtime.set_legacy_test_type('HappyPath') - patch_installation_successful, maintenance_window_exceeded = runtime.patch_installer.start_installation() - self.assertFalse(patch_installation_successful) # failure is in unrelated patch installation batch processing + self.assertFalse(runtime.patch_installer.start_installation()) # failure is in unrelated patch installation batch processing self.assertEqual(runtime.execution_config.max_patch_publish_date, "20240401T000000Z") self.assertEqual(runtime.package_manager.max_patch_publish_date, "") # unsupported in Zypper runtime.stop() From 8aaecfa75daabfd0e7cbf2007b3f1a1603ac23ed Mon Sep 17 00:00:00 2001 From: john feng Date: Mon, 21 Apr 2025 12:15:03 -0700 Subject: [PATCH 28/38] refactor are_all_requested_packages_installed --- src/core/src/core_logic/PatchInstaller.py | 2 +- src/core/src/service_interfaces/StatusHandler.py | 2 +- src/core/tests/Test_CoreMain.py | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/core/src/core_logic/PatchInstaller.py b/src/core/src/core_logic/PatchInstaller.py index 8dc9932a8..46d8cb6a0 100644 --- a/src/core/src/core_logic/PatchInstaller.py +++ b/src/core/src/core_logic/PatchInstaller.py @@ -836,7 +836,7 @@ def log_final_installation_metric(self, patch_installation_successful, maintenan def __check_installation_status_can_set_to_warning(self, patch_installation_successful, maintenance_window_exceeded): """ Verify patch installation status can be set to warning from failed. """ # type (bool, bool) -> bool - self.__enable_installation_status_to_warning_flag = not patch_installation_successful and not maintenance_window_exceeded and self.status_handler.check_all_requested_packages_install_state() + self.__enable_installation_status_to_warning_flag = not patch_installation_successful and not maintenance_window_exceeded and self.status_handler.are_all_requested_packages_installed() return self.__enable_installation_status_to_warning_flag def set_patch_installation_status_to_warning_from_failed(self): diff --git a/src/core/src/service_interfaces/StatusHandler.py b/src/core/src/service_interfaces/StatusHandler.py index a373dae72..a78bab542 100644 --- a/src/core/src/service_interfaces/StatusHandler.py +++ b/src/core/src/service_interfaces/StatusHandler.py @@ -263,7 +263,7 @@ def get_os_name_and_version(self): self.composite_logger.log_error("Unable to determine platform information: {0}".format(repr(error))) return "unknownDist_unknownVer" - def check_all_requested_packages_install_state(self): + def are_all_requested_packages_installed(self): # type (none) -> bool """ Check if all requested package(s) are installed. """ diff --git a/src/core/tests/Test_CoreMain.py b/src/core/tests/Test_CoreMain.py index 8b224442b..568d6c5d4 100644 --- a/src/core/tests/Test_CoreMain.py +++ b/src/core/tests/Test_CoreMain.py @@ -1332,11 +1332,11 @@ def test_warning_status_when_packages_initially_fail_but_succeed_on_retry_no_bat # 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 + original_check_all_requested_packages_install_state= runtime.status_handler.are_all_requested_packages_installed # 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 + runtime.status_handler.are_all_requested_packages_installed = self.mock_check_all_requested_packages_install_state # Run CoreMain to execute the installation try: From ecdfccdc7a07576ce29ef779f9bc272a51b24882 Mon Sep 17 00:00:00 2001 From: john feng Date: Mon, 21 Apr 2025 14:19:18 -0700 Subject: [PATCH 29/38] refactor code --- src/core/src/core_logic/PatchInstaller.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/core/src/core_logic/PatchInstaller.py b/src/core/src/core_logic/PatchInstaller.py index 46d8cb6a0..8b595af81 100644 --- a/src/core/src/core_logic/PatchInstaller.py +++ b/src/core/src/core_logic/PatchInstaller.py @@ -693,7 +693,7 @@ def mark_installation_completed(self): 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 to a restriction in CRP, where installation substatus should be marked as warning only after the implicit (2nd) assessment operation - and all supposed packages are installed as expected. """ + and all expected packages are installed as expected. """ self.status_handler.set_installation_substatus_json(status=Constants.STATUS_WARNING) @@ -817,6 +817,7 @@ def __log_progress_status(self, maintenance_window, installed_update_count): self.composite_logger.log(progress_status) def __send_metadata_to_health_store(self): + """ store patch metadata in status files for health store. """ 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( From 928ab88b11339114f4feb2b1605bfb17a733f9ab Mon Sep 17 00:00:00 2001 From: john feng Date: Mon, 21 Apr 2025 14:29:55 -0700 Subject: [PATCH 30/38] update new UTs --- src/core/tests/Test_CoreMain.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/core/tests/Test_CoreMain.py b/src/core/tests/Test_CoreMain.py index 568d6c5d4..f6ef95af3 100644 --- a/src/core/tests/Test_CoreMain.py +++ b/src/core/tests/Test_CoreMain.py @@ -64,7 +64,7 @@ def mock_batch_patching_with_no_packages(self, all_packages, all_package_version """Mock batch_patching to simulate package installation failure, return no packages""" return [], [], 0, False - def mock_check_all_requested_packages_install_state(self): + def mock_are_all_requested_packages_installed(self): return True def test_operation_fail_for_non_autopatching_request(self): @@ -1332,11 +1332,11 @@ def test_warning_status_when_packages_initially_fail_but_succeed_on_retry_no_bat # Store original methods original_batch_patching = runtime.patch_installer.batch_patching - original_check_all_requested_packages_install_state= runtime.status_handler.are_all_requested_packages_installed + original_are_all_requested_packages_installed = runtime.status_handler.are_all_requested_packages_installed # Mock batch_patching with packages to return [], [], false runtime.patch_installer.batch_patching = self.mock_batch_patching_with_no_packages - runtime.status_handler.are_all_requested_packages_installed = self.mock_check_all_requested_packages_install_state + runtime.status_handler.are_all_requested_packages_installed = self.mock_are_all_requested_packages_installed # Run CoreMain to execute the installation try: @@ -1346,7 +1346,7 @@ def test_warning_status_when_packages_initially_fail_but_succeed_on_retry_no_bat 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.status_handler.are_all_requested_packages_installed = original_are_all_requested_packages_installed runtime.stop() def __check_telemetry_events(self, runtime): From 48826cc48f9337cafe35bcef2958494e45c97c5b Mon Sep 17 00:00:00 2001 From: john feng Date: Wed, 23 Apr 2025 15:20:21 -0700 Subject: [PATCH 31/38] add pr 314 changes --- src/core/src/CoreMain.py | 4 +- src/core/src/core_logic/PatchInstaller.py | 45 +++++++++-------------- src/core/tests/Test_CoreMain.py | 3 -- src/core/tests/Test_PatchInstaller.py | 15 +++++--- 4 files changed, 29 insertions(+), 38 deletions(-) diff --git a/src/core/src/CoreMain.py b/src/core/src/CoreMain.py index a52fecb41..61de40415 100644 --- a/src/core/src/CoreMain.py +++ b/src/core/src/CoreMain.py @@ -93,7 +93,7 @@ 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() @@ -101,7 +101,7 @@ def __init__(self, argv): if patch_assessment_successful and patch_installation_successful: patch_installer.mark_installation_completed() overall_patch_installation_operation_successful = True - elif patch_installer.set_patch_installation_status_to_warning_from_failed(): + 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) diff --git a/src/core/src/core_logic/PatchInstaller.py b/src/core/src/core_logic/PatchInstaller.py index 8b595af81..30812b3b7 100644 --- a/src/core/src/core_logic/PatchInstaller.py +++ b/src/core/src/core_logic/PatchInstaller.py @@ -126,7 +126,7 @@ def start_installation(self, simulate=False): 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 @@ -284,7 +284,7 @@ def install_updates(self, maintenance_window, package_manager, simulate=False): successful_parent_package_install_count_in_batch_patching = self.successful_parent_package_install_count if len(packages) == 0: - self.log_final_installation_metric(patch_installation_successful, maintenance_window_exceeded, maintenance_window, installed_update_count) + self.log_final_metrics(maintenance_window, patch_installation_successful, maintenance_window_exceeded, installed_update_count) return installed_update_count, patch_installation_successful, maintenance_window_exceeded else: 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), @@ -383,7 +383,7 @@ def install_updates(self, maintenance_window, package_manager, simulate=False): self.composite_logger.log_debug("\nPerforming final system state reconciliation...") installed_update_count += self.perform_status_reconciliation_conditionally(package_manager, True) - self.log_final_installation_metric(patch_installation_successful, maintenance_window_exceeded, maintenance_window, installed_update_count) + 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 attempted_parent_package_install_count_in_sequential_patching = self.attempted_parent_package_install_count - attempted_parent_package_install_count_in_batch_patching @@ -418,14 +418,6 @@ def log_final_metrics(self, maintenance_window, patch_installation_successful, m 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. @@ -693,8 +685,11 @@ def mark_installation_completed(self): 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 to a restriction in CRP, where installation substatus should be marked as warning only after the implicit (2nd) assessment operation - and all expected packages are installed as expected. """ - + and all customer requested packages are installed. """ + + # Update status handler error msg + self.__log_final_warning_metric() + 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 @@ -826,21 +821,15 @@ def __send_metadata_to_health_store(self): wait_after_update=False) # region - Failed packages retry succeeded - def log_final_installation_metric(self, patch_installation_successful, maintenance_window_exceeded, maintenance_window, installed_update_count): - """ log final installation operation status. """ - warning_status = self.__check_installation_status_can_set_to_warning(patch_installation_successful, maintenance_window_exceeded) - if warning_status: - self.log_final_warning_metric(maintenance_window, installed_update_count) - else: - self.log_final_metrics(maintenance_window, patch_installation_successful, maintenance_window_exceeded, installed_update_count) + def __log_final_warning_metric(self): + """ Log the final metrics for status handler error json for setting installation status to warning. """ - def __check_installation_status_can_set_to_warning(self, patch_installation_successful, maintenance_window_exceeded): - """ Verify patch installation status can be set to warning from failed. """ + message = "All requested package(s) are installed. Any patch errors marked are from previous attempts." + self.composite_logger.log(message) + self.status_handler.add_error_to_status(message=message, error_code=Constants.PatchOperationErrorCodes.PACKAGES_RETRY_SUCCEEDED, current_operation_override_for_error=Constants.INSTALLATION) + + def should_patch_installation_status_be_set_to_warning(self, patch_installation_successful, maintenance_window_exceeded): + """ Check if patch installation status can be set to warning from failed. """ # type (bool, bool) -> bool - self.__enable_installation_status_to_warning_flag = not patch_installation_successful and not maintenance_window_exceeded and self.status_handler.are_all_requested_packages_installed() - return self.__enable_installation_status_to_warning_flag - - def set_patch_installation_status_to_warning_from_failed(self): - """Access enable_installation_warning_status value""" - return self.__enable_installation_status_to_warning_flag + return not patch_installation_successful and not maintenance_window_exceeded and self.status_handler.are_all_requested_packages_installed() # endregion diff --git a/src/core/tests/Test_CoreMain.py b/src/core/tests/Test_CoreMain.py index f6ef95af3..4d37238f5 100644 --- a/src/core/tests/Test_CoreMain.py +++ b/src/core/tests/Test_CoreMain.py @@ -1363,9 +1363,6 @@ 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"] diff --git a/src/core/tests/Test_PatchInstaller.py b/src/core/tests/Test_PatchInstaller.py index c6282ad89..b23ecc719 100644 --- a/src/core/tests/Test_PatchInstaller.py +++ b/src/core/tests/Test_PatchInstaller.py @@ -233,7 +233,8 @@ def test_patch_installer_for_azgps_coordinated(self): runtime.package_manager.current_source_list = os.path.join(argument_composer.temp_folder, "temp2.list") # Path change runtime.set_legacy_test_type('HappyPath') - self.assertTrue(runtime.patch_installer.start_installation()) + patch_installation_successful, maintenance_window_exceeded = runtime.patch_installer.start_installation() + self.assertTrue(patch_installation_successful) self.assertEqual(runtime.execution_config.max_patch_publish_date, "20240401T000000Z") self.assertEqual(runtime.package_manager.max_patch_publish_date,"20240401T000000Z") # supported and conditions met runtime.stop() @@ -241,7 +242,8 @@ def test_patch_installer_for_azgps_coordinated(self): argument_composer.maximum_duration = "PT30M" runtime = RuntimeCompositor(argument_composer.get_composed_arguments(), True, Constants.APT) runtime.set_legacy_test_type('HappyPath') - self.assertFalse(runtime.patch_installer.start_installation()) # failure is in unrelated patch installation batch processing + patch_installation_successful, maintenance_window_exceeded = runtime.patch_installer.start_installation() + self.assertFalse(patch_installation_successful) # failure is in unrelated patch installation batch processing self.assertEqual(runtime.execution_config.max_patch_publish_date, "20240401T000000Z") self.assertEqual(runtime.package_manager.max_patch_publish_date, "") # reason: not enough time to use @@ -253,21 +255,24 @@ def test_patch_installer_for_azgps_coordinated(self): runtime = RuntimeCompositor(argument_composer.get_composed_arguments(), True, Constants.APT) runtime.set_legacy_test_type('HappyPath') runtime.package_manager.install_security_updates_azgps_coordinated = lambda: (1, "Failed") - self.assertFalse(runtime.patch_installer.start_installation()) + patch_installation_successful, maintenance_window_exceeded = runtime.patch_installer.start_installation() + self.assertFalse(patch_installation_successful) self.assertEqual(runtime.execution_config.max_patch_publish_date, "20240401T000000Z") self.assertEqual(runtime.package_manager.max_patch_publish_date, "") # reason: the strict SDP is forced to fail with the lambda above runtime.stop() runtime = RuntimeCompositor(argument_composer.get_composed_arguments(), True, Constants.YUM) runtime.set_legacy_test_type('HappyPath') - self.assertTrue(runtime.patch_installer.start_installation()) + patch_installation_successful, maintenance_window_exceeded = runtime.patch_installer.start_installation() + self.assertTrue(patch_installation_successful) self.assertEqual(runtime.execution_config.max_patch_publish_date, "20240401T000000Z") self.assertEqual(runtime.package_manager.max_patch_publish_date, "") # unsupported in Yum runtime.stop() runtime = RuntimeCompositor(argument_composer.get_composed_arguments(), True, Constants.ZYPPER) runtime.set_legacy_test_type('HappyPath') - self.assertFalse(runtime.patch_installer.start_installation()) # failure is in unrelated patch installation batch processing + patch_installation_successful, maintenance_window_exceeded = runtime.patch_installer.start_installation() + self.assertFalse(patch_installation_successful) # failure is in unrelated patch installation batch processing self.assertEqual(runtime.execution_config.max_patch_publish_date, "20240401T000000Z") self.assertEqual(runtime.package_manager.max_patch_publish_date, "") # unsupported in Zypper runtime.stop() From 9d1042823e8158cd996526e22e41eb405fee1eab Mon Sep 17 00:00:00 2001 From: john feng Date: Wed, 23 Apr 2025 15:37:58 -0700 Subject: [PATCH 32/38] remove unused variable --- src/core/src/core_logic/PatchInstaller.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/core/src/core_logic/PatchInstaller.py b/src/core/src/core_logic/PatchInstaller.py index 30812b3b7..d2f38e8f6 100644 --- a/src/core/src/core_logic/PatchInstaller.py +++ b/src/core/src/core_logic/PatchInstaller.py @@ -53,7 +53,6 @@ def __init__(self, env_layer, execution_config, composite_logger, telemetry_writ self.stopwatch = Stopwatch(self.env_layer, self.telemetry_writer, self.composite_logger) - self.__enable_installation_status_to_warning_flag = False def start_installation(self, simulate=False): """ Kick off a patch installation run """ From 00e9cbf9ea649f722f8f49fe05e345a8b4119ed8 Mon Sep 17 00:00:00 2001 From: john feng Date: Tue, 29 Apr 2025 14:29:44 -0700 Subject: [PATCH 33/38] add condition to prevent operation override --- src/core/src/core_logic/PatchAssessor.py | 4 +++- src/core/src/core_logic/PatchInstaller.py | 5 ++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/core/src/core_logic/PatchAssessor.py b/src/core/src/core_logic/PatchAssessor.py index d82203334..e2b864a9e 100644 --- a/src/core/src/core_logic/PatchAssessor.py +++ b/src/core/src/core_logic/PatchAssessor.py @@ -42,7 +42,9 @@ def __init__(self, env_layer, execution_config, composite_logger, telemetry_writ def start_assessment(self): """ Start a patch assessment """ - self.status_handler.set_current_operation(Constants.ASSESSMENT) + # Prevent operation override during installation operation + if self.status_handler.get_current_operation() != Constants.INSTALLATION: + self.status_handler.set_current_operation(Constants.ASSESSMENT) self.raise_if_telemetry_unsupported() self.raise_if_min_python_version_not_met() diff --git a/src/core/src/core_logic/PatchInstaller.py b/src/core/src/core_logic/PatchInstaller.py index d2f38e8f6..40126190a 100644 --- a/src/core/src/core_logic/PatchInstaller.py +++ b/src/core/src/core_logic/PatchInstaller.py @@ -53,7 +53,6 @@ def __init__(self, env_layer, execution_config, composite_logger, telemetry_writ self.stopwatch = Stopwatch(self.env_layer, self.telemetry_writer, self.composite_logger) - def start_installation(self, simulate=False): """ Kick off a patch installation run """ self.status_handler.set_current_operation(Constants.INSTALLATION) @@ -326,7 +325,6 @@ def install_updates(self, maintenance_window, package_manager, simulate=False): # 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 @@ -825,10 +823,11 @@ def __log_final_warning_metric(self): message = "All requested package(s) are installed. Any patch errors marked are from previous attempts." self.composite_logger.log(message) - self.status_handler.add_error_to_status(message=message, error_code=Constants.PatchOperationErrorCodes.PACKAGES_RETRY_SUCCEEDED, current_operation_override_for_error=Constants.INSTALLATION) + self.status_handler.add_error_to_status(message=message, error_code=Constants.PatchOperationErrorCodes.PACKAGES_RETRY_SUCCEEDED) def should_patch_installation_status_be_set_to_warning(self, patch_installation_successful, maintenance_window_exceeded): """ Check if patch installation status can be set to warning from failed. """ # type (bool, bool) -> bool return not patch_installation_successful and not maintenance_window_exceeded and self.status_handler.are_all_requested_packages_installed() # endregion + From 430d8c8885a6de664a3c2e64c3795da9e48c7839 Mon Sep 17 00:00:00 2001 From: john feng Date: Tue, 29 Apr 2025 14:45:09 -0700 Subject: [PATCH 34/38] undo line breaks --- src/core/src/core_logic/PatchInstaller.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/core/src/core_logic/PatchInstaller.py b/src/core/src/core_logic/PatchInstaller.py index 11a1ad759..6474d18dc 100644 --- a/src/core/src/core_logic/PatchInstaller.py +++ b/src/core/src/core_logic/PatchInstaller.py @@ -325,8 +325,8 @@ def install_updates(self, maintenance_window, package_manager, simulate=False): # 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) + 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 install_result = package_manager.install_update_and_dependencies_and_get_status(package_and_dependencies, package_and_dependency_versions, simulate) @@ -379,7 +379,6 @@ def install_updates(self, maintenance_window, package_manager, simulate=False): 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 From 51ffaa1710cc1fdeb684ab66f1b20908a44d4581 Mon Sep 17 00:00:00 2001 From: john feng Date: Tue, 29 Apr 2025 14:53:37 -0700 Subject: [PATCH 35/38] add extra line --- src/core/src/core_logic/PatchInstaller.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/core/src/core_logic/PatchInstaller.py b/src/core/src/core_logic/PatchInstaller.py index 6474d18dc..28c09bb30 100644 --- a/src/core/src/core_logic/PatchInstaller.py +++ b/src/core/src/core_logic/PatchInstaller.py @@ -327,6 +327,7 @@ def install_updates(self, maintenance_window, package_manager, simulate=False): 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 install_result = package_manager.install_update_and_dependencies_and_get_status(package_and_dependencies, package_and_dependency_versions, simulate) From 2611f989f6b5bbad689644f04b2eed6b3e93879a Mon Sep 17 00:00:00 2001 From: john feng Date: Tue, 29 Apr 2025 14:59:50 -0700 Subject: [PATCH 36/38] remove line break --- src/core/src/core_logic/PatchInstaller.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/src/core_logic/PatchInstaller.py b/src/core/src/core_logic/PatchInstaller.py index 28c09bb30..ec2569d62 100644 --- a/src/core/src/core_logic/PatchInstaller.py +++ b/src/core/src/core_logic/PatchInstaller.py @@ -325,9 +325,9 @@ def install_updates(self, maintenance_window, package_manager, simulate=False): # 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) + 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 install_result = package_manager.install_update_and_dependencies_and_get_status(package_and_dependencies, package_and_dependency_versions, simulate) From ca21c3a63a4bc881ce022840451493755138df31 Mon Sep 17 00:00:00 2001 From: john feng Date: Fri, 2 May 2025 11:21:24 -0700 Subject: [PATCH 37/38] revert assessor and add set_current_operation --- src/core/src/core_logic/PatchAssessor.py | 4 +--- src/core/src/core_logic/PatchInstaller.py | 3 +++ 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/core/src/core_logic/PatchAssessor.py b/src/core/src/core_logic/PatchAssessor.py index e2b864a9e..d82203334 100644 --- a/src/core/src/core_logic/PatchAssessor.py +++ b/src/core/src/core_logic/PatchAssessor.py @@ -42,9 +42,7 @@ def __init__(self, env_layer, execution_config, composite_logger, telemetry_writ def start_assessment(self): """ Start a patch assessment """ - # Prevent operation override during installation operation - if self.status_handler.get_current_operation() != Constants.INSTALLATION: - self.status_handler.set_current_operation(Constants.ASSESSMENT) + self.status_handler.set_current_operation(Constants.ASSESSMENT) self.raise_if_telemetry_unsupported() self.raise_if_min_python_version_not_met() diff --git a/src/core/src/core_logic/PatchInstaller.py b/src/core/src/core_logic/PatchInstaller.py index ec2569d62..d7550f8aa 100644 --- a/src/core/src/core_logic/PatchInstaller.py +++ b/src/core/src/core_logic/PatchInstaller.py @@ -684,6 +684,9 @@ def mark_installation_completed_with_warning(self): This is set outside of start_installation function 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. """ + # setting current operation to ensure correct substatus is updated + self.status_handler.set_current_operation(Constants.INSTALLATION) + # Update status handler error msg self.__log_final_warning_metric() From 963af02c66a83933bce2e40a355b13faeffc3695 Mon Sep 17 00:00:00 2001 From: john feng Date: Thu, 8 May 2025 15:48:47 -0700 Subject: [PATCH 38/38] add assertion for pendingpatchcount --- src/core/tests/Test_CoreMain.py | 41 +-------------------------------- 1 file changed, 1 insertion(+), 40 deletions(-) diff --git a/src/core/tests/Test_CoreMain.py b/src/core/tests/Test_CoreMain.py index 4d37238f5..3d438318b 100644 --- a/src/core/tests/Test_CoreMain.py +++ b/src/core/tests/Test_CoreMain.py @@ -60,13 +60,6 @@ def mock_batch_patching_with_packages(self, all_packages, all_package_versions, """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_are_all_requested_packages_installed(self): - return True - def test_operation_fail_for_non_autopatching_request(self): # Test for non auto patching request argument_composer = ArgumentComposer() @@ -1316,39 +1309,6 @@ def test_warning_status_when_packages_initially_fail_but_succeed_on_retry(self): 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_are_all_requested_packages_installed = runtime.status_handler.are_all_requested_packages_installed - - # Mock batch_patching with packages to return [], [], false - runtime.patch_installer.batch_patching = self.mock_batch_patching_with_no_packages - runtime.status_handler.are_all_requested_packages_installed = self.mock_are_all_requested_packages_installed - - # 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.are_all_requested_packages_installed = original_are_all_requested_packages_installed - runtime.stop() - def __check_telemetry_events(self, runtime): all_events = os.listdir(runtime.telemetry_writer.events_folder_path) self.assertTrue(len(all_events) > 0) @@ -1374,6 +1334,7 @@ def __assertion_pkg_succeed_on_retry(self, runtime): # 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()) + self.assertEqual(json.loads(substatus_file_data[1]["formattedMessage"]["message"])["pendingPatchCount"], 0) # Verify at least one error detail about package retry error_details = json.loads(substatus_file_data[1]["formattedMessage"]["message"])["errors"]["details"]