Make some changes to prepare for thin-opensds (#949)

Signed-off-by: leonwanghui <wanghui71leon@gmail.com>

contrib/drivers/scutech/cms/replication.go

@@ -72,8 +72,8 @@ func (r *ReplicationDriver) CreateReplication(opt *pb.CreateReplicationOpts) (*m
 	}
 
 	if _, err := cmsadm.Up(); err != nil {
-                return nil, err
-        }
+		return nil, err
+	}
 
 	return replica, nil
 }

contrib/drivers/scutech/cms/scmstask.go

@@ -19,24 +19,24 @@ import (
 )
 
 type CmsVolume struct {
-	VolumeId string
+	VolumeId   string
 	VolumeName string
 }
 
 type CmsTask struct {
-	bandwidth int64
-	cdpFlag bool
+	bandwidth   int64
+	cdpFlag     bool
 	taskVolumes map[string]string
-	volumeList map[string]CmsVolume
+	volumeList  map[string]CmsVolume
 }
 
 func NewCmsTask(bandwidth int64, cdpFlag bool) *CmsTask {
 	return &CmsTask{
-		bandwidth: bandwidth,
-		cdpFlag: cdpFlag,
+		bandwidth:   bandwidth,
+		cdpFlag:     cdpFlag,
 		taskVolumes: make(map[string]string),
-		volumeList: make(map[string]CmsVolume),
-    }
+		volumeList:  make(map[string]CmsVolume),
+	}
 }
 
 func checkVolume(c *CmsTask, volumeId string) bool {

contrib/drivers/scutech/cms/scmsutils.go

@@ -15,31 +15,30 @@
 package scms
 
 import (
+	"errors"
 	"os/exec"
 	"strconv"
-	"errors"
 )
 
 const CMS_ADM = "/opt/cmsagent/cmsadm"
 
 const (
 	CMS_CREATE = "--create"
 	CMS_DELETE = "--remove"
-	CMS_START = "--up"
-	CMS_STOP = "--down"
-	CMS_QUERY = "--query"
+	CMS_START  = "--up"
+	CMS_STOP   = "--down"
+	CMS_QUERY  = "--query"
 )
 
 type CmsAdm struct {
 }
 
-
-func NewCmsAdm () *CmsAdm {
+func NewCmsAdm() *CmsAdm {
 	return &CmsAdm{}
 }
 
 func (c *CmsAdm) CreateTask(t *CmsTask, arg ...string) ([]byte, error) {
-	var argv = []string {CMS_CREATE}
+	var argv = []string{CMS_CREATE}
 
 	var option string
 	option = "-b " + strconv.FormatInt(t.bandwidth, 10)
@@ -63,12 +62,12 @@ func (c *CmsAdm) CreateTask(t *CmsTask, arg ...string) ([]byte, error) {
 }
 
 func (c *CmsAdm) DeleteTask(arg ...string) ([]byte, error) {
-	var argv = []string {CMS_DELETE}
+	var argv = []string{CMS_DELETE}
 	return cmdExec(CMS_ADM, argv)
 }
 
 func (c *CmsAdm) Up() ([]byte, error) {
-	var argv = []string {CMS_START}
+	var argv = []string{CMS_START}
 	return cmdExec(CMS_ADM, argv)
 }
 
@@ -82,10 +81,10 @@ func (c *CmsAdm) Query() ([]byte, error) {
 	return cmdExec(CMS_ADM, argv)
 }
 
-func cmdExec(cmd string, argv[]string) ([]byte, error) {
+func cmdExec(cmd string, argv []string) ([]byte, error) {
 	out, err := exec.Command(cmd, argv[0:]...).Output()
 	if err != nil {
 		err = errors.New(string(out))
 	}
-	return out,err
+	return out, err
 }

pkg/api/controllers/fileshare.go

@@ -50,38 +50,45 @@ func (f *FileSharePortal) CreateFileShareAcl() {
 		return
 	}
 	ctx := c.GetContext(f.Ctx)
-	// Get profile
-	var prf *model.ProfileSpec
 	var fileshareacl = model.FileShareAclSpec{
 		BaseModel: &model.BaseModel{},
 	}
+
 	// Unmarshal the request body
 	if err := json.NewDecoder(f.Ctx.Request.Body).Decode(&fileshareacl); err != nil {
 		reason := fmt.Sprintf("parse fileshare access rules request body failed: %s", err.Error())
 		f.ErrorHandle(model.ErrorBadRequest, reason)
 		log.Error(reason)
 		return
 	}
-	result, err := util.CreateFileShareAclDBEntry(c.GetContext(f.Ctx), &fileshareacl)
+
+	fileshare, err := db.C.GetFileShare(ctx, fileshareacl.FileShareId)
 	if err != nil {
-		reason := fmt.Sprintf("createFileshareAcldbentry failed: %s", err.Error())
+		reason := fmt.Sprintf("getFileshare failed in create fileshare acl: %s", err.Error())
 		f.ErrorHandle(model.ErrorBadRequest, reason)
 		log.Error(reason)
 		return
 	}
-	fileshare, err := db.C.GetFileShare(ctx, result.FileShareId)
-	if err != nil {
-		reason := fmt.Sprintf("getFileshare failed in createfileshare acl: %s", err.Error())
-		f.ErrorHandle(model.ErrorBadRequest, reason)
-		log.Error(reason)
-		return
+	// If user doesn't specified profile, using profile derived from fileshare
+	if len(fileshareacl.ProfileId) == 0 {
+		log.Warning("User doesn't specified profile id, using profile derived from file share")
+		fileshareacl.ProfileId = fileshare.ProfileId
 	}
-	prf, err = db.C.GetProfile(ctx, fileshare.ProfileId)
+	prf, err := db.C.GetProfile(ctx, fileshareacl.ProfileId)
 	if err != nil {
 		errMsg := fmt.Sprintf("get profile failed: %s", err.Error())
 		f.ErrorHandle(model.ErrorBadRequest, errMsg)
 		return
 	}
+
+	result, err := util.CreateFileShareAclDBEntry(c.GetContext(f.Ctx), &fileshareacl)
+	if err != nil {
+		reason := fmt.Sprintf("createFileshareAcldbentry failed: %s", err.Error())
+		f.ErrorHandle(model.ErrorBadRequest, reason)
+		log.Error(reason)
+		return
+	}
+
 	// Marshal the result.
 	body, err := json.Marshal(result)
 	if err != nil {

pkg/api/controllers/profile.go

@@ -64,7 +64,7 @@ func (p *ProfilePortal) CreateProfile() {
 				profile.ProvisioningProperties.DataStorage.StorageAccessCapability = []string{"Read", "Write", "Execute"}
 			}
 		}
-		if io := pp.IOConnectivity; io.IsEmpty(){
+		if io := pp.IOConnectivity; io.IsEmpty() {
 			if io.AccessProtocol == "" {
 				profile.ProvisioningProperties.IOConnectivity.AccessProtocol = "nfs"
 			}

pkg/api/controllers/volume.go

@@ -69,6 +69,8 @@ func (v *VolumePortal) CreateVolume() {
 	if volume.ProfileId == "" {
 		log.Warning("Use default profile when user doesn't specify profile.")
 		prf, err = db.C.GetDefaultProfile(ctx)
+		// Assign the default profile id to volume so that users can know which
+		// profile is used for creating a volume.
 		volume.ProfileId = prf.Id
 	} else {
 		prf, err = db.C.GetProfile(ctx, volume.ProfileId)
@@ -600,22 +602,17 @@ func (v *VolumeSnapshotPortal) CreateVolumeSnapshot() {
 		return
 	}
 
-	// get profile
-	var prf *model.ProfileSpec
-	var err error
-
-	// If user doesn't specified profile, using profile derived form volume
+	// If user doesn't specified profile, using profile derived from volume
 	if len(snapshot.ProfileId) == 0 {
-		log.Warning("User doesn't specified profile id, using profile derived form volume")
+		log.Warning("User doesn't specified profile id, using profile derived from volume")
 		vol, err := db.C.GetVolume(ctx, snapshot.VolumeId)
 		if err != nil {
 			v.ErrorHandle(model.ErrorBadRequest, err.Error())
 			return
 		}
 		snapshot.ProfileId = vol.ProfileId
 	}
-	prf, err = db.C.GetProfile(ctx, snapshot.ProfileId)
-
+	prf, err := db.C.GetProfile(ctx, snapshot.ProfileId)
 	if err != nil {
 		errMsg := fmt.Sprintf("get profile failed: %s", err.Error())
 		v.ErrorHandle(model.ErrorBadRequest, errMsg)

pkg/api/util/db.go

@@ -39,13 +39,19 @@ func CreateFileShareAclDBEntry(ctx *c.Context, in *model.FileShareAclSpec) (*mod
 	if in.Id == "" {
 		in.Id = uuid.NewV4().String()
 	}
-
 	if in.CreatedAt == "" {
 		in.CreatedAt = time.Now().Format(constants.TimeFormat)
 	}
 	if in.UpdatedAt == "" {
 		in.UpdatedAt = time.Now().Format(constants.TimeFormat)
 	}
+
+	// validate profileId
+	if in.ProfileId == "" {
+		errMsg := "profile id can not be empty when creating fileshare acl in db!"
+		log.Error(errMsg)
+		return nil, errors.New(errMsg)
+	}
 	// validate type
 	if in.Type != "ip" {
 		errMsg := fmt.Sprintf("invalid fileshare type: %v. Supported type is: ip", in.Type)
@@ -96,7 +102,6 @@ func CreateFileShareAclDBEntry(ctx *c.Context, in *model.FileShareAclSpec) (*mod
 		log.Error("file shareid is not valid: ", err)
 		return nil, err
 	}
-
 	if fileshare.Status != model.FileShareAvailable {
 		var errMsg = "only the status of file share is available, the acl can be created"
 		log.Error(errMsg)
@@ -125,6 +130,12 @@ func CreateFileShareDBEntry(ctx *c.Context, in *model.FileShareSpec) (*model.Fil
 	if in.Id == "" {
 		in.Id = uuid.NewV4().String()
 	}
+	// validate profileId
+	if in.ProfileId == "" {
+		errMsg := "profile id can not be empty when creating fileshare in db!"
+		log.Error(errMsg)
+		return nil, errors.New(errMsg)
+	}
 	// validate the size
 	if in.Size <= 0 {
 		errMsg := fmt.Sprintf("invalid fileshare size: %d", in.Size)
@@ -209,6 +220,12 @@ func CreateFileShareSnapshotDBEntry(ctx *c.Context, in *model.FileShareSnapshotS
 		log.Error(errMsg)
 		return nil, errors.New(errMsg)
 	}
+	// validate profileId
+	if in.ProfileId == "" {
+		errMsg := "profile id can not be empty when creating fileshare snapshot in db!"
+		log.Error(errMsg)
+		return nil, errors.New(errMsg)
+	}
 
 	// Check existence of fileshare snapshot name #931
 	filesnaps, err := db.C.ListFileShareSnapshots(ctx)
@@ -286,6 +303,11 @@ func CreateVolumeDBEntry(ctx *c.Context, in *model.VolumeSpec) (*model.VolumeSpe
 		log.Error(errMsg)
 		return nil, errors.New(errMsg)
 	}
+	if in.ProfileId == "" {
+		errMsg := "profile id can not be empty when creating volume in db"
+		log.Error(errMsg)
+		return nil, errors.New(errMsg)
+	}
 	if in.SnapshotId != "" {
 		snap, err := db.C.GetVolumeSnapshot(ctx, in.SnapshotId)
 		if err != nil {
@@ -459,13 +481,18 @@ func DeleteVolumeAttachmentDBEntry(ctx *c.Context, in *model.VolumeAttachmentSpe
 // to be creating in the DB, the real operation would be executed in another new
 // thread.
 func CreateVolumeSnapshotDBEntry(ctx *c.Context, in *model.VolumeSnapshotSpec) (*model.VolumeSnapshotSpec, error) {
+	if in.ProfileId == "" {
+		errMsg := "profile id can not be empty when creating volume snapshot in db"
+		log.Error(errMsg)
+		return nil, errors.New(errMsg)
+	}
 	vol, err := db.C.GetVolume(ctx, in.VolumeId)
 	if err != nil {
 		log.Error("get volume failed in create volume snapshot method: ", err)
 		return nil, err
 	}
 	if vol.Status != model.VolumeAvailable && vol.Status != model.VolumeInUse {
-		var errMsg = "only the status of volume is available or in-use, the snapshot can be created"
+		errMsg := "only the status of volume is available or in-use, the snapshot can be created"
 		log.Error(errMsg)
 		return nil, errors.New(errMsg)
 	}
@@ -478,6 +505,7 @@ func CreateVolumeSnapshotDBEntry(ctx *c.Context, in *model.VolumeSnapshotSpec) (
 	}
 
 	in.Status = model.VolumeSnapCreating
+	in.Metadata = utils.MergeStringMaps(in.Metadata, vol.Metadata)
 	return db.C.CreateVolumeSnapshot(ctx, in)
 }
 

pkg/api/util/db_test.go

@@ -39,6 +39,7 @@ func TestCreateVolumeDBEntry(t *testing.T) {
 		Name:        "volume sample",
 		Description: "This is a sample volume for testing",
 		Size:        int64(1),
+		ProfileId:   "3769855c-a102-11e7-b772-17b880d2f537",
 		Status:      model.VolumeCreating,
 	}
 
@@ -65,6 +66,17 @@ func TestCreateVolumeDBEntry(t *testing.T) {
 		expectedError := fmt.Sprintf("invalid volume size: %d", in.Size)
 		assertTestResult(t, err.Error(), expectedError)
 	})
+
+	t.Run("The profile id should not be empty", func(t *testing.T) {
+		in.Size, in.ProfileId = int64(1), ""
+		mockClient := new(dbtest.Client)
+		mockClient.On("CreateVolume", context.NewAdminContext(), in).Return(&SampleVolumes[0], nil)
+		db.C = mockClient
+
+		_, err := CreateVolumeDBEntry(context.NewAdminContext(), in)
+		expectedError := "profile id can not be empty when creating volume in db"
+		assertTestResult(t, err.Error(), expectedError)
+	})
 }
 
 func TestCreateVolumeFromSnapshotDBEntry(t *testing.T) {
@@ -73,6 +85,7 @@ func TestCreateVolumeFromSnapshotDBEntry(t *testing.T) {
 		Name:        "volume sample",
 		Description: "This is a sample volume for testing",
 		Size:        int64(1),
+		ProfileId:   "3769855c-a102-11e7-b772-17b880d2f537",
 		Status:      model.VolumeCreating,
 		SnapshotId:  "3769855c-a102-11e7-b772-17b880d2f537",
 	}
@@ -361,6 +374,7 @@ func TestCreateVolumeSnapshotDBEntry(t *testing.T) {
 		Name:        "sample-snapshot-01",
 		Description: "This is the first sample snapshot for testing",
 		Size:        int64(1),
+		ProfileId:   "3769855c-a102-11e7-b772-17b880d2f537",
 		Status:      "creating",
 		Metadata:    map[string]string{"a": "a"},
 	}
@@ -378,6 +392,18 @@ func TestCreateVolumeSnapshotDBEntry(t *testing.T) {
 		}
 		assertTestResult(t, result, expected)
 	})
+
+	t.Run("The profile id should not be empty", func(t *testing.T) {
+		req.ProfileId = ""
+		mockClient := new(dbtest.Client)
+		mockClient.On("GetVolume", context.NewAdminContext(), "bd5b12a8-a101-11e7-941e-d77981b584d8").Return(vol, nil)
+		mockClient.On("CreateVolumeSnapshot", context.NewAdminContext(), req).Return(&SampleSnapshots[0], nil)
+		db.C = mockClient
+
+		_, err := CreateVolumeSnapshotDBEntry(context.NewAdminContext(), req)
+		expectedError := "profile id can not be empty when creating volume snapshot in db"
+		assertTestResult(t, err.Error(), expectedError)
+	})
 }
 
 func TestDeleteVolumeSnapshotDBEntry(t *testing.T) {

pkg/controller/fileshare/filesharecontroller_test.go

@@ -226,3 +226,11 @@ func (fc *fakefileshareClient) AttachVolume(ctx context.Context, in *pb.AttachVo
 func (fc *fakefileshareClient) DetachVolume(ctx context.Context, in *pb.DetachVolumeOpts, opts ...grpc.CallOption) (*pb.GenericResponse, error) {
 	return nil, nil
 }
+
+func (fc *fakefileshareClient) GetMetrics(ctx context.Context, in *pb.GetMetricsOpts, opts ...grpc.CallOption) (*pb.GenericResponse, error) {
+	return nil, nil
+}
+
+func (fc *fakefileshareClient) GetUrls(ctx context.Context, in *pb.NoParams, opts ...grpc.CallOption) (*pb.GenericResponse, error) {
+	return nil, nil
+}

pkg/controller/volume/volumecontroller_test.go

@@ -371,3 +371,11 @@ func (fc *fakeClient) CreateFileShareSnapshot(ctx context.Context, in *pb.Create
 func (fc *fakeClient) DeleteFileShareSnapshot(ctx context.Context, in *pb.DeleteFileShareSnapshotOpts, opts ...grpc.CallOption) (*pb.GenericResponse, error) {
 	return nil, nil
 }
+
+func (fc *fakeClient) GetMetrics(ctx context.Context, in *pb.GetMetricsOpts, opts ...grpc.CallOption) (*pb.GenericResponse, error) {
+	return nil, nil
+}
+
+func (fc *fakeClient) GetUrls(ctx context.Context, in *pb.NoParams, opts ...grpc.CallOption) (*pb.GenericResponse, error) {
+	return nil, nil
+}

pkg/db/drivers/etcd/etcd.go

@@ -2287,7 +2287,7 @@ func (c *Client) UpdateVolumeAttachment(ctx *c.Context, attachmentId string, att
 		result.Metadata = utils.MergeStringMaps(result.Metadata, attachment.Metadata)
 	}
 	// Update DriverVolumeType
-	if len(attachment.DriverVolumeType) > 0{
+	if len(attachment.DriverVolumeType) > 0 {
 		result.DriverVolumeType = attachment.DriverVolumeType
 	}
 	// Update connectionData