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

Query service is headless #264

Closed
m1o1 opened this issue Mar 5, 2019 · 13 comments
Closed

Query service is headless #264

m1o1 opened this issue Mar 5, 2019 · 13 comments
Assignees

Comments

@m1o1
Copy link

m1o1 commented Mar 5, 2019

I created a Jaeger instance with:

apiVersion: io.jaegertracing/v1alpha1
kind: Jaeger
metadata:
  name: simple-prod
spec:
  strategy: production
  ingress:
    enabled: false
  storage:
    type: elasticsearch
    options:
      es:
        server-urls: http://elasticsearch:9200

This created headless services for the query and collector service. Istio has trouble routing to headless services with VirtualServices, so I was not able to route to the Jaeger UI with one. I disabled ingresses on the Jaeger object because Istio uses Gateways/VirtualServices instead of the Kubernetes Ingress object.

@jpkrohling
Copy link
Contributor

@objectiser do you remember if there's a reason to have the services as headless?

@objectiser
Copy link
Contributor

There is a PR istio/istio#9508 that has been open for some time that provides ingressgateway and some other common services required within Istio, that works in conjunction with the Jaeger operator.

@m1o1
Copy link
Author

m1o1 commented Mar 5, 2019

@objectiser I don't see a Gateway or VirtualService looking at the changed files. Where can I find the config for this ingressgateway? We're trying to add the route to our existing ingressgateway (instead of creating a new one, since for now we don't want more than 1 external IP address) but ran into this issue.

@objectiser
Copy link
Contributor

The gateway and virtualservice are already defined here, so this PR just replaces the Jaeger impl from all-in-one template to using the joperator.

The definitions at the specific location are all generic, intended to be independent of the backend provider that has been select (e.g. jaeger or zipkin - but more may be added). So there are two generic services, zipkin provides the endpoint for reporting zipkin complaint span data and tracing provides access to the UI. But if you are defining your own gateway you could map it directly onto the jaeger-query.

@m1o1
Copy link
Author

m1o1 commented Mar 19, 2019

Thank you @objectiser - The tracing service itself is not headless, so that must be why it works with the VirtualService. Is Istio just ignoring the query service that the Jaeger operator makes or is it somehow disabling it (in "production" mode)? I'll try doing that too.

I still believe that the jaeger operator should not be making headless services though (except for elasticsearch/cassandra perhaps - but the jaeger-query and jaeger-collector services should not be) and that this is a bug.

@jpkrohling
Copy link
Contributor

jpkrohling commented Mar 19, 2019

@pavolloffay has mentioned a blog post which suggests that we need headless services for proper load balancing: https://medium.com/google-cloud/loadbalancing-grpc-for-kubernetes-cluster-services-3ba9a8d8fc03

For context, starting with #333 we'll start using gRPC's load balancing feature using DNS as the service discovery mechanism.

@m1o1
Copy link
Author

m1o1 commented Mar 19, 2019

Oh I see - that makes sense - but is that necessary for the Query service? That will only be accessed by a browser over HTTP(S) right (or are you using grpc-web or something for this)?

Sorry I'm not sure why I automatically included the collector service in my description. I only really need the query service as non-headless.

@m1o1 m1o1 changed the title Query and collector services are headless services Query service is headless Mar 19, 2019
@jpkrohling
Copy link
Contributor

For the query service, it actually makes sense. We'd most likely not need load balancing in front of the query anyway, as it's a low traffic service. If we ever need load balancing for that, we could create an internal-only headless service.

@m1o1
Copy link
Author

m1o1 commented Mar 20, 2019

Ok - also based on @objectiser's comment, I actually do think we would need the collector to be non-headless, since Istio is reporting the spans via HTTP on port 9411, and we are using Istio. Unless I misunderstood that. I'll hold off re-editing this issue title back to include the collector for now.

@jpkrohling
Copy link
Contributor

Sorry, we should have kept the discussion here. I have a branch that produces two services for the collector, one headless and one with a cluster IP. Would you be able to test this branch? You could just deploy the simple-prod example setting the number of replicas of the collector to 2 and generate some traffic. Then, verify that all collector instances got spans by running something like this:

$ kubectl port-forward simple-prod-collector-7798c588f7-p9pgn 14268:14268
$ curl localhost:14268/metrics 2>/dev/null | grep jaeger_collector_spans_received_total 

@jpkrohling jpkrohling self-assigned this Mar 20, 2019
@objectiser
Copy link
Contributor

@jpkrohling Wondering whether it would be better to have the default name used by collector and query to be load balanced, as they are more likely to be used by end users, and then the headless (collector) service have the modified name, as it is only going to be used internally between the agent and collector?

@jpkrohling
Copy link
Contributor

${instance_name}-collector-internal for the headless?

@objectiser
Copy link
Contributor

Or maybe be more explicit ${instance_name}-oollector-headless ?

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

No branches or pull requests

3 participants