containers / docker-lvm-plugin

Docker volume plugin for LVM volumes
GNU Lesser General Public License v3.0
155 stars 64 forks source link

Add optional argument to select volume group #52

Closed echa closed 5 years ago

echa commented 6 years ago

This PR adds a new option vg that allows to specify a volume group different from the default defined in /etc/docker/docker-lvm-plugin at a per-volume basis. The volume group is also stored in /var/lib/docker-lvm-plugin/lvmVolumesConfig.json to have it available for volume mount and remove operations.

Usage

docker volume create -d lvm --opt size=100G --opt vg=vg1 --name=data

This PR should also resolve #7 since it provides a convenient way to specify a volume group on docker command line, in docker-compose and other docker config files (swarm, etc).

Tested and works with Docker 1.13.1 on CentOS 7.4.

# docker info
Containers: 0
 Running: 0
 Paused: 0
 Stopped: 0
Images: 0
Server Version: 1.13.1
Storage Driver: devicemapper
 Pool Name: vg0-docker--pool
 Pool Blocksize: 524.3 kB
 Base Device Size: 4.295 GB
 Backing Filesystem: xfs
 Data file:
 Metadata file:
 Data Space Used: 15.73 MB
 Data Space Total: 21.47 GB
 Data Space Available: 21.46 GB
 Metadata Space Used: 106.5 kB
 Metadata Space Total: 960.5 MB
 Metadata Space Available: 960.4 MB
 Thin Pool Minimum Free Space: 2.147 GB
 Udev Sync Supported: true
 Deferred Removal Enabled: true
 Deferred Deletion Enabled: true
 Deferred Deleted Device Count: 0
 Library Version: 1.02.146-RHEL7 (2018-01-22)
Logging Driver: gelf
Cgroup Driver: systemd
Plugins:
 Volume: local lvm
 Network: bridge host macvlan null overlay
Swarm: inactive
Runtimes: docker-runc runc
Default Runtime: docker-runc
Init Binary: docker-init
containerd version:  (expected: aa8187dbd3b7ad67d8e5e3a15115d3eef43a7ed1)
runc version: N/A (expected: 9df8b306d01f59d3a8029be411de015b7304dd8f)
init version: N/A (expected: 949e6facb77383876aeff8a6944dde66b3089574)
Security Options:
 seccomp
  Profile: default
Kernel Version: 4.16.8-1.el7.elrepo.x86_64
Operating System: CentOS Linux 7 (Core)
OSType: linux
Architecture: x86_64
Number of Docker Hooks: 3
CPUs: 48
Total Memory: 377.5 GiB
Name: XXX
ID: SC35:4TGP:5KDA:BJXK:LTOA:I7O2:63TC:QIIG:3TN4:LGAZ:7SBH:PMJP
Docker Root Dir: /var/lib/docker
Debug Mode (client): false
Debug Mode (server): false
Registry: https://index.docker.io/v1/
WARNING: bridge-nf-call-ip6tables is disabled
Labels:
Experimental: false
Insecure Registries:
 127.0.0.0/8
Live Restore Enabled: true
Registries: docker.io (secure)
shishir-a412ed commented 6 years ago

@echa Thanks for the PR ! Will take a look soon.

shishir-a412ed commented 6 years ago

Also adding @rhatdan to take another look.

shishir-a412ed commented 6 years ago

@echa Once you fix these review comments, can you also squash your commits into one ? If you are new to squashing, here is a link to help https://github.com/todotxt/todo.txt-android/wiki/Squash-All-Commits-Related-to-a-Single-Issue-into-a-Single-Commit

Also an example in your scenario would be:

1) Right now your git log has 5 commits.

529c479b545e2502dbc3ee123dd0de020d9a8a46 revert err definition
35d87e9efd6f784ffd12ec513e1c05c161228680 err on removing unmanaged volume
11ea6272655cebdcb0edefe5942dea62ba267f8e update docu and manpage
946e3bb30b595b170cc4fe5efa7df4102abee729 make vg selection compatible with legacy lvmVolumesConfig.json
230ab1c60e4a662b0a062548c9879b5febcf2c51 add optional argument to select volume group

2) To squash all your commits into 230ab1c60e4a662b0a062548c9879b5febcf2c51 you can do:

 git rebase -i HEAD~5

3) You will get an interactive editor: Leave the first pick as it is, and replace the rest of the pick with s s = squash.

Save your changes.

4) It will take you to a second interactive screen, where you can set your commit message. Remove everything else except your commit message: Add optional argument to select volume group (Change a to A)

Save your changes.

5) Force push your branch to github.

 git push -u origin master -f
 Here `origin` is your fork.

Thanks for being patient ! I think we are pretty close to merging now. PS: Just FYI, You will have 6 commits, once you fix these review comments.

shishir-a412ed commented 6 years ago

@echa Any updates on this one ?

shishir-a412ed commented 5 years ago

Closing this in favor of #79

/cc @rhatdan @echa