From b0825b3f36c23c1d1932990b3e1ed5fbbc766089 Mon Sep 17 00:00:00 2001 From: Pooneh Mortazavi Date: Fri, 8 Feb 2019 11:36:29 -0800 Subject: [PATCH] Update GameServerSet scheduling when Fleet scheduling is changed. --- pkg/fleets/controller.go | 3 +- pkg/fleets/controller_test.go | 44 ++++++++++++++++++- test/e2e/fleet_test.go | 75 +++++++++++++++++++++++++++++++++ test/e2e/framework/framework.go | 36 +++++++++------- 4 files changed, 139 insertions(+), 19 deletions(-) diff --git a/pkg/fleets/controller.go b/pkg/fleets/controller.go index a01aa15c4d..91e3e93fb8 100644 --- a/pkg/fleets/controller.go +++ b/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) diff --git a/pkg/fleets/controller_test.go b/pkg/fleets/controller_test.go index 2e0bf73506..f67b684111 100644 --- a/pkg/fleets/controller_test.go +++ b/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() diff --git a/test/e2e/fleet_test.go b/test/e2e/fleet_test.go index f4932a2478..ecd0175b74 100644 --- a/test/e2e/fleet_test.go +++ b/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) diff --git a/test/e2e/framework/framework.go b/test/e2e/framework/framework.go index dd7be0013f..add710dbad 100644 --- a/test/e2e/framework/framework.go +++ b/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 }) }