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

CLI flag for docker create(run) to change block device size. #19367

Merged
merged 1 commit into from
Mar 29, 2016

Conversation

shishir-a412ed
Copy link
Contributor

Signed-off-by: Shishir Mahajan shishir.mahajan@redhat.com

@shishir-a412ed
Copy link
Contributor Author

Now that daemon option (--storage-opt dm.basesize) has been introduced upstream, I would like to rekindle the discussion on introducing CLI flag (e.g. --storage-opt size:30G) for docker create/run which would allow a user to set containers rootfs size at creation time (provided it is greater than the base device size)

There were two major problems with the existing implementation (daemon option):

  1. This daemon option would allow the user to expand the basedevice size, however that does not necessarily mean that all new containers would be of this new size. I ll give an example. Lets assume my current docker has 2 images (fedora, ubuntu) and 2 containers (fedora_container, ubuntu_container) with the default basedevice size=10G.
    Now I restart the daemon and expanded the base device size to 30G (docker daemon --storage-opt dm.basesize=30G).

If I create a new container on any of the existing images (fedora, ubuntu) the rootfs size of those containers would still be 10G as they are snapshots of the existing image block device and not the basedevice which we just expanded. I feel this would be a little confusing to the user as he might be expecting the new size to show up. To make it work, I would need to remove my existing images (possibly containers associated with it) and repull them in order for my new containers rootfs to be of new base device size (dm.basesize=30G)

  1. With this approach the heaviest application (container) would dictate the size for the rest of the containers. e.g. If I want to have 100 containers on my infrastructure and one of them is a data intensive application requiring 100G of space. I would have to set the base device size (dm.basesize=100G) to 100G. Even though there are other 99 containers which only needed 200 MB of space.

This solution gives more flexibility and solves these problems.

Shishir

@shishir-a412ed
Copy link
Contributor Author

ping @calavera @rhvgoyal @tiborvass

Can we atleast start the design discussions ?

Shishir

@shishir-a412ed shishir-a412ed force-pushed the rootfs_size_configurable_cli branch 8 times, most recently from 5650290 to 4c2efec Compare February 1, 2016 14:31
@shishir-a412ed
Copy link
Contributor Author

@calavera @rhvgoyal @tiborvass
Any updates on this one ?

Shishir

@shishir-a412ed shishir-a412ed force-pushed the rootfs_size_configurable_cli branch 2 times, most recently from 8b690f2 to bb686db Compare February 4, 2016 15:00
@anusha-ragunathan
Copy link
Contributor

Please fix test failures.

@shishir-a412ed
Copy link
Contributor Author

@anusha-ragunathan Fixed. Please check now.
Don't worry about the vendor failure. That change would eventually go to engine-api and will be vendored into docker. Lets atleast start the design discussions.

Shishir

@tiborvass tiborvass added the status/needs-attention Calls for a collective discussion during a review session label Feb 5, 2016
@shishir-a412ed shishir-a412ed force-pushed the rootfs_size_configurable_cli branch 2 times, most recently from 10581ff to 0144a96 Compare February 9, 2016 15:47
@thaJeztah
Copy link
Member

@shishir-a412ed it should be there (see 928ea1e)

@shishir-a412ed shishir-a412ed force-pushed the rootfs_size_configurable_cli branch 3 times, most recently from 0651e54 to 7521e1c Compare March 23, 2016 18:38
@shishir-a412ed
Copy link
Contributor Author

@thaJeztah Please check now.

@thaJeztah
Copy link
Member

Thanks @shishir-a412ed, LGTM

@shishir-a412ed shishir-a412ed force-pushed the rootfs_size_configurable_cli branch 3 times, most recently from fdf91e4 to 59f002e Compare March 23, 2016 19:55
@thaJeztah
Copy link
Member

janky hitting #21408 (not related)

Signed-off-by: Shishir Mahajan <shishir.mahajan@redhat.com>
@shishir-a412ed
Copy link
Contributor Author

@thaJeztah any updates on this one ?

@thaJeztah
Copy link
Member

oh! I think it needs one more LGTM for the documentation

ping @vdemeester @MHBauer ptal

@vdemeester
Copy link
Member

LGTM 🐮

@vdemeester vdemeester merged commit e6aa40a into moby:master Mar 29, 2016
@@ -60,6 +60,27 @@ func (s *DockerSuite) TestCreateArgs(c *check.C) {

}

// Make sure we can grow the container's rootfs at creation time.
func (s *DockerSuite) TestCreateGrowRootfs(c *check.C) {
testRequires(c, Devicemapper)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there something missing? I can not find any code about Devicemapper testRequirement

Copy link
Member

Choose a reason for hiding this comment

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

@thaJeztah thaJeztah added this to the 1.12.0 milestone Mar 29, 2016
shishir-a412ed pushed a commit to shishir-a412ed/docker that referenced this pull request May 5, 2016
…ock device size

Signed-off-by: Shishir Mahajan <shishir.mahajan@redhat.com>

$ docker create -it --storage-opt size=120G fedora /bin/bash

This (size) will allow to set the container rootfs size to 120G at creation time.
Copy link
Contributor

@tiborvass tiborvass Jun 17, 2016

Choose a reason for hiding this comment

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

@thaJeztah @SvenDowideit it should be noted that htis is devicemapper specific. The flag is not, but the value is. Followup pr someone?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tiborvass what are you looking for ?

Copy link
Contributor

Choose a reason for hiding this comment

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

do all storage drivers have a size option ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tiborvass I think this would be easier to discuss on irc. We can discuss this on Monday.

Copy link
Contributor

Choose a reason for hiding this comment

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

Note that this is no longer devicemapper specific. Windows will have support when #23391 is merged. ZFS also supports this since #21946. There might be more that I missed as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@DrewRay
Copy link

DrewRay commented Jul 22, 2016

What about on docker build ? can we somehow get it to create the new images with a larger base size?

@shishir-a412ed
Copy link
Contributor Author

@DrewRay Please see discussions going on at #22701

With this patch, I can create/run a container with a size bigger than base device size. However there were certain limitations.

  1. Even if I start a container with bigger size, I cannot commit it. This affects docker commit.
  2. I cannot build an image bigger than base device size. This affects docker build.
  3. I cannot pull an image bigger than base device size. This affects docker pull

Upvote on the Issue #22701, If you feel this is a useful feature.
I have patches for all 3 of them. I can open a PR if there is enough interest in these features.

Shishir

liusdu pushed a commit to liusdu/moby that referenced this pull request Oct 30, 2017
CloudRAN need this.
cherry-pick from moby#19367

Signed-off-by: Shishir Mahajan <shishir.mahajan@redhat.com>
Signed-off-by: Lei Jitang <leijitang@huawei.com>
liusdu pushed a commit to liusdu/moby that referenced this pull request Oct 30, 2017
CLI flag for docker create(run) to change block device size

CloudRAN need this.
cherry-pick from moby#19367

Signed-off-by: Shishir Mahajan <shishir.mahajan@redhat.com>
Signed-off-by: Lei Jitang <leijitang@huawei.com>


See merge request docker/docker!245
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.

None yet