-
Notifications
You must be signed in to change notification settings - Fork 817
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
Fixes race condition bug with Pod not being scheduled before Ready() #357
Fixes race condition bug with Pod not being scheduled before Ready() #357
Conversation
Build Failed 😱 Build Id: 112175e2-565d-4f70-8bf4-5a3ea7c9d9c1 Build Logs
|
pkg/gameservers/controller_test.go
Outdated
@@ -109,7 +120,7 @@ func TestControllerSyncGameServer(t *testing.T) { | |||
|
|||
err = c.syncGameServer("default/test") | |||
assert.Nil(t, err) | |||
assert.Equal(t, 2, updateCount, "update reactor should twice") | |||
assert.Equal(t, 3, updateCount, "update reactor should twice") |
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.
hum.. thrice? 😄
This fix does several things: 1. Breaks out ip and port setting of the `Status` into its own state, to make implementation and testing easier. 1. Listen for Pod updates, and if a Pod is a `GameServer` pod that has had its `NodeName` set, then queue the backing `GameServer` 1. Finally - if Ready() gets called before any of this, populate the `Status` ip and port values at this time as well. Fixes googleforgames#354
acad386
to
7ba2552
Compare
Build Failed 😱 Build Id: 53501db2-ff21-45fe-bac8-320d81f081e7 Build Logs
|
Build Failed 😱 Build Id: 25a57b4d-5288-4c6d-b678-f2140f3bc6f7 Build Logs
|
Build Failed 😱 Build Id: 018199ba-a1af-4bc2-8ef1-08ad81093b1a Build Logs
|
Build Succeeded 👏 Build Id: df8054fd-4d03-4010-94a3-2d932110c9b2 The following development artifacts have been built, and will exist for the next 30 days:
(experimental) To install this version:
|
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
This fix does several things:
Status
into its own state, to makeimplementation and testing easier.
GameServer
pod that has hadits
NodeName
set, then queue the backingGameServer
Status
ipand port values at this time as well.
Fixes #354