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

Document pod level resources #48471

Merged
merged 1 commit into from
Nov 29, 2024

Conversation

ndixita
Copy link
Contributor

@ndixita ndixita commented Oct 21, 2024

Description

Placeholder PR for documentation changes for Pod Level Resources Alpha

Issue

kubernetes/enhancements#2837

@k8s-ci-robot k8s-ci-robot added this to the 1.32 milestone Oct 21, 2024
@k8s-ci-robot k8s-ci-robot added area/blog Issues or PRs related to the Kubernetes Blog subproject area/localization General issues or PRs related to localization area/release-eng Issues or PRs related to the Release Engineering subproject area/web-development Issues or PRs related to the kubernetes.io's infrastructure, design, or build processes language/bn Issues or PRs related to Bengali language language/de Issues or PRs related to German language labels Oct 21, 2024
@k8s-ci-robot k8s-ci-robot added language/en Issues or PRs related to English language language/es Issues or PRs related to Spanish language cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. language/fr Issues or PRs related to French language language/hi Issues or PRs related to Hindi language language/id Issues or PRs related to Indonesian language language/it Issues or PRs related to Italian language language/ja Issues or PRs related to Japanese language language/ko Issues or PRs related to Korean language size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. language/pl Issues or PRs related to Polish language language/pt Issues or PRs related to Portuguese language labels Oct 21, 2024
Copy link

netlify bot commented Oct 21, 2024

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

Name Link
🔨 Latest commit 0374213
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-io-vnext-staging/deploys/6749047ea880be0008323bbb

@k8s-ci-robot k8s-ci-robot added language/ru Issues or PRs related to Russian language language/uk Issues or PRs related to Ukrainian language language/vi Issues or PRs related to Vietnamese language language/zh Issues or PRs related to Chinese language sig/docs Categorizes an issue or PR as relevant to SIG Docs. sig/release Categorizes an issue or PR as relevant to SIG Release. labels Oct 21, 2024
@ndixita ndixita force-pushed the pod-resources-alpha branch from 876023a to 74dcdef Compare October 21, 2024 17:18
@ndixita ndixita changed the title KEP 2837: draft PR for Pod Level Resources KEP KEP KEP 2837: draft PR for Pod Level Resources KEP Oct 21, 2024
@ndixita ndixita force-pushed the pod-resources-alpha branch from e917e13 to 9286c45 Compare November 28, 2024 05:17
Copy link
Contributor

@sftim sftim left a comment

Choose a reason for hiding this comment

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

I'm afraid that this PR doesn't look complete (even for an alpha feature).

I'd expect to see a change to https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ as well as to the Tasks section of the doc (to be clear: the Tasks part is already covered).

For alpha you don't need to write in depth, but the concept explanation of Pod resources does need to cover the option of specifying resource requests and limits at Pod level.

@sftim
Copy link
Contributor

sftim commented Nov 28, 2024

/sig node

@k8s-ci-robot k8s-ci-robot added the sig/node Categorizes an issue or PR as relevant to SIG Node. label Nov 28, 2024
@sftim
Copy link
Contributor

sftim commented Nov 28, 2024

Here's the key piece of feedback: #48471 (review)

I think if that gets covered, we can merge the PR and then plan some post-merge fixups.

@ndixita ndixita force-pushed the pod-resources-alpha branch from 9286c45 to 44594f9 Compare November 28, 2024 22:51
@ndixita
Copy link
Contributor Author

ndixita commented Nov 28, 2024

This is a straightforward example of the work, well done on the time crunch @ndixita . in the future, I think we could add more detailed docs on the different interactions with OOM score/QoS class, but that's not in scope now I think

LGTM from tech perspective

@sftim this is where I have approvals from sig-node

@ndixita
Copy link
Contributor Author

ndixita commented Nov 28, 2024

Here's the key piece of feedback: #48471 (review)

I think if that gets covered, we can merge the PR and then plan some post-merge fixups.

Tried to address this. PTAL

@ndixita ndixita force-pushed the pod-resources-alpha branch from 44594f9 to 0741984 Compare November 28, 2024 23:03
Copy link
Contributor

@sftim sftim left a comment

Choose a reason for hiding this comment

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

Thanks. I've made suggestions that should mean we can merge this if you do another round of edits.

@ndixita ndixita force-pushed the pod-resources-alpha branch 2 times, most recently from 9bf04fb to 0fd613a Compare November 28, 2024 23:54
@ndixita ndixita force-pushed the pod-resources-alpha branch from 0fd613a to 0374213 Compare November 29, 2024 00:02
@haircommander
Copy link
Contributor

still LGTM!

Copy link
Contributor

@sftim sftim left a comment

Choose a reason for hiding this comment

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

/lgtm

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

LGTM label has been added.

Git tree hash: 82c26a92eb39483896e27796fe8aaf218eb6da99

@chanieljdan
Copy link
Contributor

Docs lgtm & tech lgtm noted

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chanieljdan

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 Nov 29, 2024
@k8s-ci-robot k8s-ci-robot merged commit d1a0c76 into kubernetes:dev-1.32 Nov 29, 2024
6 checks passed
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. sig/node Categorizes an issue or PR as relevant to SIG Node. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Development

Successfully merging this pull request may close these issues.

10 participants