Support overlay.d (and symlinks)

[cgwalters]

EDIT: Version 2

I'd like to encourage the pattern of adding e.g. fedora-coreos-config
as a git submodule, rather than forking it. There are a lot
of subtle things going on here, and in practice no one wants to
"own" it. If someone forks it even to do something as simple as add
packages, it's easy for things to diverge.

For the treefile, I think it works OK to just use include: parent-config/blah.yaml
in the derived treefile; we always expect people doing derivation to
make their own manifest.yaml anyways.

But I want to inherit from the overlay too.

My first idea here was to simply:
ln -sr fedora-coreos-config/overlay overlay
but that failed because (for probably bad reasons) ostree wants
a real directory as input.

Let's make this a bit more general by first adding support
for an overlay.d directory. This allows us to "group"
overlays by purpose; if it helps you could think of them like
"inline packages".

Next, explicitly support symlinks here. So for now for Silverblue
I'm just doing:

mkdir overlay.d
ln -sr fedora-coreos-config/overlay overlay.d/fcos

But perhaps in the future we'll change the FCOS overlay to overlay.d, and
this would allow us to cherry-pick from that in Silverblue and
other derived systems.

[jlebon]

I think the idea makes sense, though it seems odd to me that we're automatically handling the parent overlay, but letting the parent manifest be "manually integrated". Given that we support multiple includes now, I think we could just make coreos-assembler generate a tmp manifest like:

include:
  - parent-config/manifest.yaml
  - manifest.yaml

?

[cgwalters]

I think the idea makes sense, though it seems odd to me that we're automatically handling the parent overlay, but letting the parent manifest be "manually integrated".

Yeah.

Given that we support multiple includes now, I think we could just make coreos-assembler generate a tmp manifest like:

But you're conflating rpm-ostree inheritance with cosa's extensions. Or are you thinking that cosa would parse the rpm-ostree manifests and if any in the chain have an overlay/ directory we pull that in?

[ajeddeloh]

I'd like to encourage the pattern of adding e.g. fedora-coreos-config as a git submodule, rather than forking it.

I assume you mean users adding it themselves, not as a part of this repo?

Taking a step back, maybe we shouldn't encourage either. What if we just said "bind in your config repo at /srv/config" or whatever and left it up to the users for how they want to manage it? It would also simplify cosa init a bit.

[cgwalters]

I assume you mean users adding it themselves, not as a part of this repo?

"This repo" is cosa, and right, I don't think we should add fedora-coreos-config here.

What if we just said "bind in your config repo at /srv/config" or whatever and left it up to the users for how they want to manage it?

Isn't that how it works today? We support that.

To clarify what I'm doing, see

https://github.com/cgwalters/fedora-silverblue-config/tree/f30-coreos-based

And notice the fedora-coreos-config git submodule, and e.g.
https://github.com/cgwalters/fedora-silverblue-config/blob/e20821d64ed67dc86827561574296dc28e4c1b10/fedora-common-ostree.yaml#L8

[jlebon]

But you're conflating rpm-ostree inheritance with cosa's extensions.

I guess that's what I mean. If we introduce a concept like "parent configs", I'd expect the child configs to inherit everything by default. Having it just inherit the overlay seems arbitrary to me. The manifests and overlays were written to work together.

To clarify what I'm doing, see
https://github.com/cgwalters/fedora-silverblue-config/tree/f30-coreos-based

Hmm, that doesn't have a parent-config symlink in it?

Or are you thinking that cosa would parse the rpm-ostree manifests and if any in the chain have an overlay/ directory we pull that in?

Not sure if I follow why it'd need to parse the manifests. It would just accumulate the parent manifest.yamls and overlay/ dirs as it walks the chain, right?

[jlebon]

Maybe a simpler approach here is to fold the overlay/ semantics into the treefile. Then the "parent config" thing is purely a src config concept which cosa doesn't really need to know about.

[cgwalters]

Maybe a simpler approach here is to fold the overlay/ semantics into the treefile. Then the "parent config" thing is purely a src config concept which cosa doesn't really need to know about.

Right...a lot more work in rpm-ostree for that though. And so far rpm-ostree is relatively "un-opinionated" about things, including whether or not you e.g. store the configs in git. Whereas cosa is much more so, including convention over configuration exactly like this overlay/ directory.

[jlebon]

And so far rpm-ostree is relatively "un-opinionated" about things, including whether or not you e.g. store the configs in git.

This wouldn't require rpm-ostree to know about git, though right? It could just be like an add-dirs or something and it does the ostree commit for you. (Which I think Dusty was arguing for at some point).

Right...a lot more work in rpm-ostree for that though.

Right, that was my take on the unopinionated approach. My opinionated cosa-level take is #639 (comment).

I guess I don't see why cosa can't generate a manifest that essentially does what you're doing there: https://github.com/cgwalters/fedora-silverblue-config/blob/e20821d64ed67dc86827561574296dc28e4c1b10/fedora-common-ostree.yaml#L8.

[cgwalters]

This wouldn't require rpm-ostree to know about git, though right? It could just be like an add-dirs or something and it does the ostree commit for you.

But what about e.g. the git timestamp thing?

[cgwalters]

I guess I don't see why cosa can't generate a manifest that essentially does what you're doing there:

The problem is right now we don't want to inherit all of the manifest necessarily. E.g. RHCOS doesn't want moby-engine (and neither IMO does Silverblue by default although it's a much more debatable thing).

[jlebon]

But what about e.g. the git timestamp thing?

I think we could just canonicalize the commit timestamp to epoch too? Actually, we could do that here too.

The problem is right now we don't want to inherit all of the manifest necessarily.

Right, gotcha. So we want to inherit some things but not some other things. To me, that reinforces making it part of the treefile itself where you can better define the exact semantics you want. E.g. this patch is making overlay/ always inherited, but what happens if FCOS wants to add something to the overlay/ that RHCOS and Silverblue don't want? Seems like pure luck that it's not already the case.

I guess we can add more "layers/tiers/-common" type semantics to make sharing easier, as long as it's not at the expense of making hacking on FCOS much harder.

Anyway, no issues getting this in to play with it for now! Though maybe let's address #639 (comment)?

[cgwalters]

I think we could just canonicalize the commit timestamp to epoch too? Actually, we could do that here too.

What I more mean is that today rpm-ostree doesn't try at all to detect if the manifests are in git.

E.g. this patch is making overlay/ always inherited, but what happens if FCOS wants to add something to the overlay/ that RHCOS and Silverblue don't want? Seems like pure luck that it's not already the case.

Yeah...there are two things here. First, in practice most of the stuff in the overlay is "infrastructure". The second is that it's not too hard to e.g. rm or replace some parts of overlay in e.g. a postprocess in a later manifest, but today we don't have support for e.g. removing packages from an included manifest.

If we go this route...we could try to extract a fedora-coreos-core-config that's just (ignition + ostree infrastructure) for example, but not e.g. the fedora-release-coreos package, and have fedora-coreos-config inherit from it?

[cgwalters]

Though, recursive git submodules get hairy, maybe that's too much overhead...

Another short-term approach may be to have overlay.d/{core,fcos}, and e.g. in overlay/core would only be the "core infrastructure" things for supporting (ignition,ostree) systems, and overlay/fcos would be e.g. something that tweaked the FCOS branding or perhaps was related to moby-engine or so.

Or to rephrase this, in fedora-coreos-config we'd have fedora-coreos-core.yaml which matches overlay.d/core - just the core stuff. And inheritance would be done via symlinking in the derived module, something like:

ln -sr fedora-coreos-config/overlay.d/core overlay.d/fcos-core
in silverblue, so we'd get just the core overlay.

[cgwalters]

For now I added the missing -d test, though...it wouldn't be too hard to implement overlay.d instead if people like that idea.

[jlebon]

Hmm, Travis doesn't look happy:

In src/cmdlib.sh line 287:
    if [ -n "${parent_config}" -a -d "${parent_config}/overlay" ]; then
                               ^-- SC2166: Prefer [ p ] && [ q ] as [ p -a q ] is not well defined.
For more information:
  https://www.shellcheck.net/wiki/SC2166 -- Prefer [ p ] && [ q ] as [ p -a q...
make: *** [Makefile:19: src/.cmdlib.sh.shellchecked] Error 1

For now I added the missing -d test, though...it wouldn't be too hard to implement overlay.d instead if people like that idea.

I'm good with just merging this for now if it unblocks you, but yeah we should have more design discussions around sharing things with RHCOS, Silverblue, and IoT. (Would be nice if we had an "experimental" mode here, but I guess technically we haven't done a 1.0.0 release yet 😄).

[cgwalters]

OK, rewrote this with a more general overlay.d and symlink approach!

[cgwalters]

I think we could just canonicalize the commit timestamp to epoch too? Actually, we could do that here too.

Hum, I had thought we were actually using the git timestamp, hadn't noticed you'd just set the epoch. I guess...the timestamp doesn't really matter, I can't offhand think of what we'd gain from making it match the git timestamp.

[cgwalters]

Ooh exciting I broke the new CI. Hmm...

[ajeddeloh]

👍 to overlay.d

[cgwalters]

OK updated and should fix comments and bugs!

[ajeddeloh]

LGTM

[jlebon]

One comment, otherwise LGTM assuming Jenkins CI is happy!

[jlebon]

Hmm, git_timestamp is a local var in the other function though, no? Although ShellCheck isn't complaining; am I missing something?

[cgwalters]

The answer basically is we need to stop writing in shell script...

[cgwalters]

On this topic, https://timryan.org/2018/07/02/moving-from-the-shell-to-rust-with-commandspec.html is interesting...

[jlebon]

Some more thoughts on this in coreos/fedora-coreos-config#180 (comment).

I think we could just canonicalize the commit timestamp to epoch too? Actually, we could do that here too.

What I more mean is that today rpm-ostree doesn't try at all to detect if the manifests are in git.

Just following up on this, I'm not sure if I understand the concern here, so I might be missing something. I think you're talking about git because of the change detection logic, right? Yeah, folding this into rpm-ostree means that it would have to e.g. hash over the directories so it can calculate the state digest. It's unfortunate, but I don't think it's a big deal. (It's similar to the fact that we're still deriving the full libsolv transaction just to get the list of RPMs).

We still retain the advantage of git here by not triggering the pipeline if the git commit of the src config repo didn't change. If it did change, something has to rehash those files to check for a change. Right now, that's cosa via ostree commit, though we don't lose anything by just letting rpm-ostree do it, right?