prometheus-community / ansible

Ansible Collection for Prometheus
https://prometheus-community.github.io/ansible/
Apache License 2.0
399 stars 134 forks source link

fix: scrape config files path #377

Closed manzsolutions-lpr closed 1 month ago

manzsolutions-lpr commented 6 months ago

Unfortunately simply rebasing the original PR I missed that the paths in the actual role are wrong. I have now updated them ~and took the chance to also re-use the variable's value for the actual configuration not just the glob for copying~.

//edit: Cannot reuse variable since it would lead to /etc/prometheus/prometheus/scrapes/....

manzsolutions-lpr commented 6 months ago

@gardar I would also add "notify"-Statements to all the file uploads to trigger a reload of Prometheus. May I add this in this PR?

manzsolutions-lpr commented 5 months ago

@gardar Are these simple tests what you were looking for?

And I’m unsure if my approach for the "default" target is correct.

manzsolutions-lpr commented 5 months ago

I rebased the branch on the latest master but unfortunately now all CI jobs for 2.9 failed. It appears to me that this may be due to reasons unrelated to the branch but I do not see a way to retry the jobs except by maybe pushing a new commit.

Could anyone just re-trigger the CI jobs, please?

manzsolutions-lpr commented 5 months ago

I finally figured out that I couldn’t see CI job logs due to uMatrix not allowing XHR to "core.windows".

Anyway, the issue is entirely unrelated to this branch:

WARNING: Failed to pull docker image "quay.io/ansible/default-test-container:1.10.1".

manzsolutions-lpr commented 5 months ago

Unfortunately the situation hasn't changed with the latest commits: https://github.com/prometheus-community/ansible/actions/runs/9756163117/job/26925952278?pr=377

gardar commented 5 months ago

@manzsolutions-lpr there is a upcoming fix for the failing tests in #395

manzsolutions-lpr commented 2 months ago

@SuperQ Is there anything I can do to make the review easier for you?

gardar commented 2 months ago

Can you please rebase and fix the conflicts with the main branch?

github-actions[bot] commented 1 month ago

Docs Build 📝

Thank you for contribution!✨

This PR has been merged and the docs are now incorporated into main: https://prometheus-community.github.io/ansible/branch/main

manzsolutions-lpr commented 1 month ago

@SuperQ I did rename it, do you have any other suggestions?

Not too familiar with the Github UI, do I have to mark somehow that the requested change was performed?

manzsolutions-lpr commented 1 month ago

Do you mind rebasing this again? I just did some significant refactoring of the roles in #425

While I hopefully solved these merge conflicts one last time I’m not comfortable to sink any more time into this PR. It has dragged on for more than four months now despite my best efforts to comply with requested changes and feedback in a timely manner due to reasons mostly outside of my control.

Thank you for your ongoing work on this project!

gardar commented 1 month ago

Do you mind rebasing this again? I just did some significant refactoring of the roles in #425

While I hopefully solved these merge conflicts one last time I’m not comfortable to sink any more time into this PR. It has dragged on for more than four months now despite my best efforts to comply with requested changes and feedback in a timely manner due to reasons mostly outside of my control.

Thank you for your ongoing work on this project!

LGTM! Thanks for the patience, you caught us at a bad time working through some technical debt 😅

gardar commented 1 month ago

ping @SuperQ

gardar commented 1 month ago

All set on my end. @SuperQ Any final thoughts before we merge this?