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

Jump to bottom

Add volumesnapshot content deletion policy #40

Merged
merged 2 commits into from
Nov 20, 2018
Merged

Add volumesnapshot content deletion policy #40

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7a374a3
Generated files
jingxu97 Oct 10, 2018
96e3bd1
Add volumesnapshot content deletion policy
jingxu97 Oct 10, 2018
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
22 changes: 22 additions & 0 deletions pkg/apis/volumesnapshot/v1alpha1/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,11 @@ type VolumeSnapshotClass struct {
// to the snapshotter.
// +optional
Parameters map[string]string `json:"parameters,omitempty" protobuf:"bytes,3,rep,name=parameters"`

// Optional: what happens to a snapshot content when released from its snapshot.
// The default policy is Delete if not specified.
// +optional
DeletionPolicy *DeletionPolicy `json:"deletionPolicy,omitempty" protobuf:"bytes,4,opt,name=deletionPolicy"`
}

// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
Expand Down Expand Up @@ -195,6 +200,11 @@ type VolumeSnapshotContentSpec struct {
// be used if it is available.
// +optional
VolumeSnapshotClassName *string `json:"snapshotClassName" protobuf:"bytes,4,opt,name=snapshotClassName"`

// Optional: what happens to a snapshot content when released from its snapshot. It will be set to Delete by default
// if not specified
// +optional
DeletionPolicy *DeletionPolicy `json:"deletionPolicy" protobuf:"bytes,5,opt,name=deletionPolicy"`
}

// VolumeSnapshotSource represents the actual location and type of the snapshot. Only one of its members may be specified.
Expand Down Expand Up @@ -232,3 +242,15 @@ type CSIVolumeSnapshotSource struct {
// +optional
RestoreSize *int64 `json:"restoreSize,omitempty" protobuf:"bytes,4,opt,name=restoreSize"`
}

// DeletionPolicy describes a policy for end-of-life maintenance of volume snapshot contents
type DeletionPolicy string

const (
// VolumeSnapshotContentDelete means the snapshot content will be deleted from Kubernetes on release from its volume snapshot.
VolumeSnapshotContentDelete DeletionPolicy = "Delete"

// VolumeSnapshotContentRetain means the snapshot will be left in its current state on release from its volume snapshot.
// The default policy is Retain if not specified.
VolumeSnapshotContentRetain DeletionPolicy = "Retain"
)
10 changes: 10 additions & 0 deletions pkg/apis/volumesnapshot/v1alpha1/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

17 changes: 17 additions & 0 deletions pkg/client/clientset/versioned/typed/volumesnapshot/v1alpha1/volumesnapshot.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

17 changes: 17 additions & 0 deletions pkg/client/clientset/versioned/typed/volumesnapshot/v1alpha1/volumesnapshotclass.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

