Improved fleetautoscalers - fleetautoscalers_test.go unit tests + applyWebhookPolicy refactoring #1531

akremsa · 2020-05-08T14:02:51Z

What type of PR is this?

/kind cleanup

What this PR does / Why we need it:
Added missing unit tests to TestApplyWebhookPolicy and refactored applyWebhookPolicy method.
Added an absent error check after b, err := json.Marshal(faReq) which could be a potential bug.
Refactored TestComputeDesiredFleetSize and TestApplyBufferPolicy and added new test cases.

Special notes for your reviewer:
Before:

After:

By the way, I suggest using url.ParseRequestURI instead of url.Parse.

aLekSer · 2020-05-08T14:06:10Z

pkg/fleetautoscalers/fleetautoscalers.go

-		if w.Service.Path != nil {
-			servicePath = *w.Service.Path
+
+	if w.URL != nil && w.Service != nil {


👍 additional check would be great. Validated on Update but still something wrong could happen.

Agree, taking into account the fact that another validation happens in a different package.

agones-bot · 2020-05-08T14:22:44Z

Build Failed 😱

Build Id: 844f2438-299c-4881-8cf7-16bba813a284

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

agones-bot · 2020-05-08T15:03:46Z

Build Failed 😱

Build Id: 041001a9-ea5f-41a0-89f2-04673a1dcc88

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

agones-bot · 2020-05-08T23:19:15Z

Build Failed 😱

Build Id: d1007c0a-4537-45d5-b11c-3e9005b711b6

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

agones-bot · 2020-05-09T00:06:34Z

Build Failed 😱

Build Id: b866e474-b138-4493-a79a-da7c67692885

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

agones-bot · 2020-05-09T00:47:03Z

Build Succeeded 👏

Build Id: b9784537-393c-45f5-af58-b7834e9e9303

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

image: gcr.io/agones-images/agones-controller:1.6.0-3523987
image: gcr.io/agones-images/agones-sdk:1.6.0-3523987
image: gcr.io/agones-images/agones-ping:1.6.0-3523987
Linux C++ SDK (build): agonessdk-1.6.0-3523987-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.6.0-3523987.zip

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

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

To install this version:

git fetch https://github.com/GoogleCloudPlatform/agones.git pull/1531/head:pr_1531 && git checkout pr_1531
helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.6.0-3523987

agones-bot · 2020-05-11T09:53:01Z

Build Succeeded 👏

Build Id: f1e91a9b-7572-435d-a04b-438c68041fd7

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

image: gcr.io/agones-images/agones-controller:1.6.0-5de2cf0
image: gcr.io/agones-images/agones-sdk:1.6.0-5de2cf0
image: gcr.io/agones-images/agones-ping:1.6.0-5de2cf0
Linux C++ SDK (build): agonessdk-1.6.0-5de2cf0-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.6.0-5de2cf0.zip

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

https://5de2cf0-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/GoogleCloudPlatform/agones.git pull/1531/head:pr_1531 && git checkout pr_1531
helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.6.0-5de2cf0

agones-bot · 2020-05-11T11:12:49Z

Build Succeeded 👏

Build Id: 725f0237-7f6e-4331-9ef7-d179b107072b

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

image: gcr.io/agones-images/agones-controller:1.6.0-465adf4
image: gcr.io/agones-images/agones-sdk:1.6.0-465adf4
image: gcr.io/agones-images/agones-ping:1.6.0-465adf4
Linux C++ SDK (build): agonessdk-1.6.0-465adf4-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.6.0-465adf4.zip

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

https://465adf4-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/GoogleCloudPlatform/agones.git pull/1531/head:pr_1531 && git checkout pr_1531
helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.6.0-465adf4

agones-bot · 2020-05-11T13:02:09Z

Build Failed 😱

Build Id: d20a5fcf-347f-4932-b59a-8c8c3261498b

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

agones-bot · 2020-05-11T13:20:38Z

Build Failed 😱

Build Id: 9f99225f-13e1-4c00-911a-5b42b55e5212

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

pkg/fleetautoscalers/fleetautoscalers.go

agones-bot · 2020-05-12T15:20:20Z

Build Failed 😱

Build Id: cf961221-8a4f-4eb8-8b9a-5de92468c5a5

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

agones-bot · 2020-05-14T12:01:43Z

Build Failed 😱

Build Id: fa3521c7-7519-41c1-96d8-c2fc73fec90c

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

agones-bot · 2020-05-15T12:44:06Z

Build Failed 😱

Build Id: 851235fe-2a39-4b35-bfb7-d83a8019dd72

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

pkg/fleetautoscalers/fleetautoscalers.go

pkg/fleetautoscalers/fleetautoscalers_test.go

pkg/fleetautoscalers/fleetautoscalers.go

pkg/fleetautoscalers/fleetautoscalers_test.go

aLekSer

@akremsa Alexey, please add nil check to service and this PR is good to go. I will double check that nothing is broken, if we follow the guide here :
https://agones.dev/site/docs/getting-started/create-fleetautoscaler/

akremsa · 2020-06-08T22:46:21Z

@aLekSer done

added nil fleet case fix URL fix

fix

fix fix

added more tests linter fix

agones-bot · 2020-06-09T11:54:02Z

Build Succeeded 👏

Build Id: 2224389a-34f6-4b46-8ef8-f533eceac619

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

image: gcr.io/agones-images/agones-controller:1.7.0-3fcde1f
image: gcr.io/agones-images/agones-sdk:1.7.0-3fcde1f
image: gcr.io/agones-images/agones-ping:1.7.0-3fcde1f
Linux C++ SDK (build): agonessdk-1.7.0-3fcde1f-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.7.0-3fcde1f.zip

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

https://3fcde1f-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/GoogleCloudPlatform/agones.git pull/1531/head:pr_1531 && git checkout pr_1531
helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.7.0-3fcde1f

aLekSer

Thanks for this changes.

google-oss-robot · 2020-06-10T11:34:59Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: akremsa, aLekSer
To complete the pull request process, please assign markmandel
You can assign the PR to them by writing /assign @markmandel in a comment when ready.

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

Needs approval from an approver in each of these files:

OWNERS

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

…lyWebhookPolicy refactoring (googleforgames#1531)

google-oss-robot requested review from dzlier-gcp and pooneh-m May 8, 2020 14:02

google-oss-robot added the size/L label May 8, 2020

aLekSer reviewed May 8, 2020

View reviewed changes

akremsa force-pushed the fleetautoscalers_unit_tests branch from 0dd3cec to 3630834 Compare May 8, 2020 14:44

akremsa force-pushed the fleetautoscalers_unit_tests branch from 3630834 to 6183853 Compare May 8, 2020 23:00

akremsa force-pushed the fleetautoscalers_unit_tests branch from 6183853 to 5f60476 Compare May 8, 2020 23:45

akremsa force-pushed the fleetautoscalers_unit_tests branch from 5f60476 to 3523987 Compare May 9, 2020 00:29

akremsa force-pushed the fleetautoscalers_unit_tests branch from 3523987 to 5de2cf0 Compare May 11, 2020 09:34

akremsa changed the title ~~Improved fleetautoscalers - TestApplyWebhookPolicy unit tests + applyWebhookPolicy refactoring~~ Improved fleetautoscalers - fleetautoscalers_test.go unit tests + applyWebhookPolicy refactoring May 11, 2020

google-oss-robot added size/XL and removed size/L labels May 11, 2020

akremsa force-pushed the fleetautoscalers_unit_tests branch from 1378040 to f2cb6bd Compare May 11, 2020 13:04

aLekSer reviewed May 11, 2020

View reviewed changes

pkg/fleetautoscalers/fleetautoscalers.go Outdated Show resolved Hide resolved

aLekSer reviewed May 11, 2020

View reviewed changes

pkg/fleetautoscalers/fleetautoscalers.go Outdated Show resolved Hide resolved

akremsa force-pushed the fleetautoscalers_unit_tests branch from f2cb6bd to 6239f90 Compare May 12, 2020 15:03

akremsa force-pushed the fleetautoscalers_unit_tests branch from 6239f90 to d9ee2ba Compare May 14, 2020 11:58

akremsa force-pushed the fleetautoscalers_unit_tests branch from d9ee2ba to c303f1c Compare May 15, 2020 12:33

aLekSer reviewed Jun 8, 2020

View reviewed changes

pkg/fleetautoscalers/fleetautoscalers.go Outdated Show resolved Hide resolved

aLekSer reviewed Jun 8, 2020

View reviewed changes

pkg/fleetautoscalers/fleetautoscalers.go Outdated Show resolved Hide resolved

aLekSer reviewed Jun 8, 2020

View reviewed changes

pkg/fleetautoscalers/fleetautoscalers.go Outdated Show resolved Hide resolved

aLekSer reviewed Jun 8, 2020

View reviewed changes

pkg/fleetautoscalers/fleetautoscalers_test.go Outdated Show resolved Hide resolved

aLekSer reviewed Jun 8, 2020

View reviewed changes

pkg/fleetautoscalers/fleetautoscalers.go Show resolved Hide resolved

aLekSer reviewed Jun 8, 2020

View reviewed changes

pkg/fleetautoscalers/fleetautoscalers_test.go Show resolved Hide resolved

aLekSer reviewed Jun 8, 2020

View reviewed changes

pkg/fleetautoscalers/fleetautoscalers_test.go Show resolved Hide resolved

aLekSer suggested changes Jun 8, 2020

View reviewed changes

akremsa force-pushed the fleetautoscalers_unit_tests branch from fa6e359 to 139140e Compare June 8, 2020 22:45

akremsa requested a review from aLekSer June 8, 2020 22:46

alexey-kremsa-globant added 9 commits June 9, 2020 14:32

Made TestApplyWebhookPolicy to be table-driven

fce11ec

refactored applyWebhookPolicy

6e4a4e7

Added more unit tests and a couple of fixes in fleetautoscalers

979cd9d

added nil fleet case fix URL fix

added test to check a generated URL

1d37baf

fix

refactored TestComputeDesiredFleetSize and TestApplyBufferPolicy

32ca582

fix fix

split up applyWebhookPolicy function

bbc0391

buildURLFromWebhookPolicy refactoring - update

2cad9e5

fixed failing tests according to golang 1.14.4 version

a63c24b

added more tests linter fix

add w.Service nil check

3fcde1f

akremsa force-pushed the fleetautoscalers_unit_tests branch from 139140e to 3fcde1f Compare June 9, 2020 11:32

aLekSer approved these changes Jun 10, 2020

View reviewed changes

google-oss-robot added the lgtm label Jun 10, 2020

pooneh-m merged commit 48013e6 into googleforgames:master Jun 10, 2020

markmandel added this to the 1.7.0 milestone Jun 30, 2020

markmandel added area/tests Unit tests, e2e tests, anything to make sure things don't break kind/cleanup Refactoring code, fixing up documentation, etc labels Jun 30, 2020

ilkercelikyilmaz pushed a commit to ilkercelikyilmaz/agones that referenced this pull request Oct 23, 2020

Improved fleetautoscalers - fleetautoscalers_test.go unit tests + app…

af20add

…lyWebhookPolicy refactoring (googleforgames#1531)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improved fleetautoscalers - fleetautoscalers_test.go unit tests + applyWebhookPolicy refactoring #1531

Improved fleetautoscalers - fleetautoscalers_test.go unit tests + applyWebhookPolicy refactoring #1531

akremsa commented May 8, 2020 •

edited

Loading

aLekSer May 8, 2020

akremsa May 8, 2020

agones-bot commented May 8, 2020

agones-bot commented May 8, 2020

agones-bot commented May 8, 2020

agones-bot commented May 9, 2020

agones-bot commented May 9, 2020

agones-bot commented May 11, 2020

agones-bot commented May 11, 2020

agones-bot commented May 11, 2020

agones-bot commented May 11, 2020

agones-bot commented May 12, 2020

agones-bot commented May 14, 2020

agones-bot commented May 15, 2020

aLekSer left a comment

akremsa commented Jun 8, 2020

agones-bot commented Jun 9, 2020

aLekSer left a comment

google-oss-robot commented Jun 10, 2020

Improved fleetautoscalers - fleetautoscalers_test.go unit tests + applyWebhookPolicy refactoring #1531

Improved fleetautoscalers - fleetautoscalers_test.go unit tests + applyWebhookPolicy refactoring #1531

Conversation

akremsa commented May 8, 2020 • edited Loading

aLekSer May 8, 2020

Choose a reason for hiding this comment

akremsa May 8, 2020

Choose a reason for hiding this comment

agones-bot commented May 8, 2020

agones-bot commented May 8, 2020

agones-bot commented May 8, 2020

agones-bot commented May 9, 2020

agones-bot commented May 9, 2020

agones-bot commented May 11, 2020

agones-bot commented May 11, 2020

agones-bot commented May 11, 2020

agones-bot commented May 11, 2020

agones-bot commented May 12, 2020

agones-bot commented May 14, 2020

agones-bot commented May 15, 2020

aLekSer left a comment

Choose a reason for hiding this comment

akremsa commented Jun 8, 2020

agones-bot commented Jun 9, 2020

aLekSer left a comment

Choose a reason for hiding this comment

google-oss-robot commented Jun 10, 2020

akremsa commented May 8, 2020 •

edited

Loading