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.

Sign up for GitHub

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

Jump to bottom

Add resource types for ingress and egress bandwidth #12187

Closed
wants to merge 1 commit into from
Closed

Add resource types for ingress and egress bandwidth #12187

wants to merge 1 commit into from

Conversation

brendandburns
Copy link
Contributor

@bgrant0607 @johnwilkes

The code to implement this is in #11988

The basic intent is to use the tc utility to limit bandwidth on the Pod's IP address on the cbr0 bridge

@k8s-bot
Copy link

k8s-bot commented Aug 4, 2015

GCE e2e build/test passed for commit e95270c89c1220196c77476c3cc82f169742706d.

@ncdc
Copy link
Member

ncdc commented Aug 4, 2015

@eparis

@eparis
Copy link
Contributor

eparis commented Aug 4, 2015

@rajatchopra @dcbw @danwillians

@eparis
Copy link
Contributor

eparis commented Aug 4, 2015

@danwinship

@bgrant0607
Copy link
Member

Is there a proposal somewhere?

bgrant0607
bgrant0607 reviewed Aug 5, 2015
View reviewed changes
pkg/api/types.go
@@ -1391,7 +1391,10 @@ const (
ResourceMemory ResourceName = "memory"
// Volume size, in bytes (e,g. 5Gi = 5GiB = 5 * 1024 * 1024 * 1024)
ResourceStorage ResourceName = "storage"
// Number of Pods that may be running on this Node: see ResourcePods
// Bandwidth available for egress from a pod (bits/second)
ResourceEgressBandwidth = "egress_bandwidth"
Copy link
Member

Choose a reason for hiding this comment

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

We don't have underscores in the API, and this shouldn't pass validation. I suggest just egress and ingress. Qualities such as latency aren't consumable resources.

cc @thockin @erictune

@bgrant0607
Copy link
Member

Also, documentation? Issue number?

@bgrant0607 bgrant0607 added the area/api Indicates an issue on api area. label Aug 5, 2015
@brendandburns
Copy link
Contributor Author

@bgrant0607

Issues are #11965 and #2856

I've added documentation, the implementation details are in #11988

Comments addressed as well, please re-check.
--brendan

@k8s-bot
Copy link

k8s-bot commented Aug 5, 2015

GCE e2e build/test passed for commit e98dc9ac3f298cd26d2570b86725d8a19f75ee07.

johnwilkes
johnwilkes reviewed Aug 5, 2015
View reviewed changes
docs/user-guide/bandwidth-resources.md
egress: "2Mi"
```

## How Pods with Resource Limits are Scheduled
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious To Know Why There Are So Many Capitals Here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed.

@bgrant0607
Copy link
Member

cc @erictune

davidopp
davidopp reviewed Aug 11, 2015
View reviewed changes
docs/user-guide/compute-resources.md
@@ -74,7 +74,7 @@ configured.

Although limits can only be specified on individual containers, it is convenient to talk about pod
resource limits. A *pod resource limit* for a particular resource type is the sum of the resource
limits of that type for each container in the pod, with unset values treated as zero.
requests of that type for each container in the pod, with unset values treated as zero.
Copy link
Member

Choose a reason for hiding this comment

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

why did you change this to requests?

@derekwaynecarr derekwaynecarr mentioned this pull request Aug 12, 2015
Closed
@k8s-github-robot k8s-github-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 27, 2015
@k8s-github-robot
Copy link

Labelling this PR as size/L

@bgrant0607
Copy link
Member

Another reference: http://research.microsoft.com/apps/pubs/default.aspx?id=244865

@k8s-bot
Copy link

k8s-bot commented Sep 2, 2015

GCE e2e build/test passed for commit 6971267.

@eparis eparis mentioned this pull request Sep 12, 2015
Closed
@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 15, 2015
derekwaynecarr
derekwaynecarr reviewed Sep 30, 2015
View reviewed changes
docs/user-guide/bandwidth-resources.md
## How pods with resource limits are scheduled

When a pod is created, the Kubernetes scheduler selects a node for the pod to
run on. Each node has a maximum capacity for each of the resource types: the
Copy link
Member

Choose a reason for hiding this comment

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

How does the node report bandwidth capacity? This alludes to CPU and memory, but does not discuss how ingress, and egress are expressed. Does it require the operator to define this?

@k8s-github-robot k8s-github-robot added the kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API label Oct 9, 2015
@erictune erictune added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Oct 29, 2015
@erictune
Copy link
Member

Brendan: it looks like this feature is active in kubelet by default. So, suggest either: flag default off in kubelet or update doc to reflect the "alpha" status of the feature and the use of annotation, and then merge this doc.

@erictune
Copy link
Member

@brendandburns

@erictune erictune assigned erictune and unassigned bgrant0607 Oct 29, 2015
@davidopp davidopp added team/control-plane priority/backlog Higher priority than priority/awaiting-more-evidence. labels Oct 31, 2015
@davidopp davidopp mentioned this pull request Nov 7, 2015
Closed
@k8s-bot
Copy link

k8s-bot commented Nov 14, 2015

GCE e2e build/test failed for commit 6971267.

@mali11
Copy link

mali11 commented Dec 3, 2015

Without adding the new-added resource names to the standardResources, won't the API fail the input validation? or if I missed something..

@k8s-bot
Copy link

k8s-bot commented Feb 16, 2016

GCE e2e build/test failed for commit 6971267.

@k8s-github-robot k8s-github-robot added kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/old-docs labels Mar 1, 2016
@eparis eparis mentioned this pull request Mar 3, 2016
Merged
@k8s-github-robot
Copy link

Adding label:do-not-merge because PR changes docs prohibited to auto merge
See http://kubernetes.github.io/docs/editdocs/ for information about editting docs

@k8s-github-robot k8s-github-robot added the do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. label Mar 4, 2016
@erictune
Copy link
Member

Idle for several months. Closing.

@erictune erictune closed this Mar 23, 2016
@krallistic krallistic mentioned this pull request Mar 24, 2017
Open
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@erictune erictune

Labels
area/api Indicates an issue on api area. do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. priority/backlog Higher priority than priority/awaiting-more-evidence. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet