Set Minimum Buffer Size to 1 by Kalaiselvi84 · Pull Request #3749 · agones-dev/agones

Kalaiselvi84 · 2024-04-09T00:01:40Z

What type of PR is this?

Uncomment only one /kind <> line, press enter to put that in a new line, and remove leading whitespace from that line:

/kind breaking

/kind bug

/kind cleanup
/kind documentation
/kind feature
/kind hotfix
/kind release

What this PR does / Why we need it:

Which issue(s) this PR fixes:

Closes #3715

Special notes for your reviewer:

agones-bot · 2024-04-09T00:10:12Z

Build Failed 😱

Build Id: 5c6745d8-e9bd-4b46-bce8-02c4a14caa71

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

agones-bot · 2024-04-09T00:37:19Z

Build Failed 😱

Build Id: bbb1c989-426c-4122-9b0b-cd9466a83b07

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

agones-bot · 2024-04-11T03:32:46Z

Build Succeeded 👏

Build Id: 712da535-b430-42c8-8ca3-17b7984c5428

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):

https://e5fb5e2-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/3749/head:pr_3749 && git checkout pr_3749
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.40.0-dev-e5fb5e2-amd64

agones-bot · 2024-04-11T18:13:23Z

Build Failed 😱

Build Id: 66436d88-6521-44c3-9c79-607057d80e29

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

agones-bot · 2024-04-11T19:54:35Z

Build Failed 😱

Build Id: 7eb17ccc-c585-4e1a-9046-ccddc3fba753

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

markmandel · 2024-04-16T23:01:09Z

@igooch is there any e2e tests here to update that would also need to be updated?

agones-bot · 2024-04-17T00:12:28Z

Build Succeeded 👏

Build Id: d91f7765-28d9-4b1c-b3f6-ded9c6f48d62

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):

https://2fc8bad-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/3749/head:pr_3749 && git checkout pr_3749
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.40.0-dev-2fc8bad-amd64

agones-bot · 2024-04-18T00:25:15Z

Build Succeeded 👏

Build Id: 34fd4157-9785-4272-b92f-fb6033506da4

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):

https://78d2675-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/3749/head:pr_3749 && git checkout pr_3749
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.40.0-dev-78d2675-amd64

agones-bot · 2024-04-18T19:22:41Z

Build Failed 😱

Build Id: 02453ae0-b466-4e72-b307-7638dd5acfcc

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

agones-bot · 2024-04-19T03:29:57Z

Build Succeeded 👏

Build Id: 7d8aa50a-6335-4459-bdd6-04942ecff669

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):

https://451b4f8-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/3749/head:pr_3749 && git checkout pr_3749
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.40.0-dev-451b4f8-amd64

agones-bot · 2024-04-24T18:34:35Z

Build Succeeded 👏

Build Id: 9463b6c3-1d29-438b-ac92-734a48f801ef

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):

https://ec72f7f-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/3749/head:pr_3749 && git checkout pr_3749
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.41.0-dev-ec72f7f-amd64

agones-bot · 2024-05-02T01:26:58Z

Build Succeeded 👏

Build Id: 204267c3-cfd9-4cb1-8c57-4c07d9361cf0

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):

https://f98e459-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/3749/head:pr_3749 && git checkout pr_3749
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.41.0-dev-f98e459-amd64

igooch

On about line 322 I'd recommend doing something like:

		// If the Aggregated Allocated Counts is 0 then desired capacity gets calculated as 0. If the
		// capacity of 1 replica is equal to or greater than minimum capacity we can exit early.
		if aggAllocatedCount <= 0 && capacity >= minCapacity {
			return 1, true, nil
		}

It's a bit convoluted, but it will get us the desired result. I think this takes care of all the edge cases, but if you can think of any others let me know.

igooch · 2024-05-09T06:52:05Z

+		if replicas == 1 {
+			return 1, true, nil
+		}


Suggested change

if replicas == 1 {

return 1, true, nil

}

This doesn't quite work, because there are cases where scaling down to 1 replica is desired (not limited). This is the case the test case "scale down".

I have rolled back all the changes from the scaleDown method and modified the code as per your suggestion. I adjusted the TestApplyCounterPolicy/scale down test case to not be limited: limited: false. Both the TestApplyCounterPolicy and TestApplyListPolicy tests have passed.

agones-bot · 2024-05-09T18:19:46Z

Build Succeeded 👏

Build Id: 429b7ce6-718e-4849-8d6c-0ae510f2acd6

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):

https://1b4f2b9-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/3749/head:pr_3749 && git checkout pr_3749
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.41.0-dev-1b4f2b9-amd64

igooch

LGTM

Set Minimums for DesiredCapacity and Buffer to 1

61173f3

Kalaiselvi84 requested a review from markmandel April 9, 2024 00:01

github-actions Bot added kind/bug These are bugs. size/S labels Apr 9, 2024

fix lint

9649961

markmandel requested a review from igooch April 10, 2024 18:06

Kalaiselvi84 added 2 commits April 11, 2024 02:19

Fix TestApplyListPolicy

9ae00f1

Merge branch 'main' into i_3715

e5fb5e2

igooch reviewed Apr 11, 2024

View reviewed changes

Comment thread pkg/fleetautoscalers/fleetautoscalers.go Outdated

Comment thread pkg/fleetautoscalers/fleetautoscalers_test.go

review changes

2982cf5

Kalaiselvi84 changed the title ~~Set Minimums for DesiredCapacity and Buffer to 1~~ Set Minimum Buffer Size to 1 Apr 11, 2024

Merge branch 'main' into i_3715

2fc8bad

Merge branch 'main' into i_3715

78d2675

igooch reviewed Apr 18, 2024

View reviewed changes

Comment thread pkg/fleetautoscalers/fleetautoscalers_test.go Outdated

Prevent scaling down to zero replicas in scaleDown logic

615d377

modify TestApplyCounterPolicy

451b4f8

igooch reviewed Apr 19, 2024

View reviewed changes

Comment thread pkg/fleetautoscalers/fleetautoscalers.go Outdated

markmandel reviewed Apr 19, 2024

View reviewed changes

Comment thread pkg/fleetautoscalers/fleetautoscalers.go Outdated

Kalaiselvi84 added 2 commits April 19, 2024 19:40

remove number of replicas check

956194a

modified scaleDown method and used helper function

1115b59

Merge branch 'main' into i_3715

ec72f7f

Kalaiselvi84 commented Apr 24, 2024

View reviewed changes

Comment thread pkg/fleetautoscalers/fleetautoscalers.go Outdated

Merge branch 'main' into i_3715

f98e459

igooch reviewed May 9, 2024

View reviewed changes

Kalaiselvi84 added 2 commits May 9, 2024 17:07

suggested changes

8b61640

Merge branch 'main' into i_3715

1b4f2b9

igooch approved these changes May 9, 2024

View reviewed changes

igooch merged commit 8749377 into agones-dev:main May 9, 2024

Conversation

Kalaiselvi84 commented Apr 9, 2024

Uh oh!

agones-bot commented Apr 9, 2024

Uh oh!

agones-bot commented Apr 9, 2024

Uh oh!

agones-bot commented Apr 11, 2024

Uh oh!

Uh oh!

Uh oh!

agones-bot commented Apr 11, 2024

Uh oh!

agones-bot commented Apr 11, 2024

Uh oh!

markmandel commented Apr 16, 2024

Uh oh!

agones-bot commented Apr 17, 2024

Uh oh!

agones-bot commented Apr 18, 2024

Uh oh!

Uh oh!

agones-bot commented Apr 18, 2024

Uh oh!

agones-bot commented Apr 19, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

agones-bot commented Apr 24, 2024

Uh oh!

agones-bot commented May 2, 2024

Uh oh!

igooch left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

igooch May 9, 2024

Choose a reason for hiding this comment

Uh oh!

Kalaiselvi84 May 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

agones-bot commented May 9, 2024

Uh oh!

igooch left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Kalaiselvi84 May 9, 2024 •

edited

Loading