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

[jaeger] Have an ability to make collector svc headless #136

Merged
merged 2 commits into from
Nov 8, 2022
Merged

[jaeger] Have an ability to make collector svc headless #136

merged 2 commits into from
Nov 8, 2022

Conversation

spacentropy
Copy link
Contributor

Hi.
gRPC client balancing requires headless services in order to work. More info here - https://kubernetes.io/blog/2018/11/07/grpc-load-balancing-on-kubernetes-without-tears/
This change allows to set collector service type to headless. We use it in production without problems.

Copy link
Member

@naseemkullah naseemkullah left a comment

Choose a reason for hiding this comment

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

Thanks!

Left one suggestion to wrap the rendered value around an if statement.

Also please add the value in the REAME's values table.

Base automatically changed from master to main February 27, 2021 18:28
@naseemkullah naseemkullah changed the title Have an ability to make collector svc headless [jaeger] Have an ability to make collector svc headless Feb 27, 2021
@pavelnikolov
Copy link
Contributor

@spacentropy are you planning to add the if statement that @naseemkullah has suggested above?

@spacentropy
Copy link
Contributor Author

Hi. Sorry for the Loooooong delay. I've rebased the branch and applied the suggested changes.

Thanks!

@spacentropy
Copy link
Contributor Author

Hi! Let's return to this. I've made the requested changes. Can we proceed to merging this? @pavelnikolov

Signed-off-by: Gleb Lesnikov <g.lesnikov@dodopizza.com>
Signed-off-by: Gleb Lesnikov <g.lesnikov@dodopizza.com>
@spacentropy
Copy link
Contributor Author

@pavelnikolov Thanks for the approval! Can you please merge? I've rebased & signed commits.

@pavelnikolov pavelnikolov merged commit b2373eb into jaegertracing:main Nov 8, 2022
@spacentropy spacentropy deleted the collector-clusterip branch November 8, 2022 12:47
@indreek
Copy link

indreek commented Apr 26, 2023

@pavelnikolov Why did you remove loadBalancerIP field? If I plan to run only one instance of collector, i don't need Linkerd in front. It would be fully customizable if you wish to use k8s lb.

{{- if and (eq .Values.collector.service.type "LoadBalancer") .Values.collector.service.loadBalancerIP }}
  loadBalancerIP: {{ .Values.collector.service.loadBalancerIP }}
{{- end -}}

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.

4 participants