Add a new container/store module #99

cgwalters · 2021-09-23T22:45:45Z

tests: Use --no-bindings for base commit

Since we're not meaning to fetch this via libostree, using
bindings inhibits native pulls for forthcoming container copy
work.

Add a new container/store module

The initial scope of this project was just "encapsulating" ostree
commits in containers.

However, when doing that a very, very natural question arises:
Why not support deriving from that base image container, and
have the tooling natively support importing it?

This initial prototype code implements that. Here, we still use
the tar::import path for the base image - we expect it to have
a pre-generated ostree commit.

This new container::store module processes layered images and
generates (client side) ostree commits from the tar layers.

There's a whole lot of new infrastructure we need around mapping
ostree refs to blobs and images, etc.

cgwalters · 2021-09-24T14:33:15Z

This will close #12 once it has more tests and validation.

cgwalters · 2021-09-24T14:34:55Z

Open design question: right now I'm trying to strongly differentiate non-layered (i.e. pure ostree encapsulation, ideally GPG signed) from layered (from e.g. Dockerfile style build). But, does that make sense? Should we have a --layered option, or should we just drop the distinction and automatically apply a layered container if we're pointed at one?

cgwalters · 2021-09-24T21:37:49Z

Rebased 🏄 - now depends on #102

And when I tried this in the real world, I hit error: Preparing /etc: Tree contains both /etc and /usr/etc precisely because containers/buildah#3523

[root@cosa-devsh ~]# ostree ls d799d4ce5c3184774d6e73767d9fbb2e26b9d11d8669b252a70d616d44a791c5 /etc
d00755 0 0      0 /etc
-00700 0 0      0 /etc/resolv.conf
[root@cosa-devsh ~]#

So we need to teach our tar importer to drop that junk.

cgwalters · 2021-09-27T18:36:11Z

OK upgrades don't work yet, we're not storing tags correctly.

cgwalters · 2021-09-27T19:34:07Z

OK upgrades don't work yet, we're not storing tags correctly.

Fixed

cgwalters · 2021-10-04T22:18:08Z

Trying to use this to deploy the initial container, we also need a copy API I think so we can schlep the already-downloaded container from the cosa side into the VM without re-pulling.

This also ties into #115

cgwalters · 2021-10-05T17:23:18Z

OK lifting draft on this. There's a huge amount of stuff left to do, but I think iterating on main will be better than having this sit in a PR.

Since we're not meaning to fetch this via libostree, using bindings inhibits native pulls for forthcoming `container copy` work.

The initial scope of this project was just "encapsulating" ostree commits in containers. However, when doing that a very, very natural question arises: Why not support *deriving* from that base image container, and have the tooling natively support importing it? This initial prototype code implements that. Here, we still use the `tar::import` path for the base image - we expect it to have a pre-generated ostree commit. This new `container::store` module processes layered images and generates (client side) ostree commits from the tar layers. There's a whole lot of new infrastructure we need around mapping ostree refs to blobs and images, etc.

lucab

LGTM, though I think @travier was going through reviewing this right now too, so could wait a bit before merging.

lucab · 2021-10-06T16:03:40Z

lib/tests/it/main.rs

