Skip to content

Design: Make it possible to evict agones-controller by separating into controller/extensions #2797

@zmerlynn

Description

@zmerlynn

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 into agones-controller and agones-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 with replicas=2 and a PodDisruptionBudget, though, to allow for eviction during upgrades/autoscaling.
  • Remove safe-to-evict=false from agones-controller, and either just let it be, if we think pod restart time after eviction is fast enough, or configure it to run replicas=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 from agones-controller. It would not be deployed until we ....
  • Add a new SplitControllerAndExtensions feature flag to values.yaml
    • In agones-controller, SplitControllerAndExtensions would disable the pieces that agones-extensions serves.
    • Split controller.yaml: If SplitControllerAndExtensions is on, we would create two Deployments instead, agones-controller and agones-extensions.
    • The labels and service need to be changed. Ideally we would change agones-controller-service to agones-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 to replicas=2 and add a PodDisruptionBudget for agones-extensions, because we can.

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 to agones-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
  • In the SplitControllerAndExtensions=true case, we default to replicas=2

Alternatives Considered

Instead of splitting the binaries, what else did I consider?

  • Remove safe-to-evict=false from agones-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 to N and nothing else: Most Kubernetes control loops are designed to "own" the control loop. In particular, if N>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 to N 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.

Metadata

Metadata

Labels

kind/designProposal discussing new features / fixes and how they should be implementedkind/featureNew features for Agones

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions