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

feat: Add Custom bootloader names so pretty_name can be static #3289

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

antheas
Copy link

@antheas antheas commented Aug 28, 2024

In certain hardware surveys (e.g., Steam), in order for a distribution to be accounted for the PRETTY_NAME os-release variable is used.

Currently, this name plays a dual role in ostree, where it is displayed on GRUB. This necessitates it changing every version, so that rollbacks are intuitive for users. If this happens, the distribution image has to either be unfairly represented in those surveys OR the version that is displayed in GRUB has to be unintuitive and frozen.

Therefore, add a new variable in /etc/os-release called BOOTLOADER_NAME that will be used with priority compared to the PRETTY_NAME of the distribution, thereby allowing an intuitive name for both applications.

(this PR is untested; for now)

Copy link

openshift-ci bot commented Aug 28, 2024

Hi @antheas. Thanks for your PR.

I'm waiting for a ostreedev member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@ericcurtin
Copy link
Collaborator

@antheas can you share what your /etc/os-release files look before/after this change? @cgwalters name is littered all over the PRETTY_NAME code here and in rpm-ostree, so he's probably the best to take a look at this, to see if it will cause no regressions.

@antheas
Copy link
Author

antheas commented Aug 28, 2024

Hi Eric,

As I wrote above, we have not deployed the change yet as we would need to fork ostree. But we have made the change to os-release.

I do not think there will be regressions because of this, unless there is another BOOTLOADER_NAME conflict. But if it is littered everywhere, perhaps this is not the correct place to make the change.

For your reference, here is how grub looks right now:
image

Which is confusing and not ideal.

Here is how an /etc/os-release looks:
image
(excuse the picture, had to use a VM)

Here is the file that provides the os-release file:
https://github.com/ublue-os/bazzite/blob/main/system_files/desktop/shared/usr/libexec/containerbuild/image-info

There is no change to the /etc/os-release file, other than adding the optional BOOTLOADER_NAME variable as a substitute.

@antheas
Copy link
Author

antheas commented Aug 28, 2024

Seems like PRETTY_NAME is only used in the place I sent and in the rpm-ostree backend code that implements mutate-os-release and is unrelated.

@ericcurtin
Copy link
Collaborator

Yup I noticed similar, the os-release file has different names in the /ostree/deploy location:

/ostree/deploy/fedora/var/lib/containers/storage/overlay/b4bbae57b10b4c10585b70e04ce63d4fae26099228dfa62d7124e6b02451cd80/diff/etc/os-release:PRETTY_NAME="CentOS Stream 9"
/ostree/deploy/fedora/var/lib/containers/storage/overlay/b4bbae57b10b4c10585b70e04ce63d4fae26099228dfa62d7124e6b02451cd80/diff/usr/lib/os-release:PRETTY_NAME="CentOS Stream 9"
/ostree/deploy/fedora/var/lib/containers/storage/overlay/174f5685490326fc0a1c0f5570b8663732189b327007e47ff13d2ca59673db02/diff/etc/os-release:PRETTY_NAME="CentOS Linux 7 (Core)"
/ostree/deploy/fedora/var/lib/containers/storage/overlay/174f5685490326fc0a1c0f5570b8663732189b327007e47ff13d2ca59673db02/diff/usr/lib/os-release:PRETTY_NAME="CentOS Linux 7 (Core)"
/ostree/deploy/fedora/var/lib/containers/storage/overlay/8ff7ad910417a7b8a49019008335921d2aac0e3304a19ce258deabf431e59801/diff/etc/os-release:PRETTY_NAME="Fedora Linux 39 (Container Image)"
/ostree/deploy/fedora/var/lib/containers/storage/overlay/8ff7ad910417a7b8a49019008335921d2aac0e3304a19ce258deabf431e59801/diff/usr/lib/os-release:PRETTY_NAME="Fedora Linux 39 (Container Image)"
/ostree/deploy/fedora/var/lib/containers/storage/overlay/9fc7c00bf80202cbfbffb65aed5db8eba1d610a6c13505de9dffacc861487495/diff/etc/os-release:PRETTY_NAME="Fedora Linux 39 (Container Image)"
/ostree/deploy/fedora/var/lib/containers/storage/overlay/9fc7c00bf80202cbfbffb65aed5db8eba1d610a6c13505de9dffacc861487495/diff/usr/lib/os-release:PRETTY_NAME="Fedora Linux 39 (Container Image)"

@antheas
Copy link
Author

antheas commented Aug 28, 2024

Looking at my deployment I can confirm. Both files are the same though.

Your images appear to have the same issue. PRETTY_NAME is pinned so you cant tell different versions of those deployments apart.

In our case, Bazzite Stable (F40.20240826.0) would be a lot cleaner on GRUB.

@cgwalters
Copy link
Member

Thanks for filing this!

One thing tangentially related to this is there's a relatively recent addition to the os-release spec called IMAGE_VERSION that I think we should start using (and soft-deprecate the OSTREE_VERSION field). Doing that needs some work on our build systems (though, just like with OCI, we have synchronization issues with the external metadata (commit, manifest.json) and the in-os file.

Hmmm...it looks to me like the boot loader code is trying to inject the ostree version in the commit digest if PRETTY_NAME doesn't already contain it. So wouldn't it work to just make the PRETTY_NAME static today?

@antheas
Copy link
Author

antheas commented Aug 28, 2024

I cannot find that piece of code. The only reference to that is mutate_os_release_contents in rpm-ostree which is part of the backend code.

@dbnicholson
Copy link
Member

On Endless OS we have a fairly static PRETTY_NAME, but the version metadata from the commit is included in the BLS title. So, we almost never touch /etc/os-release, but grub shows a version specific title since we dutifully add the version metadata in the commits.

In your case, I would set PRETTY_NAME=Bazzite Stable in /etc/os-release and make your ostree commit with --add-metadata=version=F40.20240826.0. Then the BLS title will be Bazzite Stable F40.20240826.0 (ostree:0).

@dbnicholson
Copy link
Member

Another thing that could be useful is if you could set the BLS title entirely by commit metadata instead of relying on a bunch of heuristics in ostree. I.e., --add-metadata=ostree.title=My Distro (1.0.0) would result in a BLS title of My Distro (1.0.0) (ostree:0). I feel like always need to append the ostree index number since the title is not guaranteed to be unique between deployed commits. Maybe not, though.

@antheas
Copy link
Author

antheas commented Aug 29, 2024

In universal blue, we are using an OCI based workflow, so most images cannot modify the commit metadata.

They can do OCI tags or os-release.

Also, this metadata fact escaped us. Does rpm ostree respect the version tag of a base commit when deploying an OCI image?

@antheas antheas marked this pull request as draft August 29, 2024 08:39
@antheas
Copy link
Author

antheas commented Aug 29, 2024

Switched to draft as something has to be done about deployment_version first.

@antheas
Copy link
Author

antheas commented Aug 29, 2024

Dan also brought up the valid point of (ostree:0). I personally think that the deployment ID needs to be in the bootloader somewhere, but I will concede that (ostree:X) is not the best.

Perhaps we can printf it into the bootloader name, so that it is optional and customizable.

I think I would prefer it to be 0: Bazzite Stable ....

I will also note that Stable here is the branch and it can also take the values Unstable, Testing and PR. In which case the versioning format changes. Unstable and PR use a shortened commit hash, and Stable, Testing use the upstream Kinoite version. So it is not that clearcut to insert into PRETTY_NAME, as it should always be the same for hardware surveys.

  /* XXX The SYSLINUX bootloader backend actually parses the title string
   *     (specifically, it looks for the substring "(ostree"), so further
   *     changes to the title format may require updating that backend. */

What does the above mean? Is (ostree set in stone?

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.

4 participants