fix: scrape config files path #377
Conversation
manzsolutions-lpr
changed the title
manzsolutions-lpr
force-pushed
the
fix-scrape-config-path
branch
from
f91150b to
e1d97f9
Compare
manzsolutions-lpr
changed the title
|
@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? |
There was a problem hiding this comment.
@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?
Sure, go ahead.
Could you also please add tests to molecule molecule so that we can verify that it actually works as intended?
manzsolutions-lpr
force-pushed
the
fix-scrape-config-path
branch
from
171bef4 to
c1920cc
Compare
manzsolutions-lpr
force-pushed
the
fix-scrape-config-path
branch
from
c1920cc to
0b110a2
Compare
manzsolutions-lpr
force-pushed
the
fix-scrape-config-path
branch
from
0d1b083 to
c507105
Compare
roles/prometheus/defaults/main.yml
Outdated
| - prometheus/scrapes/*.yml | ||
| - prometheus/scrapes/*.json |
There was a problem hiding this comment.
This should probably be scrape_configs.
There was a problem hiding this comment.
Done. Please note that this implies a breaking change which requires manual clean-up by people who attempted to use this broken functionality.
There was a problem hiding this comment.
Could you do the cleanup or add a warning if manual cleanup is required in the preflight?
There was a problem hiding this comment.
Just deleting the folder as a clean-up is risky since people may have added their own scrape files but since it’s broken it would not have had an effect either way. What would you rather have?
Impact of not cleaning it up (automatically or manually): There is an orphaned folder with unused configuration files probably confusing administrators.
There was a problem hiding this comment.
I opted to remove the folder after all since it was never actually used but automatically created for every installation making it a hassle even for users who never attempted to use the feature to begin with.
manzsolutions-lpr
force-pushed
the
fix-scrape-config-path
branch
from
053626f to
543cd62
Compare
Docs Build 📝Thank you for contribution!✨ This PR has been merged and the docs are now incorporated into |
|
@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
force-pushed
the
fix-scrape-config-path
branch
from
543cd62 to
c5da177
Compare
Signed-off-by: Leonhard Mayr <leonhard.mayr@manz.at>
Signed-off-by: Leonhard Mayr <leonhard.mayr@manz.at>
Signed-off-by: Leonhard Mayr <leonhard.mayr@manz.at>
Signed-off-by: Leonhard Mayr <leonhard.mayr@manz.at>
Signed-off-by: Leonhard Mayr <leonhard.mayr@manz.at>
manzsolutions-lpr
force-pushed
the
fix-scrape-config-path
branch
from
c5da177 to
549ca65
Compare
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! |
|
ping @SuperQ |
|
All set on my end. @SuperQ Any final thoughts before we merge this? |
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/....