New test for correct btrfs setup #19242

pablo-herranz · 2024-05-06T13:41:52Z

The main idea is to check for the correct btrfs layout on the disk. Snapshots and rollbacks are already covered in other test modules.

Related ticket: https://progress.opensuse.org/issues/66613
Verification runs:

ricardobranco777

I'm not sure this PR addresses the requirements setup in the poo ticket as it only runs some commands that are guaranteed not to fail, without validating anything.

tests/console/btrfs_check.pm

mloviska · 2024-05-07T08:38:46Z

The main point of the ticket is to check the layout of images. So the test should run only for minimal-vm or sle-micro. This layout is defined by the corresponding kiwi file. For jeos/minimal-vm it is https://build.suse.de/projects/SUSE:SLE-15-SP6:GA:TEST/packages/kiwi-templates-Minimal/files/Minimal.kiwi?expand=1 defined by <systemdisk> section. This layout might change from flavor to flavor and products

mloviska

I like the approach so far, however we really need to test this code thoroughly with different hypervisors. Please use jeos-filesystem test suite from the latest job group.

mloviska · 2024-05-10T10:35:06Z

tests/console/btrfs_check.pm

+sub run {
+    select_console 'root-console';
+
+    my $kiwi_volumes = qq(


this will a bit more complicated. Given the fact that we have 3 archs (no ppc) the layout will differ. Especially when we are talking about boot subvolumes.

Any clues for checking all the possibilities?

mloviska · 2024-05-10T10:36:33Z

tests/console/btrfs_check.pm

+/usr/local
+/var);
+
+    my $test_volumes = script_output("lsblk --list --noheadings --output MOUNTPOINTS /dev/vda3 | sort");


I would rather see a list of subvolumes, hence using btrfs commands, but that might be a matter of taste. The good part is that with subvolumes you would use / rather than a drive. Drives might have different labels, depends on hypervisor mostly

Nice, I'll see if I can remove all the "unnecessary" info and keep only the last column for comparison purposes.

mloviska · 2024-05-10T10:37:37Z

tests/console/btrfs_check.pm

+    record_info('test_volumes', "$test_volumes");
+    record_info('kiwi_volumes', "$kiwi_volumes");
+    die "Volumes mismatch from Kiwi definition"
+      if $kiwi_volumes !~ $test_volumes;


Not sure whether this works or not. The order is always guaranteed? I think nested loops would be more robust.

Yep, nice idea. I'm already working on it.

mloviska · 2024-05-10T10:38:53Z

tests/console/btrfs_check.pm

+    assert_script_run('rpm -qa > /tmp/rpm_qa.txt');
+    upload_logs('/tmp/rpm_qa.txt');
+    upload_logs('/var/log/zypper.log');
+    $self->SUPER::post_fail_hook;


This needs to be reworked. We are interested in the filesystem, hence a custom post_fail_hook would suit better here

This is my first time writing a post_fail_hook, so I just copy-pasted 😅

IMO this does not need a custom post_fail_hook at all. All relevant info for possible failures is already visible.

Vogtinator · 2024-05-16T11:46:56Z

tests/console/btrfs_check.pm

+
+    # Prepare the arrays of subvolumes for comparing
+    # TODO: the subvolumes list vary depending on the arch and the version
+    my @kiwi_volumes = qw(@ @/.snapshots @/home @/opt @/root @/srv @/tmp @/usr/local @/var);


/tmp is a tmpfs on SL(E)M and openSUSE and must not be a subvolume

grisu48 · 2024-05-16T12:22:53Z

tests/console/btrfs_check.pm

+    # Compare that the lists of subvolumes match
+    foreach my $kiwi (@kiwi_volumes) {
+        my $found = 0;
+        foreach my $test (@test_volumes) {


This can be simplified by using the any module from List::Util I think

Suggested change

foreach my $test (@test_volumes) {

die "Subvolume $kiwi not present" unless (any {$_ eq $kiwi} @test_volumes);

And you will need a use List::Util 'any'; in the beginning.

I think that can simplify this routine a lot.

Let's put this to the test!
https://www.youtube.com/watch?v=sByxTCQU8Rc

I'll remember this reference.

You're goddamn right!!
https://youtu.be/S9RVS8cjNN0

Vogtinator · 2024-05-17T10:56:24Z

tests/console/btrfs_check.pm

+use List::Util qw(any);
+
+sub run {
+    select_console 'root-console';


Could use select_serial_terminal;

Done ✔️

The main idea is to check for the correct btrfs layout on the disk. Snapshots and rollbacks are already covered in other test modules.

pablo-herranz force-pushed the poo66613_test-for-correct-btrfs-setup branch from acb310e to 149f484 Compare May 6, 2024 13:44

This comment was marked as off-topic.

Sign in to view

pablo-herranz force-pushed the poo66613_test-for-correct-btrfs-setup branch 3 times, most recently from 1d01292 to 57cb0b2 Compare May 7, 2024 07:23

pablo-herranz marked this pull request as ready for review May 7, 2024 07:55

m-dati approved these changes May 7, 2024

View reviewed changes

ricardobranco777 requested changes May 7, 2024

View reviewed changes

tests/console/btrfs_check.pm Outdated Show resolved Hide resolved

tests/console/btrfs_check.pm Outdated Show resolved Hide resolved

pablo-herranz force-pushed the poo66613_test-for-correct-btrfs-setup branch from 57cb0b2 to 48822fe Compare May 7, 2024 08:29

pablo-herranz marked this pull request as draft May 7, 2024 08:29

pablo-herranz added the WIP Work in progress label May 7, 2024

pablo-herranz changed the title ~~New test for correct btrfs setup~~ [WIP] New test for correct btrfs setup May 7, 2024

pablo-herranz changed the title ~~[WIP] New test for correct btrfs setup~~ New test for correct btrfs setup May 7, 2024

pablo-herranz force-pushed the poo66613_test-for-correct-btrfs-setup branch 16 times, most recently from 0baf37f to a1a4dd1 Compare May 10, 2024 09:49

mloviska reviewed May 10, 2024

View reviewed changes

pablo-herranz force-pushed the poo66613_test-for-correct-btrfs-setup branch 9 times, most recently from 43e9605 to 301d6ed Compare May 16, 2024 09:49

pablo-herranz removed the WIP Work in progress label May 16, 2024

pablo-herranz marked this pull request as ready for review May 16, 2024 11:38

pablo-herranz added the RFC Request for Comments label May 16, 2024

Vogtinator reviewed May 16, 2024

View reviewed changes

grisu48 reviewed May 16, 2024

View reviewed changes

pablo-herranz force-pushed the poo66613_test-for-correct-btrfs-setup branch 3 times, most recently from f70f66c to c6b5186 Compare May 16, 2024 12:56

pablo-herranz added the WIP Work in progress label May 16, 2024

pablo-herranz force-pushed the poo66613_test-for-correct-btrfs-setup branch from c6b5186 to ac6a1ac Compare May 16, 2024 13:08

pablo-herranz removed WIP Work in progress RFC Request for Comments labels May 17, 2024

ricardobranco777 approved these changes May 17, 2024

View reviewed changes

mloviska approved these changes May 17, 2024

View reviewed changes

Vogtinator reviewed May 17, 2024

View reviewed changes

pablo-herranz force-pushed the poo66613_test-for-correct-btrfs-setup branch 2 times, most recently from 276ede7 to 2ab14dd Compare May 20, 2024 07:40

New test for correct btrfs setup

c94c212

The main idea is to check for the correct btrfs layout on the disk. Snapshots and rollbacks are already covered in other test modules.

pablo-herranz force-pushed the poo66613_test-for-correct-btrfs-setup branch from 2ab14dd to c94c212 Compare May 20, 2024 08:26

pablo-herranz merged commit 7095520 into os-autoinst:master May 20, 2024
10 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New test for correct btrfs setup #19242

New test for correct btrfs setup #19242

pablo-herranz commented May 6, 2024 •

edited

This comment was marked as off-topic.

ricardobranco777 left a comment

mloviska commented May 7, 2024

mloviska left a comment

mloviska May 10, 2024

pablo-herranz May 10, 2024

mloviska May 10, 2024

pablo-herranz May 10, 2024

mloviska May 10, 2024

pablo-herranz May 10, 2024

mloviska May 10, 2024

pablo-herranz May 10, 2024

Vogtinator May 16, 2024

Vogtinator May 16, 2024 •

edited

grisu48 May 16, 2024

pablo-herranz May 16, 2024

grisu48 May 16, 2024

pablo-herranz May 16, 2024

Vogtinator May 17, 2024

pablo-herranz May 20, 2024

	foreach my $test (@test_volumes) {
	die "Subvolume $kiwi not present" unless (any {$_ eq $kiwi} @test_volumes);

New test for correct btrfs setup #19242

New test for correct btrfs setup #19242

Conversation

pablo-herranz commented May 6, 2024 • edited

This comment was marked as off-topic.

ricardobranco777 left a comment

Choose a reason for hiding this comment

mloviska commented May 7, 2024

mloviska left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Vogtinator May 16, 2024 • edited

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

pablo-herranz commented May 6, 2024 •

edited

Vogtinator May 16, 2024 •

edited