-
Notifications
You must be signed in to change notification settings - Fork 11
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
fix: configure prometheus_remote_write config for non-leader #168
Conversation
48c81b2
to
a99d14e
Compare
from ops.testing import Harness | ||
|
||
|
||
class TestUpdateStatus(unittest.TestCase): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder how much of this test should reside in canonical/grafana-cloud-integrator
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the only part that's testable in grafana-cloud-integrator is added already here. The rest of this test is grafana-agent business logic (confirm the settings appear in the place they need to be). Although tbh this unit test is a pretty large unit test, more an integration test that just doesn't need juju...
e244bdf
to
009a3f8
Compare
…orrect As shown in [this issue](canonical/grafana-cloud-integrator#12), we previously did not have test coverage to assert that prometheus_remote_write config was written successfully on non-leader units. The current commit adds a test to assert these configs are available for both leader and non-leader units.
9ca9b77
to
7696acb
Compare
Issue
This PR fixes this issue where grafana-agent is not correctly setting its prometheus_remote_write settings for non-leader units, leading to non-leader units not properly writing their data to prometheus
Solution
The issue is caused by leadership guards in the grafana-cloud-integrator library's
GrafanaCloudConfigRequirer
relation handling. Specifically, it does nothing on relation changed/relation broken events.The solution here is:
Context
Testing Instructions
where we should see both units having the same data:
Upgrade Notes