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

Update monitoring role for config policy #40

Merged
merged 1 commit into from
Dec 6, 2022
Merged

Update monitoring role for config policy #40

merged 1 commit into from
Dec 6, 2022

Conversation

gparvin
Copy link
Member

@gparvin gparvin commented Dec 6, 2022

As mentioned in a comment in the metrics doc, our config policy metrics are not showing up on managed clusters, but it works on the hub I'm guessing because the roles are obtained another way. I followed this doc to make these updates:
https://docs.openshift.com/container-platform/4.11/operators/operator_sdk/osdk-monitoring-prometheus.html After updating, the prometheus on the managed cluster was able to start collecting the metrics.

Refs:

Signed-off-by: Gus Parvin [email protected]

As mentioned in a comment in the metrics doc, our config policy
metrics are not showing up on managed clusters, but it works on the
hub I'm guessing because the roles are obtained another way.  I
followed this doc to make these updates:
https://docs.openshift.com/container-platform/4.11/operators/operator_sdk/osdk-monitoring-prometheus.html
After updating, the prometheus on the managed cluster was able
to start collecting the metrics.

Refs:
 - https://issues.redhat.com/browse/ACM-2324

Signed-off-by: Gus Parvin <[email protected]>
@openshift-ci openshift-ci bot added the approved label Dec 6, 2022
@gparvin gparvin requested review from JustinKuli and removed request for mprahl December 6, 2022 19:17
@@ -18,6 +18,9 @@ rules:
resources:
- services
- pods
- endpoints
- nodes
Copy link
Member

Choose a reason for hiding this comment

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

Seems odd that nodes are required here. I can kind of understand endpoints and maybe secrets though... But this is what's doc'd so, it seems good to me!

@openshift-ci
Copy link

openshift-ci bot commented Dec 6, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gparvin, JustinKuli

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-cherrypick-robot

@gparvin: cannot checkout release-2.6: error checking out release-2.6: exit status 1. output: error: pathspec 'release-2.6' did not match any file(s) known to git

In response to this:

/cherry-pick release-2.6

I recreated this on ACM 2.6 and will cherry pick since it's not really a code change, just RBAC settings that can provide a better experience for anyone that needs to look at the metrics.

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-merge-robot pushed a commit that referenced this pull request Dec 15, 2022
Reducing the permissions added by pr
#40
We discovered only the endpoints permission was needed.  The secrets
and nodes was not needed.  Since those were documented there is a
chance there could be some scenario where the extra permissions are
needed.  Preferring to keep the rbac more restrictive.

Signed-off-by: Gus Parvin <[email protected]>
magic-mirror-bot bot pushed a commit to stolostron/governance-policy-addon-controller that referenced this pull request Dec 15, 2022
Reducing the permissions added by pr
open-cluster-management-io/governance-policy-addon-controller#40
We discovered only the endpoints permission was needed.  The secrets
and nodes was not needed.  Since those were documented there is a
chance there could be some scenario where the extra permissions are
needed.  Preferring to keep the rbac more restrictive.

Signed-off-by: Gus Parvin <[email protected]>
(cherry picked from commit 1755befaf76eee44a7fe36a4a1f575d953f5d4fc)
magic-mirror-bot bot pushed a commit to stolostron/governance-policy-addon-controller that referenced this pull request Dec 15, 2022
Reducing the permissions added by pr
open-cluster-management-io/governance-policy-addon-controller#40
We discovered only the endpoints permission was needed.  The secrets
and nodes was not needed.  Since those were documented there is a
chance there could be some scenario where the extra permissions are
needed.  Preferring to keep the rbac more restrictive.

Signed-off-by: Gus Parvin <[email protected]>
(cherry picked from commit 1755befaf76eee44a7fe36a4a1f575d953f5d4fc)
dhaiducek pushed a commit to openshift-cherrypick-robot/governance-policy-addon-controller that referenced this pull request Dec 19, 2022
Reducing the permissions added by pr
open-cluster-management-io/governance-policy-addon-controller#40
We discovered only the endpoints permission was needed.  The secrets
and nodes was not needed.  Since those were documented there is a
chance there could be some scenario where the extra permissions are
needed.  Preferring to keep the rbac more restrictive.

Signed-off-by: Gus Parvin <[email protected]>
(cherry picked from commit 1755befaf76eee44a7fe36a4a1f575d953f5d4fc)
openshift-merge-robot pushed a commit to stolostron/governance-policy-addon-controller that referenced this pull request Jan 4, 2023
Reducing the permissions added by pr
open-cluster-management-io/governance-policy-addon-controller#40
We discovered only the endpoints permission was needed.  The secrets
and nodes was not needed.  Since those were documented there is a
chance there could be some scenario where the extra permissions are
needed.  Preferring to keep the rbac more restrictive.

Signed-off-by: Gus Parvin <[email protected]>
(cherry picked from commit 1755befaf76eee44a7fe36a4a1f575d953f5d4fc)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants