Squashed commit of the following:

commit 727ebdd
Author: Bodong Yang <[email protected]>
Date:   Thu Apr 25 12:03:09 2024 +0900

    refine(v3.7.x): refine bootctrl.common CMDHelperFuncs (tier4#289)

    This PR does the following things:
    1. refines the common module's subprocess wrapper methods.
    2. refines and simplify the boot_control.common.CMDHelperFuncs implementation a lot.
    3. cleanup and remove boot_contro.errors, now each boot controller implementation has their own error types.
    4. use mount slot helper's prepare_standby_dev instead of each boot controller's prepare_standby_dev method.
    5. integrate the new CMDHelperFuncs to each boot controller and fix up test files accordingly.

    NOTE that cboot boot controller implementation is temporarily removed, it will be added back in tier4#287 .

otaclient/app/boot_control/_common.py

@@ -44,7 +44,12 @@
 
 
 class CMDHelperFuncs:
-    """HelperFuncs bundle for wrapped linux cmd."""
+    """HelperFuncs bundle for wrapped linux cmd.
+
+    When underlying subprocess call failed and <raise_exception> is True,
+        functions defined in this class will raise the original exception
+        to the upper caller.
+    """
 
     @classmethod
     def get_attrs_by_dev(
@@ -53,7 +58,16 @@ def get_attrs_by_dev(
         """Get <attr> from <dev>.
 
         This is implemented by calling:
-            lsblk -in -o <attr> <dev>
+            `lsblk -in -o <attr> <dev>`
+
+        Args:
+            attr (PartitionToken): the attribute to retrieve from the <dev>.
+            dev (Path | str): the target device path.
+            raise_exception (bool, optional): raise exception on subprocess call failed.
+                Defaults to True.
+
+        Returns:
+            str: <attr> of <dev>.
         """
         cmd = ["lsblk", "-ino", attr, str(dev)]
         return subprocess_check_output(cmd, raise_exception=raise_exception)
@@ -62,10 +76,20 @@ def get_attrs_by_dev(
     def get_dev_by_token(
         cls, token: PartitionToken, value: str, *, raise_exception: bool = True
     ) -> Optional[list[str]]:
-        """Get a list of device(s) that matches the token=value pair.
+        """Get a list of device(s) that matches the <token>=<value> pair.
 
         This is implemented by calling:
             blkid -o device -t <TOKEN>=<VALUE>
+
+        Args:
+            token (PartitionToken): which attribute of device to match.
+            value (str): the value of the attribute.
+            raise_exception (bool, optional): raise exception on subprocess call failed.
+                Defaults to True.
+
+        Returns:
+            Optional[list[str]]: If there is at least one device found, return a list
+                contains all found device(s), otherwise None.
         """
         cmd = ["blkid", "-o", "device", "-t", f"{token}={value}"]
         if res := subprocess_check_output(cmd, raise_exception=raise_exception):
@@ -76,13 +100,14 @@ def get_current_rootfs_dev(cls, *, raise_exception: bool = True) -> str:
         """Get the devpath of current rootfs dev.
 
         This is implemented by calling
-            findmnt -nfc -o SOURCE /
+            findmnt -nfc -o SOURCE <ACTIVE_ROOTFS_PATH>
 
-        Returns:
-            full path to dev of the current rootfs.
+        Args:
+            raise_exception (bool, optional): raise exception on subprocess call failed.
+                Defaults to True.
 
-        Raises:
-            subprocess.CalledProcessError on failed command call.
+        Returns:
+            str: the devpath of current rootfs device.
         """
         cmd = ["findmnt", "-nfco", "SOURCE", cfg.ACTIVE_ROOTFS_PATH]
         return subprocess_check_output(cmd, raise_exception=raise_exception)
@@ -94,8 +119,16 @@ def get_mount_point_by_dev(cls, dev: str, *, raise_exception: bool = True) -> st
         This is implemented by calling:
             findmnt <dev> -nfo TARGET <dev>
 
-        NOTE: findmnt raw result might have multiple lines if target dev is bind mounted.
-            use option -f to only show the first file system.
+        NOTE: option -f is used to only show the first file system.
+
+        Args:
+            dev (str): the device to check against.
+            raise_exception (bool, optional): raise exception on subprocess call failed.
+                Defaults to True.
+
+        Returns:
+            str: the FIRST mountpint of the <dev>, or empty string if <raise_exception> is False
+                and the subprocess call failed(due to dev is not mounted or other reasons).
         """
         cmd = ["findmnt", "-nfo", "TARGET", dev]
         return subprocess_check_output(cmd, raise_exception=raise_exception)
@@ -104,10 +137,18 @@ def get_mount_point_by_dev(cls, dev: str, *, raise_exception: bool = True) -> st
     def get_dev_by_mount_point(
         cls, mount_point: str, *, raise_exception: bool = True
     ) -> str:
-        """Return the underlying mounted dev of the given mount_point.
+        """Return the source dev of the given <mount_point>.
 
         This is implemented by calling:
             findmnt -no SOURCE <mount_point>
+
+        Args:
+            mount_point (str): mount_point to check against.
+            raise_exception (bool, optional): raise exception on subprocess call failed.
+                Defaults to True.
+
+        Returns:
+            str: the source device of <mount_point>.
         """
         cmd = ["findmnt", "-no", "SOURCE", mount_point]
         return subprocess_check_output(cmd, raise_exception=raise_exception)
@@ -121,7 +162,13 @@ def is_target_mounted(
         This is implemented by calling:
             findmnt <target>
 
-            and see if there is any matching device.
+        Args:
+            target (Path | str): the target to check against. Could be a device or a mount point.
+            raise_exception (bool, optional): raise exception on subprocess call failed.
+                Defaults to True.
+
+        Returns:
+            bool: return True if the target has at least one mount_point.
         """
         cmd = ["findmnt", target]
         return bool(subprocess_check_output(cmd, raise_exception=raise_exception))
@@ -133,29 +180,54 @@ def get_parent_dev(cls, child_device: str, *, raise_exception: bool = True) -> s
 
         This function is implemented by calling:
             lsblk -idpno PKNAME <child_device>
+
+        Args:
+            child_device (str): the device to find parent device from.
+            raise_exception (bool, optional): raise exception on subprocess call failed.
+                Defaults to True.
+
+        Returns:
+            str: the parent device of the specific <child_device>.
         """
         cmd = ["lsblk", "-idpno", "PKNAME", child_device]
         return subprocess_check_output(cmd, raise_exception=raise_exception)
 
     @classmethod
     def set_ext4_fslabel(cls, dev: str, fslabel: str, *, raise_exception: bool = True):
+        """Set <fslabel> to ext4 formatted <dev>.
+
+        This is implemented by calling:
+            e2label <dev> <fslabel>
+
+        Args:
+            dev (str): the ext4 partition device.
+            fslabel (str): the fslabel to be set.
+            raise_exception (bool, optional): raise exception on subprocess call failed.
+                Defaults to True.
+        """
         cmd = ["e2label", dev, fslabel]
         subprocess_call(cmd, raise_exception=raise_exception)
 
     @classmethod
     def mount_rw(
         cls, target: str, mount_point: Path | str, *, raise_exception: bool = True
     ):
-        """Mount the target to the mount_point read-write.
+        """Mount the <target> to <mount_point> read-write.
+
+        This is implemented by calling:
+            mount -o rw --make-private --make-unbindable <target> <mount_point>
 
         This is implemented by calling:
             mount -o rw --make-private --make-unbindable <target> <mount_point>
 
         NOTE: pass args = ["--make-private", "--make-unbindable"] to prevent
               mount events propagation to/from this mount point.
 
-        Raises:
-            CalledProcessError on failed mount.
+        Args:
+            target (str): target to be mounted.
+            mount_point (Path | str): mount point to mount to.
+            raise_exception (bool, optional): raise exception on subprocess call failed.
+                Defaults to True.
         """
         # fmt: off
         cmd = [
@@ -172,13 +244,16 @@ def mount_rw(
     def bind_mount_ro(
         cls, target: str, mount_point: Path | str, *, raise_exception: bool = True
     ):
-        """Bind mount the target to the mount_point read-only.
+        """Bind mount the <target> to <mount_point> read-only.
 
         This is implemented by calling:
             mount -o bind,ro --make-private --make-unbindable <target> <mount_point>
 
-        Raises:
-            CalledProcessError on failed mount.
+        Args:
+            target (str): target to be mounted.
+            mount_point (Path | str): mount point to mount to.
+            raise_exception (bool, optional): raise exception on subprocess call failed.
+                Defaults to True.
         """
         # fmt: off
         cmd = [
@@ -193,9 +268,18 @@ def bind_mount_ro(
 
     @classmethod
     def umount(cls, target: Path | str, *, raise_exception: bool = True):
-        """Try to unmount the <target>.
+        """Try to umount the <target>.
 
-        This function will first check whether the <target> is mounted.
+        This is implemented by calling:
+            umount <target>
+
+        Before calling umount, the <target> will be check whether it is mounted,
+            if it is not mounted, this function will return directly.
+
+        Args:
+            target (Path | str): target to be umounted.
+            raise_exception (bool, optional): raise exception on subprocess call failed.
+                Defaults to True.
         """
         # first try to check whether the target(either a mount point or a dev)
         # is mounted
@@ -215,15 +299,17 @@ def mkfs_ext4(
         fsuuid: Optional[str] = None,
         raise_exception: bool = True,
     ):
-        """Call mkfs.ext4 on <dev>.
-
-        If fslabel and/or fsuuid provided, use them in prior, otherwise
-            try to preserve fsuuid and fslabel if possible.
-
-        NOTE: -F is used to bypass interactive confirmation.
-
-        Raises:
-            Original CalledProcessError from the failed command execution.
+        """Create new ext4 formatted filesystem on <dev>, optionally with <fslabel>
+            and/or <fsuuid>.
+
+        Args:
+            dev (str): device to be formatted to ext4.
+            fslabel (Optional[str], optional): fslabel of the new ext4 filesystem. Defaults to None.
+                When it is None, this function will try to preserve the previous fslabel.
+            fsuuid (Optional[str], optional): fsuuid of the new ext4 filesystem. Defaults to None.
+                When it is None, this function will try to preserve the previous fsuuid.
+            raise_exception (bool, optional): raise exception on subprocess call failed.
+                Defaults to True.
         """
         cmd = ["mkfs.ext4", "-F"]
 
@@ -250,23 +336,23 @@ def mkfs_ext4(
             cmd.extend(["-L", fslabel])
 
         cmd.append(dev)
-        logger.warning(f"execute {cmd=}")
+        logger.warning(f"format {dev} to ext4: {cmd=}")
         subprocess_call(cmd, raise_exception=raise_exception)
 
     @classmethod
     def mount_ro(
         cls, *, target: str, mount_point: str | Path, raise_exception: bool = True
     ):
-        """Mount target on mount_point read-only.
+        """Mount <target> to <mount_point> read-only.
 
-        If the target device is mounted, we bind mount the target device to mount_point,
+        If the target device is mounted, we bind mount the target device to mount_point.
         if the target device is not mounted, we directly mount it to the mount_point.
 
-        This method mount the target as ro with make-private flag and make-unbindable flag,
-        to prevent ANY accidental writes/changes to the target.
-
-        Raises:
-            CalledProcessError on failed mounting.
+        Args:
+            target (str): target to be mounted.
+            mount_point (str | Path): mount point to mount to.
+            raise_exception (bool, optional): raise exception on subprocess call failed.
+                Defaults to True.
         """
         # NOTE: set raise_exception to false to allow not mounted
         #       not mounted dev will have empty return str
@@ -293,10 +379,17 @@ def mount_ro(
 
     @classmethod
     def reboot(cls, args: Optional[list[str]] = None) -> NoReturn:
-        """Reboot the whole system otaclient running at and terminate otaclient.
+        """Reboot the system, with optional args passed to reboot command.
+
+        This is implemented by calling:
+            reboot [args[0], args[1], ...]
 
-        NOTE(20230614): this command MUST also make otaclient exit immediately.
+        NOTE(20230614): this command makes otaclient exit immediately.
         NOTE(20240421): rpi_boot's reboot takes args.
+
+        Args:
+            args (Optional[list[str]], optional): args passed to reboot command.
+                Defaults to None, not passing any args.
         """
         cmd = ["reboot"]
         if args:

otaclient/app/common.py

@@ -125,6 +125,20 @@ def subprocess_run_wrapper(
     check_output: bool,
     timeout: Optional[float] = None,
 ) -> subprocess.CompletedProcess[bytes]:
+    """A wrapper for subprocess.run method.
+
+    NOTE: this is for the requirement of customized subprocess call
+        in the future, like chroot or nsenter before execution.
+
+    Args:
+        cmd (str | list[str]): command to be executed.
+        check (bool): if True, raise CalledProcessError on non 0 return code.
+        check_output (bool): if True, the UTF-8 decoded stdout will be returned.
+        timeout (Optional[float], optional): timeout for execution. Defaults to None.
+
+    Returns:
+        subprocess.CompletedProcess[bytes]: the result of the execution.
+    """
     if isinstance(cmd, str):
         cmd = shlex.split(cmd)
 
@@ -144,18 +158,17 @@ def subprocess_check_output(
     default: str = "",
     timeout: Optional[float] = None,
 ) -> str:
-    """Run the command and return its output if possible.
-    NOTE: this method will call decode and strip on the raw output.
+    """Run the <cmd> and return UTF-8 decoded stripped stdout.
 
-    Params:
-        cmd: string or list of string of command to be called.
-        raise_exception: Whether to raise the exception from
-            underlaying subprocess.check_output.
-        default: when <raise_exception> is True, return this <default>.
+    Args:
+        cmd (str | list[str]): command to be executed.
+        raise_exception (bool, optional): raise the underlying CalledProcessError. Defaults to False.
+        default (str, optional): if <raise_exception> is False, return <default> on underlying
+            subprocess call failed. Defaults to "".
+        timeout (Optional[float], optional): timeout for execution. Defaults to None.
 
-    Raises:
-        The original CalledProcessError from calling subprocess.check_output if
-            the execution failed and <raise_exception> is True.
+    Returns:
+        str: UTF-8 decoded stripped stdout.
     """
     try:
         res = subprocess_run_wrapper(
@@ -180,11 +193,12 @@ def subprocess_call(
     raise_exception: bool = False,
     timeout: Optional[float] = None,
 ) -> None:
-    """Run the <cmd> without checking its output.
+    """Run the <cmd>.
 
-    Raises:
-        The original CalledProcessError from calling subprocess.check_output if
-            the execution failed and <raise_exception> is True.
+    Args:
+        cmd (str | list[str]): command to be executed.
+        raise_exception (bool, optional): raise the underlying CalledProcessError. Defaults to False.
+        timeout (Optional[float], optional): timeout for execution. Defaults to None.
     """
     try:
         subprocess_run_wrapper(cmd, check=True, check_output=False, timeout=timeout)
@@ -549,7 +563,7 @@ def shutdown(self, *, raise_last_exc: bool):
             logger.debug("shutdown retry task map")
             if self._running_inst:
                 self._running_inst.shutdown(raise_last_exc=raise_last_exc)
-            # NOTE: passthrough the exception from underlaying running_inst
+            # NOTE: passthrough the exception from underlying running_inst
         finally:
             self._running_inst = None
             self._executor.shutdown(wait=True)

otaclient/app/proto/README.md

@@ -143,7 +143,7 @@ After the compiled python code is ready, we can define the wrappers accordingly
         #   type hints for each attributes when manually create instances
         #   of wrapper types.
         #   NOTE: this __init__ is just for typing use, it will not be
-        #         used by the underlaying MessageWrapper
+        #         used by the underlying MessageWrapper
         #   NOTE: if pyi file is also generated when compiling the proto file,
         #         the __init__ in pyi file can be used directly with little adjustment
         def __init__(