-
Notifications
You must be signed in to change notification settings - Fork 368
MON-2207: Expose Authorization settings for remote write in the CMO configuration #1598
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
MON-2207: Expose Authorization settings for remote write in the CMO configuration #1598
Conversation
08ea476
to
3be3391
Compare
/test e2e-agnostic-upgrade |
2 similar comments
/test e2e-agnostic-upgrade |
/test e2e-agnostic-upgrade |
/retest |
95182d9
to
a110124
Compare
/retest |
/retest |
e5d9b08
to
b22afbd
Compare
/retest-required |
/skip |
/retest-required Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
/retest-required Please review the full test history for this PR and help us cut down flakes. |
Ping @sferich888 for px approval. |
/retest-required Please review the full test history for this PR and help us cut down flakes. |
3 similar comments
/retest-required Please review the full test history for this PR and help us cut down flakes. |
/retest-required Please review the full test history for this PR and help us cut down flakes. |
/retest-required Please review the full test history for this PR and help us cut down flakes. |
@lihongyan1 please help to review |
/retest-required |
/retest-required Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
/retest-required Please review the full test history for this PR and help us cut down flakes. |
Ping @Senthamilarasu-STA for px approval. |
/skip |
/label qe-approved |
88b9d1d
to
62ea862
Compare
/label px-approved |
62ea862
to
d4998c8
Compare
Test now fails if the number of function and remoteWrite targets differ Test now reports failed assersions with expected/got error message
d4998c8
to
fcf55a3
Compare
test/e2e/prometheus_test.go
Outdated
@@ -228,7 +228,7 @@ func TestPrometheusRemoteWrite(t *testing.T) { | |||
tlsConfig: | |||
ca: | |||
secret: | |||
name: %[2]s | |||
name:%[2]s |
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.
One last nit :) This change seems unintended and unrelated?
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.
No, that one actually is an improvement. When I was playing around with the tests I noticed that the ConfigMap was not properly formatted when I printed it with -o yaml
with "\t" and "\n" showing and it was because of this space.
/retest |
/retest-required |
1 similar comment
/retest-required |
fcf55a3
to
a6b416a
Compare
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jan--f, JoaoBraveCoding, simonpasquier The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@JoaoBraveCoding: all tests passed! Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
Problem: Although the prometheus-operator already supports RemoteWriteSpec.Authorization, the CMO did not support this configuration.
Solution: Allow CMO to pass the RemoteWriteSpec.Authorization from the ConfigMap to the Prometheus CRD
Issues: https://issues.redhat.com/browse/MON-2207