* [PATCH v3 4/4] ci: test partition layout of splitted-rootfs image @ 2022-05-12 12:15 Felix Moessbauer 2022-05-23 14:19 ` Anton Mikanovich 0 siblings, 1 reply; 3+ messages in thread From: Felix Moessbauer @ 2022-05-12 12:15 UTC (permalink / raw) To: isar-users; +Cc: henning.schild, jan.kiszka, florian.bezdeka, Felix Moessbauer This adds a test that checks if splitting the rootfs across multiple partitions correctly works. The following is checked: file-permissions (based on wic internal check): WIC already has a built-in check for correct file permissions. In case the permission db is missing, a WIC warning is emitted. We simply check for this warning. partition layout and excluded paths: Specifying exclude paths is error prone. Hence, we check that all generated partitions do not contain excluded paths. Further, we check that the expected paths are there. Signed-off-by: Felix Moessbauer <felix.moessbauer@siemens.com> --- testsuite/cibase.py | 58 ++++++++++++++++++++++++++++++++++++++++++ testsuite/cibuilder.py | 6 ++++- testsuite/citest.py | 14 ++++++++++ 3 files changed, 77 insertions(+), 1 deletion(-) diff --git a/testsuite/cibase.py b/testsuite/cibase.py index b25c356e..fd272ef5 100755 --- a/testsuite/cibase.py +++ b/testsuite/cibase.py @@ -164,3 +164,61 @@ class CIBaseTest(CIBuilder): ['do_rootfs_install_setscene', '!do_rootfs_install']) ]): self.fail("Failed rebuild package and image") + + def perform_partition_layout_test(self, mc_target, **kwargs): + def _mount(device, mountpoint): + process.run(f'mount -o ro {device} {mountpoint}', sudo=True, ignore_status=False) + + def _umount(mountpoint): + process.run(f'umount -l {mountpoint}', sudo=True, ignore_status=True) + + def check_output_for_warnings(target, machine): + wic_output_files = glob.glob(f'{self.build_dir}/tmp/work/*/{target}-{machine}/*/temp/log.do_image_wic') + if len(wic_output_files) == 0: + self.fail('could not find WIC output file') + with open(wic_output_files[0], 'r') as output: + # in case we had permission problems, a WIC warning is in the logs + for line in output: + token = line.split() + if len(token) > 0 and token[0] == 'WARNING:': + self.fail(f'WIC issue found: {line}') + + def check_part_layout(target, machine): + pattern = f'{self.build_dir}/tmp/deploy/images/{machine}/{target}-*-{machine}.wic.p*' + partitions = sorted(glob.glob(pattern)) + if len(partitions) != 3: + self.fail(f'expected 3 partitions, but got {len(partitions)} (in {pattern})') + mountpoints = [os.path.join(self.build_dir, mp) for mp in 'mnt_efi mnt_root mnt_home'.split()] + for i in range(3): + os.makedirs(mountpoints[i], exist_ok=True) + _umount(mountpoints[i]) + _mount(partitions[i], mountpoints[i]) + + # in boot partition, we expect /boot + if not os.path.isdir(os.path.join(mountpoints[0], 'EFI')): + self.fail('boot partition does not provide /EFI') + # in root partition, boot and home should be excluded + if not os.path.isdir(os.path.join(mountpoints[1], 'etc')) or \ + os.path.isdir(os.path.join(mountpoints[1], 'home')) or \ + os.path.isdir(os.path.join(mountpoints[1], 'boot')): + self.fail('root partition does not contain expected dirs') + # home partition should contain home of user "user" + if not os.path.isdir(os.path.join(mountpoints[2], 'user')): + self.fail('home partition does not contain home of user') + + [_umount(mnt) for mnt in mountpoints] + + mc_spec = mc_target.split(':') + target = mc_spec[2] + machine = mc_spec[1].split('-')[0] + + self.configure( + sstate=False, + compat_arch=False, + interactive_user=True, + **kwargs) + + self.bitbake(mc_target, **kwargs) + + check_output_for_warnings(target, machine) + check_part_layout(target, machine) diff --git a/testsuite/cibuilder.py b/testsuite/cibuilder.py index bc48d47f..e3fbb859 100755 --- a/testsuite/cibuilder.py +++ b/testsuite/cibuilder.py @@ -54,7 +54,7 @@ class CIBuilder(Test): def configure(self, compat_arch=True, cross=None, debsrc_cache=False, container=False, ccache=False, sstate=False, offline=False, - gpg_pub_key=None, **kwargs): + gpg_pub_key=None, interactive_user=False, **kwargs): # write configuration file and set bitbake_args # can run multiple times per test case self.check_init() @@ -107,6 +107,10 @@ class CIBuilder(Test): f.write('BASE_REPO_KEY="file://' + gpg_pub_key + '"\n') if distro_apt_premir: f.write('DISTRO_APT_PREMIRRORS = "%s"\n' % distro_apt_premir) + if interactive_user: + f.write('USERS += "user"\n') + f.write('USER_user[home] = "/home/user"\n') + f.write('USER_user[flags] = "create-home"\n') # include ci_build.conf in local.conf with open(self.build_dir + '/conf/local.conf', 'r+') as f: diff --git a/testsuite/citest.py b/testsuite/citest.py index 8d00054a..26101010 100755 --- a/testsuite/citest.py +++ b/testsuite/citest.py @@ -318,3 +318,17 @@ class VmBootTestFull(CIBaseTest): def test_amd64_focal(self): self.init() self.vm_start('amd64','focal') + +class SplittedRootfsTest(CIBaseTest): + + """ + Test partition layout of splitted rootfs image + + :avocado: tags=splittedrootfs,full + """ + + def test_wic_partition_layout(self): + mc_target = 'mc:qemuamd64-bookworm:isar-image-base' + + self.init('build-wic-rootfs') + self.perform_partition_layout_test(mc_target) -- 2.30.2 ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v3 4/4] ci: test partition layout of splitted-rootfs image 2022-05-12 12:15 [PATCH v3 4/4] ci: test partition layout of splitted-rootfs image Felix Moessbauer @ 2022-05-23 14:19 ` Anton Mikanovich 2022-05-28 14:32 ` Moessbauer, Felix 0 siblings, 1 reply; 3+ messages in thread From: Anton Mikanovich @ 2022-05-23 14:19 UTC (permalink / raw) To: Felix Moessbauer, isar-users; +Cc: henning.schild, jan.kiszka, florian.bezdeka 12.05.2022 15:15, Felix Moessbauer wrote: > This adds a test that checks if splitting the rootfs > across multiple partitions correctly works. > > The following is checked: > > file-permissions (based on wic internal check): > WIC already has a built-in check for correct > file permissions. In case the permission db is missing, > a WIC warning is emitted. We simply check for this warning. > > partition layout and excluded paths: > Specifying exclude paths is error prone. > Hence, we check that all generated partitions do not contain > excluded paths. Further, we check that the expected paths are there. > > Signed-off-by: Felix Moessbauer <felix.moessbauer@siemens.com> > --- > testsuite/cibase.py | 58 ++++++++++++++++++++++++++++++++++++++++++ > testsuite/cibuilder.py | 6 ++++- > testsuite/citest.py | 14 ++++++++++ > 3 files changed, 77 insertions(+), 1 deletion(-) > > diff --git a/testsuite/cibase.py b/testsuite/cibase.py > index b25c356e..fd272ef5 100755 > --- a/testsuite/cibase.py > +++ b/testsuite/cibase.py > @@ -164,3 +164,61 @@ class CIBaseTest(CIBuilder): > ['do_rootfs_install_setscene', '!do_rootfs_install']) > ]): > self.fail("Failed rebuild package and image") > + > + def perform_partition_layout_test(self, mc_target, **kwargs): > + def _mount(device, mountpoint): > + process.run(f'mount -o ro {device} {mountpoint}', sudo=True, ignore_status=False) > + > + def _umount(mountpoint): > + process.run(f'umount -l {mountpoint}', sudo=True, ignore_status=True) Do not introduce new lazy umounts, please. If umount fails we need to fix the reason, but do not mask it. You can maybe use avocado.utils.partition.Partition class for all mount, umount and mountpoint check operations. > + > + def check_output_for_warnings(target, machine): > + wic_output_files = glob.glob(f'{self.build_dir}/tmp/work/*/{target}-{machine}/*/temp/log.do_image_wic') > + if len(wic_output_files) == 0: > + self.fail('could not find WIC output file') > + with open(wic_output_files[0], 'r') as output: > + # in case we had permission problems, a WIC warning is in the logs > + for line in output: > + token = line.split() > + if len(token) > 0 and token[0] == 'WARNING:': > + self.fail(f'WIC issue found: {line}') > + > + def check_part_layout(target, machine): > + pattern = f'{self.build_dir}/tmp/deploy/images/{machine}/{target}-*-{machine}.wic.p*' > + partitions = sorted(glob.glob(pattern)) > + if len(partitions) != 3: Can you make partitions count and mount names to be configurable? It will make this code easier to maintain in case any target changes. > + self.fail(f'expected 3 partitions, but got {len(partitions)} (in {pattern})') > + mountpoints = [os.path.join(self.build_dir, mp) for mp in 'mnt_efi mnt_root mnt_home'.split()] > + for i in range(3): > + os.makedirs(mountpoints[i], exist_ok=True) > + _umount(mountpoints[i]) Why not to check for mountpoint here? Unmounting without check will cause unnecessary errors in job logs. Moreover, what is the case when this umount will be really needed? > + > + def check_output_for_warnings(target, machine): > + wic_output_files = glob.glob(f'{self.build_dir}/tmp/work/*/{target}-{machine}/*/temp/log.do_image_wic') > + if len(wic_output_files) == 0: > + self.fail('could not find WIC output file') > + with open(wic_output_files[0], 'r') as output: > + # in case we had permission problems, a WIC warning is in the logs > + for line in output: > + token = line.split() > + if len(token) > 0 and token[0] == 'WARNING:': > + self.fail(f'WIC issue found: {line}') > + > + def check_part_layout(target, machine): > + pattern = f'{self.build_dir}/tmp/deploy/images/{machine}/{target}-*-{machine}.wic.p*' > + partitions = sorted(glob.glob(pattern)) > + if len(partitions) != 3: > + self.fail(f'expected 3 partitions, but got {len(partitions)} (in {pattern})') > + mountpoints = [os.path.join(self.build_dir, mp) for mp in 'mnt_efi mnt_root mnt_home'.split()] > + for i in range(3): > + os.makedirs(mountpoints[i], exist_ok=True) > + _umount(mountpoints[i]) > + _mount(partitions[i], mountpoints[i]) > + > + # in boot partition, we expect /boot > + if not os.path.isdir(os.path.join(mountpoints[0], 'EFI')): > + self.fail('boot partition does not provide /EFI') > + # in root partition, boot and home should be excluded > + if not os.path.isdir(os.path.join(mountpoints[1], 'etc')) or \ > + os.path.isdir(os.path.join(mountpoints[1], 'home')) or \ > + os.path.isdir(os.path.join(mountpoints[1], 'boot')): > + self.fail('root partition does not contain expected dirs') > + # home partition should contain home of user "user" > + if not os.path.isdir(os.path.join(mountpoints[2], 'user')): > + self.fail('home partition does not contain home of user') > + > + [_umount(mnt) for mnt in mountpoints] Please cover mount-umount stuff with try-finally to be sure there will be no mounts left in case of failure. > + > + mc_spec = mc_target.split(':') > + target = mc_spec[2] > + machine = mc_spec[1].split('-')[0] > + > + self.configure( > + sstate=False, > + compat_arch=False, > + interactive_user=True, > + **kwargs) > + > + self.bitbake(mc_target, **kwargs) > + > + check_output_for_warnings(target, machine) > + check_part_layout(target, machine) > diff --git a/testsuite/cibuilder.py b/testsuite/cibuilder.py > index bc48d47f..e3fbb859 100755 > --- a/testsuite/cibuilder.py > +++ b/testsuite/cibuilder.py > @@ -54,7 +54,7 @@ class CIBuilder(Test): > > def configure(self, compat_arch=True, cross=None, debsrc_cache=False, > container=False, ccache=False, sstate=False, offline=False, > - gpg_pub_key=None, **kwargs): > + gpg_pub_key=None, interactive_user=False, **kwargs): > # write configuration file and set bitbake_args > # can run multiple times per test case > self.check_init() > @@ -107,6 +107,10 @@ class CIBuilder(Test): > f.write('BASE_REPO_KEY="file://' + gpg_pub_key + '"\n') > if distro_apt_premir: > f.write('DISTRO_APT_PREMIRRORS = "%s"\n' % distro_apt_premir) > + if interactive_user: > + f.write('USERS += "user"\n') > + f.write('USER_user[home] = "/home/user"\n') > + f.write('USER_user[flags] = "create-home"\n') > > # include ci_build.conf in local.conf > with open(self.build_dir + '/conf/local.conf', 'r+') as f: > diff --git a/testsuite/citest.py b/testsuite/citest.py > index 8d00054a..26101010 100755 > --- a/testsuite/citest.py > +++ b/testsuite/citest.py > @@ -318,3 +318,17 @@ class VmBootTestFull(CIBaseTest): > def test_amd64_focal(self): > self.init() > self.vm_start('amd64','focal') > + > +class SplittedRootfsTest(CIBaseTest): > + > + """ > + Test partition layout of splitted rootfs image > + > + :avocado: tags=splittedrootfs,full > + """ > + > + def test_wic_partition_layout(self): > + mc_target = 'mc:qemuamd64-bookworm:isar-image-base' > + > + self.init('build-wic-rootfs') > + self.perform_partition_layout_test(mc_target) Do we need the logs from build-wic-rootfs for analyzing in case CI fails? If yes - this path should be added into 'artifacts' part of .gitlab-ci.yml ^ permalink raw reply [flat|nested] 3+ messages in thread
* RE: [PATCH v3 4/4] ci: test partition layout of splitted-rootfs image 2022-05-23 14:19 ` Anton Mikanovich @ 2022-05-28 14:32 ` Moessbauer, Felix 0 siblings, 0 replies; 3+ messages in thread From: Moessbauer, Felix @ 2022-05-28 14:32 UTC (permalink / raw) To: Anton Mikanovich, isar-users Cc: Schild, Henning, jan.kiszka, Bezdeka, Florian > -----Original Message----- > From: Anton Mikanovich <amikan@ilbers.de> > Sent: Monday, May 23, 2022 4:19 PM > To: Moessbauer, Felix (T CED SES-DE) <felix.moessbauer@siemens.com>; isar- > users@googlegroups.com > Cc: Schild, Henning (T CED SES-DE) <henning.schild@siemens.com>; Kiszka, Jan > (T CED) <jan.kiszka@siemens.com>; Bezdeka, Florian (T CED SES-DE) > <florian.bezdeka@siemens.com> > Subject: Re: [PATCH v3 4/4] ci: test partition layout of splitted-rootfs image > > 12.05.2022 15:15, Felix Moessbauer wrote: > > This adds a test that checks if splitting the rootfs across multiple > > partitions correctly works. > > > > The following is checked: > > > > file-permissions (based on wic internal check): > > WIC already has a built-in check for correct file permissions. In case > > the permission db is missing, a WIC warning is emitted. We simply > > check for this warning. > > > > partition layout and excluded paths: > > Specifying exclude paths is error prone. > > Hence, we check that all generated partitions do not contain excluded > > paths. Further, we check that the expected paths are there. > > > > Signed-off-by: Felix Moessbauer <felix.moessbauer@siemens.com> > > --- > > testsuite/cibase.py | 58 ++++++++++++++++++++++++++++++++++++++++++ > > testsuite/cibuilder.py | 6 ++++- > > testsuite/citest.py | 14 ++++++++++ > > 3 files changed, 77 insertions(+), 1 deletion(-) > > > > diff --git a/testsuite/cibase.py b/testsuite/cibase.py index > > b25c356e..fd272ef5 100755 > > --- a/testsuite/cibase.py > > +++ b/testsuite/cibase.py > > @@ -164,3 +164,61 @@ class CIBaseTest(CIBuilder): > > ['do_rootfs_install_setscene', '!do_rootfs_install']) > > ]): > > self.fail("Failed rebuild package and image") > > + > > + def perform_partition_layout_test(self, mc_target, **kwargs): > > + def _mount(device, mountpoint): > > + process.run(f'mount -o ro {device} {mountpoint}', > > + sudo=True, ignore_status=False) > > + > > + def _umount(mountpoint): > > + process.run(f'umount -l {mountpoint}', sudo=True, > > + ignore_status=True) > Do not introduce new lazy umounts, please. > If umount fails we need to fix the reason, but do not mask it. Looks like the avocado partition module has bugs. Whenever I mount a partition with the module, I'm no longer able to unmount it (resource busy...). The python process still has a handle somewhere on the partition. By that, I'll stick to the umount logic but remove the laziness. > > You can maybe use avocado.utils.partition.Partition class for all mount, umount > and mountpoint check operations. > > + > > + def check_output_for_warnings(target, machine): > > + wic_output_files = glob.glob(f'{self.build_dir}/tmp/work/*/{target}- > {machine}/*/temp/log.do_image_wic') > > + if len(wic_output_files) == 0: > > + self.fail('could not find WIC output file') > > + with open(wic_output_files[0], 'r') as output: > > + # in case we had permission problems, a WIC warning is in the logs > > + for line in output: > > + token = line.split() > > + if len(token) > 0 and token[0] == 'WARNING:': > > + self.fail(f'WIC issue found: {line}') > > + > > + def check_part_layout(target, machine): > > + pattern = f'{self.build_dir}/tmp/deploy/images/{machine}/{target}-*- > {machine}.wic.p*' > > + partitions = sorted(glob.glob(pattern)) > > + if len(partitions) != 3: > Can you make partitions count and mount names to be configurable? > It will make this code easier to maintain in case any target changes. > > + self.fail(f'expected 3 partitions, but got {len(partitions)} (in > {pattern})') > > + mountpoints = [os.path.join(self.build_dir, mp) for mp in 'mnt_efi > mnt_root mnt_home'.split()] > > + for i in range(3): > > + os.makedirs(mountpoints[i], exist_ok=True) > > + _umount(mountpoints[i]) > Why not to check for mountpoint here? > Unmounting without check will cause unnecessary errors in job logs. > Moreover, what is the case when this umount will be really needed? > > + > > + def check_output_for_warnings(target, machine): > > + wic_output_files = glob.glob(f'{self.build_dir}/tmp/work/*/{target}- > {machine}/*/temp/log.do_image_wic') > > + if len(wic_output_files) == 0: > > + self.fail('could not find WIC output file') > > + with open(wic_output_files[0], 'r') as output: > > + # in case we had permission problems, a WIC warning is in the logs > > + for line in output: > > + token = line.split() > > + if len(token) > 0 and token[0] == 'WARNING:': > > + self.fail(f'WIC issue found: {line}') > > + > > + def check_part_layout(target, machine): > > + pattern = f'{self.build_dir}/tmp/deploy/images/{machine}/{target}-*- > {machine}.wic.p*' > > + partitions = sorted(glob.glob(pattern)) > > + if len(partitions) != 3: > > + self.fail(f'expected 3 partitions, but got {len(partitions)} (in > {pattern})') > > + mountpoints = [os.path.join(self.build_dir, mp) for mp in 'mnt_efi > mnt_root mnt_home'.split()] > > + for i in range(3): > > + os.makedirs(mountpoints[i], exist_ok=True) > > + _umount(mountpoints[i]) > > + _mount(partitions[i], mountpoints[i]) > > + > > + # in boot partition, we expect /boot > > + if not os.path.isdir(os.path.join(mountpoints[0], 'EFI')): > > + self.fail('boot partition does not provide /EFI') > > + # in root partition, boot and home should be excluded > > + if not os.path.isdir(os.path.join(mountpoints[1], 'etc')) or \ > > + os.path.isdir(os.path.join(mountpoints[1], 'home')) or \ > > + os.path.isdir(os.path.join(mountpoints[1], 'boot')): > > + self.fail('root partition does not contain expected dirs') > > + # home partition should contain home of user "user" > > + if not os.path.isdir(os.path.join(mountpoints[2], 'user')): > > + self.fail('home partition does not contain home of > > + user') > > + > > + [_umount(mnt) for mnt in mountpoints] > Please cover mount-umount stuff with try-finally to be sure there will be no > mounts left in case of failure. Yes. > > + > > + mc_spec = mc_target.split(':') > > + target = mc_spec[2] > > + machine = mc_spec[1].split('-')[0] > > + > > + self.configure( > > + sstate=False, > > + compat_arch=False, > > + interactive_user=True, > > + **kwargs) > > + > > + self.bitbake(mc_target, **kwargs) > > + > > + check_output_for_warnings(target, machine) > > + check_part_layout(target, machine) > > diff --git a/testsuite/cibuilder.py b/testsuite/cibuilder.py index > > bc48d47f..e3fbb859 100755 > > --- a/testsuite/cibuilder.py > > +++ b/testsuite/cibuilder.py > > @@ -54,7 +54,7 @@ class CIBuilder(Test): > > > > def configure(self, compat_arch=True, cross=None, debsrc_cache=False, > > container=False, ccache=False, sstate=False, offline=False, > > - gpg_pub_key=None, **kwargs): > > + gpg_pub_key=None, interactive_user=False, **kwargs): > > # write configuration file and set bitbake_args > > # can run multiple times per test case > > self.check_init() > > @@ -107,6 +107,10 @@ class CIBuilder(Test): > > f.write('BASE_REPO_KEY="file://' + gpg_pub_key + '"\n') > > if distro_apt_premir: > > f.write('DISTRO_APT_PREMIRRORS = "%s"\n' % > > distro_apt_premir) > > + if interactive_user: > > + f.write('USERS += "user"\n') > > + f.write('USER_user[home] = "/home/user"\n') > > + f.write('USER_user[flags] = "create-home"\n') > > > > # include ci_build.conf in local.conf > > with open(self.build_dir + '/conf/local.conf', 'r+') as f: > > diff --git a/testsuite/citest.py b/testsuite/citest.py index > > 8d00054a..26101010 100755 > > --- a/testsuite/citest.py > > +++ b/testsuite/citest.py > > @@ -318,3 +318,17 @@ class VmBootTestFull(CIBaseTest): > > def test_amd64_focal(self): > > self.init() > > self.vm_start('amd64','focal') > > + > > +class SplittedRootfsTest(CIBaseTest): > > + > > + """ > > + Test partition layout of splitted rootfs image > > + > > + :avocado: tags=splittedrootfs,full > > + """ > > + > > + def test_wic_partition_layout(self): > > + mc_target = 'mc:qemuamd64-bookworm:isar-image-base' > > + > > + self.init('build-wic-rootfs') > > + self.perform_partition_layout_test(mc_target) > Do we need the logs from build-wic-rootfs for analyzing in case CI fails? > If yes - this path should be added into 'artifacts' part of .gitlab-ci.yml Will be covered in v4. Felix ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-05-28 14:32 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-05-12 12:15 [PATCH v3 4/4] ci: test partition layout of splitted-rootfs image Felix Moessbauer 2022-05-23 14:19 ` Anton Mikanovich 2022-05-28 14:32 ` Moessbauer, Felix
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox