Prioritise Allocation from Nodes with Allocated/Ready GameServers

One of the first parts for Node autoscaling (googleforgames#368) - make sure we essentially
bin pack our allocated game servers.

This change makes allocation first prioritise allocation from `Nodes` that
already have the most `Allocated` `GameServers`, and then in the case of a tie,
to the `Nodes` that have the most `Ready` `GameServers`.

This sets us up for the next part, such that when we scale down a Fleet,
it removes `GameServers` from `Nodes` that have the least `GameServers` on
them.

pkg/fleetallocation/controller.go

@@ -272,12 +272,7 @@ func (c *Controller) allocate(f *stablev1alpha1.Fleet, fam *stablev1alpha1.Fleet
 		return allocation, err
 	}
 
-	for _, gs := range gsList {
-		if gs.Status.State == stablev1alpha1.Ready && gs.ObjectMeta.DeletionTimestamp.IsZero() {
-			allocation = gs
-			break
-		}
-	}
+	allocation = findReadyGameServerForAllocation(gsList)
 
 	if allocation == nil {
 		return allocation, ErrNoGameServerReady

pkg/fleetallocation/controller_test.go

@@ -141,6 +141,8 @@ func TestControllerMutationValidationHandler(t *testing.T) {
 }
 
 func TestControllerAllocate(t *testing.T) {
+	t.Parallel()
+
 	f, gsSet, gsList := defaultFixtures(4)
 	c, m := newFakeController()
 	n := metav1.Now()
@@ -210,6 +212,62 @@ func TestControllerAllocate(t *testing.T) {
 	assert.False(t, updated)
 }
 
+func TestControllerAllocatePriority(t *testing.T) {
+	t.Parallel()
+
+	f, gsSet, gsList := defaultFixtures(4)
+	c, m := newFakeController()
+
+	n1 := "node1"
+	n2 := "node2"
+	gsList[0].Status.NodeName = n1
+	gsList[1].Status.NodeName = n2
+	gsList[2].Status.NodeName = n1
+	gsList[3].Status.NodeName = n1
+
+	m.AgonesClient.AddReactor("list", "fleets", func(action k8stesting.Action) (bool, runtime.Object, error) {
+		return true, &v1alpha1.FleetList{Items: []v1alpha1.Fleet{*f}}, nil
+	})
+	m.AgonesClient.AddReactor("list", "gameserversets", func(action k8stesting.Action) (bool, runtime.Object, error) {
+		return true, &v1alpha1.GameServerSetList{Items: []v1alpha1.GameServerSet{*gsSet}}, nil
+	})
+	m.AgonesClient.AddReactor("list", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) {
+		return true, &v1alpha1.GameServerList{Items: gsList}, nil
+	})
+
+	gsWatch := watch.NewFake()
+	m.AgonesClient.AddWatchReactor("gameservers", k8stesting.DefaultWatchReactor(gsWatch, nil))
+	m.AgonesClient.AddReactor("update", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) {
+		ua := action.(k8stesting.UpdateAction)
+		gs := ua.GetObject().(*v1alpha1.GameServer)
+		gsWatch.Modify(gs)
+		return true, gs, nil
+	})
+
+	_, cancel := agtesting.StartInformers(m)
+	defer cancel()
+
+	// priority should be node1, then node2
+	gs, err := c.allocate(f, nil)
+	assert.Nil(t, err)
+	assert.Equal(t, n1, gs.Status.NodeName)
+
+	gs, err = c.allocate(f, nil)
+	assert.Nil(t, err)
+	assert.Equal(t, n1, gs.Status.NodeName)
+	gs, err = c.allocate(f, nil)
+	assert.Nil(t, err)
+	assert.Equal(t, n1, gs.Status.NodeName)
+	gs, err = c.allocate(f, nil)
+	assert.Nil(t, err)
+	assert.Equal(t, n2, gs.Status.NodeName)
+
+	// should have none left
+	_, err = c.allocate(f, nil)
+	assert.NotNil(t, err)
+
+}
+
 func TestControllerAllocateMutex(t *testing.T) {
 	t.Parallel()
 

pkg/fleetallocation/find.go

@@ -0,0 +1,76 @@
+// Copyright 2018 Google Inc. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package fleetallocation
+
+import (
+	"agones.dev/agones/pkg/apis/stable/v1alpha1"
+)
+
+// nodeCount is just a convenience data structure for
+// keeping relevant GameServer counts about Nodes
+type nodeCount struct {
+	ready     int64
+	allocated int64
+}
+
+// findReadyGameServerForAllocation is a O(n) implementation to find a GameServer with priority
+// on Nodes with GameServers that are allocated, and then Nodes with the most
+// Ready GameServers
+func findReadyGameServerForAllocation(gsList []*v1alpha1.GameServer) *v1alpha1.GameServer {
+	counts := map[string]*nodeCount{}
+	// track potential gameservers, one for each node
+	gsOptions := map[string]*v1alpha1.GameServer{}
+
+	for _, gs := range gsList {
+		if gs.DeletionTimestamp.IsZero() &&
+			(gs.Status.State == v1alpha1.Allocated || gs.Status.State == v1alpha1.Ready) {
+			_, ok := counts[gs.Status.NodeName]
+			if !ok {
+				counts[gs.Status.NodeName] = &nodeCount{}
+			}
+
+			if gs.Status.State == v1alpha1.Allocated {
+				counts[gs.Status.NodeName].allocated++
+			} else if gs.Status.State == v1alpha1.Ready {
+				counts[gs.Status.NodeName].ready++
+				gsOptions[gs.Status.NodeName] = gs
+			}
+		}
+	}
+
+	var bestCount *nodeCount
+	var bestGS *v1alpha1.GameServer
+	for nodeName, count := range counts {
+		update := false
+		// if there is no best GameServer, then this node & GameServer is the always the best
+		if bestGS == nil {
+			update = true
+		} else if count.allocated == bestCount.allocated && count.ready > bestCount.ready {
+			update = true
+		} else if count.allocated > bestCount.allocated {
+			update = true
+		}
+
+		if update {
+			// we may not have any GameServers on this node, so check first!
+			if gs, ok := gsOptions[nodeName]; ok {
+				bestCount = count
+				bestGS = gs
+			}
+		}
+	}
+
+	return bestGS
+}

pkg/fleetallocation/find_test.go

@@ -0,0 +1,48 @@
+// Copyright 2018 Google Inc. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package fleetallocation
+
+import (
+	"testing"
+
+	"agones.dev/agones/pkg/apis/stable/v1alpha1"
+	"github.com/stretchr/testify/assert"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+func TestFindReadyGameServer(t *testing.T) {
+	n := metav1.Now()
+
+	gsList := []*v1alpha1.GameServer{
+		{ObjectMeta: metav1.ObjectMeta{Name: "gs6", DeletionTimestamp: &n}, Status: v1alpha1.GameServerStatus{NodeName: "node1", State: v1alpha1.Ready}},
+		{ObjectMeta: metav1.ObjectMeta{Name: "gs1"}, Status: v1alpha1.GameServerStatus{NodeName: "node1", State: v1alpha1.Ready}},
+		{ObjectMeta: metav1.ObjectMeta{Name: "gs2"}, Status: v1alpha1.GameServerStatus{NodeName: "node2", State: v1alpha1.Ready}},
+		{ObjectMeta: metav1.ObjectMeta{Name: "gs3"}, Status: v1alpha1.GameServerStatus{NodeName: "node1", State: v1alpha1.Allocated}},
+		{ObjectMeta: metav1.ObjectMeta{Name: "gs4"}, Status: v1alpha1.GameServerStatus{NodeName: "node1", State: v1alpha1.Allocated}},
+		{ObjectMeta: metav1.ObjectMeta{Name: "gs5"}, Status: v1alpha1.GameServerStatus{NodeName: "node1", State: v1alpha1.Error}},
+	}
+
+	gs := findReadyGameServerForAllocation(gsList)
+	assert.Equal(t, "node1", gs.Status.NodeName)
+	assert.Equal(t, v1alpha1.Ready, gs.Status.State)
+	// mock that the first game server is allocated
+	gsList[1].Status.State = v1alpha1.Allocated
+	gs = findReadyGameServerForAllocation(gsList)
+	assert.Equal(t, "node2", gs.Status.NodeName)
+	assert.Equal(t, v1alpha1.Ready, gs.Status.State)
+	gsList[2].Status.State = v1alpha1.Allocated
+	gs = findReadyGameServerForAllocation(gsList)
+	assert.Nil(t, gs)
+}