-
Notifications
You must be signed in to change notification settings - Fork 801
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
Fix bug: Disabled health checking not implemented #317
Fix bug: Disabled health checking not implemented #317
Conversation
Build Succeeded 👏 Build Id: 47566091-5c15-40fc-bbfc-f3138c72abf2 The following development artifacts have been built, and will exist for the next 30 days:
(experimental) To install this version:
|
pkg/gameservers/sdkserver_test.go
Outdated
@@ -157,7 +171,7 @@ func TestSidecarHealthLastUpdated(t *testing.T) { | |||
|
|||
sc, err := defaultSidecar(m) | |||
assert.Nil(t, err) | |||
sc.healthDisabled = false | |||
sc.health = v1alpha1.Health{Disabled: true} |
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.
Doesn't it change the intent of the test?
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.
Ooh! Good catch 👍 Lemme check that one out!
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.
Fixed! Incoming shortly.
a715fc9
to
c0b5c72
Compare
Build Failed 😱 Build Id: 07db5664-78d0-4617-abdd-8881ea6f37b2 Build Logs
|
This commit fixes actually two bugs: 1) While we documented that you could disabled health checking we never actually implemented it! This fixes that. Thankfully, the work that has been done to retrieve `GameServer` details through the SDK makes this relatively easy. 2) SDK Server sidecar never had the necessary RBAC permissions to send events to the `GameServer` CRD. This is now fixed as well.
c0b5c72
to
5da96b3
Compare
Build Succeeded 👏 Build Id: 85ababe5-a4c5-487a-90dd-f8ac1bf9e9e0 The following development artifacts have been built, and will exist for the next 30 days:
(experimental) To install this version:
|
This commit fixes actually two bugs:
While we documented that you could disabled health checking we never
actually implemented it! This fixes that. Thankfully, the work that has been
done to retrieve
GameServer
details through the SDK makes this relativelyeasy.
SDK Server sidecar never had the necessary RBAC permissions to send events
to the
GameServer
CRD. This is now fixed as well.