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.

Sign up for GitHub

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

Jump to bottom

feat(thanos) ruler component #534

Open
wants to merge 34 commits into
base: main
Choose a base branch
from
Open

feat(thanos) ruler component #534

wants to merge 34 commits into from
+209 −24

Conversation

trouaux
Copy link

@trouaux trouaux commented Nov 26, 2024 •
edited by viennaa
Loading

closes #351

@trouaux trouaux force-pushed the thanos-ruler-component branch 2 times, most recently from 709212f to 5531122 Compare November 27, 2024 12:27
viennaa
viennaa requested changes Nov 27, 2024
View reviewed changes
Copy link
Contributor

@viennaa viennaa left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for tackling your first issue and opening this PR.

We need to change the deployment to a statefulset. Will continue the PR after this change is in.

Please add testing for the respective statefulset to come up.

Please fix the linting test.

thanos/charts/templates/ruler.yaml Outdated Show resolved Hide resolved
thanos/charts/templates/ruler.yaml Outdated Show resolved Hide resolved
thanos/charts/templates/ruler.yaml Outdated Show resolved Hide resolved
@trouaux trouaux force-pushed the thanos-ruler-component branch from 5531122 to 6d0aa6f Compare November 27, 2024 14:51
@trouaux trouaux force-pushed the thanos-ruler-component branch from 72b3af0 to 46903d6 Compare November 29, 2024 10:21
@trouaux trouaux changed the title [WIP] thanos ruler compoment [WIP] feath(thanos) ruler component Jan 8, 2025
@trouaux trouaux changed the title [WIP] feath(thanos) ruler component feat(thanos) ruler component Jan 8, 2025
@trouaux trouaux marked this pull request as ready for review January 8, 2025 14:17
@trouaux trouaux requested a review from a team as a code owner January 8, 2025 14:17
@trouaux trouaux requested a review from a team as a code owner January 8, 2025 14:17
viennaa
viennaa reviewed Jan 15, 2025
View reviewed changes
Copy link
Contributor

@viennaa viennaa left a comment

Choose a reason for hiding this comment

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

Little improvement :)

thanos/charts/values.yaml Outdated Show resolved Hide resolved
@trouaux trouaux mentioned this pull request Jan 21, 2025
Open
ibakshay
ibakshay reviewed Jan 21, 2025
View reviewed changes
Copy link
Contributor

@ibakshay ibakshay left a comment

Choose a reason for hiding this comment

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

LGTM! left a minor comment :)

viennaa
viennaa requested changes Jan 24, 2025
View reviewed changes
Copy link
Contributor

@viennaa viennaa left a comment

Choose a reason for hiding this comment

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

Thanks for pursuing the ruler component config 👍.

I think the cert handling is missing here, so we need to ensure to have config in place, to talk to an alertmanager.

Please also add thanosruler.spec.routePrefix to have it available at the ruler subpath after the query url.

thanos/charts/Chart.yaml Outdated Show resolved Hide resolved
thanos/charts/templates/ruler.yaml Outdated Show resolved Hide resolved
thanos/charts/values.yaml Show resolved Hide resolved
thanos/charts/templates/ruler.yaml Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ibakshay ibakshay ibakshay left review comments

@viennaa viennaa viennaa requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[thanos] Add Ruler component
3 participants
@trouaux @viennaa @ibakshay