RHEL 9: Filesystem customizations for edge-raw-image #255

say-paul · 2023-11-13T12:51:53Z

This PR enables File system customization for edge-raw-image.

say-paul · 2023-11-13T13:33:16Z

having the following error:

Nov 13 18:03:33 saypaul-thinkpadx1carbongen9.pnq.csb osbuild-worker[1450]: time="2023-11-13T18:03:33+05:30" level=info msg="    Failed to open file \"/sys/fs/selinux/checkreqprot\": Read-only file system" jobId=3230a5f5-ee82-416e-9be3-11e7089cc948
Nov 13 18:03:33 saypaul-thinkpadx1carbongen9.pnq.csb osbuild-worker[1450]: time="2023-11-13T18:03:33+05:30" level=info msg="    LVM2: using vg name '25287c67-6499-43cd-a25f-a80f90f36cb0'" jobId=3230a5f5-ee82-416e-9be3-11e7089cc948
Nov 13 18:03:33 saypaul-thinkpadx1carbongen9.pnq.csb osbuild-worker[1450]: time="2023-11-13T18:03:33+05:30" level=info msg="      Physical volume \"/dev/mapper/osbuild-luks-46d73afa-d0b3-4c68-860c-ca1c52f87211\" successfully created." jobId=3230a5f5-ee82-416e-9be3-11e7089cc948
Nov 13 18:03:33 saypaul-thinkpadx1carbongen9.pnq.csb osbuild-worker[1450]: time="2023-11-13T18:03:33+05:30" level=info msg="      Creating devices file /etc/lvm/devices/system.devices" jobId=3230a5f5-ee82-416e-9be3-11e7089cc948
Nov 13 18:03:33 saypaul-thinkpadx1carbongen9.pnq.csb osbuild-worker[1450]: time="2023-11-13T18:03:33+05:30" level=info msg="      Volume group \"25287c67-6499-43cd-a25f-a80f90f36cb0\" successfully created" jobId=3230a5f5-ee82-416e-9be3-11e7089cc948
Nov 13 18:03:33 saypaul-thinkpadx1carbongen9.pnq.csb osbuild-worker[1450]: time="2023-11-13T18:03:33+05:30" level=info msg="      WARNING: Logical volume 25287c67-6499-43cd-a25f-a80f90f36cb0/rootlv not zeroed." jobId=3230a5f5-ee82-416e-9be3-11e7089cc948
Nov 13 18:03:33 saypaul-thinkpadx1carbongen9.pnq.csb osbuild-worker[1450]: time="2023-11-13T18:03:33+05:30" level=info msg="      Logical volume \"rootlv\" created." jobId=3230a5f5-ee82-416e-9be3-11e7089cc948
Nov 13 18:03:33 saypaul-thinkpadx1carbongen9.pnq.csb osbuild-worker[1450]: time="2023-11-13T18:03:33+05:30" level=info msg="      Volume group \"25287c67-6499-43cd-a25f-a80f90f36cb0\" has insufficient free space (252 extents): 256 required." jobId=3230a5f5-ee82-416e-9be3-11e7089cc948
Nov 13 18:03:33 saypaul-thinkpadx1carbongen9.pnq.csb osbuild-worker[1450]: time="2023-11-13T18:03:33+05:30" level=info msg="    Traceback (most recent call last):" jobId=3230a5f5-ee82-416e-9be3-11e7089cc948
Nov 13 18:03:33 saypaul-thinkpadx1carbongen9.pnq.csb osbuild-worker[1450]: time="2023-11-13T18:03:33+05:30" level=info msg="      File \"/run/osbuild/bin/org.osbuild.lvm2.create\", line 110, in <module>" jobId=3230a5f5-ee82-416e-9be3-11e7089cc948
Nov 13 18:03:33 saypaul-thinkpadx1carbongen9.pnq.csb osbuild-worker[1450]: time="2023-11-13T18:03:33+05:30" level=info msg="        ret = main(args[\"devices\"], args[\"options\"])" jobId=3230a5f5-ee82-416e-9be3-11e7089cc948
Nov 13 18:03:33 saypaul-thinkpadx1carbongen9.pnq.csb osbuild-worker[1450]: time="2023-11-13T18:03:33+05:30" level=info msg="      File \"/run/osbuild/bin/org.osbuild.lvm2.create\", line 105, in main" jobId=3230a5f5-ee82-416e-9be3-11e7089cc948
Nov 13 18:03:33 saypaul-thinkpadx1carbongen9.pnq.csb osbuild-worker[1450]: time="2023-11-13T18:03:33+05:30" level=info msg="        subprocess.run(cmd, encoding='utf8', check=True)" jobId=3230a5f5-ee82-416e-9be3-11e7089cc948
Nov 13 18:03:33 saypaul-thinkpadx1carbongen9.pnq.csb osbuild-worker[1450]: time="2023-11-13T18:03:33+05:30" level=info msg="      File \"/usr/lib64/python3.9/subprocess.py\", line 528, in run" jobId=3230a5f5-ee82-416e-9be3-11e7089cc948
Nov 13 18:03:33 saypaul-thinkpadx1carbongen9.pnq.csb osbuild-worker[1450]: time="2023-11-13T18:03:33+05:30" level=info msg="        raise CalledProcessError(retcode, process.args," jobId=3230a5f5-ee82-416e-9be3-11e7089cc948
Nov 13 18:03:33 saypaul-thinkpadx1carbongen9.pnq.csb osbuild-worker[1450]: time="2023-11-13T18:03:33+05:30" level=info msg="    subprocess.CalledProcessError: Command '['lvcreate', '-an', '--size', '1073741824B', '-n', 'foo_barlv', '25287c67-6499-43cd-a25f-a80f90f36cb0']' returned non-zero exit status 5." jobId=3230a5f5-ee82-416e-9be3-11e7089cc948

