Skip to content

Conversation

@acherla
Copy link

@acherla acherla commented Jun 2, 2025

Linked to: #3729

@CLAassistant
Copy link

CLAassistant commented Jun 2, 2025

CLA assistant check
All committers have signed the CLA.

@acherla acherla changed the title Add extraContainers parameter to tempo-distributed helm chart [tempo-distributed] Add extraContainers parameter to tempo-distributed helm chart Jun 2, 2025
acherla and others added 6 commits June 2, 2025 19:24
Signed-off-by: Arun Cherla <[email protected]>
Signed-off-by: Arun Cherla <[email protected]>
Signed-off-by: Arun Cherla <[email protected]>
Signed-off-by: Arun Cherla <[email protected]>
Signed-off-by: Arun Cherla <[email protected]>
@acherla
Copy link
Author

acherla commented Jun 6, 2025

@Sheikh-Abubaker any reason why this wasnt merged? Do i need to bump it to 141.2?

@Sheikh-Abubaker
Copy link
Collaborator

@Sheikh-Abubaker any reason why this wasnt merged? Do i need to bump it to 141.2?

Yes in order to fix the failed CI checks

@acherla
Copy link
Author

acherla commented Jun 16, 2025

@Sheikh-Abubaker any reason why this wasnt merged? Do i need to bump it to 141.2?

Yes in order to fix the failed CI checks

This was done upped from 1.142.1 to 2

{{- with .Values.ingester.extraVolumes }}
{{- toYaml . | nindent 8 }}
{{- end }}
{{- with .Values.ingester.extraContainers }}
Copy link
Collaborator

@Sheikh-Abubaker Sheikh-Abubaker Jun 16, 2025

Choose a reason for hiding this comment

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

@acherla, You'd need to address some indentations, The extraContainers should be place within spec.containers.extraContainers. as of now it is placed within spec.extraContainers, imo it should be placed after this line here

Please address the same changes for the remaining templates below as well, ensuring extraContainers is placed within spec.containers.extraContainers instead of spec.extraContainers.

You could also validate if extraContainers are rendered in correct order by running helm template.

Copy link
Author

Choose a reason for hiding this comment

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

Done

{{- with .Values.querier.extraVolumeMounts }}
{{- toYaml . | nindent 12 }}
{{- end }}
{{- with .Values.querier.extraContainers }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

+1

Copy link
Author

Choose a reason for hiding this comment

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

Done

{{- with .Values.queryFrontend.extraVolumeMounts }}
{{- toYaml . | nindent 12 }}
{{- end }}
{{- with .Values.queryFrontend.extraContainers }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

+1

Copy link
Author

Choose a reason for hiding this comment

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

Done

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.

3 participants