fix: persists file handling should be at PROCESS_POSTUPDATE phase (#274)

This PR fixes the following unexpected behavior due to persist file handling is processed in wrong phase(should be in PROCESS_POSTUPDATE, but in APPLY_UPDATE):

1. all files are processed, but no progress is made and phase is still stuck at APPLY_UPDATE phase,
2. if persist file handling also copying the swapfile, APPLY_UPDATE will be stuck with even longer time.

This PR fixes the above issue by move persist file handling out of create_standby package, and implements it in PROCESS_POSTUPDATE phase.

otaclient/app/create_standby/rebuild_mode.py

@@ -78,28 +78,6 @@ def _process_dirs(self):
         for entry in self.delta_bundle.get_new_dirs():
             entry.mkdir_relative_to_mount_point(self.standby_slot_mp)
 
-    def _process_persistents(self):
-        """NOTE: just copy from legacy mode"""
-        from ..copy_tree import CopyTree
-
-        _passwd_file = Path(cfg.PASSWD_FILE)
-        _group_file = Path(cfg.GROUP_FILE)
-        _copy_tree = CopyTree(
-            src_passwd_file=_passwd_file,
-            src_group_file=_group_file,
-            dst_passwd_file=self.standby_slot_mp / _passwd_file.relative_to("/"),
-            dst_group_file=self.standby_slot_mp / _group_file.relative_to("/"),
-        )
-
-        for _perinf in self._ota_metadata.iter_metafile(MetafilesV1.PERSISTENT_FNAME):
-            _perinf_path = Path(_perinf.path)
-            if (
-                _perinf_path.is_file()
-                or _perinf_path.is_dir()
-                or _perinf_path.is_symlink()
-            ):  # NOTE: not equivalent to perinf.path.exists()
-                _copy_tree.copy_with_parents(_perinf_path, self.standby_slot_mp)
-
     def _process_symlinks(self):
         for _symlink in self._ota_metadata.iter_metafile(
             MetafilesV1.SYMBOLICLINK_FNAME
@@ -211,7 +189,9 @@ def create_standby_slot(self):
         self._process_dirs()
         self._process_regulars()
         self._process_symlinks()
-        self._process_persistents()
+        # NOTE(20240219): mv persist file handling logic from
+        #                 create_standby implementation to post_update hook.
+        # self._process_persistents()
         self._save_meta()
         # cleanup on successful finish
         # NOTE: if failed, the tmp dirs will not be removed now,

otaclient/app/ota_client.py

@@ -286,6 +286,32 @@ def _update_standby_slot(self):
 
         logger.info("finished updating standby slot")
 
+    def _process_persistents(self):
+        """NOTE: just copy from legacy mode"""
+        from .copy_tree import CopyTree
+
+        standby_slot_mp = self._boot_controller.get_standby_slot_path()
+
+        _passwd_file = Path(cfg.PASSWD_FILE)
+        _group_file = Path(cfg.GROUP_FILE)
+        _copy_tree = CopyTree(
+            src_passwd_file=_passwd_file,
+            src_group_file=_group_file,
+            dst_passwd_file=standby_slot_mp / _passwd_file.relative_to("/"),
+            dst_group_file=standby_slot_mp / _group_file.relative_to("/"),
+        )
+
+        for _perinf in self._otameta.iter_metafile(
+            ota_metadata.MetafilesV1.PERSISTENT_FNAME
+        ):
+            _perinf_path = Path(_perinf.path)
+            if (
+                _perinf_path.is_file()
+                or _perinf_path.is_dir()
+                or _perinf_path.is_symlink()
+            ):  # NOTE: not equivalent to perinf.path.exists()
+                _copy_tree.copy_with_parents(_perinf_path, standby_slot_mp)
+
     def _execute_update(
         self,
         version: str,
@@ -391,13 +417,15 @@ def _execute_update(
             )
 
         # ------ post update ------ #
-        logger.info("local update finished, wait on all subecs...")
-        logger.info("enter boot control post update phase...")
-        # boot controller postupdate
+        logger.info("enter post update phase...")
         self.update_phase = wrapper.UpdatePhase.PROCESSING_POSTUPDATE
+        # NOTE(20240219): move persist file handling here
+        self._process_persistents()
+
+        # boot controller postupdate
         next(_postupdate_gen := self._boot_controller.post_update())
 
-        # wait for sub ecu if needed before rebooting
+        logger.info("local update finished, wait on all subecs...")
         self._control_flags.wait_can_reboot_flag()
         next(_postupdate_gen, None)  # reboot
 

tests/test_create_standby.py

@@ -87,11 +87,6 @@ def test_update_with_create_standby_RebuildMode(self, mocker: MockerFixture):
         from otaclient.app.ota_client import _OTAUpdater, OTAClientControlFlags
         from otaclient.app.create_standby.rebuild_mode import RebuildMode
 
-        # TODO: not test process_persistent currently,
-        #       as we currently directly compare the standby slot
-        #       with the OTA image.
-        RebuildMode._process_persistents = mocker.MagicMock()
-
         # ------ execution ------ #
         otaclient_control_flags = typing.cast(
             OTAClientControlFlags, mocker.MagicMock(spec=OTAClientControlFlags)
@@ -102,6 +97,7 @@ def test_update_with_create_standby_RebuildMode(self, mocker: MockerFixture):
             proxy=None,
             control_flags=otaclient_control_flags,
         )
+        _updater._process_persistents = persist_handler = mocker.MagicMock()
         # NOTE: mock the shutdown method as we need to assert before the
         #       updater is closed.
         _updater_shutdown = _updater.shutdown
@@ -115,6 +111,7 @@ def test_update_with_create_standby_RebuildMode(self, mocker: MockerFixture):
         time.sleep(2)  # wait for downloader to record stats
 
         # ------ assertions ------ #
+        persist_handler.assert_called_once()
         # --- assert update finished
         _updater.shutdown.assert_called_once()
         otaclient_control_flags.wait_can_reboot_flag.assert_called_once()

tests/test_ota_client.py

@@ -122,13 +122,13 @@ def _delta_generate(self, prepare_ab_slots):
 
     @pytest.fixture(autouse=True)
     def mock_setup(self, mocker: pytest_mock.MockerFixture, _delta_generate):
-        from otaclient.app.proxy_info import ProxyInfo
         from otaclient.app.configs import BaseConfig
 
         # ------ mock boot_controller ------ #
         self._boot_control = typing.cast(
             BootControllerProtocol, mocker.MagicMock(spec=BootControllerProtocol)
         )
+
         # ------ mock create_standby ------ #
         self._create_standby = typing.cast(
             StandbySlotCreatorProtocol,
@@ -168,6 +168,7 @@ def test_OTAUpdater(self, mocker: pytest_mock.MockerFixture):
             proxy=None,
             control_flags=otaclient_control_flags,
         )
+        _updater._process_persistents = process_persists_handler = mocker.MagicMock()
 
         _updater.execute(
             version=cfg.UPDATE_VERSION,
@@ -190,6 +191,7 @@ def test_OTAUpdater(self, mocker: pytest_mock.MockerFixture):
         # assert create standby module is used
         self._create_standby.calculate_and_prepare_delta.assert_called_once()
         self._create_standby.create_standby_slot.assert_called_once()
+        process_persists_handler.assert_called_once()
 
 
 class Test_OTAClient: