Check for DeletionTimestamp of fleet and gameserverset before scaling

googleforgames · Apr 1, 2022 · 5a02f8c · 5a02f8c
1 parent 3f00aa6
commit 5a02f8c
Show file tree

Hide file tree

Showing 4 changed files with 171 additions and 0 deletions.
diff --git a/pkg/fleetautoscalers/controller.go b/pkg/fleetautoscalers/controller.go
@@ -292,6 +292,11 @@ func (c *Controller) syncFleetAutoscaler(ctx context.Context, key string) error
 		return err
 	}
 
+	// Don't do anything, the fleet is marked for deletion
+	if !fleet.DeletionTimestamp.IsZero() {
+		return nil
+	}
+
 	currentReplicas := fleet.Status.Replicas
 	desiredReplicas, scalingLimited, err := computeDesiredFleetSize(fas, fleet)
 	if err != nil {

diff --git a/pkg/fleetautoscalers/controller_test.go b/pkg/fleetautoscalers/controller_test.go
@@ -283,6 +283,46 @@ func TestControllerSyncFleetAutoscaler(t *testing.T) {
 
 	assert.NoError(t, utilruntime.ParseFeatures(string(utilruntime.FeatureCustomFasSyncInterval)+"=false"))
 
+	t.Run("no scaling up because fleet is marked for deletion, buffer policy", func(t *testing.T) {
+		t.Parallel()
+		c, m := newFakeController()
+		fas, f := defaultFixtures()
+		fas.Spec.Policy.Buffer.BufferSize = intstr.FromInt(7)
+
+		f.Spec.Replicas = 5
+		f.Status.Replicas = 5
+		f.Status.AllocatedReplicas = 5
+		f.Status.ReadyReplicas = 0
+		f.DeletionTimestamp = &metav1.Time{
+			Time: time.Now(),
+		}
+
+		m.AgonesClient.AddReactor("list", "fleetautoscalers", func(action k8stesting.Action) (bool, runtime.Object, error) {
+			return true, &autoscalingv1.FleetAutoscalerList{Items: []autoscalingv1.FleetAutoscaler{*fas}}, nil
+		})
+
+		m.AgonesClient.AddReactor("update", "fleetautoscalers", func(action k8stesting.Action) (bool, runtime.Object, error) {
+			assert.FailNow(t, "fleetautoscaler should not update")
+			return false, nil, nil
+		})
+
+		m.AgonesClient.AddReactor("list", "fleets", func(action k8stesting.Action) (bool, runtime.Object, error) {
+			return true, &agonesv1.FleetList{Items: []agonesv1.Fleet{*f}}, nil
+		})
+
+		m.AgonesClient.AddReactor("update", "fleets", func(action k8stesting.Action) (bool, runtime.Object, error) {
+			assert.FailNow(t, "fleet should not update")
+			return false, nil, nil
+		})
+
+		ctx, cancel := agtesting.StartInformers(m, c.fleetSynced, c.fleetAutoscalerSynced)
+		defer cancel()
+
+		err := c.syncFleetAutoscaler(ctx, "default/fas-1")
+		assert.Nil(t, err)
+		agtesting.AssertNoEvent(t, m.FakeRecorder.Events)
+	})
+
 	t.Run("scaling up, buffer policy", func(t *testing.T) {
 		t.Parallel()
 		c, m := newFakeController()
@@ -394,6 +434,51 @@ func TestControllerSyncFleetAutoscaler(t *testing.T) {
 		agtesting.AssertNoEvent(t, m.FakeRecorder.Events)
 	})
 
+	t.Run("no scaling up because fleet is marked for deletion, webhook policy", func(t *testing.T) {
+		t.Parallel()
+		c, m := newFakeController()
+		fas, f := defaultWebhookFixtures()
+		f.Spec.Replicas = 50
+		f.Status.Replicas = f.Spec.Replicas
+		f.Status.AllocatedReplicas = 45
+		f.Status.ReadyReplicas = 0
+		f.DeletionTimestamp = &metav1.Time{
+			Time: time.Now(),
+		}
+
+		ts := testServer{}
+		server := httptest.NewServer(ts)
+		defer server.Close()
+
+		fas.Spec.Policy.Webhook.URL = &(server.URL)
+		fas.Spec.Policy.Webhook.Service = nil
+
+		m.AgonesClient.AddReactor("list", "fleetautoscalers", func(action k8stesting.Action) (bool, runtime.Object, error) {
+			return true, &autoscalingv1.FleetAutoscalerList{Items: []autoscalingv1.FleetAutoscaler{*fas}}, nil
+		})
+
+		m.AgonesClient.AddReactor("update", "fleetautoscalers", func(action k8stesting.Action) (bool, runtime.Object, error) {
+			assert.FailNow(t, "fleetautoscaler should not update")
+			return false, nil, nil
+		})
+
+		m.AgonesClient.AddReactor("list", "fleets", func(action k8stesting.Action) (bool, runtime.Object, error) {
+			return true, &agonesv1.FleetList{Items: []agonesv1.Fleet{*f}}, nil
+		})
+
+		m.AgonesClient.AddReactor("update", "fleets", func(action k8stesting.Action) (bool, runtime.Object, error) {
+			assert.FailNow(t, "fleet should not update")
+			return false, nil, nil
+		})
+
+		ctx, cancel := agtesting.StartInformers(m, c.fleetSynced, c.fleetAutoscalerSynced)
+		defer cancel()
+
+		err := c.syncFleetAutoscaler(ctx, "default/fas-1")
+		assert.Nil(t, err)
+		agtesting.AssertNoEvent(t, m.FakeRecorder.Events)
+	})
+
 	t.Run("scaling down, buffer policy", func(t *testing.T) {
 		t.Parallel()
 		c, m := newFakeController()
@@ -448,6 +533,47 @@ func TestControllerSyncFleetAutoscaler(t *testing.T) {
 		agtesting.AssertNoEvent(t, m.FakeRecorder.Events)
 	})
 
+	t.Run("no scaling down because fleet is marked for deletion, buffer policy", func(t *testing.T) {
+		t.Parallel()
+		c, m := newFakeController()
+		fas, f := defaultFixtures()
+		fas.Spec.Policy.Buffer.BufferSize = intstr.FromInt(8)
+
+		f.Spec.Replicas = 20
+		f.Status.Replicas = 20
+		f.Status.AllocatedReplicas = 5
+		f.Status.ReadyReplicas = 15
+		f.DeletionTimestamp = &metav1.Time{
+			Time: time.Now(),
+		}
+
+		m.AgonesClient.AddReactor("list", "fleetautoscalers", func(action k8stesting.Action) (bool, runtime.Object, error) {
+			return true, &autoscalingv1.FleetAutoscalerList{Items: []autoscalingv1.FleetAutoscaler{*fas}}, nil
+		})
+
+		m.AgonesClient.AddReactor("update", "fleetautoscalers", func(action k8stesting.Action) (bool, runtime.Object, error) {
+			assert.FailNow(t, "fleetautoscaler should not update")
+			return true, nil, nil
+		})
+
+		m.AgonesClient.AddReactor("list", "fleets", func(action k8stesting.Action) (bool, runtime.Object, error) {
+			return true, &agonesv1.FleetList{Items: []agonesv1.Fleet{*f}}, nil
+		})
+
+		m.AgonesClient.AddReactor("update", "fleets", func(action k8stesting.Action) (bool, runtime.Object, error) {
+			assert.FailNow(t, "fleet should not update")
+
+			return false, nil, nil
+		})
+
+		ctx, cancel := agtesting.StartInformers(m, c.fleetSynced, c.fleetAutoscalerSynced)
+		defer cancel()
+
+		err := c.syncFleetAutoscaler(ctx, "default/fas-1")
+		assert.Nil(t, err)
+		agtesting.AssertNoEvent(t, m.FakeRecorder.Events)
+	})
+
 	t.Run("no scaling no update", func(t *testing.T) {
 		t.Parallel()
 		c, m := newFakeController()

diff --git a/pkg/gameserversets/controller.go b/pkg/gameserversets/controller.go
@@ -307,6 +307,12 @@ func (c *Controller) syncGameServerSet(ctx context.Context, key string) error {
 
 	numServersToAdd, toDelete, isPartial := computeReconciliationAction(gsSet.Spec.Scheduling, list, c.counter.Counts(),
 		int(gsSet.Spec.Replicas), maxGameServerCreationsPerBatch, maxGameServerDeletionsPerBatch, maxPodPendingCount)
+
+	// GameserverSet is marked for deletion then don't add gameservers.
+	if !gsSet.DeletionTimestamp.IsZero() {
+		numServersToAdd = 0
+	}
+
 	status := computeStatus(list)
 	fields := logrus.Fields{}
 

diff --git a/pkg/gameserversets/controller_test.go b/pkg/gameserversets/controller_test.go
@@ -426,6 +426,40 @@ func TestControllerWatchGameServers(t *testing.T) {
 func TestSyncGameServerSet(t *testing.T) {
 	t.Parallel()
 
+	t.Run("gameservers are not recreated when set is marked for deletion", func(t *testing.T) {
+		gsSet := defaultFixture()
+		gsSet.DeletionTimestamp = &metav1.Time{
+			Time: time.Now(),
+		}
+		list := createGameServers(gsSet, 5)
+
+		// mark some as shutdown
+		list[0].Status.State = agonesv1.GameServerStateShutdown
+		list[1].Status.State = agonesv1.GameServerStateShutdown
+
+		c, m := newFakeController()
+		m.AgonesClient.AddReactor("list", "gameserversets", func(action k8stesting.Action) (bool, runtime.Object, error) {
+			return true, &agonesv1.GameServerSetList{Items: []agonesv1.GameServerSet{*gsSet}}, nil
+		})
+		m.AgonesClient.AddReactor("list", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) {
+			return true, &agonesv1.GameServerList{Items: list}, nil
+		})
+		m.AgonesClient.AddReactor("update", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) {
+			assert.FailNow(t, "gameserver should not update")
+			return false, nil, nil
+		})
+		m.AgonesClient.AddReactor("create", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) {
+			assert.FailNow(t, "new gameservers should not be created")
+
+			return false, nil, nil
+		})
+
+		ctx, cancel := agtesting.StartInformers(m, c.gameServerSetSynced, c.gameServerSynced)
+		defer cancel()
+
+		c.syncGameServerSet(ctx, gsSet.ObjectMeta.Namespace+"/"+gsSet.ObjectMeta.Name) // nolint: errcheck
+	})
+
 	t.Run("adding and deleting unhealthy gameservers", func(t *testing.T) {
 		gsSet := defaultFixture()
 		list := createGameServers(gsSet, 5)