MON-4608: add interrupts to NodeExporterCollectorConfig CRD types

[midu16]

Summary

Add NodeExporterCollectorInterruptsConfig to the NodeExporterCollectorConfig CRD type, enabling users to configure the node-exporter interrupts collector via the ClusterMonitoring
CR.

Unlike simpler collectors (softirqs, zoneinfo) that use a plain collectionPolicy on/off switch, the interrupts collector follows the systemd pattern — a union with collectionPolicy
as the discriminator and an optional collect sub-struct containing an include list of regex patterns. This design matches the CMO implementation on the
node-exporter-collector-interrupts branch, which passes the include patterns to
--collector.interrupts.name-include.

Changes

• Added Interrupts field to NodeExporterCollectorConfig
• Added NodeExporterCollectorInterruptsConfig (union: collectionPolicy + collect)
• Added NodeExporterCollectorInterruptsCollectConfig with include list (+listType=set, max 50 items)
• Added NodeExporterInterruptsIncludePattern validated string type (1–1024 chars)
• XValidation rule: collect is forbidden when collectionPolicy is not Collect
• Regenerated CRDs, deepcopy, OpenAPI, swagger docs, and model names

Cross-reference

• CMO branch: node-exporter-collector-interrupts — uses Interrupts.Include []string internally; config_merge.go will translate from this CRD's collectionPolicy/collect.include
  to the internal representation when vendoring.
• Sibling PR pattern: follows the same framework as the zoneinfo branch (node-exporter-collector-zoneinfo), extended with the union pattern from the systemd collector.

Verification

• make update (full regeneration) ✅
• make build ✅
• make -C config/v1alpha1 test — 262 tests passed ✅
• make lint — 0 issues ✅

[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]

Hello @midu16! 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.

[openshift-ci-robot]

@midu16: This pull request references MON-4608 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 sub-task to target the "5.0.0" version, but no target version was set.

Details

In response to this:

Summary

Add NodeExporterCollectorInterruptsConfig to the NodeExporterCollectorConfig CRD type, enabling users to configure the node-exporter interrupts collector via the ClusterMonitoring
CR.

Unlike simpler collectors (softirqs, zoneinfo) that use a plain collectionPolicy on/off switch, the interrupts collector follows the systemd pattern — a union with collectionPolicy
as the discriminator and an optional collect sub-struct containing an include list of regex patterns. This design matches the CMO implementation on the
node-exporter-collector-interrupts branch, which passes the include patterns to
--collector.interrupts.name-include.

Changes

• Added Interrupts field to NodeExporterCollectorConfig
• Added NodeExporterCollectorInterruptsConfig (union: collectionPolicy + collect)
• Added NodeExporterCollectorInterruptsCollectConfig with include list (+listType=set, max 50 items)
• Added NodeExporterInterruptsIncludePattern validated string type (1–1024 chars)
• XValidation rule: collect is forbidden when collectionPolicy is not Collect
• Regenerated CRDs, deepcopy, OpenAPI, swagger docs, and model names

Cross-reference

• CMO branch: node-exporter-collector-interrupts — uses Interrupts.Include []string internally; config_merge.go will translate from this CRD's collectionPolicy/collect.include
  to the internal representation when vendoring.
• Sibling PR pattern: follows the same framework as the zoneinfo branch (node-exporter-collector-zoneinfo), extended with the union pattern from the systemd collector.

Verification

• make update (full regeneration) ✅
• make build ✅
• make -C config/v1alpha1 test — 262 tests passed ✅
• make lint — 0 issues ✅

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.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Adds the disabled-by-default interrupts node-exporter collector to the ClusterMonitoring API and CRD. It supports collection policies and optional regex filtering for /proc/interrupts lines. Validation enforces policy, length, printable-ASCII, uniqueness, and list-size rules. Updates netClass and systemd validation messages and test expectations.

Suggested reviewers: joelspeed, marioferh

🚥 Pre-merge checks | ✅ 15

✅ Passed checks (15 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the addition of interrupts to the NodeExporterCollectorConfig CRD types.
Description check	✅ Passed	The description directly explains the interrupts collector configuration, validation rules, generated artifacts, and verification steps.
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	The diff adds nine static YAML case names. The runner passes each name to DescribeTable without interpolation; no changed Ginkgo title contains dynamic data or unstable identifiers.
Test Structure And Quality	✅ Passed	The PR changes declarative YAML validation fixtures, not Ginkgo It blocks or cluster operations; the stated Ginkgo quality requirements do not apply.
Microshift Test Compatibility	✅ Passed	The PR adds no Ginkgo e2e tests. Its changed test file is declarative YAML validation data, so the MicroShift API compatibility check does not apply.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	The commit adds no Ginkgo e2e tests or topology-dependent logic; its test changes are declarative CRD validation cases in YAML.
Topology-Aware Scheduling Compatibility	✅ Passed	The commit changes only ClusterMonitoring API types, CRD schemas, generated metadata, and validation tests; the complete added-line scan finds no scheduling constraints.
Ote Binary Stdout Contract	✅ Passed	The PR changes only API types, generated schemas, and YAML tests; the diff adds no main/init/TestMain/suite setup or stdout/log writes.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	The diff adds no new Ginkgo e2e tests. The changed YAML contains API validation fixtures only, with no IPv4 literals, URL construction, or external connectivity.
No-Weak-Crypto	✅ Passed	The diff adds only collector API types, validation, generated schemas, deepcopy code, and tests; added-line scans found no MD5, SHA1, DES, RC4, Blowfish, ECB, crypto, or token-comparison usage.
Container-Privileges	✅ Passed	The diff adds Go types, tests, and ClusterMonitoring/CustomResourceDefinition schemas only; no added privileged, host namespace, SYS_ADMIN, root, or allowPrivilegeEscalation settings.
No-Sensitive-Data-In-Logs	✅ Passed	The diff adds API types, validation, generated schema/docs, and tests only; scans found no new logging calls or password, token, API-key, PII, hostname, or customer-data literals.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[qodo-for-rh-openshift]

PR Summary by Qodo

Add interrupts collector config to ClusterMonitoring NodeExporter CRD

✨ Enhancement ⚙️ Configuration changes 🕐 40+ Minutes

AI Description

• Add interrupts collector configuration to the ClusterMonitoring Node Exporter config.
• Model interrupts as a union with optional collect.include regex filters.
• Regenerate CRDs and OpenAPI artifacts to publish the new schema.

Diagram

graph TD
  A["ClusterMonitoring CR"] --> B["NodeExporterCollectorConfig"] --> C["Interrupts union config"] --> D["CMO config merge"] --> E["node-exporter args"] --> F["interrupts collector"]
  F --> G["/proc/interrupts"]
  subgraph Legend
    direction LR
    _cr["CRD/CR"] ~~~ _svc(["Controller/Service"]) ~~~ _fs[["Host file"]]
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Flat on/off only (collectionPolicy without collect/include)

• ➕ Simpler schema and fewer validation rules
• ➕ Lower risk of user misconfiguration around regex patterns
• ➖ Cannot bound metric cardinality when enabled
• ➖ Does not match node-exporter’s interrupts collector filtering capabilities

2. Single string regex (nameInclude) instead of list of patterns

• ➕ Direct mapping to a single node-exporter flag value
• ➕ Simpler merging logic than set semantics
• ➖ Harder for users to manage multiple patterns safely
• ➖ Loses uniqueness/set behavior and per-pattern validation ergonomics

Recommendation: The chosen union design (collectionPolicy discriminator + optional collect.include set) is the best fit: it matches the established 'systemd pattern' in this API, allows optional filtering to control cardinality, and enforces correctness with XValidation (collect forbidden unless Collect). The added list constraints (+listType=set, max 50, per-item length) provide reasonable guardrails while keeping the UX flexible.

Files changed (9) +472 / -1

Enhancement (3) +118 / -0

types_cluster_monitoring.goAdd interrupts collector types and wire into NodeExporterCollectorConfig +69/-0

Add interrupts collector types and wire into NodeExporterCollectorConfig

• Introduces the 'Interrupts' field on 'NodeExporterCollectorConfig'. Adds 'NodeExporterCollectorInterruptsConfig' as a union keyed by 'collectionPolicy', with an optional 'collect.include' set of regex patterns and an XValidation rule forbidding 'collect' unless 'Collect'. Defines a validated string type for include patterns.

config/v1alpha1/types_cluster_monitoring.go

zz_generated.deepcopy.goRegenerate deepcopy implementations for interrupts config types +39/-0

Regenerate deepcopy implementations for interrupts config types

• Adds deepcopy methods for 'NodeExporterCollectorInterruptsConfig' and 'NodeExporterCollectorInterruptsCollectConfig', and updates 'NodeExporterCollectorConfig' deepcopy to copy the new 'Interrupts' field.

config/v1alpha1/zz_generated.deepcopy.go

zz_generated.model_name.goRegister OpenAPI model names for interrupts config types +10/-0

Register OpenAPI model names for interrupts config types

• Adds OpenAPI model name functions for the new interrupts config structs so they are emitted consistently in generated OpenAPI.

config/v1alpha1/zz_generated.model_name.go

Documentation (1) +20 / -0

zz_generated.swagger_doc_generated.goRegenerate swagger docs for interrupts collector fields and types +20/-0

Regenerate swagger docs for interrupts collector fields and types

• Extends SwaggerDoc maps to document the new 'interrupts' field and the new interrupts config types, including the union behavior and include semantics.

config/v1alpha1/zz_generated.swagger_doc_generated.go

Other (5) +334 / -1

0000_10_config-operator_01_clustermonitorings.crd.yamlRegenerate CRD manifest to include interrupts collector schema +66/-0

Regenerate CRD manifest to include interrupts collector schema

• Publishes the new 'interrupts' field in the ClusterMonitoring CRD, including union semantics, 'collect.include' constraints (set, min/max items), and the XValidation rule tying 'collect' to 'collectionPolicy=Collect'.

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

ClusterMonitoringConfig.yamlRegenerate featuregated CRD manifest with interrupts schema +66/-0

Regenerate featuregated CRD manifest with interrupts schema

• Mirrors the CRD schema changes for feature-gated manifests, ensuring the 'interrupts' collector config and validations are present in the featuregated output.

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

zz_generated.openapi.goEmit OpenAPI schemas for interrupts union and include set +86/-1

Emit OpenAPI schemas for interrupts union and include set

• Adds OpenAPI definitions for 'NodeExporterCollectorInterruptsConfig' and 'NodeExporterCollectorInterruptsCollectConfig', wires 'interrupts' into 'NodeExporterCollectorConfig', and includes union metadata ('x-kubernetes-unions') and list-type extensions.

openapi/generated_openapi/zz_generated.openapi.go

openapi.jsonRegenerate JSON OpenAPI output with interrupts schemas +50/-0

Regenerate JSON OpenAPI output with interrupts schemas

• Updates the published OpenAPI JSON to include the new interrupts-related definitions and the 'interrupts' property on the collector config.

openapi/openapi.json

0000_10_config-operator_01_clustermonitorings.crd.yamlUpdate payload CRD manifest with interrupts collector schema +66/-0

Update payload CRD manifest with interrupts collector schema

• Propagates the regenerated CRD schema into the shipped payload manifests so clusters receive the new 'interrupts' configuration surface.

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

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@config/v1alpha1/types_cluster_monitoring.go`:
- Around line 741-750: Allow an empty Include list as documented by removing the
MinItems=1 validation marker from NodeExporterInterruptsIncludePattern in
config/v1alpha1/types_cluster_monitoring.go:741-750. Regenerate
payload-manifests/crds/0000_10_config-operator_01_clustermonitorings.crd.yaml:2215-2232
so the CRD no longer enforces a minimum item count.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

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

Review profile: CHILL

Plan: Enterprise

Run ID: ca85e130-9168-465d-a7ab-9487ed4c4c72

📥 Commits

Reviewing files that changed from the base of the PR and between 18550f1 and 2aa5ff4.

⛔ 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

[qodo-for-rh-openshift]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

1. collect omits omit behavior ✓ Resolved 📜 Skill insight ✧ Quality

Description

The new optional InterruptsConfig.Collect field is documented as optional, but does not explain
what happens when it is omitted (defaults/behavior). This violates the requirement that new optional
fields document omit behavior.

Code

config/v1alpha1/types_cluster_monitoring.go[R721-728]

+	// collect contains configuration options that apply only when the interrupts collector is actively collecting metrics
+	// (i.e. when collectionPolicy is Collect).
+	// collect is optional and may be omitted even when collectionPolicy is Collect.
+	// collect may only be set when collectionPolicy is Collect.
+	// When set, at least one field must be specified within collect.
+	// +unionMember
+	// +optional
+	Collect NodeExporterCollectorInterruptsCollectConfig `json:"collect,omitzero,omitempty"`

Relevance

●● Moderate

Team often documents omit behavior, but a similar “optional/nullable omit behavior” doc request was
previously rejected.

PR-#2851
PR-#2697

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 1507 requires every new optional field to explicitly document what happens when it
is not provided. The Collect field is +optional but its comment only states it may be omitted,
without describing the resulting behavior/defaults.

config/v1alpha1/types_cluster_monitoring.go[721-728]
Skill: api-review

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`Collect` is marked `+optional` but its comment does not describe the behavior when the field is omitted (e.g., what defaults apply / what is collected).

## Issue Context
Optional-field documentation must clearly state omit behavior ("When omitted...", "Defaults to..."), especially for API/CRD fields.

## Fix Focus Areas
- config/v1alpha1/types_cluster_monitoring.go[721-728]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. include comment contradicts MinItems ✓ Resolved 📜 Skill insight ≡ Correctness

Description

The interrupts.collect.include field is documented as allowing an empty list to mean “collect all
interrupts,” but it is also constrained with +kubebuilder:validation:MinItems=1, which rejects
include: [] during CRD validation. This doc/schema mismatch creates misleading API documentation
and can confuse users (including breaking templating/merge workflows that intentionally emit empty
lists).

Code

config/v1alpha1/types_cluster_monitoring.go[R735-751]

+	// include is a list of regular expression patterns that select which interrupt lines to collect.
+	// include is optional.
+	// Each line in /proc/interrupts is matched against the same string node-exporter uses:
+	// the IRQ name, info, and devices fields joined with ";", for example "LOC;77;IO-APIC 2-edge ...".
+	// Patterns are combined with OR into a single expression anchored on both ends,
+	// so each pattern must match the entire string (use ".*" where needed).
+	// An empty list or omitting include collects all interrupt lines.
+	// Each entry must be at least 1 character and at most 1024 characters.
+	// Maximum length for this list is 50.
+	// Minimum length for this list is 1.
+	// Entries in this list must be unique.
+	// +kubebuilder:validation:MaxItems=50
+	// +kubebuilder:validation:MinItems=1
+	// +listType=set
+	// +optional
+	Include []NodeExporterInterruptsIncludePattern `json:"include,omitempty"`
+}

Relevance

● Weak

Very similar schema-vs-doc contradiction cleanup was previously rejected; likely they won’t change
MinItems/comment alignment.

PR-#2645

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 1509 disallows documentation that describes behavior for values that are made
invalid by kubebuilder validation markers. In the Go type, the comment for Include states that an
empty list (or omitting the field) collects all interrupt lines, yet the kubebuilder marker
MinItems=1 requires at least one item; this flows into the generated CRD where the same
description appears alongside minItems: 1, so include: [] is invalid in practice and the
documented behavior cannot be exercised.

config/v1alpha1/types_cluster_monitoring.go[735-751]
config/v1alpha1/types_cluster_monitoring.go[731-751]
config/v1alpha1/zz_generated.crd-manifests/0000_10_config-operator_01_clustermonitorings.crd.yaml[2187-2233]
Skill: api-review

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`NodeExporterCollectorInterruptsCollectConfig.Include` is documented as supporting an empty list (e.g., “An empty list … collects all interrupt lines”), but the field is annotated with `+kubebuilder:validation:MinItems=1`, which makes `include: []` invalid and rejected by CRD validation.

## Issue Context
This is a user-visible API contract mismatch and violates the requirement (PR Compliance ID 1509) that documentation must not describe behavior for values that are invalidated by kubebuilder markers. As implemented and generated, YAML such as `collect: { include: [] }` will fail validation despite the docs implying it is valid; this can also disrupt templating/merge workflows that emit empty lists to explicitly clear patterns.

Align the intended behavior between documentation and schema:
- If an empty list should be allowed to mean “collect all,” remove `MinItems=1` (keeping `+optional` as appropriate) and ensure the generated CRDs reflect the change.
- If an empty list should not be allowed, update the docs to say only omission collects all lines, and remove the “empty list” statement.

## Fix Focus Areas
- config/v1alpha1/types_cluster_monitoring.go[735-751]
- config/v1alpha1/zz_generated.crd-manifests/0000_10_config-operator_01_clustermonitorings.crd.yaml[2187-2233]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context used

✅ Compliance rules (platform): 29 rules

✅ Skills: api-review

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

[everettraven]

Overall this looks pretty good. A few comments.

[coderabbitai]

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

[coderabbitai]

🧹 Nitpick comments (1)

config/v1alpha1/tests/clustermonitorings.config.openshift.io/ClusterMonitoringConfig.yaml (1)

2157-2198: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Add test coverage for the new interrupts collector.

This file updates the netClass and systemd expected error messages, but it adds no test cases for the new interrupts collector. The interrupts collector introduces its own collectionPolicy/collect gate, a minProperties: 1 guard on collect, and an include list with minItems, maxItems, maxLength, minLength, and a pattern regex constraint. None of these constraints have accept or reject test cases here.

Add test cases similar to the existing netClass/systemd tests, for example:

• Accept interrupts with collectionPolicy: Collect and collect.include set.
• Reject interrupts with collect set when collectionPolicy: DoNotCollect.
• Accept interrupts with collectionPolicy: DoNotCollect and no collect.
• Reject interrupts with an empty collect: {} object.
• Reject an include entry that violates the ^[\x20-\x7E]+$ pattern or exceeds maxLength.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@config/v1alpha1/tests/clustermonitorings.config.openshift.io/ClusterMonitoringConfig.yaml`
around lines 2157 - 2198, Add validation test cases for the new interrupts
collector alongside the existing netClass and systemd cases: accept Collect with
a populated collect.include list, reject collect under DoNotCollect, accept
DoNotCollect without collect, reject an empty collect object, and reject include
entries violating the printable-ASCII pattern or maxLength constraint. Use the
existing ClusterMonitoring test structure and expected error conventions.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In
`@config/v1alpha1/tests/clustermonitorings.config.openshift.io/ClusterMonitoringConfig.yaml`:
- Around line 2157-2198: Add validation test cases for the new interrupts
collector alongside the existing netClass and systemd cases: accept Collect with
a populated collect.include list, reject collect under DoNotCollect, accept
DoNotCollect without collect, reject an empty collect object, and reject include
entries violating the printable-ASCII pattern or maxLength constraint. Use the
existing ClusterMonitoring test structure and expected error conventions.

---

ℹ️ Review info

⚙️ Run configuration

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

Review profile: CHILL

Plan: Enterprise

Run ID: 200ee26b-ff88-4769-af0f-cc28c5aa6040

📥 Commits

Reviewing files that changed from the base of the PR and between 7c5fbe3 and 8021c22.

⛔ 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 (3)

• config/v1alpha1/tests/clustermonitorings.config.openshift.io/ClusterMonitoringConfig.yaml
• config/v1alpha1/types_cluster_monitoring.go
• payload-manifests/crds/0000_10_config-operator_01_clustermonitorings.crd.yaml

🚧 Files skipped from review as they are similar to previous changes (1)

• config/v1alpha1/types_cluster_monitoring.go

[coderabbitai]

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

[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

[midu16]

@simonpasquier any feedback from your side ? Thank you!

[midu16]

@everettraven it might require an overwrite of ci/prow/verify-hypershift-integration can you help me with this ?

[simonpasquier]

/hold

we need to agree on openshift/cluster-monitoring-operator#2888 first

[midu16]

/hold

we need to agree on openshift/cluster-monitoring-operator#2888 first

@simonpasquier would appreciate your feedback for PR#2888 .

[coderabbitai]

🧹 Nitpick comments (1)

config/v1alpha1/tests/clustermonitorings.config.openshift.io/ClusterMonitoringConfig.yaml (1)

2354-2432: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add tests for the remaining include validation rules.

These cases do not test the minimum and maximum list size, duplicate patterns, empty patterns, or the 1024-character pattern limit. A regression in these schema constraints will pass this suite.

Add accepted boundary cases and rejected cases for include: [], 51 entries, duplicate entries, an empty string, and an overlength string. The API contract in config/v1alpha1/types_cluster_monitoring.go:799-816 defines these constraints.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@config/v1alpha1/tests/clustermonitorings.config.openshift.io/ClusterMonitoringConfig.yaml`
around lines 2354 - 2432, Extend the interrupts collector validation tests
around the existing include cases to cover the remaining schema constraints from
the include field definition: accepted minimum and maximum list boundaries, and
rejected empty lists, 51 entries, duplicate patterns, an empty pattern, and a
string exceeding 1024 characters. Keep the expected validation errors aligned
with the constraints in the include schema.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In
`@config/v1alpha1/tests/clustermonitorings.config.openshift.io/ClusterMonitoringConfig.yaml`:
- Around line 2354-2432: Extend the interrupts collector validation tests around
the existing include cases to cover the remaining schema constraints from the
include field definition: accepted minimum and maximum list boundaries, and
rejected empty lists, 51 entries, duplicate patterns, an empty pattern, and a
string exceeding 1024 characters. Keep the expected validation errors aligned
with the constraints in the include schema.

---

ℹ️ Review info

⚙️ Run configuration

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

Review profile: CHILL

Plan: Enterprise

Run ID: 03602dfd-e9a8-4a66-9ed5-7bd2edf99de2

📥 Commits

Reviewing files that changed from the base of the PR and between 595800a and 431f292.

⛔ 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 (3)

• config/v1alpha1/tests/clustermonitorings.config.openshift.io/ClusterMonitoringConfig.yaml
• config/v1alpha1/types_cluster_monitoring.go
• payload-manifests/crds/0000_10_config-operator_01_clustermonitorings.crd.yaml

🚧 Files skipped from review as they are similar to previous changes (2)

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

[JoelSpeed]

/override ci/prow/verify-hypershift-integration

Fix pending for this job, but this PR shouldn't affect it anyway

[openshift-ci]

@JoelSpeed: Overrode contexts on behalf of JoelSpeed: ci/prow/verify-hypershift-integration

Details

In response to this:

/override ci/prow/verify-hypershift-integration

Fix pending for this job, but this PR shouldn't affect it anyway

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.

[everettraven]

One minor doc update suggestion. Other than that, this LGTM.

[simonpasquier]

/hold cancel

2 nits otherwise lgtm too

[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

[openshift-ci]

@midu16: The following tests 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/e2e-aws-ovn-hypershift-conformance	42ec1d0	link	true	/test e2e-aws-ovn-hypershift-conformance
ci/prow/e2e-aws-serial-techpreview-2of2	42ec1d0	link	true	/test e2e-aws-serial-techpreview-2of2
ci/prow/e2e-aws-ovn-hypershift	42ec1d0	link	true	/test e2e-aws-ovn-hypershift

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.

[midu16]

/retest-required