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

Add support for service discovery on Kubernetes #129

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jocke-l
Copy link

@jocke-l jocke-l commented Oct 25, 2024

As part of a work assignment I added support for updating the proxy topology at runtime based on Kubernetes' EndpointSlices.

Even though this have not been discussed in any issue previously, I thought I'd just create the PR directly since most of the work is already done.

This functionality will also simplify #89.

How it works

ZDM_PROXY_TOPOLOGY_KUBERNETES_SERVICE acts as a feature flag. When active (it exists and is not empty), it should contain the name of a headless service that points to all the zdm-proxy pods. This service must be created in the same namespace as the zdm-proxy pods.

zdm-proxy will then continuously watch all EndpointSlices associated with this service and broadcast events to all clients. This is done by creating and starting TopologyRegistry on the ZdmProxy instance.

zdm-proxy partially intercepts REGISTER messages from the clients in order to keep track of which events the clients subscribes to. The relevant event types are TOPOLOGY_CHANGE and STATUS_CHANGE.

A separate goroutine per ClientHandler continuously receives events from the watcher and translates them to Cassandra Native Protocol event frames, which are then sent back to the client if the client had the event type registered.

Each ControlConnection also subscribes to the TopologyRegistry in order to recalculate the contents of ControlConnection.virtualHosts which, when this feature is enabled, will be based on the endpoints of each EndpointSlice.

What is missing

Currently there are no automatic tests for this feature, this is mainly because these would require a Kubernetes cluster. While it is possible to create a locally running Kubernetes cluster, or a mocked apiserver, similar to how the kubebuilder project does it, I have not given this much thought yet.

The code could also quite easily be generalized to support other service discovery mechanisms in the future, such as Consul.

@lukasz-antoniak
Copy link
Contributor

lukasz-antoniak commented Oct 30, 2024

Interesting feature. Few unrelated ideas:

  • I would add a generic interface for peers discovery and add static implementation that relies on configuration string & index number, and Kubernetes service implementation.
  • Should we add a debouncer like in most CQL drivers to accumulate add & remove events and be resilient to short connectivity issues.

func (tr *TopologyRegistry) runInformer(ctx context.Context) {
informerFactory := informers.NewFilteredSharedInformerFactory(
tr.clientset,
5*time.Minute,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we should make refresh period configurable.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, maybe. But this doesn't affect how often the topology is updated, but rather how often the informer refreshes its internal cache of kubernetes resources. The interval should be determined by trade-off between impact of a stale cache and load on the kubernets API. 5 minutes seems to be standard for most usecases.

@jocke-l
Copy link
Author

jocke-l commented Nov 2, 2024

@lukasz-antoniak

* I would add a generic interface for peers discovery and add static implementation that relies on configuration string & index number, and Kubernetes service implementation.

I agree. I will work on this when I have time.

* Should we add a debouncer like in most CQL drivers to accumulate add & remove events and be resilient to short connectivity issues.

Is this really necessary? EndpointSlices are already based on running pods and any readiness probes. If these are configured correctly, the debouncer functionality is already covered.

@jocke-l jocke-l closed this Nov 2, 2024
@jocke-l jocke-l reopened this Nov 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants