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

Assemble templates using the new basedOn setting #3072

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jandubois
Copy link
Member

@jandubois jandubois commented Jan 1, 2025

This PR creates initial support for multi-file templates, mostly based on the suggestions I gave in #2520 (reply in thread).

Instance create vs. start stages

The basic idea behind "template embedding" is that when an instance is created, the lima.yaml that is stored in the instance directory will embed settings from additional
base template(s) and external scripts into a single composite document.

Once the instance is created, there are no longer external references and the instance can be started while offline, even if it was created from online template fragments.

Merging default settings from base templates

Merging of templates is controlled by the new basedOn property, which takes the name of
a base template, e.g.

basedOn: template://docker

It is expected that in most instances only a single base template is needed, but it is possible to provide a list:

basedOn:
- variables.yaml
- http://example.com/template.yaml

The base template can in turn have their own base templates. For example my.yaml instance may be based on template://docker, which in turn could be based on template://ubuntu.

Merging is performed by copying every property from the base template that does not yet exist in the main template. For list properties the lists are appended. More details and exceptions are documented below.

Embedding of external provisioning scripts

Both provisioning scripts and probe scripts can now be located with the new file property specifying a template locator (details below). The file will be loaded and inlined under the script property. Therefore file and script are mutually exclusive.

provision:
- mode: user
  file: my-script.sh
probes:
- file: template://docker-probe.sh

Relative template locators

The values of the basedOn and file properties are "template locators". They are the usual local filenames or file://, https://, or template://URLs.

In addition they can also be relative locators: names that don't have a URL scheme and don't start with a /. These names are resolved relative to the template in which they occur (similar to relative href properties in HTML).

basedOn: base.yaml
provision:
- file: script.sh

If this template is loaded as templates/main.yaml, then the referenced template files will be fetched from templates/base.yaml and templates/script.sh.

If the template is loaded as template://main, then the files will be template://base.yaml and template://script.sh.

This means a template using relative locators will always fetch dependencies from the same (relative) location regardless of how it was loaded itself.

To make this work template://base.yaml needs to be equivalent to template://base. And it must be possible to use other file extensions, so template://script.sh loads the script and doesn't look for script.sh.yaml.

limactl template command

This PR adds new options to the limactl template copy subcommand.

  • --verbatim copies a file exactly as is.

  • --embed will merge in all base templates and shell scripts.

  • --fill will apply override.yaml, default.yaml, and builtin defaults.

Without additional options the copy command will copy the file, but turn any relative locators into absolute locators. So the copy of the template will continue to reference the same base templates and scripts. Use the --verbatim option if you intend to copy those files locally as well.

Examples

Embedding a script

Merging in template://default

Detailed merging algorithm

Each template will recusively embed all dependencies in its base templates before merging them in.

Then each property, that does not exist in the main template will be copied over from the base template.

Properties that are lists are appended.

