diff --git a/cmd/controller/main.go b/cmd/controller/main.go index 1a06884e71..f1e9a233a4 100644 --- a/cmd/controller/main.go +++ b/cmd/controller/main.go @@ -224,7 +224,7 @@ func main() { kubeClient, extClient, agonesClient, agonesInformerFactory) fleetController := fleets.NewController(health, kubeClient, extClient, agonesClient, agonesInformerFactory) fasController := fleetautoscalers.NewController(health, - kubeClient, extClient, agonesClient, agonesInformerFactory) + kubeClient, extClient, agonesClient, agonesInformerFactory, gsCounter) rs = append(rs, gsCounter, gsController, gsSetController, fleetController, fasController) diff --git a/pkg/apis/agones/v1/common.go b/pkg/apis/agones/v1/common.go index 696fc7b95a..7badd34b35 100644 --- a/pkg/apis/agones/v1/common.go +++ b/pkg/apis/agones/v1/common.go @@ -34,18 +34,18 @@ const ( ErrContainerPortRequired = "ContainerPort must be defined for Dynamic and Static PortPolicies" ErrContainerPortPassthrough = "ContainerPort cannot be specified with Passthrough PortPolicy" ErrContainerNameInvalid = "Container must be empty or the name of a container in the pod template" - // GameServerAllocationIncrement is a Counter Action that indiciates the Counter's Count should be incremented at Allocation. - GameServerAllocationIncrement string = "Increment" - // GameServerAllocationDecrement is a Counter Action that indiciates the Counter's Count should be decremented at Allocation. - GameServerAllocationDecrement string = "Decrement" - // GameServerAllocationPriorityCounter is a PriorityType for sorting Game Servers by Counter - GameServerAllocationPriorityCounter string = "Counter" - // GameServerAllocationPriorityList is a PriorityType for sorting Game Servers by List - GameServerAllocationPriorityList string = "List" - // GameServerAllocationAscending is a Priority Order where the smaller count is preferred in sorting. - GameServerAllocationAscending string = "Ascending" - // GameServerAllocationDescending is a Priority Order where the larger count is preferred in sorting. - GameServerAllocationDescending string = "Descending" + // GameServerPriorityIncrement is a Counter Action that indiciates the Counter's Count should be incremented at Allocation. + GameServerPriorityIncrement string = "Increment" + // GameServerPriorityDecrement is a Counter Action that indiciates the Counter's Count should be decremented at Allocation. + GameServerPriorityDecrement string = "Decrement" + // GameServerPriorityCounter is a PriorityType for sorting Game Servers by Counter + GameServerPriorityCounter string = "Counter" + // GameServerPriorityList is a PriorityType for sorting Game Servers by List + GameServerPriorityList string = "List" + // GameServerPriorityAscending is a Priority Order where the smaller count is preferred in sorting. + GameServerPriorityAscending string = "Ascending" + // GameServerPriorityDescending is a Priority Order where the larger count is preferred in sorting. + GameServerPriorityDescending string = "Descending" ) // AggregatedPlayerStatus stores total player tracking values @@ -236,7 +236,7 @@ type Priority struct { func (p *Priority) Validate(f string) ([]metav1.StatusCause, bool) { var causes []metav1.StatusCause - if !(p.PriorityType == GameServerAllocationPriorityCounter || p.PriorityType == GameServerAllocationPriorityList) { + if !(p.PriorityType == GameServerPriorityCounter || p.PriorityType == GameServerPriorityList) { causes = append(causes, metav1.StatusCause{ Type: metav1.CauseTypeFieldValueInvalid, Message: "Priority.Sort must be either `Counter` or `List`", @@ -244,7 +244,7 @@ func (p *Priority) Validate(f string) ([]metav1.StatusCause, bool) { }) } - if !(p.Order == GameServerAllocationAscending || p.Order == GameServerAllocationDescending || p.Order == "") { + if !(p.Order == GameServerPriorityAscending || p.Order == GameServerPriorityDescending || p.Order == "") { causes = append(causes, metav1.StatusCause{ Type: metav1.CauseTypeFieldValueInvalid, Message: "Priority.Order must be either `Ascending` or `Descending`", diff --git a/pkg/apis/agones/v1/gameserver.go b/pkg/apis/agones/v1/gameserver.go index 7d28718365..bdd171c21f 100644 --- a/pkg/apis/agones/v1/gameserver.go +++ b/pkg/apis/agones/v1/gameserver.go @@ -914,15 +914,15 @@ func (gs *GameServer) Patch(delta *GameServer) ([]byte, error) { // UpdateCount increments or decrements a CounterStatus on a Game Server by the given amount. func (gs *GameServer) UpdateCount(name string, action string, amount int64) error { - if !(action == GameServerAllocationIncrement || action == GameServerAllocationDecrement) { - return errors.Errorf("unable to UpdateCount with Name %s, Action %s, Amount %d. Allocation action must be one of %s or %s", name, action, amount, GameServerAllocationIncrement, GameServerAllocationDecrement) + if !(action == GameServerPriorityIncrement || action == GameServerPriorityDecrement) { + return errors.Errorf("unable to UpdateCount with Name %s, Action %s, Amount %d. Allocation action must be one of %s or %s", name, action, amount, GameServerPriorityIncrement, GameServerPriorityDecrement) } if amount < 0 { return errors.Errorf("unable to UpdateCount with Name %s, Action %s, Amount %d. Amount must be greater than 0", name, action, amount) } if counter, ok := gs.Status.Counters[name]; ok { cnt := counter.Count - if action == GameServerAllocationIncrement { + if action == GameServerPriorityIncrement { cnt += amount // only check for Count > Capacity when incrementing if cnt > counter.Capacity { diff --git a/pkg/apis/autoscaling/v1/fleetautoscaler.go b/pkg/apis/autoscaling/v1/fleetautoscaler.go index f671f38755..a03faddae9 100644 --- a/pkg/apis/autoscaling/v1/fleetautoscaler.go +++ b/pkg/apis/autoscaling/v1/fleetautoscaler.go @@ -429,7 +429,7 @@ func (c *CounterPolicy) ValidateCounterPolicy(causes []metav1.StatusCause) []met causes = append(causes, metav1.StatusCause{ Type: metav1.CauseTypeFieldValueInvalid, Field: "bufferSize", - Message: "bufferSize must be bigger than 0", + Message: "bufferSize must be bigger than 0", // TODO: Should we allow bufferSize to be Zero (in case of Scaling down to 0)? In case of BufferPercentage MaxCapacity can be set to 0, in case of BufferInt Max Capacity must be >= 2. }) } if c.MaxCapacity < int64(c.BufferSize.IntValue()) { diff --git a/pkg/fleetautoscalers/controller.go b/pkg/fleetautoscalers/controller.go index d654ad3f4a..4e0f264999 100644 --- a/pkg/fleetautoscalers/controller.go +++ b/pkg/fleetautoscalers/controller.go @@ -30,6 +30,7 @@ import ( "agones.dev/agones/pkg/client/informers/externalversions" listeragonesv1 "agones.dev/agones/pkg/client/listers/agones/v1" listerautoscalingv1 "agones.dev/agones/pkg/client/listers/autoscaling/v1" + "agones.dev/agones/pkg/gameservers" "agones.dev/agones/pkg/util/crd" "agones.dev/agones/pkg/util/logfields" "agones.dev/agones/pkg/util/runtime" @@ -76,6 +77,7 @@ type Extensions struct { type Controller struct { baseLogger *logrus.Entry clock clock.WithTickerAndDelayedExecution + counter *gameservers.PerNodeCounter crdGetter apiextclientv1.CustomResourceDefinitionInterface fasThreads map[types.UID]fasThread fasThreadMutex sync.Mutex @@ -87,6 +89,7 @@ type Controller struct { fleetAutoscalerSynced cache.InformerSynced workerqueue *workerqueue.WorkerQueue recorder record.EventRecorder + gameServerLister listeragonesv1.GameServerLister } // NewController returns a controller for a FleetAutoscaler @@ -95,12 +98,16 @@ func NewController( kubeClient kubernetes.Interface, extClient extclientset.Interface, agonesClient versioned.Interface, - agonesInformerFactory externalversions.SharedInformerFactory) *Controller { + agonesInformerFactory externalversions.SharedInformerFactory, + counter *gameservers.PerNodeCounter) *Controller { autoscaler := agonesInformerFactory.Autoscaling().V1().FleetAutoscalers() fleetInformer := agonesInformerFactory.Agones().V1().Fleets() + gameServers := agonesInformerFactory.Agones().V1().GameServers() + c := &Controller{ clock: clock.RealClock{}, + counter: counter, crdGetter: extClient.ApiextensionsV1().CustomResourceDefinitions(), fasThreads: map[types.UID]fasThread{}, fasThreadMutex: sync.Mutex{}, @@ -110,6 +117,7 @@ func NewController( fleetAutoscalerGetter: agonesClient.AutoscalingV1(), fleetAutoscalerLister: autoscaler.Lister(), fleetAutoscalerSynced: autoscaler.Informer().HasSynced, + gameServerLister: gameServers.Lister(), } c.baseLogger = runtime.NewLoggerWithType(c) c.workerqueue = workerqueue.NewWorkerQueueWithRateLimiter(c.syncFleetAutoscaler, c.baseLogger, logfields.FleetAutoscalerKey, autoscaling.GroupName+".FleetAutoscalerController", workerqueue.FastRateLimiter(3*time.Second)) @@ -326,7 +334,7 @@ func (c *Controller) syncFleetAutoscaler(ctx context.Context, key string) error } currentReplicas := fleet.Status.Replicas - desiredReplicas, scalingLimited, err := computeDesiredFleetSize(fas, fleet) + desiredReplicas, scalingLimited, err := computeDesiredFleetSize(fas, fleet, c.gameServerLister, c.counter.Counts()) if err != nil { c.recorder.Eventf(fas, corev1.EventTypeWarning, "FleetAutoscaler", "Error calculating desired fleet size on FleetAutoscaler %s. Error: %s", fas.ObjectMeta.Name, err.Error()) diff --git a/pkg/fleetautoscalers/controller_test.go b/pkg/fleetautoscalers/controller_test.go index e823ff0f33..26b96516e9 100644 --- a/pkg/fleetautoscalers/controller_test.go +++ b/pkg/fleetautoscalers/controller_test.go @@ -24,8 +24,10 @@ import ( "testing" "time" + "agones.dev/agones/pkg/apis" agonesv1 "agones.dev/agones/pkg/apis/agones/v1" autoscalingv1 "agones.dev/agones/pkg/apis/autoscaling/v1" + "agones.dev/agones/pkg/gameservers" agtesting "agones.dev/agones/pkg/testing" utilruntime "agones.dev/agones/pkg/util/runtime" "agones.dev/agones/pkg/util/webhooks" @@ -693,7 +695,7 @@ func TestControllerSyncFleetAutoscaler(t *testing.T) { err := c.syncFleetAutoscaler(ctx, "default/fas-1") if assert.NotNil(t, err) { - assert.Equal(t, "error calculating autoscaling fleet: fleet-1: wrong policy type, should be one of: Buffer, Webhook", err.Error()) + assert.Equal(t, "error calculating autoscaling fleet: fleet-1: wrong policy type, should be one of: Buffer, Webhook, Counter, List", err.Error()) } }) @@ -1189,7 +1191,11 @@ func defaultFixtures() (*autoscalingv1.FleetAutoscaler, *agonesv1.Fleet) { }, Spec: agonesv1.FleetSpec{ Replicas: 8, - Template: agonesv1.GameServerTemplateSpec{}, + Template: agonesv1.GameServerTemplateSpec{ + Spec: agonesv1.GameServerSpec{ + Scheduling: apis.Packed, + }, + }, }, Status: agonesv1.FleetStatus{ Replicas: 5, @@ -1245,7 +1251,8 @@ func defaultWebhookFixtures() (*autoscalingv1.FleetAutoscaler, *agonesv1.Fleet) // newFakeController returns a controller, backed by the fake Clientset func newFakeController() (*Controller, agtesting.Mocks) { m := agtesting.NewMocks() - c := NewController(healthcheck.NewHandler(), m.KubeClient, m.ExtClient, m.AgonesClient, m.AgonesInformerFactory) + counter := gameservers.NewPerNodeCounter(m.KubeInformerFactory, m.AgonesInformerFactory) + c := NewController(healthcheck.NewHandler(), m.KubeClient, m.ExtClient, m.AgonesClient, m.AgonesInformerFactory, counter) c.recorder = m.FakeRecorder return c, m } diff --git a/pkg/fleetautoscalers/fleetautoscalers.go b/pkg/fleetautoscalers/fleetautoscalers.go index a17529c28f..4083a2da70 100644 --- a/pkg/fleetautoscalers/fleetautoscalers.go +++ b/pkg/fleetautoscalers/fleetautoscalers.go @@ -30,6 +30,10 @@ import ( agonesv1 "agones.dev/agones/pkg/apis/agones/v1" autoscalingv1 "agones.dev/agones/pkg/apis/autoscaling/v1" + listeragonesv1 "agones.dev/agones/pkg/client/listers/agones/v1" + "agones.dev/agones/pkg/fleets" + "agones.dev/agones/pkg/gameservers" + gsSets "agones.dev/agones/pkg/gameserversets" "agones.dev/agones/pkg/util/runtime" "github.com/pkg/errors" "k8s.io/apimachinery/pkg/util/intstr" @@ -45,17 +49,20 @@ var client = http.Client{ } // computeDesiredFleetSize computes the new desired size of the given fleet -func computeDesiredFleetSize(fas *autoscalingv1.FleetAutoscaler, f *agonesv1.Fleet) (int32, bool, error) { +func computeDesiredFleetSize(fas *autoscalingv1.FleetAutoscaler, f *agonesv1.Fleet, + gameServerLister listeragonesv1.GameServerLister, nodeCounts map[string]gameservers.NodeCount) (int32, bool, error) { switch fas.Spec.Policy.Type { case autoscalingv1.BufferPolicyType: return applyBufferPolicy(fas.Spec.Policy.Buffer, f) case autoscalingv1.WebhookPolicyType: return applyWebhookPolicy(fas.Spec.Policy.Webhook, f) case autoscalingv1.CounterPolicyType: - return applyCounterPolicy(fas.Spec.Policy.Counter, f) + return applyCounterOrListPolicy(fas.Spec.Policy.Counter, nil, f, gameServerLister, nodeCounts) + case autoscalingv1.ListPolicyType: + return applyCounterOrListPolicy(nil, fas.Spec.Policy.List, f, gameServerLister, nodeCounts) } - return 0, false, errors.New("wrong policy type, should be one of: Buffer, Webhook") + return 0, false, errors.New("wrong policy type, should be one of: Buffer, Webhook, Counter, List") } // buildURLFromWebhookPolicy - build URL for Webhook and set CARoot for client Transport @@ -228,144 +235,424 @@ func applyBufferPolicy(b *autoscalingv1.BufferPolicy, f *agonesv1.Fleet) (int32, return replicas, limited, nil } -func applyCounterPolicy(c *autoscalingv1.CounterPolicy, f *agonesv1.Fleet) (int32, bool, error) { +func applyCounterOrListPolicy(c *autoscalingv1.CounterPolicy, l *autoscalingv1.ListPolicy, + f *agonesv1.Fleet, gameServerLister listeragonesv1.GameServerLister, + nodeCounts map[string]gameservers.NodeCount) (int32, bool, error) { + if !runtime.FeatureEnabled(runtime.FeatureCountsAndLists) { return 0, false, errors.Errorf("cannot apply CounterPolicy unless feature flag %s is enabled", runtime.FeatureCountsAndLists) } - counter, ok := f.Spec.Template.Spec.Counters[c.Key] - if !ok { - return 0, false, errors.Errorf("cannot apply CounterPolicy as counter key %s does not exist in the Fleet Spec", c.Key) - } + var isCounter bool // True if a CounterPolicy False if a ListPolicy + var key string // The specified Counter or List + var count int64 // The Count or number of Values in the template Game Server + var capacity int64 // The Capacity in the template Game Server + var aggCount int64 // The Aggregate Count of the specified Counter or List of all GameServers across the GameServerSet in the Fleet + var aggCapacity int64 // The Aggregate Capacity of the specified Counter or List of all GameServers across the GameServerSet in the Fleet + var minCapacity int64 // The Minimum Aggregate Capacity + var maxCapacity int64 // The Maximum Aggregate Capacity + + var bufferSize intstr.IntOrString + // TODO: Our aggregate counts in include Allocated, Ready, and Reserved replicas, however + // I'm not 100% sure if f.Status.Replicas includes all three states or just Allocated and Ready? + // If it's the latter, then we would need to add in f.Status.ReservedReplicas. + // There may be some additional nuance here since "Reserved instances won't be deleted on scale down, but won't cause an autoscaler to scale up" + replicas := f.Status.Replicas + + if c != nil { + isCounter = true + counter, ok := f.Spec.Template.Spec.Counters[c.Key] + if !ok { + return 0, false, errors.Errorf("cannot apply CounterPolicy as Counter key %s does not exist in the Fleet Spec", c.Key) + } - aggCounter, ok := f.Status.Counters[c.Key] - if !ok { - return 0, false, errors.Errorf("cannot apply CounterPolicy as counter key %s does not exist in the Fleet Status", c.Key) - } + aggCounter, ok := f.Status.Counters[c.Key] + if !ok { + return 0, false, errors.Errorf("cannot apply CounterPolicy as Counter key %s does not exist in the Fleet Status", c.Key) + } + + key = c.Key + count = counter.Count + capacity = counter.Capacity + aggCount = aggCounter.Count + aggCapacity = aggCounter.Capacity + minCapacity = c.MinCapacity + maxCapacity = c.MaxCapacity + bufferSize = c.BufferSize + + } else { + isCounter = false + list, ok := f.Spec.Template.Spec.Lists[l.Key] + if !ok { + return 0, false, errors.Errorf("cannot apply ListPolicy as List key %s does not exist in the Fleet Spec", l.Key) + } - var replicas float64 + aggList, ok := f.Status.Lists[l.Key] + if !ok { + return 0, false, errors.Errorf("cannot apply ListPolicy as List key %s does not exist in the Fleet Status", l.Key) + } - // Current available capacity across the fleet - availableCapacity := float64(aggCounter.Capacity - aggCounter.Count) + key = l.Key + count = int64(len(list.Values)) + capacity = list.Capacity + aggCount = aggList.Count + aggCapacity = aggList.Capacity + minCapacity = l.MinCapacity + maxCapacity = l.MaxCapacity + bufferSize = l.BufferSize + } - // How much available capacity is gained by adding one more replica to the fleet. - replicaCapacity := float64(counter.Capacity - counter.Count) + // CASES: + // No Scaling Integer (at desired Buffer) -- Check if below/above min/max capacity (Limited) and return current number of replicas if not Limited + // Scale Up Integer -- May be Limited before or after Scaling + // Scale Down Integer -- May be Limited before or after Scaling + // No Scaling Percent -- Check if Limited and return current number of replicas if not Limited + // Scale Up Percent -- May be Limited before or after Scaling + // Scale Down Percent -- May be Limited before or after Scaling + // If none of the above return error Unable to Apply Counter or List Policy + + limited, scale := isLimited(aggCapacity, minCapacity, maxCapacity) // Desired replicas based on BufferSize specified as an absolute value (i.e. 5) - if c.BufferSize.Type == intstr.Int { - // If our current available is the same as our buffer, then we already have the desired replicas - buffer := float64(c.BufferSize.IntValue()) - if availableCapacity == buffer { - replicas = float64(f.Status.Replicas) - } else { - diffReplicas := math.Floor((availableCapacity - buffer) / replicaCapacity) - // TODO: Our aggregate counts in include Allocated, Ready, and Reserved replicas, however - // I'm not 100% sure if f.Status.Replicas includes all three states or just Allocated and Ready? - // If it's the latter, then we would need to add in f.Status.ReservedReplicas. - // There may be some additional nuance here since "Reserved instances won't be deleted on scale down, but won't cause an autoscaler to scale up" - replicas = float64(f.Status.Replicas) - diffReplicas + if bufferSize.Type == intstr.Int { + buffer := int64(bufferSize.IntValue()) + + // Current available capacity across the fleet + switch availableCapacity := aggCapacity - aggCount; { + case availableCapacity == buffer: + if limited { + return scaleLimited(scale, f, gameServerLister, nodeCounts, key, isCounter, replicas, + capacity, aggCapacity, minCapacity, maxCapacity) + } + return replicas, false, nil + case availableCapacity < buffer: // Scale Up + if limited && scale == -1 { // Case where we want to scale up but we're already limited by MaxCapacity + return scaleLimited(scale, f, gameServerLister, nodeCounts, key, isCounter, replicas, + capacity, aggCapacity, minCapacity, maxCapacity) + } + return scaleUpByInteger(replicas, capacity, count, + aggCapacity, availableCapacity, maxCapacity, buffer) + case availableCapacity > buffer: // Scale Down + if limited && scale == 1 { // Case where we want to scale down but we're already limited by MinCapacity + return scaleLimited(scale, f, gameServerLister, nodeCounts, key, isCounter, replicas, + capacity, aggCapacity, minCapacity, maxCapacity) + } + return scaleDownByInteger(f, gameServerLister, nodeCounts, key, isCounter, replicas, + availableCapacity, aggCount, aggCapacity, minCapacity, buffer) } - } else { - // Desired replicas based on BufferSize specified as a percent (i.e. 5%) - bufferPercent, err := intstr.GetValueFromIntOrPercent(&c.BufferSize, 100, true) - if err != nil { - return 0, false, err + } + + // Desired replicas based on BufferSize specified as a percent (i.e. 5%) + bufferPercent, err := intstr.GetValueFromIntOrPercent(&bufferSize, 100, isCounter) + if err != nil { + return 0, false, err + } + // The desired total capacity across the fleet (see applyBufferPolicy for explanation) + desiredCapacity := float64(aggCount*100) / float64(100-bufferPercent) + + switch roundedDesiredCapacity := int64(math.Ceil(desiredCapacity)); { + case roundedDesiredCapacity == aggCapacity: + if limited { + return scaleLimited(scale, f, gameServerLister, nodeCounts, key, isCounter, replicas, + capacity, aggCapacity, minCapacity, maxCapacity) + } + return replicas, false, nil + + case roundedDesiredCapacity > aggCapacity: // Scale up + if limited && scale == -1 { // Case where we want to scale up but we're already limited by MaxCapacity + return scaleLimited(scale, f, gameServerLister, nodeCounts, key, isCounter, replicas, + capacity, aggCapacity, minCapacity, maxCapacity) } - // The desired total capacity across the fleet (see applyBufferPolicy for explanation) - desiredCapacity := float64(aggCounter.Count*100) / float64(100-bufferPercent) - if math.Ceil(desiredCapacity) == float64(aggCounter.Capacity) { - replicas = float64(f.Status.Replicas) - } else { - // TODO: How to better handle case where removing ready game servers also reduces the count? - // Using replicaCapacity or using counter.Capacity do not work in all cases. - // replicas = math.Ceil(desiredCapacity / replicaCapacity) - replicas = math.Ceil(desiredCapacity / float64(counter.Capacity)) + return scaleUpByPercent(replicas, count, aggCount, capacity, aggCapacity, maxCapacity, + desiredCapacity, float64(bufferPercent)) + + case roundedDesiredCapacity < aggCapacity: // Scale down + if limited && scale == 1 { // Case where we want to scale down but we're already limited by MinCapacity + return scaleLimited(scale, f, gameServerLister, nodeCounts, key, isCounter, replicas, + capacity, aggCapacity, minCapacity, maxCapacity) } + return scaleDownByPercent(f, gameServerLister, nodeCounts, key, isCounter, replicas, + aggCount, aggCapacity, minCapacity, float64(bufferPercent)) } - // limited indicates that the calculated scale would be above or below the range defined by - // MinCapacity and MaxCapacity - limited := false + if isCounter { + return 0, false, errors.Errorf("unable to apply CounterPolicy %v", c) + } + return 0, false, errors.Errorf("unable to apply ListPolicy %v", l) +} - if replicas < math.Ceil(float64(c.MinCapacity)/float64(counter.Capacity)) { - replicas = math.Ceil(float64(c.MinCapacity) / float64(counter.Capacity)) - limited = true +// getSortedGameServers returns the list of Game Servers for the Fleet in the order in which the +// Game Servers would be deleted. +func getSortedGameServers(f *agonesv1.Fleet, gameServerLister listeragonesv1.GameServerLister, + nodeCounts map[string]gameservers.NodeCount) ([]*agonesv1.GameServer, error) { + // TODO: Should we handle this differently for strategy Distributed? + gss := f.GetGameServerSpec() + gsList, err := fleets.ListGameServersByFleetOwner(gameServerLister, f) + if err != nil { + return nil, err } - // Note that we may have a greater MaxCapacity than stated in the CounterPolicy. For example, if - // the MaxCapacity is 10 and each replica adds 3 to the total capacity we could end up with 4 - // replicas for a total capacity of 12. - if replicas > math.Ceil(float64(c.MaxCapacity)/float64(counter.Capacity)) { - replicas = math.Ceil(float64(c.MaxCapacity) / float64(counter.Capacity)) - limited = true + gameServers := gsSets.SortGameServersByStrategy(gss.Scheduling, gsList, nodeCounts, f.Spec.Priorities) + return gameServers, nil +} + +// isLimited indicates that the calculated scale would be above or below the range defined by +// MinCapacity and MaxCapacity in the ListPolicy or CounterPolicy. +// Return 1 if the fleet needs to scale up, -1 if the fleets need to scale down, 0 if the fleet does +// not need to scale, or if the fleet is not limited. +func isLimited(aggCapacity, minCapacity, maxCapacity int64) (bool, int) { + if aggCapacity < minCapacity { // Scale up + return true, 1 } + if aggCapacity > maxCapacity { // Scale down + return true, -1 + } + return false, 0 +} - return int32(replicas), limited, nil +// scaleUpLimited scales up the fleet to meet the MinCapacity +func scaleUpLimited(replicas int32, capacity, aggCapacity, minCapacity int64) (int32, bool, error) { + if capacity == 0 { + return 0, false, errors.Errorf("cannot scale up as Capacity is equal to 0") + } + for aggCapacity < minCapacity { + aggCapacity += capacity + replicas++ + } + return replicas, true, nil } -func applyListPolicy(l *autoscalingv1.ListPolicy, f *agonesv1.Fleet) (int32, bool, error) { - if !runtime.FeatureEnabled(runtime.FeatureCountsAndLists) { - return 0, false, errors.Errorf("cannot apply ListPolicy unless feature flag %s is enabled", runtime.FeatureCountsAndLists) +// scaleDownLimited scales down the fleet to meet the MaxCapacity +func scaleDownLimited(f *agonesv1.Fleet, gameServerLister listeragonesv1.GameServerLister, + nodeCounts map[string]gameservers.NodeCount, key string, isCounter bool, replicas int32, + aggCapacity, maxCapacity int64) (int32, bool, error) { + // Game Servers in order of deletion on scale down + gameServers, err := getSortedGameServers(f, gameServerLister, nodeCounts) + if err != nil { + return 0, false, err + } + for _, gs := range gameServers { + if aggCapacity <= maxCapacity { + break + } + switch isCounter { + case true: + if counter, ok := gs.Status.Counters[key]; ok { + aggCapacity -= counter.Capacity + } + case false: + if list, ok := gs.Status.Lists[key]; ok { + aggCapacity -= list.Capacity + } + } + replicas-- } - list, ok := f.Spec.Template.Spec.Lists[l.Key] - if !ok { - return 0, false, errors.Errorf("cannot apply ListPolicy as list key %s does not exist in the Fleet Spec", l.Key) + if replicas < 0 { // This shouldn't ever happen, but putting it here just in case. + replicas = 0 } - aggList, ok := f.Status.Lists[l.Key] - if !ok { - return 0, false, errors.Errorf("cannot apply ListPolicy as list key %s does not exist in the Fleet Status", l.Key) + return replicas, true, nil +} + +func scaleLimited(scale int, f *agonesv1.Fleet, gameServerLister listeragonesv1.GameServerLister, + nodeCounts map[string]gameservers.NodeCount, key string, isCounter bool, replicas int32, + capacity, aggCapacity, minCapacity, maxCapacity int64) (int32, bool, error) { + + switch scale { + case 1: // scale up + return scaleUpLimited(replicas, capacity, aggCapacity, minCapacity) + case -1: // scale down + return scaleDownLimited(f, gameServerLister, nodeCounts, key, isCounter, replicas, + aggCapacity, maxCapacity) + case 0: + return replicas, false, nil } - var replicas float64 + return 0, false, errors.Errorf("cannot scale due to error in scaleLimited function") +} - // Current available capacity across the fleet - availableCapacity := float64(aggList.Capacity - aggList.Count) +// scaleUpByInteger +func scaleUpByInteger(replicas int32, capacity, count, aggCapacity, availableCapacity, maxCapacity, + buffer int64) (int32, bool, error) { // How much capacity is gained by adding one more replica to the fleet. - replicaCapacity := float64(list.Capacity - int64(len(list.Values))) + replicaCapacity := capacity - count + if replicaCapacity == 0 { + return 0, false, errors.Errorf("cannot scale up as adding additional replicas does not increase Capacity") + } - // Desired replicas based on BufferSize specified as an absolute value (i.e. 5) - if l.BufferSize.Type == intstr.Int { - // If our current available is the same as our buffer, then we already have the desired replicas - buffer := float64(l.BufferSize.IntValue()) - if availableCapacity == buffer { - replicas = float64(f.Status.Replicas) - } else { - diffReplicas := math.Floor((availableCapacity - buffer) / replicaCapacity) - replicas = float64(f.Status.Replicas) - diffReplicas + additionalReplicas := int64(math.Ceil((float64(buffer) - float64(availableCapacity)) / float64(replicaCapacity))) + + // Check if we've hit MaxCapacity + if (additionalReplicas*capacity)+aggCapacity > maxCapacity { + // TODO: This should do Floor division automatically, or should I switch the types to float64 then do Math.Floor to make it explicit? + additionalReplicas = (maxCapacity - aggCapacity) / capacity + return replicas + int32(additionalReplicas), true, nil + } + + return replicas + int32(additionalReplicas), false, nil +} + +func scaleDownByInteger(f *agonesv1.Fleet, gameServerLister listeragonesv1.GameServerLister, + nodeCounts map[string]gameservers.NodeCount, key string, isCounter bool, replicas int32, + availableCapacity, aggCount, aggCapacity, minCapacity, buffer int64) (int32, bool, error) { + + if aggCapacity == minCapacity { + return replicas, true, nil + } + + if availableCapacity == buffer { + return replicas, false, nil + } + + gameServers, err := getSortedGameServers(f, gameServerLister, nodeCounts) + if err != nil { + return 0, false, err + } + + for _, gs := range gameServers { + replicas-- + switch isCounter { + case true: + if counter, ok := gs.Status.Counters[key]; ok { + aggCount -= counter.Count + aggCapacity -= counter.Capacity + } + case false: + if list, ok := gs.Status.Lists[key]; ok { + aggCount -= int64(len(list.Values)) + aggCapacity -= list.Capacity + } } - } else { - // Desired replicas based on BufferSize specified as a percent (i.e. 5%) - bufferPercent, err := intstr.GetValueFromIntOrPercent(&l.BufferSize, 100, true) - if err != nil { - return 0, false, err + availableCapacity = aggCapacity - aggCount + // Check if we've hit MinCapacity + if aggCapacity == minCapacity { + return replicas, true, nil } - // The desired total capacity across the fleet (see applyBufferPolicy for explanation) - desiredCapacity := float64(aggList.Count*100) / float64(100-bufferPercent) - if math.Ceil(desiredCapacity) == float64(aggList.Capacity) { - replicas = float64(f.Status.Replicas) - } else { - // TODO: Scale down & up roesn't work in all cases -- see applyCounterPolicy and TestApplyListPolicy - replicas = math.Ceil(desiredCapacity / float64(list.Capacity)) + if aggCapacity < minCapacity { + return replicas + 1, true, nil } + // Check if we're at our desired Buffer + if availableCapacity == buffer { + return replicas, false, nil + } + if availableCapacity < buffer { + return replicas + 1, false, nil + } + } + + if replicas < 0 { // This shouldn't ever happen, but putting it here just in case. + replicas = 0 + } + + return replicas, false, nil +} + +// scaleUpByPercent +func scaleUpByPercent(currReplicas int32, count, aggCount, capacity, aggCapacity, + maxCapacity int64, desiredCapacity, bufferPercent float64) (int32, bool, error) { + + if capacity == 0 { + return 0, false, errors.Errorf("cannot scale up as Capacity is equal to 0") } - // limited indicates that the calculated scale would be above or below the range defined by - // MinCapacity and MaxCapacity + additionalReplicas := int64(math.Ceil((desiredCapacity - float64(aggCapacity)) / (float64(capacity) - float64(count)))) + replicas := currReplicas + int32(additionalReplicas) + // Case where adding a List or Counter does not change the Count (and thus does not change our desiredCapacity) + if count == 0 { + return replicas, false, nil + } + + // In case where len(list.Values) or counter.Count != 0 then we need to update desiredCapacity + // each time we add a List or Counter. Start at point where desired replicas was determined based + // on case where len(list.Values) or counter.Count == 0. + aggCount += count * additionalReplicas + aggCapacity += capacity * additionalReplicas limited := false + for { + // Check if we've reached MaxCapacity + if aggCapacity == maxCapacity { + limited = true + break + } + if aggCapacity > maxCapacity { + limited = true + replicas-- + break + } + // Check if we've reached desiredCapacity + desiredCapacity = (float64(aggCount) * 100) / (100 - bufferPercent) + desiredReplicas := math.Ceil(desiredCapacity / float64(capacity)) + if replicas >= int32(desiredReplicas) { + break + } + // Keep checking if adding one List or Counter will reach our desiredCapacity + aggCount += count + aggCapacity += capacity + replicas++ + } - if replicas < math.Ceil(float64(l.MinCapacity)/float64(list.Capacity)) { - replicas = math.Ceil(float64(l.MinCapacity) / float64(list.Capacity)) - limited = true + return replicas, limited, nil +} + +// scaleDownByPercent +// TODO: This death-spirals down to min capacity, so the customer would need to have something +// in place to prevent gameservers from being evicted elsewhere if they have Count / Values on +// them. I'm assuming this is the behavior we intend, and it just needs good documentation? +func scaleDownByPercent(f *agonesv1.Fleet, gameServerLister listeragonesv1.GameServerLister, + nodeCounts map[string]gameservers.NodeCount, key string, isCounter bool, replicas int32, + aggCount, aggCapacity, minCapacity int64, bufferPercent float64) (int32, bool, error) { + // Exit early if we're already at MinCapacity to avoid calling getSortedGameServers if unnecessary + if aggCapacity == minCapacity { + return replicas, true, nil + } + + // We first need to get the individual game servers in order of deletion on scale down, as both + // the Count and Capacity can change, so we do not know from the aggregate counts how much + // removing x game servers will affect the aggregate count and capacity. + gameServers, err := getSortedGameServers(f, gameServerLister, nodeCounts) + if err != nil { + return 0, false, err } - // Note that we may have a greater MaxCapacity than stated in the ListPolicy. For example, if - // the MaxCapacity is 10 and each replica adds 3 to the total capacity we could end up with 4 - // replicas for a total capacity of 12. - if replicas > math.Ceil(float64(l.MaxCapacity)/float64(list.Capacity)) { - replicas = math.Ceil(float64(l.MaxCapacity) / float64(list.Capacity)) - limited = true + + // Determine the desiredCapacity based on removing one gameserver + limited := false + var desiredCapacity int64 + for _, gs := range gameServers { + // Check if we've reached desiredCapacity + desiredCapacity = int64(math.Ceil((float64(aggCount) * 100) / (100 - bufferPercent))) + if desiredCapacity >= aggCapacity { + break + } + // Keep checking if adding removing one Counter or List will reach our desiredCapacity + switch isCounter { + case true: + if counter, ok := gs.Status.Counters[key]; ok { + aggCount -= counter.Count + aggCapacity -= counter.Capacity + } + case false: + if list, ok := gs.Status.Lists[key]; ok { + aggCount -= int64(len(list.Values)) + aggCapacity -= list.Capacity + } + } + replicas-- + // Check if we've reached MinCapacity + if aggCapacity == minCapacity { + limited = true // TODO: Should we have this return true or false when scaling down to 0 (MinCapacity == 0)? + break + } + if aggCapacity < minCapacity { + limited = true + replicas++ + break + } } - return int32(replicas), limited, nil + if replicas < 0 { + replicas = 0 + } + + return replicas, limited, nil } diff --git a/pkg/fleetautoscalers/fleetautoscalers_test.go b/pkg/fleetautoscalers/fleetautoscalers_test.go index f87177b1f9..953af1f907 100644 --- a/pkg/fleetautoscalers/fleetautoscalers_test.go +++ b/pkg/fleetautoscalers/fleetautoscalers_test.go @@ -26,10 +26,15 @@ import ( agonesv1 "agones.dev/agones/pkg/apis/agones/v1" autoscalingv1 "agones.dev/agones/pkg/apis/autoscaling/v1" - "agones.dev/agones/pkg/util/runtime" + "agones.dev/agones/pkg/gameservers" + agtesting "agones.dev/agones/pkg/testing" + utilruntime "agones.dev/agones/pkg/util/runtime" "github.com/stretchr/testify/assert" admregv1 "k8s.io/api/admissionregistration/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/intstr" + k8stesting "k8s.io/client-go/testing" ) const ( @@ -105,6 +110,8 @@ func (t testServer) ServeHTTP(w http.ResponseWriter, r *http.Request) { func TestComputeDesiredFleetSize(t *testing.T) { t.Parallel() + nc := map[string]gameservers.NodeCount{} + fas, f := defaultFixtures() type expected struct { @@ -157,7 +164,7 @@ func TestComputeDesiredFleetSize(t *testing.T) { expected: expected{ replicas: 0, limited: false, - err: "wrong policy type, should be one of: Buffer, Webhook", + err: "wrong policy type, should be one of: Buffer, Webhook, Counter, List", }, }, } @@ -170,7 +177,16 @@ func TestComputeDesiredFleetSize(t *testing.T) { f.Status.AllocatedReplicas = tc.statusAllocatedReplicas f.Status.ReadyReplicas = tc.statusReadyReplicas - replicas, limited, err := computeDesiredFleetSize(fas, f) + m := agtesting.NewMocks() + m.AgonesClient.AddReactor("list", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) { + return true, &agonesv1.GameServerList{Items: []agonesv1.GameServer{}}, nil + }) + + gameServers := m.AgonesInformerFactory.Agones().V1().GameServers() + _, cancel := agtesting.StartInformers(m, gameServers.Informer().HasSynced) + defer cancel() + + replicas, limited, err := computeDesiredFleetSize(fas, f, gameServers.Lister(), nc) if tc.expected.err != "" && assert.NotNil(t, err) { assert.Equal(t, tc.expected.err, err.Error()) @@ -698,12 +714,17 @@ func TestBuildURLFromWebhookPolicyNoNamespace(t *testing.T) { } } +// nolint:dupl // Linter errors on lines are duplicate of TestApplyListPolicy func TestApplyCounterPolicy(t *testing.T) { t.Parallel() - modifiedFleet := func(f func(*agonesv1.GameServerTemplateSpec, *agonesv1.FleetStatus)) *agonesv1.Fleet { - _, fleet := defaultFixtures() - f(&fleet.Spec.Template, &fleet.Status) + nc := map[string]gameservers.NodeCount{ + "n1": {Ready: 1, Allocated: 1}, + } + + modifiedFleet := func(f func(*agonesv1.Fleet)) *agonesv1.Fleet { + _, fleet := defaultFixtures() // The ObjectMeta.Name of the defaultFixtures fleet is "fleet-1" + f(fleet) return fleet } @@ -717,24 +738,25 @@ func TestApplyCounterPolicy(t *testing.T) { fleet *agonesv1.Fleet featureFlags string cp *autoscalingv1.CounterPolicy + gsList []agonesv1.GameServer want expected }{ "counts and lists not enabled": { - fleet: modifiedFleet(func(spec *agonesv1.GameServerTemplateSpec, status *agonesv1.FleetStatus) { - spec.Spec.Counters = make(map[string]agonesv1.CounterStatus) - spec.Spec.Counters["rooms"] = agonesv1.CounterStatus{ + fleet: modifiedFleet(func(f *agonesv1.Fleet) { + f.Spec.Template.Spec.Counters = make(map[string]agonesv1.CounterStatus) + f.Spec.Template.Spec.Counters["rooms"] = agonesv1.CounterStatus{ Count: 0, Capacity: 7} - status.Replicas = 10 // This should always be status.Counters.Capacity / spec.Spec.Counters.Capacity - status.ReadyReplicas = 5 - status.AllocatedReplicas = 5 // This should be at least status.Counters.Count / spec.Spec.Counters.Capacity - status.Counters = make(map[string]agonesv1.AggregatedCounterStatus) - status.Counters["rooms"] = agonesv1.AggregatedCounterStatus{ + f.Status.Replicas = 10 // This should always be status.Counters.Capacity / spec.Spec.Counters.Capacity + f.Status.ReadyReplicas = 5 + f.Status.AllocatedReplicas = 5 // This should be at least status.Counters.Count / spec.Spec.Counters.Capacity + f.Status.Counters = make(map[string]agonesv1.AggregatedCounterStatus) + f.Status.Counters["rooms"] = agonesv1.AggregatedCounterStatus{ Count: 31, Capacity: 70, } }), - featureFlags: string(runtime.FeatureCountsAndLists) + "=false", + featureFlags: string(utilruntime.FeatureCountsAndLists) + "=false", cp: &autoscalingv1.CounterPolicy{ Key: "rooms", MaxCapacity: 100, @@ -748,21 +770,21 @@ func TestApplyCounterPolicy(t *testing.T) { }, }, "fleet spec does not have counter": { - fleet: modifiedFleet(func(spec *agonesv1.GameServerTemplateSpec, status *agonesv1.FleetStatus) { - spec.Spec.Counters = make(map[string]agonesv1.CounterStatus) - spec.Spec.Counters["brooms"] = agonesv1.CounterStatus{ + fleet: modifiedFleet(func(f *agonesv1.Fleet) { + f.Spec.Template.Spec.Counters = make(map[string]agonesv1.CounterStatus) + f.Spec.Template.Spec.Counters["brooms"] = agonesv1.CounterStatus{ Count: 0, Capacity: 7} - status.Replicas = 10 - status.ReadyReplicas = 5 - status.AllocatedReplicas = 5 - status.Counters = make(map[string]agonesv1.AggregatedCounterStatus) - status.Counters["rooms"] = agonesv1.AggregatedCounterStatus{ + f.Status.Replicas = 10 + f.Status.ReadyReplicas = 5 + f.Status.AllocatedReplicas = 5 + f.Status.Counters = make(map[string]agonesv1.AggregatedCounterStatus) + f.Status.Counters["rooms"] = agonesv1.AggregatedCounterStatus{ Count: 31, Capacity: 70, } }), - featureFlags: string(runtime.FeatureCountsAndLists) + "=true", + featureFlags: string(utilruntime.FeatureCountsAndLists) + "=true", cp: &autoscalingv1.CounterPolicy{ Key: "rooms", MaxCapacity: 100, @@ -776,21 +798,21 @@ func TestApplyCounterPolicy(t *testing.T) { }, }, "fleet status does not have counter": { - fleet: modifiedFleet(func(spec *agonesv1.GameServerTemplateSpec, status *agonesv1.FleetStatus) { - spec.Spec.Counters = make(map[string]agonesv1.CounterStatus) - spec.Spec.Counters["rooms"] = agonesv1.CounterStatus{ + fleet: modifiedFleet(func(f *agonesv1.Fleet) { + f.Spec.Template.Spec.Counters = make(map[string]agonesv1.CounterStatus) + f.Spec.Template.Spec.Counters["rooms"] = agonesv1.CounterStatus{ Count: 0, Capacity: 7} - status.Replicas = 10 - status.ReadyReplicas = 5 - status.AllocatedReplicas = 5 - status.Counters = make(map[string]agonesv1.AggregatedCounterStatus) - status.Counters["brooms"] = agonesv1.AggregatedCounterStatus{ + f.Status.Replicas = 10 + f.Status.ReadyReplicas = 5 + f.Status.AllocatedReplicas = 5 + f.Status.Counters = make(map[string]agonesv1.AggregatedCounterStatus) + f.Status.Counters["brooms"] = agonesv1.AggregatedCounterStatus{ Count: 31, Capacity: 70, } }), - featureFlags: string(runtime.FeatureCountsAndLists) + "=true", + featureFlags: string(utilruntime.FeatureCountsAndLists) + "=true", cp: &autoscalingv1.CounterPolicy{ Key: "rooms", MaxCapacity: 100, @@ -804,48 +826,137 @@ func TestApplyCounterPolicy(t *testing.T) { }, }, "scale down": { - fleet: modifiedFleet(func(spec *agonesv1.GameServerTemplateSpec, status *agonesv1.FleetStatus) { - spec.Spec.Counters = make(map[string]agonesv1.CounterStatus) - spec.Spec.Counters["rooms"] = agonesv1.CounterStatus{ + fleet: modifiedFleet(func(f *agonesv1.Fleet) { + f.Spec.Template.Spec.Counters = make(map[string]agonesv1.CounterStatus) + f.Spec.Template.Spec.Counters["rooms"] = agonesv1.CounterStatus{ Count: 0, Capacity: 7} - status.Replicas = 10 - status.ReadyReplicas = 5 - status.AllocatedReplicas = 5 - status.Counters = make(map[string]agonesv1.AggregatedCounterStatus) - status.Counters["rooms"] = agonesv1.AggregatedCounterStatus{ + f.Spec.Priorities = []agonesv1.Priority{{PriorityType: "Counter", Key: "rooms", Order: "Ascending"}} + f.Status.Replicas = 8 + f.Status.ReadyReplicas = 4 + f.Status.AllocatedReplicas = 4 + f.Status.Counters = make(map[string]agonesv1.AggregatedCounterStatus) + f.Status.Counters["rooms"] = agonesv1.AggregatedCounterStatus{ Count: 31, Capacity: 70, } }), - featureFlags: string(runtime.FeatureCountsAndLists) + "=true", + featureFlags: string(utilruntime.FeatureCountsAndLists) + "=true", cp: &autoscalingv1.CounterPolicy{ Key: "rooms", MaxCapacity: 100, MinCapacity: 10, BufferSize: intstr.FromInt(10), }, + gsList: []agonesv1.GameServer{ + {ObjectMeta: metav1.ObjectMeta{ + Name: "gs1", + // We need the Label here so that the Lister can pick up that the gameserver is a part of + // the fleet. If this was a real gameserver it would also have a label for + // "agones.dev/gameserverset": "gameServerSetName". + Labels: map[string]string{"agones.dev/fleet": "fleet-1"}}, + Status: agonesv1.GameServerStatus{ + // We need NodeName here for sorting, otherwise sortGameServersByLeastFullNodes + // will return the list of GameServers in the opposite order the were return by + // ListGameServersByGameServerSetOwner (which is a nondeterministic order). + NodeName: "n1", + Counters: map[string]agonesv1.CounterStatus{ + "rooms": { + Count: 10, + Capacity: 10, + }}}}, + {ObjectMeta: metav1.ObjectMeta{ + Name: "gs2", + Labels: map[string]string{"agones.dev/fleet": "fleet-1"}}, + Status: agonesv1.GameServerStatus{ + NodeName: "n1", + Counters: map[string]agonesv1.CounterStatus{ + "rooms": { + Count: 3, + Capacity: 5, + }}}}, + {ObjectMeta: metav1.ObjectMeta{ + Name: "gs3", + Labels: map[string]string{"agones.dev/fleet": "fleet-1"}}, + Status: agonesv1.GameServerStatus{ + NodeName: "n1", + Counters: map[string]agonesv1.CounterStatus{ + "rooms": { + Count: 7, + Capacity: 7, + }}}}, + {ObjectMeta: metav1.ObjectMeta{ + Name: "gs4", + Labels: map[string]string{"agones.dev/fleet": "fleet-1"}}, + Status: agonesv1.GameServerStatus{ + NodeName: "n1", + Counters: map[string]agonesv1.CounterStatus{ + "rooms": { + Count: 11, + Capacity: 14, + }}}}, + {ObjectMeta: metav1.ObjectMeta{ + Name: "gs5", + Labels: map[string]string{"agones.dev/fleet": "fleet-1"}}, + Status: agonesv1.GameServerStatus{ + NodeName: "n1", + Counters: map[string]agonesv1.CounterStatus{ + "rooms": { + Count: 0, + Capacity: 13, + }}}}, + {ObjectMeta: metav1.ObjectMeta{ + Name: "gs6", + Labels: map[string]string{"agones.dev/fleet": "fleet-1"}}, + Status: agonesv1.GameServerStatus{ + NodeName: "n1", + Counters: map[string]agonesv1.CounterStatus{ + "rooms": { + Count: 0, + Capacity: 7, + }}}}, + {ObjectMeta: metav1.ObjectMeta{ + Name: "gs7", + Labels: map[string]string{"agones.dev/fleet": "fleet-1"}}, + Status: agonesv1.GameServerStatus{ + NodeName: "n1", + Counters: map[string]agonesv1.CounterStatus{ + "rooms": { + Count: 0, + Capacity: 7, + }}}}, + {ObjectMeta: metav1.ObjectMeta{ + Name: "gs8", + Labels: map[string]string{"agones.dev/fleet": "fleet-1"}}, + Status: agonesv1.GameServerStatus{ + NodeName: "n1", + Counters: map[string]agonesv1.CounterStatus{ + "rooms": { + Count: 0, + Capacity: 7, + }}}}, + }, want: expected{ - replicas: 6, + replicas: 2, limited: false, wantErr: false, }, }, "scale up": { - fleet: modifiedFleet(func(spec *agonesv1.GameServerTemplateSpec, status *agonesv1.FleetStatus) { - spec.Spec.Counters = make(map[string]agonesv1.CounterStatus) - spec.Spec.Counters["rooms"] = agonesv1.CounterStatus{ + fleet: modifiedFleet(func(f *agonesv1.Fleet) { + f.Spec.Template.Spec.Counters = make(map[string]agonesv1.CounterStatus) + f.Spec.Template.Spec.Counters["rooms"] = agonesv1.CounterStatus{ Count: 0, Capacity: 7} - status.Replicas = 10 - status.ReadyReplicas = 0 - status.AllocatedReplicas = 10 - status.Counters = make(map[string]agonesv1.AggregatedCounterStatus) - status.Counters["rooms"] = agonesv1.AggregatedCounterStatus{ + f.Status.Replicas = 10 + f.Status.ReadyReplicas = 0 + f.Status.AllocatedReplicas = 10 + f.Status.Counters = make(map[string]agonesv1.AggregatedCounterStatus) + f.Status.Counters["rooms"] = agonesv1.AggregatedCounterStatus{ Count: 68, Capacity: 70} }), - featureFlags: string(runtime.FeatureCountsAndLists) + "=true", + featureFlags: string(utilruntime.FeatureCountsAndLists) + "=true", cp: &autoscalingv1.CounterPolicy{ Key: "rooms", MaxCapacity: 100, @@ -859,20 +970,20 @@ func TestApplyCounterPolicy(t *testing.T) { }, }, "scale same": { - fleet: modifiedFleet(func(spec *agonesv1.GameServerTemplateSpec, status *agonesv1.FleetStatus) { - spec.Spec.Counters = make(map[string]agonesv1.CounterStatus) - spec.Spec.Counters["rooms"] = agonesv1.CounterStatus{ + fleet: modifiedFleet(func(f *agonesv1.Fleet) { + f.Spec.Template.Spec.Counters = make(map[string]agonesv1.CounterStatus) + f.Spec.Template.Spec.Counters["rooms"] = agonesv1.CounterStatus{ Count: 0, Capacity: 7} - status.Replicas = 10 - status.ReadyReplicas = 0 - status.AllocatedReplicas = 10 - status.Counters = make(map[string]agonesv1.AggregatedCounterStatus) - status.Counters["rooms"] = agonesv1.AggregatedCounterStatus{ + f.Status.Replicas = 10 + f.Status.ReadyReplicas = 0 + f.Status.AllocatedReplicas = 10 + f.Status.Counters = make(map[string]agonesv1.AggregatedCounterStatus) + f.Status.Counters["rooms"] = agonesv1.AggregatedCounterStatus{ Count: 60, Capacity: 70} }), - featureFlags: string(runtime.FeatureCountsAndLists) + "=true", + featureFlags: string(utilruntime.FeatureCountsAndLists) + "=true", cp: &autoscalingv1.CounterPolicy{ Key: "rooms", MaxCapacity: 100, @@ -885,21 +996,117 @@ func TestApplyCounterPolicy(t *testing.T) { wantErr: false, }, }, - "scale down limited": { - fleet: modifiedFleet(func(spec *agonesv1.GameServerTemplateSpec, status *agonesv1.FleetStatus) { - spec.Spec.Counters = make(map[string]agonesv1.CounterStatus) - spec.Spec.Counters["rooms"] = agonesv1.CounterStatus{ + "scale down at MinCapacity": { + fleet: modifiedFleet(func(f *agonesv1.Fleet) { + f.Spec.Template.Spec.Counters = make(map[string]agonesv1.CounterStatus) + f.Spec.Template.Spec.Counters["rooms"] = agonesv1.CounterStatus{ Count: 0, Capacity: 7} - status.Replicas = 10 - status.ReadyReplicas = 9 - status.AllocatedReplicas = 1 - status.Counters = make(map[string]agonesv1.AggregatedCounterStatus) - status.Counters["rooms"] = agonesv1.AggregatedCounterStatus{ + f.Status.Replicas = 10 + f.Status.ReadyReplicas = 9 + f.Status.AllocatedReplicas = 1 + f.Status.Counters = make(map[string]agonesv1.AggregatedCounterStatus) + f.Status.Counters["rooms"] = agonesv1.AggregatedCounterStatus{ Count: 1, Capacity: 70} }), - featureFlags: string(runtime.FeatureCountsAndLists) + "=true", + featureFlags: string(utilruntime.FeatureCountsAndLists) + "=true", + cp: &autoscalingv1.CounterPolicy{ + Key: "rooms", + MaxCapacity: 700, + MinCapacity: 70, + BufferSize: intstr.FromInt(10), + }, + want: expected{ + replicas: 10, + limited: true, + wantErr: false, + }, + }, + "scale down limited": { + fleet: modifiedFleet(func(f *agonesv1.Fleet) { + f.Spec.Template.Spec.Counters = make(map[string]agonesv1.CounterStatus) + f.Spec.Template.Spec.Counters["rooms"] = agonesv1.CounterStatus{ + Count: 0, + Capacity: 7} + f.Spec.Priorities = []agonesv1.Priority{{PriorityType: "Counter", Key: "rooms", Order: "Descending"}} + f.Status.Replicas = 4 + f.Status.ReadyReplicas = 3 + f.Status.AllocatedReplicas = 1 + f.Status.Counters = make(map[string]agonesv1.AggregatedCounterStatus) + f.Status.Counters["rooms"] = agonesv1.AggregatedCounterStatus{ + Count: 1, + Capacity: 36} + }), + featureFlags: string(utilruntime.FeatureCountsAndLists) + "=true", + cp: &autoscalingv1.CounterPolicy{ + Key: "rooms", + MaxCapacity: 700, + MinCapacity: 7, + BufferSize: intstr.FromInt(1), + }, + gsList: []agonesv1.GameServer{ + {ObjectMeta: metav1.ObjectMeta{ + Name: "gs1", + Labels: map[string]string{"agones.dev/fleet": "fleet-1"}}, + Status: agonesv1.GameServerStatus{ + NodeName: "n1", + Counters: map[string]agonesv1.CounterStatus{ + "rooms": { + Count: 0, + Capacity: 10, + }}}}, + {ObjectMeta: metav1.ObjectMeta{ + Name: "gs2", + Labels: map[string]string{"agones.dev/fleet": "fleet-1"}}, + Status: agonesv1.GameServerStatus{ + NodeName: "n1", + Counters: map[string]agonesv1.CounterStatus{ + "rooms": { + Count: 1, + Capacity: 5, + }}}}, + {ObjectMeta: metav1.ObjectMeta{ + Name: "gs3", + Labels: map[string]string{"agones.dev/fleet": "fleet-1"}}, + Status: agonesv1.GameServerStatus{ + NodeName: "n1", + Counters: map[string]agonesv1.CounterStatus{ + "rooms": { + Count: 0, + Capacity: 7, + }}}}, + {ObjectMeta: metav1.ObjectMeta{ + Name: "gs4", + Labels: map[string]string{"agones.dev/fleet": "fleet-1"}}, + Status: agonesv1.GameServerStatus{ + NodeName: "n1", + Counters: map[string]agonesv1.CounterStatus{ + "rooms": { + Count: 0, + Capacity: 14, + }}}}}, + want: expected{ + replicas: 2, + limited: true, + wantErr: false, + }, + }, + "scale down limited must scale up": { + fleet: modifiedFleet(func(f *agonesv1.Fleet) { + f.Spec.Template.Spec.Counters = make(map[string]agonesv1.CounterStatus) + f.Spec.Template.Spec.Counters["rooms"] = agonesv1.CounterStatus{ + Count: 0, + Capacity: 7} + f.Status.Replicas = 7 + f.Status.ReadyReplicas = 6 + f.Status.AllocatedReplicas = 1 + f.Status.Counters = make(map[string]agonesv1.AggregatedCounterStatus) + f.Status.Counters["rooms"] = agonesv1.AggregatedCounterStatus{ + Count: 1, + Capacity: 49} + }), + featureFlags: string(utilruntime.FeatureCountsAndLists) + "=true", cp: &autoscalingv1.CounterPolicy{ Key: "rooms", MaxCapacity: 700, @@ -913,20 +1120,20 @@ func TestApplyCounterPolicy(t *testing.T) { }, }, "scale up limited": { - fleet: modifiedFleet(func(spec *agonesv1.GameServerTemplateSpec, status *agonesv1.FleetStatus) { - spec.Spec.Counters = make(map[string]agonesv1.CounterStatus) - spec.Spec.Counters["rooms"] = agonesv1.CounterStatus{ + fleet: modifiedFleet(func(f *agonesv1.Fleet) { + f.Spec.Template.Spec.Counters = make(map[string]agonesv1.CounterStatus) + f.Spec.Template.Spec.Counters["rooms"] = agonesv1.CounterStatus{ Count: 0, Capacity: 7} - status.Replicas = 14 - status.ReadyReplicas = 0 - status.AllocatedReplicas = 14 - status.Counters = make(map[string]agonesv1.AggregatedCounterStatus) - status.Counters["rooms"] = agonesv1.AggregatedCounterStatus{ + f.Status.Replicas = 14 + f.Status.ReadyReplicas = 0 + f.Status.AllocatedReplicas = 14 + f.Status.Counters = make(map[string]agonesv1.AggregatedCounterStatus) + f.Status.Counters["rooms"] = agonesv1.AggregatedCounterStatus{ Count: 98, Capacity: 98} }), - featureFlags: string(runtime.FeatureCountsAndLists) + "=true", + featureFlags: string(utilruntime.FeatureCountsAndLists) + "=true", cp: &autoscalingv1.CounterPolicy{ Key: "rooms", MaxCapacity: 100, @@ -934,55 +1141,95 @@ func TestApplyCounterPolicy(t *testing.T) { BufferSize: intstr.FromInt(10), }, want: expected{ - replicas: 15, + replicas: 14, limited: true, wantErr: false, }, }, - "scale down by percent": { - fleet: modifiedFleet(func(spec *agonesv1.GameServerTemplateSpec, status *agonesv1.FleetStatus) { - spec.Spec.Counters = make(map[string]agonesv1.CounterStatus) - spec.Spec.Counters["rooms"] = agonesv1.CounterStatus{ + "scale up limited must scale down": { + fleet: modifiedFleet(func(f *agonesv1.Fleet) { + f.Spec.Template.Spec.Counters = make(map[string]agonesv1.CounterStatus) + f.Spec.Template.Spec.Counters["rooms"] = agonesv1.CounterStatus{ Count: 0, Capacity: 7} - status.Replicas = 10 - status.ReadyReplicas = 5 - status.AllocatedReplicas = 5 - status.Counters = make(map[string]agonesv1.AggregatedCounterStatus) - status.Counters["rooms"] = agonesv1.AggregatedCounterStatus{ - Count: 31, - Capacity: 70, + f.Spec.Priorities = []agonesv1.Priority{{PriorityType: "Counter", Key: "rooms", Order: "Descending"}} + f.Status.Replicas = 1 + f.Status.ReadyReplicas = 0 + f.Status.AllocatedReplicas = 1 + f.Status.Counters = make(map[string]agonesv1.AggregatedCounterStatus) + f.Status.Counters["rooms"] = agonesv1.AggregatedCounterStatus{ + Count: 7, + Capacity: 7} + }), + featureFlags: string(utilruntime.FeatureCountsAndLists) + "=true", + cp: &autoscalingv1.CounterPolicy{ + Key: "rooms", + MaxCapacity: 2, + MinCapacity: 0, + BufferSize: intstr.FromInt(1), + }, + gsList: []agonesv1.GameServer{ + {ObjectMeta: metav1.ObjectMeta{ + Name: "gs1", + Labels: map[string]string{"agones.dev/fleet": "fleet-1"}}, + Status: agonesv1.GameServerStatus{ + NodeName: "n1", + Counters: map[string]agonesv1.CounterStatus{ + "rooms": { + Count: 7, + Capacity: 7, + }}}}}, + want: expected{ + replicas: 0, + limited: true, + wantErr: false, + }, + }, + "scale up to MinCapacity": { + fleet: modifiedFleet(func(f *agonesv1.Fleet) { + f.Spec.Template.Spec.Counters = make(map[string]agonesv1.CounterStatus) + f.Spec.Template.Spec.Counters["rooms"] = agonesv1.CounterStatus{ + Count: 0, + Capacity: 10} + f.Spec.Priorities = []agonesv1.Priority{{PriorityType: "Counter", Key: "rooms", Order: "Descending"}} + f.Status.Replicas = 3 + f.Status.ReadyReplicas = 0 + f.Status.AllocatedReplicas = 3 + f.Status.Counters = make(map[string]agonesv1.AggregatedCounterStatus) + f.Status.Counters["rooms"] = agonesv1.AggregatedCounterStatus{ + Count: 20, + Capacity: 30, } }), - featureFlags: string(runtime.FeatureCountsAndLists) + "=true", + featureFlags: string(utilruntime.FeatureCountsAndLists) + "=true", cp: &autoscalingv1.CounterPolicy{ Key: "rooms", MaxCapacity: 100, - MinCapacity: 10, + MinCapacity: 50, BufferSize: intstr.FromString("10%"), }, want: expected{ replicas: 5, - limited: false, + limited: true, wantErr: false, }, }, "scale up by percent": { - fleet: modifiedFleet(func(spec *agonesv1.GameServerTemplateSpec, status *agonesv1.FleetStatus) { - spec.Spec.Counters = make(map[string]agonesv1.CounterStatus) - spec.Spec.Counters["players"] = agonesv1.CounterStatus{ + fleet: modifiedFleet(func(f *agonesv1.Fleet) { + f.Spec.Template.Spec.Counters = make(map[string]agonesv1.CounterStatus) + f.Spec.Template.Spec.Counters["players"] = agonesv1.CounterStatus{ Count: 0, Capacity: 1} - status.Replicas = 10 - status.ReadyReplicas = 2 - status.AllocatedReplicas = 8 - status.Counters = make(map[string]agonesv1.AggregatedCounterStatus) - status.Counters["players"] = agonesv1.AggregatedCounterStatus{ + f.Status.Replicas = 10 + f.Status.ReadyReplicas = 2 + f.Status.AllocatedReplicas = 8 + f.Status.Counters = make(map[string]agonesv1.AggregatedCounterStatus) + f.Status.Counters["players"] = agonesv1.AggregatedCounterStatus{ Count: 8, Capacity: 10, } }), - featureFlags: string(runtime.FeatureCountsAndLists) + "=true", + featureFlags: string(utilruntime.FeatureCountsAndLists) + "=true", cp: &autoscalingv1.CounterPolicy{ Key: "players", MaxCapacity: 100, @@ -995,17 +1242,55 @@ func TestApplyCounterPolicy(t *testing.T) { wantErr: false, }, }, + "scale up by percent with Count": { + fleet: modifiedFleet(func(f *agonesv1.Fleet) { + f.Spec.Template.Spec.Counters = make(map[string]agonesv1.CounterStatus) + f.Spec.Template.Spec.Counters["players"] = agonesv1.CounterStatus{ + Count: 3, + Capacity: 10} + f.Status.Replicas = 3 + f.Status.ReadyReplicas = 0 + f.Status.AllocatedReplicas = 3 + f.Status.Counters = make(map[string]agonesv1.AggregatedCounterStatus) + f.Status.Counters["players"] = agonesv1.AggregatedCounterStatus{ + Count: 20, + Capacity: 30, + } + }), + featureFlags: string(utilruntime.FeatureCountsAndLists) + "=true", + cp: &autoscalingv1.CounterPolicy{ + Key: "players", + MaxCapacity: 100, + MinCapacity: 10, + BufferSize: intstr.FromString("50%"), + }, + want: expected{ + replicas: 6, + limited: false, + wantErr: false, + }, + }, } - runtime.FeatureTestMutex.Lock() - defer runtime.FeatureTestMutex.Unlock() + utilruntime.FeatureTestMutex.Lock() + defer utilruntime.FeatureTestMutex.Unlock() for name, tc := range testCases { t.Run(name, func(t *testing.T) { - err := runtime.ParseFeatures(tc.featureFlags) + err := utilruntime.ParseFeatures(tc.featureFlags) assert.NoError(t, err) - replicas, limited, err := applyCounterPolicy(tc.cp, tc.fleet) + m := agtesting.NewMocks() + m.AgonesClient.AddReactor("list", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) { + return true, &agonesv1.GameServerList{Items: tc.gsList}, nil + }) + + informer := m.AgonesInformerFactory.Agones().V1() + _, cancel := agtesting.StartInformers(m, + informer.GameServers().Informer().HasSynced) + defer cancel() + + replicas, limited, err := applyCounterOrListPolicy(tc.cp, nil, tc.fleet, informer.GameServers().Lister(), nc) if tc.want.wantErr { assert.NotNil(t, err) @@ -1018,12 +1303,17 @@ func TestApplyCounterPolicy(t *testing.T) { } } +// nolint:dupl // Linter errors on lines are duplicate of TestApplyCounterPolicy func TestApplyListPolicy(t *testing.T) { t.Parallel() - modifiedFleet := func(f func(*agonesv1.GameServerTemplateSpec, *agonesv1.FleetStatus)) *agonesv1.Fleet { + nc := map[string]gameservers.NodeCount{ + "n1": {Ready: 0, Allocated: 2}, + } + + modifiedFleet := func(f func(*agonesv1.Fleet)) *agonesv1.Fleet { _, fleet := defaultFixtures() - f(&fleet.Spec.Template, &fleet.Status) + f(fleet) return fleet } @@ -1037,24 +1327,25 @@ func TestApplyListPolicy(t *testing.T) { fleet *agonesv1.Fleet featureFlags string lp *autoscalingv1.ListPolicy + gsList []agonesv1.GameServer want expected }{ "counts and lists not enabled": { - fleet: modifiedFleet(func(spec *agonesv1.GameServerTemplateSpec, status *agonesv1.FleetStatus) { - spec.Spec.Lists = make(map[string]agonesv1.ListStatus) - spec.Spec.Lists["gamers"] = agonesv1.ListStatus{ + fleet: modifiedFleet(func(f *agonesv1.Fleet) { + f.Spec.Template.Spec.Lists = make(map[string]agonesv1.ListStatus) + f.Spec.Template.Spec.Lists["gamers"] = agonesv1.ListStatus{ Values: []string{}, Capacity: 7} - status.Replicas = 10 // This should always be status.Lists.Capacity / spec.Spec.Lists.Capacity - status.ReadyReplicas = 5 - status.AllocatedReplicas = 5 // This should be at least status.Lists.Count / spec.Spec.Lists.Capacity - status.Lists = make(map[string]agonesv1.AggregatedListStatus) - status.Lists["gamers"] = agonesv1.AggregatedListStatus{ + f.Status.Replicas = 10 + f.Status.ReadyReplicas = 5 + f.Status.AllocatedReplicas = 5 + f.Status.Lists = make(map[string]agonesv1.AggregatedListStatus) + f.Status.Lists["gamers"] = agonesv1.AggregatedListStatus{ Count: 31, Capacity: 70, } }), - featureFlags: string(runtime.FeatureCountsAndLists) + "=false", + featureFlags: string(utilruntime.FeatureCountsAndLists) + "=false", lp: &autoscalingv1.ListPolicy{ Key: "gamers", MaxCapacity: 100, @@ -1068,21 +1359,21 @@ func TestApplyListPolicy(t *testing.T) { }, }, "fleet spec does not have list": { - fleet: modifiedFleet(func(spec *agonesv1.GameServerTemplateSpec, status *agonesv1.FleetStatus) { - spec.Spec.Lists = make(map[string]agonesv1.ListStatus) - spec.Spec.Lists["tamers"] = agonesv1.ListStatus{ + fleet: modifiedFleet(func(f *agonesv1.Fleet) { + f.Spec.Template.Spec.Lists = make(map[string]agonesv1.ListStatus) + f.Spec.Template.Spec.Lists["tamers"] = agonesv1.ListStatus{ Values: []string{}, Capacity: 7} - status.Replicas = 10 - status.ReadyReplicas = 5 - status.AllocatedReplicas = 5 - status.Lists = make(map[string]agonesv1.AggregatedListStatus) - status.Lists["gamers"] = agonesv1.AggregatedListStatus{ + f.Status.Replicas = 10 + f.Status.ReadyReplicas = 5 + f.Status.AllocatedReplicas = 5 + f.Status.Lists = make(map[string]agonesv1.AggregatedListStatus) + f.Status.Lists["gamers"] = agonesv1.AggregatedListStatus{ Count: 31, Capacity: 70, } }), - featureFlags: string(runtime.FeatureCountsAndLists) + "=true", + featureFlags: string(utilruntime.FeatureCountsAndLists) + "=true", lp: &autoscalingv1.ListPolicy{ Key: "gamers", MaxCapacity: 100, @@ -1096,21 +1387,21 @@ func TestApplyListPolicy(t *testing.T) { }, }, "fleet status does not have list": { - fleet: modifiedFleet(func(spec *agonesv1.GameServerTemplateSpec, status *agonesv1.FleetStatus) { - spec.Spec.Lists = make(map[string]agonesv1.ListStatus) - spec.Spec.Lists["gamers"] = agonesv1.ListStatus{ + fleet: modifiedFleet(func(f *agonesv1.Fleet) { + f.Spec.Template.Spec.Lists = make(map[string]agonesv1.ListStatus) + f.Spec.Template.Spec.Lists["gamers"] = agonesv1.ListStatus{ Values: []string{}, Capacity: 7} - status.Replicas = 10 - status.ReadyReplicas = 5 - status.AllocatedReplicas = 5 - status.Lists = make(map[string]agonesv1.AggregatedListStatus) - status.Lists["tamers"] = agonesv1.AggregatedListStatus{ + f.Status.Replicas = 10 + f.Status.ReadyReplicas = 5 + f.Status.AllocatedReplicas = 5 + f.Status.Lists = make(map[string]agonesv1.AggregatedListStatus) + f.Status.Lists["tamers"] = agonesv1.AggregatedListStatus{ Count: 31, Capacity: 70, } }), - featureFlags: string(runtime.FeatureCountsAndLists) + "=true", + featureFlags: string(utilruntime.FeatureCountsAndLists) + "=true", lp: &autoscalingv1.ListPolicy{ Key: "gamers", MaxCapacity: 100, @@ -1124,21 +1415,21 @@ func TestApplyListPolicy(t *testing.T) { }, }, "scale up": { - fleet: modifiedFleet(func(spec *agonesv1.GameServerTemplateSpec, status *agonesv1.FleetStatus) { - spec.Spec.Lists = make(map[string]agonesv1.ListStatus) - spec.Spec.Lists["gamers"] = agonesv1.ListStatus{ + fleet: modifiedFleet(func(f *agonesv1.Fleet) { + f.Spec.Template.Spec.Lists = make(map[string]agonesv1.ListStatus) + f.Spec.Template.Spec.Lists["gamers"] = agonesv1.ListStatus{ Values: []string{"default", "default2"}, Capacity: 3} - status.Replicas = 10 - status.ReadyReplicas = 0 - status.AllocatedReplicas = 10 - status.Lists = make(map[string]agonesv1.AggregatedListStatus) - status.Lists["gamers"] = agonesv1.AggregatedListStatus{ + f.Status.Replicas = 10 + f.Status.ReadyReplicas = 0 + f.Status.AllocatedReplicas = 10 + f.Status.Lists = make(map[string]agonesv1.AggregatedListStatus) + f.Status.Lists["gamers"] = agonesv1.AggregatedListStatus{ Count: 29, Capacity: 30, } }), - featureFlags: string(runtime.FeatureCountsAndLists) + "=true", + featureFlags: string(utilruntime.FeatureCountsAndLists) + "=true", lp: &autoscalingv1.ListPolicy{ Key: "gamers", MaxCapacity: 100, @@ -1152,26 +1443,109 @@ func TestApplyListPolicy(t *testing.T) { }, }, "scale down": { - fleet: modifiedFleet(func(spec *agonesv1.GameServerTemplateSpec, status *agonesv1.FleetStatus) { - spec.Spec.Lists = make(map[string]agonesv1.ListStatus) - spec.Spec.Lists["gamers"] = agonesv1.ListStatus{ + fleet: modifiedFleet(func(f *agonesv1.Fleet) { + f.Spec.Template.Spec.Lists = make(map[string]agonesv1.ListStatus) + f.Spec.Template.Spec.Lists["gamers"] = agonesv1.ListStatus{ Values: []string{"default"}, Capacity: 10} - status.Replicas = 10 - status.ReadyReplicas = 6 - status.AllocatedReplicas = 4 - status.Lists = make(map[string]agonesv1.AggregatedListStatus) - status.Lists["gamers"] = agonesv1.AggregatedListStatus{ - Count: 31, - Capacity: 100, + f.Spec.Priorities = []agonesv1.Priority{{PriorityType: "List", Key: "gamers", Order: "Descending"}} + f.Status.Replicas = 8 + f.Status.ReadyReplicas = 6 + f.Status.AllocatedReplicas = 4 + f.Status.Lists = make(map[string]agonesv1.AggregatedListStatus) + f.Status.Lists["gamers"] = agonesv1.AggregatedListStatus{ + Count: 15, + Capacity: 70, } }), - featureFlags: string(runtime.FeatureCountsAndLists) + "=true", + featureFlags: string(utilruntime.FeatureCountsAndLists) + "=true", lp: &autoscalingv1.ListPolicy{ Key: "gamers", - MaxCapacity: 100, + MaxCapacity: 70, MinCapacity: 10, - BufferSize: intstr.FromInt(1), + BufferSize: intstr.FromInt(10), + }, + gsList: []agonesv1.GameServer{ + {ObjectMeta: metav1.ObjectMeta{ + Name: "gs1", + Labels: map[string]string{"agones.dev/fleet": "fleet-1"}}, + Status: agonesv1.GameServerStatus{ + NodeName: "n1", + Lists: map[string]agonesv1.ListStatus{ + "gamers": { + Values: []string{}, + Capacity: 10, + }}}}, + {ObjectMeta: metav1.ObjectMeta{ + Name: "gs2", + Labels: map[string]string{"agones.dev/fleet": "fleet-1"}}, + Status: agonesv1.GameServerStatus{ + NodeName: "n1", + Lists: map[string]agonesv1.ListStatus{ + "gamers": { + Values: []string{}, + Capacity: 10, + }}}}, + {ObjectMeta: metav1.ObjectMeta{ + Name: "gs3", + Labels: map[string]string{"agones.dev/fleet": "fleet-1"}}, + Status: agonesv1.GameServerStatus{ + NodeName: "n1", + Lists: map[string]agonesv1.ListStatus{ + "gamers": { + Values: []string{}, + Capacity: 10, + }}}}, + {ObjectMeta: metav1.ObjectMeta{ + Name: "gs4", + Labels: map[string]string{"agones.dev/fleet": "fleet-1"}}, + Status: agonesv1.GameServerStatus{ + NodeName: "n1", + Lists: map[string]agonesv1.ListStatus{ + "gamers": { + Values: []string{"default1", "default2", "default3", "default4", "default5", "default6", "default7", "default8"}, + Capacity: 8, + }}}}, + {ObjectMeta: metav1.ObjectMeta{ + Name: "gs5", + Labels: map[string]string{"agones.dev/fleet": "fleet-1"}}, + Status: agonesv1.GameServerStatus{ + NodeName: "n1", + Lists: map[string]agonesv1.ListStatus{ + "gamers": { + Values: []string{"default9", "default10", "default11", "default12"}, + Capacity: 10, + }}}}, + {ObjectMeta: metav1.ObjectMeta{ + Name: "gs6", + Labels: map[string]string{"agones.dev/fleet": "fleet-1"}}, + Status: agonesv1.GameServerStatus{ + NodeName: "n1", + Lists: map[string]agonesv1.ListStatus{ + "gamers": { + Values: []string{"default"}, + Capacity: 4, + }}}}, + {ObjectMeta: metav1.ObjectMeta{ + Name: "gs7", + Labels: map[string]string{"agones.dev/fleet": "fleet-1"}}, + Status: agonesv1.GameServerStatus{ + NodeName: "n1", + Lists: map[string]agonesv1.ListStatus{ + "gamers": { + Values: []string{"default"}, + Capacity: 8, + }}}}, + {ObjectMeta: metav1.ObjectMeta{ + Name: "gs8", + Labels: map[string]string{"agones.dev/fleet": "fleet-1"}}, + Status: agonesv1.GameServerStatus{ + NodeName: "n1", + Lists: map[string]agonesv1.ListStatus{ + "gamers": { + Values: []string{"default"}, + Capacity: 10, + }}}}, }, want: expected{ replicas: 3, @@ -1180,21 +1554,21 @@ func TestApplyListPolicy(t *testing.T) { }, }, "scale up limited": { - fleet: modifiedFleet(func(spec *agonesv1.GameServerTemplateSpec, status *agonesv1.FleetStatus) { - spec.Spec.Lists = make(map[string]agonesv1.ListStatus) - spec.Spec.Lists["gamers"] = agonesv1.ListStatus{ + fleet: modifiedFleet(func(f *agonesv1.Fleet) { + f.Spec.Template.Spec.Lists = make(map[string]agonesv1.ListStatus) + f.Spec.Template.Spec.Lists["gamers"] = agonesv1.ListStatus{ Values: []string{"default", "default2"}, Capacity: 3} - status.Replicas = 10 - status.ReadyReplicas = 0 - status.AllocatedReplicas = 10 - status.Lists = make(map[string]agonesv1.AggregatedListStatus) - status.Lists["gamers"] = agonesv1.AggregatedListStatus{ + f.Status.Replicas = 10 + f.Status.ReadyReplicas = 0 + f.Status.AllocatedReplicas = 10 + f.Status.Lists = make(map[string]agonesv1.AggregatedListStatus) + f.Status.Lists["gamers"] = agonesv1.AggregatedListStatus{ Count: 29, Capacity: 30, } }), - featureFlags: string(runtime.FeatureCountsAndLists) + "=true", + featureFlags: string(utilruntime.FeatureCountsAndLists) + "=true", lp: &autoscalingv1.ListPolicy{ Key: "gamers", MaxCapacity: 30, @@ -1208,49 +1582,119 @@ func TestApplyListPolicy(t *testing.T) { }, }, "scale down limited": { - fleet: modifiedFleet(func(spec *agonesv1.GameServerTemplateSpec, status *agonesv1.FleetStatus) { - spec.Spec.Lists = make(map[string]agonesv1.ListStatus) - spec.Spec.Lists["gamers"] = agonesv1.ListStatus{ + fleet: modifiedFleet(func(f *agonesv1.Fleet) { + f.Spec.Template.Spec.Lists = make(map[string]agonesv1.ListStatus) + f.Spec.Template.Spec.Lists["gamers"] = agonesv1.ListStatus{ Values: []string{}, Capacity: 5} - status.Replicas = 10 - status.ReadyReplicas = 7 - status.AllocatedReplicas = 3 - status.Lists = make(map[string]agonesv1.AggregatedListStatus) - status.Lists["gamers"] = agonesv1.AggregatedListStatus{ + f.Spec.Priorities = []agonesv1.Priority{{PriorityType: "List", Key: "gamers", Order: "Ascending"}} + f.Status.Replicas = 4 + f.Status.ReadyReplicas = 3 + f.Status.AllocatedReplicas = 1 + f.Status.Lists = make(map[string]agonesv1.AggregatedListStatus) + f.Status.Lists["gamers"] = agonesv1.AggregatedListStatus{ Count: 3, - Capacity: 100, + Capacity: 20, } }), - featureFlags: string(runtime.FeatureCountsAndLists) + "=true", + featureFlags: string(utilruntime.FeatureCountsAndLists) + "=true", lp: &autoscalingv1.ListPolicy{ Key: "gamers", MaxCapacity: 100, MinCapacity: 10, BufferSize: intstr.FromInt(1), }, + gsList: []agonesv1.GameServer{ + {ObjectMeta: metav1.ObjectMeta{ + Name: "gs1", + Labels: map[string]string{"agones.dev/fleet": "fleet-1"}}, + Status: agonesv1.GameServerStatus{ + NodeName: "n1", + Lists: map[string]agonesv1.ListStatus{ + "gamers": { + Values: []string{}, + Capacity: 5, + }}}}, + {ObjectMeta: metav1.ObjectMeta{ + Name: "gs2", + Labels: map[string]string{"agones.dev/fleet": "fleet-1"}}, + Status: agonesv1.GameServerStatus{ + NodeName: "n1", + Lists: map[string]agonesv1.ListStatus{ + "gamers": { + Values: []string{}, + Capacity: 5, + }}}}, + {ObjectMeta: metav1.ObjectMeta{ + Name: "gs3", + Labels: map[string]string{"agones.dev/fleet": "fleet-1"}}, + Status: agonesv1.GameServerStatus{ + NodeName: "n1", + Lists: map[string]agonesv1.ListStatus{ + "gamers": { + Values: []string{}, + Capacity: 5, + }}}}, + {ObjectMeta: metav1.ObjectMeta{ + Name: "gs4", + Labels: map[string]string{"agones.dev/fleet": "fleet-1"}}, + Status: agonesv1.GameServerStatus{ + NodeName: "n1", + Lists: map[string]agonesv1.ListStatus{ + "gamers": { + Values: []string{"default1", "default2", "default3"}, + Capacity: 5, + }}}}}, want: expected{ replicas: 2, limited: true, wantErr: false, }, }, + "scale up by percent limited": { + fleet: modifiedFleet(func(f *agonesv1.Fleet) { + f.Spec.Template.Spec.Lists = make(map[string]agonesv1.ListStatus) + f.Spec.Template.Spec.Lists["gamers"] = agonesv1.ListStatus{ + Values: []string{"default", "default2", "default3"}, + Capacity: 10} + f.Status.Replicas = 3 + f.Status.ReadyReplicas = 0 + f.Status.AllocatedReplicas = 3 + f.Status.Lists = make(map[string]agonesv1.AggregatedListStatus) + f.Status.Lists["gamers"] = agonesv1.AggregatedListStatus{ + Count: 20, + Capacity: 30, + } + }), + featureFlags: string(utilruntime.FeatureCountsAndLists) + "=true", + lp: &autoscalingv1.ListPolicy{ + Key: "gamers", + MaxCapacity: 45, + MinCapacity: 10, + BufferSize: intstr.FromString("50%"), + }, + want: expected{ + replicas: 4, + limited: true, + wantErr: false, + }, + }, "scale up by percent": { - fleet: modifiedFleet(func(spec *agonesv1.GameServerTemplateSpec, status *agonesv1.FleetStatus) { - spec.Spec.Lists = make(map[string]agonesv1.ListStatus) - spec.Spec.Lists["gamers"] = agonesv1.ListStatus{ + fleet: modifiedFleet(func(f *agonesv1.Fleet) { + f.Spec.Template.Spec.Lists = make(map[string]agonesv1.ListStatus) + f.Spec.Template.Spec.Lists["gamers"] = agonesv1.ListStatus{ Values: []string{"default"}, Capacity: 3} - status.Replicas = 10 - status.ReadyReplicas = 0 - status.AllocatedReplicas = 10 - status.Lists = make(map[string]agonesv1.AggregatedListStatus) - status.Lists["gamers"] = agonesv1.AggregatedListStatus{ + f.Status.Replicas = 10 + f.Status.ReadyReplicas = 0 + f.Status.AllocatedReplicas = 10 + f.Status.Lists = make(map[string]agonesv1.AggregatedListStatus) + f.Status.Lists["gamers"] = agonesv1.AggregatedListStatus{ Count: 29, Capacity: 30, } }), - featureFlags: string(runtime.FeatureCountsAndLists) + "=true", + featureFlags: string(utilruntime.FeatureCountsAndLists) + "=true", lp: &autoscalingv1.ListPolicy{ Key: "gamers", MaxCapacity: 50, @@ -1263,45 +1707,149 @@ func TestApplyListPolicy(t *testing.T) { wantErr: false, }, }, - // "scale down by percent": { - // fleet: modifiedFleet(func(spec *agonesv1.GameServerTemplateSpec, status *agonesv1.FleetStatus) { - // spec.Spec.Lists = make(map[string]agonesv1.ListStatus) - // spec.Spec.Lists["gamers"] = agonesv1.ListStatus{ - // Values: []string{"default", "default2"}, - // Capacity: 3} - // status.Replicas = 16 - // status.ReadyReplicas = 6 - // status.AllocatedReplicas = 10 - // status.Lists = make(map[string]agonesv1.AggregatedListStatus) - // status.Lists["gamers"] = agonesv1.AggregatedListStatus{ - // Count: 32, - // Capacity: 48, - // } - // }), - // featureFlags: string(runtime.FeatureCountsAndLists) + "=true", - // lp: &autoscalingv1.ListPolicy{ - // Key: "gamers", - // MaxCapacity: 50, - // MinCapacity: 10, - // BufferSize: intstr.FromString("5%"), - // }, - // want: expected{ - // replicas: 4, // TODO: Result gives want: 12 which works for "desired capacity" of 32 + 5%, but does not actually work beause the count is 2 in a default game server (although not necessarily live game servers, which further complicates things). So by removing 4 game servers we are also removing ~8 from the count, which gives us too large of a buffer. - // limited: true, - // wantErr: false, - // }, - // }, + "scale down by percent to Zero": { + fleet: modifiedFleet(func(f *agonesv1.Fleet) { + f.Spec.Template.Spec.Lists = make(map[string]agonesv1.ListStatus) + f.Spec.Template.Spec.Lists["gamers"] = agonesv1.ListStatus{ + Values: []string{"default", "default2"}, + Capacity: 10} + f.Spec.Priorities = []agonesv1.Priority{{PriorityType: "List", Key: "gamers", Order: "Descending"}} + f.Status.Replicas = 3 + f.Status.ReadyReplicas = 3 + f.Status.AllocatedReplicas = 0 + f.Status.Lists = make(map[string]agonesv1.AggregatedListStatus) + f.Status.Lists["gamers"] = agonesv1.AggregatedListStatus{ + Count: 15, + Capacity: 30, + } + }), + featureFlags: string(utilruntime.FeatureCountsAndLists) + "=true", + lp: &autoscalingv1.ListPolicy{ + Key: "gamers", + MaxCapacity: 50, + MinCapacity: 0, + BufferSize: intstr.FromString("20%"), + }, + gsList: []agonesv1.GameServer{ + {ObjectMeta: metav1.ObjectMeta{ + Name: "gs1", + Labels: map[string]string{"agones.dev/fleet": "fleet-1"}}, + Status: agonesv1.GameServerStatus{ + NodeName: "n1", + Lists: map[string]agonesv1.ListStatus{ + "gamers": { + Values: []string{"1", "2", "3", "4", "5"}, + Capacity: 15, + }}}}, + {ObjectMeta: metav1.ObjectMeta{ + Name: "gs2", + Labels: map[string]string{"agones.dev/fleet": "fleet-1"}}, + Status: agonesv1.GameServerStatus{ + NodeName: "n1", + Lists: map[string]agonesv1.ListStatus{ + "gamers": { + Values: []string{"1", "2", "3", "4", "5", "6", "7"}, + Capacity: 10, + }}}}, + {ObjectMeta: metav1.ObjectMeta{ + Name: "gs3", + Labels: map[string]string{"agones.dev/fleet": "fleet-1"}}, + Status: agonesv1.GameServerStatus{ + NodeName: "n1", + Lists: map[string]agonesv1.ListStatus{ + "gamers": { + Values: []string{"1", "2", "3"}, + Capacity: 5, + }}}}, + }, + want: expected{ + replicas: 0, + limited: true, + wantErr: false, + }, + }, + "scale down by percent limited": { + fleet: modifiedFleet(func(f *agonesv1.Fleet) { + f.Spec.Template.Spec.Lists = make(map[string]agonesv1.ListStatus) + f.Spec.Template.Spec.Lists["gamers"] = agonesv1.ListStatus{ + Values: []string{"default", "default2"}, + Capacity: 10} + f.Spec.Priorities = []agonesv1.Priority{{PriorityType: "List", Key: "gamers", Order: "Descending"}} + f.Status.Replicas = 3 + f.Status.ReadyReplicas = 3 + f.Status.AllocatedReplicas = 0 + f.Status.Lists = make(map[string]agonesv1.AggregatedListStatus) + f.Status.Lists["gamers"] = agonesv1.AggregatedListStatus{ + Count: 15, + Capacity: 30, + } + }), + featureFlags: string(utilruntime.FeatureCountsAndLists) + "=true", + lp: &autoscalingv1.ListPolicy{ + Key: "gamers", + MaxCapacity: 50, + MinCapacity: 1, + BufferSize: intstr.FromString("20%"), + }, + gsList: []agonesv1.GameServer{ + {ObjectMeta: metav1.ObjectMeta{ + Name: "gs1", + Labels: map[string]string{"agones.dev/fleet": "fleet-1"}}, + Status: agonesv1.GameServerStatus{ + NodeName: "n1", + Lists: map[string]agonesv1.ListStatus{ + "gamers": { + Values: []string{"1", "2", "3", "4", "5"}, + Capacity: 15, + }}}}, + {ObjectMeta: metav1.ObjectMeta{ + Name: "gs2", + Labels: map[string]string{"agones.dev/fleet": "fleet-1"}}, + Status: agonesv1.GameServerStatus{ + NodeName: "n1", + Lists: map[string]agonesv1.ListStatus{ + "gamers": { + Values: []string{"1", "2", "3", "4", "5", "6", "7"}, + Capacity: 10, + }}}}, + {ObjectMeta: metav1.ObjectMeta{ + Name: "gs3", + Labels: map[string]string{"agones.dev/fleet": "fleet-1"}}, + Status: agonesv1.GameServerStatus{ + NodeName: "n1", + Lists: map[string]agonesv1.ListStatus{ + "gamers": { + Values: []string{"1", "2", "3"}, + Capacity: 5, + }}}}, + }, + want: expected{ + replicas: 1, + limited: true, + wantErr: false, + }, + }, } - runtime.FeatureTestMutex.Lock() - defer runtime.FeatureTestMutex.Unlock() + utilruntime.FeatureTestMutex.Lock() + defer utilruntime.FeatureTestMutex.Unlock() for name, tc := range testCases { t.Run(name, func(t *testing.T) { - err := runtime.ParseFeatures(tc.featureFlags) + err := utilruntime.ParseFeatures(tc.featureFlags) assert.NoError(t, err) - replicas, limited, err := applyListPolicy(tc.lp, tc.fleet) + m := agtesting.NewMocks() + m.AgonesClient.AddReactor("list", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) { + return true, &agonesv1.GameServerList{Items: tc.gsList}, nil + }) + + informer := m.AgonesInformerFactory.Agones().V1() + _, cancel := agtesting.StartInformers(m, + informer.GameServers().Informer().HasSynced) + defer cancel() + + replicas, limited, err := applyCounterOrListPolicy(nil, tc.lp, tc.fleet, informer.GameServers().Lister(), nc) if tc.want.wantErr { assert.NotNil(t, err) diff --git a/pkg/gameserverallocations/allocation_cache.go b/pkg/gameserverallocations/allocation_cache.go index 43f25db92a..b3ed5895c5 100644 --- a/pkg/gameserverallocations/allocation_cache.go +++ b/pkg/gameserverallocations/allocation_cache.go @@ -237,14 +237,14 @@ func (c *AllocationCache) ListSortedGameServers(gsa *allocationv1.GameServerAllo for _, priority := range gsa.Spec.Priorities { res := compareGameServers(&priority, gs1, gs2) switch priority.Order { - case agonesv1.GameServerAllocationAscending: + case agonesv1.GameServerPriorityAscending: if res == -1 { return true } if res == 1 { return false } - case agonesv1.GameServerAllocationDescending, "": + case agonesv1.GameServerPriorityDescending, "": if res == -1 { return false } @@ -269,7 +269,7 @@ func (c *AllocationCache) ListSortedGameServers(gsa *allocationv1.GameServerAllo func compareGameServers(p *agonesv1.Priority, gs1, gs2 *agonesv1.GameServer) int { var gs1ok, gs2ok bool switch p.PriorityType { - case agonesv1.GameServerAllocationPriorityCounter: + case agonesv1.GameServerPriorityCounter: // Check if both game servers contain the Counter. counter1, ok1 := gs1.Status.Counters[p.Key] counter2, ok2 := gs2.Status.Counters[p.Key] @@ -287,7 +287,7 @@ func compareGameServers(p *agonesv1.Priority, gs1, gs2 *agonesv1.GameServer) int } gs1ok = ok1 gs2ok = ok2 - case agonesv1.GameServerAllocationPriorityList: + case agonesv1.GameServerPriorityList: // Check if both game servers contain the List. list1, ok1 := gs1.Status.Lists[p.Key] list2, ok2 := gs2.Status.Lists[p.Key] @@ -308,13 +308,13 @@ func compareGameServers(p *agonesv1.Priority, gs1, gs2 *agonesv1.GameServer) int } // If only one game server has the Priority, prefer that server. I.e. nil < gsX when Order is // Descending (3, 2, 1, 0, nil), and nil > gsX when Order is Ascending (0, 1, 2, 3, nil). - if (gs1ok && p.Order == agonesv1.GameServerAllocationDescending) || + if (gs1ok && p.Order == agonesv1.GameServerPriorityDescending) || (gs1ok && p.Order == "") || - (gs2ok && p.Order == agonesv1.GameServerAllocationAscending) { + (gs2ok && p.Order == agonesv1.GameServerPriorityAscending) { return 1 } - if (gs1ok && p.Order == agonesv1.GameServerAllocationAscending) || - (gs2ok && p.Order == agonesv1.GameServerAllocationDescending) || + if (gs1ok && p.Order == agonesv1.GameServerPriorityAscending) || + (gs2ok && p.Order == agonesv1.GameServerPriorityDescending) || (gs2ok && p.Order == "") { return -1 } diff --git a/pkg/gameserversets/allocation_overflow.go b/pkg/gameserversets/allocation_overflow.go index f4d9249507..65ccae7c86 100644 --- a/pkg/gameserversets/allocation_overflow.go +++ b/pkg/gameserversets/allocation_overflow.go @@ -145,7 +145,7 @@ func (c *AllocationOverflowController) syncGameServerSet(ctx context.Context, ke return nil } - rest = sortGameServersByStrategy(gsSet.Spec.Scheduling, rest, c.counter.Counts()) + rest = SortGameServersByStrategy(gsSet.Spec.Scheduling, rest, c.counter.Counts(), gsSet.Spec.Priorities) rest = rest[:(overflow - matches)] opts := v1.UpdateOptions{} diff --git a/pkg/gameserversets/controller.go b/pkg/gameserversets/controller.go index 5f37533d00..95c0d5f5a2 100644 --- a/pkg/gameserversets/controller.go +++ b/pkg/gameserversets/controller.go @@ -322,7 +322,7 @@ func (c *Controller) syncGameServerSet(ctx context.Context, key string) error { list = c.stateCache.forGameServerSet(gsSet).reconcileWithUpdatedServerList(list) numServersToAdd, toDelete, isPartial := computeReconciliationAction(gsSet.Spec.Scheduling, list, c.counter.Counts(), - int(gsSet.Spec.Replicas), maxGameServerCreationsPerBatch, maxGameServerDeletionsPerBatch, maxPodPendingCount) + int(gsSet.Spec.Replicas), maxGameServerCreationsPerBatch, maxGameServerDeletionsPerBatch, maxPodPendingCount, gsSet.Spec.Priorities) // GameserverSet is marked for deletion then don't add gameservers. if !gsSet.DeletionTimestamp.IsZero() { @@ -378,7 +378,7 @@ func (c *Controller) syncGameServerSet(ctx context.Context, key string) error { // the list of game servers that were found and target replica count. func computeReconciliationAction(strategy apis.SchedulingStrategy, list []*agonesv1.GameServer, counts map[string]gameservers.NodeCount, targetReplicaCount int, maxCreations int, maxDeletions int, - maxPending int) (int, []*agonesv1.GameServer, bool) { + maxPending int, priorities []agonesv1.Priority) (int, []*agonesv1.GameServer, bool) { var upCount int // up == Ready or will become ready var deleteCount int // number of gameservers to delete @@ -441,7 +441,7 @@ func computeReconciliationAction(strategy apis.SchedulingStrategy, list []*agone handleGameServerUp(gs) case agonesv1.GameServerStateReady: handleGameServerUp(gs) - case agonesv1.GameServerStateReserved: + case agonesv1.GameServerStateReserved: // TODO: Isn't this already handled above in pass 1? handleGameServerUp(gs) // GameServerStateShutdown - already handled above @@ -478,7 +478,7 @@ func computeReconciliationAction(strategy apis.SchedulingStrategy, list []*agone } if deleteCount > 0 { - potentialDeletions = sortGameServersByStrategy(strategy, potentialDeletions, counts) + potentialDeletions = SortGameServersByStrategy(strategy, potentialDeletions, counts, priorities) toDelete = append(toDelete, potentialDeletions[0:deleteCount]...) } diff --git a/pkg/gameserversets/controller_test.go b/pkg/gameserversets/controller_test.go index 7fd64accbc..490f966879 100644 --- a/pkg/gameserversets/controller_test.go +++ b/pkg/gameserversets/controller_test.go @@ -73,6 +73,7 @@ func TestComputeReconciliationAction(t *testing.T) { wantNumServersToAdd int wantNumServersToDelete int wantIsPartial bool + priorities []agonesv1.Priority }{ { desc: "Empty", @@ -219,7 +220,7 @@ func TestComputeReconciliationAction(t *testing.T) { for _, tc := range cases { t.Run(tc.desc, func(t *testing.T) { toAdd, toDelete, isPartial := computeReconciliationAction(apis.Distributed, tc.list, map[string]gameservers.NodeCount{}, - tc.targetReplicaCount, maxTestCreationsPerBatch, maxTestDeletionsPerBatch, maxTestPendingPerBatch) + tc.targetReplicaCount, maxTestCreationsPerBatch, maxTestDeletionsPerBatch, maxTestPendingPerBatch, tc.priorities) assert.Equal(t, tc.wantNumServersToAdd, toAdd, "# of GameServers to add") assert.Len(t, toDelete, tc.wantNumServersToDelete, "# of GameServers to delete") @@ -237,7 +238,7 @@ func TestComputeReconciliationAction(t *testing.T) { counts := map[string]gameservers.NodeCount{"node1": {Ready: 1}, "node3": {Ready: 2}} toAdd, toDelete, isPartial := computeReconciliationAction(apis.Packed, list, counts, 2, - 1000, 1000, 1000) + 1000, 1000, 1000, nil) assert.Empty(t, toAdd) assert.False(t, isPartial, "shouldn't be partial") @@ -262,7 +263,7 @@ func TestComputeReconciliationAction(t *testing.T) { } toAdd, toDelete, isPartial := computeReconciliationAction(apis.Distributed, list, map[string]gameservers.NodeCount{}, - 2, 1000, 1000, 1000) + 2, 1000, 1000, 1000, nil) assert.Empty(t, toAdd) assert.False(t, isPartial, "shouldn't be partial") diff --git a/pkg/gameserversets/gameserversets.go b/pkg/gameserversets/gameserversets.go index 03fd8c1e24..65d67e64ad 100644 --- a/pkg/gameserversets/gameserversets.go +++ b/pkg/gameserversets/gameserversets.go @@ -22,6 +22,7 @@ import ( listerv1 "agones.dev/agones/pkg/client/listers/agones/v1" "agones.dev/agones/pkg/gameservers" "agones.dev/agones/pkg/util/logfields" + "agones.dev/agones/pkg/util/runtime" "github.com/pkg/errors" "github.com/sirupsen/logrus" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -40,16 +41,17 @@ func loggerForGameServerSet(log *logrus.Entry, gsSet *agonesv1.GameServerSet) *l return loggerForGameServerSetKey(log, gsSetName).WithField("gss", gsSet) } -// sortGameServersByStrategy will sort by least full nodes when Packed, and newest first when Distributed -func sortGameServersByStrategy(strategy apis.SchedulingStrategy, list []*agonesv1.GameServer, counts map[string]gameservers.NodeCount) []*agonesv1.GameServer { +// SortGameServersByStrategy will sort by least full nodes when Packed, and newest first when Distributed +func SortGameServersByStrategy(strategy apis.SchedulingStrategy, list []*agonesv1.GameServer, counts map[string]gameservers.NodeCount, priorities []agonesv1.Priority) []*agonesv1.GameServer { if strategy == apis.Packed { - return sortGameServersByLeastFullNodes(list, counts) + return sortGameServersByLeastFullNodes(list, counts, priorities) } return sortGameServersByNewFirst(list) } -// sortGameServersByLeastFullNodes sorts the list of gameservers by which gameservers reside on the least full nodes -func sortGameServersByLeastFullNodes(list []*agonesv1.GameServer, count map[string]gameservers.NodeCount) []*agonesv1.GameServer { +// SortGameServersByLeastFullNodes sorts the list of gameservers by which gameservers reside on the least full nodes +// Performs a tie-breaking sort if nodes are equally full on CountsAndLists Priorities. +func sortGameServersByLeastFullNodes(list []*agonesv1.GameServer, count map[string]gameservers.NodeCount, priorities []agonesv1.Priority) []*agonesv1.GameServer { sort.Slice(list, func(i, j int) bool { a := list[i] b := list[j] @@ -65,6 +67,7 @@ func sortGameServersByLeastFullNodes(list []*agonesv1.GameServer, count map[stri return false } + // TODO: What if they're not on the same node -- do we want to delete the gameserver of "equal" Nodes where one Node has creating gameservers and the other has Ready gameservers? // if both are in the same node, make sure to delete pre-Ready GameServers first if a.Status.NodeName == b.Status.NodeName { if a.IsBeforeReady() && b.Status.State == agonesv1.GameServerStateReady { @@ -75,12 +78,45 @@ func sortGameServersByLeastFullNodes(list []*agonesv1.GameServer, count map[stri return false } } - // if both Nodes have the same count, one node is emptied first (packed scheduling behavior) + + // Check Node total count acTotal, bcTotal := ac.Allocated+ac.Ready, bc.Allocated+bc.Ready - if acTotal == bcTotal { - return a.Status.NodeName < b.Status.NodeName + if acTotal < bcTotal { + return true } - return acTotal < bcTotal + if acTotal > bcTotal { + return false + } + + // if the Nodes have the same count and CountsAndLists flag is enabled, sort based on CountsAndLists Priorities. + if runtime.FeatureEnabled(runtime.FeatureCountsAndLists) && (priorities != nil) { + for _, priority := range priorities { + res := compareGameServerCapacity(&priority, a, b) + switch priority.Order { + case agonesv1.GameServerPriorityAscending, "": + if res == -1 { + return true + } + if res == 1 { + return false + } + case agonesv1.GameServerPriorityDescending: + if res == -1 { + return false + } + if res == 1 { + return true + } + } + } + } + + // if the gameservers are on the same node, then sort lexigraphically for a stable sort + if a.Status.NodeName == b.Status.NodeName { + return a.GetObjectMeta().GetName() < b.GetObjectMeta().GetName() + } + // if both Nodes have the same count, one node is emptied first (packed scheduling behavior) + return a.Status.NodeName < b.Status.NodeName }) return list @@ -115,3 +151,77 @@ func ListGameServersByGameServerSetOwner(gameServerLister listerv1.GameServerLis return result, nil } + +// compareGameServerCapacity compares two game servers based on a CountsAndLists Priority. First +// compares by Capacity, and then compares by Count or length of the List Values if Capacity is equal. +// NOTE: This does NOT sort by available capacity. +// Returns -1 if gs1 < gs2; 1 if gs1 > gs2; 0 if gs1 == gs2; 0 if neither gamer server has the Priority. +// If only one game server has the Priority, prefer that server. I.e. nil < gsX when Priority +// Order is Descending (3, 2, 1, 0, nil), and nil > gsX when Order is Ascending (0, 1, 2, 3, nil). +func compareGameServerCapacity(p *agonesv1.Priority, gs1, gs2 *agonesv1.GameServer) int { + var gs1ok, gs2ok bool + switch p.PriorityType { + case agonesv1.GameServerPriorityCounter: + // Check if both game servers contain the Counter. + counter1, ok1 := gs1.Status.Counters[p.Key] + counter2, ok2 := gs2.Status.Counters[p.Key] + // If both game servers have the Counter + if ok1 && ok2 { + if counter1.Capacity < counter2.Capacity { + return -1 + } + if counter1.Capacity > counter2.Capacity { + return 1 + } + if counter1.Capacity == counter2.Capacity { + if counter1.Count < counter2.Count { + return -1 + } + if counter1.Count > counter2.Count { + return 1 + } + return 0 + } + } + gs1ok = ok1 + gs2ok = ok2 + case agonesv1.GameServerPriorityList: + // Check if both game servers contain the List. + list1, ok1 := gs1.Status.Lists[p.Key] + list2, ok2 := gs2.Status.Lists[p.Key] + // If both game servers have the List + if ok1 && ok2 { + if list1.Capacity < list2.Capacity { + return -1 + } + if list1.Capacity > list2.Capacity { + return 1 + } + if list1.Capacity == list2.Capacity { + if len(list1.Values) < len(list2.Values) { + return -1 + } + if len(list1.Values) > len(list2.Values) { + return 1 + } + return 0 + } + } + gs1ok = ok1 + gs2ok = ok2 + } + // If only one game server has the Priority, prefer that server. I.e. nil < gsX when Order is + // Descending (3, 2, 1, 0, nil), and nil > gsX when Order is Ascending (0, 1, 2, 3, nil). + if (gs1ok && p.Order == agonesv1.GameServerPriorityDescending) || + (gs1ok && p.Order == "") || + (gs2ok && p.Order == agonesv1.GameServerPriorityAscending) { + return 1 + } + if (gs1ok && p.Order == agonesv1.GameServerPriorityAscending) || + (gs2ok && p.Order == agonesv1.GameServerPriorityDescending) || + (gs2ok && p.Order == "") { + return -1 + } + // If neither game server has the Priority + return 0 +} diff --git a/pkg/gameserversets/gameserversets_test.go b/pkg/gameserversets/gameserversets_test.go index 70963e2ae5..b21dc666f4 100644 --- a/pkg/gameserversets/gameserversets_test.go +++ b/pkg/gameserversets/gameserversets_test.go @@ -22,6 +22,7 @@ import ( agonesv1 "agones.dev/agones/pkg/apis/agones/v1" "agones.dev/agones/pkg/gameservers" agtesting "agones.dev/agones/pkg/testing" + utilruntime "agones.dev/agones/pkg/util/runtime" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -32,11 +33,16 @@ import ( func TestSortGameServersByLeastFullNodes(t *testing.T) { t.Parallel() + utilruntime.FeatureTestMutex.Lock() + defer utilruntime.FeatureTestMutex.Unlock() + + require.NoError(t, utilruntime.ParseFeatures(string(utilruntime.FeatureCountsAndLists)+"=true")) + nc := map[string]gameservers.NodeCount{ "n1": {Ready: 1, Allocated: 0}, "n2": {Ready: 0, Allocated: 2}, - "n3": {Ready: 2, Allocated: 0}, - "n4": {Ready: 2, Allocated: 0}, + "n3": {Ready: 2, Allocated: 2}, + "n4": {Ready: 2, Allocated: 2}, } list := []*agonesv1.GameServer{ @@ -48,21 +54,83 @@ func TestSortGameServersByLeastFullNodes(t *testing.T) { {ObjectMeta: metav1.ObjectMeta{Name: "g6"}, Status: agonesv1.GameServerStatus{NodeName: "n4", State: agonesv1.GameServerStateReady}}, {ObjectMeta: metav1.ObjectMeta{Name: "g7"}, Status: agonesv1.GameServerStatus{NodeName: "n3", State: agonesv1.GameServerStateReady}}, {ObjectMeta: metav1.ObjectMeta{Name: "g8"}, Status: agonesv1.GameServerStatus{NodeName: "n4", State: agonesv1.GameServerStateReady}}, + {ObjectMeta: metav1.ObjectMeta{Name: "g9"}, Status: agonesv1.GameServerStatus{ + NodeName: "n3", + State: agonesv1.GameServerStateAllocated, + Counters: map[string]agonesv1.CounterStatus{ + "foo": { + Count: 0, + Capacity: 100, + }}}}, + {ObjectMeta: metav1.ObjectMeta{Name: "g10"}, Status: agonesv1.GameServerStatus{ + NodeName: "n3", + State: agonesv1.GameServerStateAllocated, + Counters: map[string]agonesv1.CounterStatus{ + "foo": { + Count: 99, + Capacity: 100, + }}}}, + {ObjectMeta: metav1.ObjectMeta{Name: "g11"}, Status: agonesv1.GameServerStatus{ + NodeName: "n4", + State: agonesv1.GameServerStateAllocated, + Counters: map[string]agonesv1.CounterStatus{ + "foo": { + Count: 0, + Capacity: 90, + }}}}, + {ObjectMeta: metav1.ObjectMeta{Name: "g12"}, Status: agonesv1.GameServerStatus{ + NodeName: "n4", + State: agonesv1.GameServerStateAllocated, + Counters: map[string]agonesv1.CounterStatus{ + "foo": { + Count: 89, + Capacity: 90, + }}}}, } - result := sortGameServersByLeastFullNodes(list, nc) + priorities := []agonesv1.Priority{{ + PriorityType: "Counter", + Key: "foo", + Order: "Descending", + }} + + result := sortGameServersByLeastFullNodes(list, nc, priorities) + + require.Len(t, result, len(list)) + assert.Equal(t, "g2", result[0].ObjectMeta.Name) + assert.Equal(t, "g3", result[1].ObjectMeta.Name) + assert.Equal(t, "g4", result[2].ObjectMeta.Name) + assert.Equal(t, "g1", result[3].ObjectMeta.Name) + assert.Equal(t, "g10", result[4].ObjectMeta.Name) + assert.Equal(t, "g9", result[5].ObjectMeta.Name) + assert.Equal(t, "g12", result[6].ObjectMeta.Name) + assert.Equal(t, "g11", result[7].ObjectMeta.Name) + assert.Equal(t, "g5", result[8].ObjectMeta.Name) + assert.Equal(t, "g7", result[9].ObjectMeta.Name) + assert.Equal(t, "g6", result[10].ObjectMeta.Name) + assert.Equal(t, "g8", result[11].ObjectMeta.Name) + + priorities = []agonesv1.Priority{{ + PriorityType: "Counter", + Key: "foo", + Order: "Ascending", + }} + + result = sortGameServersByLeastFullNodes(list, nc, priorities) require.Len(t, result, len(list)) assert.Equal(t, "g2", result[0].ObjectMeta.Name) assert.Equal(t, "g3", result[1].ObjectMeta.Name) assert.Equal(t, "g4", result[2].ObjectMeta.Name) assert.Equal(t, "g1", result[3].ObjectMeta.Name) - // gs on the same node are adjacent (g5 and g7, g6 and g8). - // The order among the two is not stable, since sort.Slice is. - assert.Equal(t, "n3", result[4].Status.NodeName) - assert.Equal(t, "n3", result[5].Status.NodeName) - assert.Equal(t, "n4", result[6].Status.NodeName) - assert.Equal(t, "n4", result[7].Status.NodeName) + assert.Equal(t, "g11", result[4].ObjectMeta.Name) + assert.Equal(t, "g12", result[5].ObjectMeta.Name) + assert.Equal(t, "g9", result[6].ObjectMeta.Name) + assert.Equal(t, "g10", result[7].ObjectMeta.Name) + assert.Equal(t, "g5", result[8].ObjectMeta.Name) + assert.Equal(t, "g7", result[9].ObjectMeta.Name) + assert.Equal(t, "g6", result[10].ObjectMeta.Name) + assert.Equal(t, "g8", result[11].ObjectMeta.Name) } func TestSortGameServersByNewFirst(t *testing.T) {