diff --git a/pkg/common-controller/snapshot_controller.go b/pkg/common-controller/snapshot_controller.go index b60833784..955c9db58 100644 --- a/pkg/common-controller/snapshot_controller.go +++ b/pkg/common-controller/snapshot_controller.go @@ -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" @@ -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 diff --git a/pkg/common-controller/snapshot_controller_base.go b/pkg/common-controller/snapshot_controller_base.go index 2518dad85..4dfb5ed2c 100644 --- a/pkg/common-controller/snapshot_controller_base.go +++ b/pkg/common-controller/snapshot_controller_base.go @@ -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" @@ -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 @@ -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 @@ -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) diff --git a/pkg/common-controller/snapshot_create_test.go b/pkg/common-controller/snapshot_create_test.go index 74106404d..d5549a275 100644 --- a/pkg/common-controller/snapshot_create_test.go +++ b/pkg/common-controller/snapshot_create_test.go @@ -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"}, diff --git a/pkg/common-controller/snapshotclass_test.go b/pkg/common-controller/snapshotclass_test.go index 72ea3b5ae..83fdfb4fe 100644 --- a/pkg/common-controller/snapshotclass_test.go +++ b/pkg/common-controller/snapshotclass_test.go @@ -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},