Skip to content

Change VolumeSnapshotClassName to a pointer to string #11

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

Merged
xing-yang merged 6 commits into from
Aug 23, 2018
Merged

Change VolumeSnapshotClassName to a pointer to string #11

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d95ff46
Split GetClassFromVolumeSnapshot to two functions
xing-yang Aug 22, 2018
7baa5bf
Change VolumeSnapshotClassName to pointer to a string
xing-yang Aug 23, 2018
1893402
Re-generate deepcopy file after VolumeSnapshotClass change
xing-yang Aug 23, 2018
7140b77
Change controller to use VolumeSnapshotClassName as pointer
xing-yang Aug 23, 2018
de25b16
Fix a typo
xing-yang Aug 23, 2018
a9dd5c8
Address review comments
xing-yang Aug 23, 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
8 changes: 4 additions & 4 deletions pkg/apis/volumesnapshot/v1alpha1/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ type VolumeSnapshotSpec struct {
// Name of the VolumeSnapshotClass used by the VolumeSnapshot. If not specified, a default snapshot class will
// be used if it is available.
// +optional
VolumeSnapshotClassName string `json:"snapshotClassName" protobuf:"bytes,3,opt,name=snapshotClassName"`
VolumeSnapshotClassName *string `json:"snapshotClassName" protobuf:"bytes,3,opt,name=snapshotClassName"`
}

// VolumeSnapshotStatus is the status of the VolumeSnapshot
Expand All @@ -92,7 +92,7 @@ type VolumeSnapshotStatus struct {

// When restoring volume from the snapshot, the volume size should be equal to or
// larger than the RestoreSize if it is specified. If RestoreSize is set to nil, it means
// that the storage plugin does not have this information avaialble.
// that the storage plugin does not have this information available.
// +optional
RestoreSize *resource.Quantity `json:"restoreSize" protobuf:"bytes,2,opt,name=restoreSize"`

Expand Down Expand Up @@ -205,7 +205,7 @@ type VolumeSnapshotContentSpec struct {
// Name of the VolumeSnapshotClass used by the VolumeSnapshot. If not specified, a default snapshot class will
// be used if it is available.
// +optional
VolumeSnapshotClassName string `json:"snapshotClassName" protobuf:"bytes,4,opt,name=snapshotClassName"`
VolumeSnapshotClassName *string `json:"snapshotClassName" protobuf:"bytes,4,opt,name=snapshotClassName"`
}

// VolumeSnapshotSource represents the actual location and type of the snapshot. Only one of its members may be specified.
Expand Down Expand Up @@ -239,7 +239,7 @@ type CSIVolumeSnapshotSource struct {

// When restoring volume from the snapshot, the volume size should be equal to or
// larger than the RestoreSize if it is specified. If RestoreSize is set to nil, it means
// that the storage plugin does not have this information avaialble.
// that the storage plugin does not have this information available.
// +optional
RestoreSize *resource.Quantity `json:"restoreSize" protobuf:"bytes,4,opt,name=restoreSize"`
}
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.

164 changes: 89 additions & 75 deletions pkg/controller/snapshot_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,9 @@ func (ctrl *csiSnapshotController) getMatchSnapshotContent(snapshot *crdv1.Volum
if content.Spec.VolumeSnapshotRef != nil &&
content.Spec.VolumeSnapshotRef.Name == snapshot.Name &&
content.Spec.VolumeSnapshotRef.Namespace == snapshot.Namespace &&
content.Spec.VolumeSnapshotRef.UID == snapshot.UID {
content.Spec.VolumeSnapshotRef.UID == snapshot.UID &&
content.Spec.VolumeSnapshotClassName != nil && snapshot.Spec.VolumeSnapshotClassName != nil &&
*(content.Spec.VolumeSnapshotClassName) == *(snapshot.Spec.VolumeSnapshotClassName) {
found = true
snapshotContentObj = content
break
Expand Down Expand Up @@ -407,7 +409,8 @@ func (ctrl *csiSnapshotController) checkandBindSnapshotContent(snapshot *crdv1.V
} else if content.Spec.VolumeSnapshotRef.UID == "" {
contentClone := content.DeepCopy()
contentClone.Spec.VolumeSnapshotRef.UID = snapshot.UID
contentClone.Spec.VolumeSnapshotClassName = snapshot.Spec.VolumeSnapshotClassName
className := *(snapshot.Spec.VolumeSnapshotClassName)
contentClone.Spec.VolumeSnapshotClassName = &className
newContent, err := ctrl.clientset.VolumesnapshotV1alpha1().VolumeSnapshotContents().Update(contentClone)
if err != nil {
glog.V(4).Infof("updating VolumeSnapshotContent[%s] error status failed %v", newContent.Name, err)
Expand Down Expand Up @@ -447,11 +450,22 @@ func (ctrl *csiSnapshotController) createSnapshotOperation(snapshot *crdv1.Volum
glog.V(4).Infof("error is already set in snapshot, do not retry to create: %s", snapshot.Status.Error.Message)
return snapshot, nil
}
class, err := ctrl.GetClassFromVolumeSnapshot(snapshot)
if err != nil {
glog.Errorf("createSnapshotOperation failed to getClassFromVolumeSnapshot %s", err)
return nil, err

className := snapshot.Spec.VolumeSnapshotClassName
glog.V(5).Infof("createSnapshotOperation [%s]: VolumeSnapshotClassName [%s]", snapshot.Name, *className)
var class *crdv1.VolumeSnapshotClass
var err error
if className != nil {
class, err = ctrl.GetSnapshotClass(*className)
if err != nil {
glog.Errorf("createSnapshotOperation failed to getClassFromVolumeSnapshot %s", err)
return nil, err
}
} else {
glog.Errorf("failed to take snapshot %s without a snapshot class", snapshot.Name)
return nil, fmt.Errorf("failed to take snapshot %s without a snapshot class", snapshot.Name)
}

volume, err := ctrl.getVolumeFromVolumeSnapshot(snapshot)
if err != nil {
glog.Errorf("createSnapshotOperation failed to get PersistentVolume object [%s]: Error: [%#v]", snapshot.Name, err)
Expand Down Expand Up @@ -504,7 +518,7 @@ func (ctrl *csiSnapshotController) createSnapshotOperation(snapshot *crdv1.Volum
Namespace: snapshot.Namespace,
Name: snapshot.Name,
UID: snapshot.UID,
APIVersion: "v1alpha1",
APIVersion: "snapshot.storage.k8s.io/v1alpha1",
},
PersistentVolumeRef: volumeRef,
VolumeSnapshotSource: crdv1.VolumeSnapshotSource{
Expand All @@ -515,7 +529,7 @@ func (ctrl *csiSnapshotController) createSnapshotOperation(snapshot *crdv1.Volum
RestoreSize: resource.NewQuantity(size, resource.BinarySI),
},
},
VolumeSnapshotClassName: class.Name,
VolumeSnapshotClassName: &(class.Name),
},
}
// Try to create the VolumeSnapshotContent object several times
Expand Down Expand Up @@ -567,8 +581,8 @@ func (ctrl *csiSnapshotController) deleteSnapshotContentOperation(content *crdv1
// get secrets if VolumeSnapshotClass specifies it
var snapshotterCredentials map[string]string
snapshotClassName := content.Spec.VolumeSnapshotClassName
if len(snapshotClassName) != 0 {
if snapshotClass, err := ctrl.clientset.VolumesnapshotV1alpha1().VolumeSnapshotClasses().Get(snapshotClassName, metav1.GetOptions{}); err == nil {
if snapshotClassName != nil {
if snapshotClass, err := ctrl.classLister.Get(*snapshotClassName); err == nil {
// Resolve snapshotting secret credentials.
// No VolumeSnapshot is provided when resolving delete secret names, since the VolumeSnapshot may or may not exist at delete time.
snapshotterSecretRef, err := GetSecretReference(snapshotClass.Parameters, content.Name, nil)
Expand Down Expand Up @@ -727,75 +741,75 @@ func (ctrl *csiSnapshotController) getStorageClassFromVolumeSnapshot(snapshot *c
return storageclass, nil
}

// GetClassFromVolumeSnapshot is a helper function to get snapshot class from VolumeSnapshot.
// If snapshot spec does not specify a VolumeSnapshotClass name, this function will try to figure out
// the default one from the PVC/PV StorageClass
func (ctrl *csiSnapshotController) GetClassFromVolumeSnapshot(snapshot *crdv1.VolumeSnapshot) (*crdv1.VolumeSnapshotClass, error) {
className := snapshot.Spec.VolumeSnapshotClassName
glog.V(5).Infof("getClassFromVolumeSnapshot [%s]: VolumeSnapshotClassName [%s]", snapshot.Name, className)

if len(className) > 0 {
obj, found, err := ctrl.classStore.GetByKey(className)
if found {
class, ok := obj.(*crdv1.VolumeSnapshotClass)
if ok {
return class, nil
}
}
class, err := ctrl.clientset.VolumesnapshotV1alpha1().VolumeSnapshotClasses().Get(className, metav1.GetOptions{})
if err != nil {
glog.Errorf("failed to retrieve snapshot class %s from the API server: %q", className, err)
return nil, fmt.Errorf("failed to retrieve snapshot class %s from the API server: %q", className, err)
}
_, updateErr := ctrl.storeClassUpdate(class)
if updateErr != nil {
glog.V(4).Infof("GetClassFromVolumeSnapshot [%s]: cannot update internal cache: %v", class.Name, updateErr)
}
glog.V(5).Infof("getClassFromVolumeSnapshot [%s]: VolumeSnapshotClassName [%s]", snapshot.Name, className)
return class, nil
} else {
storageclass, err := ctrl.getStorageClassFromVolumeSnapshot(snapshot)
if err != nil {
return nil, err
}
// Find default snapshot class if available
list, err := ctrl.classLister.List(labels.Everything())
if err != nil {
return nil, err
}
defaultClasses := []*crdv1.VolumeSnapshotClass{}
// GetSnapshotClass is a helper function to get snapshot class from the class name.
func (ctrl *csiSnapshotController) GetSnapshotClass(className string) (*crdv1.VolumeSnapshotClass, error) {
glog.V(5).Infof("getSnapshotClass: VolumeSnapshotClassName [%s]", className)

for _, class := range list {
if IsDefaultAnnotation(class.ObjectMeta) && storageclass.Provisioner == class.Snapshotter {
defaultClasses = append(defaultClasses, class)
glog.V(5).Infof("getDefaultClass added: %s", class.Name)
}
}
if len(defaultClasses) == 0 {
return nil, fmt.Errorf("cannot find default snapshot class")
}
if len(defaultClasses) > 1 {
glog.V(4).Infof("getDefaultClass %d defaults found", len(defaultClasses))
return nil, fmt.Errorf("%d default snapshot classes were found", len(defaultClasses))
obj, found, err := ctrl.classStore.GetByKey(className)
if found {
class, ok := obj.(*crdv1.VolumeSnapshotClass)
if ok {
return class, nil
}
glog.V(5).Infof("getClassFromVolumeSnapshot [%s]: default VolumeSnapshotClassName [%s]", snapshot.Name, defaultClasses[0])
snapshotClone := snapshot.DeepCopy()
snapshotClone.Spec.VolumeSnapshotClassName = defaultClasses[0].Name
newSnapshot, err := ctrl.clientset.VolumesnapshotV1alpha1().VolumeSnapshots(snapshotClone.Namespace).Update(snapshotClone)
if err != nil {
glog.V(4).Infof("updating VolumeSnapshot[%s] default class failed %v", snapshotKey(snapshot), err)
}
_, updateErr := ctrl.storeSnapshotUpdate(newSnapshot)
if updateErr != nil {
// We will get an "snapshot update" event soon, this is not a big error
glog.V(4).Infof("createSnapshot [%s]: cannot update internal cache: %v", snapshotKey(snapshot), updateErr)
}
_, updateErr = ctrl.storeClassUpdate(defaultClasses[0])
if updateErr != nil {
glog.V(4).Infof("GetClassFromVolumeSnapshot [%s]: cannot update internal cache: %v", defaultClasses[0].Name, updateErr)
}
class, err := ctrl.classLister.Get(className)
if err != nil {
glog.Errorf("failed to retrieve snapshot class %s from the API server: %q", className, err)
return nil, fmt.Errorf("failed to retrieve snapshot class %s from the API server: %q", className, err)
}
_, updateErr := ctrl.storeClassUpdate(class)
if updateErr != nil {
glog.V(4).Infof("getSnapshotClass [%s]: cannot update internal cache: %v", class.Name, updateErr)
}
return class, nil
}

// SetDefaultSnapshotClass is a helper function to figure out the default snapshot class from
// PVC/PV StorageClass and update VolumeSnapshot with this snapshot class name.
func (ctrl *csiSnapshotController) SetDefaultSnapshotClass(snapshot *crdv1.VolumeSnapshot) (*crdv1.VolumeSnapshotClass, *crdv1.VolumeSnapshot, error) {
glog.V(5).Infof("SetDefaultSnapshotClass for snapshot [%s]", snapshot.Name)

storageclass, err := ctrl.getStorageClassFromVolumeSnapshot(snapshot)
if err != nil {
return nil, nil, err
}
// Find default snapshot class if available
list, err := ctrl.classLister.List(labels.Everything())
if err != nil {
return nil, nil, err
}
defaultClasses := []*crdv1.VolumeSnapshotClass{}

for _, class := range list {
if IsDefaultAnnotation(class.ObjectMeta) && storageclass.Provisioner == class.Snapshotter && ctrl.snapshotterName == class.Snapshotter {
defaultClasses = append(defaultClasses, class)
glog.V(5).Infof("get defaultClass added: %s", class.Name)
}
return defaultClasses[0], nil
}
if len(defaultClasses) == 0 {
return nil, nil, fmt.Errorf("cannot find default snapshot class")
}
if len(defaultClasses) > 1 {
glog.V(4).Infof("get DefaultClass %d defaults found", len(defaultClasses))
return nil, nil, fmt.Errorf("%d default snapshot classes were found", len(defaultClasses))
}
glog.V(5).Infof("setDefaultSnapshotClass [%s]: default VolumeSnapshotClassName [%s]", snapshot.Name, defaultClasses[0].Name)
snapshotClone := snapshot.DeepCopy()
snapshotClone.Spec.VolumeSnapshotClassName = &(defaultClasses[0].Name)
newSnapshot, err := ctrl.clientset.VolumesnapshotV1alpha1().VolumeSnapshots(snapshotClone.Namespace).Update(snapshotClone)
if err != nil {
glog.V(4).Infof("updating VolumeSnapshot[%s] default class failed %v", snapshotKey(snapshot), err)
}
_, updateErr := ctrl.storeSnapshotUpdate(newSnapshot)
if updateErr != nil {
// We will get an "snapshot update" event soon, this is not a big error
glog.V(4).Infof("setDefaultSnapshotClass [%s]: cannot update internal cache: %v", snapshotKey(snapshot), updateErr)
}
_, updateErr = ctrl.storeClassUpdate(defaultClasses[0])
if updateErr != nil {
glog.V(4).Infof("setDefaultSnapshotClass [%s]: cannot update internal cache: %v", defaultClasses[0].Name, updateErr)
}
return defaultClasses[0], newSnapshot, nil
}

// getClaimFromVolumeSnapshot is a helper function to get PVC from VolumeSnapshot.
Expand Down
30 changes: 22 additions & 8 deletions pkg/controller/snapshot_controller_base.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import (
"github.com/kubernetes-csi/external-snapshotter/pkg/connection"
"k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/client-go/kubernetes"
Expand Down Expand Up @@ -274,8 +273,8 @@ func (ctrl *csiSnapshotController) contentWorker() {
if err == nil {
// Skip update if content is for another CSI driver
snapshotClassName := content.Spec.VolumeSnapshotClassName
if len(snapshotClassName) != 0 {
if snapshotClass, err := ctrl.clientset.VolumesnapshotV1alpha1().VolumeSnapshotClasses().Get(snapshotClassName, metav1.GetOptions{}); err == nil {
if snapshotClassName != nil {
if snapshotClass, err := ctrl.classLister.Get(*snapshotClassName); err == nil {
if snapshotClass.Snapshotter != ctrl.snapshotterName {
return false
}
Expand Down Expand Up @@ -325,12 +324,27 @@ func (ctrl *csiSnapshotController) contentWorker() {
// shouldProcessSnapshot detect if snapshotter in the VolumeSnapshotClass is the same as the snapshotter
// in external controller.
func (ctrl *csiSnapshotController) shouldProcessSnapshot(snapshot *crdv1.VolumeSnapshot) bool {
class, err := ctrl.GetClassFromVolumeSnapshot(snapshot)
if err != nil {
glog.V(2).Infof("fail to get snapshot class for snapshot %s: %v", snapshotKey(snapshot), err)
ctrl.updateSnapshotErrorStatusWithEvent(snapshot, v1.EventTypeWarning, "SnapshotCreationFailed", fmt.Sprintf("Failed to create snapshot with error %v", err))
return false
className := snapshot.Spec.VolumeSnapshotClassName
var class *crdv1.VolumeSnapshotClass
var err error
if className != nil {
glog.V(5).Infof("shouldProcessSnapshot [%s]: VolumeSnapshotClassName [%s]", snapshot.Name, *className)
class, err = ctrl.GetSnapshotClass(*className)
if err != nil {
glog.Errorf("shouldProcessSnapshot failed to getSnapshotClass %s", err)
ctrl.updateSnapshotErrorStatusWithEvent(snapshot, v1.EventTypeWarning, "GetSnapshotClassFailed", fmt.Sprintf("Failed to get snapshot class with error %v", err))
return false
}
} else {
glog.V(5).Infof("shouldProcessSnapshot [%s]: SetDefaultSnapshotClass", snapshot.Name)
class, snapshot, err = ctrl.SetDefaultSnapshotClass(snapshot)
if err != nil {
glog.Errorf("shouldProcessSnapshot failed to setDefaultClass %s", err)
ctrl.updateSnapshotErrorStatusWithEvent(snapshot, v1.EventTypeWarning, "SetDefaultSnapshotClassFailed", fmt.Sprintf("Failed to set default snapshot class with error %v", err))
return false
}
}

glog.V(5).Infof("VolumeSnapshotClass Snapshotter [%s] Snapshot Controller snapshotterName [%s]", class.Snapshotter, ctrl.snapshotterName)
if class.Snapshotter != ctrl.snapshotterName {
glog.V(4).Infof("Skipping VolumeSnapshot %s for snapshotter [%s] in VolumeSnapshotClass because it does not match with the snapshotter for controller [%s]", snapshotKey(snapshot), class.Snapshotter, ctrl.snapshotterName)
Expand Down