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

Kube-proxy requests 2x cpu shares of addons. #22022

Merged
merged 1 commit into from
Feb 26, 2016
+6 −1
Merged

Kube-proxy requests 2x cpu shares of addons. #22022

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion cluster/saltbase/salt/kube-proxy/init.sls
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
- mode: 644
- makedirs: true
- dir_mode: 755
- context:
cpurequest: '200m'
- require:
- service: docker
- service: kubelet
Expand All @@ -26,7 +28,7 @@
- group: root
- mode: 644

#stop legacy kube-proxy service
#stop legacy kube-proxy service
stop_kube-proxy:
service.dead:
- name: kube-proxy
Expand Down
3 changes: 3 additions & 0 deletions cluster/saltbase/salt/kube-proxy/kube-proxy.manifest
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ spec:
containers:
- name: kube-proxy
image: {{pillar['kube_docker_registry']}}/kube-proxy:{{pillar['kube-proxy_docker_tag']}}
resources:
requests:
Copy link
Contributor

Choose a reason for hiding this comment

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

We should set limits ideally.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't want to curtail kube-proxy, I want it to get what it needs (plus I'm not 100% sure how much it will need).

But I guess it doesn't make sense giving unlimited cpu to the proxy if nothing can run on the node, and if it bumps into the limit it's because something else with higher priority came along, and it'll just be slow, unlike memory.

Copy link
Contributor

Choose a reason for hiding this comment

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

requests are OK for now I guess.
Depending on where we go with the QoS policies, this might make kube-proxy
have less priority. But given that it is a DaemonSet, I guess the policies
will be thwarted anyways.

On Thu, Feb 25, 2016 at 3:25 PM, Prashanth B notifications@github.com
wrote:

In cluster/saltbase/salt/kube-proxy/kube-proxy.manifest
#22022 (comment)
:

@@ -31,6 +31,9 @@ spec:
containers:

  • name: kube-proxy
    image: {{pillar['kube_docker_registry']}}/kube-proxy:{{pillar['kube-proxy_docker_tag']}}
    • resources:
    •  requests:

I don't want to curtail kube-proxy, I want it to get what it needs (plus
I'm not 100% sure how much it will need).

But I guess it doesn't make sense giving unlimited cpu to the proxy if
nothing can run on the node, and if it bumps into the limit it's because
something else with higher priority came along, and it'll just be slow,
unlike memory.

—
Reply to this email directly or view it on GitHub
https://github.com/kubernetes/kubernetes/pull/22022/files#r54182925.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah none of the master components set limits, for the same reason. If kube-proxy on a node is really slow it could impact a cluster spanning service, so I think we should leave it as request for now and revisit if required.

cpu: {{ cpurequest }}
command:
- /bin/sh
- -c
Expand Down