Bug 1992567: jsonnet: cleanup jsonnet codebase and align with kube-prometheus #1315

paulfantom · 2021-08-09T10:00:24Z

I added CHANGELOG entry for this change.
No user facing changes, so no entry in CHANGELOG was needed.

A cosmetic change to align with kube-prometheus and allow more visibility into each libsonnet file. I divided files into 2 groups/directories:

components - containing all main deployment objects for each operand (like prometheus or alertmanager)
utils - containing all helper libraries and transformations

This leaves only a bare minimum of files in jsonnet/ directory and thus declutters it.
rules.libsonnet file was left in jsonnet/ as it is neither a component not a library.

Additional changes:

unified add-workload-annotation.libsonnet and add-release-annotation.libsonnet into one file with addAnnotations() function
created sanitize-rules.libsonnet based on patch-rules.libsonnet and remove-runbook-urls.libsonnet with one sanitizeAlertRules() function.
slightly simplified thanos-ruler.libsonnet
fixed TODOs and simplified main.jsonnet a bit

cc @dgrisonnet

paulfantom · 2021-08-09T13:03:04Z

jsonnet/utils/sanitize-rules.libsonnet

+
+  // shorthand for rule patching, rule excluding, and runbook_url removal
+  sanitizeAlertRules(o): {
+    [k]: $.removeRunbookAnnotation($.patchRule($.excludeRule(o[k])))


This is a slight performance optimization as it will iterate only once over all objects instead of doing this 3 times.

dgrisonnet · 2021-08-09T14:19:02Z

jsonnet/utils/sanitize-rules.libsonnet

+
+  // shorthand for rule patching, rule excluding, and runbook_url removal
+  sanitizeAlertRules(o): {
+    [k]: $.removeRunbookAnnotation($.patchRule($.excludeRule(o[k])))


Shouldn't this call removeRunbookUrl instead of removeRunbookAnnotation?

removeRunbookUrl was already taken in https://github.com/openshift/cluster-monitoring-operator/pull/1315/files#diff-9425bc6534201e7b9f2c51476ae71ce5e7c9a353c12c58ec44008c7f12d0171fR131 and I didn't want to cause confusion by using the same name.

I got confused by https://github.com/openshift/cluster-monitoring-operator/pull/1315/files#diff-9425bc6534201e7b9f2c51476ae71ce5e7c9a353c12c58ec44008c7f12d0171fR215-R219, do you know if this is really needed?

Ah, you're right, this is confusing and unused. I removed it in latest commit.

Signed-off-by: paulfantom <pawel@krupa.net.pl>

…bookAnnotations

dgrisonnet · 2021-08-10T13:11:19Z

/lgtm

openshift-ci · 2021-08-10T13:11:30Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dgrisonnet, paulfantom

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:

~~OWNERS~~ [dgrisonnet,paulfantom]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-bot · 2021-08-10T14:37:47Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-08-10T15:01:45Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-ci · 2021-08-10T15:13:57Z

@paulfantom: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Rerun command
ci/prow/e2e-aws-single-node	`fcc1e97`	link	`/test e2e-aws-single-node`

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.

openshift-bot · 2021-08-10T16:34:46Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-08-10T16:46:46Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-08-10T16:58:47Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-08-10T17:10:47Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-08-10T17:22:46Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-08-10T17:34:47Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-08-10T17:46:48Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-08-10T18:10:48Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-08-10T18:22:47Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-08-10T18:34:48Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-08-10T18:46:46Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-08-11T01:52:20Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-08-11T02:05:22Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-08-11T02:18:25Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-08-11T02:31:20Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-08-11T02:44:19Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-08-11T02:57:20Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-08-11T03:10:19Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-08-11T03:23:20Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-08-11T03:36:20Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-08-11T04:54:20Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-08-11T05:07:22Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-08-11T05:20:20Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-08-11T07:25:22Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-08-11T08:13:23Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-08-11T08:25:21Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-08-11T08:37:22Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-08-11T08:49:22Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-08-11T09:01:22Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-08-11T09:14:21Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-ci · 2021-08-11T10:28:10Z

@paulfantom: This pull request references Bugzilla bug 1992567, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.9.0) matches configured target release for branch (4.9.0)
bug is in the state NEW, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Requesting review from QA contact:
/cc @juzhao

In response to this:

Bug 1992567: jsonnet: cleanup jsonnet codebase and align with kube-prometheus

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.

paulfantom · 2021-08-11T10:28:14Z

/bugzilla refresh

openshift-ci · 2021-08-11T10:28:20Z

@paulfantom: This pull request references Bugzilla bug 1992567, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.9.0) matches configured target release for branch (4.9.0)
bug is in the state NEW, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Requesting review from QA contact:
/cc @juzhao

In response to this:

/bugzilla refresh

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.

openshift-bot · 2021-08-11T10:28:57Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-ci · 2021-08-11T10:35:20Z

@paulfantom: All pull requests linked via external trackers have merged:

openshift/cluster-monitoring-operator#1315

Bugzilla bug 1992567 has been moved to the MODIFIED state.

In response to this:

Bug 1992567: jsonnet: cleanup jsonnet codebase and align with kube-prometheus

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.

openshift-ci bot requested review from bison and fpetkovski August 9, 2021 10:00

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 9, 2021

paulfantom force-pushed the jsonnet-restructure branch from a8d1df9 to ea8b82a Compare August 9, 2021 13:00

paulfantom commented Aug 9, 2021

View reviewed changes

dgrisonnet reviewed Aug 9, 2021

View reviewed changes

paulfantom added 5 commits August 10, 2021 14:55

jsonnet: cleanup jsonnet codebase and align with kube-prometheus

0d540b2

Signed-off-by: paulfantom <pawel@krupa.net.pl>

jsonnet: simplify main.jsonnet

4f4cd47

jsonnet: join library functions and simplify their usage

dc98aa3

Signed-off-by: paulfantom <pawel@krupa.net.pl>

jsonnet: improve thanos-ruler jsonnet code

4605c8d

jsonnet: remove unused removeRunbookUrl function and rename removeRun…

fcc1e97

…bookAnnotations

paulfantom force-pushed the jsonnet-restructure branch from ea8b82a to fcc1e97 Compare August 10, 2021 12:57

openshift-ci bot assigned dgrisonnet Aug 10, 2021

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 10, 2021

paulfantom changed the title ~~jsonnet: cleanup jsonnet codebase and align with kube-prometheus~~ Bug 1992567: jsonnet: cleanup jsonnet codebase and align with kube-prometheus Aug 11, 2021

openshift-ci bot added bugzilla/severity-low Referenced Bugzilla bug's severity is low for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. labels Aug 11, 2021

openshift-ci bot requested a review from juzhao August 11, 2021 10:28

openshift-ci bot merged commit e7e6b8c into openshift:master Aug 11, 2021

paulfantom deleted the jsonnet-restructure branch August 11, 2021 10:40

Bug 1992567: jsonnet: cleanup jsonnet codebase and align with kube-prometheus #1315

Bug 1992567: jsonnet: cleanup jsonnet codebase and align with kube-prometheus #1315

Conversation

paulfantom commented Aug 9, 2021 • edited

paulfantom Aug 9, 2021

Choose a reason for hiding this comment

dgrisonnet Aug 9, 2021

Choose a reason for hiding this comment

paulfantom Aug 10, 2021 • edited

Choose a reason for hiding this comment

dgrisonnet Aug 10, 2021

Choose a reason for hiding this comment

paulfantom Aug 10, 2021

Choose a reason for hiding this comment

dgrisonnet Aug 10, 2021

Choose a reason for hiding this comment

dgrisonnet commented Aug 10, 2021

openshift-ci bot commented Aug 10, 2021

openshift-bot commented Aug 10, 2021

openshift-bot commented Aug 10, 2021

openshift-ci bot commented Aug 10, 2021 • edited

openshift-bot commented Aug 10, 2021

openshift-bot commented Aug 10, 2021

openshift-bot commented Aug 10, 2021

openshift-bot commented Aug 10, 2021

openshift-bot commented Aug 10, 2021

openshift-bot commented Aug 10, 2021

openshift-bot commented Aug 10, 2021

openshift-bot commented Aug 10, 2021

openshift-bot commented Aug 10, 2021

openshift-bot commented Aug 10, 2021

openshift-bot commented Aug 10, 2021

openshift-bot commented Aug 11, 2021

openshift-bot commented Aug 11, 2021

openshift-bot commented Aug 11, 2021

openshift-bot commented Aug 11, 2021

openshift-bot commented Aug 11, 2021

openshift-bot commented Aug 11, 2021

openshift-bot commented Aug 11, 2021

openshift-bot commented Aug 11, 2021

openshift-bot commented Aug 11, 2021

openshift-bot commented Aug 11, 2021

openshift-bot commented Aug 11, 2021

openshift-bot commented Aug 11, 2021

openshift-bot commented Aug 11, 2021

openshift-bot commented Aug 11, 2021

openshift-bot commented Aug 11, 2021

openshift-bot commented Aug 11, 2021

openshift-bot commented Aug 11, 2021

openshift-bot commented Aug 11, 2021

openshift-bot commented Aug 11, 2021

openshift-ci bot commented Aug 11, 2021

paulfantom commented Aug 11, 2021

openshift-ci bot commented Aug 11, 2021

openshift-bot commented Aug 11, 2021

openshift-ci bot commented Aug 11, 2021

paulfantom commented Aug 9, 2021 •

edited

paulfantom Aug 10, 2021 •

edited

openshift-ci bot commented Aug 10, 2021 •

edited