config/fcos: Add extensions param and generate treefile with packages

[jmarrero]

This PR adds the sugar to take a list of extensions (packages) on the butane config and turns them into a treefile which rpm-ostree will consume as part of the derivation/layering enhancement. This specific PR is raised based on the discussions on: coreos/fedora-coreos-tracker#1054

[jmarrero]

@bgilbert is the way I am marshaling the YAML correct? It's OK to add that dependency of yaml.v2@v2.2.2 or am I missing some obvious util?

[jmarrero]

Still seeing:

Missing paths in TranslationSet:
$.storage.files.2.path
$.storage.files.2.contents.source
$.storage.files.2.contents
$.storage.files.2.mode
$.storage.files.2

after merges trying to figure it out.

[bgilbert]

is the way I am marshaling the YAML correct? It's OK to add that dependency of yaml.v2@v2.2.2 or am I missing some obvious util?

We already have a dependency on yaml.v3; can you use that?

Missing paths in TranslationSet:

What does butane -D say?

[jmarrero]

is the way I am marshaling the YAML correct? It's OK to add that dependency of yaml.v2@v2.2.2 or am I missing some obvious util?

We already have a dependency on yaml.v3; can you use that?

Yeah yaml.v3 works, I did not think of searching the other versions in the imports we are already using. Thanks.

Missing paths in TranslationSet:

What does butane -D say?

That output comes from butane -D.
The full output looks like:

TranslationSet: yaml → json
$.ignition → $.ignition
$.version → $.ignition.version
$.storage → $.storage
$.storage.files → $.storage.files
$.storage.files.0 → $.storage.files.0
$.storage.files.0.contents → $.storage.files.0.contents
$.inline → $.storage.files.0.contents.source
$.storage.files.0.mode → $.storage.files.0.mode
$.storage.files.0.path → $.storage.files.0.path
$.storage.files.1 → $.storage.files.1
$.storage.files.1.contents → $.storage.files.1.contents
$.storage.files.1.contents.inline → $.storage.files.1.contents.compression
$.storage.files.1.contents.inline → $.storage.files.1.contents.source
$.storage.files.1.mode → $.storage.files.1.mode
$.storage.files.1.path → $.storage.files.1.path
$.systemd → $.systemd
$.systemd.units → $.systemd.units
$.systemd.units.0 → $.systemd.units.0
$.systemd.units.0.dropins → $.systemd.units.0.dropins
$.systemd.units.0.dropins.0 → $.systemd.units.0.dropins.0
$.systemd.units.0.dropins.0.contents → $.systemd.units.0.dropins.0.contents
$.systemd.units.0.dropins.0.name → $.systemd.units.0.dropins.0.name
$.systemd.units.0.name → $.systemd.units.0.name
$.systemd.units.1 → $.systemd.units.1
$.systemd.units.1.contents → $.systemd.units.1.contents
$.systemd.units.1.enabled → $.systemd.units.1.enabled
$.systemd.units.1.name → $.systemd.units.1.name

Missing paths in TranslationSet:
$.storage.files.2.path
$.storage.files.2.contents.source
$.storage.files.2.contents
$.storage.files.2.mode
$.storage.files.2

Which if I understand is because the translation set from the procressPackages() sets the file at index 0 but after appending it is of index 2. If that is correct do I need to modify that manually or is there other method I am missing that is better than merge to deal with this?

[bgilbert]

You'll want MergeTranslatedConfigs; see below.

[jmarrero]

Working on a test now. Thanks for all the help @bgilbert

[jmarrero]

Moving back to WIP. This depends on the outcome of:
coreos/rpm-ostree#3340

[bgilbert]

LGTM generally, pending tests.

[bgilbert]

Are we sure we won't have additional options related to extensions in the future? This schema doesn't allow for any.

The usual way we do this is:

extensions:
  - name: foo
  - name: bar

[jmarrero]

We are basically adding something like the RHCOS extensions to butane: https://github.com/openshift/machine-config-operator/blob/master/docs/MachineConfiguration.md#rhcos-extensions . But I am not sure how this will evolve. This is still being discussed but for the first pass AFAIK we just care about a list of packages.

[bgilbert]

Once this spec version has been stabilized, we lose the ability to add metadata in this way without making a new top-level field. We could merge this as-is and revisit before stabilization, but if we do, we should make sure to leave clear signposts in the issue tracker so we don't forget. Or we could go for the extensible approach now. It might be worth raising with the team.

[jmarrero]

@cgwalters @jlebon any preference on how to handle this?

[jlebon]

Agree with the concern, but not sure what to recommend since it's hard to predict what we'll need to generalize (container-based extensions? more treefile support? repo-pinned extensions?).

So... I'd say let's roll with this for now and revisit before stabilization. In the worst case, we should be able to change the spec in future versions, right?

[bgilbert]

Discussed this more with @jlebon OOB. If we wanted to add extensibility in future versions, and didn't want to have a major spec bump (i.e. one that requires config changes when bumping the version number), we'd need to deprecate the extensions field and add e.g. extensions2. We've done that before but it's a bit awkward.

I think the maximum possible extensibility is:

extensions:
  packages:
    - name: foo
    - name: bar

which lets us add system-wide extension options, per-package ones, and new extension types:

extensions:
  global-option: z
  packages:
    - name: foo
      package-option: z
    - name: bar
  containers:
    - name: baz

Global options don't play well with config merging, since a child config that adds a package would be affected by global parameters it isn't aware of. New extension types could be implemented as a package field:

extensions:
  - name: foo
    package-option: z
  - name: bar
  - name: baz
    type: container

which is consistent with the design elsewhere in the schema.

I think that's a reasonable middle ground. Let's not block the current PR on this, but as a followup I think we should switch from an array of strings to an array of structs.

[cgwalters]

This is very related to what happened to packages in
https://github.com/coreos/rpm-ostree/blob/main/docs/treefile.md

We ended up adding repo-packages which allows to pin to packages from a specific repo.

--

The idea of type: container is a really interesting idea. Definitely touches on https://www.freedesktop.org/software/systemd/man/systemd-sysext.html etc.

[cgwalters]

In rpm-ostree's treefiles today to be nice we allow multiple packages in single YAML list entry. See for example https://github.com/coreos/fedora-coreos-config/blob/aa50439847f34282b41bf2eb60d478a45d2a27d7/manifests/system-configuration.yaml#L7

Might make sense to do that too to avoid the overhead in the simple cases?

[bgilbert]

We generally don't do that in Ignition configs because it wouldn't work with the config merging logic, which (with one unfortunate exception) doesn't know the semantics of what it's merging. That argument doesn't directly apply here, since this is sugar, but it still doesn't feel very Butane-like. We've favored explicitness over ergonomics when they're in conflict.

[bgilbert]

Followup in #316.

[bgilbert]

Once this spec version has been stabilized, we lose the ability to add metadata in this way without making a new top-level field. We could merge this as-is and revisit before stabilization, but if we do, we should make sure to leave clear signposts in the issue tracker so we don't forget. Or we could go for the extensible approach now. It might be worth raising with the team.

[cgwalters]

One concern @jmarrero mentioned in a chat about this is that we're implicity supporting whatever CLI arguments can be passed to rpm-ostree install. Today that includes e.g. bodhi update URLs as well as direct e.g. https://example.com/foo.rpm.

I think that's probably OK.

This also relates though to a discussion happening in libdnf
rpm-software-management/libdnf#1375

[cgwalters]

Did the question come up of potentially having butane generate default configuration files or kernel arguments when a specific extension is requested? kdump is a great example of this. https://docs.fedoraproject.org/en-US/fedora-coreos/debugging-kernel-crashes/

There may be others where we'd want to drop some config file in /etc.

IOW, should butane ever try to recognize specific extensions, or should we just pass through to rpm-ostree install.

[bgilbert]

One concern @jmarrero mentioned in a chat about this is that we're implicity supporting whatever CLI arguments can be passed to rpm-ostree install. Today that includes e.g. bodhi update URLs as well as direct e.g. https://example.com/foo.rpm.

Can we filter those out e.g. by rejecting slashes? Explicit > implicit, and we can always loosen validation rules later.

IOW, should butane ever try to recognize specific extensions, or should we just pass through to rpm-ostree install.

If certain extensions need additional configuration, we should add explicit sugar for those. Otherwise, validation has no way to know that a particular extension will invoke additional logic in a future Butane release, so a newer Butane config could produce a subtly broken Ignition config with older Butane.

[cgwalters]

Can we filter those out e.g. by rejecting slashes? Explicit > implicit, and we can always loosen validation rules later.

Actually I realized I was wrong, since the change to generate a treefile instead of forking rpm-ostree install, I think only package names/NEVRAs will be supported. (The direct https://example.com/foo.rpm stuff lives in the client binary today)