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

[loki-distributed] set unique cluster_label for loki-distributed memberlist #3057

Merged
merged 4 commits into from
Nov 15, 2024

Conversation

hobbsh
Copy link
Contributor

@hobbsh hobbsh commented Apr 3, 2024

Addressing grafana/tempo#2766 where Loki/Tempo ingesters can join each other's ring because cluster_label seems to be defaulted to something common to both components. This change should mitigate the issue in the future.

@hobbsh hobbsh changed the title chore: set unique cluster_label for loki-distributed memberlist [loki-distributed] set unique cluster_label for loki-distributed memberlist Apr 3, 2024
@hobbsh hobbsh force-pushed the unique-cluster-label-loki branch from 3694486 to d2bda40 Compare April 3, 2024 15:37
Copy link
Collaborator

@Sheikh-Abubaker Sheikh-Abubaker left a comment

Choose a reason for hiding this comment

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

Hey @hobbsh could you please bump the minor version ?

@hobbsh hobbsh force-pushed the unique-cluster-label-loki branch from 91abb21 to 636b9a5 Compare November 6, 2024 22:12
@hobbsh
Copy link
Contributor Author

hobbsh commented Nov 6, 2024

@Sheikh-Abubaker done, thank you

@Sheikh-Abubaker
Copy link
Collaborator

@hobbsh could you please sign the DCO, thanks!

@hobbsh hobbsh force-pushed the unique-cluster-label-loki branch from 91b484f to 76aa5c3 Compare November 15, 2024 00:30
@hobbsh
Copy link
Contributor Author

hobbsh commented Nov 15, 2024

@Sheikh-Abubaker Done

@@ -24,6 +24,9 @@ helm repo add grafana https://grafana.github.io/helm-charts

Major version upgrades listed here indicate that there is an incompatible breaking change needing manual actions.

### To 0.80.0
Introduces a default `cluster_label` for the ring memberlist, which will temporarily disrupt ingestion as it rolls out, unless you temporarily set `cluster_label_verification_disabled`.
Copy link
Collaborator

@Sheikh-Abubaker Sheikh-Abubaker Nov 15, 2024

Choose a reason for hiding this comment

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

@@ -22,6 +22,9 @@ helm repo add grafana https://grafana.github.io/helm-charts

Major version upgrades listed here indicate that there is an incompatible breaking change needing manual actions.

### To 0.80.0
Introduces a default `cluster_label` for the ring memberlist, which will temporarily disrupt ingestion as it rolls out, unless you temporarily set `cluster_label_verification_disabled`.
Copy link
Collaborator

@Sheikh-Abubaker Sheikh-Abubaker Nov 15, 2024

Choose a reason for hiding this comment

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

Copy link
Collaborator

@Sheikh-Abubaker Sheikh-Abubaker left a comment

Choose a reason for hiding this comment

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

Looks good! left some comments on readme, do check it out!

Signed-off-by: Wylie Hobbs <[email protected]>
@hobbsh
Copy link
Contributor Author

hobbsh commented Nov 15, 2024

@Sheikh-Abubaker Done

Copy link
Collaborator

@Sheikh-Abubaker Sheikh-Abubaker left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for your contribution, keep them coming!

@Sheikh-Abubaker Sheikh-Abubaker merged commit d822475 into grafana:main Nov 15, 2024
6 checks passed
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