Flake: TestControllerGameServerCount (#2805)

Made it deterministic in the test, and got rod of the potential race conditions. Also fix it such that the util function for generating GameServer names always produce a unique name. Closes #2804 Co-authored-by: Robert Bailey <[email protected]>
googleforgames · Nov 11, 2022 · d5cf2b0 · d5cf2b0
1 parent 5fff45b
commit d5cf2b0
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 9 deletions.
diff --git a/pkg/metrics/controller_test.go b/pkg/metrics/controller_test.go
@@ -25,6 +25,7 @@ import (
 	agtesting "agones.dev/agones/pkg/testing"
 	"agones.dev/agones/pkg/util/runtime"
 	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"go.opencensus.io/metric/metricdata"
@@ -100,14 +101,21 @@ func TestControllerGameServerCount(t *testing.T) {
 	c := newFakeController()
 	defer c.close()
 
+	c.run(t)
+	require.True(t, c.sync())
+
 	gs1 := gameServerWithFleetAndState("test-fleet", agonesv1.GameServerStateCreating)
 	c.gsWatch.Add(gs1)
+	require.Eventually(t, func() bool {
+		gs, err := c.gameServerLister.GameServers(gs1.ObjectMeta.Namespace).Get(gs1.ObjectMeta.Name)
+		assert.NoError(t, err)
+		return gs.Status.State == agonesv1.GameServerStateCreating
+	}, 5*time.Second, time.Second)
+	c.collect()
+
 	gs1 = gs1.DeepCopy()
 	gs1.Status.State = agonesv1.GameServerStateReady
 	c.gsWatch.Modify(gs1)
-
-	c.run(t)
-	require.True(t, c.sync())
 	require.Eventually(t, func() bool {
 		gs, err := c.gameServerLister.GameServers(gs1.ObjectMeta.Namespace).Get(gs1.ObjectMeta.Name)
 		assert.NoError(t, err)
@@ -121,7 +129,6 @@ func TestControllerGameServerCount(t *testing.T) {
 	c.gsWatch.Add(gameServerWithFleetAndState("", agonesv1.GameServerStatePortAllocation))
 	c.gsWatch.Add(gameServerWithFleetAndState("", agonesv1.GameServerStatePortAllocation))
 
-	c.run(t)
 	require.True(t, c.sync())
 	// Port allocation is last, so wait for that come to the state we expect
 	require.Eventually(t, func() bool {
@@ -131,10 +138,11 @@ func TestControllerGameServerCount(t *testing.T) {
 
 		for _, m := range ex.metrics {
 			if m.Descriptor.Name == gameServersCountName {
-				for _, d := range m.TimeSeries {
-					if d.LabelValues[0].Value == "none" && d.LabelValues[1].Value == defaultNs && d.LabelValues[2].Value == "PortAllocation" {
-						return d.Points[0].Value == int64(2)
-					}
+				if len(m.TimeSeries) == 4 {
+					return true
+				} else {
+					logrus.WithField("m", m).Info("Metrics")
+					return false
 				}
 			}
 		}
@@ -144,6 +152,7 @@ func TestControllerGameServerCount(t *testing.T) {
 
 	reader.ReadAndExport(exporter)
 	assertMetricData(t, exporter, gameServersCountName, []expectedMetricData{
+		{labels: []string{"test-fleet", defaultNs, "Creating"}, val: int64(0)},
 		{labels: []string{"test-fleet", defaultNs, "Ready"}, val: int64(0)},
 		{labels: []string{"test-fleet", defaultNs, "Shutdown"}, val: int64(1)},
 		{labels: []string{"none", defaultNs, "PortAllocation"}, val: int64(2)},

diff --git a/pkg/metrics/util_test.go b/pkg/metrics/util_test.go
@@ -17,6 +17,7 @@ package metrics
 import (
 	"context"
 	"reflect"
+	"strconv"
 	"testing"
 
 	agonesv1 "agones.dev/agones/pkg/apis/agones/v1"
@@ -34,6 +35,11 @@ import (
 	"k8s.io/client-go/tools/cache"
 )
 
+var (
+	// counter for unique GameServer names
+	counter = 0
+)
+
 // newFakeController returns a controller, backed by the fake Clientset
 func newFakeController() *fakeController {
 	m := agtesting.NewMocks()
@@ -124,9 +130,11 @@ func gameServerWithFleetAndState(fleetName string, state agonesv1.GameServerStat
 	if fleetName != "" {
 		lbs[agonesv1.FleetNameLabel] = fleetName
 	}
+	// ensure the name is unique each time.
+	counter++
 	gs := &agonesv1.GameServer{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      rand.String(10),
+			Name:      rand.String(10) + strconv.Itoa(counter),
 			Namespace: "default",
 			UID:       uuid.NewUUID(),
 			Labels:    lbs,