thanos-io
reloader: don't fail on envvar expansion errors
Don't fail the reloader on environment variable expansion errors.
Refer: https://github.com/prometheus-operator/prometheus-operator/issues/6136
- [x] I added CHANGELOG entry for this change.
- [ ] Change is not relevant to the end user.
Changes
- Reloader: do not fail on environment variable expansion errors.
Verification
- Modified the tests to cover the changes.
Maybe we can make the behavior configurable? FWIW most of the prometheus config reloader functionality comes from the pkg/reloader but as long as we have an option to create a Reloader instance that doesn't fail on env subst, it is also fine to keep the current behavior as the default.
Test failure seems to indicate that the filesystem changes dropped significantly since the last commit, I don't see any changes in the reloader that may visibly warrant that though. I'll take another look.
Also, the documentation failure seems unrelated (I believe some URLs were moved)?
IIUC the failure on the docs job workflow is a glitch with https://outshift.cisco.com/blog/multi-cluster-monitoring
It seems it wasn't moved, but failed to render due to a recent change.
Couldn't find target container #hubspotForm-7f3330bd-39c5-4358-b56d-44b90ae62e8e for HubSpot Form 7f3330bd-39c5-4358-b56d-44b90ae62e8e. Not rendering form onto the page
My guess is they tried to integrate HubSpot which didn't go as planned, and crashed the DOM.
@simonpasquier Makes sense, I remember doing a similar thing in node-exporter a while back (I pointed it to a local directory for convenience (faster changes since the other PR was also open and I wanted to test and develop simultaneously), but it's better to use the fork's URL in this case, as we've reached a point where this PR looks good for now (and pointing to a local directory will fail the CI).
I'll raise a PR downstream and hold off on merging this before that's been verified.