-
Notifications
You must be signed in to change notification settings - Fork 49
Add parameter enableadminAPI to prometheus configuration #773
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
Add parameter enableadminAPI to prometheus configuration #773
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: morenod 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 |
Signed-off-by: morenod <[email protected]>
9061cb1 to
77b8964
Compare
|
@morenod: The following test failed, say
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-sigs/prow repository. I understand the commands that are listed here. |
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/5e0a2470368744fa93603c27916716a5 ❌ openstack-k8s-operators-content-provider TIMED_OUT in 30m 55s |
|
Closing after seen that this changes cannot be added, they are automatically added when the field will be available on observability operator and make manifest will include it on the crd |
|
I see what you want, but the files you are modified are not the place to expose new API fields. Those yamls are autogenerated by the operator-sdk. New API entries for exposing certain Prometheus behaviour must be exposed from metricstorage_types.go, and then put where they are required by the metricstorage_controller.go. This means that the metricstorage_controller.go what it does is configure a MonitoringStack object. It uses the MonitoringStack API to get a Prometheus from cluster-observability-operator. However, MonitoringStack does not expose the enableAdminAPI from the subyacent "prometheuses.monitoring.rhobs". To be able to expose that behaviour, we would need to have that field exposed by the cluster-observability-operator first, where the PrometheusConfig struct is defined, along with EnableOtlpHttpReceiver, right here: https://github.com/rhobs/observability-operator/blob/main/pkg/apis/monitoring/v1alpha1/types.go#L232. |
|
Ah I see that you already realized while I was writing this, sorry. |
As part of the openstack
[watcher](https://opendev.org/openstack/watcher)tests we need to delete old metrics to avoid errors calculating node resource utilizations.We are doing that by patching prometheus on post_scripting, after controlplane is created, using:
oc patch prometheuses.monitoring.rhobs metric-storage --namespace=openstack --type=merge -p '{"spec":{"enableAdminAPI":true}}'hereHaving the possibility of adding this parameter to the control-plane kustomize variables at the installation time would avoid to need the post scripting, making edpm jobs easier to maintain.
I have followed the same logic than used to add
enableRemoteWriteReceiverparameter, which is also used by our tests to inject metrics.I'm also creating a similar PR on observability-operator project, not sure which will require the other to be first merged