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

cpu.share is not properly set for containers in kubernetes #8358

Closed
dchen1107 opened this issue May 15, 2015 · 8 comments
Closed

cpu.share is not properly set for containers in kubernetes #8358

dchen1107 opened this issue May 15, 2015 · 8 comments
Assignees
Labels
priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. sig/node Categorizes an issue or PR as relevant to SIG Node.

Comments

@dchen1107
Copy link
Member

I were trying to use limitRange to set default cpu request for each container to prevent overcommit a node badly before we have more intelligent scheduler, and identified the issue.

Through kubernetes I scheduled a container asking for 400m, (409 cpu.share) but kubernetes always assigns 1024 cpu.share to that cgroup.

# cat /sys/fs/cgroup/cpu/docker/63b0724fdfa08d44401bf7965689bd4105f099beb751b31d1bf26e2aeb71fadc/cpu.shares 
1024

Initially I thought kubelet doesn't pass cpu limit properly to docker, but docker inspect proves I were wrong:

# docker inspect 63b0724fdfa08d44401bf7965689bd4105f099beb751b31d1bf26e2aeb71fadc
[{
    "AppArmorProfile": "",
    "Args": [
        "-g",
        "daemon off;"
    ],
    "Config": {
        "AttachStderr": false,
        "AttachStdin": false,
        "AttachStdout": false,
        "Cmd": [
            "nginx",
            "-g",
            "daemon off;"
        ],
        "CpuShares": 409,
        "Cpuset": "",
        "Domainname": "",
        "Entrypoint": null,
        "Env": [
...

Then I thought it might be capped at docker level, but I tested it through docker cli, and proves I were wrong again:

# docker run -c 200 busybox sh
[{
    "AppArmorProfile": "",
    "Args": [],
    "Config": {
        "AttachStderr": true,
        "AttachStdin": true,
        "AttachStdout": true,
        "Cmd": [
            "sh"
        ],
        "CpuShares": 200,
...

# cat /sys/fs/cgroup/cpu/docker/efc87fa9a7c0087c60e116d0832e11d7f74a3c3c4ad51bd0348d7ec28a871037/cpu.shares 
200

It might be docker/libcontainer issue and triggered by the way how we are using docker itself. Filed the issue here for investigation. @vmarmol is going to look into it.

@dchen1107 dchen1107 added priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. sig/node Categorizes an issue or PR as relevant to SIG Node. labels May 15, 2015
@derekwaynecarr
Copy link
Member

Did you see a problem with memory as well if set? I swore I had verified this on the node a month or so ago, but maybe it was memory and not CPU.

/sub

Sent from my iPhone

On May 15, 2015, at 7:41 PM, Dawn Chen notifications@github.com wrote:

I were trying to use limitRange to set default cpu request for each container to prevent overcommit a node badly before we have more intelligent scheduler, and identified the issue.

Through kubernetes I scheduled a container asking for 400m, (409 cpu.share) but kubernetes always assigns 1024 cpu.share to that cgroup.

cat /sys/fs/cgroup/cpu/docker/63b0724fdfa08d44401bf7965689bd4105f099beb751b31d1bf26e2aeb71fadc/cpu.shares

1024
Initially I thought kubelet doesn't pass cpu limit properly to docker, but docker inspect proves I were wrong:

docker inspect 63b0724fdfa08d44401bf7965689bd4105f099beb751b31d1bf26e2aeb71fadc

[{
"AppArmorProfile": "",
"Args": [
"-g",
"daemon off;"
],
"Config": {
"AttachStderr": false,
"AttachStdin": false,
"AttachStdout": false,
"Cmd": [
"nginx",
"-g",
"daemon off;"
],
"CpuShares": 409,
"Cpuset": "",
"Domainname": "",
"Entrypoint": null,
"Env": [
...
Then I thought it might be capped at docker level, but I tested it through docker cli, and proves I were wrong again:

docker run -c 200 busybox sh

[{
"AppArmorProfile": "",
"Args": [],
"Config": {
"AttachStderr": true,
"AttachStdin": true,
"AttachStdout": true,
"Cmd": [
"sh"
],
"CpuShares": 200,
...

cat /sys/fs/cgroup/cpu/docker/efc87fa9a7c0087c60e116d0832e11d7f74a3c3c4ad51bd0348d7ec28a871037/cpu.shares

200
It might be docker/libcontainer issue and triggered by the way how we are using docker itself. Filed the issue here for investigation. @vmarmol is going to look into it.


Reply to this email directly or view it on GitHub.

@derekwaynecarr
Copy link
Member

Do we have an integration test that can verify This integration with docker and kubelet?

Sent from my iPhone

On May 15, 2015, at 7:41 PM, Dawn Chen notifications@github.com wrote:

I were trying to use limitRange to set default cpu request for each container to prevent overcommit a node badly before we have more intelligent scheduler, and identified the issue.

Through kubernetes I scheduled a container asking for 400m, (409 cpu.share) but kubernetes always assigns 1024 cpu.share to that cgroup.

cat /sys/fs/cgroup/cpu/docker/63b0724fdfa08d44401bf7965689bd4105f099beb751b31d1bf26e2aeb71fadc/cpu.shares

1024
Initially I thought kubelet doesn't pass cpu limit properly to docker, but docker inspect proves I were wrong:

docker inspect 63b0724fdfa08d44401bf7965689bd4105f099beb751b31d1bf26e2aeb71fadc

[{
"AppArmorProfile": "",
"Args": [
"-g",
"daemon off;"
],
"Config": {
"AttachStderr": false,
"AttachStdin": false,
"AttachStdout": false,
"Cmd": [
"nginx",
"-g",
"daemon off;"
],
"CpuShares": 409,
"Cpuset": "",
"Domainname": "",
"Entrypoint": null,
"Env": [
...
Then I thought it might be capped at docker level, but I tested it through docker cli, and proves I were wrong again:

docker run -c 200 busybox sh

[{
"AppArmorProfile": "",
"Args": [],
"Config": {
"AttachStderr": true,
"AttachStdin": true,
"AttachStdout": true,
"Cmd": [
"sh"
],
"CpuShares": 200,
...

cat /sys/fs/cgroup/cpu/docker/efc87fa9a7c0087c60e116d0832e11d7f74a3c3c4ad51bd0348d7ec28a871037/cpu.shares

200
It might be docker/libcontainer issue and triggered by the way how we are using docker itself. Filed the issue here for investigation. @vmarmol is going to look into it.


Reply to this email directly or view it on GitHub.

@vmarmol
Copy link
Contributor

vmarmol commented May 16, 2015

From my testing this applies to both CPU and memory. Docker accepts the limit (and reports it), but it doesn't ask libcontainer to set the limit.

@derekwaynecarr we do not have a test today, we need one :)

@dchen1107
Copy link
Member Author

I swore I checked memory a while back, and it works too. This is my first time checking cpu though. I teared down my cluster just now. Will bring up one and reported it back soon.

We do have integration tests but running a fake docker daemon. We also have a bunch of e2e tests to make sure pods on node running as expected, but no resource related validation. I am filing an issue to add e2e test for this.

@vmarmol
Copy link
Contributor

vmarmol commented May 16, 2015

Memory is not set either, all resource limits are dropped.

@dchen1107
Copy link
Member Author

filed #8365

@vmarmol
Copy link
Contributor

vmarmol commented May 16, 2015

Just found the issue, it is in go-dockerclient and how we use it. This has actually never worked I think :(

We set the limits in the Config here which is what is provided by the Docker client we use.

But the resource fields actually don't exist in the official Docker API. There is verbiage that states that it should be in HostConfig which we also set but not for the resource values. However, the Docker client doesn't have these fields in HostConfig.

What is even more confusing is that Docker inspect will happily show the config we provide with the unknown fields, but since they are not in HostConfig they don't take effect.

Will file and fix the issue in go-dockerclient and then change our use to fill in HostConfig.

@vmarmol
Copy link
Contributor

vmarmol commented May 16, 2015

The plot thickens, looks like it was a breaking change in the Docker remote API. The last version had the field on the request: https://docs.docker.com/reference/api/docker_remote_api_v1.17/#create-a-container

@vmarmol vmarmol self-assigned this May 19, 2015
vmarmol added a commit to vmarmol/kubernetes that referenced this issue May 19, 2015
This allows for backwards and forwards compatability since old Docker
versions expect it in Create() and newer ones do so in Start().

Fixes kubernetes#8358
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. sig/node Categorizes an issue or PR as relevant to SIG Node.
Projects
None yet
Development

No branches or pull requests

3 participants