quarkusio / quarkus

Quarkus: Supersonic Subatomic Java.
https://quarkus.io
Apache License 2.0
13.82k stars 2.69k forks source link

Rest Client Kubernetes service discovery #11358

Closed jclingan closed 1 year ago

jclingan commented 4 years ago

Description MicroProfile Rest Client's @RegisterRestClient uses either an annotation parameter (baseUri) or config property to point to the backend REST service's location. On Kubernetes, this can be determined by looking up the backend REST service's Kubernetes Service object using the Kubernetes Client API, which contains exposed ports, IP addresses, etc.

Implementation ideas A couple of ideas:

  1. Extend @RegisterRestClient annotation with an optional parameter. @RegisterRestClient(service="backend-service")

This would be interesting to add to the MicroProfile Config spec in the future (albeit broader in scope than just Kube), so this approach is interesting but perhaps a step too far at this stage.

  1. Create a new annotation, similar to Micronaut Service Discovery. @Client("backend-service")

This approach decouples itself from @RegisterRestClient annotation, and perhaps the SmallRye implementation uses this value, if provided, instead of looking up /mp-rest/url property.

quarkusbot commented 4 years ago

/cc @geoand /cc @phillip-kruger

geoand commented 4 years ago

Thanks for reporting John. I'll see what this implies hopefully sometime this week or the next.

rquinio commented 4 years ago

Just to clarify, is this for headless K8s services (ClusterIP=None), for which client-side load balancing towards the endpoints would have to be implemented in MP REST client (round-robin between available endpoints ?) ?

Because for full-blown services (i.e. with a k8s load balancer), it's possible today to use KubeDNS resolution:

/mp-rest/url=http://backend-service:8080/
jclingan commented 4 years ago

@rquinio This is still intended to be server-side LB. The primary point is that the information in the /mp-rest/... URL is available via the Kubernetes Service. Why not just use the information from the Service instead of requiring the developer to define a property? It's simply a developer joy/convenience RFE.

geoand commented 4 years ago

cc @kenfinnigan

geoand commented 3 years ago

@michalszynkiewicz maybe this is something you want take into accout if you like at rebasing the MP REST Client when Quarkus REST is in?

michalszynkiewicz commented 3 years ago

That's interesting, I made myself a note to take a look at it but won't assign it to me in case someone else would like to do it before I have time for it.

geoand commented 3 years ago

@michalszynkiewicz is this on your radar?

michalszynkiewicz commented 3 years ago

still in my todo list, and still I don't want to assign it to myself in case someone else would have time for it earlier

geoand commented 3 years ago

and still I don't want to assign it to myself in case someone else would have time for it earlier

seems rather unlikely at this point :)

michalszynkiewicz commented 3 years ago

I think we will cover it with https://github.com/smallrye/smallrye/issues/52

geoand commented 3 years ago

This has since been covered, right?

michalszynkiewicz commented 3 years ago

Yes and no. We have means to configure the client for kube now with Stork, but it requires a few lines of configuration. I think this issue was rather to make great UX, this is definitely something we could improve.

We could do sth like:

WDYT @cescoffier @aureamunoz ?

aureamunoz commented 3 years ago

It sounds nice, I like the idea. For me and what I have in mind it seems doable so it would worth give a try and have some more concrete feedback. I agree with the idea to improve the configuration part.

michalszynkiewicz commented 3 years ago

Maybe @StorkService would actually be better. It could be used for both gRPC and Rest Client Reactive (and any new integrations).

@cescoffier you once mentioned a simplified config, is this what you had in mind?

cescoffier commented 3 years ago

That's an interesting idea. It would reduce the configuration complexity.

michalszynkiewicz commented 2 years ago

@aureamunoz @cescoffier we are quite close with what we have now with Stork, I think. But it surely can be improved. I removed myself from assignees

cescoffier commented 1 year ago

Handled by Stork - stork://backend-service