Skip to content

Bug 1808123: UPSTREAM 275: Allows VolumeSnapshot to be deleted if the class isn't found #17

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
openshift-merge-robot merged 1 commit into from
Mar 20, 2020
Merged

Bug 1808123: UPSTREAM 275: Allows VolumeSnapshot to be deleted if the class isn't found #17

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
Allows VolumeSnapshot to be deleted if the class isn't found
  • Loading branch information
@huffmanca
huffmanca committed Mar 19, 2020
commit 2fcb6330c55433d47f8674e3ec4323732ec69214
4 changes: 2 additions & 2 deletions pkg/common-controller/snapshot_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import (

crdv1 "github.com/kubernetes-csi/external-snapshotter/pkg/apis/volumesnapshot/v1beta1"
"github.com/kubernetes-csi/external-snapshotter/pkg/utils"
"k8s.io/api/core/v1"
v1 "k8s.io/api/core/v1"
storagev1 "k8s.io/api/storage/v1"
apierrs "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/resource"
Expand Down Expand Up @@ -1092,7 +1092,7 @@ func (ctrl *csiSnapshotCommonController) getSnapshotClass(className string) (*cr
class, err := ctrl.classLister.Get(className)
if err != nil {
klog.Errorf("failed to retrieve snapshot class %s from the informer: %q", className, err)
return nil, fmt.Errorf("failed to retrieve snapshot class %s from the informer: %q", className, err)
return nil, err
}

return class, nil
Expand Down
17 changes: 12 additions & 5 deletions pkg/common-controller/snapshot_controller_base.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import (
storagelisters "github.com/kubernetes-csi/external-snapshotter/pkg/client/listers/volumesnapshot/v1beta1"
"github.com/kubernetes-csi/external-snapshotter/pkg/utils"

"k8s.io/api/core/v1"
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/util/wait"
Expand Down Expand Up @@ -215,8 +215,11 @@ func (ctrl *csiSnapshotCommonController) snapshotWorker() {
// The volume snapshot still exists in informer cache, the event must have
// been add/update/sync
newSnapshot, err := ctrl.checkAndUpdateSnapshotClass(snapshot)
if err == nil {
klog.V(5).Infof("passed checkAndUpdateSnapshotClass for snapshot %q", key)
if err == nil || (newSnapshot.ObjectMeta.DeletionTimestamp != nil && errors.IsNotFound(err)) {
// If the VolumeSnapshotClass is not found, we still need to process an update
// so that syncSnapshot can delete the snapshot, should it still exist in the
// cluster after it's been removed from the informer cache
klog.V(5).Infof("updating snapshot %q; snapshotClass may have already been removed", key)
ctrl.updateSnapshot(newSnapshot)
}
return false
Expand All @@ -243,7 +246,10 @@ func (ctrl *csiSnapshotCommonController) snapshotWorker() {
return false
}
newSnapshot, err := ctrl.checkAndUpdateSnapshotClass(snapshot)
if err == nil {
if err == nil || errors.IsNotFound(err) {
// We should still handle deletion events even if the VolumeSnapshotClass
// is not found in the cluster
klog.V(5).Infof("deleting snapshot %q; snapshotClass may have already been removed", key)
ctrl.deleteSnapshot(newSnapshot)
}
return false
Expand Down Expand Up @@ -329,7 +335,8 @@ func (ctrl *csiSnapshotCommonController) checkAndUpdateSnapshotClass(snapshot *c
if err != nil {
klog.Errorf("checkAndUpdateSnapshotClass failed to getSnapshotClass %v", err)
ctrl.updateSnapshotErrorStatusWithEvent(snapshot, v1.EventTypeWarning, "GetSnapshotClassFailed", fmt.Sprintf("Failed to get snapshot class with error %v", err))
return nil, err
// we need to return the original snapshot even if the class isn't found, as it may need to be deleted
return newSnapshot, err
}
} else {
klog.V(5).Infof("checkAndUpdateSnapshotClass [%s]: SetDefaultSnapshotClass", snapshot.Name)
Expand Down
2 changes: 1 addition & 1 deletion pkg/common-controller/snapshot_create_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ func TestCreateSnapshotSync(t *testing.T) {
initialContents: nocontents,
expectedContents: nocontents,
initialSnapshots: newSnapshotArray("snap7-1", "snapuid7-1", "claim7-1", "", classNonExisting, "", &False, nil, nil, nil, false, true, nil),
expectedSnapshots: newSnapshotArray("snap7-1", "snapuid7-1", "claim7-1", "", classNonExisting, "", &False, nil, nil, newVolumeError("Failed to create snapshot content with error failed to get input parameters to create snapshot snap7-1: \"failed to retrieve snapshot class non-existing from the informer: \\\"volumesnapshotclass.snapshot.storage.k8s.io \\\\\\\"non-existing\\\\\\\" not found\\\"\""), false, true, nil),
expectedSnapshots: newSnapshotArray("snap7-1", "snapuid7-1", "claim7-1", "", classNonExisting, "", &False, nil, nil, newVolumeError("Failed to create snapshot content with error failed to get input parameters to create snapshot snap7-1: \"volumesnapshotclass.snapshot.storage.k8s.io \\\"non-existing\\\" not found\""), false, true, nil),
initialClaims: newClaimArray("claim7-1", "pvc-uid7-1", "1Gi", "volume7-1", v1.ClaimBound, &classEmpty),
initialVolumes: newVolumeArray("volume7-1", "pv-uid7-1", "pv-handle7-1", "1Gi", "pvc-uid7-1", "claim7-1", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty),
expectedEvents: []string{"Warning SnapshotContentCreationFailed"},
Expand Down
2 changes: 1 addition & 1 deletion pkg/common-controller/snapshotclass_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func TestUpdateSnapshotClass(t *testing.T) {
name: "1-3 - snapshot class name not found",
initialContents: nocontents,
initialSnapshots: newSnapshotArray("snap1-1", "snapuid1-1", "claim1-1", "content1-1", "missing-class", "content1-1", &True, nil, nil, nil, false, true, nil),
expectedSnapshots: newSnapshotArray("snap1-1", "snapuid1-1", "claim1-1", "content1-1", "missing-class", "content1-1", &False, nil, nil, newVolumeError("Failed to get snapshot class with error failed to retrieve snapshot class missing-class from the informer: \"volumesnapshotclass.snapshot.storage.k8s.io \\\"missing-class\\\" not found\""), false, true, nil),
expectedSnapshots: newSnapshotArray("snap1-1", "snapuid1-1", "claim1-1", "content1-1", "missing-class", "content1-1", &False, nil, nil, newVolumeError("Failed to get snapshot class with error volumesnapshotclass.snapshot.storage.k8s.io \"missing-class\" not found"), false, true, nil),
initialClaims: newClaimArray("claim1-1", "pvc-uid1-1", "1Gi", "volume1-1", v1.ClaimBound, &sameDriver),
initialVolumes: newVolumeArray("volume1-1", "pv-uid1-1", "pv-handle1-1", "1Gi", "pvc-uid1-1", "claim1-1", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty),
initialStorageClasses: []*storage.StorageClass{sameDriverStorageClass},
Expand Down