diff --git a/pkg/fleetautoscalers/fleetautoscalers.go b/pkg/fleetautoscalers/fleetautoscalers.go index 8663be99ad..0e7cc428ed 100644 --- a/pkg/fleetautoscalers/fleetautoscalers.go +++ b/pkg/fleetautoscalers/fleetautoscalers.go @@ -355,7 +355,7 @@ func applyCounterOrListPolicy(c *autoscalingv1.CounterPolicy, l *autoscalingv1.L } 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 + if limited { // 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) } diff --git a/pkg/fleetautoscalers/fleetautoscalers_test.go b/pkg/fleetautoscalers/fleetautoscalers_test.go index 48a27b3588..eec82195c0 100644 --- a/pkg/fleetautoscalers/fleetautoscalers_test.go +++ b/pkg/fleetautoscalers/fleetautoscalers_test.go @@ -1570,6 +1570,31 @@ func TestApplyListPolicy(t *testing.T) { wantErr: true, }, }, + "fleet does not have any replicas": { + 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} + f.Status.Replicas = 0 + f.Status.ReadyReplicas = 0 + f.Status.AllocatedReplicas = 0 + f.Status.Lists = make(map[string]agonesv1.AggregatedListStatus) + f.Status.Lists["gamers"] = agonesv1.AggregatedListStatus{} + }), + featureFlags: string(utilruntime.FeatureCountsAndLists) + "=true", + lp: &autoscalingv1.ListPolicy{ + Key: "gamers", + MaxCapacity: 100, + MinCapacity: 10, + BufferSize: intstr.FromInt(10), + }, + want: expected{ + replicas: 2, + limited: true, + wantErr: false, + }, + }, "scale up": { fleet: modifiedFleet(func(f *agonesv1.Fleet) { f.Spec.Template.Spec.Lists = make(map[string]agonesv1.ListStatus) diff --git a/pkg/gameserversets/controller.go b/pkg/gameserversets/controller.go index 4292f886e4..dc0b169a7b 100644 --- a/pkg/gameserversets/controller.go +++ b/pkg/gameserversets/controller.go @@ -320,7 +320,7 @@ func (c *Controller) syncGameServerSet(ctx context.Context, key string) error { numServersToAdd = 0 } - status := computeStatus(list) + status := computeStatus(gsSet, list) fields := logrus.Fields{} for _, gs := range list { @@ -616,7 +616,7 @@ func parallelize(gameServers chan *agonesv1.GameServer, parallelism int, work fu // syncGameServerSetStatus synchronises the GameServerSet State with active GameServer counts func (c *Controller) syncGameServerSetStatus(ctx context.Context, gsSet *agonesv1.GameServerSet, list []*agonesv1.GameServer) error { - return c.updateStatusIfChanged(ctx, gsSet, computeStatus(list)) + return c.updateStatusIfChanged(ctx, gsSet, computeStatus(gsSet, list)) } // updateStatusIfChanged updates GameServerSet status if it's different than provided. @@ -633,8 +633,13 @@ func (c *Controller) updateStatusIfChanged(ctx context.Context, gsSet *agonesv1. } // computeStatus computes the status of the game server set. -func computeStatus(list []*agonesv1.GameServer) agonesv1.GameServerSetStatus { +func computeStatus(gsSet *agonesv1.GameServerSet, list []*agonesv1.GameServer) agonesv1.GameServerSetStatus { var status agonesv1.GameServerSetStatus + + // Initialize list status with empty lists from spec + if runtime.FeatureEnabled(runtime.FeatureCountsAndLists) { + status.Lists = createInitialListStatus(gsSet) + } for _, gs := range list { if gs.IsBeingDeleted() { // don't count GS that are being deleted @@ -687,6 +692,14 @@ func computeStatus(list []*agonesv1.GameServer) agonesv1.GameServerSetStatus { return status } +func createInitialListStatus(gsSet *agonesv1.GameServerSet) map[string]agonesv1.AggregatedListStatus { + list := make(map[string]agonesv1.AggregatedListStatus) + for name := range gsSet.Spec.Template.Spec.Lists { + list[name] = agonesv1.AggregatedListStatus{} + } + return list +} + // aggregateCounters adds the contents of a CounterStatus map to an AggregatedCounterStatus map. func aggregateCounters(aggCounterStatus map[string]agonesv1.AggregatedCounterStatus, counterStatus map[string]agonesv1.CounterStatus, diff --git a/pkg/gameserversets/controller_test.go b/pkg/gameserversets/controller_test.go index c3c86d9616..511be442d9 100644 --- a/pkg/gameserversets/controller_test.go +++ b/pkg/gameserversets/controller_test.go @@ -283,7 +283,7 @@ func TestComputeStatus(t *testing.T) { defer utilruntime.FeatureTestMutex.Unlock() require.NoError(t, utilruntime.ParseFeatures(fmt.Sprintf("%s=false", utilruntime.FeatureCountsAndLists))) - + gsSet := defaultFixture() cases := []struct { list []*agonesv1.GameServer wantStatus agonesv1.GameServerSetStatus @@ -310,7 +310,7 @@ func TestComputeStatus(t *testing.T) { } for _, tc := range cases { - assert.Equal(t, tc.wantStatus, computeStatus(tc.list)) + assert.Equal(t, tc.wantStatus, computeStatus(gsSet, tc.list)) } }) @@ -320,6 +320,7 @@ func TestComputeStatus(t *testing.T) { require.NoError(t, utilruntime.ParseFeatures(fmt.Sprintf("%s=true", utilruntime.FeaturePlayerTracking))) + gsSet := defaultFixture() var list []*agonesv1.GameServer gs1 := gsWithState(agonesv1.GameServerStateAllocated) gs1.Status.Players = &agonesv1.PlayerStatus{Count: 5, Capacity: 10} @@ -344,7 +345,7 @@ func TestComputeStatus(t *testing.T) { Lists: map[string]agonesv1.AggregatedListStatus{}, } - assert.Equal(t, expected, computeStatus(list)) + assert.Equal(t, expected, computeStatus(gsSet, list)) }) t.Run("counters", func(t *testing.T) { @@ -353,6 +354,7 @@ func TestComputeStatus(t *testing.T) { require.NoError(t, utilruntime.ParseFeatures(fmt.Sprintf("%s=true", utilruntime.FeatureCountsAndLists))) + gsSet := defaultFixture() var list []*agonesv1.GameServer gs1 := gsWithState(agonesv1.GameServerStateAllocated) gs1.Status.Counters = map[string]agonesv1.CounterStatus{ @@ -407,7 +409,7 @@ func TestComputeStatus(t *testing.T) { Lists: map[string]agonesv1.AggregatedListStatus{}, } - assert.Equal(t, expected, computeStatus(list)) + assert.Equal(t, expected, computeStatus(gsSet, list)) }) t.Run("lists", func(t *testing.T) { @@ -416,6 +418,7 @@ func TestComputeStatus(t *testing.T) { require.NoError(t, utilruntime.ParseFeatures(fmt.Sprintf("%s=true", utilruntime.FeatureCountsAndLists))) + gsSet := defaultFixture() var list []*agonesv1.GameServer gs1 := gsWithState(agonesv1.GameServerStateAllocated) gs1.Status.Lists = map[string]agonesv1.ListStatus{ @@ -460,7 +463,45 @@ func TestComputeStatus(t *testing.T) { }, } - assert.Equal(t, expected, computeStatus(list)) + assert.Equal(t, expected, computeStatus(gsSet, list)) + }) + + t.Run("lists with no gameservers", func(t *testing.T) { + utilruntime.FeatureTestMutex.Lock() + defer utilruntime.FeatureTestMutex.Unlock() + + require.NoError(t, utilruntime.ParseFeatures(fmt.Sprintf("%s=true", utilruntime.FeatureCountsAndLists))) + + gsSet := defaultFixture() + gsSet.Spec.Template.Spec.Lists = map[string]agonesv1.ListStatus{ + "firstList": {Capacity: 10, Values: []string{"a", "b"}}, + "secondList": {Capacity: 1000, Values: []string{"1", "2"}}, + } + var list []*agonesv1.GameServer + + expected := agonesv1.GameServerSetStatus{ + Replicas: 0, + ReadyReplicas: 0, + ReservedReplicas: 0, + AllocatedReplicas: 0, + Counters: nil, + Lists: map[string]agonesv1.AggregatedListStatus{ + "firstList": { + AllocatedCount: 0, + AllocatedCapacity: 0, + Capacity: 0, + Count: 0, + }, + "secondList": { + AllocatedCount: 0, + AllocatedCapacity: 0, + Capacity: 0, + Count: 0, + }, + }, + } + + assert.Equal(t, expected, computeStatus(gsSet, list)) }) } diff --git a/test/e2e/fleet_test.go b/test/e2e/fleet_test.go index dc74d4014a..365ead45a3 100644 --- a/test/e2e/fleet_test.go +++ b/test/e2e/fleet_test.go @@ -1980,12 +1980,23 @@ func defaultFleet(namespace string) *agonesv1.Fleet { return fleetWithGameServerSpec(&gs.Spec, namespace) } +// defaultEmptyFleet returns a default fleet configuration with no replicas. +func defaultEmptyFleet(namespace string) *agonesv1.Fleet { + gs := framework.DefaultGameServer(namespace) + return fleetWithGameServerSpecAndReplicas(&gs.Spec, namespace, 0) +} + // fleetWithGameServerSpec returns a fleet with specified gameserver spec func fleetWithGameServerSpec(gsSpec *agonesv1.GameServerSpec, namespace string) *agonesv1.Fleet { + return fleetWithGameServerSpecAndReplicas(gsSpec, namespace, replicasCount) +} + +// fleetWithGameServerSpecAndReplicas returns a fleet with specified gameserver spec and specified replica count +func fleetWithGameServerSpecAndReplicas(gsSpec *agonesv1.GameServerSpec, namespace string, replicas int32) *agonesv1.Fleet { return &agonesv1.Fleet{ ObjectMeta: metav1.ObjectMeta{GenerateName: "simple-fleet-1.0", Namespace: namespace}, Spec: agonesv1.FleetSpec{ - Replicas: replicasCount, + Replicas: replicas, Template: agonesv1.GameServerTemplateSpec{ Spec: *gsSpec, }, diff --git a/test/e2e/fleetautoscaler_test.go b/test/e2e/fleetautoscaler_test.go index 5cdc2b1801..78a164ea12 100644 --- a/test/e2e/fleetautoscaler_test.go +++ b/test/e2e/fleetautoscaler_test.go @@ -1209,6 +1209,87 @@ func TestListAutoscaler(t *testing.T) { } } +func TestListAutoscalerWithNoReplicas(t *testing.T) { + if !runtime.FeatureEnabled(runtime.FeatureCountsAndLists) { + t.SkipNow() + } + t.Parallel() + + ctx := context.Background() + client := framework.AgonesClient.AgonesV1() + log := e2e.TestLogger(t) + + flt := defaultEmptyFleet(framework.Namespace) + flt.Spec.Template.Spec.Lists = map[string]agonesv1.ListStatus{ + "games": { + Capacity: 5, + }, + } + + flt, err := client.Fleets(framework.Namespace).Create(ctx, flt.DeepCopy(), metav1.CreateOptions{}) + require.NoError(t, err) + defer client.Fleets(framework.Namespace).Delete(ctx, flt.ObjectMeta.Name, metav1.DeleteOptions{}) // nolint:errcheck + framework.AssertFleetCondition(t, flt, e2e.FleetReadyCount(flt.Spec.Replicas)) + + fleetautoscalers := framework.AgonesClient.AutoscalingV1().FleetAutoscalers(framework.Namespace) + + listFas := func(f func(fap *autoscalingv1.FleetAutoscalerPolicy)) *autoscalingv1.FleetAutoscaler { + fas := autoscalingv1.FleetAutoscaler{ + ObjectMeta: metav1.ObjectMeta{Name: flt.ObjectMeta.Name + "-list-autoscaler", Namespace: framework.Namespace}, + Spec: autoscalingv1.FleetAutoscalerSpec{ + FleetName: flt.ObjectMeta.Name, + Policy: autoscalingv1.FleetAutoscalerPolicy{ + Type: autoscalingv1.ListPolicyType, + }, + Sync: &autoscalingv1.FleetAutoscalerSync{ + Type: autoscalingv1.FixedIntervalSyncType, + FixedInterval: autoscalingv1.FixedIntervalSync{ + Seconds: 1, + }, + }, + }, + } + f(&fas.Spec.Policy) + return &fas + } + testCases := map[string]struct { + fas *autoscalingv1.FleetAutoscaler + wantFasErr bool + wantReplicas int32 + }{ + "Scale Up to MinCapacity": { + fas: listFas(func(fap *autoscalingv1.FleetAutoscalerPolicy) { + fap.List = &autoscalingv1.ListPolicy{ + Key: "games", + BufferSize: intstr.FromInt(3), + MinCapacity: 16, + MaxCapacity: 100, + } + }), + wantFasErr: false, + wantReplicas: 4, // Capacity:20 + }, + } + for name, testCase := range testCases { + t.Run(name, func(t *testing.T) { + + fas, err := fleetautoscalers.Create(ctx, testCase.fas, metav1.CreateOptions{}) + if testCase.wantFasErr { + assert.Error(t, err) + return + } + assert.NoError(t, err) + + framework.AssertFleetCondition(t, flt, e2e.FleetReadyCount(testCase.wantReplicas)) + fleetautoscalers.Delete(ctx, fas.ObjectMeta.Name, metav1.DeleteOptions{}) // nolint:errcheck + + // Return to starting 0 replicas + framework.ScaleFleet(t, log, flt, 0) + framework.AssertFleetCondition(t, flt, e2e.FleetReadyCount(0)) + }) + } +} + func TestListAutoscalerAllocated(t *testing.T) { if !runtime.FeatureEnabled(runtime.FeatureCountsAndLists) { t.SkipNow()