MON-4625: add support for the nvmesubsystem collector

[simonpasquier]

No description provided.

[openshift-merge-bot]

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: LGTM mode

[openshift-ci-robot]

@simonpasquier: This pull request references MON-4625 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 "5.0.0" version, but no target version was set.

Details

In 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.

[openshift-ci]

Hello @simonpasquier! Some important instructions when contributing to openshift/api:
API design plays an important part in the user experience of OpenShift and as such API PRs are subject to a high level of scrutiny to ensure they follow our best practices. If you haven't already done so, please review the OpenShift API Conventions and ensure that your proposed changes are compliant. Following these conventions will help expedite the api review process for your PR.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 4d11411b-2e9f-4271-9f1f-e6eaf8b25146

📥 Commits

Reviewing files that changed from the base of the PR and between 72624b9 and 1602b88.

⛔ Files ignored due to path filters (7)

• config/v1alpha1/zz_generated.crd-manifests/0000_10_config-operator_01_clustermonitorings.crd.yaml is excluded by !**/zz_generated.crd-manifests/*
• config/v1alpha1/zz_generated.deepcopy.go is excluded by !**/zz_generated*
• config/v1alpha1/zz_generated.featuregated-crd-manifests/clustermonitorings.config.openshift.io/ClusterMonitoringConfig.yaml is excluded by !**/zz_generated.featuregated-crd-manifests/**
• config/v1alpha1/zz_generated.model_name.go is excluded by !**/zz_generated*
• config/v1alpha1/zz_generated.swagger_doc_generated.go is excluded by !**/zz_generated*
• openapi/generated_openapi/zz_generated.openapi.go is excluded by !openapi/**, !**/zz_generated*
• openapi/openapi.json is excluded by !openapi/**

📒 Files selected for processing (2)

• config/v1alpha1/types_cluster_monitoring.go
• payload-manifests/crds/0000_10_config-operator_01_clustermonitorings.crd.yaml

---

📝 Walkthrough

Walkthrough

Adds an optional nvmExpressSubsystem node-exporter collector configuration to the ClusterMonitoring API. The configuration includes a required collectionPolicy using Collect or DoNotCollect, with the collector enabled by default. The corresponding ClusterMonitoring CRD schema now defines the new object and validates the allowed policy values.

Possibly related PRs

• openshift/api#2956: Adds a structurally similar node-exporter collector configuration using collectionPolicy.

Suggested reviewers: danielmellado, marioferh

🚥 Pre-merge checks | ✅ 14 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	❓ Inconclusive	No pull request description was provided, so the change intent can't be assessed from it.	Add a short description of the new nvmesubsystem collector config and its collectionPolicy behavior.

✅ Passed checks (14 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title matches the change: it adds support for the nvmesubsystem collector.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names	✅ Passed	No test files changed, and scanned touched files contain no Ginkgo It/Describe/Context/When titles.
Test Structure And Quality	✅ Passed	No Ginkgo test files or test code were changed; the PR only updates config/CRD/openapi generation, so the test-structure check is not applicable.
Microshift Test Compatibility	✅ Passed	PR only adds config/CRD schema for nvmesubsystem; no new *_test.go or Ginkgo test code, so no MicroShift-unsupported test usage to flag.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	PASS: PR only adds config/CRD schema for nvmesubsystem; no new Ginkgo e2e tests or node-topology assumptions were added.
Topology-Aware Scheduling Compatibility	✅ Passed	PR only adds node-exporter collector schema/CRD entries; no deployment, replica, affinity, toleration, or topology-aware scheduling changes.
Ote Binary Stdout Contract	✅ Passed	Only schema/CRD types were added; no main/init/TestMain/suite setup or stdout logging changes were introduced.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	No new Ginkgo/e2e tests were added; changes are limited to config/CRD schema for nvmesubsystem, so IPv4/public-internet compatibility is not applicable.
No-Weak-Crypto	✅ Passed	PR only adds node-exporter config/CRD schema for nvmesubsystem; no weak crypto, custom crypto, or secret/token comparisons were introduced.
Container-Privileges	✅ Passed	No privileged/hostPID/hostNetwork/allowPrivilegeEscalation/SYS_ADMIN fields were added; the PR only adds nvmesubsystem API/CRD schema.
No-Sensitive-Data-In-Logs	✅ Passed	The patch only adds nvmesubsystem config/schema fields; no logging calls or sensitive-data outputs appear in the touched files.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

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.12.2)

Error: build linters: unable to load custom analyzer "kubeapilinter": tools/_output/bin/kube-api-linter.so, plugin: not implemented
The command is terminated due to an error: build linters: unable to load custom analyzer "kubeapilinter": tools/_output/bin/kube-api-linter.so, plugin: not implemented

---

_{Comment @coderabbitai help to get the list of available commands.}

[qodo-for-rh-openshift]

PR Summary by Qodo

Add ClusterMonitoringConfig support for node-exporter nvmesubsystem collector

✨ Enhancement ⚙️ Configuration changes 🕐 20-40 Minutes

AI Description

• Add nvmExpressSubsystem collector configuration to NodeExporterCollectorConfig API.
• Regenerate CRD schemas to expose the new field in ClusterMonitoringConfig.
• Update OpenAPI/Swagger artifacts to include the new collector config type.

Diagram

graph TD
U["Cluster admin"] --> CMC["ClusterMonitoringConfig"] --> OP["Monitoring Operator"] --> NE["node-exporter nvmesubsystem"]
API["API types (Go)"] --> GEN["Code generation"] --> CRD["CRD manifests"]
GEN --> OAPI["OpenAPI/Swagger"]
CMC -. "validated by" .-> CRD

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Generic collector map/extension mechanism

• ➕ Fewer per-collector API additions/regenerations
• ➕ Allows enabling new node-exporter collectors without API changes
• ➖ Weaker schema validation and documentation
• ➖ Harder to enforce stable field names/semantics
• ➖ Less discoverable for users via CRD/OpenAPI

2. Rely solely on platform defaults (no explicit API knob)

• ➕ No API surface growth
• ➕ Avoids shipping additional CRD/OpenAPI changes
• ➖ Users cannot disable/enable the collector explicitly
• ➖ Harder to manage cardinality/performance tradeoffs and compliance needs

Recommendation: The PR’s typed, per-collector configuration is the best fit for a stable, discoverable API: it preserves strong validation and consistent semantics across collectors. A generic extension point would reduce churn but would significantly degrade UX and validation for a core, user-facing CRD.

Files changed (9) +172 / -1

Enhancement (1) +21 / -0

types_cluster_monitoring.goAdd NVMExpressSubsystem collector config type and field +21/-0

Add NVMExpressSubsystem collector config type and field

• Extends NodeExporterCollectorConfig with a new nvmExpressSubsystem field and introduces NodeExporterCollectorNVMExpressSubsystemConfig with a required collectionPolicy to enable/disable the nvmesubsystem collector.

config/v1alpha1/types_cluster_monitoring.go

Documentation (1) +10 / -0

zz_generated.swagger_doc_generated.goAdd Swagger documentation for nvmExpressSubsystem and its type +10/-0

Add Swagger documentation for nvmExpressSubsystem and its type

• Extends NodeExporterCollectorConfig SwaggerDoc map with nvmExpressSubsystem and adds a dedicated SwaggerDoc map/function for NodeExporterCollectorNVMExpressSubsystemConfig.

config/v1alpha1/zz_generated.swagger_doc_generated.go

Other (7) +141 / -1

0000_10_config-operator_01_clustermonitorings.crd.yamlExpose nvmExpressSubsystem in generated ClusterMonitoringConfig CRD schema +22/-0

Expose nvmExpressSubsystem in generated ClusterMonitoringConfig CRD schema

• Adds the nvmExpressSubsystem object to the CRD’s OpenAPI v3 schema, including description, required collectionPolicy, and allowed enum values.

config/v1alpha1/zz_generated.crd-manifests/0000_10_config-operator_01_clustermonitorings.crd.yaml

zz_generated.deepcopy.goRegenerate deepcopy for new NVMExpressSubsystem config +17/-0

Regenerate deepcopy for new NVMExpressSubsystem config

• Updates NodeExporterCollectorConfig deepcopy to copy the new field and adds DeepCopy/DeepCopyInto implementations for NodeExporterCollectorNVMExpressSubsystemConfig.

config/v1alpha1/zz_generated.deepcopy.go

ClusterMonitoringConfig.yamlUpdate featuregated CRD manifest with nvmExpressSubsystem schema +22/-0

Update featuregated CRD manifest with nvmExpressSubsystem schema

• Mirrors the CRD schema addition for nvmExpressSubsystem in the featuregated manifest variant.

config/v1alpha1/zz_generated.featuregated-crd-manifests/clustermonitorings.config.openshift.io/ClusterMonitoringConfig.yaml

zz_generated.model_name.goRegister OpenAPI model name for NVMExpressSubsystem config +5/-0

Register OpenAPI model name for NVMExpressSubsystem config

• Adds OpenAPIModelName for NodeExporterCollectorNVMExpressSubsystemConfig so it can be referenced in generated OpenAPI definitions.

config/v1alpha1/zz_generated.model_name.go

zz_generated.openapi.goAdd OpenAPI schema definition and reference for NVMExpressSubsystem config +31/-1

Add OpenAPI schema definition and reference for NVMExpressSubsystem config

• Registers the new model in GetOpenAPIDefinitions, wires it into the NodeExporterCollectorConfig schema properties, and adds the schema generator for NodeExporterCollectorNVMExpressSubsystemConfig (including enum validation).

openapi/generated_openapi/zz_generated.openapi.go

openapi.jsonRegenerate OpenAPI JSON with nvmExpressSubsystem field and definition +22/-0

Regenerate OpenAPI JSON with nvmExpressSubsystem field and definition

• Adds the nvmExpressSubsystem property to the NodeExporterCollectorConfig definition and introduces the new NodeExporterCollectorNVMExpressSubsystemConfig definition with required collectionPolicy and enum constraints.

openapi/openapi.json

0000_10_config-operator_01_clustermonitorings.crd.yamlShip updated ClusterMonitoringConfig CRD in payload manifests +22/-0

Ship updated ClusterMonitoringConfig CRD in payload manifests

• Propagates the generated CRD schema change into the payload-manifests copy so clusters receive the new nvmExpressSubsystem field in the published CRD.

payload-manifests/crds/0000_10_config-operator_01_clustermonitorings.crd.yaml

[qodo-for-rh-openshift]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

_{To customize comments, go to the Qodo configuration screen, or learn more in the docs.}

[openshift-ci]

@simonpasquier: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/verify-hypershift-integration	1602b88	link	true	/test verify-hypershift-integration

Full PR test history. Your PR dashboard.

Details

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.

[everettraven]

/lgtm

[openshift-merge-bot]

Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test e2e-aws-ovn
/test e2e-aws-ovn-hypershift
/test e2e-aws-ovn-hypershift-conformance
/test e2e-aws-ovn-techpreview
/test e2e-aws-serial-1of2
/test e2e-aws-serial-2of2
/test e2e-aws-serial-techpreview-1of2
/test e2e-aws-serial-techpreview-2of2
/test e2e-azure
/test e2e-gcp
/test e2e-upgrade
/test e2e-upgrade-out-of-change
/test minor-e2e-upgrade-minor

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: everettraven

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [everettraven]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment