[Refinement] boot_control.grub: improve robustness against boot load/init, split out ota_status control and slot mount logic #257

Bodong-Yang · 2023-09-06T08:35:47Z

Description

This PR introduces several internal refinements to grub boot_control implementation as follow. With this PR, grub boot_control implementation aligns with grub boot control design documentation.

Robustness improved over boot control load/init

This PR refines the grub boot_control procedures, aligns the behavior with the design document. Comparing to the previous implementation, it handles the condition of ota-partition symlink broken and migration from non-ota-partition booted system properly during boot_control loading, please check Behavior changes for more details.

Also, during finalizing switch boot, previously otaclient will do the ota-partition symlink switching before doing grub-update, this potentially breaks the atomic switching boot. This PR fixes this issue.

`OTAStatusFilesControl` and `SlotMountHelper`

These two classes are common shared helper classes that implement ota_status files control and slots preparing/mounting, which can be used by any detailed boot_control implementation(previously only used by rpi_boot, this PR make grub_boot also use these classes). They are introduced since #182 .

The goal for introducing these classes is to abstract ota_status files control and slots preparing/mounting from detailed boot_control implementation, improve boot_control implementation's cohesive, reduce code duplication and ease maintaining.
Especially, the OTAStatusFilesControl class aligns with ota_status control design.

Split out ota_status files control and slot preparing/mounting from boot_control implementation

This PR splits and removes the ota_status files control logic from grub boot_control implementation, using OTAStatusFilesControl and SlotMountHelper instead of mixins combination (OTAStatusMixin, VersionControlMixin, PrepareMountMixin, these mixins are deprecated in flavor of OTAStatusFilesControl and SlotMountHelper classes).

Now grub boot_control doesn't implement the ota_status files control and slot preparing/mounting, but using APIs from OTAStatusFilesControl and SlotMountHelper, making grub boot_control implementation more focuses on grub control logic, reduce code duplication and ease the maintaining.

Check list

related documents are provided.
test file(s) that covers the change(s) is implemented.
local test is passed.
VM test: boot migrate from non-ota-partition booted system test.
VM test: try to fix ota-partition symlink missing/corrupted.
VM test: normal OTA.

Changes

common.OTAStatusFilesControl: split loading and parsing status and slot_in_use file into two methods,
common.OTAStatusFilesControl: takes force_initialize param in __init__, ota_status will initialize if boot_control is initialized,
common.SlotMountHelper: standby_boot_dir is always <standby_slot_mp>/boot, standby_slot creator no-longer needs to care about where the kernel/initrd.img should be placed, this will be handled by boot_controller,
protocol: rename get_ota_status to get_booted_ota_status to distinguish this ota_status with live_ota_status maintained by otaclient,
grub_boot: remove _SymlinkABPartitionDetecter, only detect AB partition by partition information(GrubABPartitionDetecter),
grub_boot: use OTAStatusFilesControl to handle ota_status files control, switch boot detecting, etc, remove superseded/overridden codes,
grub_boot: use SlotMountHelper to handle slots preparing/mounting in each update phases, remove superseded/overridden codes,
grub_boot: refine boot loading procedures, checking the present of ota-partition files during loading, handles the case of ota-partition symlink corrupted, in such case a full boot_control initializing will engage and all ota-partition files will be recreated,
grub_boot: indicates OTAStatusFilesControl to initialize unconditionally if boot_control is initialized,
grub_boot: _get_current_booted_kernel_and_initrd now use standard way to detect booted initrd.img,
grub_boot: _prepare_kernel_initrd_links now uses first found kernel, and try to find the corresponding initrd.img with the same version string,
grub_boot: split ensure_ota_partition_symlinks into two new methods _ensure_ota_partition_symlinks and _ensure_standby_slot_boot_files_symlinks,
grub_boot: remove reprepare_standby_ota_partition_file and reprepare_active_ota_partition_file methods, now grub_reboot_to_standby and finalizing_switch_boot implements boot control with helper methods on their own,
grub_boot: grub_reboot_to_standby and finalizing_switch_boot now ensure the present of all ota-partition symlinks before grub-update,
grub_boot: finalizing_switch_boot now does switch boot after grub-update, before grub-update all ota-partition symlinks will be ensured,
rpi_boot: change according to boot_control.ota_status -> boot_control.booted_ota_status rename,
config: add DEFAULT_VERSION_STR, value is empty string to align with previous implementation,
ota_client: change according to boot_control.get_ota_status -> boot_control.get_booted_ota_status API rename,
fix up test_grub to make it work again,
add new test_ota_status_control,
minor fix up to test_rpi_boot and test_ota_client.

Behavior changes

Does this PR introduce behavior change(s)?

Yes, internal behaivor (will not impact user experience).

Previous behavior

otaclient will crash on ota-partition symlink missing/corrupted, even the system is booted normally.
otaclient cannot correctly migrates non-ota-partition booted system in some cases.
grub boot_control implements the detailed ota_status files control, including reading/writing/updating ota_status files during each OTA stages, switch boot mode detection, ota_status transition, etc.
grub boot_control prepares slots devices and mounting slots directly using commands provided by CMDHelperFuncs.
during finalizing switch boot, otaclient does ota-partition symlink switch before doing grub-update.
in _prepare_kernel_initrd_links, grub boot_control will use the first bound kernel and first found initrd.img, but these two files might have different version.

Behavior with this PR

otaclient now will not crash on ota-partition symlink missing, but instead try to fix it first or migrate.
otaclient now can properly migrate from non-ota-partition booted system.
grub boot_control no-longer directly managing ota_status files, but calling corresponding APIs fromOTAStatusFilesControl during each OTA stages.
grub boot_control no-longer directly managing slots mounting/preparing, but calling corresponding APIs from SlotMountHelper during each OTA stages.
behavior of finalizing switch boot now aligns with documentation, which otaclient will do grub-update before switching ota-partition symlink.
in _prepare_kernel_initrd_links, grub boot control will use the first found kernel, and use its version string to find corresponding initrd.img.

Breaking change

Does this PR introduce breaking change?

No.

…e_sync

…initrd.img

…y itself instead of standby slot creator doing so

…symlink now

Bodong-Yang · 2023-09-13T08:13:00Z

@obi-t4 Some changes are required due to update to the documentation, so I will turn the PR to draft for now.

… initrd.img are with the same version

…al vars

…s_symlinks now takes params

…ymlinks

… anymore

obi-t4

I'm yet reviewing but did you confirm that this is working on the USB installer environment?

obi-t4 · 2023-09-13T08:17:25Z

otaclient/app/boot_control/configs.py

+    GRUB_CFG_FNAME: str = "grub.cfg"
    GRUB_CFG_PATH: str = "/boot/grub/grub.cfg"


Suggested change

GRUB_CFG_FNAME: str = "grub.cfg"

GRUB_CFG_PATH: str = "/boot/grub/grub.cfg"

GRUB_CFG_FNAME: str = "grub.cfg"

GRUB_CFG_PATH: str = f"/boot/grub/{GRUB_CFG_FNAME}"

And fix spelling inconsistencies: FNAME and FILE

GRUB_CFG_FNAME .. BOOT_OTA_PARTITION_FILE

@obi-t4 Ah about the constant naming, currently I am using the following rules:

FNAME means the constant only contains the file name,

FPATH means the constant contains the file full path,

A single FILE is not good, I will prepare another PR to update the configs constant naming.

Bodong-Yang · 2023-09-21T01:59:18Z

I'm yet reviewing but did you confirm that this is working on the USB installer environment?

@obi-t4 Yes, otaclient grub boot controller supports automatically migrating a normal ubuntu installation to use ota-partition boot mechanism(and it will take effect starts from next reboot).

Bodong-Yang · 2023-09-21T02:06:11Z

