Skip to content
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

Session data propagation to dedicated server #277

Closed
defat opened this issue Jun 27, 2018 · 18 comments
Closed

Session data propagation to dedicated server #277

defat opened this issue Jun 27, 2018 · 18 comments
Assignees
Labels
area/user-experience Pertaining to developers trying to use Agones, e.g. SDK, installation, etc kind/design Proposal discussing new features / fixes and how they should be implemented kind/feature New features for Agones
Milestone

Comments

@defat
Copy link

defat commented Jun 27, 2018

This is a feature request.
There are two options to get dedicated server for game session using Agones: create or allocate it from fleet. Using creation option session data such as map/game mode/level etc. can be passed to dedicated server with environment variables listed in pod template. Allocation from fleet lacks such possibility.
It would be great to have Agones SDK calling some callback method OnAllocation implemented on dedicated server with some AllocationParams object containing map<const char*, const char*> SessionData field. This session data can be passed through FleetAllocation object.
Another cool feature would be complting fleet allocation (through setting it status to Ready or smth like this) only after dedicated server loaded level and is fully ready to accept players' connections. Agones SDK could be extended with Loaded() method for this.

@markmandel markmandel added kind/feature New features for Agones kind/design Proposal discussing new features / fixes and how they should be implemented area/user-experience Pertaining to developers trying to use Agones, e.g. SDK, installation, etc labels Jun 27, 2018
@markmandel
Copy link
Member

So thanks for bringing this up - being able pass data to the GameServer has been front of mind for me recently as well (I'm going to limit this ticket to this one issue for now - if we want to talk about status management, let's start a different ticket)

My current thinking has been more along the following, although I think we end up in the same place - let me know what you think:

  1. Be able to access the GameServer crd data via the SDK. This is useful for several reasons (game server processes could know their own IP) - but also, since GameServers have annotations and labels, this could be a mechanism for passing custom data.

  2. Extend FleetAllocation to have some kind of configuration option to manipulate the label/annotation information on the GameServer before marking it as Allocated - this gives you a mechanism to basically pass arbitrary key value pairs down to the GameServer -- and should have some other nice other applications as well (and be introspectable by standard Kubernetes tooling)

Maybe something like:

apiVersion: "stable.agones.dev/v1alpha1"
kind: FleetAllocation
metadata:
  generateName: fleet-allocation-example-
spec:
  fleetName: fleet-example
  metadata:
     labels:
       foo: bar
     annotations:
       thing: stuff   

Wherein the metadata section would apply the labels and annotations to the GameServer object before moving it to an Allocated state. (open to better names that just metadata if anyone can think of one)

The interesting question will be - having a callback such that the SDK will send a notification whenever a State changes on the GameServer process. I think we can do that - and we should do it for all State changes, to make it as applicable to as many things as possible.

How does all that sound to you?

@defat
Copy link
Author

defat commented Jun 27, 2018

Yeah, this sounds extremely convenient, especially if annotations will be able to contain some complex structures such as json objects (maybe quoted to be strings).
Thank you for such fast reply!

@cyriltovena
Copy link
Collaborator

cyriltovena commented Jun 27, 2018

metadata on allocation is great, but callback is mandatory and we had an offline discussion with @EricFortin about that, we were thinking that the game server should also be able answer back to let know that it is ready, let's say it will load a new map and we want to receive player only when the map is loaded.

PS: @defat we're glad to see this feature request :)

@markmandel
Copy link
Member

@Kuqd agreed - the callback is a must. I have some thoughts there - the only potential issue I see is handling the REST version of a streaming gRPC from sidecar->process (rather than the other way around) - but i feel like someone should have solved this.

I also started #279 to tackle the propagation of events from the game server back up into Agones, so the design of two of these don't get intermixed.

@markmandel
Copy link
Member

Just to be mega clear, my thought was: on the callback - I basically figure that it will get called whenever the GameServer resource is updated, and then the callback code can look for differences and perform whatever action is deems necessary. Sounds good?

That way you could respond to just about any event - state changes, label changes, annotation changes, etc. Which is nice, as if you want to do stuff to the GameServer from your own code outside of Agones, you can still react to it in the game server process through the SDK.

WDYT?

@defat
Copy link
Author

defat commented Jul 4, 2018

Sorry for my late answer...

Having a callback on a dedicated server that's called on updates to GameServer resource sounds more flexible, but this introduces one more step in allocation process (option when session parameters are set separately from creating FleetAllocation). I'm really not sure about reasons for controlling application to modify any dedicated server state after allocation happend. Moreover "dedicated" means it really dedicates to particular game session with parameters that does not seem to need any changes during it's lifetime...
Another thing to consider is failing parameters to be updated when allocation is done.

So... As for me setting parameters in fleet allocation looks more reasonable, at least for now. Having a callback on a dedicated server that will be called on fleet allocation or any other situations in future seems the right way.

@markmandel markmandel self-assigned this Jul 11, 2018
@markmandel
Copy link
Member

First part of this work in progress here: https://github.com/markmandel/agones/tree/feature/sdk-gameserver-data

Being able to get at the GameServer data from within the SDK.

markmandel added a commit to markmandel/agones that referenced this issue Jul 15, 2018
…ration.

Extended the gRPC SDK to include a `GameServer()` function for both Go, C++
and REST by extending the backing `sdk.proto` and implementing the functionality
in the sdk sidecar.

This is the start of work needed for googleforgames#277.
markmandel added a commit to markmandel/agones that referenced this issue Jul 15, 2018
…ration.

Extended the gRPC SDK to include a `GameServer()` function for both Go, C++
and REST by extending the backing `sdk.proto` and implementing the functionality
in the sdk sidecar.

This is the start of work needed for googleforgames#277.
markmandel added a commit to markmandel/agones that referenced this issue Jul 17, 2018
…ration.

Extended the gRPC SDK to include a `GameServer()` function for both Go, C++
and REST by extending the backing `sdk.proto` and implementing the functionality
in the sdk sidecar.

This is the start of work needed for googleforgames#277.
markmandel added a commit that referenced this issue Jul 17, 2018
…ration.

Extended the gRPC SDK to include a `GameServer()` function for both Go, C++
and REST by extending the backing `sdk.proto` and implementing the functionality
in the sdk sidecar.

This is the start of work needed for #277.
@fdasoghe
Copy link

This is a very interesting feature for me as well.
Just a little question/doubt: since currently there's no way to pass metadata configuration to the FleetAllocation, how can be useful the SDK GameServer() function? I mean, GameServer() can return only standard Kubernetes metadata, which are not the focus of this feature request, or am I missing something?

@victor-prodan
Copy link
Contributor

GameServer call is very important for two reason:

  • it retrieves the public ip and port
  • it can be used to detect allocation by polling at regular intervals

So the most useful thing to do now is to ce able to pass metadata to the server with a fleet allocation

@fdasoghe
Copy link

GameServer call is very important for two reason:

  • it retrieves the public ip and port
  • it can be used to detect allocation by polling at regular intervals

Sorry but I don't get it. Why they are so important? And for who? The game manager already knows all of that (from the FleetAllocation creation response), and the game server itself should not need public ip and port. About the allocation via polling, I think they are already working on some event propagation specifically to notify the game server about its lifecycle (and events are much more preferable than polling, of course).

So the most useful thing to do now is to ce able to pass metadata to the server with a fleet allocation

Agree, and indeed I'm very eager to see it implemented because my current project needs it badly :-)

@victor-prodan
Copy link
Contributor

@think01 : not all games are the same, there are different flows that make sense in different conditions. in my case, it is very important for the server to know its own public ip and address.

Back on topic, I have given this some thought and I don't fully agree with @markmandel's initial proposal of overriding the server's labels and/or annotations. In my head, these should be read-only properties describing the fleet itself and that can be used for searching/filtering.

it makes poor sense to me to add a empty label just so it can be overwritten at allocation time. It's not a good separation of concerns imo.

I prefer adding an additional field, something like 'allocationData' or 'allocationMeta' that it set only by a fleet allocation and retrieved by GameServer call.

it would be set like this:

gsCopy := allocation.DeepCopy()
gsCopy.Status.State = stablev1alpha1.Allocated
gsCopy.Status.AllocationMeta = fa.Spec.AllocationMeta

What do you think?

@fdasoghe
Copy link

fdasoghe commented Jul 27, 2018

not all games are the same, there are different flows that make sense in different conditions. in my case, it is very important for the server to know its own public ip and address.

Fair point.

it makes poor sense to me to add a empty label just so it can be overwritten at allocation time. It's not a good separation of concerns imo.

Well, the empty label could not be necessary: a FleetAllocation could add a new label/annotation/whatever to the GameServer metadatas. Anyway I agree on your concerns about the proposed mechanism to use the standard metadata of the GameServer object: if every FleetAllocation apply its metadata to a GameServer, how can we know what are the valid values of the last allocation? it looks better to add an AllocationData that can be securely wiped when a GameServer gets disallocated.

EDIT: I just saw this #307 Good work! Exactly what's needed 👍

@markmandel
Copy link
Member

I'm writing notes on #307 - but I have to strongly disagree with creating a AllocationMeta.

Annotations and labels are not read only values - there are there to be mutated as need be. There are multiple cases within Kubernetes where changing label values is useful and encouraged.

If isolation is an issue, then use a prefix - and this is actually best practice as well for this very reason.

There is no need to set blank annotations/labels at creation time either. You can check to see if the labels exist if you need to know if they are populated.

Also, we would end up building all the same infrastructure that exists, specifically around labels (label selectors, filtering etc) into our new AllocationMeta - when we could leverage what exists in K8s already. This doesn't add any value, but creates more work.

@fdasoghe
Copy link

fdasoghe commented Jul 29, 2018

Uhm. Maybe I'm missing (or misunderstanding) the purpose of these "allocation metadata". For what I've understood so far (and my own point of view), these datas are generated by the Game Manager when it needs to start a new game session, allocating a new Game Server. In my use case, they are "domain data", that is they are specific to my game and their only purpose is to be consumed by the Game Server.

For this reason I considered a good idea to separate them from the general purpose Kubernetes metadata. Now, I understand and can agree with your last point, @markmandel about leveraging existing infrastructure and best practice about "standard" Kubernetes labels and annotations. But for what I've seen so far, the usual process in Kubernetes is to use annotations to quickly introduce a new feature (for example they did this for init containers, as detailed here https://kubernetes.io/docs/concepts/workloads/pods/init-containers/) and afterward migrate it to a more consolidated API with dedicated syntax.

Besides this, it seems unusual to me to use specific game's data session to filter game servers based on labels. Maybe a complete solution would be to have both capabilities: a way to assign standard labels and annotations and another way to pass (possibly more structured) allocation data. About this last point, annotations can contains only stringed key-value pairs. I know for sure in my use case this will be very cumbersome to overcome: I'll have to pass structured data marshalled in a string used as value in the annotation, very much like they did when introduced init container. It doesn't look like a suitable way to pass potential complex game session data to the game server.

Just my 2 cents.

@markmandel
Copy link
Member

@think01 thanks for the feedback -

My point re: prefixes still stands. You should use a prefix as standard practice (maybe this is reason that we should impose one?) this separates your annotations and metadata away from anything that K8s/GKE/other systems would provide.

Your point re: being able to pass unstructured data is totally valid (especially around forcing marshall/unmarshalling -- you can get around this by using multiple keys, but if you want to send an array, you are stuck), unfortunately - if we want to use the CRD to pass data through, then we will be limited to map[string]string data anyway - much as we are with standard annotations and labels, just because that is the most flexible data structure we can use in CRDs, regardless of whether we author them ourselves or not - there isn't an option to do a "just add whatever data structure you like".

My point would be - let's start with with K8s Annotations and Labels. Develop with them for a while, see what problems arise - and then we can always add more mechanisms if this doesn't work out. Start with the simple solution first, see what feedback we get, and then we can always add more from there.

It's much harder to remove a feature, than it is to add a new one later on.

Thoughts?

@markmandel
Copy link
Member

FYI, very early work on the gameserver change callback over here:
https://github.com/markmandel/agones/tree/feature/gs-callback

@fdasoghe
Copy link

@markmandel

My point would be - let's start with with K8s Annotations and Labels. Develop with them for a while, see what problems arise - and then we can always add more mechanisms if this doesn't work out. Start with the simple solution first, see what feedback we get, and then we can always add more from there.

Yes I totally agree. And to be honest for my project could be already enough: I can use annotations to pass a session token and some other string identifier and using some key-value store like Redis from which the Game Server can fetch more structured session data using ids from its annotations.

Cannot wait to start using it ;-)

@markmandel markmandel added this to the 0.4.0 milestone Jul 30, 2018
@victor-prodan
Copy link
Contributor

victor-prodan commented Jul 30, 2018 via email

cyriltovena pushed a commit to cyriltovena/agones that referenced this issue Jul 31, 2018
…ration.

Extended the gRPC SDK to include a `GameServer()` function for both Go, C++
and REST by extending the backing `sdk.proto` and implementing the functionality
in the sdk sidecar.

This is the start of work needed for googleforgames#277.
markmandel added a commit to markmandel/agones that referenced this issue Aug 8, 2018
This implements the gRPC system to send messages up to a unidirectional
stream from the sdk-server to the SDK.

This has been implemented in the Go, C++ and REST SDK.

Closes googleforgames#277
markmandel added a commit to markmandel/agones that referenced this issue Aug 10, 2018
This implements the gRPC system to send messages up to a unidirectional
stream from the sdk-server to the SDK.

This has been implemented in the Go, C++ and REST SDK.

Closes googleforgames#277
markmandel added a commit that referenced this issue Aug 10, 2018
This implements the gRPC system to send messages up to a unidirectional
stream from the sdk-server to the SDK.

This has been implemented in the Go, C++ and REST SDK.

Closes #277
victor-prodan pushed a commit to victor-prodan/agones that referenced this issue Sep 3, 2018
This implements the gRPC system to send messages up to a unidirectional
stream from the sdk-server to the SDK.

This has been implemented in the Go, C++ and REST SDK.

Closes googleforgames#277
victor-prodan pushed a commit to victor-prodan/agones that referenced this issue Sep 3, 2018
This implements the gRPC system to send messages up to a unidirectional
stream from the sdk-server to the SDK.

This has been implemented in the Go, C++ and REST SDK.

Closes googleforgames#277
victor-prodan pushed a commit to victor-prodan/agones that referenced this issue Sep 3, 2018
This implements the gRPC system to send messages up to a unidirectional
stream from the sdk-server to the SDK.

This has been implemented in the Go, C++ and REST SDK.

Closes googleforgames#277
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/user-experience Pertaining to developers trying to use Agones, e.g. SDK, installation, etc kind/design Proposal discussing new features / fixes and how they should be implemented kind/feature New features for Agones
Projects
None yet
Development

No branches or pull requests

5 participants