move resource options from Config to hostConfig #10298
Conversation
|
@crosbymichael @unclejack @LK4D4 please take a look at this one, once this is good to go, the movement for other options will on their way soon. Changes for test cases will in subsequent PR, these changes won't made any test cases fail. |
|
@hqhq cool, thank you! But I think that this will be in 1.18 api version and we're now in sort of "code freeze" |
|
approach looks good to me. Maybe we should do all the fields in one PR so we get everything updated in one merge. |
|
and something in the release notes and API recent changes for those that were using the API. |
76f2b44 to
931f6b7
Compare
|
updated, add all resource option changes in this PR. @SvenDowideit do you mean add something for |
|
Docs LGTM |
|
ping @crosbymichael @LK4D4 |
|
@hqhq You've got to rebase the PR because it can't be merged. |
931f6b7 to
c1888c1
Compare
|
@unclejack @crosbymichael @LK4D4 |
|
A couple of copy-edits, otherwise docs LGTM. |
|
I presume these API changes need to move to the 1.18 API doc |
c1888c1 to
d70569f
Compare
|
Why are we changing the CLI flag? |
|
ping @ashahab-altiscale for the lxc template change |
|
@jfrazelle Thanks for letting me know. Can you start an lxc build on this? |
|
@cpuguy83 Do you mean we change --cpuset to --cpuset-cpus? |
d70569f to
cea4166
Compare
|
LGTM |
|
LGTM just needs commits to be squashed :) and maybe @crosbymichael because he is very good at catching weird things in the configs |
|
Hi, sorry for the late, I'm not sure how these commits can be squashed any more, are you proposing to make all the changes in one commit? |
|
Hi, this PR is depended by a lot of PRs, can we pick this up so we can move on? |
|
ping. |
Cgroup resources are host dependent, they should be in hostConfig.
For backward compatibility, we just copy it to hostConfig, and leave it in
Config for now, so there is no regressions, but the right way to use this
throught json is to put it in HostConfig, like:
{
"Hostname": "",
...
"HostConfig": {
"CpuShares": 512,
"Memory": 314572800,
...
}
}
As we will add CpusetMems, CpusetCpus is definitely a better name, but some
users are already using Cpuset in their http APIs, we also make it compatible.
The main idea is keep using Cpuset in Config Struct, and make it has the same
value as CpusetCpus, but not always, some scenarios:
- Users use --cpuset in docker command, it can setup cpuset.cpus and can
get Cpuset field from docker inspect or other http API which will get
config info.
- Users use --cpuset-cpus in docker command, ditto.
- Users use Cpuset field in their http APIs, ditto.
- Users use CpusetCpus field in their http APIs, they won't get Cpuset field
in Config info, because by then, they should already know what happens
to Cpuset.
Signed-off-by: Qiang Huang <h.huangqiang@huawei.com>
cea4166 to
837eec0
Compare
|
@crosbymichael @jfrazelle @LK4D4 @cpuguy83 Please review this, it have been taken too long (more than 4 months since the original PR), a lot of cgroup improvement are depending on this one, hope we can make some progress soon. Thanks. |
|
As far as I can tell, this PR has both code and docs LGTM, and the commits have been squashed as requested. Thanks for you patience @hqhq, I'm merging this hoping that nobody will have any objections. |
move resource options from Config to hostConfig
Cgroup resources are host dependent, they should be in hostConfig.
For backward compatibility, we just copy it to hostConfig, and leave it in
Config for now, so there is no regressions, but the right way to use this
throught json is to put it in HostConfig, like:
{
"Hostname": "",
...
"HostConfig": {
"Memory": 314572800,
...
}
}
Signed-off-by: Qiang Huang h.huangqiang@huawei.com