Skip to content

ci: add Windows workflow#1

Open
timcoding1988 wants to merge 3 commits into
mainfrom
ci/windows-poc
Open

ci: add Windows workflow#1
timcoding1988 wants to merge 3 commits into
mainfrom
ci/windows-poc

Conversation

@timcoding1988
Copy link
Copy Markdown

@timcoding1988 timcoding1988 commented May 12, 2026

windows workflow

@github-advanced-security
Copy link
Copy Markdown

You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool.

What Enabling Code Scanning Means:

  • The 'Security' tab will display more code scanning analysis results (e.g., for the default branch).
  • Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results.
  • You will be able to see the analysis results for the pull request's branch on this overview once the scans have completed and the checks have passed.

For more information about GitHub Code Scanning, check out the documentation.

Comment thread .github/workflows/windows-machine.yml Fixed
Comment thread .github/workflows/windows-machine.yml Fixed
Comment thread .github/workflows/windows-machine.yml Fixed
Comment thread .github/workflows/windows-machine.yml Fixed
Comment thread .github/workflows/windows-machine.yml Fixed
Comment thread .github/workflows/windows-machine.yml Fixed
Comment thread .github/workflows/windows-machine.yml Fixed
Comment thread .github/workflows/windows-machine.yml Fixed
Comment thread .github/workflows/windows-installer.yml Fixed
Comment thread .github/workflows/windows-installer.yml Fixed
Comment thread .github/workflows/windows-installer.yml Fixed
Comment thread .github/workflows/windows-unit.yml Fixed
Comment thread .github/workflows/windows-unit.yml Fixed
Comment thread .github/workflows/windows-unit.yml Fixed
Comment thread .github/workflows/windows-installer.yml Fixed
Comment thread .github/workflows/windows-unit.yml Fixed
@timcoding1988 timcoding1988 changed the title DNM ci: add Windows Hyper-V PoC workflow DNM ci: add Windows workflow May 12, 2026
Comment thread .github/workflows/windows-installer.yml Fixed
Comment thread .github/workflows/windows-machine.yml Fixed
Comment thread .github/workflows/windows-unit.yml Fixed
@timcoding1988 timcoding1988 changed the title DNM ci: add Windows workflow POC ci: add Windows workflow May 12, 2026
Comment thread .github/workflows/windows.yml Fixed
Comment thread .github/workflows/windows.yml Fixed
Comment thread .github/workflows/windows.yml Fixed
Comment thread .github/workflows/windows.yml Fixed
Comment thread .github/workflows/windows.yml Fixed
Comment thread .github/workflows/windows.yml Fixed
Comment thread .github/workflows/windows.yml Fixed
Comment thread .github/workflows/windows.yml Fixed
@timcoding1988 timcoding1988 force-pushed the ci/windows-poc branch 2 times, most recently from 31ec05e to 8ebf211 Compare May 18, 2026 17:04
Comment thread .github/workflows/windows-unit.yml Fixed
Comment thread .github/workflows/windows.yml Fixed
Comment thread .github/workflows/windows.yml Fixed
Comment thread .github/workflows/windows-unit.yml Fixed
@timcoding1988 timcoding1988 force-pushed the ci/windows-poc branch 4 times, most recently from e38a085 to 19dd1dc Compare May 19, 2026 00:48
Comment thread .github/workflows/windows-unit.yml Outdated
Comment thread .github/workflows/windows-unit.yml Outdated
Comment thread .github/workflows/windows-unit.yml Outdated
Comment thread .github/workflows/windows-unit.yml Outdated
Comment thread .github/workflows/windows.yml
Comment thread .github/workflows/windows.yml Outdated

machine:
name: machine (${{ matrix.provider }})
needs: installer
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Running e2e machine tests doesn't require the installer. The only prerequisites to run machine tests are podman and gvproxy binaries.

That also means that installer and machine tests can be split into 2 separate workflows.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My suggestion was task 1 build the installer and upload, then task 2 machine test depend on it and download the artifact. So the machine tests should resuse all binaries from the installer so we actually test with the final product that users will use, otherwise we will never know if the installer ships broken things.

It also means we only need one task to build and uplaod and then two tasks who download and do not have to recompile again.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Luap99 the problem is that we need to update the source code to do that. e2e tests aren't designed to use the Podman binary from $PATH. Apart from that I agree that it would be better to run the machine e2e tests with a version of Podman installed with the instller.

But, if we do that (and I am +1 to update the code to do it), we need to split the build of the installer and the tests of the installer in two separate tasks. That's because we don't need to wait the completion of the installer tests to run the e2e tests.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well that is what the PODMAN_BINARY env is far already, the test should not have to use $PATH, just set PODMAN_BINARY to wherever the installer installs it.
https://github.com/containers/podman/blob/c995882ecfe9ef45dcf63da17c8d84c3baa04858/pkg/machine/e2e/config_test.go#L114-L116

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we set $PODMAN_BINARY we don't test that the installer updates $PATH. We don't verify the real user scenario. The installer can be broken, but the tests will pass just fine.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well the point of the machine tests is to test that the resulting binaries of the installer work, updating $PATH feels wrong because you can then never be sure what the test is using, could be a different podman from $PATH, maybe not a problem for windows but on linux that can be a problem.

I thought your installer tests cover the exact installer details? I.e. that should check for $PATH config on so on to ensure this part works while the machine tests should ensure they actually can be used to run podman machine

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anyhow I guess we do not need to worry about the fine details to much, we can tweak behaviours later at any time.

I guess we can keep the current logic as is for the initial ci integration and the update the workflows later.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I.e. that should check for $PATH config on so on to ensure this part works while the machine

FWIW my installer tests don't verify the $PATH update. We could do that, but it's not trivial because it requires starting a new session.

Comment thread .github/workflows/windows.yml
Comment thread .github/workflows/windows.yml Outdated
Comment thread .github/workflows/windows.yml Outdated
Comment thread .github/workflows/windows.yml Outdated
@timcoding1988 timcoding1988 marked this pull request as ready for review May 21, 2026 18:15
@timcoding1988 timcoding1988 requested a review from l0rd May 21, 2026 18:17
@timcoding1988
Copy link
Copy Markdown
Author

Copy link
Copy Markdown
Contributor

@l0rd l0rd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few minor comments but other than that LGTM 👍

Comment thread .github/workflows/windows-machine.yml Outdated
Comment thread .github/workflows/windows-machine.yml Outdated
Comment thread .github/workflows/windows-installer.yml Outdated
Comment thread .github/workflows/windows-installer.yml Outdated
@timcoding1988 timcoding1988 requested a review from l0rd May 26, 2026 17:26
@timcoding1988 timcoding1988 force-pushed the ci/windows-poc branch 2 times, most recently from 0f7a7e9 to 598dd94 Compare May 26, 2026 18:26
Signed-off-by: Tim Zhou <tizhou@redhat.com>
Signed-off-by: Tim Zhou <tizhou@redhat.com>
Signed-off-by: Tim Zhou <tizhou@redhat.com>
@timcoding1988 timcoding1988 changed the title POC ci: add Windows workflow ci: add Windows workflow May 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants