Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extend PodResources API for Dynamic Resource Allocation #39978

Merged

Conversation

moshe010
Copy link
Contributor

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

@k8s-ci-robot k8s-ci-robot added this to the 1.27 milestone Mar 14, 2023
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Mar 14, 2023
@moshe010 moshe010 changed the title doc: extend PodResources API for Dynamic Resource Allocation WIP doc: extend PodResources API for Dynamic Resource Allocation Mar 14, 2023
@k8s-ci-robot 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 Mar 14, 2023
@k8s-ci-robot k8s-ci-robot added the language/en Issues or PRs related to English language label Mar 14, 2023
@k8s-ci-robot k8s-ci-robot added the sig/docs Categorizes an issue or PR as relevant to SIG Docs. label Mar 14, 2023
@moshe010 moshe010 force-pushed the pod-resource-api-dra-doc-upstream branch from 2fc4899 to d3fadde Compare March 16, 2023 08:49
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Mar 16, 2023
@moshe010 moshe010 changed the title WIP doc: extend PodResources API for Dynamic Resource Allocation Extend PodResources API for Dynamic Resource Allocation Mar 16, 2023
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 16, 2023
repeated ClaimResource claim_resources = 4;
}

// ClaimResource contains per plugin resource information
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// ClaimResource contains per plugin resource information
// ClaimResource contains per-plugin resource information

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Comment on lines 372 to 374
The `Get` endpoint provides information on resources of a running pod. It exposes similar information as
describe in the `List` endpoint. The `Get` endpoint requires `pod_name` and `pod_namesapce` of the running
pod.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The `Get` endpoint provides information on resources of a running pod. It exposes similar information as
describe in the `List` endpoint. The `Get` endpoint requires `pod_name` and `pod_namesapce` of the running
pod.
The `Get` endpoint provides information on resources of a running Pod. It exposes information
similar to those described in the `List` endpoint. The `Get` endpoint requires `pod_name` and
`pod_namespace` of the running Pod.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the way, the design to use _ in the field name is a bad decision against the convention to use camelCase names.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so this pod_namespace and pod_name taken from the proto file syntax which is underscore. For golang it will be camelCase according to golang conventions. I updated in the commit.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@moshe010 Would you mind responding to @tengqm when you get the chance? Tomorrow, April 4th, is the Docs complete — All PRs reviewed and ready to merge deadline.

sorry for the late response I was OOO on Monday.

}
```

To enable this feature `kubelet` must be started with the following flag:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
To enable this feature `kubelet` must be started with the following flag:
To enable this feature, you must start your kubelet services with the following flag:

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Comment on lines 390 to 391
The `Get` enpoint can provide information on dynamic resources
of a running pod allocated by the dynamic resource allocation API. To enable this feature `kubelet`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The `Get` enpoint can provide information on dynamic resources
of a running pod allocated by the dynamic resource allocation API. To enable this feature `kubelet`
The `Get` endpoint can provide Pod information related to dynamic resources
allocated by the dynamic resource allocation API. To enable this feature, you must

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done


The `Get` enpoint can provide information on dynamic resources
of a running pod allocated by the dynamic resource allocation API. To enable this feature `kubelet`
must be started with the following flags:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
must be started with the following flags:
ensure your kubelet services are started with the following flags:

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@@ -162,6 +162,12 @@ gets scheduled onto one node and then cannot run there, which is bad because
such a pending Pod also blocks all other resources like RAM or CPU that were
set aside for it.

## Monitoring resources

The kubelet provides a gRPC service to enable discovery of Dynamic Resources of
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The kubelet provides a gRPC service to enable discovery of Dynamic Resources of
The kubelet provides a gRPC service to enable discovery of dynamic resources of

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Comment on lines 583 to 584
- `KubeletPodResourcesGet`: Enable the kubelet's pod resources gRPC endpoint
`Get` functionality. This API augments the [resource allocation reporting](/docs/concepts/extend-kubernetes/compute-storage-net/device-plugins/#monitoring-device-plugin-resources).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- `KubeletPodResourcesGet`: Enable the kubelet's pod resources gRPC endpoint
`Get` functionality. This API augments the [resource allocation reporting](/docs/concepts/extend-kubernetes/compute-storage-net/device-plugins/#monitoring-device-plugin-resources).
- `KubeletPodResourcesGet`: Enable the `Get` gRPC endpoint on kubelet's for Pod resources.
This API augments the [resource allocation reporting](/docs/concepts/extend-kubernetes/compute-storage-net/device-plugins/#monitoring-device-plugin-resources).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@mickeyboxell
Copy link
Contributor

@moshe010 Would you mind responding to @tengqm when you get the chance? Tomorrow, April 4th, is the Docs complete — All PRs reviewed and ready to merge deadline.

@moshe010 moshe010 force-pushed the pod-resource-api-dra-doc-upstream branch from d3fadde to 7671435 Compare April 4, 2023 07:11
@netlify
Copy link

netlify bot commented Apr 4, 2023

👷 Deploy Preview for kubernetes-io-vnext-staging processing.

Name Link
🔨 Latest commit eaf9199
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-io-vnext-staging/deploys/642bcf0208b605000837b4f8

Signed-off-by: Moshe Levi <moshele@nvidia.com>
@moshe010 moshe010 force-pushed the pod-resource-api-dra-doc-upstream branch from 7671435 to eaf9199 Compare April 4, 2023 07:17
@mickeyboxell
Copy link
Contributor

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mickeyboxell

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:

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 4, 2023
@LukeMwila
Copy link

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 4, 2023
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 9e5b21034cb7ee674a50780647aa7c537404a9a0

@k8s-ci-robot k8s-ci-robot merged commit f62d58d into kubernetes:dev-1.27 Apr 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. language/en Issues or PRs related to English language lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/docs Categorizes an issue or PR as relevant to SIG Docs. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants