Add jsonnet function to patch upstream rules by dgrisonnet · Pull Request #1073 · openshift/cluster-monitoring-operator

dgrisonnet · 2021-03-09T09:49:04Z

Wtih the recent requests to patch upstream alerts to match OCP use cases, we will need a jsonnet function to easily apply the downstream patches.

This PR is based on the changes made in #1044.

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

dgrisonnet · 2021-03-09T09:50:01Z

Holding as we need to wait for #1044.

/hold

dgrisonnet · 2021-03-09T09:54:17Z

This PR is still WIP as we need to wait for the jsonnet refactor before applying these functions in the main script.

@paulfantom can you please do a first pass on this PR? I tested it on top of your refactor and the assets are correctly generated.

paulfantom · 2021-03-09T15:01:39Z

Can we implement a wrapping function like sanitizeRules which would do all necessary operations on rules - patching, excluding, removing fields (like runbook_url annotation)? Without such we will end up composing all functions in main.jsonnet file and it looks a bit ugly.

dgrisonnet · 2021-03-10T18:41:35Z

@paulfantom are you OK with implementing sanitizeRules in a separate PR?

dgrisonnet · 2021-03-10T18:41:41Z

/unhold

dgrisonnet · 2021-03-11T15:19:08Z

@s-urbaniak @paulfantom I applied your comments PTAL

paulfantom · 2021-03-15T08:32:58Z

/lgtm
/retest

paulfantom · 2021-03-16T15:43:18Z

@dgrisonnet this needs a rebase :(

Signed-off-by: Damien Grisonnet <dgrisonn@redhat.com>

paulfantom · 2021-03-16T15:58:42Z

/lgtm

Merge patch and exclude intermediate functions. Use std.startsWith instead of the `==` operator for comparison to make it easier to modify group of rules. Signed-off-by: Damien Grisonnet <dgrisonn@redhat.com>

s-urbaniak · 2021-03-17T10:35:22Z

/lgtm

openshift-ci-robot · 2021-03-17T10:35:39Z

[APPROVALNOTIFIER] This PR is APPROVED

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

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [dgrisonnet,paulfantom,s-urbaniak]

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

paulfantom · 2021-03-17T10:38:21Z

/retest

openshift-bot · 2021-03-17T11:36:54Z

/retest

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

openshift-bot · 2021-03-17T15:03:45Z

/retest

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

openshift-bot · 2021-03-17T16:36:12Z

/retest

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

dgrisonnet · 2021-03-17T21:02:26Z

/retest

dgrisonnet · 2021-03-18T09:09:22Z

/test e2e-agnostic-upgrade

dgrisonnet · 2021-03-18T15:08:21Z

/test e2e-agnostic-upgrade

paulfantom · 2021-03-18T18:25:43Z

/retest

dgrisonnet · 2021-03-19T09:39:39Z

/retest

dgrisonnet · 2021-03-19T13:44:05Z

/retest

openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 9, 2021

openshift-ci-robot requested review from lilic and s-urbaniak March 9, 2021 09:49

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

openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 9, 2021

paulfantom reviewed Mar 9, 2021

View reviewed changes

Comment thread jsonnet/exclude-rules.libsonnet Outdated

dgrisonnet force-pushed the patch-rules branch from 47af660 to 0ecc5a4 Compare March 9, 2021 10:50

dgrisonnet mentioned this pull request Mar 10, 2021

WIP: remove API server SLO alerts and recording rules #1076

Closed

2 tasks

dgrisonnet force-pushed the patch-rules branch from 0ecc5a4 to 8025942 Compare March 10, 2021 18:38

dgrisonnet changed the title ~~WIP: Add jsonnet function to patch upstream rules~~ Add jsonnet function to patch upstream rules Mar 10, 2021

openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 10, 2021

openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 10, 2021

s-urbaniak reviewed Mar 11, 2021

View reviewed changes

Comment thread jsonnet/exclude-rules.libsonnet Outdated

paulfantom reviewed Mar 11, 2021

View reviewed changes

Comment thread jsonnet/patch-rules.libsonnet

dgrisonnet force-pushed the patch-rules branch 2 times, most recently from 5a2a43d to 6be0dc0 Compare March 11, 2021 16:05

openshift-ci-robot assigned paulfantom Mar 15, 2021

openshift-ci-robot added lgtm Indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Mar 15, 2021

dgrisonnet added 3 commits March 16, 2021 16:55

jsonnet: refactor excludeRules function

cc96a81

Signed-off-by: Damien Grisonnet <dgrisonn@redhat.com>

jsonnet: add function to patch rules

07a94f2

Signed-off-by: Damien Grisonnet <dgrisonn@redhat.com>

jsonnet/main: integrate patchRules

3c767ef

Signed-off-by: Damien Grisonnet <dgrisonn@redhat.com>

dgrisonnet force-pushed the patch-rules branch from 6be0dc0 to f9ca2b5 Compare March 16, 2021 15:56

openshift-ci-robot removed lgtm Indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Mar 16, 2021

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 16, 2021

jsonnet: refactor patch and exclude rules

c21c172

Merge patch and exclude intermediate functions. Use std.startsWith instead of the `==` operator for comparison to make it easier to modify group of rules. Signed-off-by: Damien Grisonnet <dgrisonn@redhat.com>

dgrisonnet force-pushed the patch-rules branch from f9ca2b5 to c21c172 Compare March 16, 2021 16:23

openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Mar 16, 2021

openshift-ci-robot assigned s-urbaniak Mar 17, 2021

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 17, 2021

openshift-merge-robot merged commit 02d1245 into openshift:master Mar 19, 2021

Uh oh!

Conversation

dgrisonnet commented Mar 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dgrisonnet commented Mar 9, 2021

Uh oh!

dgrisonnet commented Mar 9, 2021

Uh oh!

Uh oh!

paulfantom commented Mar 9, 2021

Uh oh!

dgrisonnet commented Mar 10, 2021

Uh oh!

dgrisonnet commented Mar 10, 2021

Uh oh!

Uh oh!

dgrisonnet commented Mar 11, 2021

Uh oh!

Uh oh!

paulfantom commented Mar 15, 2021

Uh oh!

paulfantom commented Mar 16, 2021

Uh oh!

paulfantom commented Mar 16, 2021

Uh oh!

s-urbaniak commented Mar 17, 2021

Uh oh!

openshift-ci-robot commented Mar 17, 2021

Uh oh!

paulfantom commented Mar 17, 2021

Uh oh!

openshift-bot commented Mar 17, 2021

Uh oh!

openshift-bot commented Mar 17, 2021

Uh oh!

openshift-bot commented Mar 17, 2021

Uh oh!

dgrisonnet commented Mar 17, 2021

Uh oh!

dgrisonnet commented Mar 18, 2021

Uh oh!

dgrisonnet commented Mar 18, 2021

Uh oh!

paulfantom commented Mar 18, 2021

Uh oh!

dgrisonnet commented Mar 19, 2021

Uh oh!

dgrisonnet commented Mar 19, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

dgrisonnet commented Mar 9, 2021 •

edited

Loading