Add MachineHostType specification

[n1hility]

This PR enables an upcoming PR for Windows volume/mount support

Signed-off-by: Jason T. Greene [email protected]

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: n1hility
To complete the pull request process, please assign mheon after the PR has been reviewed.
You can assign the PR to them by writing /assign @mheon in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[rhatdan]

Should this be under MachineConfig?

[n1hility]

Should this be under MachineConfig? (@rhatdan)

My thinking was it shouldn't be because like its peer machine_enabled, it pertains to the host environment the engine is running in, as opposed to the settings in [machine] which instead refer to the nested guests. If it's overloaded this could lead to confuse about which side of the relationship host or guest is supposed to have the setting, granted a user should need to set machine_enabled or machine_type. Happy to move it if you prefer it there regardless.

Perhaps in either case it should be renamed to machine_host_type?

[rhatdan]

I am fine with leaving it where it is. I just wonder if users would get confused with machine content in two different places.

[baude]

we did have another way to detect this ... iirc it was an environment variable being set by the ignition file. lemme see if I can find the reference.

[baude]

i t wasnt an environment exactly.

https://github.com/containers/podman/blob/d106b294b428fbb10f59d4cafe72c3dcaa4e73bb/pkg/machine/ignition.go#L395

[n1hility]

i t wasnt an environment exactly.

https://github.com/containers/podman/blob/d106b294b428fbb10f59d4cafe72c3dcaa4e73bb/pkg/machine/ignition.go#L395

Right this value is effectively just one additional piece of information on top of this same setting. noting that not only is this podman instance running under a machine but which type of machine that is.

I pushed an update that renames this to machine_host_type, in case that helps remove some of the ambiguity the name might imply.

[Luap99]

Maybe I am a bit late for this, but could we use a special file instead, i.e. /etc/containers/podman-machine and write the type into that.
Adding such fields to containers.conf feels awkward to me because we do not expect users to actually be able to change this. (same for the existing field). Instead this should be set once once at init time by podman.

I do not want to force you to change the logic. The field currently works fine and we need to continue to support this for backwards compat anyway. I just want to point out that options in containers.conf are confusing to users when they are never expected to use them.

[rhatdan]

I tend to agree with @Luap99 since I don't see users ever changing the config.

[n1hility]

I like the file better as well. Should the machine_enable value be deprecated in favor of this as well?

[rhatdan]

Yes lets deprecate it. Remove it from documentation and have it checked second.

[n1hility]

cool. Closing this one will adjust the other PR and do a separate change for the deprecation