Skip to content
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.

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

[RFC] distro: drop hardcoded partiton table UUIDs #816

Closed
wants to merge 1 commit into from

Conversation

mvo5
Copy link
Contributor

@mvo5 mvo5 commented Jul 29, 2024

The current distro partition tables contain hardcoded UUIDs for various elements like the partition table, the partition and the filesystem UUIDs. Hardcoding these seems slightly unusual, given that a UUID is supposed to be (practically) unique and that the images library will DTRT if no UUID is set.

This commit removes the hardcoded PartitionTable UUID as a starting point to spark a discussion. I have only a limited understanding of the history of this and while I did do a bit of git archeology I could not find a commit with a rational why the UUIDs are hardcoded [0] so I'm asking for a review from some of the people who are around for a long time. I would love to understand if there is a reason to have these hardcoded UUIDs and i'm happy to do a PR that adds comments etc that will explain this reason. If however we do not need/want the hardcoded UUIDs i will continue and remove them from the partition and filesystems too. If we go down this path I'm sure this is a risky change because some people probably rely by now on the fact that the UUID is actually quite predictable (also https://xkcd.com/1172) and because this behavior is here since day 1 (or so it seems). Also @achilleas-k mentioned "Chestertons fence" to me so I'm trying to be extra cautious. Help is greatly appreciated to help me understand this better!

C.f. osbuild/bootc-image-builder#568

[0] I found
osbuild/osbuild-composer@2225463#diff-de240f0bca8f03daafeb653ff429aafa12f43b5e0dc8ba327c56108a6ff864f3R115 and osbuild/osbuild-composer@0dd17ae

The current distro partition tables contain hardcoded UUIDs for
various elements like the partition table and the partition and
the filesystem UUIDs. Hardcoding these seems slightly unusual,
given that a UUID is supposed to be unique.

This commit removes the hardcoded PartitionTable UUID.
@mvo5 mvo5 requested review from teg, ondrejbudai and thozza July 29, 2024 20:28
@mvo5 mvo5 changed the title [RFC] distro: drop hardcoded partiton table UUIDsR [RFC] distro: drop hardcoded partiton table UUIDs Jul 29, 2024
@cgwalters
Copy link
Contributor

It may make sense to hoist them all to a shared const mysteriousUuid = ... first with an explanatory comment?

the images library will DTRT if no UUID is set.

What is right thing here? All zeros so tools like systemd-repart will know to init on firstboot?

@achilleas-k
Copy link
Member

Let's split this PR into a RHEL 10 change (and merge it) and keep the other ones as draft until we finish evaluating the impact.

@achilleas-k
Copy link
Member

All zeros so tools like systemd-repart will know to init on firstboot?

Does repart do this automatically or does it need to be configured with drop-ins? Afaik, it would need drop-in configs, which we don't write, but maybe something to consider for the future.

@thozza
Copy link
Member

thozza commented Jul 31, 2024

I think that this predates my start in the team, but my gut feeling is that the UUIDs are simply the values that were used by the original images built by imagefactory and used as the base for our own image definitions. IOW someone inspected the partition table in the image and use it as is to produce "the same image". 🤷

Let's split this PR into a RHEL 10 change (and merge it) and keep the other ones as draft until we finish evaluating the impact.

I would add Fedora to the list of distros where we could do it.

@achilleas-k
Copy link
Member

my gut feeling is that the UUIDs are simply the values that were used by the original images built by imagefactory and used as the base for our own image definitions

This sounds extremely plausible.

@mvo5
Copy link
Contributor Author

mvo5 commented Jul 31, 2024

Thanks all for the input! Closing in favor of #823

@mvo5 mvo5 closed this Jul 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants