Process PV finalizer for migrated PV when CSIMigration turned off

pkg/controller/controller.go

@@ -36,9 +36,14 @@ import (
 	"k8s.io/client-go/tools/cache"
 	"k8s.io/client-go/tools/record"
 	"k8s.io/client-go/util/workqueue"
+	csitrans "k8s.io/csi-translation-lib"
 	"k8s.io/klog/v2"
 )
 
+const (
+	annMigratedTo = "pv.kubernetes.io/migrated-to"
+)
+
 // CSIAttachController is a controller that attaches / detaches CSI volumes using provided Handler interface
 type CSIAttachController struct {
 	client        kubernetes.Interface
@@ -55,6 +60,7 @@ type CSIAttachController struct {
 
 	shouldReconcileVolumeAttachment bool
 	reconcileSync                   time.Duration
+	translator                      AttacherCSITranslator
 }
 
 // Handler is responsible for handling VolumeAttachment events from informer.
@@ -91,6 +97,7 @@ func NewCSIAttachController(client kubernetes.Interface, attacherName string, ha
 		pvQueue:                         workqueue.NewNamedRateLimitingQueue(paRateLimiter, "csi-attacher-pv"),
 		shouldReconcileVolumeAttachment: shouldReconcileVolumeAttachment,
 		reconcileSync:                   reconcileSync,
+		translator:                      csitrans.New(),
 	}
 
 	volumeAttachmentInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
@@ -217,7 +224,24 @@ func (ctrl *CSIAttachController) syncVA() {
 }
 
 func (ctrl *CSIAttachController) processFinalizers(pv *v1.PersistentVolume) bool {
-	return pv.DeletionTimestamp != nil && sets.NewString(pv.Finalizers...).Has(GetFinalizerName(ctrl.attacherName))
+	if sets.NewString(pv.Finalizers...).Has(GetFinalizerName(ctrl.attacherName)) {
+		if pv.DeletionTimestamp != nil {
+			return true
+		}
+
+		// if PV is provisioned by in-tree plugin and does not have migrated-to label
+		// this normally means this is a rollback scenario, we need to remove the finalizer as well
+		if ctrl.translator.IsPVMigratable(pv) {
+			if ann := pv.Annotations; ann != nil {
+				if migratedToDriver := ann[annMigratedTo]; migratedToDriver == ctrl.attacherName {
+					// migrated-to annonation detected, keep the finalizer
+					return false
+				}
+			}
+			return true
+		}
+	}
+	return false
 }
 
 // syncPV deals with one key off the queue.  It returns false when it's time to quit.

pkg/controller/controller_test.go

@@ -19,8 +19,10 @@ package controller
 import (
 	"testing"
 
+	v1 "k8s.io/api/core/v1"
 	storage "k8s.io/api/storage/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	csitrans "k8s.io/csi-translation-lib"
 )
 
 func TestShouldEnqueueVAChange(t *testing.T) {
@@ -129,3 +131,96 @@ func TestShouldEnqueueVAChange(t *testing.T) {
 		})
 	}
 }
+
+func TestProcessFinalizers(t *testing.T) {
+	type testcase struct {
+		name           string
+		pv             *v1.PersistentVolume
+		expectedResult bool
+	}
+
+	c := &CSIAttachController{}
+	c.translator = csitrans.New()
+	c.attacherName = "pd.csi.storage.gke.io"
+	time := metav1.Now()
+
+	testcases := []testcase{
+		{
+			name:           "nothing interesting in the PV",
+			pv:             &v1.PersistentVolume{},
+			expectedResult: false,
+		},
+		{
+			name: "no deletion timestamp, has finalizer",
+			pv: &v1.PersistentVolume{
+				ObjectMeta: metav1.ObjectMeta{
+					Finalizers: []string{"external-attacher/pd-csi-storage-gke-io"},
+				},
+			},
+			expectedResult: false,
+		},
+		{
+			name: "Has deletion timestamp, has finalizer",
+			pv: &v1.PersistentVolume{
+				ObjectMeta: metav1.ObjectMeta{
+					DeletionTimestamp: &time,
+					Finalizers:        []string{"external-attacher/pd-csi-storage-gke-io"},
+				},
+			},
+			expectedResult: true,
+		},
+		{
+			name: "no deletion timestamp, has finalizer, migrated PV, no migrated-to annotation",
+			pv: &v1.PersistentVolume{
+				ObjectMeta: metav1.ObjectMeta{
+					Finalizers: []string{"external-attacher/pd-csi-storage-gke-io"},
+				},
+				Spec: v1.PersistentVolumeSpec{
+					PersistentVolumeSource: v1.PersistentVolumeSource{
+						GCEPersistentDisk: &v1.GCEPersistentDiskVolumeSource{},
+					},
+				},
+			},
+			expectedResult: true,
+		},
+		{
+			name: "no deletion timestamp, has finalizer, migrated PV, no migrated-to annotation with random anno",
+			pv: &v1.PersistentVolume{
+				ObjectMeta: metav1.ObjectMeta{
+					Finalizers:  []string{"external-attacher/pd-csi-storage-gke-io"},
+					Annotations: map[string]string{"random": "random"},
+				},
+				Spec: v1.PersistentVolumeSpec{
+					PersistentVolumeSource: v1.PersistentVolumeSource{
+						GCEPersistentDisk: &v1.GCEPersistentDiskVolumeSource{},
+					},
+				},
+			},
+			expectedResult: true,
+		},
+		{
+			name: "no deletion timestamp, has finalizer, migrated PV, has migrated-to annotation",
+			pv: &v1.PersistentVolume{
+				ObjectMeta: metav1.ObjectMeta{
+					Finalizers: []string{"external-attacher/pd-csi-storage-gke-io"},
+					Annotations: map[string]string{
+						"pv.kubernetes.io/migrated-to": "pd.csi.storage.gke.io",
+					},
+				},
+				Spec: v1.PersistentVolumeSpec{
+					PersistentVolumeSource: v1.PersistentVolumeSource{
+						GCEPersistentDisk: &v1.GCEPersistentDiskVolumeSource{},
+					},
+				},
+			},
+			expectedResult: false,
+		},
+	}
+
+	for _, tc := range testcases {
+		result := c.processFinalizers(tc.pv)
+		if result != tc.expectedResult {
+			t.Errorf("Error executing test %v: expected result %v, got %v", tc.name, tc.expectedResult, result)
+		}
+	}
+}

pkg/controller/csi_handler.go

@@ -625,10 +625,29 @@ func (h *csiHandler) SyncNewOrUpdatedPersistentVolume(pv *v1.PersistentVolume) {
 	klog.V(4).Infof("CSIHandler: processing PV %q", pv.Name)
 	// Sync and remove finalizer on given PV
 	if pv.DeletionTimestamp == nil {
-		// Don't process anything that has no deletion timestamp.
-		klog.V(4).Infof("CSIHandler: processing PV %q: no deletion timestamp, ignoring", pv.Name)
-		h.pvQueue.Forget(pv.Name)
-		return
+		ignore := true
+
+		// if the PV is migrated this means CSIMigration is disabled so we need to remove the finalizer
+		// to give back the control of the PV to Kube-Controller-Manager
+		if h.translator.IsPVMigratable(pv) {
+			ignore = false
+			if ann := pv.Annotations; ann != nil {
+				if migratedToDriver := ann[annMigratedTo]; migratedToDriver == h.attacherName {
+					ignore = true
+				} else {
+					klog.V(4).Infof("CSIHandler: PV %q is an in-tree PV but does not have migrated-to annotation "+
+						"or the annotation does not match. Expect %v, Get %v. Remove the finalizer for this PV ",
+						pv.Name, h.attacherName, migratedToDriver)
+				}
+			}
+		}
+
+		if ignore {
+			// Don't process anything that has no deletion timestamp.
+			klog.V(4).Infof("CSIHandler: processing PV %q: no deletion timestamp, ignoring", pv.Name)
+			h.pvQueue.Forget(pv.Name)
+			return
+		}
 	}
 
 	// Check if the PV has finalizer