For provision and probes the order is reversed: the base entries are inserted before the main template entries, so that latter are executed last (see #2932 for discussion).

Special rules

The dns entries are not merged. The entries from a base template are only copied when the main template list is empty.

mountTypesSupported will have any duplicate entries removed.

The basedOn property is removed.

For provision and probes the file property is removed, and the actual content of the script is stored in the usual script property.

For minimumLimaVersion and vmOpts.qemy.minimumVersion the values are compared. and the base version is only copied when it is greater than the main version.

Combining list entries

Three list properties (additionalDisks, mounts, and networks) use combining logic based on a unique key (disk name, mount point, and interface name, respectively).

If a latter entry matches the key of an earlier entry, then any missing fields in the earlier entry are filled in from the latter entry, which is then deleted. This matches the logic already used for merging override.yaml and default.yaml.

There is a new wildcard feature now, where * will match any existing key, so can be used to provide defaults for all entries. This is mostly in preparation to use this mechanism for the builtin defaults.

For the combining list mechanism to work, all base templates must be merged (so all lists must contain the complete set of entries). Otherwise the key matching mechanism wouldn't work globally, but only for the parent template at each level.

@jandubois
Copy link
Member Author

This PR still needs some integration tests, but I wanted to create it before 2024 was over. 😄

@jandubois jandubois force-pushed the based-on branch 2 times, most recently from ed038b6 to 757d8f3 Compare January 1, 2025 07:04
It allows a template to be constructed by merging values from
one or more base templates together. This merge process will
maintain all comments from both the template and the bases.

The template is assembled before an instance is created, and
only the combined template is stored as lima.yaml in the instance
directory.

There merging semantics are otherwise similar to how lima.yaml
is combined with override.yaml, defaults.yaml, and the builtin
default values.

Signed-off-by: Jan Dubois <[email protected]>
@@ -7,6 +7,7 @@ import (
)

type LimaYAML struct {
BasedOn BaseTemplates `yaml:"basedOn,omitempty" json:"basedOn,omitempty" jsonschema:"nullable"`
Copy link
Member

Choose a reason for hiding this comment

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

basedOn → base, for simplicity?

Copy link
Member

Choose a reason for hiding this comment

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

I still prefer this to be extensible to support digests

Copy link
Member

Choose a reason for hiding this comment

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

Also probably this should be marked as an experimental in v1.1.

Copy link
Member Author

@jandubois jandubois Jan 1, 2025

Choose a reason for hiding this comment

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

basedOn → base, for simplicity?

I don't really care, but I thought basedOn is more intuitive to somebody who hasn't read the docs: "This template is based on this other template". But maybe base confers the same meaning.

The other argument I have is that basedOn works for both single and multiple arguments:

basedOn: "template://ubuntu"
basedOn: [variables.yaml, base.yaml]

Whereas base: [variables.yaml, base.yaml] looks slightly less right. However, bases: base.yaml is even worse.

Not a strong argument, and we already have various singular/plural issues in our property names already, so I will change this to base, even though I still think basedOn is better. 😉

I still prefer this to be extensible to support digests

We can always add this later, when (if) we actually need it. This PR already uses a custom unmarshaller to treat basedOn: base.yaml the same as basedOn: [base.yaml] (requested by @nirs on Slack, to optimize for the common use case).

It will be trivial to extend it to treat both as basedOn: [{locator: base.yaml}] if we need to.

I already tried to explain why a simple digest per file is not really going to work in #3019 (comment). Let's keep the discussion over there; this should not be required for this PR.

Also probably this should be marked as an experimental in v1.1.

Absolutely. This should remain as experimental until all parts of it have been implemented, and we have gained some experience actually using it, so that we can still change it in incompatible ways.

Copy link
Member

Choose a reason for hiding this comment

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

("compose:" might be an option, but on second thought probably we should just reserve it for the VM equivalent of Docker Compose)

Copy link
Member

Choose a reason for hiding this comment

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

What about inherit:?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm fine with base if you don't like basedOn.

I don't want to invoke the ghost of "multiple inheritance"... 😄

@@ -215,6 +218,7 @@ type Provision struct {
Mode ProvisionMode `yaml:"mode,omitempty" json:"mode,omitempty" jsonschema:"default=system"`
SkipDefaultDependencyResolution *bool `yaml:"skipDefaultDependencyResolution,omitempty" json:"skipDefaultDependencyResolution,omitempty"`
Script string `yaml:"script" json:"script"`
File *string `yaml:"file,omitempty" json:"file,omitempty" jsonschema:"nullable"`
Copy link
Member

Choose a reason for hiding this comment

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

Same, needs to support digest

Copy link
Member Author

Choose a reason for hiding this comment

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

Again, this can be added later when we need it.

Copy link
Member

Choose a reason for hiding this comment

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

Yep, but at least we have to decide whether we will just use the existing file locator structure, or we will have to introduce a new microformatted string

Copy link
Member Author

@jandubois jandubois Jan 1, 2025

Choose a reason for hiding this comment

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

Let's have the discussion over there for now. It is not urgent to get this PR merged; we can take our time to discuss it.

I just want to keep the discussion in a separate "discussion", so it doesn't get buried when this PR gets closed.

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

Successfully merging this pull request may close these issues.

2 participants