Extend the PodResources API to include resources allocated by DRA #115847

moshe010 · 2023-02-17T02:14:19Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

It extend the kubelet PodResources API List() method to return Dynamic Resources and it add Get() to query specific pod

Which issue(s) this PR fixes:

KEP: kubernetes/enhancements#3695

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Extended the kubelet's PodResources API to include resources allocated in `ResourceClaims` via `DynamicResourceAllocation`. Additionally, added a new `Get()` method to query a specific pod for its resources.

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

- [KEP]:  https://github.com/kubernetes/enhancements/pull/3738

k8s-ci-robot · 2023-02-17T02:14:28Z

Hi @moshe010. Thanks for your PR.

I'm waiting for a kubernetes 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.

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/test-infra repository.

moshe010 · 2023-02-17T02:21:43Z

/assign @ffromani @swatisehgal @bart0sh @klueska

I need to add unit tests and e2e tests. I would appreciate reviews to make sure we are aligned

ffromani

We need to distinguish two topics here.

adding checkpoint to DRA. I'm still learning my way into DRA so I can't say. I'd defer comments to DRA developers (which are already tagged) and trust their judgement. IF DRA would benefit from adding checkpoint, I'd do in a separate PR, but really, I trust their call here.
podresources API proper. Pretty much straightforward, the direction seems good with only minor things spotted so far

I'm aware podresources e2e tests need some work and I'm on it in the coming days/weeks, it will be needed to re-graduate the feature to GA on it, which is one of my goals for 1.27/28

pkg/kubelet/cm/dra/manager.go

pkg/kubelet/cm/dra/state/state_checkpoint.go

pkg/kubelet/apis/podresources/server_v1.go

pkg/kubelet/cm/fake_container_manager.go

pkg/kubelet/server/server.go

bart0sh · 2023-02-19T18:53:19Z

/triage accepted
/priority important-soon
/ok-to-test

ffromani

/lgtm

final LGTM call is @klueska 's

needs approval for the feature gate part

ffromani · 2023-03-14T17:49:26Z

pkg/kubelet/apis/podresources/server_v1_test.go

+								Devices:          []*podresourcesapi.ContainerDevices{},
+								CpuIds:           cpus,
+								Memory:           memory,
+								DynamicResources: draDevs,


can be postponed.

pkg/kubelet/apis/podresources/server_v1_test.go

ffromani · 2023-03-14T17:50:21Z

pkg/kubelet/apis/podresources/server_v1_test.go

@@ -515,11 +753,54 @@ func equalListResponse(respA, respB *podresourcesapi.ListPodResourcesResponse) b
 			if !equalContainerDevices(cntA.Devices, cntB.Devices) {
 				return false
 			}
+
+			if !euqalDynamicResources(cntA.DynamicResources, cntB.DynamicResources) {


let's fix the typo (cosmetic only, because the typo is used consistently!) before beta. Not blocking now.

k8s-ci-robot · 2023-03-14T17:51:14Z

LGTM label has been added.

Git tree hash: 89ea6386ced8e5f5c4a101d7f00b21d338565bcf

ffromani · 2023-03-14T17:53:09Z

/hold

because:

/lgtm

final LGTM call is @klueska 's

ffromani · 2023-03-14T17:53:21Z

/retest

ffromani · 2023-03-14T18:23:33Z

/test pull-kubernetes-unit

unrelated failure

klueska · 2023-03-14T18:47:09Z

Thanks for all your hard work on this @moshe010
/lgtm
/approve

k8s-ci-robot · 2023-03-14T18:58:02Z

@klueska: /release-note-edit must be used with a release note block.

In response to this:

/release-note-edit

release-note
We extend the kubelet's PodResources API to include resources allocated in `ResourceClaims` via `DynamicResourceAllocation`. Additionally, we add a new `Get()` method to query a specific pod for its resources rather than forcing users to rely solely on the existing `List()` method which returns resources for all pods.

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/test-infra repository.

k8s-ci-robot · 2023-03-14T19:00:13Z

@klueska: /release-note-edit must be used with a release note block.

In response to this:

/release-note-edit

We extend the kubelet's PodResources API to include resources allocated in `ResourceClaims` via `DynamicResourceAllocation`. Additionally, we add a new `Get()` method to query a specific pod for its resources rather than forci

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/test-infra repository.

k8s-ci-robot · 2023-03-14T19:01:51Z

@klueska: /release-note-edit must be used with a release note block.

In response to this:

/release-note-edit

We extend the kubelet's PodResources API to include resources allocated in `ResourceClaims` via `DynamicResourceAllocation`. Additionally, we add a new `Get()` method to query a specific pod for its resources rather than forci

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/test-infra repository.

klueska · 2023-03-14T19:21:06Z

/assign dchen1107 derekwaynecarr
For feature gate approval

ffromani · 2023-03-14T19:52:14Z

/hold cancel

Kevin reviewed

dims · 2023-03-14T19:58:16Z

/approve

new alpha feature gates look good to me!

dchen1107 · 2023-03-14T20:09:28Z

/approve the feature gate

k8s-ci-robot · 2023-03-14T20:10:05Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dchen1107, dims, klueska, moshe010

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~pkg/features/OWNERS~~ [dchen1107,dims]
~~pkg/kubelet/OWNERS~~ [dchen1107,dims,klueska]
~~staging/src/k8s.io/kubelet/OWNERS~~ [dchen1107,dims,klueska]

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

moshe010 · 2023-03-19T11:34:49Z

follow-up PR #116742 to address the no-blocking issues

k8s-ci-robot added needs-priority Indicates a PR lacks a `priority/foo` label and requires one. area/kubelet sig/node Categorizes an issue or PR as relevant to SIG Node. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Feb 17, 2023

k8s-ci-robot requested review from andrewsykim and bobbypage February 17, 2023 02:15

moshe010 marked this pull request as draft February 17, 2023 02:17

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 17, 2023

k8s-ci-robot assigned bart0sh, ffromani, klueska and swatisehgal Feb 17, 2023

ffromani reviewed Feb 17, 2023

View reviewed changes

bart0sh added this to Waiting on Author in SIG Node PR Triage Feb 19, 2023

bart0sh moved this from Waiting on Author to Triage in SIG Node PR Triage Feb 19, 2023

SergeyKanzhelev moved this from Needs Reviewer to Needs Approver in SIG Node PR Triage Mar 14, 2023

ffromani reviewed Mar 14, 2023

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 14, 2023

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 14, 2023

k8s-ci-robot requested review from dchen1107 and derekwaynecarr March 14, 2023 19:20

k8s-ci-robot assigned dchen1107 and derekwaynecarr Mar 14, 2023

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 14, 2023

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 14, 2023

k8s-ci-robot merged commit 28fa3cb into kubernetes:master Mar 14, 2023

SIG Node PR Triage automation moved this from Needs Approver to Done Mar 14, 2023

k8s-ci-robot added this to the v1.27 milestone Mar 14, 2023

This was referenced Mar 20, 2023

kubelet PodResources API: follow-up review comments #116742

Merged

REQUEST: New membership for moshe010 kubernetes/org#4103

Closed

klueska mentioned this pull request Jan 16, 2024

KEP-3695: extend pod resource API to for DRA kubernetes/enhancements#3738

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extend the PodResources API to include resources allocated by DRA #115847

Extend the PodResources API to include resources allocated by DRA #115847

moshe010 commented Feb 17, 2023 •

edited

k8s-ci-robot commented Feb 17, 2023

moshe010 commented Feb 17, 2023

ffromani left a comment

bart0sh commented Feb 19, 2023

ffromani left a comment

ffromani Mar 14, 2023

ffromani Mar 14, 2023

k8s-ci-robot commented Mar 14, 2023

ffromani commented Mar 14, 2023

ffromani commented Mar 14, 2023

ffromani commented Mar 14, 2023

klueska commented Mar 14, 2023

k8s-ci-robot commented Mar 14, 2023

k8s-ci-robot commented Mar 14, 2023

k8s-ci-robot commented Mar 14, 2023

klueska commented Mar 14, 2023

ffromani commented Mar 14, 2023

dims commented Mar 14, 2023

dchen1107 commented Mar 14, 2023

k8s-ci-robot commented Mar 14, 2023

moshe010 commented Mar 19, 2023

Extend the PodResources API to include resources allocated by DRA #115847

Extend the PodResources API to include resources allocated by DRA #115847

Conversation

moshe010 commented Feb 17, 2023 • edited

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.:

k8s-ci-robot commented Feb 17, 2023

moshe010 commented Feb 17, 2023

ffromani left a comment

Choose a reason for hiding this comment

bart0sh commented Feb 19, 2023

ffromani left a comment

Choose a reason for hiding this comment

ffromani Mar 14, 2023

Choose a reason for hiding this comment

ffromani Mar 14, 2023

Choose a reason for hiding this comment

k8s-ci-robot commented Mar 14, 2023

ffromani commented Mar 14, 2023

ffromani commented Mar 14, 2023

ffromani commented Mar 14, 2023

klueska commented Mar 14, 2023

k8s-ci-robot commented Mar 14, 2023

k8s-ci-robot commented Mar 14, 2023

k8s-ci-robot commented Mar 14, 2023

klueska commented Mar 14, 2023

ffromani commented Mar 14, 2023

dims commented Mar 14, 2023

dchen1107 commented Mar 14, 2023

k8s-ci-robot commented Mar 14, 2023

moshe010 commented Mar 19, 2023

moshe010 commented Feb 17, 2023 •

edited