Skip to content

Conversation

samuelkarp
Copy link
Member

Previous versions of libcontainer allowed CpuShares that were greater
than the maximum or less than the minimum supported by the kernel, and
relied on the kernel to do the right thing. Newer libcontainer fails
after creating the container if the requested CpuShares is different
from what was actually created by the kernel, which breaks compatibility
with earlier Docker Remote API versions. This change explicitly adjusts
the requested CpuShares in API versions < 1.20.

Signed-off-by: Samuel Karp skarp@amazon.com

Fixes #13721

@vieux
Copy link
Contributor

vieux commented Jun 3, 2015

ping @crosbymichael

Previous versions of libcontainer allowed CpuShares that were greater
than the maximum or less than the minimum supported by the kernel, and
relied on the kernel to do the right thing. Newer libcontainer fails
after creating the container if the requested CpuShares is different
from what was actually created by the kernel, which breaks compatibility
with earlier Docker Remote API versions. This change explicitly adjusts
the requested CpuShares in API versions < 1.20.

Signed-off-by: Samuel Karp <skarp@amazon.com>
@samuelkarp samuelkarp force-pushed the CpuShareRemoteAPI branch from 28c4868 to ed39fbe Compare June 5, 2015 00:20
@jessfraz
Copy link
Contributor

jessfraz commented Jun 5, 2015

I feel like there might be a better place to put this but other than that LGTM

@samuelkarp
Copy link
Member Author

Thanks @jfrazelle. I don't disagree; I was mostly trying to avoid sending the version (or a flag) even deeper into the internals. Keeping it in the API layer does this, but at the expense of making the API layer aware of kernel limits.

@icecrime icecrime added this to the 1.7.0 milestone Jun 11, 2015
@icecrime
Copy link
Contributor

Thanks @samuelkarp! The original patch in libcontainer makes sense, but I agree the Docker API impact is undesirable: LGTM.

@icecrime
Copy link
Contributor

API documentation was already very vague on accepted value ("CpuShares - An integer value containing the CPU Shares for container (ie. the relative weight vs other containers"). I don't think it needs updating.

icecrime pushed a commit that referenced this pull request Jun 11, 2015
Adjust disallowed CpuShares in /containers/create
@icecrime icecrime merged commit 71ead0e into moby:master Jun 11, 2015
@jessfraz
Copy link
Contributor

cherry-picked

@amylindburg
Copy link
Contributor

Oh Man you are most awesome.

riuvshin pushed a commit to riuvshin/che-plugins that referenced this pull request Sep 21, 2015
riuvshin pushed a commit to codenvy-legacy/che-plugins that referenced this pull request Sep 21, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants