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 KubernetesEndpointGroup #5001

Merged
merged 16 commits into from Apr 9, 2024
Merged

Add KubernetesEndpointGroup #5001

merged 16 commits into from Apr 9, 2024

Conversation

ikhoon
Copy link
Contributor

@ikhoon ikhoon commented Jun 30, 2023

Motivation:

It is tricky to send requests to a Kubernetes cluster from outside servers without ingress.
There is no way to send traffic directly to the pod, but we can send traffic to the port of nodes (NodePort) where the pods are located.

This PR proposes a new EndpointGroup that can send requests with CSLB using NodeIP and NodePort to pods in Kubernetes. This way is not an ideal CSLB where servers and clients communicate directly, but it will be a safer way to send traffic without going through ingress which can be SPOF.

Modifications:

  • Add KubernetesEndpointGroup on top of KubernetesClient to dynamically obtain Kubernetes resources.
    • Permission to watch services, nodes, pods is required to fetch endpoints.
    • service.ports[*].nodePort is used to create the port of Endpoint.
  • Watch API is used to track changes in Kubernetes with a minimal delay.
    • ADDED and MODIFIED events are used to update resouces.
    • DELETED is used to remove the resouce.
    • BOOKMARK event is not used and ERROR may be ignorable.
  • Test KubernetesEndpointGroup with both a real Kubernetes cluster and a mock Kubernetes server.

Result:

// Create a KubernetesEndpointGroup that fetches the endpoints of the 'my-service' service in the 'default'
// namespace. The Kubernetes client will be created with the default configuration in the $HOME/.kube/config.
KubernetesClient kubernetesClient = new KubernetesClientBuilder().build();
KubernetesEndpointGroup
  .builder(kubernetesClient)
  .namespace("default")
  .serviceName("my-service")
  .build();

// If you want to use a custom configuration, you can create a KubernetesEndpointGroup as follows:
// The custom configuration would be useful when you want to access Kubernetes from outside the cluster.
Config config =
  new ConfigBuilder()
    .withMasterUrl("https://my-k8s-master")
    .withOauthToken("my-token")
    .build();
KubernetesEndpointGroup
  .builder(config)
  .namespace("my-namespace")
  .serviceName("my-service")
  .build();

@ikhoon
Copy link
Contributor Author

ikhoon commented Jul 5, 2023

I managed to finish implementing our own Kubernetes authentications by porting https://github.com/kubernetes-client/java. However, I'm not sure the code is maintainable when there are new changes on the Kubernetes side.

On second thought, it would be much better to use a well-known Kubernetes client implementation instead. https://github.com/kubernetes-client/java is hard-coded with OkHttp but https://github.com/fabric8io/kubernetes-client has an integration layer for various client implementations.

I filed fabric8io/kubernetes-client#5307 to support for Armeria backend. I will continue working on this PR once the Armeria backend for the Fabric Kubernetes client is implemented. It will make this PR simple and easy to review for maintainers.

@ikhoon ikhoon changed the title Add K8sEndpointGroup Add KubernetesEndpointGroup Mar 12, 2024
@ikhoon ikhoon added this to the 1.28.0 milestone Mar 12, 2024
@ikhoon ikhoon marked this pull request as ready for review March 12, 2024 09:07
Copy link
Member

@minwoox minwoox left a comment

Choose a reason for hiding this comment

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

Great job! 🚀 🚀 🚀

Copy link
Contributor

@jrhee17 jrhee17 left a comment

Choose a reason for hiding this comment

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

Only nits for me 👍 Thanks @ikhoon 🙇 👍 🙇

public KubernetesEndpointGroup build() {
checkState(serviceName != null, "serviceName not set");
return new KubernetesEndpointGroup(kubernetesClient, namespace, serviceName, portName, autoClose,
selectionStrategy, shouldAllowEmptyEndpoints(),
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks likely that either Watcher<Pod> or Watcher<Node> will emit an event.
However, both events are required to construct endpoints in maybeUpdateEndpoints.

Since most users wait for EndpointGroup#whenReady to validate the endpoint before using it, I wonder if setting allowEmptyEndpoints=false is a more sensitive default. I think as it stands, the initial endpoints will almost always be an empty list.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed.

.map(NodeAddress::getAddress)
.findFirst().orElse(null);
if (nodeIp == null) {
logger.debug("No 'InternalIP' is found in {}. node: {}", nodeName, node);
Copy link
Contributor

Choose a reason for hiding this comment

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

Question) Is there no need to delete the nodeToIp entry in this case since there is no valid nodeip?

case ADDED:
case MODIFIED:
final String nodeIp = node.getStatus().getAddresses().stream()
.filter(address -> "InternalIP".equals(address.getType()))
Copy link
Contributor

Choose a reason for hiding this comment

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

Question) Does ExternalIP/HostName not work? I think it's fine to only handle InternaIP for now, but just in case someone asks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In fact, I don't know much about how Kubernetes operates and which IP is preferred.

Let me move this filter to the builder. InternalIP will be chosen by default but overridable.

Copy link
Contributor

@jrhee17 jrhee17 left a comment

Choose a reason for hiding this comment

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

Still looks good 👍 👍 👍

@ikhoon
Copy link
Contributor Author

ikhoon commented Apr 2, 2024

There are leak reports in CI builds which turned out to be bugs in tests of https://github.com/fabric8io/kubernetes-client. I will disable the tests until the upstream fixes it.
fabric8io/kubernetes-client#5852

@ikhoon
Copy link
Contributor Author

ikhoon commented Apr 3, 2024

I found another leak in tests fabric8io/kubernetes-client#5854

@ikhoon ikhoon merged commit 388a328 into line:main Apr 9, 2024
16 checks passed
@ikhoon ikhoon deleted the k8s-endpointgroup branch April 9, 2024 08:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add K8sEndpointGroup
3 participants