From 1161b400c01256fd6f53e0c6d1bca385b9fbd06a Mon Sep 17 00:00:00 2001
From: Mark Mandel <markmandel@google.com>
Date: Thu, 19 Dec 2019 13:04:10 -0800
Subject: [PATCH] Better error message for Health Check Failure (#1222)

Includes more e2e and unit tests to cover
this scenario.

Should improve experience in #1217
---
 pkg/sdkserver/sdkserver.go      |  1 +
 pkg/sdkserver/sdkserver_test.go | 41 +++++++++++++++++++++++++++++++++
 test/e2e/gameserver_test.go     | 16 +++++++++++++
 3 files changed, 58 insertions(+)

diff --git a/pkg/sdkserver/sdkserver.go b/pkg/sdkserver/sdkserver.go
index 535731389c..02d3fbd84b 100644
--- a/pkg/sdkserver/sdkserver.go
+++ b/pkg/sdkserver/sdkserver.go
@@ -307,6 +307,7 @@ func (s *SDKServer) updateState() error {
 	switch gs.Status.State {
 	case agonesv1.GameServerStateUnhealthy:
 		level = corev1.EventTypeWarning
+		message = "Health check failure"
 	case agonesv1.GameServerStateReserved:
 		s.gsUpdateMutex.Lock()
 		if s.gsReserveDuration != nil {
diff --git a/pkg/sdkserver/sdkserver_test.go b/pkg/sdkserver/sdkserver_test.go
index 9b25267445..81ac3f024d 100644
--- a/pkg/sdkserver/sdkserver_test.go
+++ b/pkg/sdkserver/sdkserver_test.go
@@ -412,6 +412,47 @@ func TestSidecarHealthLastUpdated(t *testing.T) {
 	wg.Wait()
 }
 
+func TestSidecarUnhealthyMessage(t *testing.T) {
+	t.Parallel()
+
+	m := agtesting.NewMocks()
+	sc, err := NewSDKServer("test", "default", m.KubeClient, m.AgonesClient)
+	assert.NoError(t, err)
+
+	m.AgonesClient.AddReactor("list", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) {
+		gs := agonesv1.GameServer{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: "test", Namespace: "default",
+			},
+			Spec: agonesv1.GameServerSpec{},
+			Status: agonesv1.GameServerStatus{
+				State: agonesv1.GameServerStateStarting,
+			},
+		}
+		gs.ApplyDefaults()
+		return true, &agonesv1.GameServerList{Items: []agonesv1.GameServer{gs}}, nil
+	})
+
+	stop := make(chan struct{})
+	defer close(stop)
+
+	sc.informerFactory.Start(stop)
+	assert.True(t, cache.WaitForCacheSync(stop, sc.gameServerSynced))
+
+	sc.recorder = m.FakeRecorder
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	go func() {
+		err := sc.Run(ctx.Done())
+		assert.Nil(t, err)
+	}()
+
+	// manually push through an unhealthy state change
+	sc.enqueueState(agonesv1.GameServerStateUnhealthy)
+	agtesting.AssertEventContains(t, m.FakeRecorder.Events, "Health check failure")
+}
+
 func TestSidecarHealthy(t *testing.T) {
 	t.Parallel()
 
diff --git a/test/e2e/gameserver_test.go b/test/e2e/gameserver_test.go
index 8dc1bb4f08..6c4a813c20 100644
--- a/test/e2e/gameserver_test.go
+++ b/test/e2e/gameserver_test.go
@@ -160,6 +160,22 @@ func TestSDKSetAnnotation(t *testing.T) {
 	assert.NotEmpty(t, gs.ObjectMeta.Annotations[agonesv1.VersionAnnotation])
 }
 
+func TestUnhealthyGameServerAfterHealthCheckFail(t *testing.T) {
+	t.Parallel()
+	gs := defaultGameServer(defaultNs)
+	gs.Spec.Health.FailureThreshold = 1
+
+	gs, err := framework.CreateGameServerAndWaitUntilReady(defaultNs, gs)
+	assert.NoError(t, err)
+
+	reply, err := e2eframework.SendGameServerUDP(gs, "UNHEALTHY")
+	assert.NoError(t, err)
+	assert.Equal(t, "ACK: UNHEALTHY\n", reply)
+
+	_, err = framework.WaitForGameServerState(gs, agonesv1.GameServerStateUnhealthy, time.Minute)
+	assert.NoError(t, err)
+}
+
 func TestUnhealthyGameServersWithoutFreePorts(t *testing.T) {
 	t.Parallel()
 	nodes, err := framework.KubeClient.CoreV1().Nodes().List(metav1.ListOptions{})