OCPBUGS-60552: [4.17] podresources: list: use active pods #2426

haircommander · 2025-08-26T13:22:16Z

What type of PR is this?

cherry-pick of #2413

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

The podresources API List implementation uses the internal data of the resource managers as source of truth. Looking at the implementation here: https://github.com/kubernetes/kubernetes/blob/v1.34.0-alpha.0/pkg/kubelet/apis/podresources/server_v1.go#L60 we take care of syncing the device allocation data before querying the device manager to return its pod->devices assignment. This is needed because otherwise the device manager (and all the other resource managers) would do the cleanup asynchronously, so the `List` call will return incorrect data. But we don't do this syncing neither for CPUs or for memory, so when we report these we will get stale data as the issue kubernetes#132020 demonstrates. For CPU manager, we however have the reconcile loop which cleans the stale data periodically. Turns out this timing interplay was actually the reason the existing issue kubernetes#119423 seemed fixed (see: kubernetes#119423 (comment)). But it's actually timing. If in the reproducer we set the `cpuManagerReconcilePeriod` to a time very high (>= 5 minutes), then the issue still reproduces against current master branch (https://github.com/kubernetes/kubernetes/blob/v1.34.0-alpha.0/test/e2e_node/podresources_test.go#L983). Taking a step back, we can see multiple problems: 1. not syncing the resource managers internal data before to query for pod assignment (no removeStaleState calls) but most importantly 2. the List call iterate overs all the pod known to the kubelet. But the resource managers do NOT hold resources for non-running pod, so it is better, actually it's correct to iterate only over the active pods. This will also avoid issue 1 above. Furthermore, the resource managers all iterate over the active pods anyway: `List` is using all the pods known about: 1. https://github.com/kubernetes/kubernetes/blob/v1.34.0-alpha.0/pkg/kubelet/kubelet.go#L3135 goes in 2. https://github.com/kubernetes/kubernetes/blob/v1.34.0-alpha.0/pkg/kubelet/pod/pod_manager.go#L215 But all the resource managers are using the list of active pods: 1. https://github.com/kubernetes/kubernetes/blob/v1.34.0-alpha.0/pkg/kubelet/kubelet.go#L1666 goes in 2. https://github.com/kubernetes/kubernetes/blob/v1.34.0-alpha.0/pkg/kubelet/kubelet_pods.go#L198 So this change will also make the `List` view consistent with the resource managers view, which is also a promise of the API currently broken. We also need to acknowledge the the warning in the docstring of GetActivePods. Arguably, having the endpoint using a different podset wrt the resource managers with the related desync causes way more harm than good. And arguably, it's better to fix this issue in just one place instead of having the `List` use a different pod set for unclear reason. For these reasons, while important, I don't think the warning per se invalidated this change. We need to further acknowledge the `List` endpoint used the full pod list since its inception. So, we will add a Feature Gate to disable this fix and restore the old behavior. We plan to keep this Feature Gate for quite a long time (at least 4 more releases) considering how stable this change was. Should a consumer of the API being broken by this change, we have the option to restore the old behavior and to craft a more elaborate fix. The old `v1alpha1` endpoint will be not modified intentionally. ***RELEASE-4.19 BACKPORT NOTE*** dropped the versioned feature gate entry as we don't have the versioned geature gates in this version. Signed-off-by: Francesco Romani <[email protected]>

openshift-ci-robot · 2025-08-26T13:22:25Z

@haircommander: This pull request references Jira Issue OCPBUGS-60552, which is invalid:

expected the bug to target either version "4.17." or "openshift-4.17.", but it targets "4.19.z" instead
expected dependent Jira Issue OCPBUGS-60524 to be in one of the following states: VERIFIED, RELEASE PENDING, CLOSED (ERRATA), CLOSED (CURRENT RELEASE), CLOSED (DONE), CLOSED (DONE-ERRATA), but it is POST instead

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

What type of PR is this?

cherry-pick of #2413

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

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.

openshift-ci-robot · 2025-08-26T13:22:29Z

@haircommander: the contents of this pull request could not be automatically validated.

The following commits are valid:

22692b9|UPSTREAM: 132028: podresources: list: use active pods in list: the upstream PR kubernetes/kubernetes#132028 has merged

The following commits could not be validated and must be approved by a top-level approver:

b8b7df5|UPSTREAM: : remove the backward compat featuregate for 132028: does not specify an upstream backport in the commit message

Comment /validate-backports to re-evaluate validity of the upstream PRs, for example when they are merged upstream.

openshift-ci-robot · 2025-08-26T15:04:46Z

@haircommander: the contents of this pull request could not be automatically validated.

The following commits are valid:

22692b9|UPSTREAM: 132028: podresources: list: use active pods in list: the upstream PR kubernetes/kubernetes#132028 has merged

The following commits could not be validated and must be approved by a top-level approver:

0903239|UPSTREAM: : remove the backward compat featuregate for 132028: does not specify an upstream backport in the commit message

Comment /validate-backports to re-evaluate validity of the upstream PRs, for example when they are merged upstream.

In order to facilitate backports (see OCPBUGS-56785) we prefer to remove the feature gate added as safety measure upstream and disable this escape hatch upstream added. This commit must be dropped once we rebase on top of 1.34. Signed-off-by: Francesco Romani <[email protected]>

openshift-ci-robot · 2025-08-26T15:35:55Z

@haircommander: the contents of this pull request could not be automatically validated.

The following commits are valid:

22692b9|UPSTREAM: 132028: podresources: list: use active pods in list: the upstream PR kubernetes/kubernetes#132028 has merged

The following commits could not be validated and must be approved by a top-level approver:

8e70b62|UPSTREAM: : remove the backward compat featuregate for 132028: does not specify an upstream backport in the commit message

Comment /validate-backports to re-evaluate validity of the upstream PRs, for example when they are merged upstream.

haircommander · 2025-08-26T18:07:45Z

/retest

nee1esh · 2025-08-27T21:28:00Z

/retest-required

haircommander · 2025-08-29T16:42:41Z

/retest-required

haircommander · 2025-09-04T13:37:00Z

/jira refresh

openshift-ci-robot · 2025-09-04T13:37:05Z

@haircommander: This pull request references Jira Issue OCPBUGS-60552, which is invalid:

expected the bug to target either version "4.17." or "openshift-4.17.", but it targets "4.19.z" instead

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

Details

In response to this:

/jira refresh

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.

haircommander · 2025-09-04T13:53:55Z

/jira refresh

openshift-ci-robot · 2025-09-04T13:54:06Z

@haircommander: This pull request references Jira Issue OCPBUGS-60552, which is valid. The bug has been moved to the POST state.

7 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (4.17.z) matches configured target version for branch (4.17.z)
bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, POST)
release note type set to "Release Note Not Required"
dependent bug Jira Issue OCPBUGS-60524 is in the state Verified, which is one of the valid states (VERIFIED, RELEASE PENDING, CLOSED (ERRATA), CLOSED (CURRENT RELEASE), CLOSED (DONE), CLOSED (DONE-ERRATA))
dependent Jira Issue OCPBUGS-60524 targets the "4.18.z" version, which is one of the valid target versions: 4.18.0, 4.18.z
bug has dependents

No GitHub users were found matching the public email listed for the QA contact in Jira ([email protected]), skipping review request.

Details

In response to this:

/jira refresh

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.

rphillips · 2025-09-04T16:47:10Z

/label backport-risk-assessed
/lgtm
/approve

openshift-ci · 2025-09-04T16:57:43Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: haircommander, mrunalp, rphillips

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:

~~pkg/kubelet/DOWNSTREAM_OWNERS~~ [mrunalp]

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

wangke19 · 2025-09-05T03:47:55Z

/verified later

openshift-ci-robot · 2025-09-05T03:47:58Z

@wangke19: /verified later <@username> requires at least one GitHub @username to be specified (it can be a comma delimited list). It indicates the engineer(s) that will be performing the verification. See https://docs.ci.openshift.org/docs/architecture/jira/#premerge-verification for more information.

Details

In response to this:

/verified later

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.

wangke19 · 2025-09-05T03:48:58Z

/verified later @wangke19

openshift-ci-robot · 2025-09-05T03:49:10Z

@wangke19: This PR has been marked to be verified later by @wangke19.

Details

In response to this:

/verified later @wangke19

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.

haircommander · 2025-09-08T13:16:05Z

@tkashem @bertinatto could one of y'all validate commits here please?

tkashem · 2025-09-08T19:21:28Z

/remove-label backports/unvalidated-commits
/label backports/validated-commits

openshift-ci-robot · 2025-09-08T21:10:42Z

/retest-required

Remaining retests: 0 against base HEAD 025ecb6 and 2 for PR HEAD 8e70b62 in total

openshift-ci-robot · 2025-09-08T21:41:08Z

/retest-required

Remaining retests: 0 against base HEAD 025ecb6 and 2 for PR HEAD 8e70b62 in total

openshift-ci · 2025-09-09T01:36:38Z

@haircommander: all tests passed!

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.

openshift-ci-robot · 2025-09-09T01:41:39Z

@haircommander: Jira Issue OCPBUGS-60552: All pull requests linked via external trackers have merged:

openshift/kubernetes#2426

This pull request has the verified-later tag and will need to be manually moved to VERIFIED after testing. Jira Issue OCPBUGS-60552 has been moved to the MODIFIED state.

Details

In response to this:

What type of PR is this?

cherry-pick of #2413

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

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.

openshift-bot · 2025-09-09T02:48:37Z

[ART PR BUILD NOTIFIER]

Distgit: openshift-enterprise-pod
This PR has been included in build openshift-enterprise-pod-container-v4.17.0-202509090235.p0.gc3b3eee.assembly.stream.el9.
All builds following this will include this PR.

openshift-bot · 2025-09-09T03:18:36Z

[ART PR BUILD NOTIFIER]

Distgit: openshift-enterprise-hyperkube
This PR has been included in build openshift-enterprise-hyperkube-container-v4.17.0-202509090235.p0.gc3b3eee.assembly.stream.el9.
All builds following this will include this PR.

openshift-bot · 2025-09-09T03:33:39Z

[ART PR BUILD NOTIFIER]

Distgit: ose-installer-kube-apiserver-artifacts
This PR has been included in build ose-installer-kube-apiserver-artifacts-container-v4.17.0-202509090235.p0.gc3b3eee.assembly.stream.el9.
All builds following this will include this PR.

haircommander · 2025-09-09T13:09:56Z

/cherry-pick release-4.16

openshift-cherrypick-robot · 2025-09-09T13:10:45Z

@haircommander: #2426 failed to apply on top of branch "release-4.16":

Applying: UPSTREAM: 132028: podresources: list: use active pods in list
Using index info to reconstruct a base tree...
M	pkg/features/kube_features.go
M	pkg/kubelet/apis/podresources/server_v1.go
M	pkg/kubelet/apis/podresources/server_v1_test.go
M	pkg/kubelet/kubelet.go
Falling back to patching base and 3-way merge...
Auto-merging pkg/kubelet/kubelet.go
Auto-merging pkg/kubelet/apis/podresources/server_v1_test.go
CONFLICT (content): Merge conflict in pkg/kubelet/apis/podresources/server_v1_test.go
Auto-merging pkg/kubelet/apis/podresources/server_v1.go
CONFLICT (content): Merge conflict in pkg/kubelet/apis/podresources/server_v1.go
Auto-merging pkg/features/kube_features.go
CONFLICT (content): Merge conflict in pkg/features/kube_features.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config advice.mergeConflict false"
Patch failed at 0001 UPSTREAM: 132028: podresources: list: use active pods in list

Details

In response to this:

/cherry-pick release-4.16

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-merge-robot · 2025-09-09T20:02:19Z

Fix included in accepted release 4.17.0-0.nightly-2025-09-09-150403

openshift/kubernetes#2426 has landed in openshift, which means that the behavior described is enabled by default and we want the scheduler to adapt to this behavior by default iff the user didn't explicitly set the informer mode in the CR. Until now, the scheduler works in a dedicated informer mode which is meant to take into account the pods in terminal state (such as pods that ran and completed) in the PFP computation to ensure it matches the PFP computed by the RTE and reported in NRT. The intended behavior which the new kubelet behavior is about ignoring such pods and accounting only for active pods, so if this behavior is enabled in the RTE-NRT, while kept the default dedicated in the scheduler there will be misalignment in the computed vs the expected PFP from scheduler's POV vs NRT's POV and a scheduler stall will happen that will never recover. In this commit we adjust the informer default value to Shared (instead of Dedicated) only if both below conditions are met: 1. the cluster version supports the fixed kubelet which is met if the cluster version is equal or greater to the known-to-be fixed OCP version. 2. the user didn't set the Spec.SchedulerInformer field in the NRS CR This modification will enable the shared mode which in turn includes only running pods (= active pods) in the PFP computation from POV allowing ultimately PFP alignment with NRT. Signed-off-by: Shereen Haj <[email protected]> (cherry picked from commit 6a56840) (cherry picked from commit 4ecc5db) (cherry picked from commit 3e2c92b)

openshift-ci bot requested review from mrunalp and sjenning August 26, 2025 13:23

haircommander force-pushed the podresources-list-active-pods-backport-4.17 branch from b8b7df5 to 0903239 Compare August 26, 2025 15:04

haircommander force-pushed the podresources-list-active-pods-backport-4.17 branch from 0903239 to 8e70b62 Compare August 26, 2025 15:35

openshift-ci-robot added jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. and removed jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. labels Sep 4, 2025

openshift-ci bot added the backport-risk-assessed Indicates a PR to a release branch has been evaluated and considered safe to accept. label Sep 4, 2025

openshift-ci bot assigned duanwei33, gangwgr, kasturinarra, lyman9966 and wangke19 Sep 4, 2025

openshift-ci bot assigned zhouying7780 and rphillips Sep 4, 2025

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 4, 2025

mrunalp approved these changes Sep 4, 2025

View reviewed changes

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 4, 2025

openshift-ci-robot added verified-later verified Signifies that the PR passed pre-merge verification criteria labels Sep 5, 2025

openshift-ci bot added backports/validated-commits Indicates that all commits come to merged upstream PRs. and removed backports/unvalidated-commits Indicates that not all commits come to merged upstream PRs. labels Sep 8, 2025

openshift-merge-bot bot merged commit c3b3eee into openshift:release-4.17 Sep 9, 2025
21 checks passed

haircommander mentioned this pull request Sep 9, 2025

OCPBUGS-60553: [4.16]: podresources: list: use active pods #2454

Merged

OCPBUGS-60552: [4.17] podresources: list: use active pods #2426

OCPBUGS-60552: [4.17] podresources: list: use active pods #2426

Uh oh!

Conversation

haircommander commented Aug 26, 2025

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

Uh oh!

openshift-ci-robot commented Aug 26, 2025

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

Uh oh!

openshift-ci-robot commented Aug 26, 2025

Uh oh!

openshift-ci-robot commented Aug 26, 2025

Uh oh!

openshift-ci-robot commented Aug 26, 2025

Uh oh!

haircommander commented Aug 26, 2025

Uh oh!

nee1esh commented Aug 27, 2025

Uh oh!

haircommander commented Aug 29, 2025

Uh oh!

haircommander commented Sep 4, 2025

Uh oh!

openshift-ci-robot commented Sep 4, 2025

Uh oh!

haircommander commented Sep 4, 2025

Uh oh!

openshift-ci-robot commented Sep 4, 2025

Uh oh!

rphillips commented Sep 4, 2025

Uh oh!

openshift-ci bot commented Sep 4, 2025

Uh oh!

wangke19 commented Sep 5, 2025

Uh oh!

openshift-ci-robot commented Sep 5, 2025

Uh oh!

wangke19 commented Sep 5, 2025

Uh oh!

openshift-ci-robot commented Sep 5, 2025

Uh oh!

haircommander commented Sep 8, 2025

Uh oh!

tkashem commented Sep 8, 2025

Uh oh!

openshift-ci-robot commented Sep 8, 2025

Uh oh!

openshift-ci-robot commented Sep 8, 2025

Uh oh!

openshift-ci bot commented Sep 9, 2025

Uh oh!

Uh oh!

openshift-ci-robot commented Sep 9, 2025

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

Uh oh!

openshift-bot commented Sep 9, 2025

Uh oh!

openshift-bot commented Sep 9, 2025

Uh oh!

openshift-bot commented Sep 9, 2025

Uh oh!

haircommander commented Sep 9, 2025

Uh oh!

openshift-cherrypick-robot commented Sep 9, 2025