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

Add GetAllocatableResource to PodResource API #2404

Conversation

ffromani
Copy link
Contributor

@ffromani ffromani commented Feb 2, 2021

In order to simplify and make more understandable the KEP, and
to comply with the new process, we extract the unit of work still
ongoing in this KEP from #1884

Work in this area was done during the 1.20 and 1.21 cycles in
kubernetes/kubernetes#95734

Rationale, discussion and documentation for all the changes including
the one proposed in this KEP have been described in
https://github.com/kubernetes/enhancements/tree/master/keps/sig-node/2043-pod-resource-concrete-assigments
and reported here were relevant

Signed-off-by: Francesco Romani [email protected]

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Feb 2, 2021
@k8s-ci-robot
Copy link
Contributor

Hi @fromanirh. 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.

@k8s-ci-robot k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 2, 2021
@k8s-ci-robot k8s-ci-robot added kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory sig/node Categorizes an issue or PR as relevant to SIG Node. labels Feb 2, 2021
@ffromani
Copy link
Contributor Author

ffromani commented Feb 2, 2021

rationale for this split in this slack conversation

the main goal is to make this change easier to review

@derekwaynecarr
Copy link
Member

@ffromani ffromani force-pushed the podresources-get-alloctable-resources branch from d3de0da to 1181c5b Compare February 2, 2021 18:19
@ffromani
Copy link
Contributor Author

ffromani commented Feb 2, 2021

/assign @derekwaynecarr @johnbelamaric

@ffromani ffromani force-pushed the podresources-get-alloctable-resources branch from 1181c5b to 2d90789 Compare February 4, 2021 19:36
- Metric name: `pod_resources_endpoint_requests_total`
- Components exposing the metric: kubelet

* **What are the reasonable SLOs (Service Level Objectives) for the above SLIs?** N/A or refer to Kubelet SLIs.
Copy link
Member

Choose a reason for hiding this comment

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

do we have SLIs/SLOs for the pod-resources api in general? should we expect < x% of requests to be errors?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

in general this API is expected to never fail. I'll just leave "N/A" here.

Copy link
Member

@johnbelamaric johnbelamaric left a comment

Choose a reason for hiding this comment

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

By the way, not all of the PRR sections need to be complete for alpha readiness to be approved.

@ffromani
Copy link
Contributor Author

ffromani commented Feb 4, 2021

e2e tests addition after the feature gate addition: kubernetes/kubernetes@e5c5db0

@ffromani ffromani force-pushed the podresources-get-alloctable-resources branch from 2d90789 to bf341b5 Compare February 4, 2021 21:34
Copy link
Member

@derekwaynecarr derekwaynecarr left a comment

Choose a reason for hiding this comment

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

just the following requests:

  • metric for failure
  • clarify the GRPC API always returns an error if invoked when feature gate is disabled

thanks for separating this into its own enhancement!

In order to simplify and make more understandable the KEP, and
to comply with the new process, we extract the unit of work still
ongoing in this KEP from kubernetes#1884

Work in this area was done during the 1.20 and 1.21 cycles  in
kubernetes/kubernetes#95734

Rationale, discussion and documentation for all the changes including
the one proposed in this KEP have been described in
https://github.com/kubernetes/enhancements/tree/master/keps/sig-node/2043-pod-resource-concrete-assigments
and reported here were relevant

Signed-off-by: Francesco Romani <[email protected]>
@ffromani ffromani force-pushed the podresources-get-alloctable-resources branch from ed2dbf2 to 9e04029 Compare February 8, 2021 18:26
Copy link
Member

@ehashman ehashman 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 Feb 8, 2021
@k8s-ci-robot k8s-ci-robot merged commit 47ff509 into kubernetes:master Feb 8, 2021
@k8s-ci-robot k8s-ci-robot added this to the v1.21 milestone Feb 8, 2021
@ffromani ffromani deleted the podresources-get-alloctable-resources branch February 8, 2021 19:24
ffromani added a commit to ffromani/kubernetes that referenced this pull request Mar 4, 2021
ffromani added a commit to ffromani/kubernetes that referenced this pull request Mar 5, 2021
ffromani added a commit to ffromani/kubernetes that referenced this pull request Mar 7, 2021
ffromani added a commit to ffromani/kubernetes that referenced this pull request Mar 7, 2021
ffromani added a commit to ffromani/kubernetes that referenced this pull request Mar 9, 2021
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. kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants