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

Generic /cmd endpoint and dm specific operations (resize/resize-pool/trim-pool) #4202

Closed
wants to merge 6 commits into from

Conversation

alexlarsson
Copy link
Contributor

This series adds a generic /cmd endpoint and cli support for it, so that drivers and other form of plugins can add arbitrary new operations, which are accessible as "docker : args..." .

Additionally it adds devicemapper specific operations dm:resize-pool, dm:resize and dm:trim-pool. These allow resizeing the devicemapper pool, indivdual devicemapper devices (like images and containers) and finally allows you to return unused space in the pool to the host root filesystem (when using loopback).

The last one is interesting because it lets you get space back if the discard trick we're using on delete doesn't work (older kernel) or is disabled (because its very slow).

args = append(args, strings.Split(argsString, " ")...)
}

job := eng.Job("driver", args...)
Copy link
Contributor

Choose a reason for hiding this comment

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

I would prefer this way:
job := eng.Job("driver", vars["operation"], strings.Split(r.Form.Get("args"), " ")...)
if you don't mind, I think it's easier to read

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@vieux I had it like that at first, but it doesn't handle the case where there are zero args well. It splits "" into [""].

@vieux
Copy link
Contributor

vieux commented Feb 19, 2014

@alexlarsson , the operation is called resize but it's never possible to shrink right ? Should we try to find another name ?

@@ -202,8 +202,8 @@ func FindLoopDeviceFor(file *osFile) *osFile {
if err != nil {
return nil
}
targetInode := stat.Sys().(*sysStatT).Ino
targetDevice := stat.Sys().(*sysStatT).Dev
targetInode := stat.Sys().(*syscall.Stat_t).Ino
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this retated to the resize or it fixes another bug ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, its used in the codepath for the resize, and its broken.
stat is returned from osFile.Stat() which calls *os.File.Stat(), which returns a syscall.Stat_t, not a sysStatT, so the casts broke.

@alexlarsson
Copy link
Contributor Author

New series moves "driver" to the right order in the usage output.

@vieux "grow" maybe? Not super great either.

@alexlarsson
Copy link
Contributor Author

This last rebase also adds the "trim-pool" command that allows you to regain space on the host filesystem in case you have an older kernel as per #3182 (comment)

@vieux
Copy link
Contributor

vieux commented Feb 20, 2014

docker driver trim-pool hangs on my side, if it doesn't work on old kernels I'm not sure we should include this. /cc @creack @crosbymichael

@crosbymichael
Copy link
Contributor

@vieux what is your kernel version ? I have 3.12

@vieux
Copy link
Contributor

vieux commented Feb 20, 2014

3.11.0-15-generic

On Thu, Feb 20, 2014 at 11:08 AM, Michael Crosby
notifications@github.comwrote:

@vieux https://github.com/vieux what is your kernel version ? I have
3.12

Reply to this email directly or view it on GitHubhttps://github.com//pull/4202#issuecomment-35657127
.

Victor VIEUX
http://vvieux.com

@alexlarsson
Copy link
Contributor Author

@vieux Are you sure it hangs? It can take quite some time.
Also, do you have /usr/sbin/thin_dump on your machine?

@vieux
Copy link
Contributor

vieux commented Feb 20, 2014

No I don't have /usr/sbin/thin_dump

@alexlarsson
Copy link
Contributor Author

trim-pool doesn't really use any paricular kernel features.
It just uses thin_dump to parse the metadata partition, extract the list of unused blocks on the device and discard them.

@alexlarsson
Copy link
Contributor Author

@vieux Ah, i guess we should check that :)

@alexlarsson
Copy link
Contributor Author

@vieux I tried with thin_dump not installed and got:

# docker driver trim-pool
2014/02/21 09:55:53 Error: driver: Error trimming pool: exec: "thin_dump": executable file not found in $PATH

Do you not get something like that?

@vieux
Copy link
Contributor

vieux commented Feb 21, 2014

Yes I retried and I get 2014/02/21 18:16:45 Error: driver: Error trimming pool: exec: "thin_dump": executable file not found in $PATH

But I'm certain that the first time it freezed.

could you pleas try @crosbymichael

@unclejack
Copy link
Contributor

@alexlarsson Do you think it'd be a good idea to allow users to customize the size of the default base fs?

@alexlarsson
Copy link
Contributor Author

@unclejack That seems like a nice idea. It'd have to be set the first time the daemon runs though.

@alexlarsson
Copy link
Contributor Author

So, what is the opinion on this?

@unclejack
Copy link
Contributor

@alexlarsson This is useful and it's going to be needed sooner or later. Being able to set the initial base fs size would be useful as well. Couldn't it be changed, provided that there are no containers and no images?

I'll test this PR on Fedora as soon as possible.

@alexlarsson
Copy link
Contributor Author

@unclejack Well, the base image can be re-created at any time i guess. The only problem is that existing images and containers will not pick that up. That may not be a problem though,

@alexlarsson
Copy link
Contributor Author

Rebased on latest master

@alexlarsson
Copy link
Contributor Author

rebased to latest master

@lcarstensen
Copy link

I've tested this latest rebase on RHEL6.5 and verified "device trim-pool" operates correctly/as-advertised. As there is no other way offered presently to trim the devicemapper data file, I'd appreciate upstream's consideration in merging this.

For my own purposes I've backported to 0.9.1: https://github.com/lcarstensen/docker/tree/dm-operations-v0.9.1

lcarstensen pushed a commit to lcarstensen/docker that referenced this pull request Apr 2, 2014
@crosbymichael crosbymichael self-assigned this Apr 9, 2014
@lcarstensen
Copy link

@shykes @crosbymichael I've heard this may be waiting on an introduction of plugins to api/client/* - may I request that you confirm that and cross reference a branch / issue / label / milestone / somesuch here?

FWIW I've been using this PR for several weeks now in a moderate environment - just tracking the merge so I don't have to keep carrying the patch on each release for that environment.

@crosbymichael
Copy link
Contributor

Maybe we can merge the code into the devicemapper package first. I'm not sure about the top level command and if the code was available to sys admins then we could have a small contrib tool.

@shykes
Copy link
Contributor

shykes commented Apr 19, 2014

@alexlarsson I'm +1 on the feature, but this will be the first implementation of driver-specific operations, so I want to set the right precedent.

What do you think of this syntax (inspired by the heroku and dokku command-line)

docker devicemapper:resize f80c3b95e 20G
docker devicemapper:resize-pool 200G

Implementation changes:

Instead of a new /driver route in the remote API, this could be sent on a more generic /cmd route, which would simply execute a job. The /cmd route wouldn't be specific to driver operations: we can use it as a catch-all for all commands which are not known to map to another route. The nice thing about this is that we can gradually move more and more commands to this /cmd route, basically making it a generic http1 beam transport.

On the server side, we just need to make sure that there is a job handler registered on the engine for "devicemapper:resize" and "devicemapper:resize-pool". This can be done in builtins/builtins.go, with any sort of "plugin-y" scaffolding that we want.

In fact we could use this as a simple scaffolding for plugins.

@alexlarsson
Copy link
Contributor Author

I rebased this series to latest. I'll keep this pull open for now to keep the code, then i'll make a separate pull request with only the /cmd part. Then when that is in i can rebase the devicemapper specific ops.

@alexlarsson
Copy link
Contributor Author

New rebased set. Note, this includes the changes from #5380, so that should probably be pulled first.

@alexlarsson
Copy link
Contributor Author

rebased on latest master, still containes the changes from #5380, so that should probably be pulled first.

@shykes shykes added this to the 1.0 milestone Jun 4, 2014
@crosbymichael crosbymichael modified the milestones: 1.1, 1.0 Jun 5, 2014
This allows registering handlers after you've started using the engine,
as well as setting hack envs.

Docker-DCO-1.1-Signed-off-by: Alexander Larsson <alexl@redhat.com> (github: alexlarsson)
If you do
  docker devmapper:resize -o Key=Value -o Foo=Bar plugin:op arg1 arg2

This will do a GET http request to /cmd/devmapper/resize with the
arguments and options as keys. This will then try to spawn a job on
the engine called "devmapper:resize" with the arg1, arg2, etc as
arguments and the -o options as environment.

Also, any graph driver implementing the engine.Installer interface
will get installed on the engine when the daemon is created.

This will be used by e.g. the devicemapper to allow driver-specific
operations like pool resizing.

Docker-DCO-1.1-Signed-off-by: Alexander Larsson <alexl@redhat.com> (github: alexlarsson)
This lets you live-resize the devicemapper loopback files for the pool.
Use it like this:
 docker dm:resize-pool 140G

Also, it changes the way pool sizes are reported to use the units.HumanSize
call which uses base 1000, not 1024, to match the FromHumanSize call.

Docker-DCO-1.1-Signed-off-by: Alexander Larsson <alexl@redhat.com> (github: alexlarsson)
This lets you resize individual images or containers like:

docker dm:resize f4eff8c9f36d3114c3a16fc7b213512153a2d25caef6f11f385020e6558600f1 12G

Docker-DCO-1.1-Signed-off-by: Alexander Larsson <alexl@redhat.com> (github: alexlarsson)
This adds a BlockDeviceDiscardAll which discards everything on the device,
and makes BlockDeviceDiscard take offset and length arguments.

Docker-DCO-1.1-Signed-off-by: Alexander Larsson <alexl@redhat.com> (github: alexlarsson)
This command suspends the pool, extracts all metadata from the
metadata pool and then manually discards all regions not in use on the
data device. This will re-sparsify the underlying loopback file and
regain space on the host operating system.

This is required in some cases because the discards we do when
deleting images and containers isn't enought to fully free all space
unless you have a very new kernel.  See:
moby#3182 (comment)

Docker-DCO-1.1-Signed-off-by: Alexander Larsson <alexl@redhat.com> (github: alexlarsson)

Conflicts:
	daemon/graphdriver/devmapper/driver.go
@alexlarsson alexlarsson changed the title devicemapper resize and resize-pool operations Generic /cmd endpoint and dm specific operations (resize/resize-pool/trim-pool) Jun 9, 2014
@vbatts vbatts added the Runtime label Jul 1, 2014
@crosbymichael crosbymichael removed their assignment Jul 18, 2014
@rhatdan
Copy link
Contributor

rhatdan commented Sep 4, 2014

What is the current state of this pull request?

@tiborvass
Copy link
Contributor

@vieux @crosbymichael are we still good on this one?

@crosbymichael
Copy link
Contributor

I would say that we can close this for now because it's not the direction being taken with the API.

@alexlarsson
Copy link
Contributor Author

@crosbymichael This api is what @shykes proposed at #4202 (comment), if some other API is a better approach can you document that so we can fix this, because the feature is quite useful.

@crosbymichael crosbymichael self-assigned this Oct 17, 2014
@crosbymichael
Copy link
Contributor

We think the design and implementation from the earlier discussion do not apply today. I'm sure this is currently handled by an external tool and should probably be based on new API discussions.

vbatts added a commit to vbatts/moby that referenced this pull request Jan 7, 2015
Largely bringing in the Trim and resize logic by @alexlarsson and
moby#4202

Signed-off-by: Vincent Batts <vbatts@redhat.com>
@ashish235
Copy link

None of the commands are working and is known by the docker daemon. Please can someone suggest the method to use it? I need to increase the size of my containers to 50 GB.

@rhatdan
Copy link
Contributor

rhatdan commented Jul 9, 2015

@rhvgoyal ^^

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants