-
Notifications
You must be signed in to change notification settings - Fork 168
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
Support ostree-format: oci
in image.yaml
#2216
Conversation
47a965b
to
63cd777
Compare
Looks like a gangplank race:
|
@darkmuggle when you get a sec ⬆️ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor comments, but LGTM overall!
We're not making it the default yet, but I'd like to potentially
e.g. switch the FCOSnext
stream or so.
Maybe rawhide
first would make more. (edit: ...sense :) )
src/cosalib/cmdlib.py
Outdated
build_repo = os.path.join(repo, '../../cache/repo-build') | ||
subprocess.check_call(['sudo', 'rpm-ostree', 'ex-container', 'import', '--repo', build_repo, | ||
'--write-ref', buildmeta['buildid'], 'oci-archive:' + tarfile]) | ||
subprocess.check_call(['sudo', 'ostree', f'--repo={repo}', 'pull-local', build_repo, buildmeta['buildid']]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we chown -R
the whole repo back here? (See e.g. runcompose_tree
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Urg, good catch. Actually I'd only tested this whole thing in the privileged path. I added some (as yet untested) code to handle unprivileged.
The more I think about this though, the more I feel like the whole tmp/repo
thing was a mistake. Basically, if we're just going to eat the write amplification of making a big tarball (and qcow2) as a general rule, then keeping a tmp/repo
is mostly pointless. All we need is the commit object to do things like rpmdb diffs.
And so e.g. we should then switch create_disk.sh
to run e.g. rpm-ostree ex-container import
instead. Almost everything else operating on tmp/repo
just wants the commit object.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I think all the places which only need the commit object already do that optimization, no? If not, then yeah we shouldn't unpack for that. Everything else though does need the repo on disk to e.g. copy files out/inspect/build new trees and it seems like we'd be wasting an important optimization if we just nuked the repo instead of leaving it around after a build.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For cosa buildfast
at least, I could imagine teaching ostree commit
to take e.g. --base path/to/my.ociarchive
in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it seems like we'd be wasting an important optimization if we just nuked the repo instead of leaving it around after a build.
The problem we hit is that for the disk image we hit issues with qemu reading xattrs over 9p which meant we can't directly read cache/repo-build
. That might be fixed by virtiofs.
I'm not proposing deleting cache/repo-build
but...leaving it as a cache just for rpm-ostree, not for anything else.
I want to dispatch off this in Python code, and Gangplank may want it in Go code, so let's export this so it will be available in child processes.
Part of implementing coreos/fedora-coreos-tracker#812 A whole lot of the story of coreos-assembler is threaded with the tension between ostree and disk images. They have fundamentally different tradeoffs. And now I'm trying to add container images to the mix. The idea of capturing an ostree repo in archive mode as a tarball is a cosa invention. We don't actually ship anything that way. The proposal in the above linked issue is to "productize" support for shipping ostree-in-container, because containers are just slightly fancy tarballs. This patch adds support for: `echo 'ostree-format: oci' >> image.yaml` in the config git. When enabled, the `images/ostree` is replaced with an `oci-archive` format of an "ostree-in-container", which we might shorten to `ostcontainer` or so. The code is updated to call out to rpm-ostree's latest (really ostree-rs-ext's latest) code to perform the export and import. We're not making it the default yet, but I'd like to potentially e.g. switch the FCOS `next` stream or so. The next step after this lands is to add separate code in the pipeline to push the image to a registry. There's also a *lot* of deduplication/rationalization to come later around `cosa upload-oscontainer` etc.
63cd777
to
b2bf6e4
Compare
Part of implementing coreos/fedora-coreos-tracker#812
A whole lot of the story of coreos-assembler is threaded
with the tension between ostree and disk images. They
have fundamentally different tradeoffs. And now I'm trying
to add container images to the mix.
The idea of capturing an ostree repo in archive mode as a tarball
is a cosa invention. We don't actually ship anything that way.
The proposal in the above linked issue is to "productize" support
for shipping ostree-in-container, because containers are just
slightly fancy tarballs.
This patch adds support for:
echo 'ostree-format: oci' >> image.yaml
in the config git.
When enabled, the
images/ostree
is replaced with anoci-archive
format of an "ostree-in-container", which we might shorten to
ostcontainer
or so. The code is updated to call out torpm-ostree's latest (really ostree-rs-ext's latest) code
to perform the export and import.
We're not making it the default yet, but I'd like to potentially
e.g. switch the FCOS
next
stream or so.The next step after this lands is to add separate code in the
pipeline to push the image to a registry.
There's also a lot of deduplication/rationalization to
come later around
cosa upload-oscontainer
etc.