17 changes: 17 additions & 0 deletions pkg/client/clientset/versioned/typed/volumesnapshot/v1alpha1/volumesnapshotcontent.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions pkg/controller/csi_handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func (handler *csiHandler) DeleteSnapshot(content *crdv1.VolumeSnapshotContent,

err := handler.csiConnection.DeleteSnapshot(ctx, content.Spec.CSI.SnapshotHandle, snapshotterCredentials)
if err != nil {
return fmt.Errorf("failed to delete snapshot data %s: %q", content.Name, err)
return fmt.Errorf("failed to delete snapshot content %s: %q", content.Name, err)
}

return nil
Expand All @@ -92,7 +92,7 @@ func (handler *csiHandler) GetSnapshotStatus(content *crdv1.VolumeSnapshotConten

csiSnapshotStatus, timestamp, size, err := handler.csiConnection.GetSnapshotStatus(ctx, content.Spec.CSI.SnapshotHandle)
if err != nil {
return false, 0, 0, fmt.Errorf("failed to list snapshot data %s: %q", content.Name, err)
return false, 0, 0, fmt.Errorf("failed to list snapshot content %s: %q", content.Name, err)
}
return csiSnapshotStatus, timestamp, size, nil

Expand Down
11 changes: 6 additions & 5 deletions pkg/controller/framework_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -744,7 +744,7 @@ func newTestController(kubeClient kubernetes.Interface, clientset clientset.Inte
}

// newContent returns a new content with given attributes
func newContent(name, className, snapshotHandle, volumeUID, volumeName, boundToSnapshotUID, boundToSnapshotName string, size *int64, creationTime *int64) *crdv1.VolumeSnapshotContent {
func newContent(name, className, snapshotHandle, volumeUID, volumeName, boundToSnapshotUID, boundToSnapshotName string, deletionPolicy *crdv1.DeletionPolicy, size *int64, creationTime *int64) *crdv1.VolumeSnapshotContent {
content := crdv1.VolumeSnapshotContent{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Expand All @@ -766,6 +766,7 @@ func newContent(name, className, snapshotHandle, volumeUID, volumeName, boundToS
UID: types.UID(volumeUID),
Name: volumeName,
},
DeletionPolicy: deletionPolicy,
},
}
if boundToSnapshotName != "" {
Expand All @@ -781,14 +782,14 @@ func newContent(name, className, snapshotHandle, volumeUID, volumeName, boundToS
return &content
}

func newContentArray(name, className, snapshotHandle, volumeUID, volumeName, boundToSnapshotUID, boundToSnapshotName string, size *int64, creationTime *int64) []*crdv1.VolumeSnapshotContent {
func newContentArray(name, className, snapshotHandle, volumeUID, volumeName, boundToSnapshotUID, boundToSnapshotName string, deletionPolicy *crdv1.DeletionPolicy, size *int64, creationTime *int64) []*crdv1.VolumeSnapshotContent {
return []*crdv1.VolumeSnapshotContent{
newContent(name, className, snapshotHandle, volumeUID, volumeName, boundToSnapshotUID, boundToSnapshotName, size, creationTime),
newContent(name, className, snapshotHandle, volumeUID, volumeName, boundToSnapshotUID, boundToSnapshotName, deletionPolicy, size, creationTime),
}
}

func newContentWithUnmatchDriverArray(name, className, snapshotHandle, volumeUID, volumeName, boundToSnapshotUID, boundToSnapshotName string, size *int64, creationTime *int64) []*crdv1.VolumeSnapshotContent {
content := newContent(name, className, snapshotHandle, volumeUID, volumeName, boundToSnapshotUID, boundToSnapshotName, size, creationTime)
func newContentWithUnmatchDriverArray(name, className, snapshotHandle, volumeUID, volumeName, boundToSnapshotUID, boundToSnapshotName string, deletionPolicy *crdv1.DeletionPolicy, size *int64, creationTime *int64) []*crdv1.VolumeSnapshotContent {
content := newContent(name, className, snapshotHandle, volumeUID, volumeName, boundToSnapshotUID, boundToSnapshotName, deletionPolicy, size, creationTime)
content.Spec.VolumeSnapshotSource.CSI.Driver = "fake"
return []*crdv1.VolumeSnapshotContent{
content,
Expand Down
27 changes: 24 additions & 3 deletions pkg/controller/snapshot_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,24 @@ func (ctrl *csiSnapshotController) syncContent(content *crdv1.VolumeSnapshotCont
snapshot = nil
}
if snapshot == nil {
ctrl.deleteSnapshotContent(content)
}
if content.Spec.DeletionPolicy != nil {
switch *content.Spec.DeletionPolicy {
case crdv1.VolumeSnapshotContentRetain:
glog.V(4).Infof("VolumeSnapshotContent[%s]: policy is Retain, nothing to do", content.Name)

case crdv1.VolumeSnapshotContentDelete:
glog.V(4).Infof("VolumeSnapshotContent[%s]: policy is Delete", content.Name)
ctrl.deleteSnapshotContent(content)
default:
// Unknown VolumeSnapshotDeletionolicy
ctrl.eventRecorder.Event(content, v1.EventTypeWarning, "SnapshotUnknownDeletionPolicy", "Volume Snapshot Content has unrecognized deletion policy")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the default policy is actually the same as Retain?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, it should be Delete. I fixed the logic here.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For default, it won't call deleteSnapshotContent. So default is still equal to Retain? You said it should be the same as Delete earlier.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there are two cases.
For dynamic creation, we add deletePolicy when creating snapshotContent. Check "createSnapshotOperation" function in controller

For static binding case, if no deletePolicy is set, by default it is retain.

}
return nil
}
// By default, we use Retain policy if it is not set by users
glog.V(4).Infof("VolumeSnapshotContent[%s]: by default the policy is Retain", content.Name)

}
return nil
}

Expand Down Expand Up @@ -538,6 +553,10 @@ func (ctrl *csiSnapshotController) createSnapshotOperation(snapshot *crdv1.Volum
return nil, err
}

if class.DeletionPolicy == nil {
class.DeletionPolicy = new(crdv1.DeletionPolicy)
*class.DeletionPolicy = crdv1.VolumeSnapshotContentDelete
}
snapshotContent := &crdv1.VolumeSnapshotContent{
ObjectMeta: metav1.ObjectMeta{
Name: contentName,
Expand All @@ -554,8 +573,10 @@ func (ctrl *csiSnapshotController) createSnapshotOperation(snapshot *crdv1.Volum
},
},
VolumeSnapshotClassName: &(class.Name),
DeletionPolicy: class.DeletionPolicy,
},
}
glog.V(3).Infof("volume snapshot content %v", snapshotContent)
// Try to create the VolumeSnapshotContent object several times
for i := 0; i < ctrl.createSnapshotContentRetryCount; i++ {
glog.V(5).Infof("createSnapshot [%s]: trying to save volume snapshot content %s", snapshotKey(snapshot), snapshotContent.Name)
Expand All @@ -565,7 +586,7 @@ func (ctrl *csiSnapshotController) createSnapshotOperation(snapshot *crdv1.Volum
glog.V(3).Infof("volume snapshot content %q for snapshot %q already exists, reusing", snapshotContent.Name, snapshotKey(snapshot))
err = nil
} else {
glog.V(3).Infof("volume snapshot content %q for snapshot %q saved", snapshotContent.Name, snapshotKey(snapshot))
glog.V(3).Infof("volume snapshot content %q for snapshot %q saved, %v", snapshotContent.Name, snapshotKey(snapshot), snapshotContent)
}
break
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/controller/snapshot_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import (
)

func storeVersion(t *testing.T, prefix string, c cache.Store, version string, expectedReturn bool) {
content := newContent("contentName", classEmpty, "sid1-1", "vuid1-1", "volume1-1", "snapuid1-1", "snap1-1", nil, nil)
content := newContent("contentName", classEmpty, "sid1-1", "vuid1-1", "volume1-1", "snapuid1-1", "snap1-1", nil, nil, nil)
content.ResourceVersion = version
ret, err := storeObjectUpdate(c, content, "content")
if err != nil {
Expand Down Expand Up @@ -82,7 +82,7 @@ func TestControllerCacheParsingError(t *testing.T) {
// There must be something in the cache to compare with
storeVersion(t, "Step1", c, "1", true)

content := newContent("contentName", classEmpty, "sid1-1", "vuid1-1", "volume1-1", "snapuid1-1", "snap1-1", nil, nil)
content := newContent("contentName", classEmpty, "sid1-1", "vuid1-1", "volume1-1", "snapuid1-1", "snap1-1", nil, nil, nil)
content.ResourceVersion = "xxx"
_, err := storeObjectUpdate(c, content, "content")
if err == nil {
Expand Down
Loading