Skip to content

Commit

Permalink
refactor: refine persist_file_handling internal implementation (#354)
Browse files Browse the repository at this point in the history 
This PR refines the persist_file_handling module's internal implementation, reduces the code cognitive complexity, and refines the workflow of entry processing. Also now otaclient won't break out the OTA on failed entry during persist file handling.

Internal behavior change:
* OTA will not break on failed entry during persist file handling processing.
  • Loading branch information
@Bodong-Yang
Bodong-Yang authored Jul 11, 2024
1 parent 36f8b3a commit a7684fc
Show file tree
Hide file tree
Showing 2 changed files with 94 additions and 65 deletions.
7 changes: 4 additions & 3 deletions src/otaclient/app/ota_client.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -377,10 +377,11 @@ def _process_persistents(self, ota_metadata: ota_metadata_parser.OTAMetadata):
)
continue

if (
_per_fpath.is_file() or _per_fpath.is_dir() or _per_fpath.is_symlink()
): # NOTE: not equivalent to perinf.path.exists()
try:
_handler.preserve_persist_entry(_per_fpath)
except Exception as e:
_err_msg = f"failed to preserve {_per_fpath}: {e!r}, skip"
logger.warning(_err_msg)

def _execute_update(self):
"""Implementation of OTA updating."""
Expand Down
152 changes: 90 additions & 62 deletions src/otaclient_common/persist_file_handling.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
map_gid_by_grpnam,
map_uid_by_pwnam,
)
from otaclient_common.typing import StrOrPath

logger = logging.getLogger(__name__)

Expand All @@ -43,13 +44,13 @@ class PersistFilesHandler:

def __init__(
self,
src_passwd_file: str | Path,
src_group_file: str | Path,
dst_passwd_file: str | Path,
dst_group_file: str | Path,
src_passwd_file: StrOrPath,
src_group_file: StrOrPath,
dst_passwd_file: StrOrPath,
dst_group_file: StrOrPath,
*,
src_root: str | Path,
dst_root: str | Path,
src_root: StrOrPath,
dst_root: StrOrPath,
):
self._uid_mapper = lru_cache()(
partial(
Expand Down Expand Up @@ -87,7 +88,7 @@ def map_gid_by_grpnam(*, src_db: ParsedGroup, dst_db: ParsedGroup, gid: int) ->
return _mapped_gid

def _chown_with_mapping(
self, _src_stat: os.stat_result, _dst_path: str | Path
self, _src_stat: os.stat_result, _dst_path: StrOrPath
) -> None:
_src_uid, _src_gid = _src_stat.st_uid, _src_stat.st_gid
try:
Expand All @@ -108,12 +109,13 @@ def _rm_target(_target: Path) -> None:
"""Remove target with proper methods."""
if _target.is_symlink() or _target.is_file():
return _target.unlink(missing_ok=True)
elif _target.is_dir():
if _target.is_dir():
return shutil.rmtree(_target, ignore_errors=True)
elif _target.exists():
raise ValueError(
f"{_target} is not normal file/symlink/dir, failed to remove"
)
# NOTE that exists will follow symlink, so we need to check symlink first
if not _target.exists():
return

raise ValueError(f"{_target} is not normal file/symlink/dir, failed to remove")

def _prepare_symlink(self, _src_path: Path, _dst_path: Path) -> None:
_dst_path.symlink_to(os.readlink(_src_path))
Expand All @@ -134,66 +136,36 @@ def _prepare_file(self, _src_path: Path, _dst_path: Path) -> None:
os.chmod(_dst_path, _src_stat.st_mode)
self._chown_with_mapping(_src_stat, _dst_path)

def _prepare_parent(self, _origin_entry: Path) -> None:
for _parent in reversed(_origin_entry.parents):
def _prepare_parent(self, _path_relative_to_root: Path) -> None:
"""
NOTE that we intensionally keep the already there parents' permission
setting on destination.
"""
for _parent in reversed(_path_relative_to_root.parents):
_src_parent, _dst_parent = (
self._src_root / _parent,
self._dst_root / _parent,
)
if _dst_parent.is_dir(): # keep the origin parent on dst as it
continue
if _dst_parent.is_symlink() or _dst_parent.is_file():
_dst_parent.unlink(missing_ok=True)
self._prepare_dir(_src_parent, _dst_parent)
continue
if _dst_parent.exists():
raise ValueError(
f"{_dst_parent=} is not a normal file/symlink/dir, cannot cleanup"
)
self._prepare_dir(_src_parent, _dst_parent)

# API

def preserve_persist_entry(
self, _persist_entry: str | Path, *, src_missing_ok: bool = True
):
logger.info(f"preserving {_persist_entry}")
# persist_entry in persists.txt must be rooted at /
origin_entry = Path(_persist_entry).relative_to("/")
src_path = self._src_root / origin_entry
dst_path = self._dst_root / origin_entry

# ------ src is symlink ------ #
# NOTE: always check if symlink first as is_file/is_dir/exists all follow_symlinks
if src_path.is_symlink():
self._rm_target(dst_path)
self._prepare_parent(origin_entry)
self._prepare_symlink(src_path, dst_path)
return
# keep the origin parent on dst as it
# NOTE that is_dir will follow symlink.
if _dst_parent.is_dir():
continue

# ------ src is file ------ #
if src_path.is_file():
self._rm_target(dst_path)
self._prepare_parent(origin_entry)
self._prepare_file(src_path, dst_path)
return
if not _dst_parent.exists():
self._prepare_dir(_src_parent, _dst_parent)
continue

# ------ src is not regular file/symlink/dir ------ #
# we only process normal file/symlink/dir
if src_path.exists() and not src_path.is_dir():
raise ValueError(f"{src_path=} must be either a file/symlink/dir")

# ------ src doesn't exist ------ #
if not src_path.exists():
_err_msg = f"{src_path=} not found"
logger.warning(_err_msg)
if not src_missing_ok:
raise ValueError(_err_msg)
return
raise ValueError(
f"{_dst_parent=} is not a normal file/symlink/dir, cannot cleanup"
)

# ------ src is dir ------ #
# dive into src_dir and preserve everything under the src dir
self._prepare_parent(origin_entry)
def _recursively_prepare_dir(self, src_path: Path):
"""Dive into src_dir and preserve everything under the src dir."""
for src_curdir, dnames, fnames in os.walk(src_path, followlinks=False):
src_cur_dpath = Path(src_curdir)
dst_cur_dpath = self._dst_root / src_cur_dpath.relative_to(self._src_root)
Expand All @@ -206,14 +178,70 @@ def preserve_persist_entry(
for _fname in fnames:
_src_fpath, _dst_fpath = src_cur_dpath / _fname, dst_cur_dpath / _fname
self._rm_target(_dst_fpath)

# NOTE that fnames also contain symlink to normal file
if _src_fpath.is_symlink():
self._prepare_symlink(_src_fpath, _dst_fpath)
continue
self._prepare_file(_src_fpath, _dst_fpath)
else:
self._prepare_file(_src_fpath, _dst_fpath)

# symlinks to dirs also included in dnames, we must handle it
for _dname in dnames:
_src_dpath, _dst_dpath = src_cur_dpath / _dname, dst_cur_dpath / _dname
if _src_dpath.is_symlink():
self._rm_target(_dst_dpath)
self._prepare_symlink(_src_dpath, _dst_dpath)
# NOTE that we don't need to create dir here, as os.walk will take us
# to this folder later, we will create the folder in the dest when
# we enter the src folder.

# API

def preserve_persist_entry(self, _persist_entry: StrOrPath):
"""Preserve <_persist_entry> from active slot to standby slot.
Args:
_persist_entry (StrOrPath): The canonical path of the entry to be preserved.
Raises:
ValueError: Raised when src <_persist_entry> is not a regular file, symlink or directory,
or failed to prepare destination.
"""
# persist_entry in persists.txt must be rooted at /
path_relative_to_root = Path(_persist_entry).relative_to("/")
src_path = self._src_root / path_relative_to_root
dst_path = self._dst_root / path_relative_to_root

# ------ src is symlink ------ #
# NOTE: always check if symlink first as is_file/is_dir/exists all follow_symlinks
if src_path.is_symlink():
logger.info(
f"preserving symlink: {_persist_entry}, points to {os.readlink(src_path)}"
)
self._rm_target(dst_path)
self._prepare_parent(path_relative_to_root)
self._prepare_symlink(src_path, dst_path)
return

# ------ src is file ------ #
if src_path.is_file():
logger.info(f"preserving normal file: {_persist_entry}")
self._rm_target(dst_path)
self._prepare_parent(path_relative_to_root)
self._prepare_file(src_path, dst_path)
return

# ------ src is dir ------ #
if src_path.is_dir():
logger.info(f"recursively preserve directory: {_persist_entry}")
self._prepare_parent(path_relative_to_root)
self._recursively_prepare_dir(src_path)
return

# ------ src is not regular file/symlink/dir or missing ------ #
_err_msg = f"{src_path=} doesn't exist"
if src_path.exists():
_err_msg = f"src must be either a file/symlink/dir, skip {_persist_entry=}"

logger.warning(_err_msg)
raise ValueError(_err_msg)

1 comment on commit a7684fc

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coverage

Coverage Report
FileStmtsMissCoverMissing
src/ota_metadata/legacy
   __init__.py110100% 
   parser.py3353888%100, 156, 161, 197–198, 208–209, 212, 224, 282, 292–295, 334–337, 417, 420, 428–430, 443, 452–453, 456–457, 669–670, 673, 700–702, 752, 755–757
   types.py841384%37, 40–42, 112–116, 122–125
src/ota_proxy
   __init__.py361072%59, 61, 63, 72, 81–82, 102, 104–106
   __main__.py770%16–18, 20, 22–23, 25
   _consts.py150100% 
   cache_control.py68494%71, 91, 113, 121
   config.py180100% 
   db.py1461589%75, 81, 103, 113, 116, 145–147, 166, 199, 208–209, 229, 258, 300
   errors.py50100% 
   orm.py1121091%92, 97, 102, 108, 114, 141–142, 155, 232, 236
   ota_cache.py4018678%98–99, 218, 229, 256–258, 278, 294–295, 297, 320–321, 327, 331, 333, 360–362, 378, 439–440, 482–483, 553, 566–569, 619, 638–639, 671–672, 683, 717–721, 725–727, 729, 731–738, 740–742, 745–746, 750–751, 755, 802, 810–812, 891–894, 898, 901–902, 916–917, 919–921, 925–926, 932–933, 964, 970, 997, 1026–1028
   server_app.py1383971%76, 79, 85, 101, 103, 162, 171, 213–214, 216–218, 221, 226–228, 231–232, 235, 238, 241, 244, 257–258, 261–262, 264, 267, 293–296, 299, 313–315, 321–323
   utils.py23195%33
src/otaclient
   __init__.py5260%17, 19
   __main__.py110%16
   log_setting.py52590%53, 55, 64–66
src/otaclient/app
   __main__.py110%16
   configs.py760100% 
   errors.py1200100% 
   interface.py50100% 
   main.py46589%52–53, 75–77
   ota_client.py37310871%76, 84, 105, 132, 134–135, 137, 141, 145–146, 151–152, 158, 160, 198–201, 207, 211, 217, 336, 348–349, 351, 360, 363, 368–369, 372, 378, 380–384, 403–406, 409–416, 444–447, 493–494, 498, 500–501, 531–532, 541–548, 555, 558–564, 609–612, 620, 656–658, 663–665, 668–669, 671–672, 674, 732–733, 736, 744–745, 748, 759–760, 763, 771–772, 775, 786, 805, 832, 851, 869
   ota_client_stub.py39410972%76–78, 80–81, 89–92, 95–97, 101, 106–107, 109–110, 113, 115–116, 119–121, 124–125, 128–130, 135–140, 144, 147–151, 153–154, 162–164, 167, 204–206, 211, 247, 272, 275, 278, 382, 406, 408, 432, 478, 535, 605–606, 645, 664–666, 672–675, 679–681, 688–690, 693, 697–700, 753, 842–844, 851, 881–882, 885–889, 898–907, 914, 920, 923–924, 928, 931
   update_stats.py104991%57, 103, 105, 114, 116, 125, 127, 148, 179
src/otaclient/app/boot_control
   __init__.py40100% 
   _common.py24811254%74–75, 96–98, 114–115, 135–136, 155–156, 175–176, 195–196, 218–220, 235–236, 260–266, 287, 295, 313, 321, 340–341, 344–345, 368, 370–379, 381–390, 392–394, 413, 416, 424, 432, 448–450, 452–457, 550, 555, 560, 673, 677–678, 681, 689, 691–692, 718–719, 721–724, 729, 735–736, 739–740, 742, 749–750, 761–767, 775–777, 781–782, 785–786, 789, 795
   _grub.py41712869%216, 264–267, 273–277, 314–315, 322–327, 330–336, 339, 342–343, 348, 350–352, 361–367, 369–370, 372–374, 383–385, 387–389, 468–469, 473–474, 526, 532, 558, 580, 584–585, 600–602, 626–629, 641, 645–647, 649–651, 710–713, 738–741, 764–767, 779–780, 783–784, 819, 825, 845–846, 848, 860, 863, 866, 869, 873–875, 893–896, 924–927, 932–940, 945–953
   _jetson_cboot.py27021420%69–70, 77–78, 96–105, 117, 124–125, 137, 143–144, 154–156, 168–169, 180–181, 184–185, 188–189, 192–196, 199–200, 204–205, 210–211, 213–217, 219–225, 227–228, 233, 236, 239–240, 243, 247–248, 252–253, 257, 260, 263, 267–273, 275–277, 282, 285, 288, 292, 299, 301–304, 317, 320, 324, 326–328, 332, 339, 341, 344, 350–351, 356, 364, 372–374, 383–384, 386–388, 394, 397–399, 403–404, 406, 409, 418–420, 423, 426, 429–434, 436–438, 441, 444, 448–453, 457–459, 464–465, 469–470, 473, 476, 479–480, 483, 486, 491, 494, 497–498, 500, 502, 505, 508, 510–511, 514–518, 523–524, 526, 534–538, 540, 543, 546, 557–558, 563, 573, 576–584, 589–597, 602–610, 616–618, 621, 624
   _jetson_common.py1416653%50, 74, 129–134, 136, 141–143, 148–151, 159–160, 167–168, 173–174, 190–191, 193–195, 198–200, 203, 207, 211, 215–217, 223–224, 226, 259, 285–286, 288–290, 294–297, 299–300, 302–306, 308, 315–316, 319, 321, 331, 334–335, 338, 340
   _rpi_boot.py28613453%54, 57, 121–122, 126, 134–137, 151–154, 161–162, 164–165, 170–171, 174–175, 184–185, 223, 229–233, 236, 254–256, 260–262, 267–269, 273–275, 285–286, 289, 292, 294–295, 297–298, 300–302, 308, 311–312, 322–325, 333–337, 339, 341–342, 347–348, 355–361, 392, 394–397, 407–410, 414–415, 417–421, 449–452, 471–474, 479, 482, 500–503, 508–516, 521–529, 546–549, 555–557, 560, 563
   configs.py380100% 
   protocol.py40100% 
   selecter.py382631%44–46, 49–50, 54–55, 58–60, 63, 65, 69, 77–79, 81–82, 84–85, 89, 91–93, 95, 97
src/otaclient/app/create_standby
   __init__.py12558%28–30, 32, 34
   common.py2244480%59, 62–63, 67–69, 71, 75–76, 78, 124, 172–174, 176–178, 180, 183–186, 190, 201, 275–276, 278–283, 295, 332, 360, 363–365, 381–382, 396, 400, 422–423
   interface.py50100% 
   rebuild_mode.py97990%89–91, 103–108
src/otaclient/configs
   _common.py80100% 
   ecu_info.py57198%107
   proxy_info.py52296%88, 90
src/otaclient_api/v2
   __init__.py140100% 
   api_caller.py39684%45–47, 83–85
   api_stub.py170100% 
   types.py2562391%86, 89–92, 131, 209–210, 212, 259, 262–263, 506–508, 512–513, 515, 518–519, 522–523, 586
src/otaclient_common
   __init__.py34876%42–44, 59, 61, 67, 74–75
   common.py1541987%41, 45, 200, 203–205, 220, 227–229, 295–297, 307, 316–318, 364, 368
   downloader.py2001095%110–111, 129, 156, 372, 426, 430, 518–519, 528
   linux.py611575%51–53, 59, 69, 74, 76, 108–109, 133–134, 190, 195–196, 198
   logging.py29196%55
   persist_file_handling.py1181884%113, 118, 150–152, 163, 192–193, 228–232, 242–244, 246–247
   proto_streamer.py42880%33, 48, 66–67, 72, 81–82, 100
   proto_wrapper.py3984887%87, 165, 172, 184–186, 205, 210, 221, 257, 263, 268, 299, 303, 307, 402, 462, 469, 472, 492, 499, 501, 526, 532, 535, 537, 562, 568, 571, 573, 605, 609, 611, 625, 642, 669, 672, 676, 707, 713, 760–763, 765, 803–805
   retry_task_map.py80791%164–165, 167, 179–182
   typing.py25388%69–70, 72
TOTAL5944137076% 

Tests Skipped Failures Errors Time
173 0 💤 0 ❌ 0 🔥 5m 26s ⏱️

Please sign in to comment.