Merge pull request #48999 from vladimirvivien/scaleio-vol-attribs-update

Automatic merge from submit-queue (batch tested with PRs 49871, 49422, 49092, 49858, 48999) ScaleIO Volume Plugin - Volume attribute fixes and updates **What this PR does / why we need it**: This is a housekeeping PR for small enhancements and fixes to the ScaleIO volume plugin to address issues: - Enforcement of fsGroup - Enable ScaleIO multiple-instance volume mapping - Tighter validation of PVC parameters - Injection of default PVC capacity when omitted - Better alignment of PVC, PV, and volume names for dynamic provisioning **Special notes for your reviewer**: **Release note**: ```release-note Enforcement of fsGroup; enable ScaleIO multiple-instance volume mapping; default PVC capacity; alignment of PVC, PV, and volume names for dynamic provisioning ```
kubernetes · Aug 2, 2017 · 0cb5ec7 · 0cb5ec7
2 parents ba118b4 + fda99bd
commit 0cb5ec7
Show file tree

Hide file tree

Showing 9 changed files with 202 additions and 58 deletions.
diff --git a/pkg/volume/scaleio/BUILD b/pkg/volume/scaleio/BUILD
@@ -22,6 +22,7 @@ go_test(
         "//pkg/volume:go_default_library",
         "//pkg/volume/testing:go_default_library",
         "//vendor/github.com/codedellemc/goscaleio/types/v1:go_default_library",
+        "//vendor/github.com/golang/glog:go_default_library",
         "//vendor/k8s.io/api/core/v1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/types:go_default_library",

diff --git a/pkg/volume/scaleio/sio_client.go b/pkg/volume/scaleio/sio_client.go
@@ -45,7 +45,7 @@ type sioInterface interface {
 	FindVolume(name string) (*siotypes.Volume, error)
 	Volume(sioVolumeID) (*siotypes.Volume, error)
 	CreateVolume(name string, sizeGB int64) (*siotypes.Volume, error)
-	AttachVolume(sioVolumeID) error
+	AttachVolume(sioVolumeID, bool) error
 	DetachVolume(sioVolumeID) error
 	DeleteVolume(sioVolumeID) error
 	IID() (string, error)
@@ -217,8 +217,9 @@ func (c *sioClient) CreateVolume(name string, sizeGB int64) (*siotypes.Volume, e
 	return c.Volume(sioVolumeID(createResponse.ID))
 }
 
-// AttachVolume maps the scaleio volume to an sdc node.
-func (c *sioClient) AttachVolume(id sioVolumeID) error {
+// AttachVolume maps the scaleio volume to an sdc node.  If the multipleMappings flag
+// is true, ScaleIO will allow other SDC to map to that volume.
+func (c *sioClient) AttachVolume(id sioVolumeID, multipleMappings bool) error {
 	if err := c.init(); err != nil {
 		glog.Error(log("failed to init'd client in attach volume: %v", err))
 		return err
@@ -232,7 +233,7 @@ func (c *sioClient) AttachVolume(id sioVolumeID) error {
 
 	params := &siotypes.MapVolumeSdcParam{
 		SdcID: iid,
-		AllowMultipleMappings: "false",
+		AllowMultipleMappings: strconv.FormatBool(multipleMappings),
 		AllSdcs:               "",
 	}
 	volClient := sio.NewVolume(c.client)

diff --git a/pkg/volume/scaleio/sio_mgr.go b/pkg/volume/scaleio/sio_mgr.go
@@ -27,7 +27,7 @@ import (
 
 type storageInterface interface {
 	CreateVolume(string, int64) (*siotypes.Volume, error)
-	AttachVolume(string) (string, error)
+	AttachVolume(string, bool) (string, error)
 	IsAttached(string) (bool, error)
 	DetachVolume(string) error
 	DeleteVolume(string) error
@@ -103,8 +103,9 @@ func (m *sioMgr) CreateVolume(volName string, sizeGB int64) (*siotypes.Volume, e
 	return vol, nil
 }
 
-// AttachVolume maps a ScaleIO volume to the running node
-func (m *sioMgr) AttachVolume(volName string) (string, error) {
+// AttachVolume maps a ScaleIO volume to the running node.  If flag multiMaps,
+// ScaleIO will allow other SDC to map to volume.
+func (m *sioMgr) AttachVolume(volName string, multipleMappings bool) (string, error) {
 	client, err := m.getClient()
 	if err != nil {
 		glog.Error(log("attach volume failed: %v", err))
@@ -139,7 +140,7 @@ func (m *sioMgr) AttachVolume(volName string) (string, error) {
 	}
 
 	// attach volume, get deviceName
-	if err := client.AttachVolume(sioVolumeID(vol.ID)); err != nil {
+	if err := client.AttachVolume(sioVolumeID(vol.ID), multipleMappings); err != nil {
 		glog.Error(log("attachment for volume %s failed :%v", volName, err))
 		return "", err
 	}

diff --git a/pkg/volume/scaleio/sio_mgr_test.go b/pkg/volume/scaleio/sio_mgr_test.go
@@ -99,7 +99,7 @@ func TestMgrCreateVolume(t *testing.T) {
 func TestMgrAttachVolume(t *testing.T) {
 	mgr := newTestMgr(t)
 	mgr.CreateVolume("test-vol-0001", 8*1024*1024)
-	device, err := mgr.AttachVolume("test-vol-0001")
+	device, err := mgr.AttachVolume("test-vol-0001", false)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -111,8 +111,8 @@ func TestMgrAttachVolume(t *testing.T) {
 func TestMgrAttachVolume_AlreadyAttached(t *testing.T) {
 	mgr := newTestMgr(t)
 	mgr.CreateVolume("test-vol-0001", 8*1024*1024)
-	mgr.AttachVolume("test-vol-0001")
-	dev, err := mgr.AttachVolume("test-vol-0001")
+	mgr.AttachVolume("test-vol-0001", false)
+	dev, err := mgr.AttachVolume("test-vol-0001", false)
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
@@ -124,7 +124,8 @@ func TestMgrAttachVolume_AlreadyAttached(t *testing.T) {
 func TestMgrAttachVolume_VolumeNotFoundError(t *testing.T) {
 	mgr := newTestMgr(t)
 	mgr.CreateVolume("test-vol-0001", 8*1024*1024)
-	_, err := mgr.AttachVolume("test-vol-0002")
+	_, err := mgr.AttachVolume("test-vol-0002", false)
+
 	if err == nil {
 		t.Error("attachVolume should fail with volume not found error")
 	}
@@ -137,7 +138,7 @@ func TestMgrAttachVolume_WaitForAttachError(t *testing.T) {
 		c := mgr.client.(*fakeSio)
 		close(c.waitAttachCtrl)
 	}()
-	_, err := mgr.AttachVolume("test-vol-0001")
+	_, err := mgr.AttachVolume("test-vol-0001", false)
 	if err == nil {
 		t.Error("attachVolume should fail with attach timeout error")
 	}
@@ -146,7 +147,7 @@ func TestMgrAttachVolume_WaitForAttachError(t *testing.T) {
 func TestMgrDetachVolume(t *testing.T) {
 	mgr := newTestMgr(t)
 	mgr.CreateVolume("test-vol-0001", 8*1024*1024)
-	mgr.AttachVolume("test-vol-0001")
+	mgr.AttachVolume("test-vol-0001", false)
 	if err := mgr.DetachVolume("test-vol-0001"); err != nil {
 		t.Fatal(err)
 	}
@@ -162,7 +163,7 @@ func TestMgrDetachVolume(t *testing.T) {
 func TestMgrDetachVolume_VolumeNotFound(t *testing.T) {
 	mgr := newTestMgr(t)
 	mgr.CreateVolume("test-vol-0001", 8*1024*1024)
-	mgr.AttachVolume("test-vol-0001")
+	mgr.AttachVolume("test-vol-0001", false)
 	err := mgr.DetachVolume("test-vol-0002")
 	if err == nil {
 		t.Fatal("expected a volume not found failure")
@@ -181,7 +182,7 @@ func TestMgrDetachVolume_VolumeNotAttached(t *testing.T) {
 func TestMgrDetachVolume_VolumeAlreadyDetached(t *testing.T) {
 	mgr := newTestMgr(t)
 	mgr.CreateVolume("test-vol-0001", 8*1024*1024)
-	mgr.AttachVolume("test-vol-0001")
+	mgr.AttachVolume("test-vol-0001", false)
 	mgr.DetachVolume("test-vol-0001")
 	err := mgr.DetachVolume("test-vol-0001")
 	if err != nil {
@@ -192,7 +193,7 @@ func TestMgrDetachVolume_VolumeAlreadyDetached(t *testing.T) {
 func TestMgrDetachVolume_WaitForDetachError(t *testing.T) {
 	mgr := newTestMgr(t)
 	mgr.CreateVolume("test-vol-0001", 8*1024*1024)
-	mgr.AttachVolume("test-vol-0001")
+	mgr.AttachVolume("test-vol-0001", false)
 	err := mgr.DetachVolume("test-vol-0001")
 	if err != nil {
 		t.Error("detachVolume failed")
@@ -227,6 +228,7 @@ type fakeSio struct {
 	waitAttachCtrl chan struct{}
 	waitDetachCtrl chan struct{}
 	devs           map[string]string
+	isMultiMap     bool
 }
 
 func newFakeSio() *fakeSio {
@@ -261,7 +263,8 @@ func (f *fakeSio) CreateVolume(volName string, sizeGB int64) (*siotypes.Volume,
 	return f.volume, nil
 }
 
-func (f *fakeSio) AttachVolume(id sioVolumeID) error {
+func (f *fakeSio) AttachVolume(id sioVolumeID, multiMaps bool) error {
+	f.isMultiMap = multiMaps
 	_, err := f.Volume(id)
 	if err != nil {
 		return err

diff --git a/pkg/volume/scaleio/sio_plugin.go b/pkg/volume/scaleio/sio_plugin.go
@@ -149,6 +149,7 @@ var _ volume.PersistentVolumePlugin = &sioPlugin{}
 func (p *sioPlugin) GetAccessModes() []api.PersistentVolumeAccessMode {
 	return []api.PersistentVolumeAccessMode{
 		api.ReadWriteOnce,
+		api.ReadOnlyMany,
 	}
 }
 

diff --git a/pkg/volume/scaleio/sio_util.go b/pkg/volume/scaleio/sio_util.go
@@ -68,11 +68,13 @@ var (
 	nsSep       = "%"
 	sdcRootPath = "/opt/emc/scaleio/sdc/bin"
 
-	secretNotFoundErr       = errors.New("secret not found")
-	configMapNotFoundErr    = errors.New("configMap not found")
-	gatewayNotProvidedErr   = errors.New("gateway not provided")
-	secretRefNotProvidedErr = errors.New("secret ref not provided")
-	systemNotProvidedErr    = errors.New("secret not provided")
+	secretNotFoundErr              = errors.New("secret not found")
+	configMapNotFoundErr           = errors.New("configMap not found")
+	gatewayNotProvidedErr          = errors.New("ScaleIO gateway not provided")
+	secretRefNotProvidedErr        = errors.New("secret ref not provided")
+	systemNotProvidedErr           = errors.New("ScaleIO system not provided")
+	storagePoolNotProvidedErr      = errors.New("ScaleIO storage pool not provided")
+	protectionDomainNotProvidedErr = errors.New("ScaleIO protection domain not provided")
 )
 
 // mapScaleIOVolumeSource maps attributes from a ScaleIOVolumeSource to config
@@ -107,6 +109,12 @@ func validateConfigs(config map[string]string) error {
 	if config[confKey.system] == "" {
 		return systemNotProvidedErr
 	}
+	if config[confKey.storagePool] == "" {
+		return storagePoolNotProvidedErr
+	}
+	if config[confKey.protectionDomain] == "" {
+		return protectionDomainNotProvidedErr
+	}
 
 	return nil
 }
@@ -119,8 +127,6 @@ func applyConfigDefaults(config map[string]string) {
 		b = false
 	}
 	config[confKey.sslEnabled] = strconv.FormatBool(b)
-	config[confKey.protectionDomain] = defaultString(config[confKey.protectionDomain], "default")
-	config[confKey.storagePool] = defaultString(config[confKey.storagePool], "default")
 	config[confKey.storageMode] = defaultString(config[confKey.storageMode], "ThinProvisioned")
 	config[confKey.fsType] = defaultString(config[confKey.fsType], "xfs")
 	b, err = strconv.ParseBool(config[confKey.readOnly])

diff --git a/pkg/volume/scaleio/sio_util_test.go b/pkg/volume/scaleio/sio_util_test.go
@@ -115,10 +115,10 @@ func TestUtilApplyConfigDefaults(t *testing.T) {
 	if data[confKey.system] != "sio" {
 		t.Error("Unexpected system value")
 	}
-	if data[confKey.protectionDomain] != "default" {
+	if data[confKey.protectionDomain] != "" {
 		t.Error("Unexpected protection domain value")
 	}
-	if data[confKey.storagePool] != "default" {
+	if data[confKey.storagePool] != "" {
 		t.Error("Unexpected storage pool value")
 	}
 	if data[confKey.volumeName] != "sio-vol" {