Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed independent deletion of RO snapshot and UT #216

Merged
merged 2 commits into from
Aug 30, 2023
Merged

Fixed independent deletion of RO snapshot and UT #216

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
22fbfad
cleand logs
shefali-malhotra Aug 30, 2023
264f704
Merge branch 'main' into UT_fix
shefali-malhotra Aug 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 12 additions & 11 deletions common/utils/utils.go
View file
Open in desktop
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
Open in desktop
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