Bump ovn-controller CPU request to 50m to prevent CPU starvation - #3051
Bump ovn-controller CPU request to 50m to prevent CPU starvation#3051shreyasbe wants to merge 1 commit into
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
shreyasbe
changed the title
WalkthroughThe managed and self-hosted OVN Kubernetes node DaemonSet manifests increase the ChangesOVN controller resources
Estimated code review effort: 1 (Trivial) | ~2 minutes Suggested reviewers: Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (3 errors)
✅ Passed checks (21 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
@coderabbitai: why aren't you asking for "changes requested" via GUI? |
|
Tip For best results, initiate chat on the files or code changes.
If you'd like CodeRabbit to behave differently (e.g., more strongly flag certain issues, or you want stricter gating), that would need to be handled through your repository's branch protection rules or required status checks rather than CodeRabbit's review state, since CodeRabbit's core behavior here isn't currently configurable via YAML settings. |
|
@coderabbitai: I want it to be hard gating, how can I achieve that? |
|
Tip For best results, initiate chat on the files or code changes.
|
| resources: | ||
| requests: | ||
| cpu: 10m | ||
| cpu: 100m |
There was a problem hiding this comment.
@coderabbitai why did you miss
cluster-network-operator/.coderabbit.yaml
Lines 109 to 113 in e7f4de6
@shreyasbe you are missing: https://github.com/openshift/cluster-network-operator/blob/master/bindata/network/ovn-kubernetes/managed/ovnkube-node.yaml
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
cat -n .coderabbit.yaml | sed -n '95,125p'
echo "---managed ovnkube-node.yaml ovn-controller resources---"
rg -n "ovn-controller" -A 40 bindata/network/ovn-kubernetes/managed/ovnkube-node.yaml | head -80Length of output: 2084
🏁 Script executed:
#!/bin/bash
fd -i coderabbit.yaml
find / -maxdepth 2 -iname ".coderabbit.yaml" 2>/dev/nullLength of output: 229
🏁 Script executed:
#!/bin/bash
ls -la | grep -i coderabbit
git show HEAD:.coderabbit.yaml 2>&1 | sed -n '95,125p'Length of output: 233
🏁 Script executed:
#!/bin/bash
pwd
ls -la
git log --oneline -3
git show e7f4de6204c26e2f8784fc4c7e2b508b08324fb5:.coderabbit.yaml | sed -n '95,125p'Length of output: 4099
@kyrtapz You're right, and I apologize for the miss — the managed/self-hosted alignment check in .coderabbit.yaml (lines 109-113) should have flagged this. I confirmed the drift directly:
bindata/network/ovn-kubernetes/managed/ovnkube-node.yaml (HyperShift control-plane) still has:
- name: ovn-controller
...
resources:
requests:
cpu: 10m
memory: 300Miwhile bindata/network/ovn-kubernetes/self-hosted/ovnkube-node.yaml was bumped to cpu: 100m in this PR. Since the same CPU-starvation issue (long poll intervals under bulk UDN creation) would equally affect the HyperShift-managed ovn-controller, the managed/ovnkube-node.yaml file should get the identical change for consistency and to avoid drift between the two topologies.
✏️ Learnings added
Learnt from: kyrtapz
Repo: openshift/cluster-network-operator PR: 3051
File: bindata/network/ovn-kubernetes/self-hosted/ovnkube-node.yaml:189-189
Timestamp: 2026-07-14T16:57:34.955Z
Learning: In openshift/cluster-network-operator, when a resource request/limit (e.g. CPU/memory) is changed for a container in `bindata/network/ovn-kubernetes/self-hosted/ovnkube-node.yaml` (e.g. the `ovn-controller` container), the parallel container in `bindata/network/ovn-kubernetes/managed/ovnkube-node.yaml` (HyperShift control-plane) must be checked and updated to match, per the managed/self-hosted alignment rule in `.coderabbit.yaml`. Missing this is the `#1` source of HyperShift drift bugs.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
There was a problem hiding this comment.
Thank you @kyrtapz for the pointer. I have addressed the code changes.
3388762 to
bfa0556
Compare
shreyasbe
changed the title
|
/test 5.0-upgrade-from-stable-4.22-e2e-gcp-ovn-upgrade |
|
/test 5.0-upgrade-from-stable-4.22-e2e-aws-ovn-upgrade |
|
/retest-required |
|
/lgtm |
|
Scheduling required tests: Scheduling tests matching the |
openshift-ci
Bot
added
the
approved
|
/retest-required |
|
/retest |
|
/test e2e-aws-ovn-upgrade-ipsec |
|
/retest |
3 similar comments
|
/retest |
|
/retest |
|
/retest |
|
/test e2e-aws-ovn-hypershift-conformance |
Signed-off-by: Shreyas Be <52690686+shreyasbe@users.noreply.github.com>
bfa0556 to
8e09af9
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 `@bindata/network/ovn-kubernetes/managed/ovnkube-node.yaml`:
- Line 153: Add a regression assertion in the existing template test covering
the rendered ovn-controller CPU request of 50m for both managed and self-hosted
manifests, using the relevant template test symbols. Keep the test focused on
verifying this bindata resource request and document any exception only if one
applies.
🪄 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: 6eb9b2e7-695f-4ea8-99cc-2629773954a9
📒 Files selected for processing (2)
bindata/network/ovn-kubernetes/managed/ovnkube-node.yamlbindata/network/ovn-kubernetes/self-hosted/ovnkube-node.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- bindata/network/ovn-kubernetes/self-hosted/ovnkube-node.yaml
shreyasbe
changed the title
|
/lgtm |
|
Scheduling required tests: Scheduling tests matching the |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kyrtapz, shreyasbe The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/retest-required |
|
/retest |
|
/test e2e-metal-ipi-ovn-ipv6-ipsec |
Hi @kyrtapz, I've analyzed the failing jobs, Which are not related to this CPU request change. Similar failure also seen other PRs as well. I am rerunning the failed job again: #3051 (comment) |
|
/test e2e-metal-ipi-ovn-ipv6-ipsec |
1 similar comment
|
/test e2e-metal-ipi-ovn-ipv6-ipsec |
|
/test e2e-metal-ipi-ovn-ipv6-ipsec |
1 similar comment
|
/test e2e-metal-ipi-ovn-ipv6-ipsec |
|
@shreyasbe: The following tests failed, say
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. |
3 participants
Why this change is needed:
The default 10m CPU request for ovn-controller causes CPU starvation during bulk User Defined Network (UDN) creation, resulting in "Unreasonably long poll interval" warnings (1000ms-4000ms+) and delayed network provisioning.
What this change does:
Increases the ovn-controller CPU request from 10m to 50m in the ovnkube-node DaemonSet to ensure sufficient CPU resources during heavy control-plane reconciliation operations.
Testing: