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

Use containerd runtime based Windows image #2910

Merged
merged 4 commits into from
Feb 1, 2023

Conversation

gongmax
Copy link
Collaborator

@gongmax gongmax commented Jan 18, 2023

What type of PR is this?

Uncomment only one /kind <> line, press enter to put that in a new line, and remove leading whitespace from that line:

/kind breaking
/kind bug
/kind cleanup
/kind documentation
/kind feature
/kind hotfix

What this PR does / Why we need it:
With Kubernetes 1.24, GKE no longer supports node images with the dockershim runtime, which includes windows_ltsc

Which issue(s) this PR fixes:

Closes #2582

Special notes for your reviewer:
Tested by run make gcloud-test-cluster with GCP_CLUSTER_NODEPOOL_WINDOWSINITIALNODECOUNT greater than 0 to create the Windows node pool

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: f1b8195e-c5fd-4eb4-92b2-50f158fe526f

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:

  • git fetch https://github.com/googleforgames/agones.git pull/2910/head:pr_2910 && git checkout pr_2910
  • helm install agones ./install/helm/agones --namespace agones-system --agones.image.release=us-docker.pkg.dev/agones-images/ci --set agones.image.tag=1.29.0-139d6ca-amd64

@markmandel
Copy link
Member

I'm assuming we're just manually testing Agones with this containerd nodes for now? If so, I assume everything worked?

@gongmax
Copy link
Collaborator Author

gongmax commented Jan 18, 2023

I'm assuming we're just manually testing Agones with this containerd nodes for now? If so, I assume everything worked?

I only tested the windows node pool can be created with this image. Do you want me to test by running windows GameServers on these nodes? I don't think we have any e2e test covering windows GameServer.

@markmandel
Copy link
Member

So I think there are two things to test:

  1. Does the Agones sidecar images work on these nodes
  2. Can you take an example, such as the Xonotic windows example, and see if that still works?

@markmandel
Copy link
Member

Just to be extra clear - happy to approve this once we've done manual tests that work (or realise that everything is broken, and have a work stream to fix it).

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 57f57065-6ea3-4c9d-b7c2-2eae5b3e43c5

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:

  • git fetch https://github.com/googleforgames/agones.git pull/2910/head:pr_2910 && git checkout pr_2910
  • helm install agones ./install/helm/agones --namespace agones-system --agones.image.release=us-docker.pkg.dev/agones-images/ci --set agones.image.tag=1.29.0-139d6ca-amd64

@markmandel
Copy link
Member

Just checking in, see if we have done any manual tests on this?

@gongmax
Copy link
Collaborator Author

gongmax commented Jan 31, 2023

Hey Mark, sorry for the delay! I did some manual tests but the game server failed the health check. But the same issue also happens for the older Agones version (1.25) which support the k8s version that allows docker based windows image. I didn't get chance to look deeper into the health check failure yet.

@gongmax
Copy link
Collaborator Author

gongmax commented Jan 31, 2023

I can hold on this PR and put some effort to figure out why the example game server fails health check. Or we can merge this PR first to fix the not-working windows node pool template. Either way, I believe the example game server health check failure is a separate issue which already exists for a while.

@markmandel
Copy link
Member

No worries on delay, just checking in!

Fun question - did the Agones sidecar fail to run? (I'm assuming not, because the health check failed?) if the sidecar ran, then I think we are good to merge this 👍🏻

@gongmax
Copy link
Collaborator Author

gongmax commented Jan 31, 2023

The container agones-gameserver-sidecar can actually start without any issue.

@markmandel
Copy link
Member

Then unless anyone has objections, I concur with your earlier statement. Let's merge this, and return to the unhealthy game server issue at a later date 👍🏻 Sound good?

@gongmax
Copy link
Collaborator Author

gongmax commented Jan 31, 2023

SGTM

@google-oss-prow google-oss-prow bot added the lgtm label Jan 31, 2023
@google-oss-prow
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gongmax, 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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@markmandel markmandel enabled auto-merge (squash) January 31, 2023 23:51
@google-oss-prow google-oss-prow bot removed the lgtm label Jan 31, 2023
@google-oss-prow
Copy link

New changes are detected. LGTM label has been removed.

@markmandel
Copy link
Member

@gongmax can you please file an issue with the example windows game server failing? Since you have the most information 😄

@gongmax
Copy link
Collaborator Author

gongmax commented Jan 31, 2023

Sure, will do!

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: ecf2f928-8d32-4ec0-b19e-35a35ce9794b

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 87b68291-4630-4818-9f7d-4bdc81e60aad

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:

  • git fetch https://github.com/googleforgames/agones.git pull/2910/head:pr_2910 && git checkout pr_2910
  • helm install agones ./install/helm/agones --namespace agones-system --agones.image.release=us-docker.pkg.dev/agones-images/ci --set agones.image.tag=1.30.0-972bc10-amd64

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 59ffd149-f0de-4e35-9b9a-6feecf0ac206

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:

  • git fetch https://github.com/googleforgames/agones.git pull/2910/head:pr_2910 && git checkout pr_2910
  • helm install agones ./install/helm/agones --namespace agones-system --agones.image.release=us-docker.pkg.dev/agones-images/ci --set agones.image.tag=1.30.0-a514774-amd64

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 9372fded-82fd-43a1-9f85-41d7f7839954

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:

  • git fetch https://github.com/googleforgames/agones.git pull/2910/head:pr_2910 && git checkout pr_2910
  • helm install agones ./install/helm/agones --namespace agones-system --agones.image.release=us-docker.pkg.dev/agones-images/ci --set agones.image.tag=1.30.0-155cc81-amd64

@markmandel markmandel merged commit fa32a31 into googleforgames:main Feb 1, 2023
@Kalaiselvi84 Kalaiselvi84 added this to the 1.30.0 milestone Feb 28, 2023
@Kalaiselvi84 Kalaiselvi84 added the area/tests Unit tests, e2e tests, anything to make sure things don't break label Feb 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved area/tests Unit tests, e2e tests, anything to make sure things don't break size/XS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade windows node image on GKE
4 participants