Add started by and fix reboot always (#77)

* Add started by and fix reboot always

* Augmented test

Author: kjohn-msft

src/core/src/bootstrap/Constants.py

@@ -30,7 +30,7 @@ def __iter__(self):
     UNKNOWN = "Unknown"
 
     # Extension version (todo: move to a different file)
-    EXT_VERSION = "1.6.19"
+    EXT_VERSION = "1.6.20"
 
     # Runtime environments
     TEST = 'Test'
@@ -186,6 +186,11 @@ class RebootStatus(EnumBackport):
         COMPLETED = "Completed"
         FAILED = "Failed"
 
+    # StartedBy Patch Assessment Summary Status Values
+    class PatchAssessmentSummaryStartedBy(EnumBackport):
+        USER = "User"
+        PLATFORM = "Platform"
+
     # Maintenance Window
     PACKAGE_INSTALL_EXPECTED_MAX_TIME_IN_MINUTES = 5
 

src/core/src/core_logic/PatchInstaller.py

@@ -277,14 +277,17 @@ def mark_installation_completed(self):
                 # todo: temp fix to test auto patching, this will be reset to using the maintenanceRunId string as is, once the corresponding changes in RSM are made
                 # patch_version = str(self.execution_config.maintenance_run_id)
                 patch_version = datetime.datetime.strptime(self.execution_config.maintenance_run_id.split(" ")[0], "%m/%d/%Y").strftime('%Y.%m.%d')
-                self.status_handler.set_patch_metadata_for_healthstore_substatus_json(patch_version=patch_version if patch_version is not None and patch_version != "" else Constants.PATCH_VERSION_UNKNOWN,
-                                                                                      report_to_healthstore=True,
-                                                                                      wait_after_update=False)
+
             except ValueError as e:
                 error_message = "Maintenance Run Id is in incorrect format. Expected=[DateTimeUTC]. Actual=[{0}]. Error=[{1}]".format(str(self.execution_config.maintenance_run_id), repr(e))
                 self.composite_logger.log_error(error_message)
                 raise Exception(error_message)
 
+            self.status_handler.set_patch_metadata_for_healthstore_substatus_json(
+                patch_version=patch_version if patch_version is not None and patch_version != "" else Constants.PATCH_VERSION_UNKNOWN,
+                report_to_healthstore=True,
+                wait_after_update=False)
+
     # region Installation Progress support
     def perform_status_reconciliation_conditionally(self, package_manager, condition=True):
         """Periodically based on the condition check, writes out success records as required; returns count of detected installs.

src/core/src/core_logic/RebootManager.py

@@ -104,6 +104,11 @@ def start_reboot_if_required_and_time_available(self, current_time_available):
             self.composite_logger.log(" - There was no reboot pending detected. Reboot is being skipped as it's not required, as per patch installation configuration (" + str(Constants.REBOOT_IF_REQUIRED) + ").")
             return False
 
+        # prevent repeated reboots
+        if self.reboot_setting == Constants.REBOOT_ALWAYS and not reboot_pending and self.status_handler.get_installation_reboot_status() == Constants.RebootStatus.COMPLETED:
+            self.composite_logger.log(" - At least one reboot has occurred, and there's no reboot pending, so the conditions for the 'Reboot Always' setting is fulfilled and reboot won't be repeated.")
+            return False
+
         # attempt to reboot is enough time is available
         if self.reboot_setting == Constants.REBOOT_ALWAYS or (self.reboot_setting == Constants.REBOOT_IF_REQUIRED and reboot_pending):
             if self.is_reboot_time_available(current_time_available):

src/core/src/service_interfaces/StatusHandler.py

@@ -202,6 +202,10 @@ def get_os_name_and_version(self):
     # endregion
 
     # region - Installation Reboot Status
+    def get_installation_reboot_status(self):
+        """ Safe retrieval of currently stored reboot status (stateful) """
+        return self.__installation_reboot_status
+
     def set_installation_reboot_status(self, new_reboot_status):
         """ Valid reboot statuses: NotNeeded, Required, Started, Failed, Completed """
         if new_reboot_status not in [Constants.RebootStatus.NOT_NEEDED, Constants.RebootStatus.REQUIRED, Constants.RebootStatus.STARTED, Constants.RebootStatus.FAILED, Constants.RebootStatus.COMPLETED]:
@@ -270,7 +274,10 @@ def __new_assessment_summary_json(self, assessment_packages_json):
             else:
                 other_patch_count += 1
 
-        # Compose substatus message
+        # discern started by
+        started_by = Constants.PatchAssessmentSummaryStartedBy.PLATFORM if self.execution_config.operation == Constants.AUTO_ASSESSMENT else Constants.PatchAssessmentSummaryStartedBy.USER
+
+        # Compose sub-status message
         return {
             "assessmentActivityId": str(self.execution_config.activity_id),
             "rebootPending": self.is_reboot_pending,
@@ -279,6 +286,7 @@ def __new_assessment_summary_json(self, assessment_packages_json):
             "patches": assessment_packages_json,
             "startTime": str(self.execution_config.start_time),
             "lastModifiedTime": str(self.env_layer.datetime.timestamp()),
+            "startedBy": str(started_by),
             "errors": self.__set_errors_json(self.__assessment_total_error_count, self.__assessment_errors)
         }
 

src/core/tests/Test_RebootManager.py

@@ -74,6 +74,27 @@ def test_reboot_always_time_available(self):
         self.assertEqual(reboot_manager.start_reboot_if_required_and_time_available(20), True)
         runtime.stop()
 
+    def test_reboot_always_runs_only_once_if_no_reboot_is_required(self):
+        reboot_setting_in_api = 'Always'
+        argument_composer = ArgumentComposer()
+        argument_composer.reboot_setting = reboot_setting_in_api
+        runtime = RuntimeCompositor(argument_composer.get_composed_arguments(), True, Constants.YUM)
+        reboot_manager = runtime.reboot_manager
+
+        # Validate single reboot scenario
+        runtime.status_handler.is_reboot_pending = True
+        self.assertEqual(reboot_manager.start_reboot_if_required_and_time_available(20), True)
+
+        # mock completing the reboot once, with no reboot required
+        runtime.status_handler.set_installation_reboot_status(Constants.RebootStatus.REQUIRED)
+        runtime.status_handler.set_installation_reboot_status(Constants.RebootStatus.STARTED)
+        runtime.status_handler.is_reboot_pending = False
+        runtime.status_handler.set_installation_reboot_status(Constants.RebootStatus.COMPLETED)
+
+        # no further reboot should be required
+        self.assertEqual(reboot_manager.start_reboot_if_required_and_time_available(20), False)
+        runtime.stop()
+
     def test_reboot_always_time_not_available(self):
         reboot_setting_in_api = 'Always'
         argument_composer = ArgumentComposer()

src/core/tests/Test_StatusHandler.py

@@ -33,7 +33,6 @@ def tearDown(self):
     def test_set_package_assessment_status(self):
         packages, package_versions = self.runtime.package_manager.get_all_updates()
         self.runtime.status_handler.set_package_assessment_status(packages, package_versions)
-        substatus_file_data = []
         with self.runtime.env_layer.file_system.open(self.runtime.execution_config.status_file_path, 'r') as file_handle:
             substatus_file_data = json.load(file_handle)[0]["status"]["substatus"][0]
         self.assertEqual(substatus_file_data["name"], Constants.PATCH_ASSESSMENT_SUMMARY)
@@ -42,6 +41,21 @@ def test_set_package_assessment_status(self):
         self.assertEqual(json.loads(substatus_file_data["formattedMessage"]["message"])["patches"][1]["name"], "samba-common-bin")
         self.assertEqual(json.loads(substatus_file_data["formattedMessage"]["message"])["patches"][2]["name"], "samba-libs")
         self.assertTrue("python-samba_2:4.4.5+dfsg-2ubuntu5.4" in str(json.loads(substatus_file_data["formattedMessage"]["message"])["patches"][0]["patchId"]))
+        self.assertEqual(json.loads(substatus_file_data["formattedMessage"]["message"])["startedBy"], Constants.PatchAssessmentSummaryStartedBy.USER)
+
+    def test_set_package_assessment_status_started_by(self):
+        self.runtime.execution_config.operation = Constants.AUTO_ASSESSMENT
+        packages, package_versions = self.runtime.package_manager.get_all_updates()
+        self.runtime.status_handler.set_package_assessment_status(packages, package_versions)
+        with self.runtime.env_layer.file_system.open(self.runtime.execution_config.status_file_path, 'r') as file_handle:
+            substatus_file_data = json.load(file_handle)[0]["status"]["substatus"][0]
+        self.assertEqual(substatus_file_data["name"], Constants.PATCH_ASSESSMENT_SUMMARY)
+        self.assertEqual(len(json.loads(substatus_file_data["formattedMessage"]["message"])["patches"]), 3)
+        self.assertEqual(json.loads(substatus_file_data["formattedMessage"]["message"])["patches"][0]["name"], "python-samba")
+        self.assertEqual(json.loads(substatus_file_data["formattedMessage"]["message"])["patches"][1]["name"], "samba-common-bin")
+        self.assertEqual(json.loads(substatus_file_data["formattedMessage"]["message"])["patches"][2]["name"], "samba-libs")
+        self.assertTrue("python-samba_2:4.4.5+dfsg-2ubuntu5.4" in str(json.loads(substatus_file_data["formattedMessage"]["message"])["patches"][0]["patchId"]))
+        self.assertEqual(json.loads(substatus_file_data["formattedMessage"]["message"])["startedBy"], Constants.PatchAssessmentSummaryStartedBy.PLATFORM)
 
     def test_set_package_install_status(self):
         packages, package_versions = self.runtime.package_manager.get_all_updates()

src/extension/src/Constants.py

@@ -28,7 +28,7 @@ def __iter__(self):
                         yield item
 
     # Extension version (todo: move to a different file)
-    EXT_VERSION = "1.6.19"
+    EXT_VERSION = "1.6.20"
 
     # Runtime environments
     TEST = 'Test'

src/extension/src/manifest.xml

@@ -2,7 +2,7 @@
 <ExtensionImage xmlns="http://schemas.microsoft.com/windowsazure">
   <ProviderNameSpace>Microsoft.CPlat.Core</ProviderNameSpace>
   <Type>LinuxPatchExtension</Type>
-  <Version>1.6.19</Version>
+  <Version>1.6.20</Version>
   <Label>Microsoft Azure VM InGuest Patch Extension for Linux Virtual Machines</Label>
   <HostingResources>VmRole</HostingResources>
   <MediaLink></MediaLink>