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.

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

Bug 1741685: collection-scripts: gather monitoring related resources #126

Closed
wants to merge 1 commit into from

Conversation

paulfantom
Copy link

Add script to gather ServiceMonitors and PrometheusRules from multiple openshift-* namespaces managed by multiple operators.

@openshift-ci-robot
Copy link

@paulfantom: This pull request references Bugzilla bug 1741685, which is invalid:

  • expected the bug to target the "4.2.0" release, but it targets "4.3.0" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

Bug 1741685: collection-scripts: gather monitoring related resources

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/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Sep 2, 2019
@paulfantom paulfantom force-pushed the gather_monitoring branch 2 times, most recently from 301ec3b to 078f0fa Compare September 2, 2019 15:05
@openshift-ci-robot openshift-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Sep 2, 2019
@paulfantom
Copy link
Author

/cc @s-urbaniak

@s-urbaniak
Copy link

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 9, 2019
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: paulfantom, s-urbaniak
To complete the pull request process, please assign sferich888
You can assign the PR to them by writing /assign @sferich888 in a comment when ready.

The full list of commands accepted by this bot can be found 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

@paulfantom
Copy link
Author

/bugzilla refresh

@openshift-ci-robot openshift-ci-robot added bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. and removed bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. labels Sep 26, 2019
@openshift-ci-robot
Copy link

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

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.

# we use nested loops to nicely output objects partitioned per namespace, kind
for resource in ${resources[@]}; do
echo "INFO: Gathering ServiceMonitors in ${resource} namespace"
/usr/bin/oc get "${target}.servicemonitors.monitoring.coreos.com" -n "${resource}" -o yaml > "${BASE_COLLECTION_PATH}/${target}/${resource}.yaml"
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we be collecting this with a related resource?

Copy link
Author

Choose a reason for hiding this comment

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

This is almost impossible to gather with related resource right now as related resource would need to be implemented in every operator managing namespace in which ServiceMonitor is deployed.

I agree that related resource would be the best approach. It is just not feasible now.

Copy link
Contributor

Choose a reason for hiding this comment

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

I still think we should push this to related resource across this operators over a script like this.

@deads2k do you agree?

Choose a reason for hiding this comment

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

@sferich888 agreed, we'll implement related resources with respect to service monitors deployed in the openshift-monitoring namespace only. Please note that service monitors deployed by other operators are not going to be covered and must be implemented in their corresponding operator code.

We'll close this one out in favor of a PR against cluster-monitoring-operator setting those related fields for its own assets.

@openshift-ci-robot
Copy link

New changes are detected. LGTM label has been removed.

@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Sep 26, 2019
@paulfantom
Copy link
Author

/close

@openshift-ci-robot
Copy link

@paulfantom: Closed this PR.

In response to this:

/close

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants