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

WIP: Use podman pull to fetch containers #215

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

cgwalters
Copy link
Collaborator

Prep in https://github.com/containers/bootc/pull/214Move pull code into deploy


WIP: Use podman pull to fetch containers

See #147 (comment)

With this bootc starts to really gain support for a different backend
than ostree. Here we basically just fork off podman pull to
fetch container images into an alternative root in
/ostree/container-storage,
(Because otherwise basic things like podman image prune would
delete the OS image)

This is quite distinct from our use of skopeo in the ostree-ext project
because suddenly now we gain support for things
implemented in the containers/storage library like zstd:chunked and
OCI crypt.

However...today we still need to generate a final flattened
filesystem tree (and an ostree commit) in order to maintain
compatibilty with stuff in rpm-ostree. (A corrollary to this is
we're not booting into a podman mount overlayfs stack)
Related to this, we also need to handle SELinux labeling.

Hence, we implement "layer squashing", and then do some final
"postprocessing" on the resulting image matching the same logic
that's done in ostree-ext such as etc -> usr/etc and handling /var.

Note this also really wants
ostreedev/ostree#3106
to avoid duplicating disk space.


@cgwalters
Copy link
Collaborator Author

Demo:

$ env RUST_LOG=debug /run/hostsrv/src/github/containers/bootc/target/release/bootc switch --backend container quay.io/cgwalters/ostest
DEBUG Re-executing current process for _ostree_unshared
DEBUG Already in a mount namespace
DEBUG Current security context is unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
DEBUG Copying self to temporary file for re-exec
DEBUG Label for /tmp/.tmpnua4Vn is system_u:object_r:install_exec_t:s0
DEBUG Created "/tmp/.tmpnua4Vn"
DEBUG Re-executing _bootc_selinuxfs_mounted="/tmp/.tmpnua4Vn" "/tmp/.tmpnua4Vn" "switch" "--backend" "container" "quay.io/cgwalters/ostest"
DEBUG Already in a mount namespace
DEBUG Current security context is unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
DEBUG Removing temporary file
DEBUG Pulling docker://quay.io/cgwalters/ostest
Trying to pull quay.io/cgwalters/ostest:latest...
Getting image source signatures
Copying blob 797644653c72 skipped: already exists  
Copying blob 797644653c72 skipped: already exists  
Copying blob ef5675472650 skipped: already exists  
...
Writing manifest to image destination
DEBUG Creating temporary container for c864f541834eaef553a6e49d5a7da8f91a7fec26a7fdd9ce492a004499a4eef6
DEBUG Mounting 6674154c085cda1abf3f17b28c1fede55e74f975ad1acb0ec2d03252c2de340e
DEBUG Merging layer: ostree/container-storage/overlay/1cc5fa9dff7d8c9ce68cb1a635aede85960085b7bf5c6bce2ff91527fa0fb5ac/diff
DEBUG Merging layer: ostree/container-storage/overlay/f16810ac59f484691c15a8f186df29e54ac378e801972e3ca8416896bc882814/diff
DEBUG Merging layer: ostree/container-storage/overlay/705dd7ffd5fea45eee3dbe85688091c696f918746576419d1a22dfa2d666e8b3/diff
...
DEBUG Writing ostree commit
829754790d5c947c4816e15c3e07a11db72c7e64928e5620fa32b9a36f6c8409
Queued for next boot: ostree-image-signed:docker://quay.io/cgwalters/ostest
  Version: 39.20231123.1
  Digest: sha256:f7afa32ff5a924a9660c7ebd0c567cd37d9d85c6598ea08784961e13477d24e8
$

Now, we can also expose every single podman command but operating on our internal root, like:

$ /run/hostsrv/src/github/containers/bootc/target/release/bootc internal-podman images
REPOSITORY                TAG         IMAGE ID      CREATED       SIZE
quay.io/cgwalters/ostest  latest      c864f541834e  27 hours ago  1.24 GB
$

And also for example, we can optimize pushes and pulls between the bootc storage and the default containers-storage in /var/lib/containers. (Particularly if they're on the same filesystem); i.e. we want something like a handy bootc push-to-podman as well as bootc pull-from-podman as sugar for effectively skopeo copy containers-storage:[overlay@/ostree/containers-storage]:foo containers-storage:foo.

Copy link
Member

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

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

@rhatdan PTAL
FYI @giuseppe

lib/src/podman.rs Outdated Show resolved Hide resolved
lib/src/podman.rs Show resolved Hide resolved
lib/src/podman_ostree.rs Show resolved Hide resolved
@cgwalters cgwalters force-pushed the podman-pull branch 5 times, most recently from 91856eb to 0415f12 Compare December 4, 2023 22:28
@@ -0,0 +1,72 @@
//! # Copy of the ostree authfile bits as they're not public
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

OK, #581 merged

@github-actions github-actions bot added the area/install Issues related to `bootc install` label Jun 5, 2024
@jeckersb
Copy link
Contributor

jeckersb commented Jun 5, 2024

Ok, I've pushed my rebased and seemingly-working fork onto the original here so I can continue to iterate here instead of off in my own world. I know there are things that are still half-done or hacked-around that needs cleaned up, but this is at least something people can look at and build and play around with.

ostree_container::store::query_image_commit(repo, &self.ostree_commit)
.map(|v| Some(v.manifest))
}
// TODO: Figure out if we can get the OCI manifest from podman
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think it is possible to fetch a manifest with a podman command; you'd need to fall back to skopeo inspect or something else

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We can continue to use https://github.com/containers/containers-image-proxy-rs/ for this stuff, see this API which is what's used by the ostree-container code today.

We can consider extending the proxy API too with other things we need, as it's a reliable and tested bridge between the Rust and Go sides.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Actually also, we could fork skopeo copy and not podman pull too. That may help keep things clearer even in the short term.

}
// Now recurse
merge_layer(layer, pathbuf, output, state)?;
} else if (src_meta.mode() & libc::S_IFMT) == libc::S_IFCHR && src_meta.rdev() == 0 {
Copy link
Member

Choose a reason for hiding this comment

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

are opaque directories handled (or should they)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think you're right, we need to do that. Huh. So I did this search for Go code referencing trusted.overlay.opaque, it's pretty interesting all the stuff out there...including this windows tar2ext4 code. There's some interesting hits on the Rust side too, like nydus.

//! which means the ostree deduplication is pointless. In theory this is fixable by going back
//! and changing the containers-storage files, but...
//!
//! ## Medium term: Unify containers-storage and ostree with composefs
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

One thought here...we could bite the bullet and add Go code to this project, and vendor c/storage and c/image. It'd give us the ability to even handle patches for those if need be that we release before waiting for podman or skopeo.

It would also help clear the way for us to do things like copy the podman code for quadlet files to do stuff like containers/podman#22785 right away, again without depending on podman changes in the short term.

Notable tradeoffs either way. But, the more I think about it the more I lean that direction. Does anyone have strong feelings about this?

Copy link
Member

Choose a reason for hiding this comment

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

I'd rather stick with the tools. The tools are always released in sync to make sure that no change to c/storage, for instance, causes any unexpected regression. OCP ran into issues because of using different versions of c/storage in CRI-O and Podman.

@@ -138,41 +153,62 @@ pub(crate) fn create_imagestatus(

/// Given an OSTree deployment, parse out metadata into our spec.
#[context("Reading deployment metadata")]
fn boot_entry_from_deployment(
async fn boot_entry_from_deployment(
Copy link
Contributor

Choose a reason for hiding this comment

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

This whole function should probably be reworked, it's really hard to follow.

ImageState::from(*ostree_container::store::query_image_commit(repo, &csum)?)
}
};
//let cached = imgstate.cached_update.map(|cached| {
Copy link
Contributor

Choose a reason for hiding this comment

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

I ignored this while moving things around, needs to be addressed properly.

.map(|d| boot_entry_from_deployment(sysroot, d))
.transpose()
.context("Rollback deployment")?;
let staged = if let Some(d) = deployments.staged.as_ref() {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't love how this changed, but I couldn't figure out a better way to do it since await snuck in here and it doesn't play as well with the chaining+closures.

@@ -439,7 +454,7 @@ async fn upgrade(opts: UpgradeOpts) -> Result<()> {
}
}
} else {
let fetched = crate::deploy::pull(sysroot, imgref, opts.quiet).await?;
let fetched = crate::deploy::pull(sysroot, spec.backend, imgref, opts.quiet).await?;
Copy link
Contributor

Choose a reason for hiding this comment

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

In the branch above here where we handle --check, it doesn't know how to store cached updates properly for the podman backend. This in turn means we don't have a good way to represent that in bootc status (it will always just be None currently)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We have a lot of choice for how to represent this; it strongly relates to the question of image GC though. I think today, podman and c/storage generally handle this by creating containers which hold references to images.

Copy link
Contributor

Choose a reason for hiding this comment

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

This also feels like more reason re: your previous comment to switch to using skopeo instead of podman, since we could use skopeo inspect to only fetch the metadata in the --check case. If I'm following correctly from the way ostree does it today it's similar in that it's just saving the manifest/config.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep

@jeckersb
Copy link
Contributor

jeckersb commented Jun 25, 2024

Something else that needs fixed:

ERROR Status: Computing status: Booted deployment: Reading deployment metadata: Reading manifest data from commit: Missing ostree.manifest-digest metadata on merge commit

I get that if I do a hack/Containerfile build, podman-bootc run that, then bootc switch --backend container quay.io/fedora/fedora-bootc:40, then reboot and finally bootc status.

Nevermind this was just me being temporarily dumb. Of course if I switch to the latest fedora image and then boot into it, the bootc binary there isn't going to know how to handle the podman backend properly.

lib/src/status.rs Outdated Show resolved Hide resolved
cgwalters and others added 5 commits July 2, 2024 11:36
We'll use this even in cases where we don't have the `install`
feature.

Signed-off-by: Colin Walters <walters@verbum.org>
See containers#147 (comment)

With this bootc starts to really gain support for a different backend
than ostree.  Here we basically just fork off `podman pull` to
fetch container images into an *alternative root* in
`/ostree/container-storage`,
(Because otherwise basic things like `podman image prune` would
 delete the OS image)

This is quite distinct from our use of `skopeo` in the ostree-ext project
because suddenly now we gain support for things
implemented in the containers/storage library like `zstd:chunked` and
OCI crypt.

*However*...today we still need to generate a final flattened
filesystem tree (and an ostree commit) in order to maintain
compatibilty with stuff in rpm-ostree.  (A corrollary to this is
we're not booting into a `podman mount` overlayfs stack)
Related to this, we also need to handle SELinux labeling.

Hence, we implement "layer squashing", and then do some final
"postprocessing" on the resulting image matching the same logic
that's done in ostree-ext such as `etc -> usr/etc` and handling `/var`.

Note this also really wants
ostreedev/ostree#3106
to avoid duplicating disk space.

Signed-off-by: Colin Walters <walters@verbum.org>
Signed-off-by: John Eckersberg <jeckersb@redhat.com>
Signed-off-by: John Eckersberg <jeckersb@redhat.com>
@vrothberg
Copy link
Member

Trying to catch up: Which items are open in this PR?

@jeckersb
Copy link
Contributor

jeckersb commented Jul 8, 2024

Trying to catch up: Which items are open in this PR?

Colin and I discussed this a bit last week before the holiday, the first thing I want to land is a prep change that adds the machinery for multiple backends, which initially will have just one implemented backend (the current "OstreeContainer" backend). This will force the backend API to get fleshed out, as well as help decouple the backend bits from everything else so I can (hopefully) spend less time rebasing this as things change around it. I'm going to draft a proposed backend API in a new PR that I will link here, and we can debate the design over there.

Once that's in, we finish cleaning up the ideas here into the second backend implementation. This PR as-is implements the backend via podman pull, however above discussions indicate we probably want to swap that out for skopeo copy/inspect instead. I think ideally those skopeo bits would live either in a reimagined containers-image-proxy-rs or a new sibliing containers-storage-proxy-rs library or similar. For the short-term, in bootc we'll probably just exec out to skopeo similar to how this draft execs podman. Switching to skopeo lets us handle caching update metadata via skopeo inspect without pulling down the payload the same way the current ostree backend works.

There's also the opaque directory issue noted above, which I haven't given any thought to.

I think that covers the known outstanding issues?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/install Issues related to `bootc install` do-not-merge/work-in-progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants