Add support for advanced partitioning customizations (COMPOSER-2399) #4535

achilleas-k · 2024-12-18T13:06:50Z

This PR enables the disk customizations in osbuild-composer.

Tests for the new blueprint configurations have been added to filesystem.sh.

This pull request includes:

adequate testing for the new functionality or fixed issue
adequate documentation informing people about the change such as
- submit a PR for the READMEs listed here
- submit a PR for the osbuild.org website repository if this PR changed any behavior not covered by the automatically updated READMEs

Add DiskCustomization and all its children to the internal blueprint. Add the conversion to the images counterpart to the Convert() function.

Move the creation of the blueprint to a function. We'll then add a separate blueprint function for the new, alternative partitioning customization.

Version 94 of osbuild-composer is over a year old now. It should be fine to test these unconditionally.

Add separate blueprint creation functions, one for each partitioning layout: - disk-plain - disk-lvm - disk-btrfs (Fedora only) The existing 'filesystem' blueprint is also kept. Each function also sets the $EXPECTED_MOUNTPOINTS variable for the mountpoint check that happens after the build.

Remove incorrect comment (not an ostree image). Add a greenprint for depsolving since it can take some time for that to run and it makes more sense than pausing output on "Preparing blueprint".

Run filesystem.sh with all the valid arguments for the customization type.

mvo5

Thanks for this PR, I have a very silly question inline, it comes down to if we can find a way to avoid some of the duplication with the images code that are currently present in this PR.

mvo5 · 2024-12-18T19:47:18Z

internal/blueprint/customizations.go

@@ -19,6 +19,7 @@ type Customizations struct {
 	Firewall           *FirewallCustomization    `json:"firewall,omitempty" toml:"firewall,omitempty"`
 	Services           *ServicesCustomization    `json:"services,omitempty" toml:"services,omitempty"`
 	Filesystem         []FilesystemCustomization `json:"filesystem,omitempty" toml:"filesystem,omitempty"`
+	Disk               *DiskCustomization        `json:"disk,omitempty" toml:"disk,omitempty"`


I have a very stupid (sorry!) question - could we simply do:

iff --git a/internal/blueprint/customizations.go b/internal/blueprint/customiza tions.go index 9ec43401f..5efe3eb40 100644 --- a/internal/blueprint/customizations.go +++ b/internal/blueprint/customizations.go @@ -5,33 +5,34 @@ import ( "reflect" "strings" + "github.com/osbuild/images/pkg/blueprint" "github.com/osbuild/images/pkg/disk" ) ... - Disk *DiskCustomization `json:"disk,omitempty" toml:"disk,omitempty"` + Disk *blueprint.DiskCustomization `json:"disk,omitempty" toml:"disk,omitempty"` ...

here (and remove disk_customizations.go in this PR)?

And then we would add a "Validate()" that simply disallows the "dc.Type" (as it seems that is the only thing we do not support compared to the images disk_blueprint)?

I think with the work we're doing to unify blueprints, it might be a good idea to just drop the whole copy from here and make it use the one from images directly.

While it makes sense, since we made the images version of blueprints used for user-facing (de)serialization from JSON, we keep a few backward compatibility layers in the composer's BPs that are missing in the images version. So it may not be doable cleanly. But let's see what you're able to do.

To @mvo5 point about duplication. Given our current situation with the BP structures, I agree that this is a code duplication. However, the intention (and we should try to get back to it) was to separate the user-facing representation of the data structures for (un)marshaling from the data structures used by the code. The goal is to refactor, change, and simplify those "internal" data structures while keeping the user-facing data structures backward compatible. We are not there anymore with BPs, but it is still, in my opinion, a worthwhile goal.

Thanks @thozza - I started a PR in osbuild/images#983 about separating the data structures/json a while ago, might be something to work on further I suppose (it is on halt currently because of the blueprint v2 discussions but it seems code-wise we probably still want/need something like this).

mvo5 · 2024-12-18T19:52:31Z

internal/blueprint/disk_customizations.go

+	"github.com/osbuild/images/pkg/datasizes"
+)
+
+type DiskCustomization struct {


See my question above, I really would like to avoid this duplication, I'm worried it will lead to drift and I am also not sure why we need it (but maybe/surely I'm missing something here?)

achilleas-k · 2024-12-19T13:33:47Z

I need to update image-info to handle btrfs subvolumes and ignore swap partitions when mounting filesystems.

thozza · 2025-01-02T05:46:14Z

I need to update image-info to handle btrfs subvolumes and ignore swap partitions when mounting filesystems.

Let's try to standardize on the osbuild-image-info and remove the image-info from this repository.

test/cases/filesystem.sh

thozza · 2025-01-02T06:22:12Z

.gitlab-ci.yml

+      - CUSTOMIZATION_TYPE: "disk-btrfs"
+        RUNNER:
+          - aws/fedora-40-x86_64
+      - CUSTOMIZATION_TYPE: "filesystem"
+        RUNNER:
+          - aws/rhel-8.10-ga-x86_64
+      - CUSTOMIZATION_TYPE: "disk-plain"
+        RUNNER:
+          - aws/rhel-9.4-ga-x86_64
+      - CUSTOMIZATION_TYPE: "disk-lvm"
+        RUNNER:
+          - aws/rhel-9.6-nightly-x86_64
+      - CUSTOMIZATION_TYPE: "disk-plain"
+        RUNNER:
+          - aws/rhel-10.0-nightly-x86_64
+      - CUSTOMIZATION_TYPE: "disk-lvm"
+        RUNNER:
+          - aws/centos-stream-9-x86_64
+      - CUSTOMIZATION_TYPE: "filesystem"
+        RUNNER:
+          - aws/centos-stream-10-x86_64


The resulting combination of which test variants are being tested on which OS is weird. Is there any intention behind it? I'd expect to see all variants being run on all runners 🤔

I didn't want the test matrix to grow too much so I added one configuration per distro version. You're right, it might be a bit confusing. Also, thinking about it, I think this setup might cause gaps in the future when an old distro version is removed from testing and a new one gets a different configuration.

achilleas-k · 2025-01-02T18:04:12Z

I need to update image-info to handle btrfs subvolumes and ignore swap partitions when mounting filesystems.

Let's try to standardize on the osbuild-image-info and remove the image-info from this repository.

#4543

achilleas-k added 4 commits December 18, 2024 12:02

blueprint: add local DiskCustomization

f928ce8

Add DiskCustomization and all its children to the internal blueprint. Add the conversion to the images counterpart to the Convert() function.

blueprint: test the conversion

edf99d8

test/filesystem: move blueprint writing to function

010c6fe

Move the creation of the blueprint to a function. We'll then add a separate blueprint function for the new, alternative partitioning customization.

test/filesystem: remove conditions on v94

4c09586

Version 94 of osbuild-composer is over a year old now. It should be fine to test these unconditionally.

achilleas-k requested review from ondrejbudai, mvo5 and thozza December 18, 2024 13:06

achilleas-k changed the title ~~Add support for advanced partitioning customizations~~ Add support for advanced partitioning customizations (COMPOSER-2399) Dec 18, 2024

achilleas-k force-pushed the all-the-partitioning-all-the-time branch from 75cb610 to df59325 Compare December 18, 2024 14:37

achilleas-k added 3 commits December 18, 2024 19:08

test/filessystem: minor cleanups

2ca9c3f

Remove incorrect comment (not an ostree image). Add a greenprint for depsolving since it can take some time for that to run and it makes more sense than pausing output on "Preparing blueprint".

gitlab: run all versions of filesystem.sh

423fb63

Run filesystem.sh with all the valid arguments for the customization type.

achilleas-k force-pushed the all-the-partitioning-all-the-time branch from df59325 to 423fb63 Compare December 18, 2024 18:08

mvo5 reviewed Dec 18, 2024

View reviewed changes

thozza reviewed Jan 2, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for advanced partitioning customizations (COMPOSER-2399) #4535

Add support for advanced partitioning customizations (COMPOSER-2399) #4535

achilleas-k commented Dec 18, 2024

mvo5 left a comment

mvo5 Dec 18, 2024

achilleas-k Dec 18, 2024

thozza Jan 2, 2025

mvo5 Jan 6, 2025

mvo5 Dec 18, 2024

achilleas-k commented Dec 19, 2024

thozza commented Jan 2, 2025

thozza Jan 2, 2025

achilleas-k Jan 2, 2025

achilleas-k commented Jan 2, 2025

Add support for advanced partitioning customizations (COMPOSER-2399) #4535

Are you sure you want to change the base?

Add support for advanced partitioning customizations (COMPOSER-2399) #4535

Conversation

achilleas-k commented Dec 18, 2024

mvo5 left a comment

Choose a reason for hiding this comment

mvo5 Dec 18, 2024

Choose a reason for hiding this comment

achilleas-k Dec 18, 2024

Choose a reason for hiding this comment

thozza Jan 2, 2025

Choose a reason for hiding this comment

mvo5 Jan 6, 2025

Choose a reason for hiding this comment

mvo5 Dec 18, 2024

Choose a reason for hiding this comment

achilleas-k commented Dec 19, 2024

thozza commented Jan 2, 2025

thozza Jan 2, 2025

Choose a reason for hiding this comment

achilleas-k Jan 2, 2025

Choose a reason for hiding this comment

achilleas-k commented Jan 2, 2025