Bugfix/28537460 Fix Auto assessment status stuck in-progress

src/core/src/core_logic/ServiceManager.py

@@ -89,8 +89,8 @@ def is_service_enabled(self):
     # endregion
 
     # region - Service Unit Management
-    def create_service_unit_file(self, exec_start, desc, after="network.target", service_type="forking", wanted_by="multi-user.target"):
-        """ Note: Service type defaults to forking because of sh to py process fork """
+    def create_service_unit_file(self, exec_start, desc, after="network.target", service_type="simple", wanted_by="multi-user.target"):
+        """ Note: Service type is simple because the shell wrapper runs Core in the foreground."""
         service_unit_content_template = "\n[Unit]" + \
                                "\nDescription={0}" + \
                                "\nAfter={1}\n" + \

src/core/tests/Test_ServiceManager.py

@@ -28,6 +28,8 @@ def setUp(self):
         self.service_manager = ServiceManager(self.runtime.env_layer, self.runtime.execution_config, self.runtime.composite_logger, self.runtime.telemetry_writer,ServiceInfo("AutoAssessment", "Auto assessment service", "path"))
         self.service_manager.service_name = "test_service"
         self.mock_systemd_service_unit_path = "/etc/systemd/system/{0}.service"
+        self.written_service_unit_path = None
+        self.written_service_unit_content = None
 
     def tearDown(self):
         self.runtime.stop()
@@ -38,6 +40,8 @@ def mock_run_command_to_set_service_file_permission(self, cmd, no_output=False,
             return 0, "permissions set"
 
     def mock_write_with_retry_valid(self, file_path_or_handle, data, mode='a+'):
+        self.written_service_unit_path = file_path_or_handle
+        self.written_service_unit_content = data
         return
 
     def mock_invoke_systemctl(self, command, description):
@@ -63,6 +67,23 @@ def test_create_service_unit_file(self):
         self.service_manager.env_layer.file_system.write_with_retry = self.mock_write_with_retry_valid
         self.service_manager.create_service_unit_file(exec_start="/bin/bash " + self.service_manager.service_exec_path, desc="Microsoft Azure Linux Patch Extension - Auto Assessment")
 
+        self.assertEqual("/etc/systemd/system/test_service.service", self.written_service_unit_path)
+        self.assertIn("\nType=simple\n", self.written_service_unit_content)
+        self.assertNotIn("Type=forking", self.written_service_unit_content)
+
+    def test_create_and_set_service_idem_generates_simple_unit(self):
+        """ Guards the production creation path, not just the helper, so a future default flip is caught """
+        self.service_manager.env_layer.run_command_output = self.mock_run_command_to_set_service_file_permission
+        self.service_manager.env_layer.file_system.write_with_retry = self.mock_write_with_retry_valid
+        self.service_manager.invoke_systemctl = self.mock_invoke_systemctl
+        self.service_manager.systemctl_daemon_reload = lambda: None
+
+        self.service_manager.create_and_set_service_idem()
+
+        self.assertIn("\nType=simple\n", self.written_service_unit_content)
+        self.assertNotIn("Type=forking", self.written_service_unit_content)
+        self.assertNotIn("Type=notify", self.written_service_unit_content)
+
     def test_start_service(self):
         # Set method calls
         self.service_manager.invoke_systemctl_called = False

src/extension/src/Constants.py

@@ -59,6 +59,13 @@ def __iter__(self):
     ENABLE_MAX_RUNTIME = 3
     DISABLE_MAX_RUNTIME = 13
 
+    # Bug 28537460: Under Type=simple a hung assessment keeps the unit active forever and every
+    # later timer fire becomes a no-op, so the run must be externally bounded.
+    # Maximum Runtime + grace time must stay under the hourly auto-assessment timer interval
+    # (core AUTO_ASSESSMENT_CRON_INTERVAL = PT1H) so process is killed within 1 hour if still running.
+    AUTO_ASSESSMENT_MAX_RUNTIME_IN_SECS = 3000      # 50m
+    AUTO_ASSESSMENT_KILL_GRACE_IN_SECS = 300        # 5m. Kill process after 55 minutes.
+
     # Telemetry Settings
     # Note: these limits are based on number of characters as confirmed with agent team
     TELEMETRY_MSG_SIZE_LIMIT_IN_CHARS = 3072

src/extension/src/ProcessHandler.py

@@ -119,10 +119,11 @@ def stage_auto_assess_sh_safely(self, core_process_command):
                                   .format(cmd_core_py_path, exec_dir, core_py_path, auto_assess_sh_path, core_process_command))
 
             # generating exec script
