limayaml: add a param field for defining variables used to customize scripts and other elements within lima.yaml.

[norio-nomura]

The defined variables can be referenced within lima.yaml as {{.Param.Key}}.
The fields where this can be utilized are as follows:

• provision[%d].script
• probes[%d].script
• copyToHost[%d].{guest,host}
• portForwards[%d].{guestSocket,hostSocket}

It also changes:

• limayaml: add a check to verify whether the variables defined in param are actually used.

Following change has been merged as a separated PR: #2501

• limactl start: fix the issue where using --set would overwrite the existing instance's lima.yaml without validation.

Following changes will be opened as a separated PR: #2515

• docker.yaml: add .param.ContainerdImageStore
  By passing the --set .param.ContainerdImageStore=true option to limactl {create,start,edit}, the .features."containerd-snapshotter" option will be enabled in docker/daemon.json inside the VM.
• docker.yaml: add .param.Rootful
  By passing the --set .param.Rootful=true option to limactl {create,start,edit}, Docker inside the VM will run in rootful mode.
• docker-rootful.yaml: make everything common except for setting .param.Rootful=true in docker.yaml.

It might be better to split this into multiple parts, but for now, I’ll open it as a draft.

Thanks,

[norio-nomura]

I have separated the fix for limactl start --set into #2501.

[norio-nomura]

can not reproduce a panic in Unit Test on local machine
https://github.com/lima-vm/lima/actions/runs/10050557033/job/27778685157?pr=2498#step:5:2584

panic: test timed out after 10m0s
...
FAIL github.com/lima-vm/lima/pkg/guestagent/kubernetesservice 600.059s

[norio-nomura]

I also want to include the following in lima.yaml:

mounts:
- location: "~/.cache/ubuntu/{{.Name}}/var/cache/apt"
  mountPoint: "/var/cache/apt"
  writable: true
- location: "~/.cache/ubuntu/{{.Name}}/var/lib/apt"
  mountPoint: "/var/lib/apt"
  writable: true

Should this be a separate PR?

[jandubois]

This PR has 2 warnings from yamllint:

$ yamllint examples/docker.yaml
examples/docker.yaml
  58:14     warning  too few spaces before comment  (comments)

$ yamllint examples/docker-rootful.yaml
examples/docker-rootful.yaml
  58:14     warning  too few spaces before comment  (comments)

You can fix by adding another space before the comments, like

-- mode: user # configure docker under non-root user
+- mode: user  # configure docker under non-root user

Personally I don't really agree with the requirement for 2 spaces between content and comments, and would change .yamllint config instead:

--- .yamllint
+++ .yamllint
@@ -7,6 +7,8 @@ rules:
     indent-sequences: false
   truthy:
     allowed-values: ['true', 'false', 'on', 'off']
+  comments:
+    min-spaces-from-content: 1
   comments-indentation: disable
   document-start: disable
   line-length: disable

[norio-nomura]

updated and rebased

[jandubois]

Not sure if I get the time to review this today, but if I do, then I will skip the examples/docker* changes; I think they should go into a separate PR.

[norio-nomura]

Updated

[norio-nomura]

opened:

• docker{,-rootful}.yaml: Use param in docker templates #2515

[AkihiroSuda]

Thanks

[AkihiroSuda]

@lima-vm/maintainers
Is this safe to merge in v0.23?

[afbjorklund]

I am not a big fan of mixing in upper case, with the rest of the yaml that uses lower case...

Even if it means having to "translate" a param: key: value into a {{Param.Key}} so that they can each look their best

[norio-nomura]

Even if it means having to "translate" a param: key: value into a {{Param.Key}} so that they can each look their best

To reference param: {key: value}, use {{.Param.key}}. There is no functionality to convert key to Key.

[jandubois]

Even if it means having to "translate" a param: key: value into a {{Param.Key}} so that they can each look their best

To reference param: {key: value}, use {{.Param.key}}. There is no functionality to convert key to Key.

I believe there is no way to reference non-exported fields from Go templates, so {{.Param.key}} cannot work.

I think what @afbjorklund meant was that you have this in your lima.yaml:

param:
   containerdImageStore: false
   rootful: false

And we would automatically capitalize the first letter when putting it into the template data struct, so you could reference it as {{.Param.Rootful}}.

Now you could argue that we are already transforming param → .Param, so doing the same for the keys is consistent with that.

Personally I don't like this, and I think param and rootful are semantically different entities, and don't need to be treated the same. I would prefer that the parameters are spelled consistently across the whole lima.yaml file and not magically change their first letter to uppercase.

I would maybe go a completely different way, and use the same conventions I would use for environment variables for params:

param:
   CONTAINERD_IMAGE_STORE: false
   ROOTFUL: false

And then reference them as {{.Param.ROOTFUL}}. While I don't think we should enforce this, it may be something we want to use in the bundled templates.

What do other people think?

Assuming we are not implementing @afbjorklund's suggestion to auto-capitalize param names (I would vote against it), we should catch parameters starting with a lowercase letter in validation.

[norio-nomura]

I believe there is no way to reference non-exported fields from Go templates, so {{.Param.key}} cannot work.

{{.Param.key}} works because Param is a map[string]string.

In my local tests, #2515 actually works when I change it to:

param:
  ContainerdImageStore: true
  rootful: true

and use {{.Param.rootful}} in combination.

Now you could argue that we are already transforming param → .Param, so doing the same for the keys is consistent with that.

The reason it is .Param for the reference is not because text/template is converting it automatically, but because it is being passed to text/template as Param in this line of the code.

[jandubois]

{{.Param.key}} works because Param is a [map[string]string]

Thanks, didn't think of that. In that case I'm even more against automatically capitalizing the first letter of params.

The reason it is .Param for the reference is not because text/template is converting it automatically, but because it is being passed to text/template as Param

I understand. And the same way we could capitalize the first letter of k before we assign the values in the param list with param[k] = v.

I just think we shouldn't.

It is also possible that I misunderstand what @afbjorklund is asking for.

Just to be clear: I think the code should be left the way it is right now.

[jandubois]

Thanks, LGTM, except it needs a comment to show that probes now do template expansion, and the regex for checking if a param is used needs to loosen up.

[jandubois]

@lima-vm/maintainers Is this safe to merge in v0.23?

I think it is, and would like to see it included!

[jandubois]

Thanks, looks good, but can you please squash commits?

[AkihiroSuda]

Thanks

[norio-nomura]

Thanks! 🙏🏻

[afbjorklund]

I understand. And the same way we could capitalize the first letter of k before we assign the values in the param list with param[k] = v.

That was indeed my suggestion, and it was purely aesthetical (there was something "wrong" to me, with Key = value)

Now the params will work like environment variables, and I suppose there is some Lowest Common Denominator in that.

But I seem to always be able to make things worse... :-)