( More precisely is, otaclient grub boot controller supports automatically migrating any system booted with general grub installation(booted with /boot/grub/grub.cfg, /boot/vmlinuz and /boot/initrd.img) with A/B partition presented to use ota-partition boot mechanism.

obi-t4

I left some comments, please have a check.

obi-t4 · 2023-09-21T04:44:29Z

otaclient/app/boot_control/_grub.py

-    def get_active_slot(self) -> str:
-        return self.active_slot
+    def __init__(self) -> None:
+        ab_detecter = GrubABPartitionDetecter()


the correct spelling is detector.

Suggested change

ab_detecter = GrubABPartitionDetecter()

ab_detector = GrubABPartitionDetector()

@Bodong-Yang Do you have a plan to fix this spelling in this PR?

@obi-t4 Ah sorry I am waiting for the test result from your side. After test is completed I will resolve all the comments at once.

Updated and fixed at 48c06ac.

obi-t4 · 2023-09-21T05:20:47Z

otaclient/app/boot_control/_grub.py

+        self._prepare_kernel_initrd_links(self.active_ota_partition_folder)
+        self._ensure_ota_partition_symlinks(active_slot=self.active_slot)


I'd like to know why these two lines are needed?

These two methods are to ensure the required symlinks are presented at the time just before grub-reboot is called.

If required symlinks are presented, these two methods won't do others things, otherwise they will re-generate the missing symlinks.

FYI, the re_symlink_atomic method used by these two methods will check the symlink before actually creating symlink again. So if the symlink is presented and already points to correct target, this method will do nothing.

Additional comments for this method are added at ebb0c4c.

otaclient/app/boot_control/_grub.py

obi-t4 · 2023-09-21T07:47:18Z

@Bodong-Yang
I'll approve this PR if USB installer is OK. Image building w/ this PR has been started.

obi-t4 · 2023-09-22T06:22:54Z

NOTE:
20230921-otaclient-grub-robust (USB install)
-> 20220621083828
-> 20221007121134-20.04-otaclient-v3.1.6
-> 20230921-otaclient-grub-robust

obi-t4 · 2023-09-22T07:15:19Z

@Bodong-Yang Test is working as expected!

Bodong-Yang · 2023-09-22T07:16:34Z

@Bodong-Yang Test is working as expected!

@obi-t4 I will resolve all the pending PR comments now, please wait a moment.

Updated: Done.

obi-t4

LGTM 👍

yn-mrse · 2023-12-19T02:27:18Z

https://github.com/tier4/autoware_ecu_system_setup/pull/817 発行のタイミングでOTA image作成が実施されており、正常動作確認はパスしている。
このPRとしてのパーティション等復旧機能自体はユニットテストを全てパスしている。

Bodong-Yang added 17 commits August 31, 2023 04:36

simplify GrubABPDetecter, simplify SymlinkABPDetecter to a function

b8b40b9

grub: use OTAStatusFilesControl and SlotMountHelper instead

4c65c27

grub: GrubControl now always check ota-partition files on startup

ecc5c52

boot_control.common: minor update, define default version str

9a68dda

boot_control.common: split load_ota_status_files into 2 methods

d2e1d3b

grub: _grub_update_for_active_slot should always use write_str_to_fil…

cc4fae3

…e_sync

grub: symlink to grub.cfg now prepared by _ensure_ota_partition_symlinks

7e9ede9

grub: refine check_active_slot_ota_partition_file

c30cd74

minor update

c62e291

grub: refine _update_fstab

ee7076d

OTAStatusControl: add force_initialize

e5f1a6d

grub: remove unused

10c7ba7

grub: still make symlinks before update grub

417b6c9

grub: get_current_booted_files now use standard way to detect booted …

24c4cc4

…initrd.img

grub: now grub control can recover from ota-partition symlink missing

9f5e296

common: SlotMountHelper now takes standby_boot_dir param

b44ee1a

grub, rpi_boot: update according to SlotMountHelper's changes

67ad942

Bodong-Yang added the refinement Improve the performance, code quality(like improving code structure/readability/error handling/robus label Sep 6, 2023

Bodong-Yang added 12 commits September 7, 2023 03:16

boot_control_protocol: deprecate get_standby_boot_dir

4473516

common: revert adding standby_boot_dir param in SlotMountHelper init

d580a62

rpi_boot: revert changes

1203c97

grub: remove standby_boot_dir param when init SlotMountHelper

edad48a

grub: now grub controller copies boot files to ota-partition folder b…

50b13d8

…y itself instead of standby slot creator doing so

grub: logging the loaded original status and slot_in_use

57e229f

grub: fix copy_boot_files_from_standby_slot

6f5ae0f

grub: fix _check_active_slot_ota_partition_file

b82c6d9

grub: _check_active_slot_ota_partition_file also check ota-partition …

b27a21a

…symlink now

fix up test_grub

39e6b9f

test_grub: minor fix

02e7285

minor fix to test_grub

d5e78bd

Merge branch 'main' into refine/grub_robust

290a0b0

Bodong-Yang marked this pull request as draft September 13, 2023 08:12

Bodong-Yang added 8 commits September 14, 2023 04:45

grub: fix _prepare_kernel_initrd_links_for_ota, now ensure kernel and…

88ba123

… initrd.img are with the same version

grub: for _grub_update, change some attrs in _GrubControl to func loc…

12c5fa3

…al vars

grub: ensure_ota_partition_symlinks and ensure_standby_slot_boot_file…

85acd39

…s_symlinks now takes params

grub: grub_update_on_booted_slot now also validate active boot file s…

c30c45f

…ymlinks

grub: update boot control init

1f1aab7

grub: aligns grub_reboot and finalize_switch_boot behavior as document

b9b8a45

grub.GrubController: finalize_update_switch_boot doesn't need partial…

a6fc838

… anymore

grub: minor updates

9b165fd

Bodong-Yang marked this pull request as ready for review September 19, 2023 02:46

obi-t4 reviewed Sep 20, 2023

View reviewed changes

obi-t4 reviewed Sep 21, 2023

View reviewed changes

otaclient/app/boot_control/_grub.py Show resolved Hide resolved

Bodong-Yang added 3 commits September 22, 2023 07:19

grub: fix typo detecter -> detector

48c06ac

grub: add comments fro grub_reboot_to_standby method

ebb0c4c

fix test_grub

ccf67cc

Bodong-Yang enabled auto-merge (squash) September 22, 2023 07:37

obi-t4 approved these changes Sep 22, 2023

View reviewed changes

Bodong-Yang merged commit 1403768 into main Sep 22, 2023
3 checks passed

Bodong-Yang deleted the refine/grub_robust branch September 22, 2023 08:04

Bodong-Yang mentioned this pull request Nov 10, 2023

[Chore] tag v3.6.1 (20231108) #260

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Refinement] boot_control.grub: improve robustness against boot load/init, split out ota_status control and slot mount logic #257

[Refinement] boot_control.grub: improve robustness against boot load/init, split out ota_status control and slot mount logic #257

Bodong-Yang commented Sep 6, 2023 •

edited

Loading

Bodong-Yang commented Sep 13, 2023

obi-t4 left a comment •

edited

Loading

obi-t4 Sep 13, 2023

obi-t4 Sep 13, 2023

Bodong-Yang Sep 21, 2023 •

edited

Loading

Bodong-Yang commented Sep 21, 2023 •

edited

Loading

Bodong-Yang commented Sep 21, 2023 •

edited

Loading

obi-t4 left a comment

obi-t4 Sep 21, 2023 •

edited

Loading

obi-t4 Sep 22, 2023

Bodong-Yang Sep 22, 2023

Bodong-Yang Sep 22, 2023

obi-t4 Sep 21, 2023

Bodong-Yang Sep 21, 2023

Bodong-Yang Sep 21, 2023

Bodong-Yang Sep 22, 2023

obi-t4 commented Sep 21, 2023 •

edited

Loading

obi-t4 commented Sep 22, 2023

obi-t4 commented Sep 22, 2023

Bodong-Yang commented Sep 22, 2023 •

edited

Loading

obi-t4 left a comment

yn-mrse commented Dec 19, 2023

		GRUB_CFG_FNAME: str = "grub.cfg"
		GRUB_CFG_PATH: str = "/boot/grub/grub.cfg"

	ab_detecter = GrubABPartitionDetecter()
	ab_detector = GrubABPartitionDetector()

		self._prepare_kernel_initrd_links(self.active_ota_partition_folder)
		self._ensure_ota_partition_symlinks(active_slot=self.active_slot)

[Refinement] boot_control.grub: improve robustness against boot load/init, split out ota_status control and slot mount logic #257

[Refinement] boot_control.grub: improve robustness against boot load/init, split out ota_status control and slot mount logic #257

Conversation

Bodong-Yang commented Sep 6, 2023 • edited Loading

Description

Robustness improved over boot control load/init

OTAStatusFilesControl and SlotMountHelper

Split out ota_status files control and slot preparing/mounting from boot_control implementation

Related documents

Check list

Changes

Behavior changes

Previous behavior

Behavior with this PR

Breaking change

Bodong-Yang commented Sep 13, 2023

obi-t4 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Bodong-Yang Sep 21, 2023 • edited Loading

Choose a reason for hiding this comment

Bodong-Yang commented Sep 21, 2023 • edited Loading

Bodong-Yang commented Sep 21, 2023 • edited Loading

obi-t4 left a comment

Choose a reason for hiding this comment

obi-t4 Sep 21, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

obi-t4 commented Sep 21, 2023 • edited Loading

obi-t4 commented Sep 22, 2023

obi-t4 commented Sep 22, 2023

Bodong-Yang commented Sep 22, 2023 • edited Loading

obi-t4 left a comment

Choose a reason for hiding this comment

yn-mrse commented Dec 19, 2023

Bodong-Yang commented Sep 6, 2023 •

edited

Loading

`OTAStatusFilesControl` and `SlotMountHelper`

obi-t4 left a comment •

edited

Loading

Bodong-Yang Sep 21, 2023 •

edited

Loading

Bodong-Yang commented Sep 21, 2023 •

edited

Loading

Bodong-Yang commented Sep 21, 2023 •

edited

Loading

obi-t4 Sep 21, 2023 •

edited

Loading

obi-t4 commented Sep 21, 2023 •

edited

Loading

Bodong-Yang commented Sep 22, 2023 •

edited

Loading