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

Use ostree checkout for buildroot #127

Merged
merged 3 commits into from
Nov 14, 2023
Merged

Use ostree checkout for buildroot #127

merged 3 commits into from
Nov 14, 2023

Conversation

dbnicholson
Copy link
Member

The buildroot construction with mmdebstrap is fragile and depends on knowing the apt sources for the OS branch. Instead, use the ostree that's going to be used for the image for the buildroot since that already has the proper apt configuration. That means that the host requires ostree rather than mmdebstrap and rsync. In 2023 I think that's reasonable.

https://phabricator.endlessm.com/T35019

Currently the buildroot is constructed from scratch using `mmdebstrap`.
That requires knowing the exact apt sources that the branch of the OS
uses. Instead, the buildroot can be created from a checkout of the
ostree that's going to be used for the deployment since it already
contains the appropriate apt/dpkg configuration.

That changes the host requirements from `mmdebstrap` and `rsync` to the
`ostree` CLI package. Ideally an `eos` container would be used rather
than the bespoke `chroot` setup, but that's a bigger change for another
day.

https://phabricator.endlessm.com/T35019
@dbnicholson dbnicholson requested a review from wjt November 13, 2023 21:01
The only use of the GPG keys is to validate the buildroot source. With
`mmdebstrap` a full GPG keyring was required. However, the `ostree`
`--gpg-import` option accepts individual keys and builds the remote
trusted keyring on its own. Stop creating the keyring and just pass the
key paths to `ostree`. After that the explicit host `gnupg` dependency
is no longer required (although `ostree` internally uses `gnupg`).

https://phabricator.endlessm.com/T35019
@dbnicholson
Copy link
Member Author

I got a little worried about the buildroot checkout corrupting the ostree objects, so I added a fixup to copy the objects. It's definitely much slower, but it should be safer. It would be nice use an overlayfs or something like that.

Copy link
Member

@wjt wjt left a comment

Choose a reason for hiding this comment

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

This looks great. One less tool to think about, one less thing to keep in sync.

Thinking about this at a high level, and assuming R is the OSTree ref that we are trying to build an image around:

  1. Pull R to /var/cache/eos-image-builder/content/ostree/repo
  2. Deploy R to /var/tmp/eos-image-builder as a mutable copy
  3. Chroot into that mutable copy, bind-mounting /var/cache/eos-image-builder/content into the chroot
  4. In eib_ostree, pull R into /var/cache/eos-image-builder/content/ostree/repo a second time. We hope this will be a no-op but in an edge case R might have been updated in the intervening seconds
  5. Pull R locally into /var/cache/eos-image-builder/tmp/ostree-co/ostree/repo
  6. Deploy R to /var/cache/eos-image-builder/tmp/ostree-co

It feels a bit like there is one too many pull/deploy, particularly in the case where the outermost system is also a deployment of R. But I think all the steps are needed!

'ostree',
f'--repo={repo_path}',
'checkout',
'--force-copy',
Copy link
Member

Choose a reason for hiding this comment

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

I checked the documentation for this argument and it says:

Never hardlink (but may reflink if available)

Oh! for btrfs or xfs or some other COW-capable filesystem.

Comment on lines +418 to +420
keys = sorted(itertools.chain.from_iterable(
glob.iglob(os.path.join(d, '*.asc')) for d in keysdirs
))
Copy link
Member

Choose a reason for hiding this comment

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

Not a criticism of your code, but I really hate how the only two options for flattening [[T]] to [T] in a list/generator comprehension are this or

    keys = sorted(
        x
        for d in keysdirs
        for x in glob.iglob(os.path.join(d, '*.asc'))
    )

neither of which is particularly pleasing on the eye.

Copy link
Member Author

Choose a reason for hiding this comment

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

Totally agree. I find the multiple fors in comprehensions so confusing that I never use them. But then I end up doing something equally ugly like this.

Comment on lines +97 to +99
# Pull the ostree. Ideally the eosminbase ostree would be used to
# minimize the buildroot, but that's not released and would fail with
# --use-production-ostree.
Copy link
Member

Choose a reason for hiding this comment

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

A shame indeed… Maybe we could change that at some point but I don't think we want to block this change on that. (Another way to look at this would be as an incentive to make the OS as small as possible! But the builder never needs GNOME Shell.)

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd hope that in the glorious future you'd just use a privileged container and then you could use the barebones eos image.

@wjt
Copy link
Member

wjt commented Nov 14, 2023

I'm running this locally and it seems to work just fine.

@dbnicholson
Copy link
Member Author

Thinking about this at a high level, and assuming R is the OSTree ref that we are trying to build an image around:

  1. Pull R to /var/cache/eos-image-builder/content/ostree/repo
  2. Deploy R to /var/tmp/eos-image-builder as a mutable copy
  3. Chroot into that mutable copy, bind-mounting /var/cache/eos-image-builder/content into the chroot
  4. In eib_ostree, pull R into /var/cache/eos-image-builder/content/ostree/repo a second time. We hope this will be a no-op but in an edge case R might have been updated in the intervening seconds
  5. Pull R locally into /var/cache/eos-image-builder/tmp/ostree-co/ostree/repo
  6. Deploy R to /var/cache/eos-image-builder/tmp/ostree-co

Tis a bit much. #4 could be skipped, but it should be fast even if the remote did get updated.

@wjt
Copy link
Member

wjt commented Nov 14, 2023

The build succeeded. Let's do it!

@wjt wjt merged commit ec864d1 into master Nov 14, 2023
2 checks passed
@wjt wjt deleted the T35019-ostree-buildroot branch November 14, 2023 16:13
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.

2 participants