-
Notifications
You must be signed in to change notification settings - Fork 813
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Graduate Counters and Lists to Beta #3801
Conversation
Build Failed 😱 Build Id: 95fef972-709e-47e2-ab60-3cbd184e3b40 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Failed 😱 Build Id: c3ff642e-3f39-4547-8ebe-0c8c37d2b5de To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@igooch Do you want to LGTM as well?
Build Failed 😱 Build Id: d65ce6af-52b7-4b4a-adfe-12faa95d38ed To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Failed 😱 Build Id: b3a4af55-25e1-4530-abd5-08105597fc37 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
I am reminded by the build failures that we need to graduate the SDKs as well! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Taking out of approved.)
Build Failed 😱 Build Id: b9766c28-7472-4c0f-a7df-0289f0bc4788 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Failed 😱 Build Id: cdd9d609-a40b-446c-b49f-54216255b126 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
@zmerlynn could you please let me know the steps to graduate CountsAndLists in the SDKs |
This is really two parts:
It's .. a little intense. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have make run-sdk-conformance-test-go
working, I'll let you duplicate on the csharp SDK test.
Added the changes as a patch file, if that's easier:
updates.PATCH
build/includes/sdk.mk
Outdated
@@ -171,7 +172,7 @@ run-sdk-conformance-test-go: | |||
# run without feature flags | |||
$(MAKE) run-sdk-conformance-test SDK_FOLDER=go GRPC_PORT=9001 HTTP_PORT=9101 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be:
$(MAKE) run-sdk-conformance-test SDK_FOLDER=go GRPC_PORT=9001 HTTP_PORT=9101 TESTS=$(DEFAULT_CONFORMANCE_TESTS),$(COUNTS_AND_LISTS_TESTS)
As it needs to be told to check the Counter and Lists tests, since it's now enabled!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, this fix isn't working for the C# and rest tests. Could there be something missing to enable the CL feature for these two tests? Please guide.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For C# the tests are not running because of these lines:
agones/test/sdk/csharp/Program.cs
Line 169 in db84735
if (featureGates.Contains("CountsAndLists")) |
agones/test/sdk/csharp/Program.cs
Line 295 in db84735
if (featureGates.Contains("CountsAndLists")) |
Which is picking up DOCKER_RUN_ARGS="--net host -e AGONES_SDK_GRPC_PORT=9005 -e AGONES_SDK_HTTP_PORT=9105 -e FEATURE_GATES=''
Even though the feature gate is enabled by default {"ctlConf":{"GameServerName":"","PodNamespace":"","Address":"","IsLocal":false,"LocalFile":"","Delay":2,"Timeout":40,"Test":"ready,allocate,setlabel,setannotation,gameserver,health,shutdown,watch,reserve,getcounter,updatecounter,setcountcounter,setcapacitycounter,getlist,updatelist,addlistvalue,removelistvalue","TestSdkName":"csharp","KubeConfig":"","GracefulTermination":true,"GRPCPort":9005,"HTTPPort":9105,"LogLevel":""},"featureGates":"CountsAndLists=true\u0026DisableResyncOnSDKServer=true\u0026Example=false\u0026GKEAutopilotExtendedDurationPods=false\u0026PlayerAllocationFilter=false\u0026PlayerTracking=false","message":"Starting sdk sidecar","severity":"info","source":"main","time":"2024-04-25T22:01:36.795794579Z","version":"1.41.0-dev-d0a74b1"}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right now I'm thinking the easiest solution would be to create a BETA_FEATURE_GATES
similar to the
Line 67 in db84735
ALPHA_FEATURE_GATES ?= "PlayerAllocationFilter=true&PlayerTracking=true&CountsAndLists=true&Example=true" |
runtime.FeatureEnabled(runtime.FeatureCountsAndLists)
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you mind submitting a PR to fix these two tests, please?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a commit.
A note: The Go SDK can see the runtime state of the SDK config, by using runtime.FeatureEnabled(runtime.FeatureCountsAndLists)
. That means that for the first Go sdk conformance test Beta is on and Alpha is off, and for the second Go sdk conformance test both Alpha and Beta are on.
For REST and C# they can't see the runtime config, and depend on an environment variable being passed through. This means that for the first C# and REST tests Beta is on and Alpha is off, and for the second test Beta is off and Alpha is on.
Build Failed 😱 Build Id: bd6c4c71-3e1a-4e32-a98b-1b4d5c9c70a4 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Failed 😱 Build Id: b48fe5d6-664a-42ac-b18c-7d5a31f464dd To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
…r Service" This reverts commit 4425ccd.
…formance tests These tests cannot see the runtime config of the SDK that sets the feature gate countsandlists to true by default, so we need to pass in an environment variable setting the feature gate to true.
Build Succeeded 👏 Build Id: 46572f7f-f529-4f7e-8016-69fac8c102ab 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:
|
Build Succeeded 👏 Build Id: 0e046e05-9248-48a1-b0e2-48a654ea20ec 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:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This PR contains the following updates: | Package | Update | Change | |---|---|---| | [agones](https://agones.dev) ([source](https://github.com/googleforgames/agones)) | minor | `1.40.0` -> `1.41.0` | --- > [!WARNING] > Some dependencies could not be looked up. Check the Dependency Dashboard for more information. --- ### Release Notes <details> <summary>googleforgames/agones (agones)</summary> ### [`v1.41.0`](https://github.com/googleforgames/agones/blob/HEAD/CHANGELOG.md#v1410-2024-06-04) [Compare Source](https://github.com/googleforgames/agones/compare/v1.40.0...v1.41.0) [Full Changelog](https://github.com/googleforgames/agones/compare/v1.40.0...v1.41.0) **Implemented enhancements:** - Configure Allocator Status Code by [@​Kalaiselvi84](https://github.com/Kalaiselvi84) in [https://github.com/googleforgames/agones/pull/3782](https://github.com/googleforgames/agones/pull/3782) - Graduate Counters and Lists to Beta by [@​Kalaiselvi84](https://github.com/Kalaiselvi84) in [https://github.com/googleforgames/agones/pull/3801](https://github.com/googleforgames/agones/pull/3801) - Passthrough autopilot - Adds an AutopilotPassthroughPort Feature Gate and new pod label by [@​vicentefb](https://github.com/vicentefb) in [https://github.com/googleforgames/agones/pull/3809](https://github.com/googleforgames/agones/pull/3809) - CountsAndLists: Move to Beta Protobuf by [@​Kalaiselvi84](https://github.com/Kalaiselvi84) in [https://github.com/googleforgames/agones/pull/3806](https://github.com/googleforgames/agones/pull/3806) - feat: support multiple port ranges by [@​nrwiersma](https://github.com/nrwiersma) in [https://github.com/googleforgames/agones/pull/3747](https://github.com/googleforgames/agones/pull/3747) - Changes `sdk-server` to Patch instead of Update by [@​igooch](https://github.com/igooch) in [https://github.com/googleforgames/agones/pull/3803](https://github.com/googleforgames/agones/pull/3803) - Generate grpc for nodejs from alpha to beta by [@​lacroixthomas](https://github.com/lacroixthomas) in [https://github.com/googleforgames/agones/pull/3825](https://github.com/googleforgames/agones/pull/3825) - Update CountsAndLists from Alpha to Beta by [@​Kalaiselvi84](https://github.com/Kalaiselvi84) in [https://github.com/googleforgames/agones/pull/3824](https://github.com/googleforgames/agones/pull/3824) - feat(gameserver): New DirectToGameServer PortPolicy allows direct traffic to a GameServer by [@​daniellee](https://github.com/daniellee) in [https://github.com/googleforgames/agones/pull/3807](https://github.com/googleforgames/agones/pull/3807) - Passthrough autopilot - Adds mutating webhook by [@​vicentefb](https://github.com/vicentefb) in [https://github.com/googleforgames/agones/pull/3833](https://github.com/googleforgames/agones/pull/3833) - Passthrough autopilot - added ports array case and updated unit tests by [@​vicentefb](https://github.com/vicentefb) in [https://github.com/googleforgames/agones/pull/3842](https://github.com/googleforgames/agones/pull/3842) - Nodejs counters and lists by [@​steven-supersolid](https://github.com/steven-supersolid) in [https://github.com/googleforgames/agones/pull/3726](https://github.com/googleforgames/agones/pull/3726) - Promote AutopilotPassthroughPort feature gate to Alpha by [@​vicentefb](https://github.com/vicentefb) in [https://github.com/googleforgames/agones/pull/3849](https://github.com/googleforgames/agones/pull/3849) **Fixed bugs:** - Helm Param Update: Default to agones.controller if agones.extensions is Missing by [@​Kalaiselvi84](https://github.com/Kalaiselvi84) in [https://github.com/googleforgames/agones/pull/3773](https://github.com/googleforgames/agones/pull/3773) - fix: rollout strategy issues by [@​nrwiersma](https://github.com/nrwiersma) in [https://github.com/googleforgames/agones/pull/3762](https://github.com/googleforgames/agones/pull/3762) - Set Minimum Buffer Size to 1 by [@​Kalaiselvi84](https://github.com/Kalaiselvi84) in [https://github.com/googleforgames/agones/pull/3749](https://github.com/googleforgames/agones/pull/3749) - Pin ltsc2019 to older SHA by [@​zmerlynn](https://github.com/zmerlynn) in [https://github.com/googleforgames/agones/pull/3829](https://github.com/googleforgames/agones/pull/3829) - TestGameServerAllocationDuringMultipleAllocationClients: Readdress flake by [@​zmerlynn](https://github.com/zmerlynn) in [https://github.com/googleforgames/agones/pull/3831](https://github.com/googleforgames/agones/pull/3831) - Refactor finalizer name to include valid domain name and path by [@​indexjoseph](https://github.com/indexjoseph) in [https://github.com/googleforgames/agones/pull/3840](https://github.com/googleforgames/agones/pull/3840) - agones-{extensions,allocator}: Be more defensive about draining by [@​zmerlynn](https://github.com/zmerlynn) in [https://github.com/googleforgames/agones/pull/3839](https://github.com/googleforgames/agones/pull/3839) - agones-{extensions,allocator}: Pause after cancelling context by [@​zmerlynn](https://github.com/zmerlynn) in [https://github.com/googleforgames/agones/pull/3843](https://github.com/googleforgames/agones/pull/3843) - Change the line to modify in Quickstart: Edit a Game Server by [@​peterzhongyi](https://github.com/peterzhongyi) in [https://github.com/googleforgames/agones/pull/3844](https://github.com/googleforgames/agones/pull/3844) **Other:** - Prep for Release v1.41.0 by [@​Kalaiselvi84](https://github.com/Kalaiselvi84) in [https://github.com/googleforgames/agones/pull/3800](https://github.com/googleforgames/agones/pull/3800) - Update site documentation to reflect firewall prefix and default to Autopilot cluster creation for Agones by [@​vicentefb](https://github.com/vicentefb) in [https://github.com/googleforgames/agones/pull/3769](https://github.com/googleforgames/agones/pull/3769) - Add a System Diagram and overview page by [@​zmerlynn](https://github.com/zmerlynn) in [https://github.com/googleforgames/agones/pull/3792](https://github.com/googleforgames/agones/pull/3792) - Update Side Menu: Preserve and Restore Scroll Position by [@​Kalaiselvi84](https://github.com/Kalaiselvi84) in [https://github.com/googleforgames/agones/pull/3805](https://github.com/googleforgames/agones/pull/3805) - fix: typo by [@​skmpf](https://github.com/skmpf) in [https://github.com/googleforgames/agones/pull/3808](https://github.com/googleforgames/agones/pull/3808) - Helm Config: Add httpUnallocatedStatusCode in Allocator Service by [@​Kalaiselvi84](https://github.com/Kalaiselvi84) in [https://github.com/googleforgames/agones/pull/3802](https://github.com/googleforgames/agones/pull/3802) - Update Docs: CountersAndLists to Beta by [@​Kalaiselvi84](https://github.com/Kalaiselvi84) in [https://github.com/googleforgames/agones/pull/3810](https://github.com/googleforgames/agones/pull/3810) - Disable Dev feature FeatureAutopilotPassthroughPort by [@​vicentefb](https://github.com/vicentefb) in [https://github.com/googleforgames/agones/pull/3815](https://github.com/googleforgames/agones/pull/3815) - Disable FeatureAutopilotPassthroughPort in features.go by [@​vicentefb](https://github.com/vicentefb) in [https://github.com/googleforgames/agones/pull/3816](https://github.com/googleforgames/agones/pull/3816) - SDK proto compatibility guarantees and deprecation policies documentation by [@​igooch](https://github.com/igooch) in [https://github.com/googleforgames/agones/pull/3774](https://github.com/googleforgames/agones/pull/3774) - Fix dangling "as of" by [@​zmerlynn](https://github.com/zmerlynn) in [https://github.com/googleforgames/agones/pull/3827](https://github.com/googleforgames/agones/pull/3827) - Steps to Promote SDK Features from Alpha to Beta by [@​Kalaiselvi84](https://github.com/Kalaiselvi84) in [https://github.com/googleforgames/agones/pull/3814](https://github.com/googleforgames/agones/pull/3814) - Adds comment for help troubleshooting issues with terraform tfstate by [@​igooch](https://github.com/igooch) in [https://github.com/googleforgames/agones/pull/3822](https://github.com/googleforgames/agones/pull/3822) - docs: improve counter and list example comments by [@​yonbh](https://github.com/yonbh) in [https://github.com/googleforgames/agones/pull/3818](https://github.com/googleforgames/agones/pull/3818) - Skip /tmp/ on yamllint by [@​zmerlynn](https://github.com/zmerlynn) in [https://github.com/googleforgames/agones/pull/3838](https://github.com/googleforgames/agones/pull/3838) - TestAllocatorAfterDeleteReplica: More logging by [@​zmerlynn](https://github.com/zmerlynn) in [https://github.com/googleforgames/agones/pull/3837](https://github.com/googleforgames/agones/pull/3837) - Instructions for upgrading golang version by [@​gongmax](https://github.com/gongmax) in [https://github.com/googleforgames/agones/pull/3819](https://github.com/googleforgames/agones/pull/3819) - Remove unused function FindGameServerContainer by [@​zmerlynn](https://github.com/zmerlynn) in [https://github.com/googleforgames/agones/pull/3841](https://github.com/googleforgames/agones/pull/3841) - Adds Unreal to the List of URL Links to Not Check by [@​igooch](https://github.com/igooch) in [https://github.com/googleforgames/agones/pull/3847](https://github.com/googleforgames/agones/pull/3847) - docs: clarify virtualization setup for Windows versions by [@​andresromerodev](https://github.com/andresromerodev) in [https://github.com/googleforgames/agones/pull/3850](https://github.com/googleforgames/agones/pull/3850) **New Contributors:** - [@​skmpf](https://github.com/skmpf) made their first contribution in [https://github.com/googleforgames/agones/pull/3808](https://github.com/googleforgames/agones/pull/3808) - [@​yonbh](https://github.com/yonbh) made their first contribution in [https://github.com/googleforgames/agones/pull/3818](https://github.com/googleforgames/agones/pull/3818) - [@​peterzhongyi](https://github.com/peterzhongyi) made their first contribution in [https://github.com/googleforgames/agones/pull/3844](https://github.com/googleforgames/agones/pull/3844) - [@​andresromerodev](https://github.com/andresromerodev) made their first contribution in [https://github.com/googleforgames/agones/pull/3850](https://github.com/googleforgames/agones/pull/3850) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4zOTAuMCIsInVwZGF0ZWRJblZlciI6IjM3LjM5MC4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJyZW5vdmF0ZS9oZWxtIiwidHlwZS9taW5vciJdfQ==-->
What type of PR is this?
/kind feature
What this PR does / Why we need it:
Which issue(s) this PR fixes:
work on #3775
Special notes for your reviewer: