Skip to content

Add resource requirements for ingress router pods#2803

Open
joseorpa wants to merge 1 commit intoopenshift:masterfrom
joseorpa:master
Open

Add resource requirements for ingress router pods#2803
joseorpa wants to merge 1 commit intoopenshift:masterfrom
joseorpa:master

Conversation

@joseorpa
Copy link
Copy Markdown

This commit introduces the Resources field in the IngressControllerSpec to allow configuration of resource limits for router pods. It also adds the RouterResourceRequirements struct to define resource requirements for the router, metrics, and logs containers. Additionally, a new feature gate, FeatureGateIngressRouterResourceLimits, is registered to enable this functionality.

Enhancement: openshift/enhancements#1877

Signed-off-by: Jose Ortiz jose.orpa@gmail.com

This commit introduces the `Resources` field in the `IngressControllerSpec` to allow configuration of resource limits for router pods. It also adds the `RouterResourceRequirements` struct to define resource requirements for the router, metrics, and logs containers. Additionally, a new feature gate, `FeatureGateIngressRouterResourceLimits`, is registered to enable this functionality.

Enhancement: openshift/enhancements#1877

Signed-off-by: Jose Ortiz jose.orpa@gmail.com
@openshift-merge-bot
Copy link
Copy Markdown
Contributor

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 openshift-ci bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Apr 14, 2026
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci bot commented Apr 14, 2026

Hello @joseorpa! 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
Copy link
Copy Markdown
Contributor

openshift-ci bot commented Apr 14, 2026

Hi @joseorpa. Thanks for your PR.

I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@openshift-ci openshift-ci bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Apr 14, 2026
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci bot commented Apr 14, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign deads2k for approval. For more information see the Code Review Process.

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

Details Needs approval from an approver in each of these files:

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

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 14, 2026

📝 Walkthrough

Walkthrough

This change introduces a new feature gate IngressRouterResourceLimits to enable resource configuration for Ingress router components. The feature gate was added to the feature matrix and registered with tech preview and dev preview release channels. A new RouterResourceRequirements struct was added to the IngressControllerSpec API type, allowing optional per-container resource requirements specification for router, metrics, and logs containers. The feature gate controls the availability of the new spec.resources field, which when set overrides default operator settings for router pod resource configuration.

🚥 Pre-merge checks | ✅ 10
✅ Passed checks (10 passed)
Check name Status Explanation
Title check ✅ Passed The pull request title clearly and concisely summarizes the main change: adding resource requirements configuration for ingress router pods.
Description check ✅ Passed The pull request description is directly related to the changeset, detailing the new Resources field, RouterResourceRequirements struct, and feature gate addition.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Stable And Deterministic Test Names ✅ Passed This pull request does not introduce any Ginkgo tests. The changes consist solely of API type definitions, feature gate registration, and documentation updates. Since no test code was added, the custom check for stable and deterministic test names is not applicable to this PR.
Test Structure And Quality ✅ Passed Pull request does not include new Ginkgo test code; only API type additions and feature gate registrations without test implementations.
Microshift Test Compatibility ✅ Passed PR adds no Ginkgo e2e tests, only modifies feature gate definitions and API type definitions.
Single Node Openshift (Sno) Test Compatibility ✅ Passed No new Ginkgo e2e tests were added in this PR; changes are limited to API definitions and feature gate registration.
Topology-Aware Scheduling Compatibility ✅ Passed Pull request introduces only optional API type definitions for resource requirements with no topology-aware scheduling logic, constraints, or cluster topology assumptions.
Ote Binary Stdout Contract ✅ Passed PR contains only API type definitions and feature gate declarations with no process-level code that could write to stdout.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed This pull request does not add any Ginkgo e2e tests. Changes consist solely of API type definitions, feature gate registration, and documentation updates. Since the custom check applies only when new Ginkgo e2e tests are added, the check is not applicable and passes.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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.11.4)

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


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@operator/v1/types_ingress.go`:
- Around line 194-195: The comment that says "When this field is set, it takes
precedence over spec.nodePlacement.resources for configuring router pod
resources." references a stale path; update the docstring above the ingress
router resource field in types_ingress.go to point to the correct current field
path (or remove the nonexistent path) instead of spec.nodePlacement.resources —
locate the comment that begins "When this field is set…" which documents the
router resource configuration field and replace the stale reference with the
correct API field path used by the current spec (or reword to a generic
statement like "takes precedence over node placement resources in the spec") so
docs no longer reference a non‑existent field.
🪄 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: Pro Plus

Run ID: a4ab075b-60b0-4bee-8c7e-0239e1d9f031

📥 Commits

Reviewing files that changed from the base of the PR and between 464776f and c972f78.

⛔ Files ignored due to path filters (1)
  • openapi/openapi.json is excluded by !openapi/**
📒 Files selected for processing (3)
  • features.md
  • features/features.go
  • operator/v1/types_ingress.go

Comment on lines +194 to +195
// When this field is set, it takes precedence over spec.nodePlacement.resources
// for configuring router pod resources.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Fix stale API docs reference to spec.nodePlacement.resources.

Line 194 references a field path that does not exist in this API type, which can mislead users reading generated docs.

✏️ Proposed doc fix
-	// When this field is set, it takes precedence over spec.nodePlacement.resources
-	// for configuring router pod resources.
+	// When this field is set, it overrides the operator's default router pod
+	// resource configuration for router pods.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// When this field is set, it takes precedence over spec.nodePlacement.resources
// for configuring router pod resources.
// When this field is set, it overrides the operator's default router pod
// resource configuration for router pods.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@operator/v1/types_ingress.go` around lines 194 - 195, The comment that says
"When this field is set, it takes precedence over spec.nodePlacement.resources
for configuring router pod resources." references a stale path; update the
docstring above the ingress router resource field in types_ingress.go to point
to the correct current field path (or remove the nonexistent path) instead of
spec.nodePlacement.resources — locate the comment that begins "When this field
is set…" which documents the router resource configuration field and replace the
stale reference with the correct API field path used by the current spec (or
reword to a generic statement like "takes precedence over node placement
resources in the spec") so docs no longer reference a non‑existent field.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant