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

On Windows avoid installing WSL during an update of Podman #24624

Merged
merged 3 commits into from
Nov 20, 2024

Conversation

l0rd
Copy link
Member

@l0rd l0rd commented Nov 20, 2024

A condition to automatically install the provider (WSL or Hyper-V) was NOT Installed (c.f. the Windows installer Installed property). But that condition is evaluated as true even during an update, and, as a consequence, the provider could be installed during an update, too. Something that we want to avoid. To fix this behavior, we are now using the condition NOT WIX_UPGRADE_DETECTED instead.

In the PR, we have also updated the Windows installer (CLI) tests to execute the update using the default installation options (to match the Windows installer GUI scenario).

Another change introduced by this PR is the default value of the "Install WSLv2 if not present" checkbox. It's now unchecked by default (as was the case for the "Install Hyper-V if not present" checkbox).

image

Does this PR introduce a user-facing change?

None

l0rd added 3 commits November 20, 2024 13:43
The Windows installer GUI has a checkbox to choose if WSL and HyperV
should be installed as part of the installation of Podman. Now, by
default, that checkbox is disabled for both WSL and HyperV.

Signed-off-by: Mario Loriedo <mario.loriedo@gmail.com>
The condition `NOT Installed` had not effect and has been replaced with
`NOT WIX_UPGRADE_DETECTED` that is `true` during installation and
`false` during updates.

The `ExePackage` WSL Kernel Install is also not installed if Podman is
already present.

Signed-off-by: Mario Loriedo <mario.loriedo@gmail.com>
The windows installer tests are command line / non interactive. To test as much as
possible the GUI / interactive scenario (that is what user do), update tests
need to use the installer with the default options. That's because when using the GUI
for an update, changing options is not possible.

Signed-off-by: Mario Loriedo <mario.loriedo@gmail.com>
Copy link
Contributor

openshift-ci bot commented Nov 20, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: l0rd

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:

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 20, 2024
@l0rd l0rd changed the title On windows avoid installing WSL during an update of podman On Windows avoid installing WSL during an update of podman Nov 20, 2024
@l0rd l0rd changed the title On Windows avoid installing WSL during an update of podman On Windows avoid installing WSL during an update of Podman Nov 20, 2024
@l0rd
Copy link
Member Author

l0rd commented Nov 20, 2024

Tests are green, @containers/podman-maintainers PTAL

@mheon
Copy link
Member

mheon commented Nov 20, 2024

LGTM
/cherry-pick v5.3

@openshift-cherrypick-robot
Copy link
Collaborator

@mheon: once the present PR merges, I will cherry-pick it on top of v5.3 in a new PR and assign it to you.

In response to this:

LGTM
/cherry-pick v5.3

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.

@baude
Copy link
Member

baude commented Nov 20, 2024

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 20, 2024
@openshift-merge-bot openshift-merge-bot bot merged commit a6ede19 into containers:main Nov 20, 2024
86 checks passed
@openshift-cherrypick-robot
Copy link
Collaborator

@mheon: new pull request created: #24627

In response to this:

LGTM
/cherry-pick v5.3

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 added a commit to l0rd/podman that referenced this pull request Feb 5, 2025
The Windows installer was able to automatically enable the
Windows features for WSL or HyperV when they were not
already enabled. This PR removes this capability.

Having the installer to automatically install the right prerequiste
(WSL or HyperV) was helpful as users won't have to do it manually to
use Podman after the installation. But it also made the code of
installer more complicated as it needed to manage the installation
of these OS features and a reboot. And we weren't able to automatically
test these scenarios that required a reboot.

In other words the Windows installer, that merely just extracted
some files in a folder, required, to support the installation of
WSL and HyperV, an advanced knowledge of WiX toolkit and of the
Windows Installer SDK, plus contributors-time to manually test
the scenarios that require a reboot.

We decided to remove this capability based on the following reasons:
- We had a couple of regressions in the last month that were hard to
  analyse and fix (containers#24624 and containers#24735)
- Podman maintainers currently have a scarce knowledge of the Windows Installer
  and there is no plan to invest in that
- Manually installing WSL or HyperV is not hard (e.g. run `wsl --install`) and
  are features that admins can manage on their fleet of Windows machines
- Competitors such as Docker Desktop don't automatically install these
  components
- Podman `machine init` currently verifies if WSL and HyperV are installed and
  guide the user to install them when they are not

Signed-off-by: Mario Loriedo <mario.loriedo@gmail.com>
l0rd added a commit to l0rd/podman that referenced this pull request Feb 5, 2025
The Windows installer was able to automatically enable the
Windows features for WSL or HyperV when they were not
already enabled. This PR removes this capability.

Having the installer to automatically install the right prerequiste
(WSL or HyperV) was helpful as users won't have to do it manually to
use Podman after the installation. But it also made the code of
installer more complicated as it needed to manage the installation
of these OS features and a reboot. And we weren't able to automatically
test these scenarios that required a reboot.

In other words the Windows installer, that merely just extracted
some files in a folder, required, to support the installation of
WSL and HyperV, an advanced knowledge of WiX toolkit and of the
Windows Installer SDK, plus contributors-time to manually test
the scenarios that require a reboot.

We decided to remove this capability based on the following reasons:
- We had a couple of regressions in the last month that were hard to
  analyse and fix (containers#24624 and containers#24735)
- Podman maintainers currently have a scarce knowledge of the Windows Installer
  and there is no plan to invest in that
- Manually installing WSL or HyperV is not hard (e.g. run `wsl --install`) and
  are features that admins can manage on their fleet of Windows machines
- Competitors such as Docker Desktop don't automatically install these
  components
- Podman `machine init` currently verifies if WSL and HyperV are installed and
  guide the user to install them when they are not

Signed-off-by: Mario Loriedo <mario.loriedo@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. release-note-none
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants