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

Merged
merged 1 commit into from Mar 29, 2016

Projects

None yet
@shishir-a412ed
Contributor

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

@shishir-a412ed
Contributor

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
Contributor

ping @calavera @rhvgoyal @tiborvass

Can we atleast start the design discussions ?

Shishir

@shishir-a412ed
Contributor

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

Shishir

@anusha-ragunathan
Contributor

Please fix test failures.

@shishir-a412ed
Contributor

@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

@shishir-a412ed
Contributor

@tiborvass @calavera @anusha-ragunathan Any updates on this ?
27 days and running :) No response.

@cpuguy83
Contributor

@shishir-a412ed Not sure if you noticed, but we just did a release, and are working on a patch release.
Hopefully can discuss soon.

@shishir-a412ed
Contributor

@cpuguy83 Thanks.

@thaJeztah
Member

OK, discussed this in the maintainers meeting, moving forward to code review

@tiborvass
Contributor

@shishir-a412ed please update vendoring

@darrenstahlmsft
Contributor

I'm looking to add something similar for Windows, which would involve a lot of duplicate work to what is done here. What is the current status?

@thaJeztah
Member

#20863 contains a vendor update for engine-api, so you can rebase after that is merged

@shishir-a412ed
Contributor

@thaJeztah Thanks !

@runcom
Member
runcom commented Mar 4, 2016

@shishir-a412ed unit tests failing:

18:25:19 # github.com/docker/docker/daemon/graphdriver/aufs
18:25:19 daemon/graphdriver/aufs/aufs_test.go:753: not enough arguments in call to d.Create
18:25:19 daemon/graphdriver/aufs/aufs_test.go:757: not enough arguments in call to d.Create
18:25:19 daemon/graphdriver/aufs/aufs_test.go:773: not enough arguments in call to d.Create
@shishir-a412ed
Contributor

@runcom Fixed.

@runcom runcom and 1 other commented on an outdated diff Mar 4, 2016
daemon/graphdriver/devmapper/deviceset.go
return err
}
+ // Grow the container rootfs.
+ if devinfo.Size > 0 {
+ info, err := devices.lookupDevice(hash)
+ if info == nil {
@runcom
runcom Mar 4, 2016 Member

can you check err here

@shishir-a412ed
shishir-a412ed Mar 4, 2016 Contributor

Fixed.

@runcom runcom and 1 other commented on an outdated diff Mar 4, 2016
daemon/graphdriver/devmapper/deviceset.go
+
+ if err := devices.growFS(info); err != nil {
+ return err
+ }
+ }
+
+ return nil
+}
+
+func (devices *DeviceSet) parseStorageOpt(storageOpt []string, devinfo *devInfo) error {
+
+ // Read size to change the block device size per container.
+ var size int64
+ var err error
+ for _, option := range storageOpt {
+ val := strings.SplitN(option, ":", 2)
@runcom
runcom Mar 4, 2016 Member

can you split w/o the 2? and check len(val) != 2 and error out here? I think you could get something like val[0] = "size" and val[1] = "1G:test" and fails below (didn't check :))

@shishir-a412ed
shishir-a412ed Mar 8, 2016 Contributor

Even in that case (size:1G:test), a split would produce val[0]=size and val[1]=1G:test.
This would eventually fail at units.RAMInBytes(val[1]) and produce the right results (error).

I think this is more human readable and easier to understand. Unless there is a bug in the current code, then I am happy to take a look and fix it. Otherwise I ll let other maintainers chime in.

@runcom runcom and 1 other commented on an outdated diff Mar 4, 2016
integration-cli/docker_cli_create_test.go
@@ -60,6 +60,45 @@ 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, DaemonIsLinux)
+ imageTest := "emptyfs"
+ name := inspectField(c, imageTest, "GraphDriver.Name")
+
+ if name != "devicemapper" {
@runcom
runcom Mar 4, 2016 Member

you can use the Devicemapper test requirement

@shishir-a412ed
shishir-a412ed Mar 4, 2016 Contributor

Fixed.

@runcom runcom and 1 other commented on an outdated diff Mar 4, 2016
integration-cli/docker_cli_create_test.go
@@ -60,6 +60,45 @@ 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, DaemonIsLinux)
+ imageTest := "emptyfs"
+ name := inspectField(c, imageTest, "GraphDriver.Name")
+
+ if name != "devicemapper" {
+ c.Skip("requires devicemapper graphdriver")
+ }
+
+ out, _ := dockerCmd(c, "create", "-P", "--storage-opt", "size:120G", "busybox", "echo")
@runcom
runcom Mar 4, 2016 Member

you don't need -P I suppose

@runcom
runcom Mar 4, 2016 Member

you can also remove echo at the end

@shishir-a412ed
shishir-a412ed Mar 4, 2016 Contributor

Fixed.

@runcom runcom and 1 other commented on an outdated diff Mar 4, 2016
integration-cli/docker_cli_create_test.go
+
+ out, _ := dockerCmd(c, "create", "-P", "--storage-opt", "size:120G", "busybox", "echo")
+
+ cleanedContainerID := strings.TrimSpace(out)
+
+ inspectOut := inspectField(c, cleanedContainerID, "HostConfig.StorageOpt")
+ c.Assert(inspectOut, checker.Equals, "[size:120G]")
+}
+
+// Make sure we cannot shrink the container's rootfs at creation time.
+func (s *DockerSuite) TestCreateShrinkRootfs(c *check.C) {
+ testRequires(c, DaemonIsLinux)
+ imageTest := "emptyfs"
+ name := inspectField(c, imageTest, "GraphDriver.Name")
+
+ if name != "devicemapper" {
@runcom
runcom Mar 4, 2016 Member

as above

@shishir-a412ed
shishir-a412ed Mar 4, 2016 Contributor

Fixed.

@runcom runcom and 1 other commented on an outdated diff Mar 4, 2016
integration-cli/docker_cli_create_test.go
+ inspectOut := inspectField(c, cleanedContainerID, "HostConfig.StorageOpt")
+ c.Assert(inspectOut, checker.Equals, "[size:120G]")
+}
+
+// Make sure we cannot shrink the container's rootfs at creation time.
+func (s *DockerSuite) TestCreateShrinkRootfs(c *check.C) {
+ testRequires(c, DaemonIsLinux)
+ imageTest := "emptyfs"
+ name := inspectField(c, imageTest, "GraphDriver.Name")
+
+ if name != "devicemapper" {
+ c.Skip("requires devicemapper graphdriver")
+ }
+
+ // Ensure this fails
+ out, exit, _ := dockerCmdWithError("create", "-P", "--storage-opt", "size:80G", "busybox", "echo")
@runcom
runcom Mar 4, 2016 Member

drop exit, and use err and out as:

c.Assert(err, check.NotNil, check.Commentf(out))
c.Assert(out, checker.Contains, "your check string...")

also drop -P and echo

@shishir-a412ed
shishir-a412ed Mar 4, 2016 Contributor

Fixed.

@runcom runcom and 1 other commented on an outdated diff Mar 6, 2016
daemon/graphdriver/devmapper/deviceset.go
return err
}
+ // Grow the container rootfs.
+ if devinfo.Size > 0 {
+ info, err := devices.lookupDevice(hash)
+ if err != nil {
+ return err
+ }
+ if info == nil {
@runcom
runcom Mar 6, 2016 Member

no need to check for info == nil since it's already done in lookupDevice and at this point info is != nil

@shishir-a412ed
shishir-a412ed Mar 8, 2016 Contributor

Fixed.

@runcom runcom and 1 other commented on an outdated diff Mar 6, 2016
daemon/graphdriver/devmapper/deviceset.go
+
+ // Read size to change the block device size per container.
+ var size int64
+ var err error
+ for _, option := range storageOpt {
+ val := strings.SplitN(option, ":", 2)
+ key := strings.ToLower(val[0])
+ switch key {
+ case "size":
+ size, err = units.RAMInBytes(val[1])
+ if err != nil {
+ return err
+ }
+ devinfo.Size = uint64(size)
+ default:
+ return fmt.Errorf("Unknown option %s\n", key)
@runcom
runcom Mar 6, 2016 Member

no need for \n

@shishir-a412ed
shishir-a412ed Mar 8, 2016 Contributor

Fixed.

@runcom runcom and 1 other commented on an outdated diff Mar 8, 2016
daemon/graphdriver/devmapper/deviceset.go
+ return err
+ }
+
+ if err := devices.growFS(info); err != nil {
+ return err
+ }
+ }
+
+ return nil
+}
+
+func (devices *DeviceSet) parseStorageOpt(storageOpt []string, devinfo *devInfo) error {
+
+ // Read size to change the block device size per container.
+ var size int64
+ var err error
@runcom
runcom Mar 8, 2016 Member

last thing, can you remove those from here and just use := below?

@shishir-a412ed
shishir-a412ed Mar 8, 2016 Contributor

Fixed.

@runcom
Member
runcom commented Mar 8, 2016

one small nit, LGTM otherwise

ping @calavera @tiborvass

@calavera
Contributor
calavera commented Mar 9, 2016

LGTM

@calavera
Contributor
calavera commented Mar 9, 2016

moved to docs review.

@thaJeztah
Member

@shishir-a412ed, had a quick look at the docs and;

It looks like you didn't update the online documentation for create and run;

Because this also adds a new option to the runconfig, so the API docs should be
updated as well to mention that option;

@shishir-a412ed
Contributor

@thaJeztah PTAL.

@thaJeztah thaJeztah and 1 other commented on an outdated diff Mar 11, 2016
docs/reference/api/docker_remote_api_v1.23.md
@@ -440,6 +441,7 @@ Json Parameters:
`Ulimits: { "Name": "nofile", "Soft": 1024, "Hard": 2048 }`
- **SecurityOpt**: A list of string values to customize labels for MLS
systems, such as SELinux.
+ - **StorageOpt**: Storage driver options per container
@thaJeztah
thaJeztah Mar 11, 2016 Member

I realize some other options are missing this information, but can we add a simple description of the format in which these options should be passed (similar to "LogConfig" below), I.e. Is it ["foo=bar", "bar=baz"], ["foo:bar", "bar:baz"], or [{"foo":"bar"}]?

@thaJeztah thaJeztah and 1 other commented on an outdated diff Mar 11, 2016
docs/reference/api/docker_remote_api.md
@@ -125,6 +125,7 @@ This section lists each version from latest to oldest. Each listing includes a
* `GET /info` now returns `KernelMemory` field, showing if "kernel memory limit" is supported.
* `POST /containers/create` now takes `PidsLimit` field, if the kernel is >= 4.3 and the pids cgroup is supported.
* `GET /containers/(id or name)/stats` now returns `pids_stats`, if the kernel is >= 4.3 and the pids cgroup is supported.
+* `POST /containers/create` now takes Storage Options `--storage-opt`. e.g `--storage-opt size:120G` can set the container rootfs size to 120G at creation time.
@thaJeztah
thaJeztah Mar 11, 2016 Member

Oh, sorry, 😢 this is describing the API, so I don't think we need to / should mention the --storage-opt flag here; only the StorageOpt

@thaJeztah thaJeztah and 1 other commented on an outdated diff Mar 11, 2016
docs/reference/commandline/create.md
@@ -81,6 +81,7 @@ Creates a new container.
--security-opt=[] Security options
--stop-signal="SIGTERM" Signal to stop a container
--shm-size=[] Size of `/dev/shm`. The format is `<number><unit>`. `number` must be greater than `0`. Unit is optional and can be `b` (bytes), `k` (kilobytes), `m` (megabytes), or `g` (gigabytes). If you omit the unit, the system uses bytes. If you omit the size entirely, the system uses `64m`.
+ --storage-opt=[] Set storage driver options per container
@thaJeztah
thaJeztah Mar 11, 2016 Member

Looks like there's mixed tabs / spaces here

@thaJeztah
thaJeztah Mar 11, 2016 Member

Also can you add the description from the man page to this document?

@shishir-a412ed
shishir-a412ed Mar 13, 2016 Contributor

Fixed. I added the description in the Examples section below. Let me know if you want me to put it somewhere else.

@thaJeztah thaJeztah commented on the diff Mar 11, 2016
docs/reference/commandline/run.md
@@ -83,6 +83,7 @@ parent = "smn_cli"
--security-opt=[] Security Options
--sig-proxy=true Proxy received signals to the process
--stop-signal="SIGTERM" Signal to stop a container
+ --storage-opt=[] Set storage driver options per container
@thaJeztah
thaJeztah Mar 11, 2016 Member

Same for this file (description) - can you add the longer description somewhere (possibly in a new section below)

@shishir-a412ed
shishir-a412ed Mar 13, 2016 Contributor

Fixed. I added the description in the Examples section below. Let me know if you want me to put it somewhere else.

@thaJeztah
Member

Thanks @shishir-a412ed, I left some comments

@shishir-a412ed
Contributor

@thaJeztah PTAL.

@thaJeztah thaJeztah and 1 other commented on an outdated diff Mar 13, 2016
docs/reference/api/docker_remote_api_v1.23.md
@@ -440,6 +441,8 @@ Json Parameters:
`Ulimits: { "Name": "nofile", "Soft": 1024, "Hard": 2048 }`
- **SecurityOpt**: A list of string values to customize labels for MLS
systems, such as SELinux.
+ - **StorageOpt**: Storage driver options per container. Options can be passed in the form
+ `[size:120G]`
@thaJeztah
thaJeztah Mar 13, 2016 Member

I think this needs quotes, or am I wrong? I.e., ["size:120G"]

@runcom
runcom Mar 13, 2016 Member

Isn't it ["size":"120G"]? (with 4 double quotes)

@thaJeztah
thaJeztah Mar 13, 2016 Member

No, that would be if it was a struct ({"key":"value"}), this is an array (of strings)

@thaJeztah
Member

Thanks @shishir-a412ed reading this, I have a few concerns about consistency with other options that I would like to have input on from other maintainers before we merge; I'll briefly put it back to design review so that nobody will merge prematurely (perhaps were inconsistent already in other places, but I'd like to be sure)

  • Do we want to use : as a separator for name/value? e.g. --log-opt uses =
  • Do we want this passed as an array of strings in the API, or as a struct ({"size":"120G"})
  • Since this option currently is devicemapper only, do we want it prefixed with dm. (so dm.size) or do we anticipate this becoming an option for other storage drivers as well?

ping @docker/core-engine-maintainers

@vdemeester
Member
  • I don't know if we have some guidelines on that (but we probably should), but I'm definitely for using = as separator because I feel this is the most used in current cli.
  • I also like the idea of dm.… option 😉
@darrenstahlmsft
Contributor

do we anticipate this becoming an option for other storage drivers as well?

Windows intends to offer the size option as well.

@thaJeztah
Member

Windows intends to offer the size option as well.

Alright, in that case, having it without prefix may make sense.

@thaJeztah
Member

ping @shishir-a412ed I just discussed this with some maintainers, and we think that

  • the command-line should take =, so --storage-opt size=123G
  • the API should take a struct instead of a list of strings, so "StorageOpt": { "size":"123G" }

ping @calavera for the final check ^^

@calavera
Contributor

the command-line should take =, so --storage-opt size=123G
the API should take a struct instead of a list of strings, so "StorageOpt": { "size":"123G" }

I agree with those two points. We should be consistent in the CLI and use the X=Y format we use everywhere, and the api should use a map, like we use in other cases like log and labels configuration:

https://github.com/docker/engine-api/blob/master/types/container/host_config.go#L207
https://github.com/docker/engine-api/blob/master/types/container/config.go#L36

@shishir-a412ed
Contributor

@calavera @thaJeztah
PTAL.

Shishir

@thaJeztah thaJeztah and 1 other commented on an outdated diff Mar 16, 2016
runconfig/opts/parse.go
@@ -528,6 +533,16 @@ func parseSecurityOpts(securityOpts []string) ([]string, error) {
return securityOpts, nil
}
+// parse storage options per container into a map
+func parseStorageOpts(storageOpts []string) map[string]string {
+ m := make(map[string]string)
+ for _, option := range storageOpts {
+ opt := strings.SplitN(option, "=", 2)
+ m[opt[0]] = opt[1]
@thaJeztah
thaJeztah Mar 16, 2016 Member

I think this will panic if option is, e.g. "foobar" (no "=")

@shishir-a412ed
shishir-a412ed Mar 17, 2016 Contributor

@thaJeztah Nice catch :). Let me fix it.

@thaJeztah
Member

Build is failing;

18:53:14 # github.com/docker/docker/runconfig/opts
18:53:14 runconfig/opts/parse.go:423: cannot use storageOpts (type map[string]string) as type []string in field value
18:54:05 Build step 'Execute shell' marked build as failure
@shishir-a412ed
Contributor

@thaJeztah once engine-api change is vendor in, jenky should be green.

@vdemeester
Member
07:21:54 ----------------------------------------------------------------------
07:21:54 FAIL: docker_cli_top_test.go:10: DockerSuite.TestTopMultipleArgs
07:21:54 
07:21:54 docker_cli_top_test.go:12:
07:21:54     out, _ := dockerCmd(c, "run", "-i", "-d", "busybox", "top")
07:21:54 /go/src/github.com/docker/docker/pkg/integration/dockerCmd_utils.go:42:
07:21:54     c.Assert(err, check.IsNil, check.Commentf(quote+"%v"+quote+" failed with errors: %s, %v", strings.Join(args, " "), out, err))
07:21:54 ... value *exec.ExitError = &exec.ExitError{ProcessState:(*os.ProcessState)(0xc82068f420), Stderr:[]uint8(nil)} ("exit status 125")
07:21:54 ... "run -i -d busybox top" failed with errors: /go/src/github.com/docker/docker/bundles/1.11.0-dev/test-integration-cli/../binary/docker: Error response from daemon: --storage-opt is not supported for vfs.
07:21:54 See '/go/src/github.com/docker/docker/bundles/1.11.0-dev/test-integration-cli/../binary/docker run --help'.
07:21:54 , exit status 125
07:21:54 

Something's wrong with integration-cli and this change 😓

@shishir-a412ed
Contributor

@vdemeester Looking into this.

@calavera
Contributor

LGTM

@thaJeztah
Member

looks like this needs rebase @shishir-a412ed 😢

@shishir-a412ed
Contributor

@thaJeztah I have rebased the branch. These errors doesn't seem to be related to this PR. WDYT ?

@thaJeztah thaJeztah and 1 other commented on an outdated diff Mar 21, 2016
docs/reference/api/docker_remote_api_v1.23.md
@@ -325,6 +325,7 @@ Create a container
"Ulimits": [{}],
"LogConfig": { "Type": "json-file", "Config": {} },
"SecurityOpt": [""],
+ "StorageOpt": [""],
@thaJeztah
thaJeztah Mar 21, 2016 Member

should this be {} ?

@thaJeztah thaJeztah and 1 other commented on an outdated diff Mar 21, 2016
docs/reference/api/docker_remote_api_v1.23.md
@@ -444,6 +445,8 @@ Json Parameters:
`Ulimits: { "Name": "nofile", "Soft": 1024, "Hard": 2048 }`
- **SecurityOpt**: A list of string values to customize labels for MLS
systems, such as SELinux.
+ - **StorageOpt**: Storage driver options per container. Options can be passed in the form
+ `["size":"120G"]`
@thaJeztah
thaJeztah Mar 21, 2016 Member

Think this should be {"size":"120G"} ?

@thaJeztah thaJeztah and 1 other commented on an outdated diff Mar 21, 2016
man/docker-create.1.md
@@ -325,6 +326,11 @@ unit, `b` is used. Set LIMIT to `-1` to enable unlimited swap.
"seccomp:unconfined" : Turn off seccomp confinement for the container
"seccomp:profile.json : White listed syscalls seccomp Json file to be used as a seccomp filter
+**--storage-opt**=[]
+ Storage driver options per container
+ $ docker create -it --storage-opt size=120G fedora /bin/bash
@thaJeztah
thaJeztah Mar 21, 2016 Member

nit: can you add a blank line before and after the example?

@thaJeztah thaJeztah and 1 other commented on an outdated diff Mar 21, 2016
man/docker-run.1.md
@@ -476,6 +477,11 @@ its root filesystem mounted as read only prohibiting any writes.
"apparmor=unconfined" : Turn off apparmor confinement for the container
"apparmor=your-profile" : Set the apparmor confinement profile for the container
+**--storage-opt**=[]
+ Storage driver options per container
+ $ docker run -it --storage-opt size=120G fedora /bin/bash
@thaJeztah
Member

ping @runcom @vdemeester can you have another look at the code changes (and docs)?

@runcom runcom commented on the diff Mar 21, 2016
runconfig/opts/parse.go
@@ -521,6 +529,20 @@ func parseSecurityOpts(securityOpts []string) ([]string, error) {
return securityOpts, nil
}
+// parse storage options per container into a map
+func parseStorageOpts(storageOpts []string) (map[string]string, error) {
+ m := make(map[string]string)
+ for _, option := range storageOpts {
+ if strings.Contains(option, "=") {
@runcom
runcom Mar 21, 2016 Member

@thaJeztah just because I want to be sure, opts from now on should only contains equal sign (for new opts) otherwise for the old opts we accept both equal and column separator right?

@thaJeztah
thaJeztah Mar 21, 2016 Member

@runcom correct. having both : and = was something we overlooked in the past, so we only fall back for "old" / "existing" options, but new options should only accept =

@runcom
Member
runcom commented Mar 21, 2016

just a question, code LGTM otherwise

@thaJeztah thaJeztah and 1 other commented on an outdated diff Mar 23, 2016
docs/reference/api/docker_remote_api_v1.23.md
@@ -325,6 +325,7 @@ Create a container
"Ulimits": [{}],
"LogConfig": { "Type": "json-file", "Config": {} },
"SecurityOpt": [""],
+ "StorageOpt": [{}],
@thaJeztah
thaJeztah Mar 23, 2016 Member

The [ and ] shouldn't be there, as it isn't an array, just a map?

@thaJeztah
Member

thanks! docs LGTM

@thaJeztah
Member

oh boy, looks like it didn't make the cut-off for the 1.23 API / 1.11, so unfortunately the API docs changes need to be moved to API 1.24 now 😢

@shishir-a412ed
Contributor

@thaJeztah
I don't see any docker_remote_api_v1.24.md file in docs/reference/api.
Is there any other PR (waiting to be merged) that would include this file, so that I can update it ?

@thaJeztah
Member

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

@shishir-a412ed
Contributor

@thaJeztah Please check now.

@thaJeztah
Member

Thanks @shishir-a412ed, LGTM

@thaJeztah
Member

janky hitting #21408 (not related)

@shishir-a412ed shishir-a412ed CLI flag for docker create(run) to change block device size.
Signed-off-by: Shishir Mahajan <shishir.mahajan@redhat.com>
b16decf
@shishir-a412ed
Contributor

@thaJeztah any updates on this one ?

@thaJeztah
Member

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

ping @vdemeester @MHBauer ptal

@vdemeester
Member

LGTM 🐮

@vdemeester vdemeester merged commit e6aa40a into docker:master Mar 29, 2016

8 checks passed

docker/dco-signed All commits signed
Details
documentation success
Details
experimental Jenkins build Docker-PRs-experimental 17066 has succeeded
Details
gccgo Jenkins build Docker-PRs-gccgo 3907 has succeeded
Details
janky Jenkins build Docker-PRs 25889 has succeeded
Details
userns Jenkins build Docker-PRs-userns 8113 has succeeded
Details
win2lin Jenkins build Docker-PRs-Win2Lin 24229 has succeeded
Details
windowsTP4 Jenkins build Docker-PRs-WoW-TP4 3796 has succeeded
Details
@mYmNeo mYmNeo commented on the diff Mar 29, 2016
integration-cli/docker_cli_create_test.go
@@ -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)
@mYmNeo
mYmNeo Mar 29, 2016 Contributor

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

@thaJeztah thaJeztah added this to the 1.12.0 milestone Mar 29, 2016
@shishir-a412ed shishir-a412ed added a commit to shishir-a412ed/docker that referenced this pull request May 5, 2016
@shishir-a412ed shishir-a412ed BACKPORT: PR #19367: CLI flag for docker create(run) to change block …
…device size

Signed-off-by: Shishir Mahajan <shishir.mahajan@redhat.com>
c7489ff
@tiborvass tiborvass commented on the diff Jun 17, 2016
docs/reference/commandline/create.md
@@ -145,6 +146,13 @@ then be used from the subsequent container:
drwx--S--- 2 1000 staff 460 Dec 5 00:51 .ssh
drwxr-xr-x 32 1000 staff 1140 Dec 5 04:01 docker
+Set storage driver options per container.
+
+ $ 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.
@tiborvass
tiborvass Jun 17, 2016 edited Contributor

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

@shishir-a412ed
shishir-a412ed Jun 17, 2016 Contributor

@tiborvass what are you looking for ?

@tiborvass
tiborvass Jun 17, 2016 Contributor

do all storage drivers have a size option ?

@shishir-a412ed
shishir-a412ed Jun 17, 2016 Contributor

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

@darrenstahlmsft
darrenstahlmsft Jun 20, 2016 Contributor

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.

@DrewRay
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
Contributor

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment