MON-4033: Add OpenShiftMetricsConfig#2726
MON-4033: Add OpenShiftMetricsConfig#2726danielmellado wants to merge 1 commit intoopenshift:masterfrom
Conversation
This commit adds configuration options for the openshift-state-metrics agent in config/v1alpha1 The new struct supports: - nodeSelector: node scheduling constraints - resources: compute resource requests and limits - tolerations: pod tolerations - topologySpreadConstraints: pod distribution across topology domains Signed-off-by: Daniel Mellado <dmellado@fedoraproject.org>
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
@danielmellado: This pull request references MON-4033 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
📝 WalkthroughWalkthroughThis pull request introduces OpenShiftStateMetricsConfig as a new configuration type within the ClusterMonitoring API (v1alpha1). The change adds the OpenShiftStateMetricsConfig field to ClusterMonitoringSpec with support for NodeSelector, Resources, Tolerations, and TopologySpreadConstraints. Corresponding deepcopy and swagger documentation methods were generated to support the new type. The CRD manifest was updated with detailed validation constraints for the new configuration field, including restrictions on resource quantities, array sizes, and nested properties. Comprehensive test cases were added to validate both successful configurations and validation failure scenarios. 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.5.0)Error: build linters: unable to load custom analyzer "kubeapilinter": tools/_output/bin/kube-api-linter.so, plugin: not implemented Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Comment |
|
Hello @danielmellado! Some important instructions when contributing to openshift/api: |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
config/v1alpha1/tests/clustermonitorings.config.openshift.io/ClusterMonitoringConfig.yaml (1)
578-591: Make the duplicate topology spread assertion more specific.Line 591 uses a very broad substring (
"Duplicate value"), which can hide failures from the wrong field/path.🔧 Suggested test assertion tightening
- expectedError: "Duplicate value" + expectedError: 'spec.openShiftStateMetricsConfig.topologySpreadConstraints[1]: Duplicate value'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@config/v1alpha1/tests/clustermonitorings.config.openshift.io/ClusterMonitoringConfig.yaml` around lines 578 - 591, The test "Should reject OpenShiftStateMetricsConfig with duplicate topologySpreadConstraints" is asserting a too-generic error ("Duplicate value"); update the expectedError to assert the precise validation message and path (for example include spec.openShiftStateMetricsConfig.topologySpreadConstraints and the duplicate topologyKey/value) so the test verifies the exact field/path that failed (target the openShiftStateMetricsConfig.topologySpreadConstraints duplicate topologyKey error string rather than the broad "Duplicate value").
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@config/v1alpha1/tests/clustermonitorings.config.openshift.io/ClusterMonitoringConfig.yaml`:
- Around line 578-591: The test "Should reject OpenShiftStateMetricsConfig with
duplicate topologySpreadConstraints" is asserting a too-generic error
("Duplicate value"); update the expectedError to assert the precise validation
message and path (for example include
spec.openShiftStateMetricsConfig.topologySpreadConstraints and the duplicate
topologyKey/value) so the test verifies the exact field/path that failed (target
the openShiftStateMetricsConfig.topologySpreadConstraints duplicate topologyKey
error string rather than the broad "Duplicate value").
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
⛔ Files ignored due to path filters (3)
config/v1alpha1/zz_generated.crd-manifests/0000_10_config-operator_01_clustermonitorings.crd.yamlis excluded by!**/zz_generated.crd-manifests/*config/v1alpha1/zz_generated.featuregated-crd-manifests/clustermonitorings.config.openshift.io/ClusterMonitoringConfig.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**openapi/generated_openapi/zz_generated.openapi.gois excluded by!openapi/**
📒 Files selected for processing (5)
config/v1alpha1/tests/clustermonitorings.config.openshift.io/ClusterMonitoringConfig.yamlconfig/v1alpha1/types_cluster_monitoring.goconfig/v1alpha1/zz_generated.deepcopy.goconfig/v1alpha1/zz_generated.swagger_doc_generated.gopayload-manifests/crds/0000_10_config-operator_01_clustermonitorings.crd.yaml
|
@danielmellado: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions 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. |
This commit adds configuration options for the openshift-state-metrics
agent in config/v1alpha1
The new struct supports:
Signed-off-by: Daniel Mellado dmellado@fedoraproject.org