Pass port into autoscaler url from webhook policy #1765

andrewgrundy · 2020-08-19T15:26:39Z

What type of PR is this?

Uncomment only one /kind <> line, hit 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

What this PR does / Why we need it:
The port in the URL created by createURL was hardcoded to 8000 rather than the port specified in the WebhookPolicy.
I have made the port default to 8000 when a port is not given and added a test to check the URL produced is correct when a port is given.

Which issue(s) this PR fixes:

Closes #

Special notes for your reviewer:

The port in the URL created by createURL was hardcoded to 8000 rather than the port specified in the WebhookPolicy. I have made the port default to 8000 when a port is not given and added a test to check the URL produced is correct when a port is given.

googlebot · 2020-08-19T15:26:43Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

agones-bot · 2020-08-19T15:49:21Z

Build Succeeded 👏

Build Id: 0f50c0c8-347e-4050-801b-0fc62312c002

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

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

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

https://51eca79-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/1765/head:pr_1765 && git checkout pr_1765
helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.9.0-51eca79

pkg/fleetautoscalers/fleetautoscalers.go

aLekSer

Overall is great, but please update document file:

agones/site/content/en/docs/Reference/fleetautoscaler.md

Line 94 in 1474c20

    
           - `path` is an optional URL path which will be sent in any request to this service. (i. e. /scale)

Add port value description.

aLekSer · 2020-08-20T06:07:50Z

Thanks for this Pull Request, it is definitely should be merged soon, please update docs file and sign the CLA.

agones-bot · 2020-08-20T09:40:33Z

Build Succeeded 👏

Build Id: 904d246f-92f9-420d-84d2-578a79952227

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

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

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

https://076a137-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/1765/head:pr_1765 && git checkout pr_1765
helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.9.0-076a137

aLekSer

Thanks for the update.

google-oss-robot · 2020-08-20T13:34:36Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: aLekSer, andrewgrundy
To complete the pull request process, please assign ericfortin
You can assign the PR to them by writing /assign @ericfortin 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

aLekSer · 2020-08-20T13:35:18Z

📝 Please visit https://cla.developers.google.com/ to sign.

roberthbailey · 2020-08-21T21:13:34Z

site/content/en/docs/Reference/fleetautoscaler.md

@@ -92,6 +92,7 @@ The `spec` field is the actual `FleetAutoscaler` specification and it is compose
      - `namespace` is the kubernetes namespace where webhook is deployed. Optional
                      If not specified, the "default" would be used
      - `path` is an optional URL path which will be sent in any request to this service. (i. e. /scale)
+      - `port` is optional, it is the port for the service which is hosting the webhook. The default is 8000 for backward compatibility. If given, it should be a valid port number (1-65535, inclusive).


Changes to the website should be guarded so that they get exposed on the next release.

See https://agones.dev/site/docs/contribute/#within-a-page

aLekSer · 2020-08-24T13:53:46Z

site/content/en/docs/Reference/fleetautoscaler.md

@@ -92,6 +92,7 @@ The `spec` field is the actual `FleetAutoscaler` specification and it is compose
      - `namespace` is the kubernetes namespace where webhook is deployed. Optional
                      If not specified, the "default" would be used
      - `path` is an optional URL path which will be sent in any request to this service. (i. e. /scale)
+      - `port` is optional, it is the port for the service which is hosting the webhook. The default is 8000 for backward compatibility. If given, it should be a valid port number (1-65535, inclusive).


Basically this way:

Suggested change

- `port` is optional, it is the port for the service which is hosting the webhook. The default is 8000 for backward compatibility. If given, it should be a valid port number (1-65535, inclusive).

{{% feature publishVersion="1.9.0" %}}

- `port` is optional, it is the port for the service which is hosting the webhook. The default is 8000 for backward compatibility. If given, it should be a valid port number (1-65535, inclusive).

{{% /feature %}}

Looks like this might be the last thing before we can merge.

(Confirmed with @thisisnotapril that we can manually approve CLA) @andrewgrundy, you good to make the change?

googlebot · 2020-08-25T20:29:55Z

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

ℹ️ Googlers: Go here for more info.

googlebot · 2020-08-25T21:45:07Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

…documentation

google-oss-robot · 2020-08-26T16:56:57Z

New changes are detected. LGTM label has been removed.

agones-bot · 2020-08-26T17:14:10Z

Build Failed 😱

Build Id: 55877946-8dc0-48de-b294-9505775e55a8

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

markmandel · 2020-08-26T17:23:17Z

Oops, I think you got caught as I was updating the e2e cluster. Let's try that again!

agones-bot · 2020-08-26T17:39:33Z

Build Failed 😱

Build Id: 4106924a-ff00-4817-97ae-5b1d37c0fdd0

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

markmandel · 2020-08-26T17:41:32Z

sdk-conformance flakiness 😢

googlebot · 2020-08-26T17:41:49Z

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

ℹ️ Googlers: Go here for more info.

agones-bot · 2020-08-26T18:00:24Z

Build Succeeded 👏

Build Id: d524d3c4-8ff2-40ba-9446-8dee8ec3857c

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

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

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

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

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/1765/head:pr_1765 && git checkout pr_1765
helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.9.0-f28bbe8

* Pass port into autoscaler url from webhook policy The port in the URL created by createURL was hardcoded to 8000 rather than the port specified in the WebhookPolicy. I have made the port default to 8000 when a port is not given and added a test to check the URL produced is correct when a port is given. * Adding port description to FleetAutoscaler documentation * Adding a feature flag around port description in the FleetAutoscaler documentation Co-authored-by: Mark Mandel <[email protected]>

googlebot added the cla: no label Aug 19, 2020

google-oss-robot requested review from markmandel and roberthbailey August 19, 2020 15:26

google-oss-robot added the size/S label Aug 19, 2020

markmandel requested a review from aLekSer August 19, 2020 16:05

markmandel reviewed Aug 19, 2020

View reviewed changes

pkg/fleetautoscalers/fleetautoscalers.go Show resolved Hide resolved

aLekSer suggested changes Aug 20, 2020

View reviewed changes

google-oss-robot assigned aLekSer Aug 20, 2020

Adding port description to FleetAutoscaler documentation

076a137

aLekSer approved these changes Aug 20, 2020

View reviewed changes

google-oss-robot added the lgtm label Aug 20, 2020

roberthbailey reviewed Aug 21, 2020

View reviewed changes

aLekSer reviewed Aug 24, 2020

View reviewed changes

thisisnotapril added cla: yes and removed cla: no labels Aug 25, 2020

googlebot added cla: no and removed cla: yes labels Aug 25, 2020

Adding a feature flag around port description in the FleetAutoscaler …

c741dfd

…documentation

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

Merge branch 'master' into fix/autoscale-webhook-port

f28bbe8

markmandel added cla: yes and removed cla: no labels Aug 26, 2020

markmandel merged commit 5709810 into googleforgames:master Aug 26, 2020

markmandel added this to the 1.9.0 milestone Aug 26, 2020

markmandel added kind/bug These are bugs. area/user-experience Pertaining to developers trying to use Agones, e.g. SDK, installation, etc labels Sep 22, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pass port into autoscaler url from webhook policy #1765

Pass port into autoscaler url from webhook policy #1765

andrewgrundy commented Aug 19, 2020 •

edited

Loading

googlebot commented Aug 19, 2020

agones-bot commented Aug 19, 2020

aLekSer left a comment

aLekSer commented Aug 20, 2020

agones-bot commented Aug 20, 2020

aLekSer left a comment •

edited

Loading

google-oss-robot commented Aug 20, 2020

aLekSer commented Aug 20, 2020

roberthbailey Aug 21, 2020

aLekSer Aug 24, 2020

markmandel Aug 26, 2020

googlebot commented Aug 25, 2020

googlebot commented Aug 25, 2020

google-oss-robot commented Aug 26, 2020

agones-bot commented Aug 26, 2020

markmandel commented Aug 26, 2020

agones-bot commented Aug 26, 2020

markmandel commented Aug 26, 2020

googlebot commented Aug 26, 2020

agones-bot commented Aug 26, 2020

Pass port into autoscaler url from webhook policy #1765

Pass port into autoscaler url from webhook policy #1765

Conversation

andrewgrundy commented Aug 19, 2020 • edited Loading

googlebot commented Aug 19, 2020

What to do if you already signed the CLA

Individual signers

Corporate signers

agones-bot commented Aug 19, 2020

aLekSer left a comment

Choose a reason for hiding this comment

aLekSer commented Aug 20, 2020

agones-bot commented Aug 20, 2020

aLekSer left a comment • edited Loading

Choose a reason for hiding this comment

google-oss-robot commented Aug 20, 2020

aLekSer commented Aug 20, 2020

roberthbailey Aug 21, 2020

Choose a reason for hiding this comment

aLekSer Aug 24, 2020

Choose a reason for hiding this comment

markmandel Aug 26, 2020

Choose a reason for hiding this comment

googlebot commented Aug 25, 2020

googlebot commented Aug 25, 2020

What to do if you already signed the CLA

Individual signers

Corporate signers

google-oss-robot commented Aug 26, 2020

agones-bot commented Aug 26, 2020

markmandel commented Aug 26, 2020

agones-bot commented Aug 26, 2020

markmandel commented Aug 26, 2020

googlebot commented Aug 26, 2020

agones-bot commented Aug 26, 2020

andrewgrundy commented Aug 19, 2020 •

edited

Loading

aLekSer left a comment •

edited

Loading