Skip to content

Commit

Permalink
Minor enhancements to CSI Snapshot (#5396)
Browse files Browse the repository at this point in the history
### Overview
1. Handle errors that happen while waiting for snapshot creation.
2. Check if the `snapshot.Status.CreationTime` is set when the snapshot is ready.
3. Fix unit test setup always setting the `storageClassName` to the last one in the list.
4. Check if the `provisioner` for `StorageClass` is same as the `snapshotter` in the `VolumeSnapshotClass`
  • Loading branch information
pavannd1 authored and Ilya Kislenko committed Apr 15, 2019
1 parent 2cec5b5 commit 6b420f4
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 15 deletions.
8 changes: 5 additions & 3 deletions pkg/kube/snapshot/snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ const (
pvcKind = "PersistentVolumeClaim"
)

var snapshotAPIGroup = "snapshot.storage.k8s.io"

// Create creates a VolumeSnapshot and returns it or any error happened meanwhile.
//
// 'name' is the name of the VolumeSnapshot.
Expand Down Expand Up @@ -210,7 +208,11 @@ func WaitOnReadyToUse(ctx context.Context, snapCli snapshotclient.Interface, sna
if err != nil {
return false, err
}
return snap.Status.ReadyToUse, nil
// Error can be set while waiting for creation
if snap.Status.Error != nil {
return false, errors.New(snap.Status.Error.Message)
}
return (snap.Status.ReadyToUse && snap.Status.CreationTime != nil), nil
})
}

Expand Down
25 changes: 13 additions & 12 deletions pkg/kube/snapshot/snapshot_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,15 +66,23 @@ func (s *SnapshotTestSuite) SetUpSuite(c *C) {
c.Logf("Failed to query VolumeSnapshotClass, skipping test. Error: %v", err)
c.Fail()
}
var snapshotterName string
if len(vscs.Items) != 0 {
s.snapshotClass = &vscs.Items[0].Name
vsClass, err := sc.VolumesnapshotV1alpha1().VolumeSnapshotClasses().Get(vscs.Items[0].Name, metav1.GetOptions{})
if err != nil {
c.Logf("Failed to get VolumeSnapshotClass, skipping test. Error: %v", err)
c.Fail()
}
snapshotterName = vsClass.Snapshotter
s.snapshotClass = &vsClass.Name
}

storageClasses, err := cli.StorageV1().StorageClasses().List(metav1.ListOptions{})
c.Assert(err, IsNil)
for _, class := range storageClasses.Items {
if isCSIProvisioner(class.Provisioner) {
if class.Provisioner == snapshotterName {
s.storageClassCSI = &class.Name
break
}
}

Expand Down Expand Up @@ -149,6 +157,7 @@ func (s *SnapshotTestSuite) TestVolumeSnapshotCloneFake(c *C) {
},
},
}
ctime := metav1.Now()
snapshot := &snapshot.VolumeSnapshot{
ObjectMeta: metav1.ObjectMeta{
Name: fakeSnapshotName,
Expand All @@ -159,7 +168,8 @@ func (s *SnapshotTestSuite) TestVolumeSnapshotCloneFake(c *C) {
VolumeSnapshotClassName: &fakeClass,
},
Status: snapshot.VolumeSnapshotStatus{
ReadyToUse: true,
ReadyToUse: true,
CreationTime: &ctime,
},
}

Expand Down Expand Up @@ -274,15 +284,6 @@ func (s *SnapshotTestSuite) TestVolumeSnapshot(c *C) {

}

func isCSIProvisioner(provisioner string) bool {
for _, part := range strings.Split(provisioner, ".") {
if part == "csi" {
return true
}
}
return false
}

func (s *SnapshotTestSuite) cleanupNamespace(c *C, ns string) {
pvcs, erra := s.cli.CoreV1().PersistentVolumeClaims(ns).List(metav1.ListOptions{})
if erra != nil {
Expand Down

0 comments on commit 6b420f4

Please sign in to comment.