with blueprint:

name = "rhel93-fs-image"
description = "rhel93-fs-image"
version = "0.0.0"
packages = []
modules = []
groups = []
distro = "rhel-93"

[customizations]

[[customizations.filesystem]]
mountpoint = "/foo"
size = 256
[customizations.ignition]
[customizations.ignition.firstboot]
url = "http://192.168.122.1:8001/config.ign"

say-paul · 2023-11-28T08:30:06Z

blueprint for customization:

~/composed_images $ cat ~/blueprints/rhel93-multiple-v2-fs-image.toml 
name = "rhel93-multiple-v2-fs-image"
description = "rhel93-fs-multiple-v2-image"
distro = "rhel-93"

[[customizations.filesystem]]
mountpoint = "/foo/bar"
size=2147483648

[[customizations.filesystem]]
mountpoint = "/foo"
size=8589934592


[[customizations.filesystem]]
mountpoint = "/var/myfiles"
size= "1 GiB"

[customizations.ignition.firstboot]
url = "http://192.168.122.1:8001/config.ign"

~/composed_images $ composer-cli blueprints show rhel93-multiple-v2-fs-image 
name = "rhel93-multiple-v2-fs-image"
description = "rhel93-fs-multiple-v2-image"
version = "0.0.0"
packages = []
modules = []
groups = []
distro = "rhel-93"

[customizations]

[[customizations.filesystem]]
mountpoint = "/foo/bar"
size = 2147483648

[[customizations.filesystem]]
mountpoint = "/foo"
size = 8589934592

[[customizations.filesystem]]
mountpoint = "/var/myfiles"
size = 1073741824
[customizations.ignition]
[customizations.ignition.firstboot]
url = "http://192.168.122.1:8001/config.ign"

say-paul · 2023-11-28T08:35:01Z

composed image

achilleas-k · 2023-11-29T18:34:53Z

I found the issue. It's a tricky detail with the way the partition table is described internally. Volume group partitions are sizeable containers, they need to have a size defined. But it seems we don't do this in our static partition tables, so the volume group's partition starts off as 0 bytes and is then grown to fill the space of the required total image size. This worked fine so far, but once the logical volume sizes add up to more than the image size, we run out of space.

