Add health check for gameserver-sidecar. #44
Conversation
|
So this is the sidecar for a GameServer, which means that to have a look and see if the health check is configured (and working) - you will need to deploy one of the example gameservers, and then if you either Assuming you have go and docker installed locally - the easiest is likely the example here: Run To deploy, there is a gameserver.yaml.
Clearly, this would be good if it was documented better! Let me know if you have more questions. |
|
OK thanks, I had tried starting up that example game server, but I guess I didn't know the exact right commands. The docs in the example don't actually say how to launch an instance, so maybe I can update that when I am done. |
|
Added #45 to track writing gameserver documentation. |
There was a problem hiding this comment.
Took this for a spin today, overall looks pretty good, just a couple of things to look at that I found.
Also, spun up a gameserver, and the healthcheck works! 👍
You can see it in the describe:
agon-gameserver-sidecar:
Image: gcr.io/agon-images/gameservers-sidecar:0.1-80b475e
Port: <none>
State: Running
Started: Sun, 07 Jan 2018 00:48:51 +0000
Ready: True
Restart Count: 0
Requests:
cpu: 100m
Liveness: http-get http://:8080/healthz delay=3s timeout=1s period=3s #success=1 #failure=3
Environment:
GAMESERVER_NAME: simple-udp
POD_NAMESPACE: default (v1:metadata.namespace)
| @@ -68,8 +70,13 @@ func main() { | |||
| if isLocal { | |||
| sdk.RegisterSDKServer(grpcServer, &Local{}) | |||
| } else { | |||
| config, err := rest.InClusterConfig() | |||
There was a problem hiding this comment.
This error is never checked. A great tool to check these type of things, which is also baked into the build image, and can be invoked in the shell is gometalinter
For example:
root@fd84b110807c:/go/src/github.com/agonio/agon# gometalinter ./gameservers/sidecar/ --deadline=1h
gameservers/sidecar/main.go:73:11:warning: ineffectual assignment to err (ineffassign)
gameservers/sidecar/main.go:73:11:warning: this value of err is never used (SA4006) (megacheck)There was a problem hiding this comment.
Oh, in case I wasn't clear on this one - we should check the error, and Fatalfif it fails.
It would be bad canonical Go not to process an error 😢
gameservers/sidecar/sidecar_test.go
Outdated
| @@ -93,3 +89,58 @@ func TestSidecarRun(t *testing.T) { | |||
| }) | |||
| } | |||
| } | |||
|
|
|||
| func TestHealthCheck(t *testing.T) { | |||
| fixtures := map[string]struct { | |||
There was a problem hiding this comment.
I don't think we need all these fixtures just to test the healthcheck. Couldn't we just run the Sidecar, and then ping the http handler, like we do in the controller example?
Or maybe there's something I'm missing here?
|
Thanks for testing it out Mark! I still wasn't able to get a game server working last week so I'm glad to see the SC health check working. Added a commit simplifying the health check test. |
gameservers/sidecar/sidecar_test.go
Outdated
| ctx, cancel := context.WithCancel(context.Background()) | ||
| defer cancel() | ||
|
|
||
| go func() { |
There was a problem hiding this comment.
I think we can also get rid of this outside goroutine - I don't think it adds anything to the test. Just keep the inner section go sc.Run() forward.
Then the select down below can also go away as well.
See the controller test:
https://github.com/googleprivate/agon/blob/master/gameservers/controller/controller_test.go#L224-L227
There was a problem hiding this comment.
Done! (On both the config error check and the test changes)
dzlier-gcp
force-pushed
the
health
branch
2 times, most recently
from
343acea
to
4841b6f
Compare
|
I have but one more request! Can you rebase it down to a single commit, and then it's LGTM! (Or if you happy, we can try the "Squash and Merge" button, which I've never tried before, but looks to do exactly the same thing) |
|
Done! |
Adding health check liveness probe to sidecar (#12).
Since sidecars for the GameServers aren't created in yaml config, I added the configuration to the controller startup.
Tested by following these instructions on a Linux machine and creating a test pod. However, I do not see any sidecar resources listed in
kubectl get all, much less a test to see if the health check is working. Output from the command is below. Please let me know how to confirm that the sidecar and health check are working as intended.kubectl get alloutput:kubectl describe po/gameservers-controller-3664581478-8d631output: