-
Notifications
You must be signed in to change notification settings - Fork 18.6k
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
ci: github action workflow for windows #43479
Conversation
1d4ca7a
to
706d653
Compare
|
4cf0310
to
dd3aaee
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks reasonable to me
Thanks for the review @corhere! About what I said in PR desc:
Does it makes sense to move that in a powershell script instead of the workflow itself? |
5eb7933
to
f3ddf44
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it makes sense to move [the scripting embedded in the workflow] in a powershell script instead of the workflow itself?
I personally don't think so. The scripting is mostly GitHub Actions-specific setup and teardown so I don't think having the ability to execute the scripting verbatim on your local machine would have much utility. I don't see enough value in extracting to powershell scripts to outweigh the cognitive overhead of introducing a new layer of abstraction.
478596d
to
d167bd3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, other than the go version
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
Only failure is
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
still LGTM
let's bring this one in
- What I did
Adds a
windows
workflow for GitHub Actions that would do the same job as Jenkins but fragmented to leverage a bit of caching and compute. There is also:integration-test
job) and we are also close to the user experience.windows-2019
Scripting is embedded in the workflow for now but planned to move it in a dedicated.ps1
file. I have not changed thehack/ci/windows.ps1
file on purpose to not disturb the current pipeline and also to be able to test side by side the behavior of Jenkins/GHA.Build time is a bit longer:
~1h40 with gha: https://github.com/moby/moby/actions/runs/2154295292
~1h30 with jenkins: https://ci-next.docker.com/public/blue/organizations/jenkins/moby/detail/PR-43475/3/pipeline/283
Not too bad as the hardware on Jenkins is "twice" better:
Jenkins: https://ci-next.docker.com/public/blue/organizations/jenkins/moby/detail/PR-43475/3/pipeline/264#step-266-log-31
GHA: https://github.com/moby/moby/runs/5988331677?check_suite_focus=true#step:10:36
We could split in a follow-up the cli integration tests using a dedicated matrix to reduce build time like we do in BuildKit.
Encountered some tests that failed:
TestSlowStdinClosing
(containerd runtime)Already reported: #43012
Now that containerd logs are shown in GHA:
Looks like a race condition:
Seems to be also linked to #33710
TestRestartContainerSuccess
,TestRestartPolicyAfterRestart
Repro everytime on GitHub Runners so might be linked to the OS configuration (maybe HyperV). Skipping the test for now.
- Description for the changelog
Adds github action workflow for windows
cc @corhere @thaJeztah