Skip to content

Commit

Permalink
address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
RaunakShah committed Oct 25, 2023
1 parent c2d5e4c commit 49306f3
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 11 deletions.
9 changes: 4 additions & 5 deletions pkg/common-controller/groupsnapshot_controller_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ func (ctrl *csiSnapshotCommonController) syncGroupSnapshot(groupSnapshot *crdv1a

klog.V(5).Infof("syncGroupSnapshot [%s]: check if we should remove finalizer on group snapshot PVC source and remove it if we can", utils.GroupSnapshotKey(groupSnapshot))

// Proceed with snapshot deletion and remove finalizers when needed
// Proceed with group snapshot deletion and remove finalizers when needed
if groupSnapshot.ObjectMeta.DeletionTimestamp != nil {
return ctrl.processGroupSnapshotWithDeletionTimestamp(groupSnapshot)
}
Expand All @@ -305,7 +305,7 @@ func (ctrl *csiSnapshotCommonController) syncGroupSnapshot(groupSnapshot *crdv1a

klog.V(5).Infof("syncGroupSnapshot: check if we should add finalizers on group snapshot [%s]", utils.GroupSnapshotKey(groupSnapshot))
if err := ctrl.checkandAddGroupSnapshotFinalizers(groupSnapshot); err != nil {
klog.Errorf("error ccheckandAddGroupSnapshotFinalizers for group snapshot [%s]: %v", utils.GroupSnapshotKey(groupSnapshot), err)
klog.Errorf("error checkandAddGroupSnapshotFinalizers for group snapshot [%s]: %v", utils.GroupSnapshotKey(groupSnapshot), err)
ctrl.eventRecorder.Event(groupSnapshot, v1.EventTypeWarning, "GroupSnapshotFinalizerError", fmt.Sprintf("Failed to check and update group snapshot: %s", err.Error()))
return err
}
Expand Down Expand Up @@ -983,7 +983,7 @@ func (ctrl *csiSnapshotCommonController) addGroupSnapshotContentFinalizer(groupS
return nil
}

// checkandAddSnapshotFinalizers checks and adds snapshot finailzers when needed
// checkandAddGroupSnapshotFinalizers checks and adds group snapshot finailzers when needed
func (ctrl *csiSnapshotCommonController) checkandAddGroupSnapshotFinalizers(groupSnapshot *crdv1alpha1.VolumeGroupSnapshot) error {
// get the group snapshot content for this group snapshot
var (
Expand Down Expand Up @@ -1117,7 +1117,6 @@ func (ctrl *csiSnapshotCommonController) checkandRemoveGroupSnapshotFinalizersAn
return nil
}

// TODO: In this needed?
// check if an individual snapshot belonging to the group snapshot is being
// used for restore a PVC
// If yes, do nothing and wait until PVC restoration finishes
Expand All @@ -1127,7 +1126,7 @@ func (ctrl *csiSnapshotCommonController) checkandRemoveGroupSnapshotFinalizersAn
return err
}
if ctrl.isVolumeBeingCreatedFromSnapshot(snapshot) {
klog.V(4).Infof("checkandRemoveGroupSnapshotFinalizersAndCheckandDeleteGroupSnapshotContent[%s]: snapshot [%s] belonging to volume group snapshot is being used to restore a PVC", utils.GroupSnapshotKey(groupSnapshot), utils.SnapshotKey(snapshot))
klog.V(4).Infof("checkandRemoveGroupSnapshotFinalizersAndCheckandDeleteGroupSnapshotContent[%s]: snapshot belonging to volume group snapshot [%s] is being used to restore a PVC", utils.GroupSnapshotKey(groupSnapshot), utils.SnapshotKey(snapshot))
ctrl.eventRecorder.Event(snapshot, v1.EventTypeWarning, "SnapshotDeletePending", "Snapshot is being used to restore a PVC")
// TODO(@xiangqian): should requeue this?
return nil
Expand Down
6 changes: 1 addition & 5 deletions pkg/sidecar-controller/csi_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ type Handler interface {
GetSnapshotStatus(content *crdv1.VolumeSnapshotContent, snapshotterListCredentials map[string]string) (bool, time.Time, int64, error)
CreateGroupSnapshot(content *crdv1alpha1.VolumeGroupSnapshotContent, volumeIDs []string, parameters map[string]string, snapshotterCredentials map[string]string) (string, string, []*csi.Snapshot, time.Time, bool, error)
GetGroupSnapshotStatus(content *crdv1alpha1.VolumeGroupSnapshotContent, snapshotterListCredentials map[string]string) (bool, time.Time, error)
DeleteGroupSnapshot(content *crdv1alpha1.VolumeGroupSnapshotContent, SanpshotID []string, snapshotterCredentials map[string]string) error
DeleteGroupSnapshot(content *crdv1alpha1.VolumeGroupSnapshotContent, SnapshotID []string, snapshotterCredentials map[string]string) error
}

// csiHandler is a handler that calls CSI to create/delete volume snapshot.
Expand Down Expand Up @@ -171,10 +171,6 @@ func (handler *csiHandler) DeleteGroupSnapshot(content *crdv1alpha1.VolumeGroupS
ctx, cancel := context.WithTimeout(context.Background(), handler.timeout)
defer cancel()

if content.Spec.VolumeGroupSnapshotRef.UID == "" {
return fmt.Errorf("cannot delete group snapshot. Group snapshot content %s not bound to a group snapshot", content.Name)
}

if len(snapshotIDs) == 0 {
return fmt.Errorf("cannot delete group snapshot. No snapshots found in the group snapshot", content.Name)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/utils/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ const (
PVCFinalizer = "snapshot.storage.kubernetes.io/pvc-as-source-protection"
// Name of finalizer on VolumeGroupSnapshotContents that are bound by VolumeGroupSnapshots
VolumeGroupSnapshotContentFinalizer = "groupsnapshot.storage.kubernetes.io/volumegroupsnapshotcontent-bound-protection"
// Name of finalizer on VolumeGroupSnapshots that iare bound to VolumeGroupSnapshotContents
// Name of finalizer on VolumeGroupSnapshots that are bound to VolumeGroupSnapshotContents
VolumeGroupSnapshotBoundFinalizer = "groupsnapshot.storage.kubernetes.io/volumegroupsnapshot-bound-protection"

IsDefaultSnapshotClassAnnotation = "snapshot.storage.kubernetes.io/is-default-class"
Expand Down

0 comments on commit 49306f3

Please sign in to comment.