Skip to content

Commit

Permalink
fix: ensure fleet autoscaler policy are namespaced
Browse files Browse the repository at this point in the history
  • Loading branch information
@lacroixthomas
lacroixthomas committed Jan 24, 2025
1 parent f560820 commit a30635d
Show file tree
Hide file tree
Showing 5 changed files with 145 additions and 105 deletions.
3 changes: 2 additions & 1 deletion pkg/fleetautoscalers/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,8 @@ func (c *Controller) syncFleetAutoscaler(ctx context.Context, key string) error
}

currentReplicas := fleet.Status.Replicas
desiredReplicas, scalingLimited, err := computeDesiredFleetSize(fas.Spec.Policy, fleet, c.gameServerLister, c.counter.Counts())
gameServerNamespacedLister := c.gameServerLister.GameServers(fleet.Namespace)
desiredReplicas, scalingLimited, err := computeDesiredFleetSize(fas.Spec.Policy, fleet, gameServerNamespacedLister, c.counter.Counts())
// If there err is nil and not an inactive schedule error (ignorable in this case), then record the event
if err != nil && !errors.Is(err, InactiveScheduleError{}) {
c.recorder.Eventf(fas, corev1.EventTypeWarning, "FleetAutoscaler",
Expand Down
47 changes: 24 additions & 23 deletions pkg/fleetautoscalers/fleetautoscalers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,20 +61,20 @@ func (InactiveScheduleError) Error() string {

// computeDesiredFleetSize computes the new desired size of the given fleet
func computeDesiredFleetSize(pol autoscalingv1.FleetAutoscalerPolicy, f *agonesv1.Fleet,
gameServerLister listeragonesv1.GameServerLister, nodeCounts map[string]gameservers.NodeCount) (int32, bool, error) {
gameServerNamespacedLister listeragonesv1.GameServerNamespaceLister, nodeCounts map[string]gameservers.NodeCount) (int32, bool, error) {
switch pol.Type {
case autoscalingv1.BufferPolicyType:
return applyBufferPolicy(pol.Buffer, f)
case autoscalingv1.WebhookPolicyType:
return applyWebhookPolicy(pol.Webhook, f)
case autoscalingv1.CounterPolicyType:
return applyCounterOrListPolicy(pol.Counter, nil, f, gameServerLister, nodeCounts)
return applyCounterOrListPolicy(pol.Counter, nil, f, gameServerNamespacedLister, nodeCounts)
case autoscalingv1.ListPolicyType:
return applyCounterOrListPolicy(nil, pol.List, f, gameServerLister, nodeCounts)
return applyCounterOrListPolicy(nil, pol.List, f, gameServerNamespacedLister, nodeCounts)
case autoscalingv1.SchedulePolicyType:
return applySchedulePolicy(pol.Schedule, f, gameServerLister, nodeCounts, time.Now())
return applySchedulePolicy(pol.Schedule, f, gameServerNamespacedLister, nodeCounts, time.Now())
case autoscalingv1.ChainPolicyType:
return applyChainPolicy(pol.Chain, f, gameServerLister, nodeCounts, time.Now())
return applyChainPolicy(pol.Chain, f, gameServerNamespacedLister, nodeCounts, time.Now())
}

return 0, false, errors.New("wrong policy type, should be one of: Buffer, Webhook, Counter, List")
Expand Down Expand Up @@ -252,7 +252,7 @@ func applyBufferPolicy(b *autoscalingv1.BufferPolicy, f *agonesv1.Fleet) (int32,
}

func applyCounterOrListPolicy(c *autoscalingv1.CounterPolicy, l *autoscalingv1.ListPolicy,
f *agonesv1.Fleet, gameServerLister listeragonesv1.GameServerLister,
f *agonesv1.Fleet, gameServerNamespacedLister listeragonesv1.GameServerNamespaceLister,
nodeCounts map[string]gameservers.NodeCount) (int32, bool, error) {

if !runtime.FeatureEnabled(runtime.FeatureCountsAndLists) {
Expand Down Expand Up @@ -350,23 +350,23 @@ func applyCounterOrListPolicy(c *autoscalingv1.CounterPolicy, l *autoscalingv1.L
switch availableCapacity := aggCapacity - aggCount; {
case availableCapacity == buffer:
if limited {
return scaleLimited(scale, f, gameServerLister, nodeCounts, key, isCounter, replicas,
return scaleLimited(scale, f, gameServerNamespacedLister, nodeCounts, key, isCounter, replicas,
capacity, aggCapacity, minCapacity, maxCapacity)
}
return replicas, false, nil
case availableCapacity < buffer: // Scale Up
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,
return scaleLimited(scale, f, gameServerNamespacedLister, nodeCounts, key, isCounter, replicas,
capacity, aggCapacity, minCapacity, maxCapacity)
}
return scaleUp(replicas, capacity, count, aggCapacity, availableCapacity, maxCapacity,
minCapacity, 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,
return scaleLimited(scale, f, gameServerNamespacedLister, nodeCounts, key, isCounter, replicas,
capacity, aggCapacity, minCapacity, maxCapacity)
}
return scaleDown(f, gameServerLister, nodeCounts, key, isCounter, replicas, aggCount,
return scaleDown(f, gameServerNamespacedLister, nodeCounts, key, isCounter, replicas, aggCount,
aggCapacity, minCapacity, buffer)
}

Expand All @@ -376,21 +376,21 @@ func applyCounterOrListPolicy(c *autoscalingv1.CounterPolicy, l *autoscalingv1.L
return 0, false, errors.Errorf("unable to apply ListPolicy %v", l)
}

func applySchedulePolicy(s *autoscalingv1.SchedulePolicy, f *agonesv1.Fleet, gameServerLister listeragonesv1.GameServerLister, nodeCounts map[string]gameservers.NodeCount, currentTime time.Time) (int32, bool, error) {
func applySchedulePolicy(s *autoscalingv1.SchedulePolicy, f *agonesv1.Fleet, gameServerNamespacedLister listeragonesv1.GameServerNamespaceLister, nodeCounts map[string]gameservers.NodeCount, currentTime time.Time) (int32, bool, error) {
// Ensure the scheduled autoscaler feature gate is enabled
if !runtime.FeatureEnabled(runtime.FeatureScheduledAutoscaler) {
return 0, false, errors.Errorf("cannot apply SchedulePolicy unless feature flag %s is enabled", runtime.FeatureScheduledAutoscaler)
}

if isScheduleActive(s, currentTime) {
return computeDesiredFleetSize(s.Policy, f, gameServerLister, nodeCounts)
return computeDesiredFleetSize(s.Policy, f, gameServerNamespacedLister, nodeCounts)
}

// If the schedule wasn't active then return the current replica amount of the fleet
return f.Status.Replicas, false, &InactiveScheduleError{}
}

func applyChainPolicy(c autoscalingv1.ChainPolicy, f *agonesv1.Fleet, gameServerLister listeragonesv1.GameServerLister, nodeCounts map[string]gameservers.NodeCount, currentTime time.Time) (int32, bool, error) {
func applyChainPolicy(c autoscalingv1.ChainPolicy, f *agonesv1.Fleet, gameServerNamespacedLister listeragonesv1.GameServerNamespaceLister, nodeCounts map[string]gameservers.NodeCount, currentTime time.Time) (int32, bool, error) {
// Ensure the scheduled autoscaler feature gate is enabled
if !runtime.FeatureEnabled(runtime.FeatureScheduledAutoscaler) {
return 0, false, errors.Errorf("cannot apply ChainPolicy unless feature flag %s is enabled", runtime.FeatureScheduledAutoscaler)
Expand All @@ -404,7 +404,7 @@ func applyChainPolicy(c autoscalingv1.ChainPolicy, f *agonesv1.Fleet, gameServer
for _, entry := range c {
switch entry.Type {
case autoscalingv1.SchedulePolicyType:
replicas, limited, err = applySchedulePolicy(entry.Schedule, f, gameServerLister, nodeCounts, currentTime)
replicas, limited, err = applySchedulePolicy(entry.Schedule, f, gameServerNamespacedLister, nodeCounts, currentTime)
// If no error was returned from the schedule policy (schedule is active and/or webhook policy within schedule was successful), then return the values given
if err == nil {
return replicas, limited, nil
Expand All @@ -417,7 +417,7 @@ func applyChainPolicy(c autoscalingv1.ChainPolicy, f *agonesv1.Fleet, gameServer
}
default:
// Every other policy type we just want to compute the desired fleet and return it
return computeDesiredFleetSize(entry.FleetAutoscalerPolicy, f, gameServerLister, nodeCounts)
return computeDesiredFleetSize(entry.FleetAutoscalerPolicy, f, gameServerNamespacedLister, nodeCounts)
}

}
Expand Down Expand Up @@ -482,12 +482,13 @@ func isScheduleActive(s *autoscalingv1.SchedulePolicy, currentTime time.Time) bo

// 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,
func getSortedGameServers(f *agonesv1.Fleet, gameServerNamespacedLister listeragonesv1.GameServerNamespaceLister,
nodeCounts map[string]gameservers.NodeCount) ([]*agonesv1.GameServer, error) {
gsList, err := fleets.ListGameServersByFleetOwner(gameServerLister, f)
gsList, err := fleets.ListGameServersByFleetOwner(gameServerNamespacedLister, f)
if err != nil {
return nil, err
}

gameServers := gssets.SortGameServersByStrategy(f.Spec.Scheduling, gsList, nodeCounts, f.Spec.Priorities)
return gameServers, nil
}
Expand Down Expand Up @@ -519,11 +520,11 @@ func scaleUpLimited(replicas int32, capacity, aggCapacity, minCapacity int64) (i
}

// scaleDownLimited scales down the fleet to meet the MaxCapacity
func scaleDownLimited(f *agonesv1.Fleet, gameServerLister listeragonesv1.GameServerLister,
func scaleDownLimited(f *agonesv1.Fleet, gameServerNamespacedLister listeragonesv1.GameServerNamespaceLister,
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)
gameServers, err := getSortedGameServers(f, gameServerNamespacedLister, nodeCounts)
if err != nil {
return 0, false, err
}
Expand Down Expand Up @@ -552,15 +553,15 @@ func scaleDownLimited(f *agonesv1.Fleet, gameServerLister listeragonesv1.GameSer
return replicas, true, nil
}

func scaleLimited(scale int, f *agonesv1.Fleet, gameServerLister listeragonesv1.GameServerLister,
func scaleLimited(scale int, f *agonesv1.Fleet, gameServerNamespacedLister listeragonesv1.GameServerNamespaceLister,
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,
return scaleDownLimited(f, gameServerNamespacedLister, nodeCounts, key, isCounter, replicas,
aggCapacity, maxCapacity)
case 0:
return replicas, false, nil
Expand Down Expand Up @@ -591,7 +592,7 @@ func scaleUp(replicas int32, capacity, count, aggCapacity, availableCapacity, ma
}

// scaleDown scales down for either Integer or Percentage Buffer.
func scaleDown(f *agonesv1.Fleet, gameServerLister listeragonesv1.GameServerLister,
func scaleDown(f *agonesv1.Fleet, gameServerNamespacedLister listeragonesv1.GameServerNamespaceLister,
nodeCounts map[string]gameservers.NodeCount, key string, isCounter bool, replicas int32,
aggCount, aggCapacity, minCapacity, buffer int64) (int32, bool, error) {
// Exit early if we're already at MinCapacity to avoid calling getSortedGameServers if unnecessary
Expand All @@ -601,7 +602,7 @@ func scaleDown(f *agonesv1.Fleet, gameServerLister listeragonesv1.GameServerList

// We first need to get the individual game servers in order of deletion on scale down, as any
// game server may have a unique value for counts and / or capacity.
gameServers, err := getSortedGameServers(f, gameServerLister, nodeCounts)
gameServers, err := getSortedGameServers(f, gameServerNamespacedLister, nodeCounts)
if err != nil {
return 0, false, err
}
Expand Down
Loading

0 comments on commit a30635d

Please sign in to comment.