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

Add AnnVolumeSnapshotBeingCreated #261

Merged
merged 10 commits into from
Apr 3, 2020
Merged

Add AnnVolumeSnapshotBeingCreated #261

Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
313507f
Add AnnVolumeSnapshotBeingCreated
xing-yang Feb 27, 2020
4c38cc8
Remove VolumeSnapshotBeingCreated annotation after driver response
xing-yang Mar 5, 2020
aa2bc2f
Remove the unnecessary driver name check
xing-yang Mar 5, 2020
7930dd2
Check error code DeadlineExceeded
xing-yang Mar 6, 2020
6ca7507
Address review comments
xing-yang Mar 8, 2020
0189c77
Check final error
xing-yang Mar 16, 2020
f32313b
checkandUpdateContentStatusOperation should not call createSnapshotOp…
xing-yang Apr 1, 2020
9f382ac
Address review comments
xing-yang Apr 2, 2020
2295e6a
Add error status check in createSnapshotWrapper
xing-yang Apr 3, 2020
f688d7b
Remove error status check
xing-yang Apr 3, 2020
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
79 changes: 40 additions & 39 deletions pkg/sidecar-controller/snapshot_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,11 @@ func (ctrl *csiSnapshotSideCarController) syncContent(content *crdv1.VolumeSnaps
// or ListSnapshots CSI methods over and over again for
// performance reasons.
if content.Status != nil && content.Status.ReadyToUse != nil && *content.Status.ReadyToUse == true {
// Try to remove AnnVolumeSnapshotBeingCreated if it is not removed yet for some reason
err := ctrl.removeAnnVolumeSnapshotBeingCreated(content)
if err != nil {
return fmt.Errorf("failed to remove VolumeSnapshotBeingCreated annotation from the content %s: %q", content.Name, err)
}
return nil
}
ctrl.checkandUpdateContentStatus(content)
Expand Down Expand Up @@ -128,10 +133,17 @@ func (ctrl *csiSnapshotSideCarController) createSnapshot(content *crdv1.VolumeSn
klog.V(5).Infof("createSnapshot for content [%s]: started", content.Name)
opName := fmt.Sprintf("create-%s", content.Name)
xing-yang marked this conversation as resolved.
Show resolved Hide resolved
ctrl.scheduleOperation(opName, func() error {
contentObj, err := ctrl.createSnapshotOperation(content)
// content.Status will be created for the first time after a snapshot
// is created by the CSI driver. If content.Status is not nil,
// we should update content status without creating snapshot again.
if content.Status != nil && content.Status.Error != nil && content.Status.Error.Message != nil && !isControllerUpdateFailError(content.Status.Error) {
klog.V(4).Infof("error is already set in snapshot, do not retry to create: %s", *content.Status.Error.Message)
return nil
}
contentObj, err := ctrl.createSnapshotWrapper(content)
if err != nil {
ctrl.updateContentErrorStatusWithEvent(content, v1.EventTypeWarning, "SnapshotCreationFailed", fmt.Sprintf("Failed to create snapshot: %v", err))
klog.Errorf("createSnapshot [%s]: error occurred in createSnapshotOperation: %v", opName, err)
klog.Errorf("createSnapshot [%s]: error occurred in createSnapshotWrapper: %v", opName, err)
return err
}

Expand Down Expand Up @@ -295,21 +307,6 @@ func (ctrl *csiSnapshotSideCarController) checkandUpdateContentStatusOperation(c
}
}

// The function goes through the snapshot creation process.
func (ctrl *csiSnapshotSideCarController) createSnapshotOperation(content *crdv1.VolumeSnapshotContent) (*crdv1.VolumeSnapshotContent, error) {
klog.Infof("createSnapshotOperation: Creating snapshot for content %s through the plugin ...", content.Name)

// content.Status will be created for the first time after a snapshot
// is created by the CSI driver. If content.Status is not nil,
// we should update content status without creating snapshot again.
if content.Status != nil && content.Status.Error != nil && content.Status.Error.Message != nil && !isControllerUpdateFailError(content.Status.Error) {
klog.V(4).Infof("error is already set in snapshot, do not retry to create: %s", *content.Status.Error.Message)
return content, nil
}

return ctrl.createSnapshotWrapper(content)
}

// This is a wrapper function for the snapshot creation process.
func (ctrl *csiSnapshotSideCarController) createSnapshotWrapper(content *crdv1.VolumeSnapshotContent) (*crdv1.VolumeSnapshotContent, error) {
klog.Infof("createSnapshotWrapper: Creating snapshot for content %s through the plugin ...", content.Name)
Expand Down Expand Up @@ -337,7 +334,7 @@ func (ctrl *csiSnapshotSideCarController) createSnapshotWrapper(content *crdv1.V
// If it is a final error, remove annotation to indicate
// storage system has responded with an error
klog.Infof("createSnapshotWrapper: CreateSnapshot for content %s returned error: %v", content.Name, err)
if isFinalError(err) {
if isCSIFinalError(err) {
err = ctrl.removeAnnVolumeSnapshotBeingCreated(content)
if err != nil {
return nil, fmt.Errorf("failed to remove VolumeSnapshotBeingCreated annotation from the content %s: %q", content.Name, err)
Expand All @@ -355,8 +352,8 @@ func (ctrl *csiSnapshotSideCarController) createSnapshotWrapper(content *crdv1.V

newContent, err := ctrl.updateSnapshotContentStatus(content, snapshotID, readyToUse, creationTime.UnixNano(), size)
if err != nil {
strerr := fmt.Sprintf("error updating volume snapshot content status for snapshot %s: %v.", content.Name, err)
klog.Error(strerr)
klog.Errorf("error updating status for volume snapshot content %s: %v.", content.Name, err)
return nil, fmt.Errorf("error updating status for volume snapshot content %s: %v.", content.Name, err)
} else {
content = newContent
}
Expand Down Expand Up @@ -607,34 +604,38 @@ func (ctrl *csiSnapshotSideCarController) shouldDelete(content *crdv1.VolumeSnap
// on VolumeSnapshotContent
// If set, it indicates snapshot is being created
func (ctrl *csiSnapshotSideCarController) setAnnVolumeSnapshotBeingCreated(content *crdv1.VolumeSnapshotContent) error {
// Set AnnVolumeSnapshotBeingCreated if it is not set yet
if !metav1.HasAnnotation(content.ObjectMeta, utils.AnnVolumeSnapshotBeingCreated) {
klog.V(5).Infof("setAnnVolumeSnapshotBeingCreated: set annotation [%s:yes] on content [%s].", utils.AnnVolumeSnapshotBeingCreated, content.Name)
contentClone := content.DeepCopy()
metav1.SetMetaDataAnnotation(&contentClone.ObjectMeta, utils.AnnVolumeSnapshotBeingCreated, "yes")
if metav1.HasAnnotation(content.ObjectMeta, utils.AnnVolumeSnapshotBeingCreated) {
// the annotation already exists, return directly
return nil
}

updatedContent, err := ctrl.clientset.SnapshotV1beta1().VolumeSnapshotContents().Update(contentClone)
if err != nil {
return newControllerUpdateError(content.Name, err.Error())
}
// update content if update is successful
content = updatedContent
// Set AnnVolumeSnapshotBeingCreated
klog.V(5).Infof("setAnnVolumeSnapshotBeingCreated: set annotation [%s:yes] on content [%s].", utils.AnnVolumeSnapshotBeingCreated, content.Name)
contentClone := content.DeepCopy()
metav1.SetMetaDataAnnotation(&contentClone.ObjectMeta, utils.AnnVolumeSnapshotBeingCreated, "yes")

_, err = ctrl.storeContentUpdate(content)
if err != nil {
klog.V(4).Infof("setAnnVolumeSnapshotBeingCreated for content [%s]: cannot update internal cache %v", content.Name, err)
return err
}
klog.V(5).Infof("setAnnVolumeSnapshotBeingCreated: volume snapshot content %+v", content)
updatedContent, err := ctrl.clientset.SnapshotV1beta1().VolumeSnapshotContents().Update(contentClone)
if err != nil {
return newControllerUpdateError(content.Name, err.Error())
}
// update content if update is successful
content = updatedContent

_, err = ctrl.storeContentUpdate(content)
if err != nil {
klog.V(4).Infof("setAnnVolumeSnapshotBeingCreated for content [%s]: cannot update internal cache %v", content.Name, err)
return err
xing-yang marked this conversation as resolved.
Show resolved Hide resolved
}
klog.V(5).Infof("setAnnVolumeSnapshotBeingCreated: volume snapshot content %+v", content)

return nil
}

// removeAnnVolumeSnapshotBeingCreated removes the VolumeSnapshotBeingCreated
// annotation from a content if there exists one.
func (ctrl csiSnapshotSideCarController) removeAnnVolumeSnapshotBeingCreated(content *crdv1.VolumeSnapshotContent) error {
if !metav1.HasAnnotation(content.ObjectMeta, utils.AnnVolumeSnapshotBeingCreated) {
// the annotation does not exit, return directly
// the annotation does not exist, return directly
return nil
}
contentClone := content.DeepCopy()
Expand All @@ -656,7 +657,7 @@ func (ctrl csiSnapshotSideCarController) removeAnnVolumeSnapshotBeingCreated(con
}

// This function checks if the error is final
func isFinalError(err error) bool {
func isCSIFinalError(err error) bool {
// Sources:
// https://github.com/grpc/grpc/blob/master/doc/statuscodes.md
// https://github.com/container-storage-interface/spec/blob/master/spec.md
Expand Down