@@ -58,7 +58,7 @@ fn generate_test_repo(dir: &Utf8Path) -> Result<Utf8PathBuf> {
        indoc! {"
        cd {dir}
        ostree --repo=repo init --mode=archive
-        ostree --repo=repo commit -b {testref} --bootable --add-metadata-string=version=42.0 --gpg-homedir={gpghome} --gpg-sign={keyid} \


I do understand the problem behind this, but I'm a bit less sure about the nuances/ramifications of it.
Are ref-bindings going to be problematic in the general case when we try to unencapsulate/import arbitrary commits built from an existing pipeline we don't control (yocto/endless/etc)?

lucab · 2021-10-06T16:16:52Z

lib/src/container/store.rs

@@ -0,0 +1,383 @@
+//! APIs for storing (layered) container images as OSTree commits


Just a general note that this module has a bunch of for loops where the inner body is already fully async, but the current await arrangement follows head-of-line blocking. Those should be easy places where to win some speedup through concurrency, it would be good to mark them with clear TODO or XXX comments.

Yeah, though as https://docs.rs/tokio/1/tokio/task/fn.spawn_blocking.html says we should probably use rayon inside loops.

I haven't investigated that.

lucab · 2021-10-06T17:08:12Z

lib/src/container/store.rs

+
+    /// Import a layered container image
+    pub async fn import(mut self, import: PreparedImport) -> Result<CompletedImport> {
+        // First download the base image (if necessary) - we need the SELinux policy


This seems like a reasonable constraint, at least as a start. I do hope we don't get any usecase going against this, in the future.

cgwalters · 2021-10-07T14:18:46Z

I think @travier was going through reviewing this right now too, so could wait a bit before merging.

OK, though since this is all experimental, not critical path stuff we can always do post-merge fixups!

cgwalters force-pushed the layered-import branch from 31f75f7 to a138e92 Compare September 24, 2021 00:45

cgwalters force-pushed the layered-import branch from a138e92 to 3de4008 Compare September 24, 2021 21:36

cgwalters force-pushed the layered-import branch 2 times, most recently from 5ef7337 to 3edff72 Compare September 25, 2021 23:00

cgwalters mentioned this pull request Sep 26, 2021

containers: support derivation #12

Closed

cgwalters force-pushed the layered-import branch from 3edff72 to 17b5b80 Compare September 26, 2021 14:49

cgwalters mentioned this pull request Sep 26, 2021

Use ostree-rs-ext 0.5.0 coreos/rpm-ostree#3139

Merged

lucab mentioned this pull request Sep 27, 2021

lib/tar/write: Translate /etc → /usr/etc by default #104

Merged

cgwalters force-pushed the layered-import branch from 17b5b80 to da2f279 Compare September 27, 2021 19:33

cgwalters force-pushed the layered-import branch 8 times, most recently from aab643e to 40d6816 Compare October 4, 2021 17:30

cgwalters force-pushed the layered-import branch 2 times, most recently from d60de71 to 4458215 Compare October 5, 2021 15:48

cgwalters marked this pull request as ready for review October 5, 2021 17:22

cgwalters force-pushed the layered-import branch from 4458215 to a569c94 Compare October 5, 2021 17:22

cgwalters changed the title ~~WIP: Add a new container/store module~~ Add a new container/store module Oct 5, 2021

cgwalters force-pushed the layered-import branch from a569c94 to b6e69f6 Compare October 5, 2021 22:36

cgwalters added 2 commits October 6, 2021 09:16

tests: Use --no-bindings for base commit

7660711

Since we're not meaning to fetch this via libostree, using bindings inhibits native pulls for forthcoming `container copy` work.

cgwalters force-pushed the layered-import branch from b6e69f6 to 944cf76 Compare October 6, 2021 13:16

lucab approved these changes Oct 6, 2021

View reviewed changes

cgwalters merged commit cc98d32 into ostreedev:main Oct 7, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a new container/store module #99

Add a new container/store module #99

cgwalters commented Sep 23, 2021 •

edited

Loading

cgwalters commented Sep 24, 2021

cgwalters commented Sep 24, 2021

cgwalters commented Sep 24, 2021 •

edited

Loading

cgwalters commented Sep 27, 2021

cgwalters commented Sep 27, 2021

cgwalters commented Oct 4, 2021

cgwalters commented Oct 5, 2021

lucab left a comment

lucab Oct 6, 2021 •

edited

Loading

lucab Oct 6, 2021

cgwalters Oct 6, 2021

lucab Oct 6, 2021

cgwalters commented Oct 7, 2021

		@@ -0,0 +1,383 @@
		//! APIs for storing (layered) container images as OSTree commits

Add a new container/store module #99

Add a new container/store module #99

Conversation

cgwalters commented Sep 23, 2021 • edited Loading

cgwalters commented Sep 24, 2021

cgwalters commented Sep 24, 2021

cgwalters commented Sep 24, 2021 • edited Loading

cgwalters commented Sep 27, 2021

cgwalters commented Sep 27, 2021

cgwalters commented Oct 4, 2021

cgwalters commented Oct 5, 2021

lucab left a comment

Choose a reason for hiding this comment

lucab Oct 6, 2021 • edited Loading

Choose a reason for hiding this comment

lucab Oct 6, 2021

Choose a reason for hiding this comment

cgwalters Oct 6, 2021

Choose a reason for hiding this comment

lucab Oct 6, 2021

Choose a reason for hiding this comment

cgwalters commented Oct 7, 2021

cgwalters commented Sep 23, 2021 •

edited

Loading

cgwalters commented Sep 24, 2021 •

edited

Loading

lucab Oct 6, 2021 •

edited

Loading