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

Support custom address broadcasting for ringpop to work in k8s #6288

Conversation

taylanisikdemir
Copy link
Member

@taylanisikdemir taylanisikdemir commented Sep 17, 2024

What changed?
Current ringpop initialization is making it difficult to run ringpop in K8s like environments. Ringpop is initialized with the address that the service instance (container) is going to listen on and it gets advertised to peers. This is problematic because listen address of replicas are not unique and cannot be used to communicate peer to peer.
Local listen address (BIND_ON_IP) should be set to 0.0.0.0 and the pod's ip address (10.x.y.z) should be used to join the ring for p2p. Pod's IP address is accessible by other pods in the k8s cluster.

This PR introduces ringpop.broadcastAddress config which will make ringpop work in k8s environments.

For example:

  • History-service-container-replica-1:
    listens and serves on 0.0.0.0:7934
    broadcasts itself to peers as 10.66.1.71 (its own pod ip)

  • History-service-container-replica-2:
    listens and serves on 0.0.0.0:7934
    broadcasts itself to peers as 10.66.1.75 (its own pod ip)

A headless k8s service can be used for discovery which will return list of pod ips 10.x.y.z.

This can be achieved on K8s by passing pod's IP address as an env variable.

 - name: BIND_ON_IP
    value: 0.0.0.0
 - name: BROADCAST_ADDRESS
    valueFrom:  
       fieldRef:
          fieldPath: status.podIP

Ringpop config would look like this:

ringpop:
    name: cadence
    broadcastAddress: 10.66.1.71
    bootstrapMode: dns
    bootstrapHosts:
    - cadence-history-headless.namespace.svc.cluster.local:7934
   ..

Other misc changes:

  • Added backoff retry for ringpop bootstrap initialization
  • Added self addr/identity/ip tags to hashring view log

How did you test it?

  • Locally built a cadence image and tried it on a K8s cluster.
  • Added unit tests for ringpop provider

Copy link

codecov bot commented Sep 17, 2024

Codecov Report

Attention: Patch coverage is 77.77778% with 6 lines in your changes missing coverage. Please review.

Project coverage is 73.20%. Comparing base (2458cad) to head (9c1be1c).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
common/peerprovider/ringpopprovider/provider.go 68.42% 3 Missing and 3 partials ⚠️
Additional details and impacted files
Files with missing lines Coverage Δ
common/membership/hashring.go 87.22% <100.00%> (+0.51%) ⬆️
common/peerprovider/ringpopprovider/config.go 83.73% <ø> (+3.25%) ⬆️
common/peerprovider/ringpopprovider/provider.go 55.86% <68.42%> (+51.95%) ⬆️

... and 4 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2458cad...9c1be1c. Read the comment docs.

@taylanisikdemir taylanisikdemir changed the title Support custom address broadcasting for ringpop to work in k8s [WIP] Support custom address broadcasting for ringpop to work in k8s Sep 17, 2024
@davidporter-id-au
Copy link
Member

Is there any kind of K8s concept of hostname that can be used to refer to the instance rather than introducing an awareness of the container's own hostport/ip? I'm not sure this would work in mesh-networks (Not that I have any experience with them)?

also, why do we need to bind on 0.0.0.0? Wouldn't binding on the same IP as what's exposed work?

@taylanisikdemir
Copy link
Member Author

Is there any kind of K8s concept of hostname that can be used to refer to the instance rather than introducing an awareness of the container's own hostport/ip? I'm not sure this would work in mesh-networks (Not that I have any experience with them)?

also, why do we need to bind on 0.0.0.0? Wouldn't binding on the same IP as what's exposed work?

This setup is more typical in K8s. Binding on 0.0.0.0 makes it easier to serve requests targeted to IPs/dns etc. Also makes it possible to port-forward for debugging.
There's a concept of pod name which can be passed as env variable similar to pod ip. It can't be used as resolved address. Pod IP is the way to go because that's what headless k8s service will return (used by dns provider)

@taylanisikdemir taylanisikdemir force-pushed the taylan/ringpop_broadcast_addr branch from f473e25 to d864fd1 Compare September 17, 2024 21:02
@taylanisikdemir taylanisikdemir changed the title [WIP] Support custom address broadcasting for ringpop to work in k8s Support custom address broadcasting for ringpop to work in k8s Sep 17, 2024
@taylanisikdemir taylanisikdemir merged commit 35a8a9c into cadence-workflow:master Sep 17, 2024
20 checks passed
@taylanisikdemir taylanisikdemir deleted the taylan/ringpop_broadcast_addr branch September 17, 2024 22:12
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.

3 participants