diff --git a/api/impl/docker/docker_test.go b/api/impl/docker/docker_test.go index b00756dd..0976fcde 100644 --- a/api/impl/docker/docker_test.go +++ b/api/impl/docker/docker_test.go @@ -13,6 +13,7 @@ import ( "github.com/contiv/volplugin/api" "github.com/contiv/volplugin/config" + "github.com/contiv/volplugin/storage" . "gopkg.in/check.v1" ) @@ -70,7 +71,7 @@ func (s *dockerSuite) TestBasic(c *C) { err := s.client.PublishPolicy("policy1", &config.Policy{ Name: "policy1", Backend: "ceph", - DriverOptions: map[string]string{"pool": "rbd"}, + DriverOptions: storage.DriverParams{"pool": "rbd"}, CreateOptions: config.CreateOptions{ Size: "10MB", }, diff --git a/config/policy.go b/config/policy.go index b00f06f9..a2bea81c 100644 --- a/config/policy.go +++ b/config/policy.go @@ -5,6 +5,7 @@ import ( "github.com/contiv/errored" "github.com/contiv/volplugin/errors" + "github.com/contiv/volplugin/storage" "github.com/coreos/etcd/client" "golang.org/x/net/context" ) @@ -18,14 +19,14 @@ var defaultDrivers = map[string]*BackendDrivers{ // Policy is the configuration of the policy. It includes default // information for items such as pool and volume configuration. type Policy struct { - Name string `json:"name"` - Unlocked bool `json:"unlocked,omitempty" merge:"unlocked"` - CreateOptions CreateOptions `json:"create"` - RuntimeOptions RuntimeOptions `json:"runtime"` - DriverOptions map[string]string `json:"driver"` - FileSystems map[string]string `json:"filesystems"` - Backends *BackendDrivers `json:"backends,omitempty"` - Backend string `json:"backend,omitempty"` + Name string `json:"name"` + Unlocked bool `json:"unlocked,omitempty" merge:"unlocked"` + CreateOptions CreateOptions `json:"create"` + RuntimeOptions RuntimeOptions `json:"runtime"` + DriverOptions storage.DriverParams `json:"driver"` + FileSystems map[string]string `json:"filesystems"` + Backends *BackendDrivers `json:"backends,omitempty"` + Backend string `json:"backend,omitempty"` } // BackendDrivers is a struct containing all the drivers used under this policy diff --git a/config/policy_test.go b/config/policy_test.go index f20f488c..d6b63f7b 100644 --- a/config/policy_test.go +++ b/config/policy_test.go @@ -1,6 +1,9 @@ package config -import . "gopkg.in/check.v1" +import ( + "github.com/contiv/volplugin/storage" + . "gopkg.in/check.v1" +) var testPolicies = map[string]*Policy{ "basic": { @@ -10,7 +13,7 @@ var testPolicies = map[string]*Policy{ Mount: "ceph", Snapshot: "ceph", }, - DriverOptions: map[string]string{"pool": "rbd"}, + DriverOptions: storage.DriverParams{"pool": "rbd"}, CreateOptions: CreateOptions{ Size: "10MB", FileSystem: defaultFilesystem, @@ -31,7 +34,7 @@ var testPolicies = map[string]*Policy{ Mount: "ceph", Snapshot: "ceph", }, - DriverOptions: map[string]string{"pool": "rbd"}, + DriverOptions: storage.DriverParams{"pool": "rbd"}, CreateOptions: CreateOptions{ Size: "20MB", FileSystem: defaultFilesystem, @@ -45,7 +48,7 @@ var testPolicies = map[string]*Policy{ Mount: "ceph", Snapshot: "ceph", }, - DriverOptions: map[string]string{"pool": "rbd"}, + DriverOptions: storage.DriverParams{"pool": "rbd"}, CreateOptions: CreateOptions{ Size: "0", FileSystem: defaultFilesystem, @@ -59,7 +62,7 @@ var testPolicies = map[string]*Policy{ Mount: "ceph", Snapshot: "ceph", }, - DriverOptions: map[string]string{"pool": "rbd"}, + DriverOptions: storage.DriverParams{"pool": "rbd"}, CreateOptions: CreateOptions{ Size: "20MB", FileSystem: defaultFilesystem, @@ -73,7 +76,7 @@ var testPolicies = map[string]*Policy{ Mount: "ceph", Snapshot: "ceph", }, - DriverOptions: map[string]string{"pool": "rbd"}, + DriverOptions: storage.DriverParams{"pool": "rbd"}, CreateOptions: CreateOptions{ Size: "not a number", FileSystem: defaultFilesystem, @@ -87,7 +90,7 @@ var testPolicies = map[string]*Policy{ Mount: "ceph", Snapshot: "ceph", }, - DriverOptions: map[string]string{"pool": "rbd"}, + DriverOptions: storage.DriverParams{"pool": "rbd"}, CreateOptions: CreateOptions{ Size: "10MB", FileSystem: defaultFilesystem, @@ -106,7 +109,7 @@ var testPolicies = map[string]*Policy{ Mount: "ceph", }, Name: "blanksize", - DriverOptions: map[string]string{"pool": "rbd"}, + DriverOptions: storage.DriverParams{"pool": "rbd"}, CreateOptions: CreateOptions{ FileSystem: defaultFilesystem, }, @@ -119,7 +122,7 @@ var testPolicies = map[string]*Policy{ Mount: "ceph", }, Name: "blanksize", - DriverOptions: map[string]string{"pool": "rbd"}, + DriverOptions: storage.DriverParams{"pool": "rbd"}, CreateOptions: CreateOptions{ FileSystem: defaultFilesystem, }, @@ -128,7 +131,7 @@ var testPolicies = map[string]*Policy{ }, "nobackend": { Name: "nobackend", - DriverOptions: map[string]string{"pool": "rbd"}, + DriverOptions: storage.DriverParams{"pool": "rbd"}, CreateOptions: CreateOptions{ Size: "10MB", FileSystem: defaultFilesystem, diff --git a/config/validation_test.go b/config/validation_test.go index 3aa2bbaa..04d4b96e 100644 --- a/config/validation_test.go +++ b/config/validation_test.go @@ -1,13 +1,16 @@ package config -import . "gopkg.in/check.v1" +import ( + "github.com/contiv/volplugin/storage" + . "gopkg.in/check.v1" +) var ( defaultBackends = &BackendDrivers{CRUD: "ceph", Mount: "ceph", Snapshot: "ceph"} VolumeConfigs = map[string]map[string]*Volume{ "valid": { "basic": { - DriverOptions: map[string]string{"pool": "rbd"}, + DriverOptions: storage.DriverParams{"pool": "rbd"}, CreateOptions: CreateOptions{Size: "10MB"}, RuntimeOptions: RuntimeOptions{UseSnapshots: false}, VolumeName: "basicvolume", @@ -15,7 +18,7 @@ var ( Backends: defaultBackends, }, "basicwithruntime": { - DriverOptions: map[string]string{"pool": "rbd"}, + DriverOptions: storage.DriverParams{"pool": "rbd"}, CreateOptions: CreateOptions{Size: "10MB"}, RuntimeOptions: RuntimeOptions{UseSnapshots: true, Snapshot: SnapshotConfig{Frequency: "10m", Keep: 10}}, VolumeName: "basicvolume", diff --git a/config/volume.go b/config/volume.go index 1193a20d..77e480c4 100644 --- a/config/volume.go +++ b/config/volume.go @@ -22,14 +22,14 @@ import ( // Volume is the configuration of the policy. It includes pool and // snapshot information. type Volume struct { - PolicyName string `json:"policy"` - VolumeName string `json:"name"` - Unlocked bool `json:"unlocked,omitempty" merge:"unlocked"` - DriverOptions map[string]string `json:"driver"` - MountSource string `json:"mount" merge:"mount"` - CreateOptions CreateOptions `json:"create"` - RuntimeOptions RuntimeOptions `json:"runtime"` - Backends *BackendDrivers `json:"backends,omitempty"` + PolicyName string `json:"policy"` + VolumeName string `json:"name"` + Unlocked bool `json:"unlocked,omitempty" merge:"unlocked"` + DriverOptions storage.DriverParams `json:"driver"` + MountSource string `json:"mount" merge:"mount"` + CreateOptions CreateOptions `json:"create"` + RuntimeOptions RuntimeOptions `json:"runtime"` + Backends *BackendDrivers `json:"backends,omitempty"` } // CreateOptions are the set of options used by apiserver during the volume @@ -102,7 +102,7 @@ func (c *Client) CreateVolume(rc *VolumeRequest) (*Volume, error) { } if resp.DriverOptions == nil { - resp.DriverOptions = map[string]string{} + resp.DriverOptions = storage.DriverParams{} } if err := resp.Validate(); err != nil { diff --git a/config/volume_test.go b/config/volume_test.go index 29fc8bc0..34580437 100644 --- a/config/volume_test.go +++ b/config/volume_test.go @@ -45,7 +45,7 @@ func (s *configSuite) TestVolumeValidate(c *C) { c.Assert(vc.Validate(), NotNil) vc = &Volume{ - DriverOptions: map[string]string{"pool": "rbd"}, + DriverOptions: storage.DriverParams{"pool": "rbd"}, CreateOptions: CreateOptions{Size: "10MB"}, RuntimeOptions: RuntimeOptions{UseSnapshots: false}, VolumeName: "", @@ -55,7 +55,7 @@ func (s *configSuite) TestVolumeValidate(c *C) { c.Assert(vc.Validate(), NotNil) vc = &Volume{ - DriverOptions: map[string]string{"pool": "rbd"}, + DriverOptions: storage.DriverParams{"pool": "rbd"}, CreateOptions: CreateOptions{Size: "10MB"}, RuntimeOptions: RuntimeOptions{UseSnapshots: false}, VolumeName: "foo", @@ -70,7 +70,7 @@ func (s *configSuite) TestVolumeValidate(c *C) { Snapshot: "ceph", CRUD: "ceph", }, - DriverOptions: map[string]string{"pool": "rbd"}, + DriverOptions: storage.DriverParams{"pool": "rbd"}, CreateOptions: CreateOptions{Size: "10MB"}, RuntimeOptions: RuntimeOptions{UseSnapshots: false}, VolumeName: "foo", @@ -229,7 +229,7 @@ func (s *configSuite) TestToDriverOptions(c *C) { Volume: storage.Volume{ Name: "policy1/test", Size: 0xa, - Params: storage.Params{"pool": "rbd"}, + Params: storage.DriverParams{"pool": "rbd"}, }, FSOptions: storage.FSOptions{ Type: "ext4", diff --git a/db/structs.go b/db/structs.go index 64c21c43..5e3a520f 100644 --- a/db/structs.go +++ b/db/structs.go @@ -2,19 +2,21 @@ package db import ( "time" + + "github.com/contiv/volplugin/storage" ) // Policy is the configuration of the policy. It includes default // information for items such as pool and volume configuration. type Policy struct { - Name string `json:"name"` - Unlocked bool `json:"unlocked,omitempty" merge:"unlocked"` - CreateOptions CreateOptions `json:"create"` - RuntimeOptions *RuntimeOptions `json:"runtime"` - DriverOptions map[string]string `json:"driver"` - FileSystems map[string]string `json:"filesystems"` - Backends *BackendDrivers `json:"backends,omitempty"` - Backend string `json:"backend,omitempty"` + Name string `json:"name"` + Unlocked bool `json:"unlocked,omitempty" merge:"unlocked"` + CreateOptions CreateOptions `json:"create"` + RuntimeOptions *RuntimeOptions `json:"runtime"` + DriverOptions storage.DriverParams `json:"driver"` + FileSystems map[string]string `json:"filesystems"` + Backends *BackendDrivers `json:"backends,omitempty"` + Backend string `json:"backend,omitempty"` } // BackendDrivers is a struct containing all the drivers used under this policy @@ -61,14 +63,14 @@ type UseSnapshot struct { // Volume is the configuration of the policy. It includes pool and // snapshot information. type Volume struct { - PolicyName string `json:"policy"` - VolumeName string `json:"name"` - Unlocked bool `json:"unlocked,omitempty" merge:"unlocked"` - DriverOptions map[string]string `json:"driver"` - MountSource string `json:"mount" merge:"mount"` - CreateOptions CreateOptions `json:"create"` - RuntimeOptions *RuntimeOptions `json:"runtime"` - Backends *BackendDrivers `json:"backends,omitempty"` + PolicyName string `json:"policy"` + VolumeName string `json:"name"` + Unlocked bool `json:"unlocked,omitempty" merge:"unlocked"` + DriverOptions storage.DriverParams `json:"driver"` + MountSource string `json:"mount" merge:"mount"` + CreateOptions CreateOptions `json:"create"` + RuntimeOptions *RuntimeOptions `json:"runtime"` + Backends *BackendDrivers `json:"backends,omitempty"` } // CreateOptions are the set of options used by apiserver during the volume diff --git a/db/test/policy_test.go b/db/test/policy_test.go index 5d9488fd..41589b41 100644 --- a/db/test/policy_test.go +++ b/db/test/policy_test.go @@ -2,6 +2,7 @@ package test import ( "github.com/contiv/volplugin/db" + "github.com/contiv/volplugin/storage" . "gopkg.in/check.v1" ) @@ -13,7 +14,7 @@ var testPolicies = map[string]*db.Policy{ Mount: "ceph", Snapshot: "ceph", }, - DriverOptions: map[string]string{"pool": "rbd"}, + DriverOptions: storage.DriverParams{"pool": "rbd"}, CreateOptions: db.CreateOptions{ Size: "10MB", FileSystem: db.DefaultFilesystem, @@ -34,7 +35,7 @@ var testPolicies = map[string]*db.Policy{ Mount: "ceph", Snapshot: "ceph", }, - DriverOptions: map[string]string{"pool": "rbd"}, + DriverOptions: storage.DriverParams{"pool": "rbd"}, CreateOptions: db.CreateOptions{ Size: "20MB", FileSystem: db.DefaultFilesystem, @@ -49,7 +50,7 @@ var testPolicies = map[string]*db.Policy{ Mount: "ceph", Snapshot: "ceph", }, - DriverOptions: map[string]string{"pool": "rbd"}, + DriverOptions: storage.DriverParams{"pool": "rbd"}, CreateOptions: db.CreateOptions{ Size: "0", FileSystem: db.DefaultFilesystem, @@ -63,7 +64,7 @@ var testPolicies = map[string]*db.Policy{ Mount: "ceph", Snapshot: "ceph", }, - DriverOptions: map[string]string{"pool": "rbd"}, + DriverOptions: storage.DriverParams{"pool": "rbd"}, CreateOptions: db.CreateOptions{ Size: "not a number", FileSystem: db.DefaultFilesystem, @@ -77,7 +78,7 @@ var testPolicies = map[string]*db.Policy{ Mount: "ceph", Snapshot: "ceph", }, - DriverOptions: map[string]string{"pool": "rbd"}, + DriverOptions: storage.DriverParams{"pool": "rbd"}, CreateOptions: db.CreateOptions{ Size: "10MB", FileSystem: db.DefaultFilesystem, @@ -96,7 +97,7 @@ var testPolicies = map[string]*db.Policy{ Mount: "ceph", }, Name: "blanksize", - DriverOptions: map[string]string{"pool": "rbd"}, + DriverOptions: storage.DriverParams{"pool": "rbd"}, CreateOptions: db.CreateOptions{ FileSystem: db.DefaultFilesystem, }, @@ -109,7 +110,7 @@ var testPolicies = map[string]*db.Policy{ Mount: "ceph", }, Name: "blanksize", - DriverOptions: map[string]string{"pool": "rbd"}, + DriverOptions: storage.DriverParams{"pool": "rbd"}, CreateOptions: db.CreateOptions{ FileSystem: db.DefaultFilesystem, }, @@ -118,7 +119,7 @@ var testPolicies = map[string]*db.Policy{ }, "nobackend": { Name: "nobackend", - DriverOptions: map[string]string{"pool": "rbd"}, + DriverOptions: storage.DriverParams{"pool": "rbd"}, CreateOptions: db.CreateOptions{ Size: "10MB", FileSystem: db.DefaultFilesystem, diff --git a/db/test/volume_test.go b/db/test/volume_test.go index 90506646..d95e77bb 100644 --- a/db/test/volume_test.go +++ b/db/test/volume_test.go @@ -148,7 +148,7 @@ func (s *testSuite) TestVolumeValidate(c *C) { c.Assert(vc.Validate(), NotNil) vc = &db.Volume{ - DriverOptions: map[string]string{"pool": "rbd"}, + DriverOptions: storage.DriverParams{"pool": "rbd"}, CreateOptions: db.CreateOptions{Size: "10MB"}, RuntimeOptions: &db.RuntimeOptions{UseSnapshots: false}, VolumeName: "", @@ -158,7 +158,7 @@ func (s *testSuite) TestVolumeValidate(c *C) { c.Assert(vc.Validate(), NotNil) vc = &db.Volume{ - DriverOptions: map[string]string{"pool": "rbd"}, + DriverOptions: storage.DriverParams{"pool": "rbd"}, CreateOptions: db.CreateOptions{Size: "10MB"}, RuntimeOptions: &db.RuntimeOptions{UseSnapshots: false}, VolumeName: "foo", @@ -173,7 +173,7 @@ func (s *testSuite) TestVolumeValidate(c *C) { Snapshot: "ceph", CRUD: "ceph", }, - DriverOptions: map[string]string{"pool": "rbd"}, + DriverOptions: storage.DriverParams{"pool": "rbd"}, CreateOptions: db.CreateOptions{Size: "10MB"}, RuntimeOptions: &db.RuntimeOptions{UseSnapshots: false}, VolumeName: "foo", @@ -206,7 +206,7 @@ func (s *testSuite) TestToDriverOptions(c *C) { Volume: storage.Volume{ Name: "basic/test", Size: 0xa, - Params: storage.Params{"pool": "rbd"}, + Params: storage.DriverParams{"pool": "rbd"}, }, FSOptions: storage.FSOptions{ Type: "ext4", diff --git a/db/volume.go b/db/volume.go index 00c939b7..69878957 100644 --- a/db/volume.go +++ b/db/volume.go @@ -39,7 +39,7 @@ func CreateVolume(vr *VolumeRequest) (*Volume, error) { } if vr.Policy.DriverOptions == nil { - vr.Policy.DriverOptions = map[string]string{} + vr.Policy.DriverOptions = storage.DriverParams{} } if err := vr.Policy.Validate(); err != nil { diff --git a/storage/backend/ceph/ceph.go b/storage/backend/ceph/ceph.go index 2357c5d3..3b3c200f 100644 --- a/storage/backend/ceph/ceph.go +++ b/storage/backend/ceph/ceph.go @@ -110,12 +110,17 @@ func (c *Driver) internalName(s string) (string, error) { // Create a volume. func (c *Driver) Create(do storage.DriverOptions) error { + var poolName string + if err := do.Volume.Params.Get("pool", &poolName); err != nil { + return err + } + intName, err := c.internalName(do.Volume.Name) if err != nil { return err } - cmd := exec.Command("rbd", "create", mkpool(do.Volume.Params["pool"], intName), "--size", strconv.FormatUint(do.Volume.Size, 10)) + cmd := exec.Command("rbd", "create", mkpool(poolName, intName), "--size", strconv.FormatUint(do.Volume.Size, 10)) er, err := runWithTimeout(cmd, do.Timeout) if er != nil { @@ -150,7 +155,11 @@ func (c *Driver) Format(do storage.DriverOptions) error { // Destroy a volume. func (c *Driver) Destroy(do storage.DriverOptions) error { - poolName := do.Volume.Params["pool"] + var poolName string + if err := do.Volume.Params.Get("pool", &poolName); err != nil { + return err + } + intName, err := c.internalName(do.Volume.Name) if err != nil { return err @@ -173,7 +182,10 @@ func (c *Driver) Destroy(do storage.DriverOptions) error { // List all volumes. func (c *Driver) List(lo storage.ListOptions) ([]storage.Volume, error) { - poolName := lo.Params["pool"] + var poolName string + if err := lo.Params.Get("pool", &poolName); err != nil { + return nil, err + } retry: er, err := executor.NewCapture(exec.Command("rbd", "ls", poolName, "--format", "json")).Run(context.Background()) @@ -196,7 +208,7 @@ retry: list := []storage.Volume{} for _, name := range textList { - list = append(list, storage.Volume{Name: c.externalName(strings.TrimSpace(name)), Params: storage.Params{"pool": poolName}}) + list = append(list, storage.Volume{Name: c.externalName(strings.TrimSpace(name)), Params: storage.DriverParams{"pool": poolName}}) } return list, nil @@ -211,7 +223,10 @@ func (c *Driver) Mount(do storage.DriverOptions) (*storage.Mount, error) { return nil, err } - poolName := do.Volume.Params["pool"] + var poolName string + if err := do.Volume.Params.Get("pool", &poolName); err != nil { + return nil, err + } volumePath, err := c.mkMountPath(poolName, intName) if err != nil { @@ -260,7 +275,11 @@ func (c *Driver) Mount(do storage.DriverOptions) (*storage.Mount, error) { // Unmount a volume. func (c *Driver) Unmount(do storage.DriverOptions) error { - poolName := do.Volume.Params["pool"] + var poolName string + if err := do.Volume.Params.Get("pool", &poolName); err != nil { + return err + } + intName, err := c.internalName(do.Volume.Name) if err != nil { return err @@ -325,7 +344,10 @@ func (c *Driver) CreateSnapshot(snapName string, do storage.DriverOptions) error return err } - poolName := do.Volume.Params["pool"] + var poolName string + if err := do.Volume.Params.Get("pool", &poolName); err != nil { + return err + } snapName = strings.Replace(snapName, " ", "-", -1) cmd := exec.Command("rbd", "snap", "create", mkpool(poolName, intName), "--snap", snapName) @@ -348,7 +370,10 @@ func (c *Driver) RemoveSnapshot(snapName string, do storage.DriverOptions) error return err } - poolName := do.Volume.Params["pool"] + var poolName string + if err := do.Volume.Params.Get("pool", &poolName); err != nil { + return err + } cmd := exec.Command("rbd", "snap", "rm", mkpool(poolName, intName), "--snap", snapName) er, err := runWithTimeout(cmd, do.Timeout) @@ -371,7 +396,10 @@ func (c *Driver) ListSnapshots(do storage.DriverOptions) ([]string, error) { return nil, err } - poolName := do.Volume.Params["pool"] + var poolName string + if err := do.Volume.Params.Get("pool", &poolName); err != nil { + return nil, err + } cmd := exec.Command("rbd", "snap", "ls", mkpool(poolName, intName)) ctx, _ := context.WithTimeout(context.Background(), do.Timeout) @@ -414,7 +442,11 @@ func (c *Driver) cleanupCopy(snapName, newName string, do storage.DriverOptions, return } - poolName := do.Volume.Params["pool"] + var poolName string + if err := do.Volume.Params.Get("pool", &poolName); err != nil { + logrus.Error(err) + return + } select { case err := <-errChan: @@ -453,9 +485,12 @@ func (c *Driver) CopySnapshot(do storage.DriverOptions, snapName, newName string return err } - poolName := do.Volume.Params["pool"] + var poolName string + if err := do.Volume.Params.Get("pool", &poolName); err != nil { + return err + } - list, err := c.List(storage.ListOptions{Params: storage.Params{"pool": poolName}}) + list, err := c.List(storage.ListOptions{Params: storage.DriverParams{"pool": poolName}}) for _, vol := range list { if intNewName == vol.Name { return errored.Errorf("Volume %q already exists", vol.Name) @@ -554,7 +589,12 @@ func (c *Driver) Validate(do *storage.DriverOptions) error { return err } - if do.Volume.Params["pool"] == "" { + var poolName string + if err := do.Volume.Params.Get("pool", &poolName); err != nil { + return err + } + + if poolName == "" { return errored.Errorf("Pool is missing in ceph storage driver.") } diff --git a/storage/backend/ceph/ceph_test.go b/storage/backend/ceph/ceph_test.go index c77f8a96..a6ab5381 100644 --- a/storage/backend/ceph/ceph_test.go +++ b/storage/backend/ceph/ceph_test.go @@ -30,7 +30,7 @@ var mountscanDriverOpts = storage.DriverOptions{ Volume: storage.Volume{ Name: "test/mountscan", Size: 10, - Params: storage.Params{"pool": "rbd"}, + Params: storage.DriverParams{"pool": "rbd"}, }, FSOptions: filesystems["ext4"], Timeout: 5 * time.Second, @@ -39,13 +39,13 @@ var mountscanDriverOpts = storage.DriverOptions{ var volumeSpec = storage.Volume{ Name: "test/pithos", Size: 10, - Params: storage.Params{"pool": "rbd"}, + Params: storage.DriverParams{"pool": "rbd"}, } var volumeSpecTestPool = storage.Volume{ Name: "test/pithos", Size: 10, - Params: storage.Params{"pool": "test"}, + Params: storage.DriverParams{"pool": "test"}, } type cephSuite struct{} @@ -230,7 +230,10 @@ again: for i := 0; i < 10; i++ { _, err := mountDrv.Mount(driverOpts) c.Assert(err, IsNil) - s.readWriteTest(c, fmt.Sprintf("/mnt/ceph/%s/test.pithos", driverOpts.Volume.Params["pool"])) + var poolName string + err = driverOpts.Volume.Params.Get("pool", &poolName) + c.Assert(err, IsNil) + s.readWriteTest(c, fmt.Sprintf("/mnt/ceph/%s/test.pithos", poolName)) c.Assert(mountDrv.Unmount(driverOpts), IsNil) } c.Assert(crudDrv.Destroy(driverOpts), IsNil) @@ -290,12 +293,16 @@ again: intName, err := (&Driver{}).internalName(driverOpts.Volume.Name) // totally cheating c.Assert(err, IsNil) + var poolName string + err = driverOpts.Volume.Params.Get("pool", &poolName) + c.Assert(err, IsNil) + (*mounts[0]).Volume.Size = 10 // correct this value even though it is an unnecessary value returned c.Assert(*mounts[0], DeepEquals, storage.Mount{ Device: "/dev/rbd0", DevMajor: 252, DevMinor: 0, - Path: strings.Join([]string{myMountpath, driverOpts.Volume.Params["pool"], intName}, "/"), + Path: strings.Join([]string{myMountpath, poolName, intName}, "/"), Volume: driverOpts.Volume, }) @@ -362,7 +369,11 @@ again: c.Assert(snapDrv.CopySnapshot(driverOpts, "testsnap", "test/image"), IsNil) c.Assert(snapDrv.CopySnapshot(driverOpts, "test", "test/image"), NotNil) - content, err := exec.Command("rbd", "ls", driverOpts.Volume.Params["pool"]).CombinedOutput() + var poolName string + err = driverOpts.Volume.Params.Get("pool", &poolName) + c.Assert(err, IsNil) + + content, err := exec.Command("rbd", "ls", poolName).CombinedOutput() c.Assert(err, IsNil) c.Assert(strings.TrimSpace(string(content)), Equals, "test.image\ntest.pithos") c.Assert(snapDrv.CopySnapshot(driverOpts, "foo", "test/image"), NotNil) @@ -370,8 +381,9 @@ again: driverOpts.Volume.Name = "test/image" c.Assert(crudDrv.Destroy(driverOpts), IsNil) - exec.Command("rbd", "snap", "unprotect", mkpool(driverOpts.Volume.Params["pool"], "test.pithos"), "--snap", "test").Run() - exec.Command("rbd", "snap", "unprotect", mkpool(driverOpts.Volume.Params["pool"], "test.pithos"), "--snap", "testsnap").Run() + + exec.Command("rbd", "snap", "unprotect", mkpool(poolName, "test.pithos"), "--snap", "test").Run() + exec.Command("rbd", "snap", "unprotect", mkpool(poolName, "test.pithos"), "--snap", "testsnap").Run() driverOpts.Volume.Name = "test/pithos" c.Assert(crudDrv.Destroy(driverOpts), IsNil) diff --git a/storage/backend/ceph/internals.go b/storage/backend/ceph/internals.go index 042d1dcf..de65b45a 100644 --- a/storage/backend/ceph/internals.go +++ b/storage/backend/ceph/internals.go @@ -23,7 +23,11 @@ type rbdMap map[string]struct { } func (c *Driver) mapImage(do storage.DriverOptions) (string, error) { - poolName := do.Volume.Params["pool"] + var poolName string + if err := do.Volume.Params.Get("pool", &poolName); err != nil { + return "", err + } + intName, err := c.internalName(do.Volume.Name) if err != nil { return "", err @@ -52,14 +56,14 @@ retry: } for _, rbd := range rbdmap { - if rbd.Name == intName && rbd.Pool == do.Volume.Params["pool"] { + if rbd.Name == intName && rbd.Pool == poolName { device = rbd.Device break } } if device == "" { - return "", errored.Errorf("Volume %s in pool %s not found in RBD showmapped output", intName, do.Volume.Params["pool"]) + return "", errored.Errorf("Volume %s in pool %s not found in RBD showmapped output", intName, poolName) } logrus.Debugf("mapped volume %q as %q", intName, device) @@ -101,7 +105,10 @@ func (c *Driver) unmapImage(do storage.DriverOptions) error { } func (c *Driver) doUnmap(do storage.DriverOptions, rbdmap rbdMap) (bool, error) { - poolName := do.Volume.Params["pool"] + var poolName string + if err := do.Volume.Params.Get("pool", &poolName); err != nil { + return false, err + } intName, err := c.internalName(do.Volume.Name) if err != nil { @@ -109,7 +116,7 @@ func (c *Driver) doUnmap(do storage.DriverOptions, rbdmap rbdMap) (bool, error) } for _, rbd := range rbdmap { - if rbd.Name == intName && rbd.Pool == do.Volume.Params["pool"] { + if rbd.Name == intName && rbd.Pool == poolName { logrus.Debugf("Unmapping volume %s/%s at device %q", poolName, intName, strings.TrimSpace(rbd.Device)) if _, err := os.Stat(rbd.Device); err != nil { @@ -187,7 +194,7 @@ func (c *Driver) getMapped(timeout time.Duration) ([]*storage.Mount, error) { Device: rbd.Device, Volume: storage.Volume{ Name: c.externalName(rbd.Name), - Params: map[string]string{ + Params: storage.DriverParams{ "pool": rbd.Pool, }, }, diff --git a/storage/backend/ceph/util.go b/storage/backend/ceph/util.go index caaaccb5..2158085a 100644 --- a/storage/backend/ceph/util.go +++ b/storage/backend/ceph/util.go @@ -13,7 +13,13 @@ func (c *Driver) MountPath(do storage.DriverOptions) (string, error) { if err != nil { return "", err } - return filepath.Join(c.mountpath, do.Volume.Params["pool"], volName), nil + + var poolName string + if err := do.Volume.Params.Get("pool", &poolName); err != nil { + return "", err + } + + return filepath.Join(c.mountpath, poolName, volName), nil } // FIXME maybe this belongs in storage/ as it's more general? diff --git a/storage/backend/nfs/mount.go b/storage/backend/nfs/mount.go index d15d6496..f6aadefc 100644 --- a/storage/backend/nfs/mount.go +++ b/storage/backend/nfs/mount.go @@ -35,7 +35,7 @@ func (d *Driver) Mounted(timeout time.Duration) ([]*storage.Mount, error) { Path: hostMount.MountPoint, Volume: storage.Volume{ Name: rel, - Params: map[string]string{ + Params: storage.DriverParams{ "mount": hostMount.MountSource, }, }, diff --git a/storage/backend/nfs/nfs.go b/storage/backend/nfs/nfs.go index 0c5af574..064115de 100644 --- a/storage/backend/nfs/nfs.go +++ b/storage/backend/nfs/nfs.go @@ -80,7 +80,12 @@ func (d *Driver) mapOptionsToString(mapOpts map[string]string) string { } func (d *Driver) mkOpts(do storage.DriverOptions) (string, error) { - mapOpts, err := d.validateConvertOptions(do.Volume.Params["options"]) + var options string + if err := do.Volume.Params.Get("options", &options); err != nil { + return "", err + } + + mapOpts, err := d.validateConvertOptions(options) if err != nil { return "", err } diff --git a/storage/backend/nfs/nfs_test.go b/storage/backend/nfs/nfs_test.go index 11794108..f58864a3 100644 --- a/storage/backend/nfs/nfs_test.go +++ b/storage/backend/nfs/nfs_test.go @@ -203,7 +203,7 @@ func (s *nfsSuite) TestNFSOptionsFromDriverOptions(c *C) { Source: source, Volume: storage.Volume{ Name: "foo/bar", - Params: storage.Params{ + Params: storage.DriverParams{ "options": "test=1", }, }, @@ -219,7 +219,7 @@ func (s *nfsSuite) TestNFSOptionsFromDriverOptions(c *C) { Source: "localhost:/mnt", Volume: storage.Volume{ Name: "foo/bar", - Params: storage.Params{ + Params: storage.DriverParams{ "options": options, }, }, @@ -234,7 +234,7 @@ func (s *nfsSuite) TestNFSOptionsFromDriverOptions(c *C) { Source: "localhost:/mnt", Volume: storage.Volume{ Name: "foo/bar", - Params: storage.Params{ + Params: storage.DriverParams{ "options": "rw,sync,test=1", }, }, diff --git a/storage/driver.go b/storage/driver.go index 855a7e83..035ec22c 100644 --- a/storage/driver.go +++ b/storage/driver.go @@ -1,9 +1,12 @@ package storage import ( + "encoding/json" "errors" + "reflect" "time" + "github.com/Sirupsen/logrus" "github.com/contiv/errored" ) @@ -12,8 +15,8 @@ var ( ErrVolumeExist = errors.New("Volume already exists") ) -// Params are parameters that relate directly to the location of the storage. -type Params map[string]string +// DriverParams are parameters that relate directly to the location of the storage. +type DriverParams map[string]interface{} // A Mount is the resulting attributes of a Mount or Unmount operation. type Mount struct { @@ -42,14 +45,14 @@ type DriverOptions struct { // ListOptions is a set of parameters used for the List operation of Driver. type ListOptions struct { - Params Params + Params DriverParams } // Volume is the basic representation of a volume name and its parameters. type Volume struct { Name string Size uint64 - Params Params + Params DriverParams } // NamedDriver is a named driver and has a method called Name() @@ -146,3 +149,51 @@ func (v Volume) Validate() error { return nil } + +// Get retrieves the value of attribute `attrName` from DriverParams +func (dp DriverParams) Get(attrName string, result interface{}) error { + if _, ok := dp[attrName]; !ok { // attrName not found in DriverParams + return nil + } + + expectedType := reflect.TypeOf(result) + if expectedType == nil { + return errored.Errorf("Cannot use as pointer type") + } else if expectedType.Kind() != reflect.Ptr { + return errored.Errorf("Cannot reference a non-pointer type %q", expectedType.Kind()) + } + + expectedKind := expectedType.Elem().Kind() + + actualType := reflect.TypeOf(dp[attrName]) + if actualType == nil { // types + return errored.Errorf("Expected %q; Cannot use value for driver.%q", expectedKind, attrName) + } + + actualKind := actualType.Kind() + + value := reflect.ValueOf(dp[attrName]) // returns zero-value for value + switch value.Kind() { + case reflect.String, reflect.Int, reflect.Float64, reflect.Float32, reflect.Bool: + if actualKind != expectedKind { + return errored.Errorf("Cannot use %q as type %q", expectedKind, actualKind) + } + reflect.ValueOf(result).Elem().Set(value) // succeeds only if the types match + case reflect.Map: + if expectedKind != reflect.Map && expectedKind != reflect.Struct { + return errored.Errorf("Expected map or struct; Cannot use %q as type %q", expectedKind, actualKind) + } + content, err := json.Marshal(value.Interface()) + if err != nil { + return err + } + + if err := json.Unmarshal(content, result); err != nil { + return err + } + default: + logrus.Info("Unknown type %q", value.Kind()) + } + + return nil +} diff --git a/storage/driver_test.go b/storage/driver_test.go index 4f9727f3..f7fc6883 100644 --- a/storage/driver_test.go +++ b/storage/driver_test.go @@ -17,7 +17,7 @@ func (s *storageSuite) TestDriverOptionsValidate(c *C) { c.Assert(do.Validate(), NotNil) - do = DriverOptions{Timeout: 1, Volume: Volume{Name: "hi", Params: map[string]string{}}} + do = DriverOptions{Timeout: 1, Volume: Volume{Name: "hi", Params: DriverParams{}}} c.Assert(do.Validate(), IsNil) do.Timeout = 0 c.Assert(do.Validate(), NotNil) @@ -29,13 +29,110 @@ func (s *storageSuite) TestVolumeValidate(c *C) { v := Volume{} c.Assert(v.Validate(), NotNil) - v = Volume{Name: "name", Size: 100, Params: map[string]string{}} + v = Volume{Name: "name", Size: 100, Params: DriverParams{}} c.Assert(v.Validate(), IsNil) v.Name = "" c.Assert(v.Validate(), NotNil) v.Name = "name" v.Params = nil c.Assert(v.Validate(), NotNil) - v.Params = map[string]string{} + v.Params = DriverParams{} c.Assert(v.Validate(), IsNil) } + +type testStruct struct { + X int + Y string +} + +func (s *storageSuite) TestParams(c *C) { + mp1 := DriverParams{"ceph": "1", "nfs": "1", "gluster": "1", "netapp": "0"} + mp2 := DriverParams{"ceph": 1, "nfs": 1, "gluster": 1, "netapp": 2, "purestorage": 2} + ts := DriverParams{"x": 1, "y": "string"} + do := DriverOptions{Timeout: 1, Volume: Volume{Name: "TestParams", Params: DriverParams{"str": "testString", "int": 1, "mp1": mp1, "mp2": mp2, "struct": ts}}} + + // Integer type + var intVal int + err := do.Volume.Params.Get("int", &intVal) + c.Assert(err, IsNil) + c.Assert(intVal == 1, Equals, true) + + // Float type + do.Volume.Params["float"] = 1.2 + var floatVal float64 + err = do.Volume.Params.Get("float", &floatVal) + c.Assert(err, IsNil) + c.Assert(floatVal == 1.2, Equals, true) + + // Bool type + var boolVal bool + do.Volume.Params["bool"] = true + err = do.Volume.Params.Get("bool", &boolVal) + c.Assert(err, IsNil) + c.Assert(boolVal == true, Equals, true) + + // String type + var strVal string + err = do.Volume.Params.Get("str", &strVal) + c.Assert(err, IsNil) + c.Assert(strVal == "testString", Equals, true) + + // Map type + var map1 map[string]string + err = do.Volume.Params.Get("mp1", &map1) + c.Assert(err, IsNil) + c.Assert(len(map1) == len(mp1), Equals, true) + c.Assert(map1["netapp"] == "0", Equals, true) + + var map2 map[string]int + err = do.Volume.Params.Get("mp2", &map2) + c.Assert(err, IsNil) + c.Assert(len(map2) == len(mp2), Equals, true) + c.Assert(map2["purestorage"] == 2, Equals, true) + + // Struct type + tStruct := testStruct{} + err = do.Volume.Params.Get("struct", &tStruct) + c.Assert(err, IsNil) + c.Assert(tStruct.X == 1, Equals, true) + c.Assert(tStruct.Y == "string", Equals, true) + + // Below are few invalid cases! + + // Attr not found + var x int + err = do.Volume.Params.Get("x", &x) + c.Assert(err, IsNil) + c.Assert(x == 0, Equals, true) // default int value + + var mp map[string]string + err = do.Volume.Params.Get("mp", &mp) + c.Assert(err, IsNil) + c.Assert(len(mp) == 0, Equals, true) // empty map + + // Type mismatches + var str int + err = do.Volume.Params.Get("str", &str) // trying to get string value into an integer + c.Assert(err, NotNil) + c.Assert(str == 0, Equals, true) + + // Marshaling should fail as its trying to pack map[string]string to string + var map3 string + err = do.Volume.Params.Get("mp1", &map3) + c.Assert(err, NotNil) + c.Assert(map3 == "", Equals, true) + + var bVal bool // capture float in bool type + err = do.Volume.Params.Get("float", &bVal) + c.Assert(err, NotNil) + c.Assert(bVal == false, Equals, true) // default bool value + + // Nil values + do.Volume.Params["nilValue"] = nil + var nilV string + err = do.Volume.Params.Get("nilValue", &nilV) + c.Assert(err, NotNil) // typeOf(nilValue) == nil; error + + err = do.Volume.Params.Get("str", nil) // does not accept values as pointers + c.Assert(err, NotNil) +} diff --git a/volsupervisor/loop.go b/volsupervisor/loop.go index 8b8a3a00..5f5c0db2 100644 --- a/volsupervisor/loop.go +++ b/volsupervisor/loop.go @@ -75,7 +75,7 @@ func (dc *DaemonConfig) pruneSnapshots(val *config.Volume) { driverOpts := storage.DriverOptions{ Volume: storage.Volume{ Name: val.String(), - Params: storage.Params{ + Params: storage.DriverParams{ "pool": val.DriverOptions["pool"], }, }, @@ -128,7 +128,7 @@ func (dc *DaemonConfig) createSnapshot(val *config.Volume) { driverOpts := storage.DriverOptions{ Volume: storage.Volume{ Name: val.String(), - Params: storage.Params{ + Params: storage.DriverParams{ "pool": val.DriverOptions["pool"], }, },