Tests update: gameserversets-controllers added missing test cases

pkg/gameserversets/controller.go

@@ -233,7 +233,7 @@ func (c *Controller) creationValidationHandler(review admv1beta1.AdmissionReview
 		}
 		review.Response.Result = &metav1.Status{
 			Status:  metav1.StatusFailure,
-			Message: "GameServerSet update is invalid",
+			Message: "GameServerSet create is invalid",
 			Reason:  metav1.StatusReasonInvalid,
 			Details: &details,
 		}
@@ -500,7 +500,7 @@ func (c *Controller) deleteGameServers(gsSet *agonesv1.GameServerSet, toDelete [
 		gsCopy.Status.State = agonesv1.GameServerStateShutdown
 		_, err := c.gameServerGetter.GameServers(gs.Namespace).Update(gsCopy)
 		if err != nil {
-			return errors.Wrapf(err, "error updating gameserver %s from status %s to Shutdown status.", gs.ObjectMeta.Name, gs.Status.State)
+			return errors.Wrapf(err, "error updating gameserver %s from status %s to Shutdown status", gs.ObjectMeta.Name, gs.Status.State)
 		}
 
 		c.stateCache.forGameServerSet(gsSet).deleted(gs)

pkg/gameserversets/controller_test.go

@@ -16,6 +16,7 @@ package gameserversets
 
 import (
 	"encoding/json"
+	"errors"
 	"net/http"
 	"strconv"
 	"testing"
@@ -32,6 +33,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	admv1beta1 "k8s.io/api/admission/v1beta1"
+	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/watch"
@@ -513,54 +515,97 @@ func TestControllerSyncUnhealthyGameServers(t *testing.T) {
 	gs3.ObjectMeta.DeletionTimestamp = &now
 	gs3.Status = agonesv1.GameServerStatus{State: agonesv1.GameServerStateReady}
 
-	var updatedCount int
+	t.Run("valid case", func(t *testing.T) {
+		var updatedCount int
+		c, m := newFakeController()
+		m.AgonesClient.AddReactor("update", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) {
+			ua := action.(k8stesting.UpdateAction)
+			gs := ua.GetObject().(*agonesv1.GameServer)
 
-	c, m := newFakeController()
-	m.AgonesClient.AddReactor("update", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) {
-		ua := action.(k8stesting.UpdateAction)
-		gs := ua.GetObject().(*agonesv1.GameServer)
+			assert.Equal(t, gs.Status.State, agonesv1.GameServerStateShutdown)
 
-		assert.Equal(t, gs.Status.State, agonesv1.GameServerStateShutdown)
+			updatedCount++
+			return true, nil, nil
+		})
 
-		updatedCount++
-		return true, nil, nil
+		_, cancel := agtesting.StartInformers(m)
+		defer cancel()
+
+		err := c.deleteGameServers(gsSet, []*agonesv1.GameServer{gs1, gs2, gs3})
+		assert.Nil(t, err)
+
+		assert.Equal(t, 3, updatedCount, "Updates should have occurred")
 	})
 
-	_, cancel := agtesting.StartInformers(m)
-	defer cancel()
+	t.Run("error on update step", func(t *testing.T) {
+		c, m := newFakeController()
+		m.AgonesClient.AddReactor("update", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) {
+			ua := action.(k8stesting.UpdateAction)
+			gs := ua.GetObject().(*agonesv1.GameServer)
 
-	err := c.deleteGameServers(gsSet, []*agonesv1.GameServer{gs1, gs2, gs3})
-	assert.Nil(t, err)
+			assert.Equal(t, gs.Status.State, agonesv1.GameServerStateShutdown)
 
-	assert.Equal(t, 3, updatedCount, "Updates should have occurred")
+			return true, nil, errors.New("update-err")
+		})
+
+		_, cancel := agtesting.StartInformers(m)
+		defer cancel()
+
+		err := c.deleteGameServers(gsSet, []*agonesv1.GameServer{gs1, gs2, gs3})
+		require.Error(t, err)
+		assert.Equal(t, "error updating gameserver test-1 from status Unhealthy to Shutdown status: update-err", err.Error())
+	})
 }
 
 func TestSyncMoreGameServers(t *testing.T) {
 	t.Parallel()
-
 	gsSet := defaultFixture()
 
-	c, m := newFakeController()
-	count := 0
-	expected := 10
+	t.Run("valid case", func(t *testing.T) {
+
+		c, m := newFakeController()
+		expected := 10
+		count := 0
+
+		m.AgonesClient.AddReactor("create", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) {
+			ca := action.(k8stesting.CreateAction)
+			gs := ca.GetObject().(*agonesv1.GameServer)
 
-	m.AgonesClient.AddReactor("create", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) {
-		ca := action.(k8stesting.CreateAction)
-		gs := ca.GetObject().(*agonesv1.GameServer)
+			assert.True(t, metav1.IsControlledBy(gs, gsSet))
+			count++
+
+			return true, gs, nil
+		})
 
-		assert.True(t, metav1.IsControlledBy(gs, gsSet))
-		count++
+		_, cancel := agtesting.StartInformers(m)
+		defer cancel()
 
-		return true, gs, nil
+		err := c.addMoreGameServers(gsSet, expected)
+		assert.Nil(t, err)
+		assert.Equal(t, expected, count)
+		agtesting.AssertEventContains(t, m.FakeRecorder.Events, "SuccessfulCreate")
 	})
 
-	_, cancel := agtesting.StartInformers(m)
-	defer cancel()
+	t.Run("error on create step", func(t *testing.T) {
+		gsSet := defaultFixture()
+		c, m := newFakeController()
+		expected := 10
+		m.AgonesClient.AddReactor("create", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) {
+			ca := action.(k8stesting.CreateAction)
+			gs := ca.GetObject().(*agonesv1.GameServer)
+
+			assert.True(t, metav1.IsControlledBy(gs, gsSet))
+
+			return true, gs, errors.New("create-err")
+		})
+
+		_, cancel := agtesting.StartInformers(m)
+		defer cancel()
 
-	err := c.addMoreGameServers(gsSet, expected)
-	assert.Nil(t, err)
-	assert.Equal(t, expected, count)
-	agtesting.AssertEventContains(t, m.FakeRecorder.Events, "SuccessfulCreate")
+		err := c.addMoreGameServers(gsSet, expected)
+		require.Error(t, err)
+		assert.Equal(t, "error creating gameserver for gameserverset test: create-err", err.Error())
+	})
 }
 
 func TestControllerSyncGameServerSetStatus(t *testing.T) {
@@ -655,7 +700,57 @@ func TestControllerUpdateValidationHandler(t *testing.T) {
 
 		result, err := c.updateValidationHandler(review)
 		require.NoError(t, err)
-		assert.True(t, result.Response.Allowed)
+		if !assert.True(t, result.Response.Allowed) {
+			// show the reason of the failure
+			require.NotNil(t, result.Response.Result)
+			require.NotNil(t, result.Response.Result.Details)
+			require.NotEmpty(t, result.Response.Result.Details.Causes)
+		}
+	})
+
+	t.Run("new object is nil, err excpected", func(t *testing.T) {
+		review := admv1beta1.AdmissionReview{
+			Request: &admv1beta1.AdmissionRequest{
+				Kind:      gvk,
+				Operation: admv1beta1.Create,
+				Object: runtime.RawExtension{
+					Raw: nil,
+				},
+				OldObject: runtime.RawExtension{
+					Raw: raw,
+				},
+			},
+			Response: &admv1beta1.AdmissionResponse{Allowed: true},
+		}
+
+		_, err := c.updateValidationHandler(review)
+		require.Error(t, err)
+		assert.Equal(t, "error unmarshalling new GameServerSet json: : unexpected end of JSON input", err.Error())
+	})
+
+	t.Run("old object is nil, err excpected", func(t *testing.T) {
+		newGSS := fixture.DeepCopy()
+		newGSS.Spec.Replicas = 10
+		newRaw, err := json.Marshal(newGSS)
+		require.NoError(t, err)
+
+		review := admv1beta1.AdmissionReview{
+			Request: &admv1beta1.AdmissionRequest{
+				Kind:      gvk,
+				Operation: admv1beta1.Create,
+				Object: runtime.RawExtension{
+					Raw: newRaw,
+				},
+				OldObject: runtime.RawExtension{
+					Raw: nil,
+				},
+			},
+			Response: &admv1beta1.AdmissionResponse{Allowed: true},
+		}
+
+		_, err = c.updateValidationHandler(review)
+		require.Error(t, err)
+		assert.Equal(t, "error unmarshalling old GameServerSet json: : unexpected end of JSON input", err.Error())
 	})
 
 	t.Run("invalid gameserverset update", func(t *testing.T) {
@@ -686,9 +781,116 @@ func TestControllerUpdateValidationHandler(t *testing.T) {
 
 		result, err := c.updateValidationHandler(review)
 		require.NoError(t, err)
+		require.NotNil(t, result.Response)
+		require.NotNil(t, result.Response.Result)
+		require.NotNil(t, result.Response.Result.Details)
+		assert.False(t, result.Response.Allowed)
+		assert.NotEmpty(t, result.Response.Result.Details.Causes)
+		assert.Equal(t, metav1.StatusFailure, result.Response.Result.Status)
+		assert.Equal(t, metav1.StatusReasonInvalid, result.Response.Result.Reason)
+		assert.Equal(t, "GameServerSet update is invalid", result.Response.Result.Message)
+	})
+}
+
+func TestCreationValidationHandler(t *testing.T) {
+	t.Parallel()
+
+	c, _ := newFakeController()
+	gvk := metav1.GroupVersionKind(agonesv1.SchemeGroupVersion.WithKind("GameServerSet"))
+	fixture := &agonesv1.GameServerSet{ObjectMeta: metav1.ObjectMeta{Name: "c1", Namespace: "default"},
+		Spec: agonesv1.GameServerSetSpec{
+			Replicas: 5,
+			Template: agonesv1.GameServerTemplateSpec{
+				Spec: agonesv1.GameServerSpec{Container: "test",
+					Template: corev1.PodTemplateSpec{
+						Spec: corev1.PodSpec{
+							Containers: []corev1.Container{{Name: "c1"}},
+						},
+					},
+				},
+			},
+		},
+	}
+	raw, err := json.Marshal(fixture)
+	require.NoError(t, err)
+
+	t.Run("valid gameserverset create", func(t *testing.T) {
+		newGSS := fixture.DeepCopy()
+		newGSS.Spec.Replicas = 10
+		newRaw, err := json.Marshal(newGSS)
+		require.NoError(t, err)
+
+		review := admv1beta1.AdmissionReview{
+			Request: &admv1beta1.AdmissionRequest{
+				Kind:      gvk,
+				Operation: admv1beta1.Create,
+				Object: runtime.RawExtension{
+					Raw: newRaw,
+				},
+			},
+			Response: &admv1beta1.AdmissionResponse{Allowed: true},
+		}
+
+		result, err := c.creationValidationHandler(review)
+		require.NoError(t, err)
+		if !assert.True(t, result.Response.Allowed) {
+			// show the reason of the failure
+			require.NotNil(t, result.Response.Result)
+			require.NotNil(t, result.Response.Result.Details)
+			require.NotEmpty(t, result.Response.Result.Details.Causes)
+		}
+	})
+
+	t.Run("object is nil, err excpected", func(t *testing.T) {
+		review := admv1beta1.AdmissionReview{
+			Request: &admv1beta1.AdmissionRequest{
+				Kind:      gvk,
+				Operation: admv1beta1.Create,
+				Object: runtime.RawExtension{
+					Raw: nil,
+				},
+			},
+			Response: &admv1beta1.AdmissionResponse{Allowed: true},
+		}
+
+		_, err := c.creationValidationHandler(review)
+		require.Error(t, err)
+		assert.Equal(t, "error unmarshalling new GameServerSet json: : unexpected end of JSON input", err.Error())
+	})
+
+	t.Run("invalid gameserverset create", func(t *testing.T) {
+		newGSS := fixture.DeepCopy()
+		newGSS.Spec.Template = agonesv1.GameServerTemplateSpec{
+			Spec: agonesv1.GameServerSpec{
+				Ports: []agonesv1.GameServerPort{{PortPolicy: agonesv1.Static}},
+			},
+		}
+		newRaw, err := json.Marshal(newGSS)
+		require.NoError(t, err)
+
+		assert.NotEqual(t, string(raw), string(newRaw))
+
+		review := admv1beta1.AdmissionReview{
+			Request: &admv1beta1.AdmissionRequest{
+				Kind:      gvk,
+				Operation: admv1beta1.Create,
+				Object: runtime.RawExtension{
+					Raw: newRaw,
+				},
+			},
+			Response: &admv1beta1.AdmissionResponse{Allowed: true},
+		}
+
+		result, err := c.creationValidationHandler(review)
+		require.NoError(t, err)
+		require.NotNil(t, result.Response)
+		require.NotNil(t, result.Response.Result)
+		require.NotNil(t, result.Response.Result.Details)
 		assert.False(t, result.Response.Allowed)
+		assert.NotEmpty(t, result.Response.Result.Details.Causes)
 		assert.Equal(t, metav1.StatusFailure, result.Response.Result.Status)
 		assert.Equal(t, metav1.StatusReasonInvalid, result.Response.Result.Reason)
+		assert.Equal(t, "GameServerSet create is invalid", result.Response.Result.Message)
 	})
 }
 

pkg/gameserversets/gameserversets_test.go

@@ -45,7 +45,7 @@ func TestSortGameServersByLeastFullNodes(t *testing.T) {
 
 	result := sortGameServersByLeastFullNodes(list, nc)
 
-	assert.Len(t, result, len(list))
+	require.Len(t, result, len(list))
 	assert.Equal(t, "g2", result[0].ObjectMeta.Name)
 	assert.Equal(t, "g3", result[1].ObjectMeta.Name)
 	assert.Equal(t, "g1", result[2].ObjectMeta.Name)
@@ -62,7 +62,7 @@ func TestSortGameServersByNewFirst(t *testing.T) {
 	l := len(list)
 
 	result := sortGameServersByNewFirst(list)
-	assert.Len(t, result, l)
+	require.Len(t, result, l)
 	assert.Equal(t, "g2", result[0].ObjectMeta.Name)
 	assert.Equal(t, "g1", result[1].ObjectMeta.Name)
 	assert.Equal(t, "g3", result[2].ObjectMeta.Name)