Skip to content

Bug 1903464: jsonnet: fix recording rules with many-to-many matching errors #965

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Dec 9, 2020
Merged

Bug 1903464: jsonnet: fix recording rules with many-to-many matching errors #965

merged 4 commits into from
Dec 9, 2020

Conversation

paulfantom
Copy link
Contributor

@paulfantom paulfantom commented Oct 22, 2020 •
edited
Loading

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

Fixing many-to-many matching. Following rules are fixed:

  • cluster:kube_persistentvolumeclaim_resource_requests_storage_bytes:provisioner:sum
  • cluster:kubelet_volume_stats_used_bytes:provisioner:sum

Still working on unit tests. I will rebase it on top of #953 once it is merged.

@openshift-ci-robot 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 Oct 22, 2020
@openshift-ci-robot
Copy link
Contributor

@paulfantom: This pull request references Bugzilla bug 1879520, 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.7.0) matches configured target release for branch (4.7.0)
  • bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

WIP: Bug 1879520: jsonnet: fix cluster:kubelet_volume_stats_used_bytes:provisioner:sum

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-robot openshift-ci-robot 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 Oct 22, 2020
@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 22, 2020
@paulfantom
Copy link
Contributor Author

paulfantom commented Nov 4, 2020 •
edited
Loading

Rebased, added tests, and regenerated. Ready for review.

@paulfantom paulfantom changed the title WIP: Bug 1879520: jsonnet: fix cluster:kubelet_volume_stats_used_bytes:provisioner:sum Bug 1879520: jsonnet: fix cluster:kubelet_volume_stats_used_bytes:provisioner:sum Nov 4, 2020
@openshift-ci-robot 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 Nov 4, 2020
simonpasquier
simonpasquier reviewed Nov 4, 2020
View reviewed changes
Copy link
Contributor

@simonpasquier simonpasquier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The BZ mentions cluster:kubelet_volume_stats_used_bytes:provisioner:sum which isn't the rule fixed here. Is this an omission?

@paulfantom
Copy link
Contributor Author

My mistake. Now this should be good for review.

simonpasquier
simonpasquier reviewed Nov 12, 2020
View reviewed changes
Copy link
Contributor

@simonpasquier simonpasquier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add another test that would have triggered the "many-to-many" errors? IIUC from the BZ, it happens when kubelet_volume_stats_used_bytes is reported twice for the same PVC but different nodes.

@dhellmann dhellmann mentioned this pull request Nov 13, 2020
Merged
2 tasks
@paulfantom
Copy link
Contributor Author

Rebased and added test case. @simonpasquier PTAL

@paulfantom
Copy link
Contributor Author

/retest

@paulfantom
Copy link
Contributor Author

CI flakes

/retest

@paulfantom paulfantom changed the title Bug 1879520: jsonnet: fix cluster:kubelet_volume_stats_used_bytes:provisioner:sum Bug 1903464: jsonnet: fix recording rules with many-to-many matching errors Dec 4, 2020
@openshift-ci-robot
Copy link
Contributor

@paulfantom: This pull request references Bugzilla bug 1903464, 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.7.0) matches configured target release for branch (4.7.0)
  • bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1903464: jsonnet: fix recording rules with many-to-many matching errors

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-robot
Copy link
Contributor

@paulfantom: This pull request references Bugzilla bug 1903464, which is valid.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target release (4.7.0) matches configured target release for branch (4.7.0)
  • bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1903464: jsonnet: fix recording rules with many-to-many matching errors

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.

@simonpasquier
Copy link
Contributor

/lgtm
/retest

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Dec 4, 2020
@paulfantom
Copy link
Contributor Author

/hold

Seems like our CI job - ci/prow/generate is broken. Assets are not generated properly.

@openshift-bot
Copy link
Contributor

/retest

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

12 similar comments
@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@s-urbaniak
Copy link
Contributor

/retest

@openshift-bot
Copy link
Contributor

/retest

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

@RiRa12621
Copy link

/test e2e-agnostic-operator

@openshift-bot
Copy link
Contributor

/retest

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

1 similar comment
@openshift-bot
Copy link
Contributor

/retest

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

@simonpasquier
Copy link
Contributor

/hold
depends on #1010

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 7, 2020
@s-urbaniak
Copy link
Contributor

/hold cancel
/retest

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 9, 2020
@openshift-bot
Copy link
Contributor

/retest

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

@openshift-merge-robot openshift-merge-robot merged commit 236cb5c into openshift:master Dec 9, 2020
@openshift-ci-robot
Copy link
Contributor

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

Bugzilla bug 1903464 has been moved to the MODIFIED state.

In response to this:

Bug 1903464: jsonnet: fix recording rules with many-to-many matching errors

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 paulfantom deleted the bz1879520 branch December 14, 2020 09:27
@paulfantom
Copy link
Contributor Author

/cherry-pick release-4.6

@openshift-cherrypick-robot
Copy link

@paulfantom: #965 failed to apply on top of branch "release-4.6":

Applying: jsonnet,test/rules: fix cluster:kubelet_volume_stats_used_bytes:provisioner:sum and add tests
Applying: jsonnet,test/rules: fix cluster:kube_persistentvolumeclaim_resource_requests_storage_bytes:provisioner:sum and add tests
Applying: assets: regenerate
Applying: *: force assets regeneration in `make generate` target
Using index info to reconstruct a base tree...
M	Makefile
Falling back to patching base and 3-way merge...
Auto-merging Makefile
CONFLICT (content): Merge conflict in Makefile
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0004 *: force assets regeneration in `make generate` target
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherry-pick release-4.6

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-robot openshift-ci-robot mentioned this pull request Dec 15, 2020
Merged
2 tasks
openshift-merge-robot added a commit that referenced this pull request Jan 27, 2021
@openshift-merge-robot
Merge pull request #1022 from paulfantom/cherrypick1903464
bbaf781 
Bug 1907830: Backporting #965
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@simonpasquier simonpasquier simonpasquier left review comments

@lilic lilic lilic left review comments

Assignees

@simonpasquier simonpasquier

@lilic lilic

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. 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. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@paulfantom @openshift-ci-robot @simonpasquier @openshift-bot @s-urbaniak @RiRa12621 @openshift-cherrypick-robot @lilic @openshift-merge-robot