test: simplify autoscaler e2e tests for minikube compatibility#4331
test: simplify autoscaler e2e tests for minikube compatibility#4331markmandel merged 3 commits intoagones-dev:mainfrom
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR simplifies the webhook autoscaler configuration in the end-to-end test by removing unnecessary fields and changing the image pull policy.
- Changed the container image pull policy from
PullAlwaystoPullIfNotPresentto improve test performance - Removed the named port reference from the container port and service port definitions
- Simplified the service port configuration by removing the
TargetPortspecification
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
3ff11a9 to
5b5d83d
Compare
|
Build Failed 😭 Build Id: 95179da2-92de-4512-9085-c3fb87bc64a8 Status: FAILURE To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
5b5d83d to
a435dd2
Compare
|
Build Succeeded 🥳 Build Id: 7bd2906b-d6c7-4e74-968f-ed9c736971c5 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: |
test/e2e/fleetautoscaler_test.go
Outdated
| Name: "newport", | ||
| Port: 8000, | ||
| TargetPort: intstr.IntOrString{StrVal: "autoscaler"}, | ||
| TargetPort: intstr.FromInt(8000), |
There was a problem hiding this comment.
I was doing some test locally, it seems to be working by using intstr.FromString("autoscaler") (and keeping the name: "autoscaler" L750
intstr.FromString add the type with it: IntOrString{Type: String, StrVal: val}, I've got it working locally, not sure how it works on the background, if you can also test on your side, otherwise it's good to me to keep it like this 😄
There was a problem hiding this comment.
It might be by default to an int for the type and the validation expect it to be so and fails the creation of the service *
I'm not fully sure about it, but it's possible
There was a problem hiding this comment.
Good find! That worked!
As an interesting thing, checked the implementation:
// FromString creates an IntOrString object with a string value.
func FromString(val string) IntOrString {
return IntOrString{Type: String, StrVal: val}
}
No Type value on the original implementation!
- Change ImagePullPolicy from PullAlways to PullIfNotPresent - Remove unnecessary container port name 'autoscaler' - Remove TargetPort reference to simplify service configuration For some reason using a name for TargetPort failed on minikube, but this fixed it 🤷♂️
a435dd2 to
2dc879f
Compare
|
Something weird happened to CI. |
|
/gcbrun |
2dc879f to
5aeb0ae
Compare
|
Build Failed 😭 Build Id: 24910709-dec4-46e7-b649-c81cbcfad24d Status: FAILURE To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
|
Build Failed 😭 Build Id: d96a7609-fb5f-4f14-aebc-c7317209824a Status: FAILURE To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
wha? |
|
/gcbrun |
|
Build Failed 😭 Build Id: 9071009d-9881-4f24-aea4-dd5152d6b9b3 Status: FAILURE To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
|
/gcbrun |
|
Build Succeeded 🥳 Build Id: cc442318-5d50-45a0-bbe5-e2ca734ebe55 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: |
…s-dev#4331) * test: simplify autoscaler e2e tests for minikube compatability - Change ImagePullPolicy from PullAlways to PullIfNotPresent - Remove unnecessary container port name 'autoscaler' - Remove TargetPort reference to simplify service configuration For some reason using a name for TargetPort failed on minikube, but this fixed it 🤷♂️ * Review updates.

What type of PR is this?
/kind cleanup
What this PR does / Why we need it:
Which issue(s) this PR fixes:
N/A
Special notes for your reviewer:
For some reason using a name for TargetPort failed on minikube, but this fixed it 🤷♂️