+            auto_assess_core_command = core_process_command + " -" + Constants.AUTO_ASSESS_ONLY + " True"
             auto_assess_sh_data = "#!/usr/bin/env bash" +\
                                   "\n# Copyright 2021 Microsoft Corporation." + \
                                   "\ncd \"$(dirname \"$0\")\"" + \
-                                  "\n" + core_process_command + " -" + Constants.AUTO_ASSESS_ONLY + " True"
+                                  "\nexec timeout -s TERM -k " + str(Constants.AUTO_ASSESSMENT_KILL_GRACE_IN_SECS) + " " + str(Constants.AUTO_ASSESSMENT_MAX_RUNTIME_IN_SECS) + " " + auto_assess_core_command
 
             # stage exec script
             if os.path.exists(auto_assess_sh_path):

src/extension/tests/Test_ProcessHandler.py

@@ -45,6 +45,8 @@ def setUp(self):
         self.proc_cmdline_path = os.path.join(self.test_dir, "proc_cmdline")
         self.ext_output_status_handler = ExtOutputStatusHandler(self.logger, self.utility, self.json_file_handler, dir_path)
         self.process = subprocess.Popen(["echo", "Hello World!"], shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+        self.written_auto_assess_sh_path = None
+        self.written_auto_assess_sh_content = None
 
     def tearDown(self):
         VirtualTerminal().print_lowlight("\n----------------- tear down test runner -----------------")
@@ -81,6 +83,10 @@ def mock_file_system_open_raises_exception(self, path, mode):
     def mock_run_command_to_set_auto_assess_shell_file_permission(self, cmd, no_output=False, chk_err=False):
         return 0, "permissions set"
 
+    def mock_write_with_retry_valid(self, file_path_or_handle, data, mode='a+'):
+        self.written_auto_assess_sh_path = file_path_or_handle
+        self.written_auto_assess_sh_content = data
+
     def mock_subprocess_popen_process_not_running_after_launch(self, command, shell, stdout, stderr):
         self.process.pid = 1
         self.process.poll = self.mock_process_poll_return_Not_None
@@ -223,6 +229,26 @@ def test_start_daemon(self):
         process_handler.env_layer.run_command_output = run_command_output_backup
         ExtEnvHandler.get_temp_folder = ext_env_handler_get_temp_folder_backup
 
+    def test_auto_assess_sh_is_bounded_by_timeout(self):
+        process_handler = ProcessHandler(self.logger, self.env_layer, self.ext_output_status_handler)
+        write_backup = process_handler.env_layer.file_system.write_with_retry
+        run_backup = process_handler.env_layer.run_command_output
+        process_handler.env_layer.file_system.write_with_retry = self.mock_write_with_retry_valid
+        process_handler.env_layer.run_command_output = self.mock_run_command_to_set_auto_assess_shell_file_permission
+
+        process_handler.stage_auto_assess_sh_safely("/usr/bin/python3 /tmp/MsftLinuxPatchCore.py -sequenceNumber 1")
+
+        self.assertIn(Constants.CORE_AUTO_ASSESS_SH_FILE_NAME, self.written_auto_assess_sh_path)
+        self.assertIn("exec timeout -s TERM -k " + str(Constants.AUTO_ASSESSMENT_KILL_GRACE_IN_SECS)
+                      + " " + str(Constants.AUTO_ASSESSMENT_MAX_RUNTIME_IN_SECS), self.written_auto_assess_sh_content)
+        self.assertIn("-" + Constants.AUTO_ASSESS_ONLY + " True", self.written_auto_assess_sh_content)
+        self.assertEqual(1, self.written_auto_assess_sh_content.count("exec "))
+        script_lines = [line for line in self.written_auto_assess_sh_content.split("\n") if line.strip()]
+        self.assertTrue(script_lines[-1].startswith("exec timeout -s TERM -k "))
+
+        process_handler.env_layer.file_system.write_with_retry = write_backup
+        process_handler.env_layer.run_command_output = run_backup
+
     def test_is_process_patching_operation(self):
         # setting mocks
         backup_file_system_open = self.env_layer.file_system.open