Tests update: portallocator_test #1723

akremsa · 2020-07-29T15:28:38Z

What type of PR is this?

/kind bug
/kind cleanup

What this PR does / Why we need it:
Made several fixes in test cases, abort tests in proper places using require package.

agones-bot · 2020-07-29T15:35:34Z

Build Failed 😱

Build Id: 49c86e86-a519-4b35-8f36-1ee035280a1c

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

agones-bot · 2020-07-29T16:02:57Z

Build Succeeded 👏

Build Id: e625c55e-8e89-455a-8bf5-33567879c545

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

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

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

https://95d2409-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/1723/head:pr_1723 && git checkout pr_1723
helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.8.0-95d2409

aLekSer · 2020-07-29T16:53:34Z

pkg/gameservers/portallocator.go

@@ -106,7 +106,6 @@ func (pa *PortAllocator) Run(stop <-chan struct{}) error {
 }

 // Allocate assigns a port to the GameServer and returns it.
-// Return ErrPortNotFound if no port is allocatable


I have just simulated what would be when we don't have enough free ports. In current version - it would be actually a warning, so error is redundant in that case:

11m Warning FailedScheduling pod/simple-udp-qbx79-c2vll 0/6 nodes are available: 2 node(s) had taints that the pod didn't tolerate, 4 node(s) didn't have free ports for the requested pod ports.

aLekSer · 2020-07-29T19:19:11Z

pkg/gameservers/portallocator_test.go

-		maxPort := int32(19) // make sure we have an even number
-		pa := NewPortAllocator(10, maxPort, m.KubeInformerFactory, m.AgonesInformerFactory)
+		minPort := int32(10)
+		maxPort := int32(20) // make sure we have an even number


Suggested change

maxPort := int32(20) // make sure we have an even number

maxPort := int32(19) // make sure we have an even number

Now we have odd number of ports. Not sure what we should change this comment or set back the number and consequently other comments with 20.

I thought this comment is related to maxPort that is why I’ve changed it.

Outside of this comment, I think this PR is good to merge?

Yes.
Are you aware of a context of this comment?
If we all are unsure about it I think we can remove one and merge.

Looks like I wrote it.... and I can't remember anymore 😕

pkg/gameservers/portallocator_test.go

agones-bot · 2020-07-30T13:49:12Z

Build Succeeded 👏

Build Id: 94459b93-d580-4c61-a367-0b4cb231f047

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

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

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

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

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/1723/head:pr_1723 && git checkout pr_1723
helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.8.0-c2a073e

fix applied review notes removed comment fix

agones-bot · 2020-08-04T20:05:49Z

Build Succeeded 👏

Build Id: 3d0652c1-d9f6-468b-b70f-41f065342f9b

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

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

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

https://9ace5c5-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/1723/head:pr_1723 && git checkout pr_1723
helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.8.0-9ace5c5

aLekSer

Fine with me

google-oss-robot · 2020-08-04T20:22:03Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: akremsa, aLekSer, 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:

~~OWNERS~~ [markmandel]

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

agones-bot · 2020-08-04T20:26:22Z

Build Succeeded 👏

Build Id: 37b4b9c6-dfe6-4110-bfdc-451d351dbe1e

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

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

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

https://951f589-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/1723/head:pr_1723 && git checkout pr_1723
helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.8.0-951f589

google-oss-robot · 2020-08-04T20:30:50Z

New changes are detected. LGTM label has been removed.

agones-bot · 2020-08-04T20:49:36Z

Build Succeeded 👏

Build Id: 25468fc6-154d-455f-be65-276d03b7341e

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

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

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

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

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/1723/head:pr_1723 && git checkout pr_1723
helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.8.0-a7af0e9

fix applied review notes removed comment fix Co-authored-by: Mark Mandel <markmandel@google.com>

google-oss-robot requested review from dzlier-gcp and markmandel July 29, 2020 15:28

google-oss-robot added the size/M label Jul 29, 2020

googlebot added the cla: yes label Jul 29, 2020

akremsa changed the title ~~Tests update: portallocator_еуые~~ Tests update: portallocator_test Jul 29, 2020

akremsa force-pushed the portallocator_tests branch from 5b5deb8 to 95d2409 Compare July 29, 2020 15:43

google-oss-robot added size/L and removed size/M labels Jul 29, 2020

aLekSer reviewed Jul 29, 2020

View reviewed changes

aLekSer reviewed Jul 30, 2020

View reviewed changes

pkg/gameservers/portallocator_test.go Outdated Show resolved Hide resolved

aLekSer reviewed Jul 30, 2020

View reviewed changes

pkg/gameservers/portallocator_test.go Outdated Show resolved Hide resolved

akremsa force-pushed the portallocator_tests branch from 95d2409 to c2a073e Compare July 30, 2020 13:18

akremsa force-pushed the portallocator_tests branch from c2a073e to 9ace5c5 Compare August 4, 2020 19:47

portallocator tests fix

951f589

fix applied review notes removed comment fix

akremsa force-pushed the portallocator_tests branch from 9ace5c5 to 951f589 Compare August 4, 2020 19:50

markmandel approved these changes Aug 4, 2020

View reviewed changes

google-oss-robot assigned markmandel Aug 4, 2020

google-oss-robot added lgtm approved labels Aug 4, 2020

aLekSer approved these changes Aug 4, 2020

View reviewed changes

google-oss-robot assigned aLekSer Aug 4, 2020

Merge branch 'master' into portallocator_tests

a7af0e9

google-oss-robot removed the lgtm label Aug 4, 2020

aLekSer merged commit 79006ee into googleforgames:master Aug 4, 2020

markmandel added this to the 1.8.0 milestone Aug 11, 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 Aug 11, 2020

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

portallocator tests fix (googleforgames#1723)

70bc793

fix applied review notes removed comment fix Co-authored-by: Mark Mandel <markmandel@google.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tests update: portallocator_test #1723

Tests update: portallocator_test #1723

akremsa commented Jul 29, 2020 •

edited

Loading

agones-bot commented Jul 29, 2020

agones-bot commented Jul 29, 2020

aLekSer Jul 29, 2020 •

edited

Loading

aLekSer Jul 29, 2020

akremsa Jul 29, 2020

markmandel Aug 4, 2020

akremsa Aug 4, 2020

markmandel Aug 4, 2020

akremsa Aug 4, 2020

agones-bot commented Jul 30, 2020

agones-bot commented Aug 4, 2020

aLekSer left a comment

google-oss-robot commented Aug 4, 2020

agones-bot commented Aug 4, 2020

google-oss-robot commented Aug 4, 2020

agones-bot commented Aug 4, 2020

	maxPort := int32(20) // make sure we have an even number
	maxPort := int32(19) // make sure we have an even number

Tests update: portallocator_test #1723

Tests update: portallocator_test #1723

Conversation

akremsa commented Jul 29, 2020 • edited Loading

agones-bot commented Jul 29, 2020

agones-bot commented Jul 29, 2020

aLekSer Jul 29, 2020 • edited Loading

Choose a reason for hiding this comment

aLekSer Jul 29, 2020

Choose a reason for hiding this comment

akremsa Jul 29, 2020

Choose a reason for hiding this comment

markmandel Aug 4, 2020

Choose a reason for hiding this comment

akremsa Aug 4, 2020

Choose a reason for hiding this comment

markmandel Aug 4, 2020

Choose a reason for hiding this comment

akremsa Aug 4, 2020

Choose a reason for hiding this comment

agones-bot commented Jul 30, 2020

agones-bot commented Aug 4, 2020

aLekSer left a comment

Choose a reason for hiding this comment

google-oss-robot commented Aug 4, 2020

agones-bot commented Aug 4, 2020

google-oss-robot commented Aug 4, 2020

agones-bot commented Aug 4, 2020

akremsa commented Jul 29, 2020 •

edited

Loading

aLekSer Jul 29, 2020 •

edited

Loading