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

Update Prometheus + Alertmanager: Unify scripts for easier maintenance #1402

Merged

Conversation

andygrunwald
Copy link
Contributor

✍️ Description

Recently, Prometheus Alertmanager was merged, see #1272

Alertmanager is very similar in the installation as Prometheus.
This Pull Request unifies the scripts for Prometheus and Alertmanager, based of the feedback from #1272.

In particular:

  • Using absolute pathes
  • Prefixing ${RELEASE} with v
  • Running update functions in /opt and not in ~
  • Fixing a bug in the Alertmanager Update script (the rm -rf should fail, as we (originally) cded into the unpacked folder, but did not cd ..`)

For Prometheus, there is no functional change.
This is just a unification of both scripts.


🛠️ Type of Change

  • Bug fix (non-breaking change that resolves an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change unexpectedly)
  • New script (a fully functional and thoroughly tested script or set of scripts)

✅ Prerequisites

  • Self-review performed (I have reviewed my code to ensure it follows established patterns and conventions.)
  • Testing performed (I have thoroughly tested my changes and verified expected functionality.)
  • Documentation updated (I have updated any relevant documentation)

📋 Additional Information (optional)

None

@andygrunwald andygrunwald requested a review from a team as a code owner January 10, 2025 13:49
@github-actions github-actions bot added the update script A change that updates a script label Jan 10, 2025
@michelroegl-brunner michelroegl-brunner changed the title Prometheus + Alertmanager: Unify scripts for easier maintenance Update Prometheus + Alertmanager: Unify scripts for easier maintenance Jan 10, 2025
@MickLesk
Copy link
Member

image

??? ^^

@andygrunwald
Copy link
Contributor Author

@MickLesk May i ask why the ????

This PR is not about merging both script to one.
This PR is about to unify both scripts to work in the exact same way.

@MickLesk
Copy link
Member

ok, lgtm, please test both scripts after merge

@MickLesk MickLesk merged commit 9a4d35b into community-scripts:main Jan 10, 2025
4 checks passed
@andygrunwald
Copy link
Contributor Author

andygrunwald commented Jan 11, 2025

Alertmanager

  1. I installed Alertmanager (via bash -c "$(wget -qLO - https://github.com/community-scripts/ProxmoxVE/raw/main/ct/prometheus-alertmanager.sh)")
  2. I downgraded manually to v0.26.0 (and verified it via http://192.168.178.38:9093/#/status)
  3. I hit update in the console and followed the instructions
  4. I verified the new version in http://192.168.178.38:9093/#/status and saw v0.27.0
  5. I verified that there are no leftovers in /opt

Prometheus

The installation is failing with

Screenshot 2025-01-11 at 09 19 06

A fix is proposed here: #1416

@andygrunwald
Copy link
Contributor Author

After #1416 has been merged, here are the testing results for Prometheus

Prometheus

  1. I installed Prometheus (via bash -c "$(wget -qLO - https://github.com/community-scripts/ProxmoxVE/raw/main/ct/prometheus.sh)")
  2. I downgraded manually to v3.0.1 (and verified it via http://192.168.178.163:9090/status)
  3. I hit update in the console and followed the instructions
  4. I verified the new version in http://192.168.178.163:9090/status and saw v3.1.0
  5. I verified that there are no leftovers in /opt

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
update script A change that updates a script
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants