Add VolumeSnapshotContent APIs to snapshot pkg #687
Conversation
| @@ -320,7 +321,7 @@ func (s *SnapshotTestSuite) testVolumeSnapshot(c *C, snapshotter snapshot.Snapsh | |||
| VolumeName: volumeCloneName, | |||
| StorageClassName: "", | |||
| SnapshotName: snapshotCloneName, | |||
| RestoreSize: nil, | |||
| RestoreSize: &sizeOriginal, | |||
There was a problem hiding this comment.
Fixes test that was failing on DigitalOcean
| if err != nil { | ||
| return "", "", err | ||
| } | ||
| deletionPolicy, err := sna.getDeletionPolicyFromClass(scName) |
There was a problem hiding this comment.
Modified to also return deletionPolicy
| @@ -97,11 +107,28 @@ func (sna *SnapshotAlpha) Get(ctx context.Context, name, namespace string) (*v1a | |||
|
|
|||
| // Delete will delete the VolumeSnapshot and returns any error as a result. | |||
| func (sna *SnapshotAlpha) Delete(ctx context.Context, name, namespace string) error { | |||
| snap, err := sna.Get(ctx, name, namespace) | |||
There was a problem hiding this comment.
Delete now also gets and deletes the underlying VolumeSnapshotContent.
NotFound errors are ignored
There was a problem hiding this comment.
Should modify the comment in the interface and probably here too.
| return sna.DeleteContent(ctx, snap.Spec.SnapshotContentName) | ||
| } | ||
|
|
||
| // DeleteContent will delete the specified VolumeSnapshotContent |
| @@ -171,6 +197,15 @@ func (sna *SnapshotAlpha) CreateFromSource(ctx context.Context, source *Source, | |||
| return sna.WaitOnReadyToUse(ctx, snapshotName, namespace) | |||
| } | |||
|
|
|||
| // CreateContentFromSource will create a 'VolumesnaphotContent' for the underlying snapshot source. | |||
There was a problem hiding this comment.
New API - CreateContentFromSource. Allows caller to create just a VolumeSnapshotContent if needed.
| @@ -52,6 +53,10 @@ type Snapshotter interface { | |||
| // 'name' is the name of the VolumeSnapshot that will be deleted. | |||
| // 'namespace' is the namespace of the VolumeSnapshot that will be deleted. | |||
| Delete(ctx context.Context, name, namespace string) error | |||
| // Delete will delete the VolumeSnapshot and returns any error as a result. | |||
There was a problem hiding this comment.
[nit] DeleteContent / VolumeSnapshotContent
| // 'snapshotName' is the name of the snapshot that will be reference the VSC | ||
| // 'namespace' is the namespace of the snapshot. |
There was a problem hiding this comment.
[nit] snapshot => VolumeSnapshot
| // | ||
| // 'annotationKey' is the annotation key which has to be present on VolumeSnapshotClass. | ||
| // 'annotationValue' is the value for annotationKey in VolumeSnapshotClass spec. | ||
| // 'storageClassName' is the name of the storageClass that shares the same driver as the VolumeSnapshotClass. | ||
| // This returns error if no VolumeSnapshotClass found. | ||
| GetVolumeSnapshotClass(annotationKey, annotationValue, storageClassName string) (string, error) | ||
| GetVolumeSnapshotClass(annotationKey, annotationValue, storageClassName string) (string, string, error) |
There was a problem hiding this comment.
Q: Since the lookup is via an annotation, how do you know that at most 1 object will be found?
The code returns the first matching VolumeSnapshotClass.
| @@ -97,11 +107,28 @@ func (sna *SnapshotAlpha) Get(ctx context.Context, name, namespace string) (*v1a | |||
|
|
|||
| // Delete will delete the VolumeSnapshot and returns any error as a result. | |||
| func (sna *SnapshotAlpha) Delete(ctx context.Context, name, namespace string) error { | |||
| snap, err := sna.Get(ctx, name, namespace) | |||
There was a problem hiding this comment.
Should modify the comment in the interface and probably here too.
|
Little late but looks good. |
Change Overview
Adds API support to operate on
VolumeSnapshotContent.Specifically, it adds a
CreateContentFromSourceandDeleteContentAPI.This supports use cases where a namespaced
VolumeSnapshotobjectis not required.
Additional changes:
GetVolumeSnapshotClassDeleteto also delete the underlyingVolumeSnapshotContent.This supports cases where the
VolumeSnapshotis markedRetainrestore size set)
Pull request type
Please check the type of change your PR introduces:
Test Plan