-
Notifications
You must be signed in to change notification settings - Fork 820
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
#54 Preliminary Windows Image Support #1894
Conversation
Build Succeeded 👏 Build Id: 6044f7ef-eae4-4d1a-a174-4c135db5f9e6 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
cc: @WeetA34 @markmandel |
Build Succeeded 👏 Build Id: b5e52a0a-c86e-478d-a246-bfeb81c0ff43 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
@WeetA34 are you able to take this for a spin? You're probably placed as the best person to see how well it works. |
Hi @markmandel
|
Yes, you would still need to apply it to your own registry, you don't have access to the You can see details in our development guide: https://github.com/googleforgames/agones/blob/master/build/README.md#running-a-test-google-kubernetes-engine-cluster |
Hi I thought 1.10.0-8d42fa4 images have been built and were available on the agones-images registry like the stable images. |
Given this is currently just a PR, I'm not sure how that would have happened 😄 If you are able to read the description of the PR, and see if it works for you to build the images as expected and you are able to use them -- would be very useful to get your feedback. |
The PR will not build the Windows images by default you have to specifically run the new make file commands. and run the |
|
Ok will try this tomorrow |
Thx to both of you |
You'll multiple images for the same thing. @markmandel I don't have really any more cycles to work on this. Can someone take this PR over? |
I honestly don't know anyone who has the knowledge you do here 😬 |
@markmandel The PR in its current form won't break Agones. I can add some instructions on how to build the binaries and containers. Though it's been over a week and no one has reviewed it so it's not clear if this is a priority for Agones. @markmandel @WeetA34 if you have a list of artifacts you want to be built on Windows I might be able to slip that in (untested). |
Hello, |
I don't know about WSL2 but it should work in a Linux VM. I cannot test this because my WSL2 environment is tainted (I run make on the host). |
Build Succeeded 👏 Build Id: 37eb489e-0c04-46e5-aad1-de72b847272d The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
Build Succeeded 👏 Build Id: 2e192be7-e21c-49e7-8715-d5ee15a40ab4 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
Build Failed 😱 Build Id: bc85ea9c-ec0b-4ded-87cb-93b485d06b1f To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Failed 😱 Build Id: 1f9871bf-33c3-48a9-9a88-d3acd616e5d6 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Expected failure, or flakiness?
|
Build Succeeded 👏 Build Id: 958d2f7f-af51-442d-a91d-cd3202c9645e The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
Build Succeeded 👏 Build Id: 2b958572-1a03-4cfd-9b44-75e153c75e9c The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
Did a quick smoke test with Nice first step! Approving! |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jeremyje, markmandel 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 |
New changes are detected. LGTM label has been removed. |
Build Succeeded 👏 Build Id: 4fe30f3a-db1f-42a5-8a17-e87ad3ea4ef3 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
Hi @jeremyje @markmandel, Regarding agones-sdk windows image, you can get a smaller image by using @EricFortin 's docker file Thank you to both of you |
On a second workstation which is behind a proxy, i had to add ARG in build-image/Dockerfile.
edit: I retried this morning and it worked |
I've seen this a few times you can see others on the internet as well reasons why this happens off the top of my head.
The way I develop is running a Hyper-V VM that runs Linux and build there. On the Windows host I run in Windows Container mode which allows me to build and run those containers. There's a Xonotic example that is not linked up that should build and run exclusively in a Windows Container environment. The qemu backed buildx context will not build Xonotic because it has powershell in it. |
At the end, it worked fine on both workstations running Docker Desktop 2.5.0.1 with WSL2 integration. |
What type of PR is this?
/kind feature
What this PR does / Why we need it:
This adds some basic Windows container support to Agones.
This PR is focused on getting the Windows support in Agones started rather than be a complete PR.
There's a few changes that Agones needs to properly support Windows:
make WITH_WINDOWS=1
will add Windows container artifacts for sdk-server. This option is disabled by default a follow up PR will be sent to turn it on.sdk-server
any binaries that can run in Windows should be written to the local file system rather than in the context of a base docker container.COPY --from
does not work between Linux and Windows container images.os.platform
cannot easily be set, see Added support for setting OS version in docker manifest annotate. docker/cli#2578 (fixed in docker 20+). The tagged bug goes into detail but since you're only able to target 1 version of Windows I strongly suggest Windows Server 2019 (WINDOWS_LTSC).WINDOWS_VERSION
was introduced to provide a set of Windows versions to target. It is set to1809
which is aliased toltsc2019
(not all Windows images support theltsc2019
tagging).Makefile
in general will need to be updated to have image tagging for Linux container images to have the suffix-linux_amd64
. This will allow you to have multi-arch and multi-platform images going forward. This means that you can use the old references for Agones (no platform/OS modifier in the tag) and it'll pull the correct image that aligns with the host.powershell
orcmd
commands in yourDockerfile.windows
. If you do this you WILL need a Windows host. The alternative is to run all the modifications in a Linux container export it to local file system (as is) and use theCOPY
command in yourDockerfile.windows
.Which issue(s) this PR fixes:
First set of changes for #54
Fixes #110
Special notes for your reviewer:
The
Dockerfile.windows
naming convention is following how Kubernetes is typically introducing Windows support. There's no strong guidance there.The
make
targets are not added to the official build.I did a very simple build pass to see if things compile.
The PR is very verbose and there's a lot of duplication. This is intentional, there are deeper changes to the Agones build system that are needed to properly fix them.