-
Notifications
You must be signed in to change notification settings - Fork 858
Description
Overview
The agones-controller
binary currently hosts a collection of k8s related things:
- control loops for
GameServer
,GameServerSet
, etc. - mutating and validating webhooks for the same
- the
v1.allocation.agones.dev
extension API
Currently, agones-controller
is run as a Deployment
with replicas=1
, and (unless configured otherwise) sets safe-to-evict=false
to prevent arbitrary eviction. In this design, we propose:
- splitting
agones-controller
intoagones-controller
andagones-extensions
(feel free to bikeshed a better name)agones-controller
would be a pure controller and consist only of the control loops (think:agones-controller
would have a Kubernetes client, but would not be an endpoint for any services except a local healthcheck)agones-extensions
would be an HA service that handles webhooks and extension APIs (think: everything in extensions.yaml)
Splitting agones-controller
then allows us to:
- Scale the (new)
agones-extensions
Deployment
arbitrarily on larger clusters (in case webhook / extension API traffic needs horizontal scaling). We should start withreplicas=2
and aPodDisruptionBudget
, though, to allow for eviction during upgrades/autoscaling. - Remove
safe-to-evict=false
fromagones-controller
, and either just let it be, if we think pod restart time after eviction is fast enough, or configure it to runreplicas=2
and add leader-election, which is a common controller pattern.
Detailed Phases
Phase 1: Split agones-extensions
services out of agones-controller
#
- Create a new
agones-extensions
binary and image with just the webhooks and extension APIs fromagones-controller
. It would not be deployed until we .... - Add a new
SplitControllerAndExtensions
feature flag tovalues.yaml
- In
agones-controller
,SplitControllerAndExtensions
would disable the pieces thatagones-extensions
serves. - Split
controller.yaml
: IfSplitControllerAndExtensions
is on, we would create twoDeployments
instead,agones-controller
andagones-extensions
. - The labels and service need to be changed. Ideally we would change
agones-controller-service
toagones-extension-service
in the split mode, changing extensions.yaml as well. This could be done as a follow-on, though - but the service selector labels definitely need to be differentiated. - We also should change the
agones-extensions
Deployment
toreplicas=2
and add aPodDisruptionBudget
foragones-extensions
, because we can.
- In
Sidebar: It's not clear how long we would want to support SplitControllerAndExtensions
, or whether this is just transitory until this phase is done. I am proposing it as a chicken switch if something breaks. I would be supportive of removing it shortly after the phase is completed - the cost of maintaining 3 deployments (1 old, 2 new) is higher, and if we keep all 3 around for a while, we should have a unit test that the deployments match except for flags.
Phase 2: Leader election of agones-controller
#
One theory is that we don't need leader election because the pod will get rescheduled anyways. My sense is that we should run with replicas=2
and leader elect, as it protects against weird cases where the pod may be evicted but can't run for a while (due to misconfiguration or reliability). The main argument is that with leader election, replicas=2
and a PodDisruptionBudget
, we don't have to reason about pod startup speed affecting controller latency, which is a win.
So, I think we should:
- Add a boolean
--leader-election
flag toagones-controller
. If enabled, we run a simple leader election callback that crashes the pod (this simplicity is why we're doing all this work!). - Add a
replicas
config to values.yaml.- If
replicas>1
, we:- Default to
--leader-election=true
- Add a
PodDisruptionBudget
- Remove
safe-to-evict=false
- Default to
- If
- In the
SplitControllerAndExtensions=true
case, we default toreplicas=2
Alternatives Considered
Instead of splitting the binaries, what else did I consider?
-
Remove
safe-to-evict=false
fromagones-controller
and nothing else: The problem with this is that typical Kubernetes control loops can be a little loose with timing, but services really can't be. If the webhook or extension services can't be reach, the client will see errors. REJECTED. -
Bump replicas of
agones-controller
toN
and nothing else: Most Kubernetes control loops are designed to "own" the control loop. In particular, ifN>1
informers all see a change and react to it, they will inevitably fight with one another. At best, this is a waste of time, and at worst, it could exacerbate correctness issues. This is typically why control loops are leader elected. -
Bump replicas of
agones-controller
toN
and use leader election, don't separate services to a different deployment: Ok, I am certain this could work in some fashion, and equally certain there are some binaries out there that implement this pattern. The leaderelection library can be used in pretty arbitrary ways, and if, for instance,OnStoppedLeading
were to terminate the control loops </furious handwaving, but probably some sort of context cancellation approach would be best> and leave the services, this approach could work just fine. We argue that the complexity is not worth it.