Update GameServerSet scheduling when Fleet scheduling is changed.

pkg/fleets/controller.go

@@ -263,9 +263,10 @@ func (c *Controller) upsertGameServerSet(fleet *stablev1alpha1.Fleet, active *st
 		return nil
 	}
 
-	if replicas != active.Spec.Replicas {
+	if replicas != active.Spec.Replicas || active.Spec.Scheduling != fleet.Spec.Scheduling {
 		gsSetCopy := active.DeepCopy()
 		gsSetCopy.Spec.Replicas = replicas
+		gsSetCopy.Spec.Scheduling = fleet.Spec.Scheduling
 		gsSetCopy, err := c.gameServerSetGetter.GameServerSets(fleet.ObjectMeta.Namespace).Update(gsSetCopy)
 		if err != nil {
 			return errors.Wrapf(err, "error updating replicas for gameserverset for fleet %s", fleet.ObjectMeta.Name)

pkg/fleets/controller_test.go

@@ -143,6 +143,44 @@ func TestControllerSyncFleet(t *testing.T) {
 		agtesting.AssertEventContains(t, m.FakeRecorder.Events, "ScalingGameServerSet")
 	})
 
+	t.Run("gameserverset with different scheduling", func(t *testing.T) {
+		f := defaultFixture()
+		f.Spec.Strategy.Type = appsv1.RecreateDeploymentStrategyType
+		c, m := newFakeController()
+		gsSet := f.GameServerSet()
+		gsSet.ObjectMeta.Name = "gsSet1"
+		gsSet.ObjectMeta.UID = "1234"
+		gsSet.Spec.Replicas = f.Spec.Replicas
+		gsSet.Spec.Scheduling = v1alpha1.Distributed
+		updated := false
+
+		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("update", "gameserversets", func(action k8stesting.Action) (bool, runtime.Object, error) {
+			updated = true
+
+			ua := action.(k8stesting.UpdateAction)
+			gsSet := ua.GetObject().(*v1alpha1.GameServerSet)
+			assert.Equal(t, f.Spec.Replicas, gsSet.Spec.Replicas)
+			assert.Equal(t, f.Spec.Scheduling, gsSet.Spec.Scheduling)
+			return true, gsSet, nil
+		})
+
+		_, cancel := agtesting.StartInformers(m, c.fleetSynced, c.gameServerSetSynced)
+		defer cancel()
+
+		err := c.syncFleet("default/fleet-1")
+		assert.Nil(t, err)
+		assert.True(t, updated, "gameserverset should have been updated")
+		agtesting.AssertEventContains(t, m.FakeRecorder.Events, "ScalingGameServerSet")
+	})
+
 	t.Run("gameserverset with different image details", func(t *testing.T) {
 		f := defaultFixture()
 		f.Spec.Strategy.Type = appsv1.RollingUpdateDeploymentStrategyType
@@ -153,6 +191,7 @@ func TestControllerSyncFleet(t *testing.T) {
 		gsSet.ObjectMeta.UID = "4321"
 		gsSet.Spec.Template.Spec.Ports = []v1alpha1.GameServerPort{{HostPort: 7777}}
 		gsSet.Spec.Replicas = f.Spec.Replicas
+		gsSet.Spec.Scheduling = f.Spec.Scheduling
 		gsSet.Status.Replicas = 5
 		updated := false
 		created := false
@@ -747,8 +786,9 @@ func defaultFixture() *v1alpha1.Fleet {
 			UID:       "1234",
 		},
 		Spec: v1alpha1.FleetSpec{
-			Replicas: 5,
-			Template: v1alpha1.GameServerTemplateSpec{},
+			Replicas:   5,
+			Scheduling: v1alpha1.Packed,
+			Template:   v1alpha1.GameServerTemplateSpec{},
 		},
 	}
 	f.ApplyDefaults()

test/e2e/fleet_test.go

@@ -731,6 +731,81 @@ func TestScaleUpAndDownInParallelStressTest(t *testing.T) {
 	wg.Wait()
 }
 
+// Creates a fleet and one GameServer with Packed scheduling.
+// Scale to two GameServers with Distributed scheduling.
+// The old GameServer has Scheduling set to 5 and the new one has it set to Distributed.
+func TestUpdateFleetScheduling(t *testing.T) {
+	t.Parallel()
+	t.Run("Updating Spec.Scheduling on fleet should be updated in GameServer",
+		func(t *testing.T) {
+			alpha1 := framework.AgonesClient.StableV1alpha1()
+
+			flt := defaultFleet()
+			flt.Spec.Replicas = 1
+			flt.Spec.Scheduling = v1alpha1.Packed
+			flt, err := alpha1.Fleets(defaultNs).Create(flt)
+
+			if assert.Nil(t, err) {
+				defer alpha1.Fleets(defaultNs).Delete(flt.ObjectMeta.Name, nil) // nolint:errcheck
+			}
+
+			assert.Equal(t, int32(1), flt.Spec.Replicas)
+			assert.Equal(t, v1alpha1.Packed, flt.Spec.Scheduling)
+
+			framework.WaitForFleetCondition(t, flt, e2e.FleetReadyCount(flt.Spec.Replicas))
+
+			const targetScale = 2
+			flt = schedulingFleetPatch(t, flt, v1alpha1.Distributed, targetScale)
+			framework.WaitForFleetCondition(t, flt, e2e.FleetReadyCount(targetScale))
+
+			assert.Equal(t, int32(targetScale), flt.Spec.Replicas)
+			assert.Equal(t, v1alpha1.Distributed, flt.Spec.Scheduling)
+
+			err = framework.WaitForFleetGameServerListCondition(flt,
+				func(gsList []v1alpha1.GameServer) bool {
+					return countFleetScheduling(gsList, v1alpha1.Distributed) == 1 &&
+						countFleetScheduling(gsList, v1alpha1.Packed) == 1
+				})
+			assert.Nil(t, err)
+		})
+}
+
+// Counts the number of gameservers with the specified scheduling strategy in a fleet
+func countFleetScheduling(gsList []v1alpha1.GameServer, scheduling v1alpha1.SchedulingStrategy) int {
+	count := 0
+	for _, gs := range gsList {
+		if gs.Spec.Scheduling == scheduling {
+			count++
+		}
+	}
+	return count
+}
+
+// Patches fleet with scheduling and scale values
+func schedulingFleetPatch(t *testing.T,
+	f *v1alpha1.Fleet,
+	scheduling v1alpha1.SchedulingStrategy,
+	scale int32) *v1alpha1.Fleet {
+
+	patch := fmt.Sprintf(`[{ "op": "replace", "path": "/spec/scheduling", "value": "%s" },
+	                       { "op": "replace", "path": "/spec/replicas", "value": %d }]`,
+		scheduling, scale)
+
+	logrus.WithField("fleet", f.ObjectMeta.Name).
+		WithField("scheduling", scheduling).
+		WithField("scale", scale).
+		WithField("patch", patch).
+		Info("updating scheduling")
+
+	fltRes, err := framework.AgonesClient.
+		StableV1alpha1().
+		Fleets(defaultNs).
+		Patch(f.ObjectMeta.Name, types.JSONPatchType, []byte(patch))
+
+	assert.Nil(t, err)
+	return fltRes
+}
+
 func scaleAndWait(t *testing.T, flt *v1alpha1.Fleet, fleetSize int32) time.Duration {
 	t0 := time.Now()
 	scaleFleetSubresource(t, flt, fleetSize)

test/e2e/framework/framework.go

@@ -172,30 +172,34 @@ func FleetReadyCount(amount int32) func(fleet *v1alpha1.Fleet) bool {
 	}
 }
 
-// WaitForFleetGameServersCondition wait for all GameServers for a given
-// fleet to match the spec.replicas and match a a condition
-func (f *Framework) WaitForFleetGameServersCondition(flt *v1alpha1.Fleet, cond func(server v1alpha1.GameServer) bool) error {
+// WaitForFleetGameServersCondition waits for all GameServers for a given fleet to match
+// a condition specified by a callback.
+func (f *Framework) WaitForFleetGameServersCondition(flt *v1alpha1.Fleet,
+	cond func(server v1alpha1.GameServer) bool) error {
+	return f.WaitForFleetGameServerListCondition(flt,
+		func(servers []v1alpha1.GameServer) bool {
+			for _, gs := range servers {
+				if !cond(gs) {
+					return false
+				}
+			}
+			return true
+		})
+}
+
+// WaitForFleetGameServerListCondition waits for the list of GameServers to match a condition
+// specified by a callback and the size of GameServers to match fleet's Spec.Replicas.
+func (f *Framework) WaitForFleetGameServerListCondition(flt *v1alpha1.Fleet,
+	cond func(servers []v1alpha1.GameServer) bool) error {
 	return wait.Poll(2*time.Second, 5*time.Minute, func() (done bool, err error) {
 		gsList, err := f.ListGameServersFromFleet(flt)
 		if err != nil {
 			return false, err
 		}
-
 		if int32(len(gsList)) != flt.Spec.Replicas {
 			return false, nil
 		}
-
-		if err != nil {
-			return false, err
-		}
-
-		for _, gs := range gsList {
-			if !cond(gs) {
-				return false, nil
-			}
-		}
-
-		return true, nil
+		return cond(gsList), nil
 	})
 }