-
Notifications
You must be signed in to change notification settings - Fork 614
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
base: master
Are you sure you want to change the base?
Conversation
This PR still needs some integration tests, but I wanted to create it before 2024 was over. 😄 |
ed038b6
to
757d8f3
Compare
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"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
basedOn → base, for simplicity?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about inherit:
?
There was a problem hiding this comment.
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"` |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
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 additionalbase 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 ofa base template, e.g.
It is expected that in most instances only a single base template is needed, but it is possible to provide a list:
The base template can in turn have their own base templates. For example
my.yaml
instance may be based ontemplate://docker
, which in turn could be based ontemplate://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 thescript
property. Thereforefile
andscript
are mutually exclusive.Relative template locators
The values of the
basedOn
andfile
properties are "template locators". They are the usual local filenames orfile://
,https://
, ortemplate://
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 relativehref
properties in HTML).If this template is loaded as
templates/main.yaml
, then the referenced template files will be fetched fromtemplates/base.yaml
andtemplates/script.sh
.If the template is loaded as
template://main
, then the files will betemplate://base.yaml
andtemplate://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 totemplate://base
. And it must be possible to use other file extensions, sotemplate://script.sh
loads the script and doesn't look forscript.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 applyoverride.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
andprobes
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
andprobes
thefile
property is removed, and the actual content of the script is stored in the usualscript
property.For
minimumLimaVersion
andvmOpts.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
, andnetworks
) 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
anddefault.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.