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

container: Cache new manifest/config in prepare, add API to query #537

Merged
merged 1 commit into from
Sep 18, 2023

Conversation

cgwalters
Copy link
Member

Closes: #496

In coreos/rpm-ostree#4486 we were working on fixing rpm-ostree upgrade --check with containers.

However, what we really want here is to persist the updated manifest (and config) that we fetch. And if we do that, we might as well just make it part of the current prepare() API so it happens automatically.

In this change, we do so via detached commit metadata. An important thing here is that the data is then automatically lifecycle bound to the merge commit - and the merge commit always changes when we fetch a new manifest.

In order to do an offline query (e.g. in rpm-ostree we want to re-synthesize a higher level summary of the queued update) add an API which allows querying a previously saved cached update.

Hence a flow like this should work:

  • OS boots
  • OS updater does a background "check for updates" via calling prepare()
  • OS updater finds an update, and renders metadata to the user or orchestration system
  • <time passes; OS update is not downloaded - e.g. user is on metered data or whatever>
  • system reboots for other reasons
  • OS updater can re-render the fact that a queued update was found without touching the network
  • User can initiate a full fetch (e.g. including image layers) targeting exactly the previously prepared fetch. This makes things much more race-free; if the image was GC'd in the meantime we correctly fail.

@cgwalters cgwalters marked this pull request as draft September 13, 2023 15:43
Closes: ostreedev#496

In coreos/rpm-ostree#4486 we
were working on fixing `rpm-ostree upgrade --check` with containers.

However, what we really want here is to *persist* the updated
manifest (and config) that we fetch.  And if we do that, we might
as well just make it part of the current `prepare()` API so it
happens automatically.

In this change, we do so via detached commit metadata.  An important
thing here is that the data is then automatically lifecycle
bound to the merge commit - and the merge commit always
changes when we fetch a new manifest.

Then, add this "cached update" metadata to the existing structure
which has image state so it can be conveniently queried *without*
re-fetching.

Hence a flow like this should work:

- OS boots
- OS updater does a background "check for updates" via calling `prepare()`
- OS updater finds an update, and renders metadata to the user
  or orchestration system
- <time passes; OS update is not downloaded - e.g. user is on
   metered data or whatever>
- system reboots for other reasons
- OS updater can re-render the fact that a queued update was
  found *without* touching the network

There's one notable piece that is missing to do conveniently:

- User can initiate a full fetch (e.g. including image layers)
  targeting *exactly* the previously prepared fetch.  This
  makes things much more race-free; if the image was GC'd
  in the meantime we correctly fail.

But it can be done manually by e.g. using a digested pull spec
temporarily.
@cgwalters cgwalters marked this pull request as ready for review September 13, 2023 19:26
@cgwalters
Copy link
Member Author

OK, I think this is probably good...still doing some further testing with rpm-ostree but I think I'm narrowing in on the last mile of bugs there.

@cgwalters
Copy link
Member Author

Thinking about this a bit more, two potential issues:

  • We don't offer a direct API to clear cached updates...not sure anyone will care, but I could imagine it
  • This actually breaks the use case for just checking for updates from a read-only repository, but again, it's not really clear that's a real use case

@jmarrero
Copy link
Member

Why not being able to check for updates from a read-only repo is an issue?
I assume once you rebase to it, you would need to rebase to another one to get new content right? (I am guessing that read-only means the contents of the repo will not change.)

@cgwalters
Copy link
Member Author

The "read only repo" case happens to day on e.g. the FCOS Live ISO. You can do rpm-ostree status, but not rpm-ostree install e.g. Now...eventually we probably should make the repo (and everything else) transiently writable with an overlayfs just like etc but that's a separate ball of wax.

OK well, this doesn't work today (try with e.g. cosa run --qemu-iso fedora-coreos-latest-live.x86_64.iso):

$ rpm-ostree upgrade --check
error: Remounting /sysroot read-write: Permission denied

And so this won't be a regression.

@jmarrero
Copy link
Member

As for "We don't offer a direct API to clear cached updates..."
If people can rm the files if needed, this lgtm.

@cgwalters
Copy link
Member Author

Yeah, one can use the low level ostree API to clear detached metadata entirely at least.

@cgwalters cgwalters merged commit f1afc04 into ostreedev:main Sep 18, 2023
8 checks passed
cgwalters added a commit to cgwalters/bootc that referenced this pull request Jan 2, 2024
In ostreedev/ostree-rs-ext#537 we
added a facility to cache metadata (manifest+config) for
container image updates with an eye for use in rpm-ostree.

The `bootc update --check` code here in bootc was also
updated to use it; but we didn't really expose it back in
the status.

This closes the gap, just bridging the cached update metadata
into status.  We want to do this as opposed to having
`bootc update --check` grow its own API for example.

Signed-off-by: Colin Walters <walters@verbum.org>
cgwalters added a commit to cgwalters/bootc that referenced this pull request Jan 2, 2024
In ostreedev/ostree-rs-ext#537 we
added a facility to cache metadata (manifest+config) for
container image updates with an eye for use in rpm-ostree.

The `bootc update --check` code here in bootc was also
updated to use it; but we didn't really expose it back in
the status.

This closes the gap, just bridging the cached update metadata
into status.  We want to do this as opposed to having
`bootc update --check` grow its own API for example.

Closes: containers#247

Signed-off-by: Colin Walters <walters@verbum.org>
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.

change importer to support caching new manifests/configs without layers
2 participants