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

[tempo-distributed] Support initContainers and extraContainers for compactor #3391

Merged
merged 4 commits into from
Jan 16, 2025

Conversation

loafoe
Copy link
Contributor

@loafoe loafoe commented Nov 1, 2024

We have a use case where we need to run an init container and extra container as part of the compactor pod.

@CLAassistant
Copy link

CLAassistant commented Nov 1, 2024

CLA assistant check
All committers have signed the CLA.

@loafoe loafoe changed the title Support initContainers and extraContainers for compactor [tempo-distributed] Support initContainers and extraContainers for compactor Nov 1, 2024
@Sheikh-Abubaker
Copy link
Collaborator

Sheikh-Abubaker commented Nov 10, 2024

@zanhsieh this PR is based on the main branch, we should close it then right ?

@zanhsieh
Copy link
Collaborator

@Sheikh-Abubaker
I don't think it's a problem since it's on @loafoe 's fork main branch, not grafana repo main branch.

@loafoe
Copy link
Contributor Author

loafoe commented Nov 13, 2024

@zanhsieh @Sheikh-Abubaker I've rebased the PR to the latest main so it's clean merge again, thanks!

@zanhsieh zanhsieh requested a review from a team as a code owner January 15, 2025 20:54
Signed-off-by: MH <[email protected]>

Signed-off-by: MH <[email protected]>
Signed-off-by: MH <[email protected]>

Signed-off-by: MH <[email protected]>
@mapno mapno merged commit dfeecb9 into grafana:main Jan 16, 2025
6 checks passed
@@ -54,6 +54,8 @@ spec:
hostAliases:
{{- toYaml . | nindent 8 }}
{{- end }}
initContainers:
{{- toYaml .Values.ingester.initContainers | nindent 8 }}
Copy link
Contributor

@marcofranssen marcofranssen Jan 30, 2025

Choose a reason for hiding this comment

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

This is a bug.

It should have been

- {{- toYaml .Values.ingester.initContainers | nindent 8 }}
+ {{- toYaml .Values.compactor.initContainers | nindent 8 }}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will be fixed in this PR: #3552 , thx!

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.

6 participants