create org.osbuild.bootupd stage by dustymabe · Pull Request #1519 · osbuild/osbuild

dustymabe · 2023-12-22T16:39:37Z

Add the bootupd stage to install GRUB on both BIOS and UEFI systems,
ensuring that your bootloader stays up-to-date.

Signed-off-by: Renata Ravanelli rravanel@redhat.com

dustymabe · 2023-12-22T16:44:50Z

The 2nd commit here will be dropped. The 3rd and 4th commits are optional and stem from a previous conversation between @achilleas-k @cgwalters and I. Over there I don't think we fully fleshed out if the optimization was completely out of the question or not. Maybe I should just open a separate PR with that optimization so we could focus discussion on it, but at the same time I don't want to introduce options to the bootupd stage here if we'd remove them later.

achilleas-k · 2024-01-04T15:02:35Z

Maybe I should just open a separate PR with that optimization so we could focus discussion on it, but at the same time I don't want to introduce options to the bootupd stage here if we'd remove them later.

That would be preferable, yeah. Can you open it separately so we can talk about that there and then keep this specific to bootupd? Thanks!

dustymabe · 2024-01-04T16:27:27Z

I broke out the controversial commits into #1527

You can now review just the first commit in this PR. The 2nd commit is just there to make tests pass until we can update the registry.gitlab.com/redhat/services/products/image-builder/ci/images/fedora-coreos next week when a new build of FCOS is out that we can use.

The EFI binaries are currently pulled from a hardcoded path in the buildroot. When moving to containers as buildroots this will no longer work as they have an alternative layout. This is an easy "fix" - make the location of the `EFI/` directory configurable. This allows us set `efi_root` to `/usr/lib/bootupd/updates/EFI/` and keep our existing `bootc-image-builder` workflow. Note that this may actually not be the desired solution and instead we want the new `bootupd`: osbuild#1519

stages/org.osbuild.bootupd

dustymabe · 2024-01-08T03:20:21Z

@achilleas-k can you pull quay.io/fedora/fedora-coreos:testing and push it to registry.gitlab.com/redhat/services/products/image-builder/ci/images/fedora-coreos:testing and I'll update this so tests pass and we can hopefully merge.

You can delete registry.gitlab.com/redhat/services/products/image-builder/ci/images/fedora-coreos:stable if you want as it should be unused now.

The update to v103 includes: - create org.osbuild.ostree.aleph stage - osbuild/osbuild#1475 - mounts: support mounting partitions - osbuild/osbuild#1501 - osbuild/mounts: adjust the source path to use when mounting devices - osbuild/osbuild#1493 And I also add the bootupd stage in here from the yet to merge PR: osbuild/osbuild#1519

The EFI binaries are currently pulled from a hardcoded path in the buildroot. When moving to containers as buildroots this will no longer work as they have an alternative layout. This is an easy "fix" - make the location of the `EFI/` directory configurable. This allows us set `efi_root` to `/usr/lib/bootupd/updates/EFI/` and keep our existing `bootc-image-builder` workflow. Note that this may actually not be the desired solution and instead we want the new `bootupd`: osbuild#1519

achilleas-k · 2024-01-08T15:41:33Z

@achilleas-k can you pull quay.io/fedora/fedora-coreos:testing and push it to registry.gitlab.com/redhat/services/products/image-builder/ci/images/fedora-coreos:testing and I'll update this so tests pass and we can hopefully merge.

Done: https://gitlab.com/redhat/services/products/image-builder/ci/images/container_registry/5710257

dustymabe · 2024-01-08T15:46:54Z

@achilleas-k can you pull quay.io/fedora/fedora-coreos:testing and push it to registry.gitlab.com/redhat/services/products/image-builder/ci/images/fedora-coreos:testing and I'll update this so tests pass and we can hopefully merge.

Done: https://gitlab.com/redhat/services/products/image-builder/ci/images/container_registry/5710257

Thanks! Pushed a new update and rebased on top of latest main. Hopefully the tests pass!

The EFI binaries are currently pulled from a hardcoded path in the buildroot. When moving to containers as buildroots this will no longer work as they have an alternative layout. This is an easy "fix" - make the location of the `EFI/` directory configurable. This allows us set `efi_src_dir` to `/usr/lib/bootupd/updates/EFI/` and keep our existing `bootc-image-builder` workflow. Note that this may actually not be the desired solution and instead we want the new `bootupd`: osbuild#1519

mvo5

Thank you for this PR - this looks fine but I would really like some tests. When looking at that I got a bit carried away and opened dustymabe#14 for your consideration. It addresses the nitpicks/ideas I have in here. I would also love to have a stage test and will look at this next.

Fwiw, we are super happy that you work on this because we will most likely use it for the bootc container builds :)

stages/org.osbuild.bootupd

mvo5

This looks good to me now, given that I changed this a bit it might be best if e.g. @achilleas-k could have a look too.

I would love to have a stage test as well to run this for real and I started to look into it but I think it's a bit involved (because of the required mount setup). I think it's okay to not block on this because there is no regression risk.

The update to v103 includes: - create org.osbuild.ostree.aleph stage - osbuild/osbuild#1475 - mounts: support mounting partitions - osbuild/osbuild#1501 - osbuild/mounts: adjust the source path to use when mounting devices - osbuild/osbuild#1493 And I also add the bootupd stage in here from the yet to merge PR: osbuild/osbuild#1519

There were some updates from code review in [1]. Let's pull them here and also update the mpp.yaml manifest to include the ostree deployment specification. [1] osbuild/osbuild#1519

mvo5

Still looks good!

There were some updates from code review in [1]. Let's pull them here and also update the mpp.yaml manifest to include the ostree deployment specification. [1] osbuild/osbuild#1519

achilleas-k

The last two commits have subjects that are too long. Can we keep them under 72 chars?
Can we also clean up the history a bit? (see comment below).
The disk option under bios: Making enforcing the name through the schema for the devices object would make it consistent with how we do it in other stages, but if we prefer to keep it explicit, I'd change the name of the option to make it clear that it maps to a "device".

stages/org.osbuild.bootupd

dustymabe · 2024-01-09T21:25:37Z

The last two commits have subjects that are too long. Can we keep them under 72 chars?

Fixed.

Can we also clean up the history a bit? (see comment below).

Tried to fix.

The disk option under bios: Making enforcing the name through the schema for the devices object would make it consistent with how we do it in other stages, but if we prefer to keep it explicit, I'd change the name of the option to make it clear that it maps to a "device".

I was leaning your way on this, but then Renata and I found a gap in
our coverage here (see the newest commit) where on ppc64le we need to
be able to install into a partition of a device so I added the
partition key too. Otherwise we could have made bios: true/false a
boolean.

Add the bootupd stage to install GRUB on both BIOS and UEFI systems, ensuring that your bootloader stays up-to-date. Signed-off-by: Renata Ravanelli <rravanel@redhat.com>

The stable stream currently doesn't have a new enough bootupd to pass the tests for the bootupd stage. Let's update to `:testing` for now and we'll switch back to `:stable` later.

Also refactor bind mounts into a helper.

It now tests bind_mounts and bootupd behavior separately.

For ppc64le we need to pass in a partition (i.e. /dev/loop0p1) rather than the root device (/dev/loop0) to the --device argument of bootupctl. Let's add a partition field and find the device node based on the user specified partition. On ppc64le this would look something like: ``` - type: org.osbuild.bootupd options: bios: device: disk partition: mpp-format-int: '{image.layout[''POWERPC-PREP-BOOT''].partnum}' static-configs: true deployment: osname: fedora-coreos ref: ostree/1/1/0 devices: disk: type: org.osbuild.loopback options: filename: disk.img partscan: true mounts: - name: root type: org.osbuild.xfs source: disk partition: mpp-format-int: '{image.layout[''root''].partnum}' target: / - name: boot type: org.osbuild.ext4 source: disk partition: mpp-format-int: '{image.layout[''boot''].partnum}' target: /boot ```

achilleas-k · 2024-01-09T22:09:17Z

I was learning your way on this, but then Renata and I found a gap in
our coverage here (see the newest commit) where on ppc64le we need to
be able to install into a partition of a device so I added the
partition key too. Otherwise we could have made bios: true/false a
boolean.

You could specify the start and size options of the device to create a loopback that points to the specific partition. In the test case for example it look like this:

        {
          "type": "org.osbuild.bootupd",
          "options": {
            "bios": true,
            "static-configs": true,
            "deployment": {
              "osname": "fedora-coreos",
              "ref": "ostree/1/1/0"
            }
          },
          "devices": {
            "disk": {
              "type": "org.osbuild.loopback",
              "options": {
                "filename": "disk.img",
                "start": 4096,
                "size": 260096
              }
            }
          }
        }

Would that work?

dustymabe · 2024-01-09T22:11:07Z

Would that work?

unfortunately not. the grub2-install stuff really doesn't like it when you map in a slice of a device and not the whole device itself :(

achilleas-k · 2024-01-09T22:40:04Z

Would that work?

unfortunately not. the grub2-install stuff really doesn't like it when you map in a slice of a device and not the whole device itself :(

Right, of course -_-
(forgot we went through all this already)

achilleas-k

LGTM. Good to go on my end!

dustymabe · 2024-01-10T15:40:51Z

Thank you @mvo5 and @achilleas-k for the help and the reviews here.

Add support for the new `org.osbuild.bootupd` stage that got added in osbuild/osbuild#1519

It finally merged upstream: osbuild/osbuild#1519

achilleas-k self-requested a review December 22, 2023 17:21

dustymabe force-pushed the dusty-bootupd branch 2 times, most recently from 3f34ccf to 25cf86a Compare January 4, 2024 16:23

dustymabe mentioned this pull request Jan 4, 2024

osbuild/util/ostree: optimize deployment_path() #1527

Closed

mvo5 self-requested a review January 4, 2024 17:35

mvo5 mentioned this pull request Jan 5, 2024

stages(grub2): allow pulling efi binaries from alternative efi roots #1529

Merged

ondrejbudai reviewed Jan 5, 2024

View reviewed changes

stages/org.osbuild.bootupd Outdated Show resolved Hide resolved

dustymabe force-pushed the dusty-bootupd branch from 25cf86a to 8adf61b Compare January 8, 2024 15:45

mvo5 reviewed Jan 9, 2024

View reviewed changes

dustymabe force-pushed the dusty-bootupd branch from 220d7e2 to cb1e881 Compare January 9, 2024 15:16

mvo5 previously approved these changes Jan 9, 2024

View reviewed changes

dustymabe dismissed mvo5’s stale review via e24eace January 9, 2024 15:34

dustymabe force-pushed the dusty-bootupd branch from cb1e881 to e24eace Compare January 9, 2024 15:34

dustymabe force-pushed the dusty-bootupd branch from e24eace to 0b1ce7c Compare January 9, 2024 16:15

dustymabe mentioned this pull request Jan 9, 2024

osbuild: update the org.osbuild.bootupd stage with latest upstream coreos/coreos-assembler#3692

Merged

mvo5 previously approved these changes Jan 9, 2024

View reviewed changes

achilleas-k requested changes Jan 9, 2024

View reviewed changes

stages/org.osbuild.bootupd Outdated Show resolved Hide resolved

stages/org.osbuild.bootupd Show resolved Hide resolved

dustymabe dismissed mvo5’s stale review via d56add7 January 9, 2024 21:25

dustymabe force-pushed the dusty-bootupd branch from 0b1ce7c to d56add7 Compare January 9, 2024 21:25

dustymabe force-pushed the dusty-bootupd branch from d56add7 to 64afb80 Compare January 9, 2024 22:05

ravanelli and others added 6 commits January 9, 2024 17:07

create org.osbuild.bootupd stage

f9c650a

Add the bootupd stage to install GRUB on both BIOS and UEFI systems, ensuring that your bootloader stays up-to-date. Signed-off-by: Renata Ravanelli <rravanel@redhat.com>

tests/fedora-coreos-container: switch to testing FCOS

8c77995

The stable stream currently doesn't have a new enough bootupd to pass the tests for the bootupd stage. Let's update to `:testing` for now and we'll switch back to `:stable` later.

stages(bootupd): add schema test

27c3500

stages(bootupd): add tests for existing behavior

a52e790

Also refactor bind mounts into a helper.

stages(bootupd): refactor test to separately test behaviors

f295673

It now tests bind_mounts and bootupd behavior separately.

dustymabe force-pushed the dusty-bootupd branch from 64afb80 to 604e0f9 Compare January 9, 2024 22:08

achilleas-k approved these changes Jan 9, 2024

View reviewed changes

achilleas-k enabled auto-merge (rebase) January 9, 2024 22:59

achilleas-k merged commit 8cce659 into osbuild:main Jan 10, 2024

mvo5 added a commit to mvo5/images that referenced this pull request Jan 15, 2024

osbuild: add new bootupd stage

a201413

Add support for the new `org.osbuild.bootupd` stage that got added in osbuild/osbuild#1519

mvo5 added a commit to mvo5/images that referenced this pull request Jan 16, 2024

osbuild: add new bootupd stage

358b932

Add support for the new `org.osbuild.bootupd` stage that got added in osbuild/osbuild#1519

mvo5 added a commit to mvo5/images that referenced this pull request Jan 16, 2024

osbuild: add new bootupd stage

3de6c47

Add support for the new `org.osbuild.bootupd` stage that got added in osbuild/osbuild#1519

mvo5 mentioned this pull request Jan 16, 2024

osbuild: add new bootupd stage (HMS-3318) osbuild/images#374

Merged

mvo5 added a commit to mvo5/images that referenced this pull request Jan 16, 2024

osbuild: add new bootupd stage

41292c8

Add support for the new `org.osbuild.bootupd` stage that got added in osbuild/osbuild#1519

github-merge-queue bot pushed a commit to osbuild/images that referenced this pull request Jan 16, 2024

osbuild: add new bootupd stage

1595514

Add support for the new `org.osbuild.bootupd` stage that got added in osbuild/osbuild#1519

dustymabe added a commit to dustymabe/coreos-assembler that referenced this pull request Jan 16, 2024

osbuild: one final update to the org.osbuild.bootupd patches

c8001a9

It finally merged upstream: osbuild/osbuild#1519

dustymabe added a commit to dustymabe/coreos-assembler that referenced this pull request Jan 16, 2024

osbuild: one final update to the org.osbuild.bootupd patches

9ff87be

It finally merged upstream: osbuild/osbuild#1519

dustymabe added a commit to dustymabe/coreos-assembler that referenced this pull request Jan 17, 2024

osbuild: one final update to the org.osbuild.bootupd patches

ee76207

It finally merged upstream: osbuild/osbuild#1519

dustymabe added a commit to coreos/coreos-assembler that referenced this pull request Jan 17, 2024

osbuild: one final update to the org.osbuild.bootupd patches

b5d715d

It finally merged upstream: osbuild/osbuild#1519

Conversation

dustymabe commented Dec 22, 2023

Uh oh!

dustymabe commented Dec 22, 2023

Uh oh!

achilleas-k commented Jan 4, 2024

Uh oh!

dustymabe commented Jan 4, 2024

Uh oh!

Uh oh!

dustymabe commented Jan 8, 2024

Uh oh!

achilleas-k commented Jan 8, 2024

Uh oh!

dustymabe commented Jan 8, 2024

Uh oh!

mvo5 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mvo5 left a comment

Choose a reason for hiding this comment

Uh oh!

mvo5 left a comment

Choose a reason for hiding this comment

Uh oh!

achilleas-k left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

dustymabe commented Jan 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

achilleas-k commented Jan 9, 2024

Uh oh!

dustymabe commented Jan 9, 2024

Uh oh!

achilleas-k commented Jan 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

achilleas-k left a comment

Choose a reason for hiding this comment

Uh oh!

dustymabe commented Jan 10, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

dustymabe commented Jan 9, 2024 •

edited

Loading

achilleas-k commented Jan 9, 2024 •

edited

Loading