Remove Memory MemorySwap CpuShares from config #495
Conversation
As I said in opencontainers#371 (comment), I'd like to remove these resource limit from image-spec. On the on hand, if we have this, we should have limit the min and max size of these value. For example, in docker, there are some min and max limit for some resource, see https://github.com/docker/docker/blob/master/daemon/daemon_unix.go#L48 Signed-off-by: Lei Jitang <[email protected]>
|
We can't remove them, it'll break compability with Docker. |
|
The interesting code is in https://github.com/docker/docker/blob/master/daemon/commit.go#L198 and https://github.com/docker/docker/blob/master/daemon/commit.go#L27. Pay attention to which fields are actually merged. It would be helpful if you could do the analysis for which fields actually end up on disk. As far as I can tell, it is defined by |
|
Adding to the above, from my inspection, these fields don't land on disk. Also, note that there are interesting lines, like https://github.com/docker/docker/blob/master/daemon/commit.go#L70, which don't seem correct. |
|
@stevvooe As the resident expert on keeping Docker compatibility what is your recommendation? |
|
@stevvooe These fields does land on disk on prev v1.6 docker, see https://github.com/docker/docker/blob/v1.6.0/runconfig/merge.go#L14 and https://github.com/docker/docker/blob/v1.6.0/runconfig/config.go#L15 , https://github.com/docker/docker/blob/v1.6.0/image/image.go#L31 but after moby/moby#10298 which landed in v1.7 docker, all cgroup field are move to |
|
LGTM After doing some further research, I found these fields are from before the Config/HostConfig split. In practice, these fields are really host-specific. Their appearance in 1.6 will be a non-factor for modern Docker deployments. If we convert them into the post-1.9 formats, they will be ignored and not written to disk. |
|
I'm 👍 if they aren't actually used (they always seemed quite an odd addition). |
|
LGTM. Thank you @stevvooe !!! |
|
yay
…On 16 December 2016 at 02:36, Stephen Day ***@***.***> wrote:
Merged #495 <#495>.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#495 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ACewNyfQn2f4ddy-tFxGoYYRJgM_gX48ks5rIesjgaJpZM4LMrVV>
.
|
|
/me will update #492 to remove these. |
5 participants
As I said in #371 (comment),
I'd like to remove these resource limit from image-spec.
On the on hand, if we have this, we should have limit
the min and max size of these value. For example, in
docker, there are some min and max limit for some
resource, see https://github.com/docker/docker/blob/master/daemon/daemon_unix.go#L48
ping @vbatts @stevvooe
Signed-off-by: Lei Jitang [email protected]