Skip to content

disk: pass default fs to FS customizations (MS-8990)#1714

Merged
achilleas-k merged 2 commits into
osbuild:mainfrom
mvo5:disk-customizations-fstype
Sep 15, 2025
Merged

disk: pass default fs to FS customizations (MS-8990)#1714
achilleas-k merged 2 commits into
osbuild:mainfrom
mvo5:disk-customizations-fstype

Conversation

@mvo5

@mvo5 mvo5 commented Jul 31, 2025

Copy link
Copy Markdown
Contributor

[needed for https://github.com//pull/1656]

Tradtionally we did not pass the filesystem with the filesystem customizations which means that we always created "xfs". This is problematic on bootc systems that require us to use the container default filesystem and arguably also on fedora that declares its default fs to be "ext4" in our image definitions.

So this commit changes the behavior so that the defaultFs is passed when creating custom mountpoints. This changes nothing on rhel/centos but does change the behavior on fedora - which should be fine as this is a bugfix.

/jira-epic HMS-8844

JIRA: HMS-8990

@mvo5 mvo5 requested review from a team and achilleas-k as code owners July 31, 2025 17:01
@mvo5 mvo5 requested review from supakeen and thozza July 31, 2025 17:01
@schutzbot

schutzbot commented Jul 31, 2025

Copy link
Copy Markdown
Collaborator

A previous version of this PR changed the images API or behaviour causing integration issues with osbuild-composer.
This is now fixed.

@mvo5 mvo5 force-pushed the disk-customizations-fstype branch 3 times, most recently from edb8711 to d2413cd Compare August 1, 2025 07:14
thozza
thozza previously approved these changes Aug 1, 2025

@thozza thozza left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks.

In general, this looks OK, I just don't like that the "default" behavior is implemented in two places.

In addition, this made me realize that we should consider defining FSType as an enum or at least as constants, instead of using string literals everywhere.

Comment thread pkg/disk/lvm.go Outdated
@mvo5 mvo5 force-pushed the disk-customizations-fstype branch from d2413cd to e4cc000 Compare August 4, 2025 11:05
@achilleas-k achilleas-k changed the title disk: pass default fs to FS customiaztions disk: pass default fs to FS customizations Aug 6, 2025
Tradtionally we did not pass the filesystem with the
filesystem customizations which means that we always
created "xfs". This is problematic on bootc systems
that require us to use the container default filesystem
and arguably also on fedora that declares its default
fs to be "ext4" in our image definitions.

So this commit changes the behavior so that the defaultFs
is passed when creating custom mountpoints. This changes
nothing on rhel/centos but does change the behavior on
fedora - which should be fine as we can consider this a
bugfix.

The manifest diff shows that the fstype on fedora moves
from xfs to ext4 for the filesystem customizations.
@mvo5 mvo5 force-pushed the disk-customizations-fstype branch from e4cc000 to 03400f5 Compare September 5, 2025 14:44
mvo5 added a commit to mvo5/images that referenced this pull request Sep 5, 2025
This implements the suggestion by Achilleas to error when
CreateMointpoint() in btrfs or lvm is called with the
wrong arguments, see
osbuild#1714 (comment)

Thanks to Achilleas.
@mvo5 mvo5 requested a review from thozza September 5, 2025 14:54
@mvo5 mvo5 changed the title disk: pass default fs to FS customizations disk: pass default fs to FS customizations (HMS-8844) Sep 5, 2025
@mvo5 mvo5 changed the title disk: pass default fs to FS customizations (HMS-8844) disk: pass default fs to FS customizations (MS-8990) Sep 5, 2025
achilleas-k
achilleas-k previously approved these changes Sep 10, 2025

@achilleas-k achilleas-k left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you! LGTM

Typo found.

Comment thread pkg/disk/btrfs.go Outdated
This implements the suggestion by Achilleas to error when
CreateMointpoint() in btrfs or lvm is called with the
wrong arguments, see
osbuild#1714 (comment)

Thanks to Achilleas.
@achilleas-k achilleas-k added this pull request to the merge queue Sep 15, 2025
@supakeen

Copy link
Copy Markdown
Member

While this is a bugfix it is also incorrect as Fedora Server should be XFS. That can be done in follow-ups.

Merged via the queue into osbuild:main with commit 36deb05 Sep 15, 2025
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants