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

Add ostree-layers and ostree-overrride-layers #1830

Closed
wants to merge 3 commits into from

Conversation

cgwalters
Copy link
Member

@cgwalters cgwalters commented May 8, 2019

The use case for ostree-layers is to support injecting non-RPM
content in a more flexible way than can be done with add-files,
and also without dropping all the way to split composes.

This starts with support on the compose tree side but down the
line I'd like to make it more convenient to do client side too.

For ostree-override-layers this is mainly a development thing
for tools like coreos-assembler. Rather than building an RPM
we just make install DESTDIR then commit and add to
ostree-override-layers.

@jlebon
Copy link
Member

jlebon commented May 8, 2019

Related discussions in #1826.

@cgwalters
Copy link
Member Author

OK so trying to flesh this out more...there's an obvious chicken-egg (:chicken: ⇆ :egg:) situation here because the layers need SELinux labeling.

Hmm. Probably the right thing is for us to always forcibly relabel them - treat them the same as pkgcache branches.

cgwalters added a commit to cgwalters/coreos-assembler that referenced this pull request May 14, 2019
This is strongly related to coreos/fedora-coreos-config#66

For development iteration, we want the ability to directly overlay
binaries.  This depends on
coreos/rpm-ostree#1830
which adds rpm-ostree ability to inject ostree refs into the build.

We could implement this in cosa by building an RPM but I think that's
silly - we end up compressing the data only to immediately decompress it.
Plus, while there are advantages to having entries for files in the RPM
database, in practice we're lying about having these files built by RPM.
So let's just directly generate an OSTree commit from them and inject
that.

(In practice for *this* use case rpm-ostree could actually learn
 a way to directly consume a directory, but indirecting via OSTree
 means it's equally easy to manage pre-generated ostree branches too
 in an efficient way rather than re-committing each time)
@cgwalters
Copy link
Member Author

Downstream consumer in coreos/coreos-assembler#521

@cgwalters
Copy link
Member Author

Hmm. Probably the right thing is for us to always forcibly relabel them - treat them the same as pkgcache branches.

This is done now indirectly by just not populating the devino cache.

@cgwalters
Copy link
Member Author

Bigger picture one thing that excites me about this is that it's pulling rpm-ostree in the other direction - we support RPMs in a lot of ways but ironically don't make it easy to consume ostree refs. This helps to fix that.

@cgwalters cgwalters changed the title WIP: add ostree-layers WIP: add ostree-layers and ostree-overrride-layers May 14, 2019
Copy link
Member

@jlebon jlebon left a comment

Choose a reason for hiding this comment

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

Nice! Looks good to me overall. Surprisingly small patchset.

Minor bikeshed: instead of ostree-layers and ostree-override-layers, how about just ostree-overlays and ostree-overrides? Matches the wording elsewhere in the CLI/codebase.

src/libpriv/rpmostree-core.c Show resolved Hide resolved
@rh-atomic-bot
Copy link

☔ The latest upstream changes (presumably 0c80aa9) made this pull request unmergeable. Please resolve the merge conflicts.

@cgwalters
Copy link
Member Author

Minor bikeshed: instead of ostree-layers and ostree-override-layers, how about just ostree-overlays and ostree-overrides? Matches the wording elsewhere in the CLI/codebase.

The difference between overlay and override feels very subtle; how about ostree-layers and ostree-overrides? Or maybe ostree-refs and ostree-override-refs?

@dustymabe
Copy link
Member

Minor bikeshed: instead of ostree-layers and ostree-override-layers, how about just ostree-overlays and ostree-overrides? Matches the wording elsewhere in the CLI/codebase.

The difference between overlay and override feels very subtle; how about ostree-layers and ostree-overrides? Or maybe ostree-refs and ostree-override-refs?

What about just enhancing add-files to support directories? Isn't that what we're doing here ?

related: coreos/coreos-assembler#521 (comment)

@cgwalters
Copy link
Member Author

What about just enhancing add-files to support directories? Isn't that what we're doing here ?

Yeah, I'd be fine doing that too I think, but: accepting content as ostree commits is just generally more flexible. Plus once we have this I'd like to rework add-files to be implemented in terms of accepting automatically-generated ostree commits.

@cgwalters
Copy link
Member Author

accepting content as ostree commits is just generally more flexible

To give a concrete example, rather than us walking all the files each time and checksumming them, a custom build system could fetch the ostree refs via HTTP. (That's really the next step here, support ostree-refs-with-remotes the same way we can take RPMs and .repo files.)

@dustymabe
Copy link
Member

What about just enhancing add-files to support directories? Isn't that what we're doing here ?

Yeah, I'd be fine doing that too I think,

I'm mostly thinking of this as a user who has wanted this feature. From that end user perspective the key in the yaml would make most sense as an extension of add-files.

but: accepting content as ostree commits is just generally more flexible.

I think I agree here. Is this one step towards #442 ?

Plus once we have this I'd like to rework add-files to be implemented in terms of accepting automatically-generated ostree commits.

Sounds logical.

@cgwalters
Copy link
Member Author

OK rebased 🏄‍♂️ and now with tests ✔️.

@cgwalters cgwalters changed the title WIP: add ostree-layers and ostree-overrride-layers Add ostree-layers and ostree-overrride-layers May 31, 2019
@cgwalters
Copy link
Member Author

And now updated with docs ✍️

Copy link
Member

@jlebon jlebon left a comment

Choose a reason for hiding this comment

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

Just some tiny nits! Also looks like one r too many in that commit title... unless you're going for the pirate accent. ☠️

src/app/rpmostree-compose-builtin-tree.c Show resolved Hide resolved
tests/compose-tests/test-misc-tweaks.sh Outdated Show resolved Hide resolved
The use case for `ostree-layers` is to support injecting non-RPM
content in a more flexible way than can be done with `add-files`,
and also without dropping all the way to split composes.

This starts with support on the `compose tree` side but down the
line I'd like to make it more convenient to do *client* side too.

For `ostree-override-layers` this is mainly a development thing
for tools like coreos-assembler.  Rather than building an RPM
we just `make install DESTDIR` then commit and add to
`ostree-override-layers`.
@cgwalters
Copy link
Member Author

All comments should be addressed ⬆️

@jlebon
Copy link
Member

jlebon commented Jun 7, 2019

@rh-atomic-bot r+ 2405a7b

@rh-atomic-bot
Copy link

⚡ Test exempted: pull fully rebased and already tested.

@cgwalters
Copy link
Member Author

One next step I want to take is something like this client side:

$ rpm-ostree install ostree://my-ostree-ref

Where a user could overlay arbitrary content generated locally as well as

$ rpm-ostree install ostree://someremote:someref

To install remote content.

Tricky issues here involve cases where ordering matters, etc. But it'd be an advanced feature.

cgwalters added a commit to cgwalters/coreos-assembler that referenced this pull request Jun 10, 2019
This is strongly related to coreos/fedora-coreos-config#66

For development iteration, we want the ability to directly overlay
binaries.  This depends on
coreos/rpm-ostree#1830
which adds to rpm-ostree the ability to inject ostree refs into the build.

We could implement this in cosa by building an RPM (like we do for the overlay mentioned above)
but I think that's silly - we end up compressing the data only to immediately decompress it.
Plus, while there are advantages to having entries for files in the RPM
database, in practice we're lying about having these files built by RPM.
So let's just directly generate an OSTree commit from them and inject
that.

In practice for *this* use case rpm-ostree could actually learn
a way to directly consume a directory, but indirecting via OSTree
means it's equally easy to manage pre-generated ostree branches too
in an efficient way rather than re-committing each time.
cgwalters added a commit to cgwalters/coreos-assembler that referenced this pull request Jun 10, 2019
This is strongly related to coreos/fedora-coreos-config#66

For development iteration, we want the ability to directly overlay
binaries.  This depends on
coreos/rpm-ostree#1830
which adds to rpm-ostree the ability to inject ostree refs into the build.

We could implement this in cosa by building an RPM (like we do for the overlay mentioned above)
but I think that's silly - we end up compressing the data only to immediately decompress it.
Plus, while there are advantages to having entries for files in the RPM
database, in practice we're lying about having these files built by RPM.
So let's just directly generate an OSTree commit from them and inject
that.

In practice for *this* use case rpm-ostree could actually learn
a way to directly consume a directory, but indirecting via OSTree
means it's equally easy to manage pre-generated ostree branches too
in an efficient way rather than re-committing each time.
cgwalters added a commit to cgwalters/coreos-assembler that referenced this pull request Jun 11, 2019
This is strongly related to coreos/fedora-coreos-config#66

For development iteration, we want the ability to directly overlay
binaries.  This depends on
coreos/rpm-ostree#1830
which adds to rpm-ostree the ability to inject ostree refs into the build.

We could implement this in cosa by building an RPM (like we do for the overlay mentioned above)
but I think that's silly - we end up compressing the data only to immediately decompress it.
Plus, while there are advantages to having entries for files in the RPM
database, in practice we're lying about having these files built by RPM.
So let's just directly generate an OSTree commit from them and inject
that.

In practice for *this* use case rpm-ostree could actually learn
a way to directly consume a directory, but indirecting via OSTree
means it's equally easy to manage pre-generated ostree branches too
in an efficient way rather than re-committing each time.
jlebon pushed a commit to coreos/coreos-assembler that referenced this pull request Jun 11, 2019
This is strongly related to coreos/fedora-coreos-config#66

For development iteration, we want the ability to directly overlay
binaries.  This depends on
coreos/rpm-ostree#1830
which adds to rpm-ostree the ability to inject ostree refs into the build.

We could implement this in cosa by building an RPM (like we do for the overlay mentioned above)
but I think that's silly - we end up compressing the data only to immediately decompress it.
Plus, while there are advantages to having entries for files in the RPM
database, in practice we're lying about having these files built by RPM.
So let's just directly generate an OSTree commit from them and inject
that.

In practice for *this* use case rpm-ostree could actually learn
a way to directly consume a directory, but indirecting via OSTree
means it's equally easy to manage pre-generated ostree branches too
in an efficient way rather than re-committing each time.
jlebon added a commit to jlebon/coreos-assembler that referenced this pull request Jun 12, 2019
So now that we have OSTree layer capabilities in rpm-ostree
(coreos/rpm-ostree#1830), I think it
makes more sense overall to make the overlay an OSTree layer.

The fact that it *is* an RPM, but differs from all the other RPMs in
various ways will (and already has) come up multiple times when we talk
about signing, changelogs, lockfiles, overrides, etc... (See [1] and [2]
for some already filed issues). Switching to an OSTree layer sidesteps
many of those issues.

Of course, we do lose some things from moving away from RPM:
1. it's harder to determine from where the overlay files come from on a
   host system (see [3] related to this)
2. invisible to `rpm-ostree db diff` (but then again, the diff we
   currently get isn't helpful)
3. not possible to issue an advisory for it to be consumed like the rest
   of the advisories (though with the current approach, making use of
   this without going through Bodhi would've required some non-trivial
   amount of work too)

Overall though, I think those are things we could (and should strive to)
fix in rpm-ostree rather than avoiding layers entirely.

[1] coreos#544
[2] coreos#553 (comment)
[3] coreos/rpm-ostree#1789
jlebon added a commit to jlebon/coreos-assembler that referenced this pull request Jun 14, 2019
So now that we have OSTree layer capabilities in rpm-ostree
(coreos/rpm-ostree#1830), I think it
makes more sense overall to make the overlay an OSTree layer.

The fact that it *is* an RPM, but differs from all the other RPMs in
various ways will (and already has) come up multiple times when we talk
about signing, changelogs, lockfiles, overrides, etc... (See [1] and [2]
for some already filed issues). Switching to an OSTree layer sidesteps
many of those issues.

Of course, we do lose some things from moving away from RPM:
1. it's harder to determine from where the overlay files come from on a
   host system (see [3] related to this)
2. invisible to `rpm-ostree db diff` (but then again, the diff we
   currently get isn't helpful)
3. not possible to issue an advisory for it to be consumed like the rest
   of the advisories (though with the current approach, making use of
   this without going through Bodhi would've required some non-trivial
   amount of work too)

Overall though, I think those are things we could (and should strive to)
fix in rpm-ostree rather than avoiding layers entirely.

[1] coreos#544
[2] coreos#553 (comment)
[3] coreos/rpm-ostree#1789
jlebon added a commit to coreos/coreos-assembler that referenced this pull request Jun 14, 2019
So now that we have OSTree layer capabilities in rpm-ostree
(coreos/rpm-ostree#1830), I think it
makes more sense overall to make the overlay an OSTree layer.

The fact that it *is* an RPM, but differs from all the other RPMs in
various ways will (and already has) come up multiple times when we talk
about signing, changelogs, lockfiles, overrides, etc... (See [1] and [2]
for some already filed issues). Switching to an OSTree layer sidesteps
many of those issues.

Of course, we do lose some things from moving away from RPM:
1. it's harder to determine from where the overlay files come from on a
   host system (see [3] related to this)
2. invisible to `rpm-ostree db diff` (but then again, the diff we
   currently get isn't helpful)
3. not possible to issue an advisory for it to be consumed like the rest
   of the advisories (though with the current approach, making use of
   this without going through Bodhi would've required some non-trivial
   amount of work too)

Overall though, I think those are things we could (and should strive to)
fix in rpm-ostree rather than avoiding layers entirely.

[1] #544
[2] #553 (comment)
[3] coreos/rpm-ostree#1789
@cgwalters
Copy link
Member Author

Followup issue in #1857

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants