Move SDKGracefulTermination To Stable #3231
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Update label-pr.yml
…ch-62 Revert "Update label-pr.yml"
…selvi84-patch-62 Revert "Revert "Update label-pr.yml""
|
Build Failed 😱 Build Id: 7c044ef5-ef45-482f-b100-a5cb683ea374 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
|
This is weird, this unit test seems to be consistently failing across multiple PRs. |
markmandel
reviewed
Jun 22, 2023
Comment on lines
143
to
144
| If the `SDKGracefulTermination` feature is enabled, when the SDK server receives the TERM signal before calling SDK.Shutdown(), | ||
| the SDK server would stay alive for the period of the terminationGracePeriodSeconds until SDK.Shutdown() has been called |
There was a problem hiding this comment.
Just some cleanup since the gate doesn't exist, and also since we're here.
Suggested change
| If the `SDKGracefulTermination` feature is enabled, when the SDK server receives the TERM signal before calling SDK.Shutdown(), | |
| the SDK server would stay alive for the period of the terminationGracePeriodSeconds until SDK.Shutdown() has been called | |
| If the SDK server receives a TERM signal before calling SDK.Shutdown(), | |
| the SDK server will stay alive for the period of the `terminationGracePeriodSeconds` until `SDK.Shutdown()` has been called. |
There was a problem hiding this comment.
made the changes as per your suggestion
markmandel
approved these changes
Jun 22, 2023
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Kalaiselvi84, 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:
Approvers can indicate their approval by writing |
|
Build Failed 😱 Build Id: bc855225-f6d1-4897-b2c7-4d51e6031e8f To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
|
That's a weird flake in generic-1.25, during helm install: |
|
New changes are detected. LGTM label has been removed. |
|
Build Succeeded 👏 Build Id: 460137b0-b89a-4dec-a069-6c8386a6da6c 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:
|
|
@markmandel, Is there anything pending that we need to wait for before merging this PR? Can we proceed with merging? |
|
Thought I had set this to automerge! |
|
Build Failed 😱 Build Id: 9051d81a-09d1-4b8a-91b6-fc32e272483a To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
CauhxMilloy
added a commit
to CauhxMilloy/agones
that referenced
this pull request
Jul 8, 2023
Note that these changes do not affect prod flows, only developer flows when running manually with extra (new) command line arguments.
* Updating SDK Server (`sdk-server/main.go`) for "out of cluster" mode.
* Adding `--kubeconfig` cli flag to pass in a suitable k8s config file.
* Makes use of `clientcmd.BuildConfigFromFlags()` to connect to the k8s cluster.
* If the flag is not provided (as is the case for typical prod use), the `BuildConfigFromFlags()` call will internally fall back to the `InClusterConfig()`.
* This is similar to the `--kubeconfig` cli arg for `cmd/controller/main.go`.
* Adding `--no-graceful-termination` cli flag.
* If set (default unset), the context `ctx` object will not be wrapped by the `NewSDKServerContext()` call. This means that exiting the program will not wait (block) for the `GameServer` state to update to `Shutdown`.
* This flag to disable graceful shutdown is added as a convenient dev flow option, to quickly terminate and restart the SDK Server when testing locally.
* Graceful termination was first added as an optional feature in googleforgames#2205.
* Fully rolled into stable (no longer optional feature) in googleforgames#3231.
* This flag does not affect normal (prod) flows (graceful termination still enabled by default).
* Updating gameservers/controller.go to handle `RequestReady` to `Ready` transition.
* `RequestReady` is set via SDK Server (from SDK calls). Need to transition to `Ready` to be allocatable.
* Updated "Local Development" guide page (`docs/Guides/Client SDKs/local.md`) with information with running "out of cluster" mode.
* Also added information about running from source code instead of prebuilt binaries -- which many devs would prefer.
* Added links between "Local Development" guide page (`docs/Guides/Client SDKs/local.md`) and "Local Game Server" guide page (`docs/Guides/local-game-server.md`) about how they can be used together.
CauhxMilloy
added a commit
to CauhxMilloy/agones
that referenced
this pull request
Jul 8, 2023
This commit adds better support for integration between Dev Servers (called [Local Game Server](https://agones.dev/site/docs/guides/local-game-server/) in guides) and [Local Development](https://agones.dev/site/docs/guides/client-sdks/local/). Note that these changes do not affect prod flows, only developer flows when running manually with extra (new) command line arguments. * Updating SDK Server (`sdk-server/main.go`) for "out of cluster" mode. * Adding `--kubeconfig` cli flag to pass in a suitable k8s config file. * Makes use of `clientcmd.BuildConfigFromFlags()` to connect to the k8s cluster. * If the flag is not provided (as is the case for typical prod use), the `BuildConfigFromFlags()` call will internally fall back to the `InClusterConfig()`. * This is similar to the `--kubeconfig` cli arg for `cmd/controller/main.go`. * Adding `--no-graceful-termination` cli flag. * If set (default unset), the context `ctx` object will not be wrapped by the `NewSDKServerContext()` call. This means that exiting the program will not wait (block) for the `GameServer` state to update to `Shutdown`. * This flag to disable graceful shutdown is added as a convenient dev flow option, to quickly terminate and restart the SDK Server when testing locally. * Graceful termination was first added as an optional feature in googleforgames#2205. * Fully rolled into stable (no longer optional feature) in googleforgames#3231. * This flag does not affect normal (prod) flows (graceful termination still enabled by default). * Updating gameservers/controller.go to handle `RequestReady` to `Ready` transition. * `RequestReady` is set via SDK Server (from SDK calls). Need to transition to `Ready` to be allocatable. * Updated "Local Development" guide page (`docs/Guides/Client SDKs/local.md`) with information with running "out of cluster" mode. * Also added information about running from source code instead of prebuilt binaries -- which many devs would prefer. * Added links between "Local Development" guide page (`docs/Guides/Client SDKs/local.md`) and "Local Game Server" guide page (`docs/Guides/local-game-server.md`) about how they can be used together.
markmandel
pushed a commit
that referenced
this pull request
Jul 26, 2023
* More Local Dev Server Support This commit adds better support for integration between Dev Servers (called [Local Game Server](https://agones.dev/site/docs/guides/local-game-server/) in guides) and [Local Development](https://agones.dev/site/docs/guides/client-sdks/local/). Note that these changes do not affect prod flows, only developer flows when running manually with extra (new) command line arguments. * Updating SDK Server (`sdk-server/main.go`) for "out of cluster" mode. * Adding `--kubeconfig` cli flag to pass in a suitable k8s config file. * Makes use of `clientcmd.BuildConfigFromFlags()` to connect to the k8s cluster. * If the flag is not provided (as is the case for typical prod use), the `BuildConfigFromFlags()` call will internally fall back to the `InClusterConfig()`. * This is similar to the `--kubeconfig` cli arg for `cmd/controller/main.go`. * Adding `--no-graceful-termination` cli flag. * If set (default unset), the context `ctx` object will not be wrapped by the `NewSDKServerContext()` call. This means that exiting the program will not wait (block) for the `GameServer` state to update to `Shutdown`. * This flag to disable graceful shutdown is added as a convenient dev flow option, to quickly terminate and restart the SDK Server when testing locally. * Graceful termination was first added as an optional feature in #2205. * Fully rolled into stable (no longer optional feature) in #3231. * This flag does not affect normal (prod) flows (graceful termination still enabled by default). * Updating gameservers/controller.go to handle `RequestReady` to `Ready` transition. * `RequestReady` is set via SDK Server (from SDK calls). Need to transition to `Ready` to be allocatable. * Updated "Local Development" guide page (`docs/Guides/Client SDKs/local.md`) with information with running "out of cluster" mode. * Also added information about running from source code instead of prebuilt binaries -- which many devs would prefer. * Added links between "Local Development" guide page (`docs/Guides/Client SDKs/local.md`) and "Local Game Server" guide page (`docs/Guides/local-game-server.md`) about how they can be used together. * * Update documentation clarification location... -ation. * * Fixing formatting (had 1 too many spaces). * * Fixing ref relative paths. * * Removing quotes missed in previous cleanups. * * As per review feedback, updating `GAMESERVER_NAME` and `POD_NAMESPACE` to be passable via cli args instead of only as environment variables. * This should still function using environment variables due to calls to `viper.BindEnv()`. * Manually verified functionality when running locally with either cli args or environment variables. * Moving values to pass through `config` struct, just as other flags are passed. * Updating documentation to use cli args instead of environment variables. * * As per review feedback, changing from negative to positive logic for graceful termination flag. * Renaming `--no-graceful-termination` to `--graceful-termination`. * Setting default value to `true`. * Manually verified locally that graceful termination is on by default. * Updating indention. * Updating documentation to show renamed cli arg. * * Updating links to use `ref` instead of absolute site link. * * Switching "out of cluster" command example to use binary instead of running from source. * Done for consistency with other examples. * * Adding controller test for RequestReady -> Ready state progression. * * Adding `RequestReady` -> `Ready` state progressing in `TestDevelopmentGameServerLifecycle` e2e test. * * Fixing call to `StartInformers()` in test. * * Moving discussion of running from source code down to the bottom. * * Adding more cross-reference links for game server allocation. * * Moving "out of cluster" documentation into its own file. * Going into much more detail about background and steps to complete. * `publishDate` set to `2023-08-15T07:00:00Z` (midnight PST) to not publish doc immediately. * `2023-08-15` chosen to coincide with next release, per https://github.com/googleforgames/agones/blob/main/docs/governance/release_process.md#release-calendar. * Adding links to new page from `Local Game Server` and `Local Development` pages. * Links are wrapped by `publishVersion="1.34.0"` to not show immediately. `1.34.0` is the next release, per https://github.com/googleforgames/agones/blob/main/docs/governance/release_process.md#release-calendar. * * Switching OS/file description back to bulleted form similar to how it was before (but now as 1 set of bullets). * * Removing link to moved section (covered by "Next Steps" section). * * Cleaning up some wording. * * Adding comments about restarting locally run binaries. * * Proofread and update.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
/kind feature
What this PR does / Why we need it:
Which issue(s) this PR fixes:
Closes #3094
Special notes for your reviewer: