Skip to content

Commit

Permalink
disk/snapshot: don't describe before create
Browse files Browse the repository at this point in the history
We have client token now, and we will not create multiple snapshots for one VS object.

And we still always describe snapshot at least once, so we sill have the latest info about it.
  • Loading branch information
huww98 committed Nov 22, 2024
1 parent a0cc17e commit 26a8d22
Show file tree
Hide file tree
Showing 2 changed files with 0 additions and 56 deletions.
19 changes: 0 additions & 19 deletions pkg/disk/cloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -742,25 +742,6 @@ func findDiskByName(name string, ecsClient cloud.ECSInterface) (*ecs.Disk, error
}
return &disks[0], err
}

func findSnapshotByName(name string) (*ecs.DescribeSnapshotsResponse, int, error) {
describeSnapShotRequest := ecs.CreateDescribeSnapshotsRequest()
describeSnapShotRequest.RegionId = GlobalConfigVar.Region
describeSnapShotRequest.SnapshotName = name
snapshots, err := GlobalConfigVar.EcsClient.DescribeSnapshots(describeSnapShotRequest)
if err != nil {
return nil, 0, err
}
if len(snapshots.Snapshots.Snapshot) == 0 {
return snapshots, 0, nil
}

if len(snapshots.Snapshots.Snapshot) > 1 {
return snapshots, len(snapshots.Snapshots.Snapshot), status.Error(codes.Internal, "find more than one snapshot with name "+name)
}
return snapshots, 1, nil
}

func findDiskSnapshotByID(id string) (*ecs.Snapshot, error) {
describeSnapShotRequest := ecs.CreateDescribeSnapshotsRequest()
describeSnapShotRequest.RegionId = GlobalConfigVar.Region
Expand Down
37 changes: 0 additions & 37 deletions pkg/disk/controllerserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -564,43 +564,6 @@ func (cs *controllerServer) CreateSnapshot(ctx context.Context, req *csi.CreateS

klog.Infof("CreateSnapshot:: Starting to create snapshot: %+v", req)
sourceVolumeID := strings.Trim(req.GetSourceVolumeId(), " ")
// Need to check for already existing snapshot name
GlobalConfigVar.EcsClient = updateEcsClient(GlobalConfigVar.EcsClient)
snapshots, snapNum, err := findSnapshotByName(req.GetName())
switch {
case snapNum == 1:
// Since err is nil, it means the snapshot with the same name already exists need
// to check if the sourceVolumeId of existing snapshot is the same as in new request.
existsSnapshot := snapshots.Snapshots.Snapshot[0]
if existsSnapshot.SourceDiskId == req.GetSourceVolumeId() {
csiSnapshot, err := formatCSISnapshot(&existsSnapshot)
if err != nil {
return nil, err
}
klog.Infof("CreateSnapshot:: Snapshot already created: name[%s], sourceId[%s], status[%v]", req.Name, req.GetSourceVolumeId(), csiSnapshot.ReadyToUse)
if csiSnapshot.ReadyToUse {
str := fmt.Sprintf("VolumeSnapshot: name: %s, id: %s is ready to use.", existsSnapshot.SnapshotName, existsSnapshot.SnapshotId)
utils.CreateEvent(cs.recorder, ref, v1.EventTypeNormal, snapshotCreatedSuccessfully, str)
}
return &csi.CreateSnapshotResponse{
Snapshot: csiSnapshot,
}, nil
}
klog.Errorf("CreateSnapshot:: Snapshot already exist with same name: name[%s], volumeID[%s]", req.Name, existsSnapshot.SourceDiskId)
err := status.Errorf(codes.AlreadyExists, "snapshot with the same name: %s but with different SourceVolumeId already exist", req.GetName())
utils.CreateEvent(cs.recorder, ref, v1.EventTypeWarning, snapshotAlreadyExist, err.Error())
return nil, err
case snapNum > 1:
klog.Errorf("CreateSnapshot:: Find Snapshot name[%s], but get more than 1 instance", req.Name)
err := status.Error(codes.Internal, "CreateSnapshot: get snapshot more than 1 instance")
utils.CreateEvent(cs.recorder, ref, v1.EventTypeWarning, snapshotTooMany, err.Error())
return nil, err
case err != nil:
klog.Errorf("CreateSnapshot:: Expect to find Snapshot name[%s], but get error: %v", req.Name, err)
e := status.Errorf(codes.Internal, "CreateSnapshot: get snapshot with error: %s", err.Error())
utils.CreateEvent(cs.recorder, ref, v1.EventTypeWarning, snapshotCreateError, e.Error())
return nil, e
}

ecsClient, err := getEcsClientByID(sourceVolumeID, "")
if err != nil {
Expand Down

0 comments on commit 26a8d22

Please sign in to comment.