The proper solution here is to have logic which resizes the volume group partition to fit the logical volumes it contains and also have it calculate the size of those LVs when needed. It would basically be the same logic we have for the partition table as a whole: the vg has a "desired size", if the sum of lv sizes inside it is bigger than that desired size we grow it, if it's not then it's set to that size.

I'll get a fix (and some tests) done asap.

say-paul · 2023-11-30T11:38:03Z

Interesting details, can we pre-calculate the space required and create the VG accordingly, and then grow rootlv to fit the remaining space in image.disk_space.

say-paul · 2023-11-30T11:43:19Z

@achilleas-k @thozza So the the PR will be waiting for another PR, any feedback on policy and how its applied?

achilleas-k · 2023-11-30T12:03:51Z

Interesting details, can we pre-calculate the space required and create the VG accordingly, and then grow rootlv to fit the remaining space in image.disk_space.

yeah that's the plan. I'm working on that today.

achilleas-k · 2023-11-30T20:48:15Z

Fix for the filesystem size calculation is here #293

internal/pathpolicy/path_policy.go

test/configs/filesystem-customizations.json

thozza

Thanks for your work. I added a few comments inline.

On a general note, I'm not a big fan of merging the policies in the way you implemented it. I admit that this is probably a bit of a personal preference, but I feel like the merging makes the code more complex. In addition, the destination of the merge function is modified in-place and can override parts of the source policy (which is not needed AFAICT and it is also not explicitly documented and not covered by the unit test).

My preference would be to not touch the default path policy and how it is checked. Then in the checkOptions() function, you could simply do something like this after the default policy check:

...
err := blueprint.CheckMountpointsPolicy(mountpoints, pathpolicy.MountpointPolicies)
if err != nil {
	return warnings, err
}

if t.rpmOstree {
	err := blueprint.CheckMountpointsPolicy(mountpoints, pathpolicy.OstreeMountpointPolicies)
	if err != nil {
		return warnings, err
	}
}
...

This way, the ostree mountpoint policy would be really just an add-on to the default polisy and very few new code and logic would need to be added.

test/configs/filesystem-customizations.json

internal/pathpolicy/policies.go

achilleas-k · 2023-12-05T10:13:29Z

Thanks for your work. I added a few comments inline.

On a general note, I'm not a big fan of merging the policies in the way you implemented it. I admit that this is probably a bit of a personal preference, but I feel like the merging makes the code more complex. In addition, the destination of the merge function is modified in-place and can override parts of the source policy (which is not needed AFAICT and it is also not explicitly documented and not covered by the unit test).

My preference would be to not touch the default path policy and how it is checked. Then in the checkOptions() function, you could simply do something like this after the default policy check:
...
err := blueprint.CheckMountpointsPolicy(mountpoints, pathpolicy.MountpointPolicies)
if err != nil {
	return warnings, err
}

if t.rpmOstree {
	err := blueprint.CheckMountpointsPolicy(mountpoints, pathpolicy.OstreeMountpointPolicies)
	if err != nil {
		return warnings, err
	}
}
...
This way, the ostree mountpoint policy would be really just an add-on to the default polisy and very few new code and logic would need to be added.

I agree with all of this. Much simpler and avoids any implicit behaviour when merging. Let's do it this way.

test/config-map.json

say-paul · 2023-12-06T06:49:20Z

If your team does not feel comfortable with enabling the FS customizations for that image type, then please at least manually boot-test the image.

I am up for enabling it for edge-ami too , but I would defer to the teams decision on this which I will update shortly.

say-paul · 2023-12-06T14:26:29Z

If your team does not feel comfortable with enabling the FS customizations for that image type, then please at least manually boot-test the image.

I am up for enabling it for edge-ami too , but I would defer to the teams decision on this which I will update shortly.

Looking into the broader epic that I am working,we need support for :
simplified-installer, edge-raw-image, edge-ami, edge-vsphere, so it makes sense to enable it for all .

Ostree specific filesystem policy to prevent users form accidentally creating custom filesystems that can ovewrite the systems filesystem. Signed-off-by: Sayan Paul <[email protected]>

Filesystem customizations is enabled for edge-raw-image, edge-ami,edge-vsphere,simplified-installer relevent testing config is added to run in CI Signed-off-by: Sayan Paul <[email protected]>

rhel only supports uefi, thus enforcing it. Signed-off-by: Sayan Paul <[email protected]>

say-paul · 2023-12-13T17:08:55Z

rebased and commit message updated

achilleas-k

LGTM. A couple of comments for follow-up stuff.

test/scripts/boot-image

test/config-map.json

achilleas-k · 2023-12-13T17:46:48Z

Created new issues from my comments for follow-up.

Extend the ostree filesystem customizations test case according to osbuild#255 (comment) Signed-off-by: Tomáš Hozza <[email protected]>

say-paul marked this pull request as draft November 13, 2023 12:52

say-paul force-pushed the filesystem_customization branch 2 times, most recently from 00b7f2c to 34a8fa8 Compare November 27, 2023 10:37

say-paul mentioned this pull request Nov 29, 2023

lvcreate is demanding 4 PE/ 16 MiB extra lvmteam/lvm2#133

Closed

say-paul force-pushed the filesystem_customization branch from 34a8fa8 to dc3549c Compare November 30, 2023 11:10

say-paul requested review from thozza and achilleas-k November 30, 2023 11:43

say-paul force-pushed the filesystem_customization branch from dc3549c to a12698b Compare December 1, 2023 09:55

say-paul marked this pull request as ready for review December 1, 2023 09:56

say-paul force-pushed the filesystem_customization branch from a12698b to 2733adf Compare December 1, 2023 11:40

achilleas-k requested changes Dec 1, 2023

View reviewed changes

internal/pathpolicy/path_policy.go Outdated Show resolved Hide resolved

achilleas-k changed the title ~~Filesystem customization~~ RHEL 9: Filesystem customizations for edge-raw-image Dec 1, 2023

say-paul force-pushed the filesystem_customization branch from 2733adf to d6a28df Compare December 4, 2023 07:32

say-paul requested a review from achilleas-k December 4, 2023 07:44

say-paul force-pushed the filesystem_customization branch from 1814442 to e325e46 Compare December 4, 2023 07:53

achilleas-k previously requested changes Dec 4, 2023

View reviewed changes

internal/pathpolicy/path_policy.go Outdated Show resolved Hide resolved

say-paul force-pushed the filesystem_customization branch from e325e46 to 53a6d61 Compare December 4, 2023 11:35

say-paul requested a review from achilleas-k December 4, 2023 12:44

achilleas-k reviewed Dec 4, 2023

View reviewed changes

test/configs/filesystem-customizations.json Outdated Show resolved Hide resolved

thozza reviewed Dec 4, 2023

View reviewed changes

test/configs/filesystem-customizations.json Outdated Show resolved Hide resolved

internal/pathpolicy/policies.go Outdated Show resolved Hide resolved

say-paul force-pushed the filesystem_customization branch from 53a6d61 to aa43332 Compare December 5, 2023 12:24

thozza reviewed Dec 5, 2023

View reviewed changes

test/config-map.json Outdated Show resolved Hide resolved

say-paul force-pushed the filesystem_customization branch from e96fb7f to 8e5c411 Compare December 6, 2023 06:46

say-paul force-pushed the filesystem_customization branch from 8e5c411 to 921e4f8 Compare December 6, 2023 06:54

say-paul force-pushed the filesystem_customization branch 5 times, most recently from f2cf7c4 to a6c7031 Compare December 13, 2023 14:56

say-paul added 3 commits December 13, 2023 22:34

filesystem/policy:added ostree specific mountpoints

5f4903f

Ostree specific filesystem policy to prevent users form accidentally creating custom filesystems that can ovewrite the systems filesystem. Signed-off-by: Sayan Paul <[email protected]>

blueprint/customization:filesystem customizations for edge images

1fc906a

Filesystem customizations is enabled for edge-raw-image, edge-ami,edge-vsphere,simplified-installer relevent testing config is added to run in CI Signed-off-by: Sayan Paul <[email protected]>

