From c87fd20f65c0a919ca0644b4c2639ca5fad8cd65 Mon Sep 17 00:00:00 2001 From: Mark Mandel Date: Wed, 16 May 2018 21:52:32 -0700 Subject: [PATCH] Rolling updates for Fleets This implements a configurable rolling update strategy for Fleets that also ensures that Allocated GameServers are not interuppted. Also includes updates to documentation. Closes #70 --- docs/fleet_spec.md | 14 +- examples/fleet.yaml | 13 +- pkg/apis/stable/v1alpha1/fleet.go | 60 +++++- pkg/apis/stable/v1alpha1/fleet_test.go | 52 +++-- pkg/fleets/controller.go | 151 +++++++++++--- pkg/fleets/controller_test.go | 264 ++++++++++++++++++++++--- 6 files changed, 465 insertions(+), 89 deletions(-) diff --git a/docs/fleet_spec.md b/docs/fleet_spec.md index cda8510d41..0d54703f14 100644 --- a/docs/fleet_spec.md +++ b/docs/fleet_spec.md @@ -18,7 +18,10 @@ metadata: spec: replicas: 2 strategy: - type: Recreate + type: RollingUpdate + rollingUpdate: + maxSurge: 25% + maxUnavailable: 25% template: metadata: labels: @@ -51,9 +54,12 @@ The `spec` field is the actual `Fleet` specification and it is composed as follo - `replicas` is the number of `GameServers` to keep Ready or Allocated in this Fleet - `strategy` is the `GameServer` replacement strategy for when the `GameServer` template is edited. - `type` "Recreate" is the only option. A "RollingUpdate" option will be implemented soon. - - `Recreate` terminates all non-allocated `GameServers`, and starts up a new set with - the new `GameServer` configuration to replace them. + - `type` is replacement strategy for when the GameServer template is changed. Default option is "RollingUpdate", but "Recreate" is also available. + - `RollingUpdate` will increment by `maxSurge` value on each iteration, while decrementing by `maxUnavailable` on each iteration, until all GameServers have been switched from one version to another. + - `Recreate` terminates all non-allocated `GameServers`, and starts up a new set with the new `GameServer` configuration to replace them. + - `rollingUpdate` is only relevant when `type: RollingUpdate` + - `maxSurge` is the amount to increment the new GameServers by. Defaults to 25% + - `maxUnavailable` is the amount to decrements GameServers by. Defaults to 25% - `template` a full `GameServer` configuration template. See the [GameServer](./gameserver_spec.md) reference for all available fields. diff --git a/examples/fleet.yaml b/examples/fleet.yaml index 49f624756b..0aab67aac1 100644 --- a/examples/fleet.yaml +++ b/examples/fleet.yaml @@ -30,10 +30,17 @@ spec: # a GameServer template - see: # https://github.com/GoogleCloudPlatform/agones/blob/master/docs/gameserver_spec.md for all the options strategy: - # The replacement strategy for when the GameServer template is changed. Current option is "Recreate", - # "RollingUpdate" will come at a later date. + # The replacement strategy for when the GameServer template is changed. Default option is "RollingUpdate", + # "RollingUpdate" will increment by maxSurge value on each iteration, while decrementing by maxUnavailable on each + # iteration, until all GameServers have been switched from one version to another. # "Recreate" terminates all non-allocated GameServers, and starts up a new set with the new details to replace them. - type: Recreate + type: RollingUpdate + # Only relevant when `type: RollingUpdate` + rollingUpdate: + # the amount to increment the new GameServers by. Defaults to 25% + maxSurge: 25% + # the amount to decrements GameServers by. Defaults to 25% + maxUnavailable: 25% template: # GameServer metadata metadata: diff --git a/pkg/apis/stable/v1alpha1/fleet.go b/pkg/apis/stable/v1alpha1/fleet.go index 0022872b6d..a74c05d520 100644 --- a/pkg/apis/stable/v1alpha1/fleet.go +++ b/pkg/apis/stable/v1alpha1/fleet.go @@ -18,6 +18,7 @@ import ( "agones.dev/agones/pkg/apis/stable" appsv1 "k8s.io/api/apps/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" ) const ( @@ -74,7 +75,6 @@ func (f *Fleet) GameServerSet() *GameServerSet { gsSet := &GameServerSet{ ObjectMeta: *f.Spec.Template.ObjectMeta.DeepCopy(), Spec: GameServerSetSpec{ - Replicas: f.Spec.Replicas, Template: f.Spec.Template, }, } @@ -102,18 +102,60 @@ func (f *Fleet) GameServerSet() *GameServerSet { // ApplyDefaults applies default values to the Fleet func (f *Fleet) ApplyDefaults() { if f.Spec.Strategy.Type == "" { - f.Spec.Strategy.Type = appsv1.RecreateDeploymentStrategyType + f.Spec.Strategy.Type = appsv1.RollingUpdateDeploymentStrategyType } + + if f.Spec.Strategy.Type == appsv1.RollingUpdateDeploymentStrategyType { + if f.Spec.Strategy.RollingUpdate == nil { + f.Spec.Strategy.RollingUpdate = &appsv1.RollingUpdateDeployment{} + } + + def := intstr.FromString("25%") + if f.Spec.Strategy.RollingUpdate.MaxSurge == nil { + f.Spec.Strategy.RollingUpdate.MaxSurge = &def + } + if f.Spec.Strategy.RollingUpdate.MaxUnavailable == nil { + f.Spec.Strategy.RollingUpdate.MaxUnavailable = &def + } + } +} + +// UpperBoundReplicas returns whichever is smaller, +// the value i, or the f.Spec.Replicas. +func (f *Fleet) UpperBoundReplicas(i int32) int32 { + if i > f.Spec.Replicas { + return f.Spec.Replicas + } + return i +} + +// LowerBoundReplicas returns 0 (the minimum value for +// replicas) if i is < 0 +func (f *Fleet) LowerBoundReplicas(i int32) int32 { + if i < 0 { + return 0 + } + return i +} + +// SumStatusAllocatedReplicas returns the total number of +// Status.AllocatedReplicas in the list of GameServerSets +func SumStatusAllocatedReplicas(list []*GameServerSet) int32 { + total := int32(0) + for _, gsSet := range list { + total += gsSet.Status.AllocatedReplicas + } + + return total } -// ReplicasMinusSumAllocated returns the number of Replicas that are -// currently configured against this fleet, subtracting the number -// of allocated GameServers counted on this list of GameServerSets -func (f *Fleet) ReplicasMinusSumAllocated(list []*GameServerSet) int32 { - result := f.Spec.Replicas +// SumStatusReplicas returns the total number of +// Status.Replicas in the list of GameServerSets +func SumStatusReplicas(list []*GameServerSet) int32 { + total := int32(0) for _, gsSet := range list { - result -= gsSet.Status.AllocatedReplicas + total += gsSet.Status.Replicas } - return result + return total } diff --git a/pkg/apis/stable/v1alpha1/fleet_test.go b/pkg/apis/stable/v1alpha1/fleet_test.go index 908d23aff7..18b019e514 100644 --- a/pkg/apis/stable/v1alpha1/fleet_test.go +++ b/pkg/apis/stable/v1alpha1/fleet_test.go @@ -50,32 +50,56 @@ func TestFleetGameServerSetGameServer(t *testing.T) { assert.Equal(t, f.ObjectMeta.Namespace, gsSet.ObjectMeta.Namespace) assert.Equal(t, f.ObjectMeta.Name+"-", gsSet.ObjectMeta.GenerateName) assert.Equal(t, f.ObjectMeta.Name, gsSet.ObjectMeta.Labels[FleetGameServerSetLabel]) - assert.Equal(t, f.Spec.Replicas, gsSet.Spec.Replicas) + assert.Equal(t, int32(0), gsSet.Spec.Replicas) assert.Equal(t, f.Spec.Template, gsSet.Spec.Template) assert.True(t, v1.IsControlledBy(gsSet, &f)) } -func TestFleetReplicasMinusSumAllocated(t *testing.T) { - f := Fleet{ - Spec: FleetSpec{Replicas: 10}, - } +func TestFleetApplyDefaults(t *testing.T) { + f := &Fleet{} + + // gate + assert.EqualValues(t, "", f.Spec.Strategy.Type) + + f.ApplyDefaults() + assert.Equal(t, appsv1.RollingUpdateDeploymentStrategyType, f.Spec.Strategy.Type) + assert.Equal(t, "25%", f.Spec.Strategy.RollingUpdate.MaxUnavailable.String()) + assert.Equal(t, "25%", f.Spec.Strategy.RollingUpdate.MaxSurge.String()) +} + +func TestFleetUpperBoundReplicas(t *testing.T) { + f := &Fleet{Spec: FleetSpec{Replicas: 10}} - assert.Equal(t, int32(10), f.ReplicasMinusSumAllocated(nil)) + assert.Equal(t, int32(10), f.UpperBoundReplicas(12)) + assert.Equal(t, int32(10), f.UpperBoundReplicas(10)) + assert.Equal(t, int32(5), f.UpperBoundReplicas(5)) +} + +func TestFleetLowerBoundReplicas(t *testing.T) { + f := &Fleet{Spec: FleetSpec{Replicas: 10}} + + assert.Equal(t, int32(5), f.LowerBoundReplicas(5)) + assert.Equal(t, int32(0), f.LowerBoundReplicas(0)) + assert.Equal(t, int32(0), f.LowerBoundReplicas(-5)) +} + +func TestSumStatusAllocatedReplicas(t *testing.T) { + f := Fleet{} gsSet1 := f.GameServerSet() gsSet1.Status.AllocatedReplicas = 2 gsSet2 := f.GameServerSet() gsSet2.Status.AllocatedReplicas = 3 - assert.Equal(t, int32(5), f.ReplicasMinusSumAllocated([]*GameServerSet{gsSet1, gsSet2})) + assert.Equal(t, int32(5), SumStatusAllocatedReplicas([]*GameServerSet{gsSet1, gsSet2})) } -func TestFleetApplyDefaults(t *testing.T) { - f := &Fleet{} - - // gate - assert.EqualValues(t, "", f.Spec.Strategy.Type) +func TestSumStatusReplicas(t *testing.T) { + fixture := []*GameServerSet{ + {Status: GameServerSetStatus{Replicas: 10}}, + {Status: GameServerSetStatus{Replicas: 15}}, + {Status: GameServerSetStatus{Replicas: 5}}, + } - f.ApplyDefaults() - assert.Equal(t, appsv1.RecreateDeploymentStrategyType, f.Spec.Strategy.Type) + assert.Equal(t, int32(30), SumStatusReplicas(fixture)) } diff --git a/pkg/fleets/controller.go b/pkg/fleets/controller.go index c2133a8c74..368d15fd12 100644 --- a/pkg/fleets/controller.go +++ b/pkg/fleets/controller.go @@ -39,6 +39,7 @@ import ( "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1beta1" k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/scheme" typedcorev1 "k8s.io/client-go/kubernetes/typed/core/v1" @@ -144,16 +145,16 @@ func (c *Controller) creationMutationHandler(review admv1beta1.AdmissionReview) return review, errors.Wrapf(err, "error creating patch for Fleet %s", fleet.ObjectMeta.Name) } - json, err := json.Marshal(patch) + jsn, err := json.Marshal(patch) if err != nil { return review, errors.Wrapf(err, "error creating json for patch for Fleet %s", fleet.ObjectMeta.Name) } - c.logger.WithField("fleet", fleet.ObjectMeta.Name).WithField("patch", string(json)).Infof("patch created!") + c.logger.WithField("fleet", fleet.ObjectMeta.Name).WithField("patch", string(jsn)).Infof("patch created!") pt := admv1beta1.PatchTypeJSONPatch review.Response.PatchType = &pt - review.Response.Patch = json + review.Response.Patch = jsn return review, nil } @@ -224,34 +225,34 @@ func (c *Controller) syncFleet(key string) error { return err } - activeGsSet, rest := c.filterGameServerSetByActive(fleet, list) - if err := c.applyDeploymentStrategy(fleet, rest); err != nil { + active, rest := c.filterGameServerSetByActive(fleet, list) + // if there isn't an active gameServerSet, create one (but don't persist yet) + if active == nil { + c.logger.WithField("fleet", fleet.ObjectMeta.Name).Info("could not find active GameServerSet, creating") + active = fleet.GameServerSet() + } + + replicas, err := c.applyDeploymentStrategy(fleet, active, rest) + if err != nil { return err } if err := c.deleteEmptyGameServerSets(fleet, rest); err != nil { return err } - // if there isn't an active gameServerSet, create one (but don't persist yet) - if activeGsSet == nil { - c.logger.WithField("fleet", fleet.ObjectMeta.Name).Info("could not find active GameServerSet, creating") - activeGsSet = fleet.GameServerSet() - } - - replicas := fleet.ReplicasMinusSumAllocated(rest) - if err := c.upsertGameServerSet(fleet, activeGsSet, replicas); err != nil { + if err := c.upsertGameServerSet(fleet, active, replicas); err != nil { return err } return c.updateFleetStatus(fleet) } // upsertGameServerSet if the GameServerSet is new, insert it -// if the replicas minus sum allocated does not match the active +// if the replicas do not match the active // GameServerSet, then update it -func (c *Controller) upsertGameServerSet(fleet *stablev1alpha1.Fleet, gsSet *stablev1alpha1.GameServerSet, replicas int32) error { - if gsSet.ObjectMeta.UID == "" { - gsSet.Spec.Replicas = replicas - gsSet, err := c.gameServerSetGetter.GameServerSets(gsSet.ObjectMeta.Namespace).Create(gsSet) +func (c *Controller) upsertGameServerSet(fleet *stablev1alpha1.Fleet, active *stablev1alpha1.GameServerSet, replicas int32) error { + if active.ObjectMeta.UID == "" { + active.Spec.Replicas = replicas + gsSet, err := c.gameServerSetGetter.GameServerSets(active.ObjectMeta.Namespace).Create(active) if err != nil { return errors.Wrapf(err, "error creating gameserverset for fleet %s", fleet.ObjectMeta.Name) } @@ -261,15 +262,15 @@ func (c *Controller) upsertGameServerSet(fleet *stablev1alpha1.Fleet, gsSet *sta return nil } - if replicas != gsSet.Spec.Replicas { - gsSetCopy := gsSet.DeepCopy() + if replicas != active.Spec.Replicas { + gsSetCopy := active.DeepCopy() gsSetCopy.Spec.Replicas = replicas 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) } c.recorder.Eventf(fleet, corev1.EventTypeNormal, "ScalingGameServerSet", - "Scaling active GameServerSet %s from %d to %d", gsSetCopy.ObjectMeta.Name, gsSet.Spec.Replicas, gsSetCopy.Spec.Replicas) + "Scaling active GameServerSet %s from %d to %d", gsSetCopy.ObjectMeta.Name, active.Spec.Replicas, gsSetCopy.Spec.Replicas) } return nil @@ -277,17 +278,21 @@ func (c *Controller) upsertGameServerSet(fleet *stablev1alpha1.Fleet, gsSet *sta // applyDeploymentStrategy applies the Fleet > Spec > Deployment strategy to all the non-active // GameServerSets that are passed in -func (c *Controller) applyDeploymentStrategy(fleet *stablev1alpha1.Fleet, list []*stablev1alpha1.GameServerSet) error { +func (c *Controller) applyDeploymentStrategy(fleet *stablev1alpha1.Fleet, active *stablev1alpha1.GameServerSet, rest []*stablev1alpha1.GameServerSet) (int32, error) { + // if there is nothing `rest`, then it's either brand Fleet, or we can just jump to the fleet value, + // since there is nothing else scaling down at this point + if len(rest) == 0 { + return fleet.Spec.Replicas, nil + } + switch fleet.Spec.Strategy.Type { case appsv1.RecreateDeploymentStrategyType: - return c.recreateDeployment(list) + return c.recreateDeployment(fleet, rest) case appsv1.RollingUpdateDeploymentStrategyType: - // in this PR, we are only implementing Recreate. - // we will do this next - panic("this is not implemented!") + return c.rollingUpdateDeployment(fleet, active, rest) } - return errors.Errorf("unexpected deployment strategy type: %s", fleet.Spec.Strategy.Type) + return 0, errors.Errorf("unexpected deployment strategy type: %s", fleet.Spec.Strategy.Type) } // deleteEmptyGameServerSets deletes all GameServerServerSets @@ -309,16 +314,102 @@ func (c *Controller) deleteEmptyGameServerSets(fleet *stablev1alpha1.Fleet, list } // recreateDeployment applies the recreate deployment strategy to all non-active -// GameServerSets -func (c *Controller) recreateDeployment(list []*stablev1alpha1.GameServerSet) error { - for _, gsSet := range list { +// GameServerSets, and return the replica count for the active GameServerSet +func (c *Controller) recreateDeployment(fleet *stablev1alpha1.Fleet, rest []*stablev1alpha1.GameServerSet) (int32, error) { + for _, gsSet := range rest { if gsSet.Spec.Replicas != 0 { c.logger.WithField("gameserverset", gsSet.ObjectMeta.Name).Info("applying recreate deployment: scaling to 0") gsSetCopy := gsSet.DeepCopy() gsSetCopy.Spec.Replicas = 0 + if _, err := c.gameServerSetGetter.GameServerSets(gsSetCopy.ObjectMeta.Namespace).Update(gsSetCopy); err != nil { + return 0, errors.Wrapf(err, "error updating gameserverset %s", gsSetCopy.ObjectMeta.Name) + } + c.recorder.Eventf(fleet, corev1.EventTypeNormal, "ScalingGameServerSet", + "Scaling inactive GameServerSet %s from %d to %d", gsSetCopy.ObjectMeta.Name, gsSet.Spec.Replicas, gsSetCopy.Spec.Replicas) + } + } + + return fleet.LowerBoundReplicas(fleet.Spec.Replicas - stablev1alpha1.SumStatusAllocatedReplicas(rest)), nil +} + +// rollingUpdateDeployment will do the rolling update of the old GameServers +// through to the new ones, based on the fleet.Spec.Strategy.RollingUpdate configuration +// and return the replica count for the active GameServerSet +func (c *Controller) rollingUpdateDeployment(fleet *stablev1alpha1.Fleet, active *stablev1alpha1.GameServerSet, rest []*stablev1alpha1.GameServerSet) (int32, error) { + replicas, err := c.rollingUpdateActive(fleet, active, rest) + if err != nil { + return replicas, err + } + if err := c.rollingUpdateRest(fleet, rest); err != nil { + return replicas, err + } + return replicas, nil +} + +// rollingUpdateActive applies the rolling update to the active GameServerSet +// and returns what its replica value should be set to +func (c *Controller) rollingUpdateActive(fleet *stablev1alpha1.Fleet, active *stablev1alpha1.GameServerSet, rest []*stablev1alpha1.GameServerSet) (int32, error) { + replicas := active.Spec.Replicas + + if active.Spec.Replicas < fleet.Spec.Replicas && active.Spec.Replicas == active.Status.Replicas { + r, err := intstr.GetValueFromIntOrPercent(fleet.Spec.Strategy.RollingUpdate.MaxSurge, int(fleet.Spec.Replicas), true) + if err != nil { + return replicas, errors.Wrapf(err, "error calculating scaling gameserverset: %s", fleet.ObjectMeta.Name) + } + surge := int32(r) + + // make sure we don't end up with more than the configured max surge + maxSurge := surge + fleet.Spec.Replicas + replicas = fleet.UpperBoundReplicas(replicas + surge) + total := stablev1alpha1.SumStatusReplicas(rest) + replicas + if total > maxSurge { + replicas = fleet.LowerBoundReplicas(replicas - (total - maxSurge)) + } + + // always leave room for Allocated GameServers + sumAllocated := stablev1alpha1.SumStatusAllocatedReplicas(rest) + + // make room for allocated game servers, but not over the fleet replica count + if replicas+sumAllocated > fleet.Spec.Replicas { + replicas = fleet.LowerBoundReplicas(replicas - sumAllocated) + } + + c.logger.WithField("gameserverset", active.ObjectMeta.Name).WithField("replicas", replicas). + Info("applying rolling update to active gameserverset") + } + + return replicas, nil +} + +// rollingUpdateRest applies the rolling update to the inactive GameServerSets +func (c *Controller) rollingUpdateRest(fleet *stablev1alpha1.Fleet, rest []*stablev1alpha1.GameServerSet) error { + if len(rest) == 0 { + return nil + } + + r, err := intstr.GetValueFromIntOrPercent(fleet.Spec.Strategy.RollingUpdate.MaxUnavailable, int(fleet.Spec.Replicas), true) + if err != nil { + return errors.Wrapf(err, "error calculating scaling gameserverset: %s", fleet.ObjectMeta.Name) + } + unavailable := int32(r) + + for _, gsSet := range rest { + if gsSet.Status.Replicas > 0 && gsSet.Status.Replicas == gsSet.Spec.Replicas { + gsSetCopy := gsSet.DeepCopy() + gsSetCopy.Spec.Replicas = fleet.LowerBoundReplicas(gsSetCopy.Spec.Replicas - unavailable) + + c.logger.WithField("gameserverset", gsSet.ObjectMeta.Name).WithField("replicas", gsSetCopy.Spec.Replicas). + Info("applying rolling update to inactive gameserverset") + if _, err := c.gameServerSetGetter.GameServerSets(gsSetCopy.ObjectMeta.Namespace).Update(gsSetCopy); err != nil { return errors.Wrapf(err, "error updating gameserverset %s", gsSetCopy.ObjectMeta.Name) } + c.recorder.Eventf(fleet, corev1.EventTypeNormal, "ScalingGameServerSet", + "Scaling inactive GameServerSet %s from %d to %d", gsSetCopy.ObjectMeta.Name, gsSet.Spec.Replicas, gsSetCopy.Spec.Replicas) + + // let's update just one at a time, slightly slower, but a simpler solution that doesn't require us + // to make sure we don't overshoot the amount that is being shutdown at any given point and time + break } } diff --git a/pkg/fleets/controller_test.go b/pkg/fleets/controller_test.go index 7806e606f7..861b70d4a5 100644 --- a/pkg/fleets/controller_test.go +++ b/pkg/fleets/controller_test.go @@ -15,20 +15,22 @@ package fleets import ( + "encoding/json" "testing" "time" - "encoding/json" - "agones.dev/agones/pkg/apis/stable/v1alpha1" agtesting "agones.dev/agones/pkg/testing" "agones.dev/agones/pkg/util/webhooks" "github.com/heptiolabs/healthcheck" "github.com/mattbaird/jsonpatch" + "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" admv1beta1 "k8s.io/api/admission/v1beta1" + appsv1 "k8s.io/api/apps/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/apimachinery/pkg/watch" k8stesting "k8s.io/client-go/testing" "k8s.io/client-go/tools/cache" @@ -52,6 +54,7 @@ func TestControllerSyncFleet(t *testing.T) { created = true assert.True(t, metav1.IsControlledBy(gsSet, f)) + assert.Equal(t, f.Spec.Replicas, gsSet.Spec.Replicas) return true, gsSet, nil }) @@ -68,10 +71,12 @@ func TestControllerSyncFleet(t *testing.T) { t.Run("gamserverset with the same number of replicas", func(t *testing.T) { t.Parallel() f := defaultFixture() + f.Spec.Strategy.Type = appsv1.RecreateDeploymentStrategyType c, m := newFakeController() gsSet := f.GameServerSet() gsSet.ObjectMeta.Name = "gsSet1" - gsSet.ObjectMeta.UID = "1234" + gsSet.ObjectMeta.UID = "4321" + gsSet.Spec.Replicas = f.Spec.Replicas m.AgonesClient.AddReactor("list", "fleets", func(action k8stesting.Action) (bool, runtime.Object, error) { return true, &v1alpha1.FleetList{Items: []v1alpha1.Fleet{*f}}, nil @@ -101,6 +106,7 @@ func TestControllerSyncFleet(t *testing.T) { t.Run("gameserverset with different number of replicas", func(t *testing.T) { f := defaultFixture() + f.Spec.Strategy.Type = appsv1.RecreateDeploymentStrategyType c, m := newFakeController() gsSet := f.GameServerSet() gsSet.ObjectMeta.Name = "gsSet1" @@ -137,12 +143,14 @@ func TestControllerSyncFleet(t *testing.T) { t.Run("gameserverset with different image details", func(t *testing.T) { f := defaultFixture() + f.Spec.Strategy.Type = appsv1.RollingUpdateDeploymentStrategyType f.Spec.Template.Spec.HostPort = 5555 c, m := newFakeController() gsSet := f.GameServerSet() gsSet.ObjectMeta.Name = "gsSet1" - gsSet.ObjectMeta.UID = "1234" + gsSet.ObjectMeta.UID = "4321" gsSet.Spec.Template.Spec.HostPort = 7777 + gsSet.Spec.Replicas = f.Spec.Replicas gsSet.Status.Replicas = 5 updated := false created := false @@ -159,6 +167,7 @@ func TestControllerSyncFleet(t *testing.T) { created = true ca := action.(k8stesting.CreateAction) gsSet := ca.GetObject().(*v1alpha1.GameServerSet) + assert.Equal(t, int32(2), gsSet.Spec.Replicas) assert.Equal(t, f.Spec.Template.Spec.HostPort, gsSet.Spec.Template.Spec.HostPort) return true, gsSet, nil @@ -168,7 +177,7 @@ func TestControllerSyncFleet(t *testing.T) { updated = true ua := action.(k8stesting.UpdateAction) gsSet := ua.GetObject().(*v1alpha1.GameServerSet) - assert.Equal(t, int32(0), gsSet.Spec.Replicas) + assert.Equal(t, int32(3), gsSet.Spec.Replicas) assert.Equal(t, "gsSet1", gsSet.ObjectMeta.Name) return true, gsSet, nil @@ -181,6 +190,7 @@ func TestControllerSyncFleet(t *testing.T) { assert.Nil(t, err) assert.True(t, updated, "gameserverset should have been updated") assert.True(t, created, "gameserverset should have been created") + agtesting.AssertEventContains(t, m.FakeRecorder.Events, "ScalingGameServerSet") agtesting.AssertEventContains(t, m.FakeRecorder.Events, "CreatingGameServerSet") }) } @@ -226,7 +236,7 @@ func TestControllerCreationMutationHandler(t *testing.T) { assert.True(t, found, "Could not find operation %#v in patch %v", op, *patch) } - assertContains(patch, jsonpatch.JsonPatchOperation{Operation: "add", Path: "/spec/strategy/type", Value: "Recreate"}) + assertContains(patch, jsonpatch.JsonPatchOperation{Operation: "add", Path: "/spec/strategy/type", Value: "RollingUpdate"}) } func TestControllerRun(t *testing.T) { @@ -340,6 +350,8 @@ func TestControllerUpdateFleetStatus(t *testing.T) { } func TestControllerFilterGameServerSetByActive(t *testing.T) { + t.Parallel() + f := defaultFixture() c, _ := newFakeController() // the same GameServer Template @@ -366,12 +378,15 @@ func TestControllerRecreateDeployment(t *testing.T) { t.Parallel() f := defaultFixture() + f.Spec.Strategy.Type = appsv1.RecreateDeploymentStrategyType + f.Spec.Replicas = 10 gsSet1 := f.GameServerSet() gsSet1.ObjectMeta.Name = "gsSet1" gsSet1.Spec.Replicas = 10 gsSet2 := f.GameServerSet() gsSet2.ObjectMeta.Name = "gsSet2" gsSet2.Spec.Replicas = 0 + gsSet2.Status.AllocatedReplicas = 1 c, m := newFakeController() @@ -386,40 +401,96 @@ func TestControllerRecreateDeployment(t *testing.T) { return true, gsSet, nil }) - err := c.recreateDeployment([]*v1alpha1.GameServerSet{gsSet1, gsSet2}) + replicas, err := c.recreateDeployment(f, []*v1alpha1.GameServerSet{gsSet1, gsSet2}) assert.Nil(t, err) assert.True(t, updated) + assert.Equal(t, f.Spec.Replicas-1, replicas) + agtesting.AssertEventContains(t, m.FakeRecorder.Events, "ScalingGameServerSet") } func TestControllerApplyDeploymentStrategy(t *testing.T) { t.Parallel() - f := defaultFixture() - gsSet1 := f.GameServerSet() - gsSet1.ObjectMeta.Name = "gsSet1" - gsSet1.Spec.Replicas = 10 - gsSet1.Status.Replicas = 10 - gsSet2 := f.GameServerSet() - gsSet2.ObjectMeta.Name = "gsSet2" - gsSet2.Spec.Replicas = 0 - gsSet2.Status.Replicas = 0 + type expected struct { + inactiveReplicas int32 + replicas int32 + } - c, m := newFakeController() + fixtures := map[string]struct { + strategyType appsv1.DeploymentStrategyType + gsSet1StatusReplicas int32 + gsSet2StatusReplicas int32 + expected expected + }{ + string(appsv1.RecreateDeploymentStrategyType): { + strategyType: appsv1.RecreateDeploymentStrategyType, + gsSet1StatusReplicas: 0, + gsSet2StatusReplicas: 0, + expected: expected{ + inactiveReplicas: 0, + replicas: 10, + }, + }, + string(appsv1.RollingUpdateDeploymentStrategyType): { + strategyType: appsv1.RecreateDeploymentStrategyType, + gsSet1StatusReplicas: 10, + gsSet2StatusReplicas: 1, + expected: expected{ + inactiveReplicas: 7, + replicas: 2, + }, + }, + } - updated := false - 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, gsSet1.ObjectMeta.Name, gsSet.ObjectMeta.Name) - assert.Equal(t, int32(0), gsSet.Spec.Replicas) + for k, v := range fixtures { + t.Run(k, func(t *testing.T) { + f := defaultFixture() + f.Spec.Strategy.Type = v.strategyType + f.Spec.Replicas = 10 + + gsSet1 := f.GameServerSet() + gsSet1.ObjectMeta.Name = "gsSet1" + gsSet1.Spec.Replicas = 10 + gsSet1.Status.Replicas = v.gsSet1StatusReplicas + + gsSet2 := f.GameServerSet() + gsSet2.ObjectMeta.Name = "gsSet2" + gsSet2.Spec.Replicas = 0 + gsSet2.Status.Replicas = v.gsSet2StatusReplicas + + c, m := newFakeController() + + updated := false + 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, gsSet1.ObjectMeta.Name, gsSet.ObjectMeta.Name) + assert.Equal(t, int32(0), gsSet.Spec.Replicas) + + return true, gsSet, nil + }) + + replicas, err := c.applyDeploymentStrategy(f, f.GameServerSet(), []*v1alpha1.GameServerSet{gsSet1, gsSet2}) + assert.Nil(t, err) + assert.True(t, updated, "update should happen") + assert.Equal(t, f.Spec.Replicas, replicas) + }) + } - return true, gsSet, nil - }) + t.Run("a single gameserverset", func(t *testing.T) { + f := defaultFixture() + f.Spec.Replicas = 10 - err := c.applyDeploymentStrategy(f, []*v1alpha1.GameServerSet{gsSet1, gsSet2}) - assert.Nil(t, err) - assert.True(t, updated, "update should happen") + gsSet1 := f.GameServerSet() + gsSet1.ObjectMeta.Name = "gsSet1" + + c, _ := newFakeController() + + replicas, err := c.applyDeploymentStrategy(f, f.GameServerSet(), []*v1alpha1.GameServerSet{}) + assert.Nil(t, err) + assert.Equal(t, f.Spec.Replicas, replicas) + }) } func TestControllerUpsertGameServerSet(t *testing.T) { @@ -521,6 +592,141 @@ func TestControllerDeleteEmptyGameServerSets(t *testing.T) { assert.True(t, deleted, "delete should happen") } +func TestControllerRollingUpdateDeployment(t *testing.T) { + t.Parallel() + + type expected struct { + inactiveSpecReplicas int32 + replicas int32 + updated bool + } + + fixtures := map[string]struct { + fleetSpecReplicas int32 + activeSpecReplicas int32 + activeStatusReplicas int32 + inactiveSpecReplicas int32 + inactiveStatusReplicas int32 + inactiveStatusAllocationReplicas int32 + expected expected + }{ + "full inactive, empty inactive": { + fleetSpecReplicas: 100, + activeSpecReplicas: 0, + activeStatusReplicas: 0, + inactiveSpecReplicas: 100, + inactiveStatusReplicas: 100, + expected: expected{ + inactiveSpecReplicas: 70, + replicas: 25, + updated: true, + }, + }, + "almost empty inactive with allocated, almost full active": { + fleetSpecReplicas: 100, + activeSpecReplicas: 75, + activeStatusReplicas: 75, + inactiveSpecReplicas: 10, + inactiveStatusReplicas: 10, + inactiveStatusAllocationReplicas: 5, + + expected: expected{ + inactiveSpecReplicas: 0, + replicas: 95, + updated: true, + }, + }, + "attempt to drive replicas over the max surge": { + fleetSpecReplicas: 100, + activeSpecReplicas: 25, + activeStatusReplicas: 25, + inactiveSpecReplicas: 95, + inactiveStatusReplicas: 95, + expected: expected{ + inactiveSpecReplicas: 65, + replicas: 30, + updated: true, + }, + }, + "statuses don't match the spec. nothing should happen": { + fleetSpecReplicas: 100, + activeSpecReplicas: 75, + activeStatusReplicas: 70, + inactiveSpecReplicas: 15, + inactiveStatusReplicas: 10, + expected: expected{ + inactiveSpecReplicas: 15, + replicas: 75, + updated: false, + }, + }, + "test smalled numbers of active and allocated": { + fleetSpecReplicas: 5, + activeSpecReplicas: 0, + activeStatusReplicas: 0, + inactiveSpecReplicas: 5, + inactiveStatusReplicas: 5, + inactiveStatusAllocationReplicas: 2, + + expected: expected{ + inactiveSpecReplicas: 3, + replicas: 2, + updated: true, + }, + }, + } + + for k, v := range fixtures { + t.Run(k, func(t *testing.T) { + f := defaultFixture() + f.ApplyDefaults() + mu := intstr.FromString("30%") + f.Spec.Strategy.RollingUpdate.MaxUnavailable = &mu + f.Spec.Replicas = v.fleetSpecReplicas + + // gate + assert.Equal(t, "25%", f.Spec.Strategy.RollingUpdate.MaxSurge.String()) + assert.Equal(t, "30%", f.Spec.Strategy.RollingUpdate.MaxUnavailable.String()) + + active := f.GameServerSet() + active.ObjectMeta.Name = "active" + active.Spec.Replicas = v.activeSpecReplicas + active.Status.Replicas = v.activeStatusReplicas + + inactive := f.GameServerSet() + inactive.ObjectMeta.Name = "inactive" + inactive.Spec.Replicas = v.inactiveSpecReplicas + inactive.Status.Replicas = v.inactiveStatusReplicas + inactive.Status.AllocatedReplicas = v.inactiveStatusAllocationReplicas + + logrus.WithField("inactive", inactive).Info("Setting up the initial inactive") + + updated := false + c, m := newFakeController() + + 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, inactive.ObjectMeta.Name, gsSet.ObjectMeta.Name) + assert.Equal(t, v.expected.inactiveSpecReplicas, gsSet.Spec.Replicas) + + return true, gsSet, nil + }) + + replicas, err := c.rollingUpdateDeployment(f, active, []*v1alpha1.GameServerSet{inactive}) + assert.Nil(t, err) + assert.Equal(t, v.expected.replicas, replicas) + assert.Equal(t, v.expected.updated, updated) + if updated { + agtesting.AssertEventContains(t, m.FakeRecorder.Events, "ScalingGameServerSet") + } else { + agtesting.AssertNoEvent(t, m.FakeRecorder.Events) + } + }) + } +} + // newFakeController returns a controller, backed by the fake Clientset func newFakeController() (*Controller, agtesting.Mocks) { m := agtesting.NewMocks()