spring-cloud / spring-cloud-kubernetes

Kubernetes integration with Spring Cloud Discovery Client, Configuration, etc...
Apache License 2.0
3.45k stars 1.03k forks source link

Support services with multiple ports in KubernetesDiscoveryClient #104

Closed manderson23 closed 5 years ago

manderson23 commented 7 years ago

I have a zookeeper Service configured as follows:

apiVersion: v1
kind: Service
metadata:
  name: zoo1
  labels:
    app: zookeeper-1
spec:
  ports:
  - name: client
    port: 2181
    protocol: TCP
  - name: follower
    port: 2888
    protocol: TCP
  - name: leader
    port: 3888
    protocol: TCP
  selector:
    app: zookeeper-1

In this case the K8 client finds an Endpoint for zoo1 where the EndpointSubset has one address and three ports.

A call to discoveryClient.getInstances("zoo1") returns the leader port only whereas I would like the client port.

Should KubernetesDiscoveryClient return three ServiceInstance in this case? One for each port?

spencergibb commented 7 years ago

Are the services on each port distinct? We do something similar in s-c-consul and s-c-zookeeper if the management port is set.

manderson23 commented 7 years ago

ZK deployment and service is configured in a similar manner to the yaml within spring-cloud-dataflow-server-kubernetes.

There are then 3 instances of each ZK service/deployment pair.

spencergibb commented 7 years ago

@iocanel thoughts?

iocanel commented 7 years ago

In theory we could return all service instances and add the port name as part of the metadata, so that the user can filter ports if needed.

The thing is that the discovery client is not just something used by the user (who can adapt to whatever convention we use), but is also used by other parts of the ecosystem, that will fail to properly handle the convention.

So I really don't know what's best and would adopt whatever practice 'spring cloud' uses for similar cases.

Now, specifically for zookeeper in kubernetes, I find it really handy to define the client port separately from peer, and election ports. Why? Because for peer and election ports you need reference them by node id (and that's why you define one service per node), while for client port this is not the case. Here's an example: https://github.com/fabric8io/fabric8-ipaas/tree/master/zookeeper-ensemble/src/main/fabric8

Zetten commented 5 years ago

I have a branch at https://github.com/Zetten/spring-cloud-kubernetes/tree/ports-as-metadata which simply adds each named port to the metadata map as port.<port-name>.

This meets my particular need, which was previously handled with Eureka instance metadata: my service exposes both HTTP and gRPC endpoints, and I'd like to be able to select between them.

I'm happy to open a PR for this small change, but I see there's some other discussion about the exact mapping of ServiceInstance metadata to Kubernetes service attributes (e.g. #260, #166) so I'm not sure how this fits in there.

Another option is to add a new Map<String, Integer> ports to the KubernetesServiceInstance type. This could then be configurably added to the map returned by getMetadata. This approach might also meet the distinction between labels vs. annotations, i.e. the ultimate metadata map can be an ordered merge of labels + annotations + ports.

spencergibb commented 5 years ago

Once #260 is taken care of, a PR would be great. I see it as something merged from possibly multiple sources.