boot/edge-ami: set boot-mode as uefi for ec2 instance

da406f8

rhel only supports uefi, thus enforcing it. Signed-off-by: Sayan Paul <[email protected]>

say-paul force-pushed the filesystem_customization branch from a6c7031 to da406f8 Compare December 13, 2023 17:07

say-paul requested a review from thozza December 13, 2023 17:09

achilleas-k approved these changes Dec 13, 2023

View reviewed changes

test/scripts/boot-image Show resolved Hide resolved

test/config-map.json Show resolved Hide resolved

This was referenced Dec 13, 2023

Check boot failures with custom mountpoints with edge-ami on RHEL 9.0 and 9.1 #316

Open

Boot images in both modes (BIOS and UEFI) in CI when they support it #317

Open

achilleas-k enabled auto-merge December 13, 2023 17:38

achilleas-k added this pull request to the merge queue Dec 13, 2023

Merged via the queue into osbuild:main with commit 15e9d28 Dec 13, 2023
8 checks passed

achilleas-k mentioned this pull request Dec 19, 2023

Extend OSTree filesystem customization policies #329

Open

thozza added a commit to thozza/osbuild-images that referenced this pull request Dec 19, 2023

Test: extend ostree fs customizations case

ba72faf

Extend the ostree filesystem customizations test case according to osbuild#255 (comment) Signed-off-by: Tomáš Hozza <[email protected]>

thozza mentioned this pull request Dec 19, 2023

Extend ostree mountpoint tests #330

Closed

thozza added a commit to thozza/osbuild-images that referenced this pull request Dec 19, 2023

Test: extend ostree fs customizations case

9ea83a5

Extend the ostree filesystem customizations test case according to osbuild#255 (comment) Signed-off-by: Tomáš Hozza <[email protected]>

This was referenced Jan 8, 2024

edge-raw-image with custom partition mount points does not boot after upgrade #352

Open

test: add custom partition mount points to raw image test case osbuild/osbuild-composer#3884

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RHEL 9: Filesystem customizations for edge-raw-image #255

RHEL 9: Filesystem customizations for edge-raw-image #255

say-paul commented Nov 13, 2023

say-paul commented Nov 13, 2023 •

edited

Loading

say-paul commented Nov 28, 2023

say-paul commented Nov 28, 2023

achilleas-k commented Nov 29, 2023 •

edited

Loading

say-paul commented Nov 30, 2023 •

edited

Loading

say-paul commented Nov 30, 2023

achilleas-k commented Nov 30, 2023

achilleas-k commented Nov 30, 2023

thozza left a comment

achilleas-k commented Dec 5, 2023

say-paul commented Dec 6, 2023 •

edited

Loading

say-paul commented Dec 6, 2023

say-paul commented Dec 13, 2023

achilleas-k left a comment

achilleas-k commented Dec 13, 2023

RHEL 9: Filesystem customizations for edge-raw-image #255

RHEL 9: Filesystem customizations for edge-raw-image #255

Conversation

say-paul commented Nov 13, 2023

say-paul commented Nov 13, 2023 • edited Loading

say-paul commented Nov 28, 2023

say-paul commented Nov 28, 2023

achilleas-k commented Nov 29, 2023 • edited Loading

say-paul commented Nov 30, 2023 • edited Loading

say-paul commented Nov 30, 2023

achilleas-k commented Nov 30, 2023

achilleas-k commented Nov 30, 2023

thozza left a comment

Choose a reason for hiding this comment

achilleas-k commented Dec 5, 2023

say-paul commented Dec 6, 2023 • edited Loading

say-paul commented Dec 6, 2023

say-paul commented Dec 13, 2023

achilleas-k left a comment

Choose a reason for hiding this comment

achilleas-k commented Dec 13, 2023

say-paul commented Nov 13, 2023 •

edited

Loading

achilleas-k commented Nov 29, 2023 •

edited

Loading

say-paul commented Nov 30, 2023 •

edited

Loading

say-paul commented Dec 6, 2023 •

edited

Loading