OCPBUGS-64847: kubelet: don't use non-admitted pods in calculation#2667
OCPBUGS-64847: kubelet: don't use non-admitted pods in calculation#2667haircommander wants to merge 1 commit into
Conversation
|
@haircommander: the contents of this pull request could not be automatically validated. The following commits could not be validated and must be approved by a top-level approver:
Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: haircommander 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 |
|
/payload-aggregate periodic-ci-openshift-hypershift-release-4.22-periodics-e2e-aws-ovn-conformance 10 |
WalkthroughAdds ChangesPod allocation state filtering
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 11 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (11 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
|
@haircommander: An error was encountered. No known errors were detected, please see the full error message for details. Full error message.
could not create PullRequestPayloadQualificationRun: rpc error: code = DeadlineExceeded desc = context deadline exceeded
Please contact an administrator to resolve this issue. |
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 `@pkg/kubelet/allocation/allocation_manager.go`:
- Around line 763-770: The allocatedPods slice is created with length
len(activePods) causing leading nil entries; change its allocation in
allocation_manager.go to use zero length and capacity len(activePods) (e.g.,
make([]*v1.Pod, 0, len(activePods))) so that appending allocatedPod from
m.UpdatePodFromAllocation(pod) produces a compact slice without nils; ensure the
loop using m.hasPodAllocatedResources and calling UpdatePodFromAllocation
continues to append to allocatedPods so callers like canAdmitPod can safely
access p.UID.
🪄 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: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 06c6f732-792e-49ce-a11f-7e67b57d2902
📒 Files selected for processing (1)
pkg/kubelet/allocation/allocation_manager.go
dad1533 to
921b35e
Compare
|
@haircommander: the contents of this pull request could not be automatically validated. The following commits could not be validated and must be approved by a top-level approver:
Comment |
|
/payload-aggregate periodic-ci-openshift-hypershift-release-4.22-periodics-e2e-aws-ovn-conformance 10 |
|
@haircommander: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/38bcb890-4f0c-11f1-8489-0586b273e428-0 |
instead, check whether the pod is allocated, and return that when we return allocated pods Signed-off-by: Peter Hunt <pehunt@redhat.com>
921b35e to
3cb4618
Compare
|
@haircommander: the contents of this pull request could not be automatically validated. The following commits could not be validated and must be approved by a top-level approver:
Comment |
|
/payload-aggregate periodic-ci-openshift-hypershift-release-4.22-periodics-e2e-aws-ovn-conformance 10 |
|
@haircommander: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/a43c4e10-4fb0-11f1-9de7-4de01fd38918-0 |
|
/retitle OCPBUGS-64847: kubelet: don't use non-admitted pods in calculation |
|
@haircommander: This pull request references Jira Issue OCPBUGS-64847, which is valid. 3 validation(s) were run on this bug
No GitHub users were found matching the public email listed for the QA contact in Jira (bgudi@redhat.com), skipping review request. The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn 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. |
|
/test k8s-e2e-conformance-aws |
|
unit test and some debugging assisted by claude 4.5 |
|
/retest |
|
@haircommander: 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. |
What type of PR is this?
What this PR does / why we need it:
Which issue(s) this PR is related to:
Special notes for your reviewer:
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:
Summary by CodeRabbit
Refactor
Tests