Add resource requirements for ingress router pods#2803
Add resource requirements for ingress router pods#2803joseorpa wants to merge 1 commit intoopenshift:masterfrom
Conversation
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
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
Hello @joseorpa! Some important instructions when contributing to openshift/api: |
|
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 Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
|
[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 |
📝 WalkthroughWalkthroughThis change introduces a new feature gate 🚥 Pre-merge checks | ✅ 10✅ Passed checks (10 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.11.4)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. Comment |
There was a problem hiding this comment.
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
⛔ Files ignored due to path filters (1)
openapi/openapi.jsonis excluded by!openapi/**
📒 Files selected for processing (3)
features.mdfeatures/features.gooperator/v1/types_ingress.go
| // When this field is set, it takes precedence over spec.nodePlacement.resources | ||
| // for configuring router pod resources. |
There was a problem hiding this comment.
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.
| // 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.
This commit introduces the
Resourcesfield in theIngressControllerSpecto allow configuration of resource limits for router pods. It also adds theRouterResourceRequirementsstruct 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