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.

Sign up for GitHub

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

Jump to bottom

Clone Kubernetes objects in API Server before encoding them #2089

Merged
merged 1 commit into from
May 3, 2021
Merged

Clone Kubernetes objects in API Server before encoding them #2089

merged 1 commit into from
May 3, 2021

Conversation

highlyunavailable
Copy link
Contributor

@highlyunavailable highlyunavailable commented May 1, 2021 •
edited
Loading

What type of PR is this?

Uncomment only one /kind <> line, hit 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:

This works around a race condition present in the encoder for Kubernetes objects, as detailed in kubernetes #82497.

Which issue(s) this PR fixes:
Closes #2086

Special notes for your reviewer:

An shallow copy is used here instead of a mutex since that is the suggested fix in the Kubernetes PR and I'd rather align with their fix strategy. I've checked for use of this in other places in Agones, but this is the only place we use this specific encoder.

@google-cla google-cla bot added the cla: yes label May 1, 2021
@highlyunavailable highlyunavailable force-pushed the copy-object-before-encode branch 2 times, most recently from 8a2ecfe to 734f0c4 Compare May 1, 2021 18:35
@highlyunavailable
Copy link
Contributor Author

Random suspicion: I believe this comment may have been a symptom of this bug:

// not testing for version/kind, seems not to come through. Strange.
// we're not building anything that needs this, so not our issue.

@highlyunavailable highlyunavailable force-pushed the copy-object-before-encode branch from 734f0c4 to 599645a Compare May 1, 2021 18:38
@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 027a12ac-9fc5-41e6-8a1d-8178a3e43971

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/2089/head:pr_2089 && git checkout pr_2089
  • helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.14.0-47d331f

@highlyunavailable highlyunavailable force-pushed the copy-object-before-encode branch from 599645a to 8876ee8 Compare May 1, 2021 18:56
@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 93d6e184-0598-4f0d-b8a7-2e55d5d7dc82

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/2089/head:pr_2089 && git checkout pr_2089
  • helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.14.0-8a2ecfe

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 1b5aa5d6-8447-4cb5-8b1b-de645e10be64

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/2089/head:pr_2089 && git checkout pr_2089
  • helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.15.0-734f0c4

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 3c6206ac-e599-4945-a6ac-b61bc5554931

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

1 similar comment
@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 3c6206ac-e599-4945-a6ac-b61bc5554931

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

@highlyunavailable highlyunavailable force-pushed the copy-object-before-encode branch from 8876ee8 to 53a2d98 Compare May 1, 2021 19:27
@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: fa930db5-e079-48ce-95fa-9ba6fc9bc3da

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/2089/head:pr_2089 && git checkout pr_2089
  • helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.15.0-599645a

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: fb311c16-4edd-4c53-9b0e-31e851cd8be5

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/2089/head:pr_2089 && git checkout pr_2089
  • helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.15.0-53a2d98

@highlyunavailable
Shallow copy Kubernetes objects in API Server before encoding them
136ba05 
This works around a race condition present in the encoder for
Kubernetes objects, as detailed in
kubernetes/kubernetes#82497

Uses the fix from kubernetes/kubernetes#101123

Fixes googleforgames#2086
@highlyunavailable highlyunavailable force-pushed the copy-object-before-encode branch from 53a2d98 to 136ba05 Compare May 3, 2021 16:19
@google-oss-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: highlyunavailable, roberthbailey

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

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 2b194e37-a1fd-4d2b-ad0d-23ee4f24b393

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/2089/head:pr_2089 && git checkout pr_2089
  • helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.15.0-136ba05

@roberthbailey roberthbailey merged commit db3a1b4 into googleforgames:main May 3, 2021
@sawagh sawagh added this to the 1.15.0 milestone Jun 1, 2021
@sawagh sawagh added area/user-experience Pertaining to developers trying to use Agones, e.g. SDK, installation, etc kind/bug These are bugs. labels Jun 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@roberthbailey roberthbailey roberthbailey approved these changes

@pooneh-m pooneh-m Awaiting requested review from pooneh-m

Assignees

@roberthbailey roberthbailey

Labels
approved area/user-experience Pertaining to developers trying to use Agones, e.g. SDK, installation, etc cla: yes kind/bug These are bugs. lgtm size/M
Projects
None yet
Milestone
1.15.0
Development

Successfully merging this pull request may close these issues.

Nil Reference/massive log spam in Controller [1.13]
5 participants
@highlyunavailable @agones-bot @google-oss-robot @roberthbailey @sawagh