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

[Chore] Add cluster role binding required for OpenShift monitoring dependent cases. #708

Merged
merged 2 commits into from
Jan 17, 2024

Conversation

IshwarKanse
Copy link
Contributor

The Thanos Querier has switched to using kube-rbac-proxy in OpenShift 4.15 which requires creation of additional role and binding to query for metrics. https://issues.redhat.com/browse/MON-3379 This PR adds the required ClusterRole and ClusterRolebinding so that the in-cluster monitoring dependent cases work on OCP 4.15.

@IshwarKanse IshwarKanse changed the title Add cluster role and binding required for OpenShift monitoring dependent cases. [Chore] Add cluster role and binding required for OpenShift monitoring dependent cases. Dec 12, 2023
@codecov-commenter
Copy link

codecov-commenter commented Dec 12, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (a2023c1) 77.66% compared to head (8d327bd) 77.66%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #708   +/-   ##
=======================================
  Coverage   77.66%   77.66%           
=======================================
  Files          68       68           
  Lines        5157     5157           
=======================================
  Hits         4005     4005           
  Misses        954      954           
  Partials      198      198           
Flag Coverage Δ
unittests 77.66% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

name: kuttl-cluster-monitoring-metrics-api
subjects:
- kind: ServiceAccount
name: prometheus-user-workload
Copy link
Collaborator

Choose a reason for hiding this comment

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

The tempo operator does not own/manage the prometheus-user-workload service account, is it really the responsibility of the tempo operator (tests) to grant this service account additional permissions?

Shouldn't this permission be granted in the CMO operator?

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 checked with the CMO dev, the CMO creates cluster-monitoring-view role which can be bound to any SA to view the metrics for user workload monitoring. This is the docs for the viewing user workload monitoring metrics. https://docs.openshift.com/container-platform/4.14/monitoring/enabling-monitoring-for-user-defined-projects.html#accessing-metrics-from-outside-cluster_enabling-monitoring-for-user-defined-projects With the new CMO change, the additional step is to also bind the cluster-monitoring-view role. This gives the flexibility to the user to use any SA.

The ClusterRoleBinding step is not a Tempo requirement and is used only by the test case to check the metrics using the check_metrics.sh script.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The ClusterRoleBinding step is not a Tempo requirement and is used only by the test case to check the metrics using the check_metrics.sh script.

Ah! Thanks for the explanation. That was the part I didn't realize, that check_metrics.sh is impersonating the prometheus-user-workload service account.
Could you add this as a comment above the ClusterRoleBinding?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added the comment. :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thank you!

@IshwarKanse IshwarKanse changed the title [Chore] Add cluster role and binding required for OpenShift monitoring dependent cases. [Chore] Add cluster role binding required for OpenShift monitoring dependent cases. Jan 15, 2024
@andreasgerstmayr andreasgerstmayr merged commit 4a10be5 into grafana:main Jan 17, 2024
11 checks passed
@IshwarKanse IshwarKanse deleted the monitoring-fix branch April 11, 2024 09:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants