Skip to content

kola: Add test case for extra partition creation #457

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jepio merged 2 commits into from
Nov 23, 2023
Merged

kola: Add test case for extra partition creation #457

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1bea94d
kola: Add test case for extra partition creation
jepio Sep 29, 2023
054a51c
kola: tests: partition_on_boot_disk: Tweak fs labels and create file
jepio Nov 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 57 additions & 0 deletions kola/tests/ignition/filesystem.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"github.com/coreos/go-semver/semver"
"github.com/flatcar/mantle/kola/cluster"
"github.com/flatcar/mantle/kola/register"
"github.com/flatcar/mantle/kola/tests/util"
"github.com/flatcar/mantle/platform/conf"
)

Expand Down Expand Up @@ -322,6 +323,17 @@ systemd:
Platforms: []string{"qemu", "qemu-unpriv"},
MinVersion: semver.Version{Major: 3033},
})

register.Register(&register.Test{
// Note: As long as the initrd does not mount /var for early
// preparations, the resulting system has a few issues, so this setup is
// not fully supported yet
Name: "cl.ignition.partition_on_boot_disk",
Run: testPartitionOnBootDisk,
ClusterSize: 0,
Distros: []string{"cl"},
Platforms: []string{"qemu", "qemu-unpriv"},
})
}

var ext4NoClobberV2_1 = conf.Ignition(`{
Expand Down Expand Up @@ -437,3 +449,48 @@ func testSwapActivation(c cluster.TestCluster) {
c.Fatalf("swap's size should be: %s, got %s", swapSize, size)
}
}

var rootDiskExtraPartition = conf.ContainerLinuxConfig(`
storage:
disks:
- device: /dev/disk/by-id/virtio-primary-disk
partitions:
- label: VAR
number: 10
start: '9GiB'
filesystems:
- name: VAR
mount:
device: /dev/disk/by-partlabel/VAR
Copy link
Member

@pothos pothos Oct 2, 2023

Choose a reason for hiding this comment

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

For /var we should also make sure that the mount point is set up in the initrd with Ignition because systemd-tmpfiles should populate /var from the initrd. I think with CLC and the translation to Ignition v3 this is complicated and if you want to test /var we should rather use Butane. If it's not really about /var (or /home and similar) but an arbitrary mount point then it's easier to use CLC.

Copy link
Member Author

@jepio jepio Oct 3, 2023

Choose a reason for hiding this comment

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

For /var we should also make sure that the mount point is set up in the initrd with Ignition because systemd-tmpfiles should populate /var from the initrd.

we don't populate /var with systemd-tmpfiles from the initrd, and ignition unmounts mounted partitions in one of the last stages. It does this to ensure a system does not depend on ignition performing the mount.

I think with CLC and the translation to Ignition v3 this is complicated and if you want to test /var we should rather use Butane. If it's not really about /var (or /home and similar) but an arbitrary mount point then it's easier to use CLC.

It's not about the mountpoint at all, it's about adding partitions to a partitioned disk (that's why this was in partitions.go).

For sure we need a testcase that ensures we don't break backwards compat with ignition v2/CLC. /var is also a location users are likely to try to put on a separate filesystem.

Copy link
Member

@pothos pothos Oct 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

We do, that's in base_image_var.conf and baselayout.conf but yeah, seems that we have a problem in general with separate /var partitions, at least on the first boot before the final system also had a chance to set up the files in /var. What one might see as result is either a failed unit or a misbehaving system, e.g., the first boot journal might not be persisted (this was a bug we fixed by letting the initrd populate /var correctly).

Copy link
Member

@pothos pothos Oct 3, 2023

Choose a reason for hiding this comment

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

FYI: The topic of separate /var is also relevant for applying sysext images from the initrd. The suggestion from Lennart was to establish a kernel command line setting similar to what systemd has for /usr. It would then be properly mounted in /sysroot/var.

Copy link
Member

@pothos pothos Oct 3, 2023

Choose a reason for hiding this comment

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

What we can also do in Flatcar is to define a partition or filesystem label to expect and then add a mount unit for that (it can be compatible with a future systemd cmdline way by having this mount unit only be set up if the cmdline option is not there, I think we have similar logic for /usr).

Copy link
Member

@pothos pothos Oct 4, 2023

Choose a reason for hiding this comment

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

Started something for supporting separate /var: flatcar/bootengine#75

Copy link
Member

@pothos pothos Oct 4, 2023

Choose a reason for hiding this comment

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

Would be good to write some files here with Ignition. We also have to add one more test case with Butane an defining the /var path in the initrd and writing some files because the uplifted v3 path is generated and can't be controlled whereas with Butane we can use the /sysroot/var mount point and have to check that this works.

Copy link
Member Author

@jepio jepio Nov 23, 2023

Choose a reason for hiding this comment

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

so i added the file creation part, but not the butane yet

format: xfs
label: VAR
files:
- filesystem: VAR
path: /hello
mode: 0644
contents:
inline: world
- filesystem: root
path: /etc/fstab
mode: 0644
contents:
inline: |
/dev/disk/by-label/VAR /var xfs defaults 0 0
`)

func testPartitionOnBootDisk(c cluster.TestCluster) {
m, err := util.NewMachineWithLargeDisk(c, "10G", rootDiskExtraPartition)
if err != nil {
c.Fatal(err)
}
out := c.MustSSH(m, "lsblk -f")
c.Logf("lsblk -f:\n%s", out)
out = c.MustSSH(m, "findmnt")
c.Logf("findmnt:\n%s", out)

c.MustSSH(m, "mountpoint /var")
c.MustSSH(m, "ls -la /dev/disk/by-partlabel/VAR")
c.MustSSH(m, "ls -la /dev/disk/by-label/VAR")
c.AssertCmdOutputContains(m, "findmnt /var", "xfs")
c.AssertCmdOutputContains(m, "cat /var/hello", "world")
}