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

Move SDK server code from pkg/gameservers to a separate package #445

Closed
jkowalski opened this issue Dec 14, 2018 · 4 comments
Closed

Move SDK server code from pkg/gameservers to a separate package #445

jkowalski opened this issue Dec 14, 2018 · 4 comments
Labels
kind/cleanup Refactoring code, fixing up documentation, etc kind/design Proposal discussing new features / fixes and how they should be implemented
Milestone

Comments

@jkowalski
Copy link
Contributor

Right now pkg/gameservers implements two disjoint features:

  • controller for GS
  • SDK server and local SDK server

I think we need to keep SDK binary small and free of dependencies it does not need (to reduce attack surface and memory footprint). Right now we're relying on linker to exclude controller-related code from sdk-server binary. While the linker is presently doing a good job, it's very easy to accidentally add an init() function which will bring unwanted dependency on controller code to SDK binary.

I suggest we move sdk.go and localsdk.go and related files to pkg/sdkserver before they become entangled.

@markmandel markmandel added kind/cleanup Refactoring code, fixing up documentation, etc kind/design Proposal discussing new features / fixes and how they should be implemented labels Dec 14, 2018
@markmandel
Copy link
Member

Hey @enocom my resident go champion 😄 what are your thoughts?

Is this a code review issue, or something you think we should take proactive action on?

I don't think I have strong opinions either way to be honest - but the original organisational model idea was to match packages to the K8s resources that they were responsible for -- so in this case the sdkserver modified the GameServer resource, so it goes in the gameservers package.

@enocom
Copy link
Contributor

enocom commented Dec 17, 2018

Splitting out the SDK server and local SDK server seems like a good idea. Their responsibilities also seem distinct from the controller.

Are there other examples in the code where a Kubernetes controller gets its own package?

@markmandel
Copy link
Member

So in K8s, all the controllers can be found in subpackages under https://github.com/kubernetes/kubernetes/tree/master/pkg/controller - so organising by type of object. Which has it's own pros and cons!

@enocom
Copy link
Contributor

enocom commented Dec 17, 2018

At this point, it seems more a matter of style. If there consumers of just the controller or just the SDK server, it might be easier to see the benefits of this refactor.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/cleanup Refactoring code, fixing up documentation, etc kind/design Proposal discussing new features / fixes and how they should be implemented
Projects
None yet
Development

No branches or pull requests

3 participants