Skip to content
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

jsonnet: Support exluding namespaces from user-workload monitoring #1312

Merged
merged 2 commits into from
Aug 22, 2021
Merged

jsonnet: Support exluding namespaces from user-workload monitoring #1312

merged 2 commits into from
Aug 22, 2021

Conversation

bison
Copy link
Contributor

@bison bison commented Aug 4, 2021

This adds support for setting an openshift.io/user-monitoring label
on namespaces to exclude them from user-workload monitoring. This is
in addition to the exclusion of namespaces that have set a true
value for the openshift.io/cluster-monitoring label.

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

@bison
jsonnet: Support exluding namespaces from user-workload monitoring
b408a82 
This adds support for setting an `openshift.io/user-monitoring` label
on namespaces to exclude them from user-workload monitoring.  This is
in addition to the exclusion of namespaces that have set a `true`
value for the `openshift.io/cluster-monitoring` label.
@openshift-ci openshift-ci bot requested review from arajkumar and prashbnair August 4, 2021 14:40
@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 4, 2021
@prashbnair
Copy link

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 5, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 5, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bison, prashbnair

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-bot
Copy link
Contributor

/retest-required

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

1 similar comment
@openshift-bot
Copy link
Contributor

/retest-required

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

@bison
Copy link
Contributor Author

bison commented Aug 5, 2021

/assign @juzhao

sferich888
sferich888 reviewed Aug 5, 2021
View reviewed changes
test/e2e/user_workload_monitoring_test.go
}

labels := ns.GetLabels()
labels["openshift.io/user-monitoring"] = "false"
Copy link

Choose a reason for hiding this comment

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

Should we use a more 'explicit' label? openshift.io/user-moitoring-excluded?

Copy link

Choose a reason for hiding this comment

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

I understand that true means that you want to 'monitor' the namespace; and false indicates that you don't want to monitor; however not having the label does the same thing *(I think). Thus making sure this is clearly documented will be EXTREMELY important.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the label was chosen to be consistent with the existing openshift.io/cluster-monitoring label. I don't have much of an opinion on the naming. I agree it needs to be documented. I will prep a docs PR.

@sferich888
Copy link

/label px-approved

@openshift-ci openshift-ci bot added the px-approved Signifies that Product Support has signed off on this PR label Aug 5, 2021
@bison bison mentioned this pull request Aug 6, 2021
Merged
@bison
Copy link
Contributor Author

bison commented Aug 6, 2021

I've added a docs PR for this: openshift/openshift-docs#35289

Hi @rkratky, could you have a look at that, and if everything looks good and we think we can get the docs in for 4.9, then can we get no-feature-freeze approval for this?

@juzhao
Copy link
Contributor

juzhao commented Aug 6, 2021

/label qe-approved

@openshift-ci openshift-ci bot added the qe-approved Signifies that QE has signed off on this PR label Aug 6, 2021
@rkratky
Copy link

rkratky commented Aug 18, 2021

/label docs-approved

@openshift-ci openshift-ci bot added the docs-approved Signifies that Docs has signed off on this PR label Aug 18, 2021
@openshift-bot
Copy link
Contributor

/retest-required

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

4 similar comments
@openshift-bot
Copy link
Contributor

/retest-required

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

@openshift-bot
Copy link
Contributor

/retest-required

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

@openshift-bot
Copy link
Contributor

/retest-required

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

@openshift-bot
Copy link
Contributor

/retest-required

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

@bison
Copy link
Contributor Author

bison commented Aug 20, 2021

/retest

1 similar comment
@bison
Copy link
Contributor Author

bison commented Aug 20, 2021

/retest

@openshift-bot
Copy link
Contributor

/retest-required

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

@openshift-bot
Copy link
Contributor

/retest-required

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

20 similar comments
@openshift-bot
Copy link
Contributor

/retest-required

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

@openshift-bot
Copy link
Contributor

/retest-required

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

@openshift-bot
Copy link
Contributor

/retest-required

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

@openshift-bot
Copy link
Contributor

/retest-required

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

@openshift-bot
Copy link
Contributor

/retest-required

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

@openshift-bot
Copy link
Contributor

/retest-required

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

@openshift-bot
Copy link
Contributor

/retest-required

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

@openshift-bot
Copy link
Contributor

/retest-required

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

@openshift-bot
Copy link
Contributor

/retest-required

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

@openshift-bot
Copy link
Contributor

/retest-required

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

@openshift-bot
Copy link
Contributor

/retest-required

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

@openshift-bot
Copy link
Contributor

/retest-required

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

@openshift-bot
Copy link
Contributor

/retest-required

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

@openshift-bot
Copy link
Contributor

/retest-required

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

@openshift-bot
Copy link
Contributor

/retest-required

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

@openshift-bot
Copy link
Contributor

/retest-required

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

@openshift-bot
Copy link
Contributor

/retest-required

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

@openshift-bot
Copy link
Contributor

/retest-required

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

@openshift-bot
Copy link
Contributor

/retest-required

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

@openshift-bot
Copy link
Contributor

/retest-required

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

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 22, 2021 •
edited
Loading

@bison: 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/versions af5d856 link /test versions

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

/retest-required

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

3 similar comments
@openshift-bot
Copy link
Contributor

/retest-required

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

@openshift-bot
Copy link
Contributor

/retest-required

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

@openshift-bot
Copy link
Contributor

/retest-required

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

@openshift-merge-robot openshift-merge-robot merged commit 87ed294 into openshift:master Aug 22, 2021
@bison bison deleted the uwm-opt-out branch August 23, 2021 09:35
@simonpasquier simonpasquier mentioned this pull request Sep 8, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sferich888 sferich888 sferich888 left review comments

@arajkumar arajkumar Awaiting requested review from arajkumar

@prashbnair prashbnair Awaiting requested review from prashbnair

Assignees

@prashbnair prashbnair

@juzhao juzhao

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. docs-approved Signifies that Docs has signed off on this PR lgtm Indicates that a PR is ready to be merged. px-approved Signifies that Product Support has signed off on this PR qe-approved Signifies that QE has signed off on this PR
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@bison @prashbnair @openshift-bot @sferich888 @juzhao @rkratky @openshift-merge-robot