From 9ce8aac55e6df65bbf49c682374871a94d379bf3 Mon Sep 17 00:00:00 2001 From: Yong Tang Date: Sat, 17 Sep 2016 12:32:31 -0700 Subject: [PATCH] Show volume options for `docker volume inspect` This fix tries to address the issue raised in 25545 where volume options at the creation time is not showed up in `docker volume inspect`. This fix adds the field `Options` in `Volume` type and persist the options in volume db so that `volume inspect` could display the options. This fix adds a couple of test cases to cover the changes. This fix fixes 25545. Signed-off-by: Yong Tang --- api/types/types.go | 1 + daemon/list.go | 2 +- daemon/volumes.go | 7 +-- docs/reference/api/docker_remote_api.md | 1 + docs/reference/api/docker_remote_api_v1.25.md | 18 +++++- integration-cli/docker_cli_volume_test.go | 22 ++++++++ volume/store/store.go | 56 +++++++++++++------ volume/volume.go | 10 +--- 8 files changed, 85 insertions(+), 32 deletions(-) diff --git a/api/types/types.go b/api/types/types.go index 7192f5ec3b..64819cb7fe 100644 --- a/api/types/types.go +++ b/api/types/types.go @@ -447,6 +447,7 @@ type Volume struct { Status map[string]interface{} `json:",omitempty"` // Status provides low-level status information about the volume Labels map[string]string // Labels is metadata specific to the volume Scope string // Scope describes the level at which the volume exists (e.g. `global` for cluster-wide or `local` for machine level) + Options map[string]string // Options holds the driver specific options to use for when creating the volume. UsageData *VolumeUsageData `json:",omitempty"` } diff --git a/daemon/list.go b/daemon/list.go index 7f898591cc..3b27102e14 100644 --- a/daemon/list.go +++ b/daemon/list.go @@ -608,7 +608,7 @@ func (daemon *Daemon) filterVolumes(vols []volume.Volume, filter filters.Args) ( } } if filter.Include("label") { - v, ok := vol.(volume.LabeledVolume) + v, ok := vol.(volume.DetailedVolume) if !ok { continue } diff --git a/daemon/volumes.go b/daemon/volumes.go index dd9292f22c..095b212a05 100644 --- a/daemon/volumes.go +++ b/daemon/volumes.go @@ -32,13 +32,12 @@ func volumeToAPIType(v volume.Volume) *types.Volume { Name: v.Name(), Driver: v.DriverName(), } - if v, ok := v.(volume.LabeledVolume); ok { + if v, ok := v.(volume.DetailedVolume); ok { tv.Labels = v.Labels() - } - - if v, ok := v.(volume.ScopedVolume); ok { + tv.Options = v.Options() tv.Scope = v.Scope() } + return tv } diff --git a/docs/reference/api/docker_remote_api.md b/docs/reference/api/docker_remote_api.md index b9fcabbddd..ebda493765 100644 --- a/docs/reference/api/docker_remote_api.md +++ b/docs/reference/api/docker_remote_api.md @@ -132,6 +132,7 @@ This section lists each version from latest to oldest. Each listing includes a * `POST /services/create` and `POST /services/(id or name)/update` now accept `Monitor` and `MaxFailureRatio` parameters, which control the response to failures during service updates. * `GET /networks/(name)` now returns `Created`. * `POST /containers/(id or name)/exec` now accepts an `Env` field, which holds a list of environment variables to be set in the context of the command execution. +* `GET /volumes`, `GET /volumes/(name)`, and `POST /volumes/create` now return the `Options` field which holds the driver specific options to use for when creating the volume. ### v1.24 API changes diff --git a/docs/reference/api/docker_remote_api_v1.25.md b/docs/reference/api/docker_remote_api_v1.25.md index f1a7bf9577..c5a097979b 100644 --- a/docs/reference/api/docker_remote_api_v1.25.md +++ b/docs/reference/api/docker_remote_api_v1.25.md @@ -2661,6 +2661,7 @@ Return docker data usage information "Mountpoint": "", "Labels": null, "Scope": "", + "Options": null "UsageData": { "Size": 0, "RefCount": 0 @@ -3328,7 +3329,12 @@ Return low-level information about the `exec` command `id`. "com.example.some-label": "some-value", "com.example.some-other-label": "some-other-value" }, - "Scope": "local" + "Scope": "local", + "Options": { + "device": "tmpfs", + "o": "size=100m,uid=1000", + "type": "tmpfs" + } } ], "Warnings": [] @@ -3382,7 +3388,8 @@ Create a volume "com.example.some-label": "some-value", "com.example.some-other-label": "some-other-value" }, - "Scope": "local" + "Scope": "local", + "Options": null } **Status codes**: @@ -3429,7 +3436,11 @@ Return low-level information on the volume `name` "com.example.some-label": "some-value", "com.example.some-other-label": "some-other-value" }, - "Scope": "local" + "Scope": "local", + "Options": { + "some-key": "some-value", + "some-other-key": "some-other-value" + }, } **Status codes**: @@ -3454,6 +3465,7 @@ response. - **Labels** - Labels set on the volume, specified as a map: `{"key":"value","key2":"value2"}`. - **Scope** - Scope describes the level at which the volume exists, can be one of `global` for cluster-wide or `local` for machine level. The default is `local`. +- **Options** - Options holds the driver specific options to use for when creating the volume. ### Remove a volume diff --git a/integration-cli/docker_cli_volume_test.go b/integration-cli/docker_cli_volume_test.go index a9c8b61de4..0969ade90d 100644 --- a/integration-cli/docker_cli_volume_test.go +++ b/integration-cli/docker_cli_volume_test.go @@ -1,6 +1,7 @@ package main import ( + "fmt" "io/ioutil" "os" "os/exec" @@ -418,3 +419,24 @@ func (s *DockerSuite) TestVolumeCLIRmForce(c *check.C) { out, _ = dockerCmd(c, "volume", "ls") c.Assert(out, checker.Contains, name) } + +func (s *DockerSuite) TestVolumeCliInspectWithVolumeOpts(c *check.C) { + testRequires(c, DaemonIsLinux) + + // Without options + name := "test1" + dockerCmd(c, "volume", "create", "-d", "local", name) + out, _ := dockerCmd(c, "volume", "inspect", "--format={{ .Options }}", name) + c.Assert(strings.TrimSpace(out), checker.Contains, "map[]") + + // With options + name = "test2" + k1, v1 := "type", "tmpfs" + k2, v2 := "device", "tmpfs" + k3, v3 := "o", "size=1m,uid=1000" + dockerCmd(c, "volume", "create", "-d", "local", name, "--opt", fmt.Sprintf("%s=%s", k1, v1), "--opt", fmt.Sprintf("%s=%s", k2, v2), "--opt", fmt.Sprintf("%s=%s", k3, v3)) + out, _ = dockerCmd(c, "volume", "inspect", "--format={{ .Options }}", name) + c.Assert(strings.TrimSpace(out), checker.Contains, fmt.Sprintf("%s:%s", k1, v1)) + c.Assert(strings.TrimSpace(out), checker.Contains, fmt.Sprintf("%s:%s", k2, v2)) + c.Assert(strings.TrimSpace(out), checker.Contains, fmt.Sprintf("%s:%s", k3, v3)) +} diff --git a/volume/store/store.go b/volume/store/store.go index 0fbf385f39..807ad1495a 100644 --- a/volume/store/store.go +++ b/volume/store/store.go @@ -23,14 +23,24 @@ const ( ) type volumeMetadata struct { - Name string - Labels map[string]string + Name string + Labels map[string]string + Options map[string]string } type volumeWrapper struct { volume.Volume - labels map[string]string - scope string + labels map[string]string + scope string + options map[string]string +} + +func (v volumeWrapper) Options() map[string]string { + options := map[string]string{} + for key, value := range v.options { + options[key] = value + } + return options } func (v volumeWrapper) Labels() map[string]string { @@ -54,10 +64,11 @@ func (v volumeWrapper) CachedPath() string { // reference counting of volumes in the system. func New(rootPath string) (*VolumeStore, error) { vs := &VolumeStore{ - locks: &locker.Locker{}, - names: make(map[string]volume.Volume), - refs: make(map[string][]string), - labels: make(map[string]map[string]string), + locks: &locker.Locker{}, + names: make(map[string]volume.Volume), + refs: make(map[string][]string), + labels: make(map[string]map[string]string), + options: make(map[string]map[string]string), } if rootPath != "" { @@ -113,6 +124,7 @@ func (s *VolumeStore) Purge(name string) { delete(s.names, name) delete(s.refs, name) delete(s.labels, name) + delete(s.options, name) s.globalLock.Unlock() } @@ -127,7 +139,9 @@ type VolumeStore struct { refs map[string][]string // labels stores volume labels for each volume labels map[string]map[string]string - db *bolt.DB + // options stores volume options for each volume + options map[string]map[string]string + db *bolt.DB } // List proxies to all registered volume drivers to get the full list of volumes @@ -186,7 +200,7 @@ func (s *VolumeStore) list() ([]volume.Volume, []string, error) { return } for i, v := range vs { - vs[i] = volumeWrapper{v, s.labels[v.Name()], d.Scope()} + vs[i] = volumeWrapper{v, s.labels[v.Name()], d.Scope(), s.options[v.Name()]} } chVols <- vols{vols: vs} @@ -283,12 +297,14 @@ func (s *VolumeStore) create(name, driverName string, opts, labels map[string]st } s.globalLock.Lock() s.labels[name] = labels + s.options[name] = opts s.globalLock.Unlock() if s.db != nil { metadata := &volumeMetadata{ - Name: name, - Labels: labels, + Name: name, + Labels: labels, + Options: opts, } volData, err := json.Marshal(metadata) @@ -305,7 +321,7 @@ func (s *VolumeStore) create(name, driverName string, opts, labels map[string]st } } - return volumeWrapper{v, labels, vd.Scope()}, nil + return volumeWrapper{v, labels, vd.Scope(), opts}, nil } // GetWithRef gets a volume with the given name from the passed in driver and stores the ref @@ -328,7 +344,7 @@ func (s *VolumeStore) GetWithRef(name, driverName, ref string) (volume.Volume, e s.setNamed(v, ref) - return volumeWrapper{v, s.labels[name], vd.Scope()}, nil + return volumeWrapper{v, s.labels[name], vd.Scope(), s.options[name]}, nil } // Get looks if a volume with the given name exists and returns it if so @@ -350,6 +366,7 @@ func (s *VolumeStore) Get(name string) (volume.Volume, error) { // it is expected that callers of this function hold any necessary locks func (s *VolumeStore) getVolume(name string) (volume.Volume, error) { labels := map[string]string{} + options := map[string]string{} if s.db != nil { // get meta @@ -368,6 +385,7 @@ func (s *VolumeStore) getVolume(name string) (volume.Volume, error) { return err } labels = meta.Labels + options = meta.Options return nil }); err != nil { @@ -388,7 +406,7 @@ func (s *VolumeStore) getVolume(name string) (volume.Volume, error) { if err != nil { return nil, err } - return volumeWrapper{vol, labels, vd.Scope()}, nil + return volumeWrapper{vol, labels, vd.Scope(), options}, nil } logrus.Debugf("Probing all drivers for volume with name: %s", name) @@ -403,7 +421,7 @@ func (s *VolumeStore) getVolume(name string) (volume.Volume, error) { continue } - return volumeWrapper{v, labels, d.Scope()}, nil + return volumeWrapper{v, labels, d.Scope(), options}, nil } return nil, errNoSuchVolume } @@ -478,7 +496,11 @@ func (s *VolumeStore) FilterByDriver(name string) ([]volume.Volume, error) { return nil, &OpErr{Err: err, Name: name, Op: "list"} } for i, v := range ls { - ls[i] = volumeWrapper{v, s.labels[v.Name()], vd.Scope()} + options := map[string]string{} + for key, value := range s.options[v.Name()] { + options[key] = value + } + ls[i] = volumeWrapper{v, s.labels[v.Name()], vd.Scope(), options} } return ls, nil } diff --git a/volume/volume.go b/volume/volume.go index 1c11914ee9..df47b0f1b0 100644 --- a/volume/volume.go +++ b/volume/volume.go @@ -68,14 +68,10 @@ type Volume interface { Status() map[string]interface{} } -// LabeledVolume wraps a Volume with user-defined labels -type LabeledVolume interface { +// DetailedVolume wraps a Volume with user-defined labels, options, and cluster scope (e.g., `local` or `global`) +type DetailedVolume interface { Labels() map[string]string - Volume -} - -// ScopedVolume wraps a volume with a cluster scope (e.g., `local` or `global`) -type ScopedVolume interface { + Options() map[string]string Scope() string Volume }