More Local Dev Server Support #3252
Conversation
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.
|
Build Failed 😱 Build Id: 4503dbc9-e7fb-48aa-acbc-682d1c6a0385 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
|
Build Failed 😱 Build Id: 185124c5-311c-436d-bd07-6be3c20c4a3d To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
|
Build Failed 😱 Build Id: e5b0adaf-7bdd-455f-ad77-9a2df4a91d5e To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
|
Build Failed 😱 Build Id: a7444b80-6426-4423-a543-5925a1376652 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
|
Build Failed 😱 Build Id: fca55011-9bbb-4a82-84c2-2c1157858811 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
|
Cleaned up to try and pass CI as much as I could. Look forward to the review. Thanks! |
|
Build Succeeded 👏 Build Id: 9cfe99b0-3100-419c-bb45-539c8ecb4ea8 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:
|
| @@ -149,3 +172,52 @@ wget https://raw.githubusercontent.com/googleforgames/agones/{{< release-branch | |||
| chmod o+r gameserver.yaml | |||
| docker run --network=host --rm -v $(pwd)/gameserver.yaml:/tmp/gameserver.yaml us-docker.pkg.dev/agones-images/release/agones-sdk:{{<release-version>}} --local -f /tmp/gameserver.yaml | |||
| ``` | |||
|
|
|||
| ## Running locally using "out of cluster" mode | |||
There was a problem hiding this comment.
Just a heads up, our docs publish on merge, so if you have new features that aren't in the current release, please wrap them in a feature shortcode like so: https://agones.dev/site/docs/contribute/
There was a problem hiding this comment.
Added publishDate to new page. Wrapped links with {{% feature publishVersion="1.34.0" %}}.
| However, there are a number of commands that are necessary/useful to run when running in "out of cluster" mode. | ||
| Here is a sample with each piece discussed after. | ||
| ```bash | ||
| GAMESERVER_NAME='my-local-server' \ |
There was a problem hiding this comment.
I feel like to do this cleanly, we should update the sdkserver to also allow command line args for the GameServer name and Pod Namespace -- just to be consistent, and make life easier for end users. Then we don't have to mix args and env vars.
Also would prefer to see these docs using a binary, not running from source.
There was a problem hiding this comment.
Then we don't have to mix args and env vars.
I agree that this would be nicer. I can add in the extra flags.
Also would prefer to see these docs using a binary, not running from source.
As mentioned in the other comment, I think running from source is nicer than downloaded binaries. But sure, for the sake of documentation consistency, I can update this.
There was a problem hiding this comment.
Just to follow up here, this work has been done. --gameserver-name and --pod-namespace can be used as cli args, and GAMESERVER_NAME and POD_NAMESPACE still work as env vars (which are used in k8s). Also updated this to be consistent and use pre-built binary commands.
| @@ -9,6 +9,8 @@ description: > | |||
|
|
|||
| You can register a local game server with Agones. This means you can run an experimental build of your game server in the Agones environment without the need of packaging and deploying it to a fleet. This allows you to quickly iterate on your game server code while still being able to plugin to your Agones environment. | |||
|
|
|||
| This can be used in combination with a [local SDK Server]({{< ref "./Client SDKs/local.md" >}}) in ["out of cluster" mode]({{< ref "./Client SDKs/local.md#running-locally-using-out-of-cluster-mode" >}}). | |||
There was a problem hiding this comment.
So when reading the "out of cluster mode" - I automatically assumed I could connect to my local game server binary.... but reading this, it doesn't seem to be the case.
I'm wondering if there's a way we can combine it into one whole process? 🤔 not entirely sure how, but it feels like it's worth exploring. WDYT?
There was a problem hiding this comment.
I automatically assumed I could connect to my local game server binary
That is the case. This is what I do for local testing. I have my custom game server running in Visual Studio, I run the SDK Server on a terminal, and I have it connect to my cluster. This allows me to allocate the game server via normal prod flows (e.g. external calls onto the allocator service) which are then fully integrated with my breakpoint-able game server in VS.
I'm wondering if there's a way we can combine it into one whole process?
I mean, the steps to do so are mentioned in documentation. One would need to call kubectl apply -f dev-gameserver.yaml, then run ./sdk-server.linux.amd64 --kubeconfig ..., then run their custom game server binary. Everything is then connected.
There was a problem hiding this comment.
A few thoughts on this:
- From reading the documentation, the combination of the two approaches wasn't immediately obvious to me. I'm almost wondering if this would be better as a part of https://agones.dev/site/docs/guides/local-game-server/ than here 🤔 (at the very least, they should probably link to each other though).
- We should be super clear that you need both to actually connect. With the current docs it's easy to assume you can and you may not realise until the very end that you can't (or even that the
agones.dev/dev-addressGameSever approach exists -- which is kinda why I'm thinking we may want this in https://agones.dev/site/docs/guides/local-game-server/ as well, since one really does lead into the other. - Possibly a wacky idea 😄 since we can pass a gameserver.yaml to the local game server for state -- could we make the "out of cluster mode" server create the
agones.dev/dev-addressfor you? It's using local kubectl permissions anyway -- save you akubectl apply? Just trying to streamline development even more.
There was a problem hiding this comment.
I'm almost wondering if this would be better as a part of https://agones.dev/site/docs/guides/local-game-server/ than here 🤔
I'm thinking we may want this in https://agones.dev/site/docs/guides/local-game-server/ as well
Ummm, this is https://agones.dev/site/docs/guides/local-game-server/ (site/content/en/docs/Guides/local-game-server.md). Maybe I'm misunderstanding you? 😅
(at the very least, they should probably link to each other though).
I added links to both pages, this is the one from https://agones.dev/site/docs/guides/local-game-server/ to https://agones.dev/site/docs/guides/client-sdks/local/. In site/content/en/docs/Guides/Client SDKs/local.md (line 180) I have the reverse link.
could we make the "out of cluster mode" server create the agones.dev/dev-address for you?
That seems like a large expansion of the behavior of the SDK Server. While it obviously isn't my call to say what's best for the codebase, it just seems like that change would be larger and more out of place.
On a slightly separate note, one of the goals here was for this dev flow to be more representative of prod. While we are manually running the SDK Server locally instead of it being created in a sidecar container, it is still running the "out of cluster" mode using the exact same logic flow as it would be in the sidecar. Adding in extra steps/logic of the SDK Server, or changing how the SDK Server and the GameServer resource interact, builds less confidence in "what is developed" == "what goes on prod".
There was a problem hiding this comment.
Ummm, this is https://agones.dev/site/docs/guides/local-game-server/ (site/content/en/docs/Guides/local-game-server.md). Maybe I'm misunderstanding you?
Sorry, I 100% wasn't clear. I was wondering it the whole new documentation set in local.md should be in this documentation - since my gut says that they are likely always going to be used together. WDYT? That also then solves any confusion around connection etc.
Also I was providing feedback generally to the docs, not just these specific pages -- understandably confusing.
could we make the "out of cluster mode" server create the agones.dev/dev-address for you?
That seems like a large expansion of the behavior of the SDK Server. While it obviously isn't my call to say what's best for the codebase, it just seems like that change would be larger and more out of place.
What do you think of the idea though? Can always be a secondary issue filed for future work, just thought it might be handy.
There was a problem hiding this comment.
I was wondering it the whole new documentation set in
local.mdshould be in this documentation - since my gut says that they are likely always going to be used together.
I suppose it could go in either place. Personally, I feel like this has more to do with running the local SDK Server (as you need to change the commands used when running it). I definitely think "out of cluster" mode should be mentioned in both spots.
What do you think of the idea though? Can always be a secondary issue filed for future work, just thought it might be handy.
If you're just asking for my opinion, I don't particularly see any use for it (for me). I suppose some might like omitting a kubectl apply, but that's not really what I'd be optimizing for. What I'd be looking for is "breakpoint locally but in the most prod-y way possible". As the expected order of lifetimes/creation on prod would be: GameServer resource, SDK Server binary, custom game server binary, I would want to keep it in that order. This means that the custom game binary, the allocation system, etc, can all be run in a consistent state (code abstracting away from this flow vs prod). Swapping around the order or operations, or changing what creates what, for dev testing (in such an integrated context) doesn't really make sense to me, as it isn't representative of how things will be working at the end.
Speaking of "other things that could be done for handy-ness":
As I alluded to in #3252 (comment), I have other desires for potential future PRs. Namely that dev servers wouldn't automatically go from Creating to Ready. Instead, they would do a normal Creating to Scheduled, and would only progress to Ready after the SDK call was made (+ controller state progression) -- all of this would match https://agones.dev/site/docs/reference/gameserver/#gameserver-state-diagram. An annotation could be used to "auto-ready" (e.g. agones.dev/auto-ready: true) to keep the current functionality. Also an annotation for agones.dev/recycle-gs: true would be nice to progress from Shutdown to Creating/Scheduled (so when the binary exits, calling SDK Shutdown, the GameServer would just be reset instead of deleted). Again, all of this is a bit different/extra -- this PR is just trying to add in the finishing touches for a very useful flow that's mostly there already. Lol, I hesitated to really go into all this extra stuff as I don't want it to distract from this (seemingly more straightforward) PR. Hopefully this helps with the perspective that I'm looking at things from.
There was a problem hiding this comment.
These are cool ideas! I'd love you to write up some issues and designs for these and we can discuss them further.
Thinking about docs organisation some more, https://agones.dev/site/docs/guides/client-sdks/local/ is about a purely local experience (and the simplest to get started with).
This file is about integrating with your online system, likely tied to how you like to run things in production (also it's further down in the docs, so it's assumed by this point, you've managed to work out how the SDK works, done some integration etc).
Jumping from "how do I integrate the SDK" to "now I'm working in a cluster" within the same page as it is now seems to me like a big leap concept with for new user onboarding to me.
So let's move the out-of-cluster documentation into a new section here, and add a link to this doc at the bottom (I'll add a suggestion to local.md on what I think will work there), since I think the annotations and the out-of-cluster mode is all interlinked (I don't see one being done without the other), without changing the learning path drastically.
There was a problem hiding this comment.
I'd love you to write up some issues and designs for these and we can discuss them further.
I went ahead and filed #3284.
let's move the out-of-cluster documentation into a new section
I've now moved all the "out of cluster" documentation into site/content/en/docs/Advanced/out-of-cluster-dev-server.md. This actually allowed me to go much further into various details. I've updated site/content/en/docs/Guides/Client SDKs/local.md and site/content/en/docs/Guides/local-game-server.md with Next Steps that link to out of cluster configuration.
|
Replied to explain a few points. Will commit more changes on relevant topics. Will re-merge main back in. Probably will get back to this tomorrow night. |
| @@ -9,6 +9,8 @@ description: > | |||
|
|
|||
| You can register a local game server with Agones. This means you can run an experimental build of your game server in the Agones environment without the need of packaging and deploying it to a fleet. This allows you to quickly iterate on your game server code while still being able to plugin to your Agones environment. | |||
|
|
|||
| This can be used in combination with a [local SDK Server]({{< ref "./Client SDKs/local.md" >}}) in ["out of cluster" mode]({{< ref "./Client SDKs/local.md#running-locally-using-out-of-cluster-mode" >}}). | |||
There was a problem hiding this comment.
A few thoughts on this:
- From reading the documentation, the combination of the two approaches wasn't immediately obvious to me. I'm almost wondering if this would be better as a part of https://agones.dev/site/docs/guides/local-game-server/ than here 🤔 (at the very least, they should probably link to each other though).
- We should be super clear that you need both to actually connect. With the current docs it's easy to assume you can and you may not realise until the very end that you can't (or even that the
agones.dev/dev-addressGameSever approach exists -- which is kinda why I'm thinking we may want this in https://agones.dev/site/docs/guides/local-game-server/ as well, since one really does lead into the other. - Possibly a wacky idea 😄 since we can pass a gameserver.yaml to the local game server for state -- could we make the "out of cluster mode" server create the
agones.dev/dev-addressfor you? It's using local kubectl permissions anyway -- save you akubectl apply? Just trying to streamline development even more.
…CE` 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.
…r 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.
… running from source. * Done for consistency with other examples.
|
Build Succeeded 👏 Build Id: 1a0faeab-1095-43a2-84cb-4d04b46b9a67 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
requested review from
zmerlynn and
gongmax
and removed request for
cyriltovena and
aLekSer
* 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.
…it was before (but now as 1 set of bullets).
|
Build Failed 😱 Build Id: d2463353-006b-4512-b321-a07b4c3a5a80 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
|
Build Failed 😱 Build Id: c368d45f-05ac-4d2c-aca0-a7793cba5c97 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
|
Build Succeeded 👏 Build Id: f60d69be-d461-4017-85a2-15bec936ffff 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: ae9fcec4-007f-4728-9ba9-7e16ed306e40 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.
LGTM!
I checked the documentation rendering too, and it looks good 👍🏻 nice!
Holding off merging until @zmerlynn takes a look just in case you had any final thoughts.
| title: "Out of Cluster Dev Server" | ||
| linkTitle: "Out of Cluster Dev Server" | ||
| date: 2023-07-22T17:21:25Z | ||
| publishDate: 2023-08-15T07:00:00Z |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: CauhxMilloy, 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 |
|
New changes are detected. LGTM label has been removed. |
|
Build Failed 😱 Build Id: 6340c7d2-3537-4c85-b230-f39f66f3e8db To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
|
gke-autopilot-1.25 flaked again. Weird. |
|
Build Succeeded 👏 Build Id: 4bcfe254-f909-4ea0-b1ae-b7814a58a834 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:
|
|
Zack's away at the moment, so I'm going to go forward and merge this - we can always clean up later if we decide we need to. Thank you very much for your contribution and continued persistence! |
What type of PR is this?
/kind feature
(but also documentation)
What this PR does / Why we need it:
This PR adds better support for integration between Dev Servers (called Local Game Server in guides) and Local Development.
Note that these changes do not affect prod flows, only developer flows when running manually with extra (new) command line arguments.
sdk-server/main.go) for "out of cluster" mode.--kubeconfigcli flag to pass in a suitable k8s config file.clientcmd.BuildConfigFromFlags()to connect to the k8s cluster.BuildConfigFromFlags()call will internally fall back to theInClusterConfig().--kubeconfigcli arg forcmd/controller/main.go.--no-graceful-terminationcli flag.ctxobject will not be wrapped by theNewSDKServerContext()call. This means that exiting the program will not wait (block) for theGameServerstate to update toShutdown.RequestReadytoReadytransition (for dev server).RequestReadyis set via SDK Server (from SDK calls). Need to transition toReadyto be allocatable.docs/Guides/Client SDKs/local.md) with information with running "out of cluster" mode.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.Which issue(s) this PR fixes:
N/A? I didn't see any issues that exactly related to what this solves.
Special notes for your reviewer:
Agones is cool. 😎 Thanks for the FOSS.