Skip to content

Commit

Permalink
Merge pull request #216 from dell/UT_fix
Browse files Browse the repository at this point in the history 
Fixed independent deletion of RO snapshot and UT
  • Loading branch information
@shefali-malhotra
shefali-malhotra authored Aug 30, 2023
2 parents 998de28 + 264f704 commit 066e908
Show file tree
Hide file tree
Showing 7 changed files with 719 additions and 702 deletions.
23 changes: 12 additions & 11 deletions common/utils/utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -367,39 +367,40 @@ func ParseNormalizedVolumeID(ctx context.Context, volID string) (string, int, st
return volumeName, exportID, accessZone, clusterName, nil
}

// GetNormalizedSnapshotID combines snapshotID ID and cluster name to form the normalized snapshot ID
// e.g. 12345 + cluster1 => 12345=_=_=cluster1
func GetNormalizedSnapshotID(ctx context.Context, snapshotID, clusterName string) string {
// GetNormalizedSnapshotID combines snapshotID ID and cluster name and access zone to form the normalized snapshot ID
// e.g. 12345 + cluster1 + accessZone => 12345=_=_=cluster1=_=_=zone1
func GetNormalizedSnapshotID(ctx context.Context, snapshotID, clusterName, accessZone string) string {
log := GetRunIDLogger(ctx)

snapID := fmt.Sprintf("%s%s%s", snapshotID, SnapshotIDSeparator, clusterName)
snapID := fmt.Sprintf("%s%s%s%s%s", snapshotID, SnapshotIDSeparator, clusterName, SnapshotIDSeparator, accessZone)

log.Debugf("combined snapshot id '%s' and cluster name '%s' to form normalized snapshot ID '%s'",
snapshotID, clusterName, snapID)
log.Debugf("combined snapshot id '%s' access zone '%s' and cluster name '%s' to form normalized snapshot ID '%s'",
snapshotID, accessZone, clusterName, snapID)

return snapID
}

// ParseNormalizedSnapshotID parses the normalized snapshot ID(using SnapshotIDSeparator) to extract the snapshot ID and cluster name(optional) that make up the normalized snapshot ID
// e.g. 12345 => 12345, ""
// e.g. 12345=_=_=cluster1 => 12345, cluster1
func ParseNormalizedSnapshotID(ctx context.Context, snapID string) (string, string, error) {
// e.g. 12345=_=_=cluster1=_=_=zone => 12345, cluster1, zone
func ParseNormalizedSnapshotID(ctx context.Context, snapID string) (string, string, string, error) {
log := GetRunIDLogger(ctx)
tokens := strings.Split(snapID, SnapshotIDSeparator)
if len(tokens) < 1 {
return "", "", fmt.Errorf("snapshot ID '%s' cannot be split into tokens", snapID)
return "", "", "", fmt.Errorf("snapshot ID '%s' cannot be split into tokens", snapID)
}

snapshotID := tokens[0]
var clusterName string
var clusterName, accessZone string
if len(tokens) > 1 {
clusterName = tokens[1]
accessZone = tokens[2]
}

log.Debugf("normalized snapshot ID '%s' parsed into snapshot ID '%s' and cluster name '%s'",
snapID, snapshotID, clusterName)

return snapshotID, clusterName, nil
return snapshotID, clusterName, accessZone, nil
}

// ParseNodeID parses NodeID to node name, node FQDN and IP address using pattern '^(.+)=#=#=(.+)=#=#=(.+)'
Expand Down
43 changes: 15 additions & 28 deletions service/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,7 @@ func (s *service) CreateVolume(
normalizedSnapshotID := snapshot.GetSnapshotId()
// parse the input snapshot id and fetch it's components
var snapshotSrcClusterName string
sourceSnapshotID, snapshotSrcClusterName, err = utils.ParseNormalizedSnapshotID(ctx, normalizedSnapshotID)
sourceSnapshotID, snapshotSrcClusterName, _, err = utils.ParseNormalizedSnapshotID(ctx, normalizedSnapshotID)
if err != nil {
return nil, status.Error(codes.InvalidArgument, utils.GetMessageWithRunID(runID, "failed to parse snapshot ID '%s', error : '%v'", normalizedSnapshotID, err))
}
Expand Down Expand Up @@ -667,7 +667,7 @@ func (s *service) createVolumeFromSnapshot(ctx context.Context, isiConfig *Isilo
var err error

// parse the input snapshot id and fetch it's components
srcSnapshotID, _, err := utils.ParseNormalizedSnapshotID(ctx, normalizedSnapshotID)
srcSnapshotID, _, _, err := utils.ParseNormalizedSnapshotID(ctx, normalizedSnapshotID)
if err != nil {
return err
}
Expand Down Expand Up @@ -1107,7 +1107,6 @@ func (s *service) ControllerPublishVolume(
if exportPath = volumeContext[ExportPathParam]; exportPath == "" {
exportPath = utils.GetPathForVolume(isiConfig.IsiPath, volName)
}

isROVolumeFromSnapshot = isiConfig.isiSvc.isROVolumeFromSnapshot(volumeContext["Path"], accessZone)

if isROVolumeFromSnapshot {
Expand Down Expand Up @@ -1600,7 +1599,7 @@ func (s *service) CreateSnapshot(

log.Infof("CreateSnapshot started")
// parse the input volume id and fetch it's components
_, _, _, clusterName, err := utils.ParseNormalizedVolumeID(ctx, req.GetSourceVolumeId())
_, _, accessZone, clusterName, err := utils.ParseNormalizedVolumeID(ctx, req.GetSourceVolumeId())
if err != nil {
return nil, status.Error(codes.NotFound, utils.GetMessageWithRunID(runID, err.Error()))
}
Expand All @@ -1626,7 +1625,6 @@ func (s *service) CreateSnapshot(
snapshotNew isi.Snapshot
params map[string]string
isiPath string
accessZone string
)
params = req.GetParameters()
if _, ok := params[IsiPathParam]; ok {
Expand All @@ -1639,29 +1637,20 @@ func (s *service) CreateSnapshot(
// use the default isiPath if not set in the storage class
isiPath = isiConfig.IsiPath
}
if _, ok := params[AccessZoneParam]; ok {
if params[AccessZoneParam] == "" {
accessZone = s.opts.AccessZone
} else {
accessZone = params[AccessZoneParam]
}
} else {
// use the default access zone if not set in the storage class
accessZone = s.opts.AccessZone
}

srcVolumeID, snapshotName, err := s.validateCreateSnapshotRequest(ctx, req, isiPath, isiConfig)
if err != nil {
return nil, status.Error(codes.InvalidArgument, utils.GetMessageWithRunID(runID, err.Error()))
}

log.Infof("snapshot name is '%s' and source volume ID is '%s' ", snapshotName, srcVolumeID)
log.Infof("snapshot name is '%s' and source volume ID is '%s' access Zone is '%s'", snapshotName, srcVolumeID, accessZone)
// check if snapshot already exists
var snapshotByName isi.Snapshot
log.Infof("check for existence of snapshot '%s'", snapshotName)
if snapshotByName, err = isiConfig.isiSvc.GetSnapshot(ctx, snapshotName); snapshotByName != nil {
if path.Base(snapshotByName.Path) == srcVolumeID {
// return the existent snapshot
return s.getCreateSnapshotResponse(ctx, strconv.FormatInt(snapshotByName.Id, 10), req.GetSourceVolumeId(), snapshotByName.Created, isiConfig.isiSvc.GetSnapshotSize(ctx, isiPath, snapshotName, accessZone), clusterName), nil
return s.getCreateSnapshotResponse(ctx, strconv.FormatInt(snapshotByName.Id, 10), req.GetSourceVolumeId(), snapshotByName.Created, isiConfig.isiSvc.GetSnapshotSize(ctx, isiPath, snapshotName, accessZone), clusterName, accessZone), nil
}
// return already exists error
return nil, status.Error(codes.AlreadyExists,
Expand All @@ -1678,7 +1667,7 @@ func (s *service) CreateSnapshot(

log.Infof("snapshot creation is successful")
// return the response
return s.getCreateSnapshotResponse(ctx, strconv.FormatInt(snapshotNew.Id, 10), req.GetSourceVolumeId(), snapshotNew.Created, isiConfig.isiSvc.GetSnapshotSize(ctx, isiPath, snapshotName, accessZone), clusterName), nil
return s.getCreateSnapshotResponse(ctx, strconv.FormatInt(snapshotNew.Id, 10), req.GetSourceVolumeId(), snapshotNew.Created, isiConfig.isiSvc.GetSnapshotSize(ctx, isiPath, snapshotName, accessZone), clusterName, accessZone), nil
}

// validateCreateSnapshotRequest validate the input params in CreateSnapshotRequest
Expand Down Expand Up @@ -1711,8 +1700,8 @@ func (s *service) validateCreateSnapshotRequest(
return srcVolumeID, snapshotName, nil
}

func (s *service) getCreateSnapshotResponse(ctx context.Context, snapshotID string, sourceVolumeID string, creationTime, sizeInBytes int64, clusterName string) *csi.CreateSnapshotResponse {
snapID := utils.GetNormalizedSnapshotID(ctx, snapshotID, clusterName)
func (s *service) getCreateSnapshotResponse(ctx context.Context, snapshotID string, sourceVolumeID string, creationTime, sizeInBytes int64, clusterName string, accessZone string) *csi.CreateSnapshotResponse {
snapID := utils.GetNormalizedSnapshotID(ctx, snapshotID, clusterName, accessZone)
return &csi.CreateSnapshotResponse{
Snapshot: s.getCSISnapshot(snapID, sourceVolumeID, creationTime, sizeInBytes),
}
Expand Down Expand Up @@ -1745,13 +1734,11 @@ func (s *service) DeleteSnapshot(
if req.GetSnapshotId() == "" {
return nil, status.Errorf(codes.FailedPrecondition, utils.GetMessageWithRunID(runID, "snapshot id to be deleted is required"))
}

// parse the input snapshot id and fetch it's components
snapshotID, clusterName, err := utils.ParseNormalizedSnapshotID(ctx, req.GetSnapshotId())
snapshotID, clusterName, accessZone, err := utils.ParseNormalizedSnapshotID(ctx, req.GetSnapshotId())
if err != nil {
return nil, status.Error(codes.InvalidArgument, fmt.Sprintf("failed to parse snapshot ID '%s', error : '%v'", req.GetSnapshotId(), err))
}

isiConfig, err := s.getIsilonConfig(ctx, &clusterName)
if err != nil {
log.Error("Failed to get Isilon config with error ", err.Error())
Expand Down Expand Up @@ -1791,8 +1778,7 @@ func (s *service) DeleteSnapshot(
return nil, status.Errorf(codes.Internal, utils.GetMessageWithRunID(runID, "cannot check the existence of the snapshot: '%s'", err.Error()))
}
}
//Reading access zone param
accessZone := isiConfig.accessZone

// Get snapshot path
snapshotSourceVolumeIsiPath, _ := isiConfig.isiSvc.GetSnapshotSourceVolumeIsiPath(ctx, snapshotID)
log.Infof("Snapshot source volume isiPath is '%s'", snapshotSourceVolumeIsiPath)
Expand All @@ -1812,7 +1798,7 @@ func (s *service) DeleteSnapshot(
// Check if there are any RO volumes created from this snapshot
// Note: This is true only for RO volumes from snapshots
if export != nil {
if err := s.processSnapshotTrackingDirectoryDuringDeleteSnapshot(ctx, export, snapshotIsiPath, &deleteSnapshot, isiConfig); err != nil {
if err := s.processSnapshotTrackingDirectoryDuringDeleteSnapshot(ctx, export, snapshotIsiPath, accessZone, &deleteSnapshot, isiConfig); err != nil {
log.Error("Failed to get RO volume from snapshot ", err.Error())
return nil, err
}
Expand All @@ -1831,15 +1817,16 @@ func (s *service) DeleteSnapshot(
func (s *service) processSnapshotTrackingDirectoryDuringDeleteSnapshot(
ctx context.Context,
export isi.Export,
snapshotIsiPath string,
snapshotIsiPath,
accessZone string,
deleteSnapshot *bool,
isiConfig *IsilonClusterConfig) error {

// Fetch log handler
ctx, log, _ := GetRunIDLog(ctx)

//get Zone details
zone, err := isiConfig.isiSvc.GetZoneByName(ctx, isiConfig.accessZone)
zone, err := isiConfig.isiSvc.GetZoneByName(ctx, accessZone)
if err != nil {
return err
}
Expand Down
Loading

0 comments on commit 066e908

Please sign in to comment.