Add Hyper-V option in windows installer

[l0rd]

Here is a list of the changes included with this PR:

• Added a radio-butto to podman.msi GUI to select WSL or Hyper-V
• The checkbox in podman.msi GUI now supports Hyper-V too: when checked Hyper-V is installed as part as the Podman installer (if it's not already installed on the machine).
• Installation creates the configuration file 99-podman-machine-provider.conf under folder %APPDATA\containers\containers.conf.d with the selected machine provider:

  [machine]
  provider="hyperv"
  

• Cirrus CI win_installer_task tests the installation with both hyperv and wsl and verifies the generated configuration file. Uninstallation is tested too.
• The background color of the podman-dialog.png has been slightly changed to match the color of the MSI package checkbox and radio-button.
• podman-setup.exe supports 2 new variables: MachineProvider (valid values are wsl and hyperv) and HyperVCheckbox (valid values are 0 and 1).

Note that podman-setup.exe GUI doesn't allow to choose the provider yet. See the screenshot below and https://github.com/containers/podman/issues/22492

Does this PR introduce a user-facing change?

Windows installer `podman-setup.exe` supports 2 new command line variables:
- `MachineProvider`: to choose the windows machine provider (can be `wsl` or `hyperv`).
- `HyperVCheckbox`: if set to `1` the installation will also install Hyper-V (if it's not installed yet)

[packit-as-a-service[bot]]

Ephemeral COPR build failed. @containers/packit-build please check.

[baude]

LGTM but I dont really have a strong understanding for most of this to accurately review. you need to rebase to pick up the BUD fixes from main.

[l0rd]

LGTM but I dont really have a strong understanding for most of this to accurately review. you need to rebase to pick up the BUD fixes from main.

Thanks. Rebased.

[l0rd]

Should we fix #22411 before 5.1? If we change the location later we might have to consider backwards compatibility problems

Absolutely. If we fix #22411 before 5.1 things will be easier to handle. I am working on that.

Also should there be a third option, not create the config file at all? So that the user can use whatever the default is without having the config file. I think that is better on updates so a users does not accidental overwrite the previous default and then wonders where the machine went.

Adding a third option (wsl/hyperv/none) is clever because it's a simple solution. But if we keep wsl as the radio-button selected by default, most users will go with that, no matter if they are installing for the first time or if they are doing an update. So that won't be effective in my opinion. We should introduce a custom action, executed before the GUI is rendered, that looks for an existing machine provider config. If it finds one, the options in the GUI will be hidden. That's for a distinct PR though.

[Luap99]

Adding a third option (wsl/hyperv/none) is clever because it's a simple solution. But if we keep wsl as the radio-button selected by default, most users will go with that, no matter if they are installing for the first time or if they are doing an update. So that won't be effective in my opinion. We should introduce a custom action, executed before the GUI is rendered, that looks for an existing machine provider config. If it finds one, the options in the GUI will be hidden. That's for a distinct PR though.

Well I mean selecting none as default seems to best option to me. We still default to WSL if nothing is set in containers.conf so it has the same effect.

[l0rd]

Well I mean selecting none as default seems to best option to me. We still default to WSL if nothing is set in containers.conf so it has the same effect.

A none that is actually wsl looks confusing.

I can update this PR so that when selecting wsl, then no config file is created. But I don't like that neither as it unnecessarily couples installer default with machine default. None of those options looks ideal to me and I would rather keep the PR as it is and work on a proper solution (dynamic options) in a distinct PR.

[n1hility]

Well I mean selecting none as default seems to best option to me. We still default to WSL if nothing is set in containers.conf so it has the same effect.

A none that is actually wsl looks confusing.

I can update this PR so that when selecting wsl, then no config file is created. But I don't like that neither as it unnecessarily couples installer default with machine default. None of those options looks ideal to me and I would rather keep the PR as it is and work on a proper solution (dynamic options) in a distinct PR.

It's usually acceptable practice to comment out the default, and only when that differs specify a value. From that perspective you could have two options a default of wsl (no config) and hyperv (config file is placed down). Then you could follow up with a custom action that just selects hyperv as the default if it's specified in the config file.

[l0rd]

It's usually acceptable practice to comment out the default, and only when that differs specify a value. From that perspective you could have two options a default of wsl (no config) and hyperv (config file is placed down). Then you could follow up with a custom action that just selects hyperv as the default if it's specified in the config file.

The reason I am not comfortable with this is that we are going to be in trouble if in the future we decide to make hyper-v the podman machine default (when there is not config file) and we forget to update the installer to create a config file when wsl is selected.

Also we are discussing about a rare case: a user that run the installer with wsl and run it again selecting hyper-v without understanding that he will lose its machine.

And in my opinion the main problem here, is not that we are allowing a config change during installation (that's fine), but that a user cannot see both wsl and hyper machines when running machine ls, and I believe this is going to be addressed anyway.

[Luap99]

It's usually acceptable practice to comment out the default, and only when that differs specify a value. From that perspective you could have two options a default of wsl (no config) and hyperv (config file is placed down). Then you could follow up with a custom action that just selects hyperv as the default if it's specified in the config file.

That won't work well, if you select hyperV the first time then WSL the seond time it would need to remove the file which complicates the installer I asumme??

Also we are discussing about a rare case: a user that run the installer with wsl and run it again selecting hyper-v without understanding that he will lose its machine.

This direction maybe but the other way around first selecting hyperV then the next time leave the default and then all of the sudden things no longer work for the user.

I think it is important to keep in mind that most users will have no idea that a containers.conf was created or even it exists which influences this. The support burden is extremely high on us to figure out what happen and why x no longer works in issues.

Also has anyone looked at how this works via podman-desktop? AFAIK they allow users to update podman and if they just force the default there it might very well overwrite hyperV breaking things unexpectedly.

And in my opinion the main problem here, is not that we are allowing a config change during installation (that's fine), but that a user cannot see both wsl and hyper machines when running machine ls, and I believe this is going to be addressed anyway.

That certainly is one problem but I don't see how/when this is going to change.

[l0rd]

The PR to support system-wide containers config https://github.com/containers/common/pull/1973

[l0rd]

A quick update:

I have updated this PR to verify if HyperV is installed using the windows service vmms (Hyper-V Virtual Machine Management service)

I am testing some changes to create the configuration file only if those conditions are met:

• The file $ProgramFiles/Red Hat/Podman/podman.exe doesn't exist
• The configuration file $ProgramData/containers/containers.conf.d/99-podman-machine-provider.conf doesn't exist

[l0rd]

The PR is updated but we should wait for https://github.com/containers/common/pull/1973 to get vendored in podman in order to merge it.

@Luap99 now the creation of %PROGRAMDATA%\containers\containers.conf.d\99-podman-machine-provider.conf is skipped if one of these conditions are met:

• %PROGRAMDATA%\containers\containers.conf.d\99-podman-machine-provider.conf exists (no matter the content)
• A pre-existing podman.exe file is found in %PROGRAMFILES%/RedHat/Podman
• The user sets the property SkipConfigFileCreation=1 while running setup.exe via the command line

Also if one of the conditions above is met the provider choice is not presented to the user when running the podman.msi GUI:

@n1hility the criteria to check for Hyper-V installation is now to look at the service vmms. And the changes to the podman.wxs are now more important to support the dynamic options on the GUI. To validate that I have tested all the permutations of the variables that I think are affecting the installation:

• selected provider: wsl/hyperv
• install the provider: yes/no
• wsl is pre-installed: yes/no
• hyperv is pre-installed: yes/no
• is there a previous podman installation: yes/no

Also all the logic is now in the podman.msi GUI and not in the setup.exe GUI. In the short term we should update the setup.exe GUI too and simultaneously simplify the msi GUI (e.g. no radio buttons / checkboxes but some debugging text showing the msi properties values) as that GUI is not exposed to the users.

[Luap99]

@Luap99 now the creation of %PROGRAMDATA%\containers\containers.conf.d\99-podman-machine-provider.conf is skipped if one of these conditions are met:

* `%PROGRAMDATA%\containers\containers.conf.d\99-podman-machine-provider.conf` exists (no matter the content)

* A pre-existing `podman.exe` file is found in `%PROGRAMFILES%/RedHat/Podman`

* The user sets the property `SkipConfigFileCreation=1` while running setup.exe via the command line

Sounds good to me.

[l0rd]

Changes to containers/common required by this PR are now vendored in containers/podman. So it's ok to merge when @n1hility approves it.

[n1hility]

Also all the logic is now in the podman.msi GUI and not in the setup.exe GUI. In the short term we should update the setup.exe GUI too and simultaneously simplify the msi GUI (e.g. no radio buttons / checkboxes but some debugging text showing the msi properties values) as that GUI is not exposed to the users.

That sounds ok as long as we aren't distributing the msi. The wsl installation aspect requires it. Also a code signed msi on its own looks like a rogue component (no podman icon).

I assume you tested manually both options just to verify no strange msi states? From a code review perspective it looks good just want to double check you verified it.

[l0rd]

I assume you tested manually both options just to verify no strange msi states? From a code review perspective it looks good just want to double check you verified it.

You mean if I've tested the standalone msi? That was always the first thing I validated as that's fastest to build and test compared to the bundle. For every wsl/hyperv permutation I have tested the msi with GUI and the setup.exe via CLI (as this PR doesn't introduces changes on the bundle template side).

[l0rd]

That sounds ok as long as we aren't distributing the msi. The wsl installation aspect requires it. Also a code signed msi on its own looks like a rogue component (no podman icon).

Agreed. That's a proposal (for a next PR) related to the observation that the standalone msi isn't published on GitHub release page and podman.io.

[n1hility]

/lgtm

[l0rd]

/retest

[openshift-ci[bot]]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: l0rd, Luap99

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

The pull request process is described here

Needs approval from an approver in each of these files:

• ~~OWNERS~~ [Luap99]

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

[l0rd]

/cherrypick v5.1

[openshift-cherrypick-robot]

@l0rd: only containers org members may request cherry picks. If you are already part of the org, make sure to change your membership to public. Otherwise you can still do the cherry-pick manually.

In response to this:

/cherrypick v5.1

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[l0rd]

/cherrypick v5.1

[openshift-cherrypick-robot]

@l0rd: new pull request created: #22836

In response to this:

/cherrypick v5.1

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.