-
Notifications
You must be signed in to change notification settings - Fork 127
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
Migrate alerting mixin from cluster-monitoring-operator #613
Conversation
…etheusrule run ./generate.sh in the jsonnet directory.
|
||
# Generate jsonnet mixin prometheusrule manifest. | ||
|
||
cd jsonnet && jb update && jsonnet -J vendor main.jsonnet | gojsontoyaml > ../manifests/0000_90_etcd-operator_03_prometheusrule.yaml |
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.
question: could we look into adding this to github.com/openshift/build-machinery-go so the rest of the control-plane can emulate this process if they would like. Then also it will become part of our Makefile workflow? make update
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.
Yes that sound good, if you don't mind I would do that in a follow-up?
Note that this would only be used if we fix things upstream or once a new release is brought in.
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.
should we add a linter test as part of the process? Is jsonnet-lint useful?
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 use jsonnetfmt integration in my vim so it formats everything correctly, but haven't tried jsonnet-lint. Can look into it.
/approve |
/test e2e-agnostic-upgrade |
/hold Until CMO PR is approved, so it's both merged on the same day into the same nightly. |
/lgtm |
namespace: 'openshift-etcd-operator', | ||
annotations: | ||
{ | ||
'include.release.openshift.io/ibm-cloud-managed': 'true', |
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 there is a non-zero chance that developers external to the team may need to add more of these annotations in the future. Maybe inject a comment into the manifest after conversion to yaml (since json doesn't support comments) indicating that the file is generated (and maybe how to update the inputs) and add a verify check that prevents a manual change from being committed (since it would be overwritten by a subsequent generate invocation)?
I'm guessing this is what @hexfusion meant by updating make, though, so it can be done in a follow-up.
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.
As discussed will do in a follow up, and sync with other folks on this.
/hold cancel |
/retest |
/retest Please review the full test history for this PR and help us cut down flakes. |
8 similar comments
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/hold seems like we have etcdHighNumberOfLeaderChanges fire but only for |
/retest Please review the full test history for this PR and help us cut down flakes. |
19 similar comments
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/hold 5 node seems broken |
/retest |
1 similar comment
/retest |
upstream test needs fixed, failure not related to code. /override ci/prow/e2e-gcp-five-control-plane-replicas |
@hexfusion: Overrode contexts on behalf of hexfusion: ci/prow/e2e-gcp-five-control-plane-replicas In response to this:
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. |
/hold cancel |
As upstream etcd uses jsonnet mixin, as discussed I used jsonnet and jb to generate the manifests. We also override openshift specific things. In the next steps I will be adding fixes for alerts but this just brings in what we already today have in OpenShift via cluster-monitoring-operator. https://github.com/openshift/cluster-monitoring-operator/blob/master/jsonnet/control-plane.libsonnet#L8
Original PromRule manifest file is located https://github.com/openshift/cluster-monitoring-operator/blob/7f4925a7203622d70b3007fbddfb6bc5cce6c1d9/assets/control-plane/etcd-prometheus-rule.yaml.
Reqs:
jsonnet
-> https://jsonnet.org/jb
-> https://github.com/jsonnet-bundler/jsonnet-bundler/hold
Mainly opening so we discuss this approach, I can cleanup any generation scripts (its hacky).