Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Adds Counts and Lists AutoScale Policies #3211

Merged
merged 13 commits into from
Jul 20, 2023
Merged

Adds Counts and Lists AutoScale Policies #3211

merged 13 commits into from
Jul 20, 2023

Conversation

igooch
Copy link
Collaborator

@igooch igooch commented Jun 12, 2023 •
edited
Loading

What type of PR is this?

/kind feature

What this PR does / Why we need it:

Adds autoscaling for Counters and Lists.

  • Computes the desired number of replicas for the Fleet (scales up or down) based on either a CounterPolicy or ListPolicy with an Integer Buffer or a Percent Buffer.

Adds Priorities for sorting Counters and Lists to GameServerSet and Fleet CRDs.

  • GameServerSet Priorities are populated from the Priorities in the Fleet CRD.
  • Priority is for Game Server deletion on Fleet scale down.
  • Priority is considered for both Distributed and Packed strategies. (Only implemented for Packed strategy in this PR. To be implemented for strategy Distributed in a future PR).
  • Priority is only considered after sorting by Node in the Packed strategy.
  • The Priority first sorts by Capacity and then by Count (or number of Values in a List) if Capacity is equal.
  • If two game servers are on the same Node, the game server with the Counter or List denoted in the BufferPolicy will be marked for deletion first before a game server without the specified Counter or List.
  • This scale down logic is held in pkg/gameserversets/gameserversets.go as it could not go in common.go due to cyclical dependencies.
  • In order for the FleetAutoScaler to use this logic the FleetAutoScaler Controller pkg/fleetautoscalers/controller.go now has a PerNodeCounter and GameServerLister.

Which issue(s) this PR fixes:

Working on #2716

Special notes for your reviewer:

I moved the type Priority struct and its basic validation to common.go from gameserverallocation.go as this struct is used by Fleet, GameServerSet, and GameServerAllocation.

@igooch igooch force-pushed the arbitrary-counts-lists-2716 branch from bb35c05 to 7e2129e Compare June 12, 2023 16:06
@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 2db1b473-4e4b-4f04-9923-b80f5e6fcfce

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 028ead36-dea7-4f3e-8337-252bfe322520

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 23623171-38dc-4193-8af8-bd713bd5cf68

The following development artifacts have been built, and will exist for the next 30 days:

A preview of the website (the last 30 builds are retained):

To install this version:

  • git fetch https://github.com/googleforgames/agones.git pull/3211/head:pr_3211 && git checkout pr_3211
  • helm install agones ./install/helm/agones --namespace agones-system --set agones.image.registry=us-docker.pkg.dev/agones-images/ci --set agones.image.tag=1.33.0-2d0c135-amd64

@igooch igooch force-pushed the arbitrary-counts-lists-2716 branch from 2d0c135 to 7e2129e Compare June 14, 2023 17:44
@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 901bae19-9fd2-499a-8ba3-1f418ed057a4

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

@igooch igooch force-pushed the arbitrary-counts-lists-2716 branch from 19ba344 to ff52172 Compare June 14, 2023 19:15
@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 5259aa72-a096-47cf-8e49-3e8b6ac2da49

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

@igooch igooch force-pushed the arbitrary-counts-lists-2716 branch from ff52172 to 0735089 Compare June 14, 2023 19:31
@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 2eb7d421-49b1-4480-9cde-7819f36994df

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

@igooch igooch force-pushed the arbitrary-counts-lists-2716 branch from 0735089 to d98c687 Compare June 20, 2023 01:18
@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: b7bacc91-1257-46c5-9110-3cb35f197be5

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

@igooch igooch force-pushed the arbitrary-counts-lists-2716 branch from f7501e0 to 3366293 Compare June 20, 2023 02:19
@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: e3392881-f65b-418e-998f-e53ffb0741a9

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

@igooch igooch force-pushed the arbitrary-counts-lists-2716 branch from 3366293 to ada2c4d Compare June 20, 2023 02:42
@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 65956908-4dc4-4235-b902-574e7bf632f5

The following development artifacts have been built, and will exist for the next 30 days:

A preview of the website (the last 30 builds are retained):

To install this version:

  • git fetch https://github.com/googleforgames/agones.git pull/3211/head:pr_3211 && git checkout pr_3211
  • helm install agones ./install/helm/agones --namespace agones-system --set agones.image.registry=us-docker.pkg.dev/agones-images/ci --set agones.image.tag=1.33.0-dev-ada2c4d-amd64

@igooch igooch force-pushed the arbitrary-counts-lists-2716 branch from ada2c4d to ae66afe Compare June 26, 2023 05:39
@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: a4d7eb7e-3a02-4e01-ac57-73329122c07f

The following development artifacts have been built, and will exist for the next 30 days:

A preview of the website (the last 30 builds are retained):

To install this version:

  • git fetch https://github.com/googleforgames/agones.git pull/3211/head:pr_3211 && git checkout pr_3211
  • helm install agones ./install/helm/agones --namespace agones-system --set agones.image.registry=us-docker.pkg.dev/agones-images/ci --set agones.image.tag=1.33.0-dev-5721396-amd64

@igooch igooch marked this pull request as ready for review June 26, 2023 14:04
@igooch igooch force-pushed the arbitrary-counts-lists-2716 branch 2 times, most recently from de83109 to ff6f18f Compare June 26, 2023 15:58
@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 347ba71c-1a48-4502-a5d2-332deb0c07e8

The following development artifacts have been built, and will exist for the next 30 days:

A preview of the website (the last 30 builds are retained):

To install this version:

  • git fetch https://github.com/googleforgames/agones.git pull/3211/head:pr_3211 && git checkout pr_3211
  • helm install agones ./install/helm/agones --namespace agones-system --set agones.image.registry=us-docker.pkg.dev/agones-images/ci --set agones.image.tag=1.34.0-dev-c43f029-amd64

markmandel
markmandel reviewed Jul 18, 2023
View reviewed changes
Copy link
Member

@markmandel markmandel left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me! @gongmax since this is such a big PR, can you also a do a review pass as well before we merge this? Your extra eyes would be appreciated!

pkg/fleetautoscalers/fleetautoscalers.go Show resolved Hide resolved
pkg/fleetautoscalers/fleetautoscalers_test.go Show resolved Hide resolved
gongmax
gongmax approved these changes Jul 19, 2023
View reviewed changes
Copy link
Collaborator

@gongmax gongmax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor comments. But overall LGTM.

pkg/fleetautoscalers/fleetautoscalers.go Show resolved Hide resolved
pkg/fleetautoscalers/fleetautoscalers.go Outdated Show resolved Hide resolved
pkg/fleetautoscalers/fleetautoscalers.go Show resolved Hide resolved
pkg/gameserversets/gameserversets.go Show resolved Hide resolved
@google-oss-prow google-oss-prow bot added lgtm and removed lgtm labels Jul 19, 2023
@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: c9fe1380-2e3f-4d29-8306-37832d2c1ef4

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

igooch added 13 commits July 20, 2023 18:08
@igooch
Adds Counts and Lists AutoScale Policies
9d067a4
@igooch
Adds Priorities to Fleet CRD
b66cfe1 
Moves existing Priority from GameServerAllocation to Commmon so that Priority can be used by both GameServerAllocation and Fleet
@igooch
Add Priorities to GameServerSet
2c8e804
@igooch
Adds edge and test cases to applyCounterOrListPolicy
89c2ca5
@igooch
Changes Counter and List to enums Fleet in CRD
76240b3 
Changes Ascending and Descending to enum in fleet CRD
Removes webhook validation of these fields as they are now enums
Changes name from PriorityType to Type to be consistent with Agones and K8s naming
@igooch
Minor changes to comments
1696723
@igooch
Minor change to remove an unnecessary line of code
3cd4357
@igooch
Minor change to deep copy the fleet priorities when creating a game s…
8f97664 
…erver set
@igooch
Cleans up some TODO comments
9123acf
@igooch
Adds Aggregated Count and Capacity for Allocated Gameservers only. Op…
77407fa 
…ens up with Aggregated Count for all statuses besides IsBeingDeleted instead of only Ready, Allocated, and Reserved.
@igooch
Generated Files
45afdb0
@igooch
Changes Scale by Percent to determine desired buffer based soley on A…
e999868 
…llocated Counts
@igooch
Minor change to remove an unnecessary check
a3661d4
@igooch igooch force-pushed the arbitrary-counts-lists-2716 branch from 2709a75 to a3661d4 Compare July 20, 2023 18:09
@google-oss-prow google-oss-prow bot added the lgtm label Jul 20, 2023
@google-oss-prow
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gongmax, igooch, markmandel

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: c484408d-7fc8-4095-a16c-7dcd3e353706

The following development artifacts have been built, and will exist for the next 30 days:

A preview of the website (the last 30 builds are retained):

To install this version:

  • git fetch https://github.com/googleforgames/agones.git pull/3211/head:pr_3211 && git checkout pr_3211
  • helm install agones ./install/helm/agones --namespace agones-system --set agones.image.registry=us-docker.pkg.dev/agones-images/ci --set agones.image.tag=1.34.0-dev-a3661d4-amd64

@markmandel markmandel merged commit bd53cd4 into googleforgames:main Jul 20, 2023
2 checks passed
@roberthbailey
Copy link
Member

🎉

@igooch igooch mentioned this pull request Jan 18, 2024
Closed
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gongmax gongmax gongmax approved these changes

@markmandel markmandel markmandel approved these changes

Assignees

@markmandel markmandel

@gongmax gongmax

Labels
approved kind/feature New features for Agones lgtm size/XXL
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@igooch @agones-bot @roberthbailey @markmandel @gongmax