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

[kube-prometheus-stack] fixes thanos ruler object store configs #3913

Merged
merged 3 commits into from
Oct 18, 2023
Merged

[kube-prometheus-stack] fixes thanos ruler object store configs #3913

merged 3 commits into from
Oct 18, 2023

Conversation

thameezb
Copy link
Contributor

What this PR does / why we need it

Fixes ThanosRuler objectstore configs

Which issue this PR fixes

fixes #3912

Special notes for your reviewer

Checklist

  • DCO signed
  • Chart Version bumped
  • Title of the PR starts with chart name (e.g. [prometheus-couchdb-exporter])

@thameezb thameezb force-pushed the fix/kube-prometheus-stack-thanos-ruler-objectstore-configs branch from 8fd7d25 to b48ee42 Compare October 18, 2023 08:40
GMartinez-Sisti
GMartinez-Sisti reviewed Oct 18, 2023
View reviewed changes
Copy link
Member

@GMartinez-Sisti GMartinez-Sisti left a comment

Choose a reason for hiding this comment

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

Suggesting to use object-storage-configs instead of objectstore-configs. WDYT?

charts/kube-prometheus-stack/templates/thanos-ruler/ruler.yaml Outdated Show resolved Hide resolved
charts/kube-prometheus-stack/templates/thanos-ruler/secret.yaml Outdated Show resolved Hide resolved
@GMartinez-Sisti
Copy link
Member

Thanks @thameezb !!

@GMartinez-Sisti GMartinez-Sisti merged commit 1a8f84a into prometheus-community:main Oct 18, 2023
4 checks passed
@dragoangel
Copy link
Contributor

Hi @GMartinez-Sisti @thameezb , this PR is totally breaking, same as #3870

This code both worked for people (including me)
I don't know what was wrong here: #3869
But I used both alertmanagersConfig and objectStorageConfig (including queryConfig) as a:

    alertmanagersConfig:
      name: thanos-monitoring-config
      key: alertmanagers.yml

    objectStorageConfig:
      name: thanos-monitoring-config
      key: objstore.yml

    queryConfig:
      name: thanos-monitoring-config
      key: query.yml

to point Thanos Ruler to my own existing secret. These PRs break this behavior. I think initially that was a misunderstanding of what should be placed in values for people who created this issue. I don't want to place sensitive data inside of values and I already have Thanos deployed via bitnami chart which utilize exact same secret in almost same way. Can you please revert back changes done in this PRs or add sort of:
existingAlertmanagersConfig, existingObjectStorageConfig, existingQueryConfig that would have same behaviour that would allows to set both name and key to link in secret. Thank you.

@dragoangel dragoangel mentioned this pull request Oct 18, 2023
Closed
@GMartinez-Sisti
Copy link
Member

Oh no! Thanks for letting us know @dragoangel. Let me see if we can easily fix it forward as you suggested, or we’ll revert it.

@dragoangel
Copy link
Contributor

Oh no! Thanks for letting us know @dragoangel. Let me see if we can easily fix it forward as you suggested, or we’ll revert it.

Thank you for your quick reply

