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] make replicas a Helm value instead of hardcoding it #494

Merged

Conversation

lisenet
Copy link
Contributor

@lisenet lisenet commented Aug 9, 2023

What this PR does

Adds a Helm value for allInOne.replicas.

Which issue this PR fixes

Checklist

  • DCO signed
  • Commits are GPG signed
  • Chart Version bumped
  • Title of the PR starts with chart name ([jaeger] or [jaeger-operator])
  • README.md has been updated to match version/contain new values

Signed-off-by: lisenet <tomas@lisenet.com>
Signed-off-by: lisenet <tomas@lisenet.com>
@lisenet lisenet force-pushed the feature-add-replicas-as-helm-value branch from bcd48fc to b626e41 Compare August 9, 2023 11:07
@pavelnikolov
Copy link
Contributor

@lisenet it doesn't make sense to use the all-in-one Jaeger with more than one replica since the data is in-memory. Therefore, different replicas wouldn't share the data between them. Am I missing something?

@mehta-ankit mehta-ankit merged commit 4e702e2 into jaegertracing:main Aug 9, 2023
3 checks passed
@pavelnikolov
Copy link
Contributor

@mehta-ankit does it make sense to have more than one replica of the all-in-one container? I think it doesn't make sense and this PR shouldn't have been merged.

@mehta-ankit
Copy link
Member

@mehta-ankit does it make sense to have more than one replica of the all-in-one container? I think it doesn't make sense and this PR shouldn't have been merged.

you do bring up a good point and I thought about it, but since this was a backwards compatible change I went ahead and merged it.

@lisenet
Copy link
Contributor Author

lisenet commented Aug 9, 2023

@pavelnikolov our use case is that we need to deploy it with zero replicas so that no deployment is running but the config is there, and we flip the switch when we run a performance test to gather Istio tracing information. It is currently not possible to do so using the current chart version.

@pavelnikolov
Copy link
Contributor

fyi #499

pavelnikolov added a commit that referenced this pull request Aug 22, 2023
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.

[Feature]: make replicas spec a Helm variable rather than hardcoding to replicas: 1
3 participants