nrdufour added a commit to nrdufour/home-ops that referenced this pull request Oct 20, 2023
@nrdufour
chore(deps): update helm release kube-prometheus-stack to v51.9.4 (#160)
ab75804 
This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [kube-prometheus-stack](https://github.com/prometheus-operator/kube-prometheus) ([source](https://github.com/prometheus-community/helm-charts)) | patch | `51.9.1` -> `51.9.4` |

---

### Release Notes

<details>
<summary>prometheus-community/helm-charts (kube-prometheus-stack)</summary>

### [`v51.9.4`](https://github.com/prometheus-community/helm-charts/releases/tag/kube-prometheus-stack-51.9.4)

[Compare Source](prometheus-community/helm-charts@kube-prometheus-stack-51.9.3...kube-prometheus-stack-51.9.4)

kube-prometheus-stack collects Kubernetes manifests, Grafana dashboards, and Prometheus rules combined with documentation and scripts to provide easy to operate end-to-end Kubernetes cluster monitoring with Prometheus using the Prometheus Operator.

#### What's Changed

-   \[kube-prometheus-stack] Update CoreDNS Dashboard. Add job template variable by [@&#8203;StianOvrevage](https://github.com/StianOvrevage) in prometheus-community/helm-charts#3798

#### New Contributors

-   [@&#8203;StianOvrevage](https://github.com/StianOvrevage) made their first contribution in prometheus-community/helm-charts#3798

**Full Changelog**: prometheus-community/helm-charts@kube-prometheus-stack-51.9.3...kube-prometheus-stack-51.9.4

### [`v51.9.3`](https://github.com/prometheus-community/helm-charts/releases/tag/kube-prometheus-stack-51.9.3)

[Compare Source](prometheus-community/helm-charts@kube-prometheus-stack-51.9.2...kube-prometheus-stack-51.9.3)

kube-prometheus-stack collects Kubernetes manifests, Grafana dashboards, and Prometheus rules combined with documentation and scripts to provide easy to operate end-to-end Kubernetes cluster monitoring with Prometheus using the Prometheus Operator.

#### What's Changed

-   \[kube-prometheus-stack] Fix etcd dashboards by [@&#8203;QuentinBisson](https://github.com/QuentinBisson) in prometheus-community/helm-charts#3917

**Full Changelog**: prometheus-community/helm-charts@kube-prometheus-stack-51.9.2...kube-prometheus-stack-51.9.3

### [`v51.9.2`](https://github.com/prometheus-community/helm-charts/releases/tag/kube-prometheus-stack-51.9.2)

[Compare Source](prometheus-community/helm-charts@kube-prometheus-stack-51.9.1...kube-prometheus-stack-51.9.2)

kube-prometheus-stack collects Kubernetes manifests, Grafana dashboards, and Prometheus rules combined with documentation and scripts to provide easy to operate end-to-end Kubernetes cluster monitoring with Prometheus using the Prometheus Operator.

#### What's Changed

-   \[kube-prometheus-stack] fixes thanos ruler object store configs by [@&#8203;thameezb](https://github.com/thameezb) in prometheus-community/helm-charts#3913

**Full Changelog**: prometheus-community/helm-charts@kube-prometheus-stack-51.9.1...kube-prometheus-stack-51.9.2

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4yNy4wIiwidXBkYXRlZEluVmVyIjoiMzcuMjcuMCIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==-->

Reviewed-on: https://git.home/nrdufour/home-ops/pulls/160
Co-authored-by: Renovate <[email protected]>
Co-committed-by: Renovate <[email protected]>
@renovate renovate bot mentioned this pull request Feb 29, 2024
Closed
1 task
Matiasmct pushed a commit to giffgaff/prometheus-charts-backup that referenced this pull request Mar 20, 2024
@thameezb @GMartinez-Sisti @Matiasmct
[kube-prometheus-stack] fixes thanos ruler object store configs (prom…
e5afedd 
…etheus-community#3913)

* fix: kube-prom-stack thanos-ruler objectstoreConfigs

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

* Update charts/kube-prometheus-stack/templates/thanos-ruler/ruler.yaml

Co-authored-by: Gabriel Martinez <[email protected]>
Signed-off-by: thameezb <[email protected]>

* Update charts/kube-prometheus-stack/templates/thanos-ruler/secret.yaml

Co-authored-by: Gabriel Martinez <[email protected]>
Signed-off-by: thameezb <[email protected]>

---------

Signed-off-by: ThameezBo <[email protected]>
Signed-off-by: thameezb <[email protected]>
Co-authored-by: Gabriel Martinez <[email protected]>
@renovate renovate bot mentioned this pull request Sep 9, 2024
Closed
1 task
@renovate renovate bot mentioned this pull request Sep 16, 2024
Closed
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GMartinez-Sisti GMartinez-Sisti GMartinez-Sisti approved these changes

@andrewgkew andrewgkew Awaiting requested review from andrewgkew andrewgkew is a code owner

@gianrubio gianrubio Awaiting requested review from gianrubio gianrubio is a code owner

@gkarthiks gkarthiks Awaiting requested review from gkarthiks gkarthiks is a code owner

@QuentinBisson QuentinBisson Awaiting requested review from QuentinBisson QuentinBisson is a code owner

@scottrigby scottrigby Awaiting requested review from scottrigby scottrigby is a code owner

@Xtigyro Xtigyro Awaiting requested review from Xtigyro Xtigyro is a code owner

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

Successfully merging this pull request may close these issues.

[kube-prometheus-stack] Fix thanos rule object store config
3 participants
@thameezb @GMartinez-Sisti @dragoangel