From b83b26b8f9577c8f1b25fe3b7bbf1fca62ba88be Mon Sep 17 00:00:00 2001
From: David Zhu <dyzz@google.com>
Date: Tue, 16 Jul 2019 16:59:49 -0700
Subject: [PATCH] Translate PV so that the pvSpec is also translated for things
 like AccessModes etc

---
 pkg/controller/csi_handler.go | 23 ++++++++++++++++-------
 pkg/controller/util.go        | 11 +++++------
 pkg/controller/util_test.go   | 17 ++++++++++++++++-
 3 files changed, 37 insertions(+), 14 deletions(-)

diff --git a/pkg/controller/csi_handler.go b/pkg/controller/csi_handler.go
index 21840f393..a397ef108 100644
--- a/pkg/controller/csi_handler.go
+++ b/pkg/controller/csi_handler.go
@@ -240,12 +240,6 @@ func getCSISource(pv *v1.PersistentVolume) (*v1.CSIPersistentVolumeSource, error
 	}
 	if pv.Spec.CSI != nil {
 		return pv.Spec.CSI, nil
-	} else if csitranslationlib.IsPVMigratable(pv) {
-		csiPV, err := csitranslationlib.TranslateInTreePVToCSI(pv)
-		if err != nil {
-			return nil, fmt.Errorf("failed to translate in tree pv to CSI: %v", err)
-		}
-		return csiPV.Spec.CSI, nil
 	}
 	return nil, fmt.Errorf("pv contained non-csi source that was not migrated")
 }
@@ -274,6 +268,15 @@ func (h *csiHandler) csiAttach(va *storage.VolumeAttachment) (*storage.VolumeAtt
 			return va, nil, fmt.Errorf("could not add PersistentVolume finalizer: %s", err)
 		}
 
+		if csitranslationlib.IsPVMigratable(pv) {
+			pv, err = csitranslationlib.TranslateInTreePVToCSI(pv)
+			if err != nil {
+				return va, nil, fmt.Errorf("failed to translate in tree pv to CSI: %v", err)
+			}
+		}
+
+		// Both csiSource and pvSpec could be translated here if the PV was
+		// migrated
 		csiSource, err = getCSISource(pv)
 		if err != nil {
 			return va, nil, err
@@ -307,7 +310,7 @@ func (h *csiHandler) csiAttach(va *storage.VolumeAttachment) (*storage.VolumeAtt
 		readOnly = false
 	}
 
-	volumeCapabilities, err := GetVolumeCapabilities(pvSpec, csiSource)
+	volumeCapabilities, err := GetVolumeCapabilities(pvSpec)
 	if err != nil {
 		return va, nil, err
 	}
@@ -354,6 +357,12 @@ func (h *csiHandler) csiDetach(va *storage.VolumeAttachment) (*storage.VolumeAtt
 		if err != nil {
 			return va, err
 		}
+		if csitranslationlib.IsPVMigratable(pv) {
+			pv, err = csitranslationlib.TranslateInTreePVToCSI(pv)
+			if err != nil {
+				return va, fmt.Errorf("failed to translate in tree pv to CSI: %v", err)
+			}
+		}
 		csiSource, err = getCSISource(pv)
 		if err != nil {
 			return va, err
diff --git a/pkg/controller/util.go b/pkg/controller/util.go
index 4cf7481ad..7f320fb56 100644
--- a/pkg/controller/util.go
+++ b/pkg/controller/util.go
@@ -18,6 +18,7 @@ package controller
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 	"regexp"
 
@@ -135,16 +136,14 @@ func GetNodeIDFromCSINode(driver string, csiNode *storage.CSINode) (string, bool
 }
 
 // GetVolumeCapabilities returns volumecapability from PV spec
-func GetVolumeCapabilities(pvSpec *v1.PersistentVolumeSpec, csiSource *v1.CSIPersistentVolumeSource) (*csi.VolumeCapability, error) {
+func GetVolumeCapabilities(pvSpec *v1.PersistentVolumeSpec) (*csi.VolumeCapability, error) {
 	m := map[v1.PersistentVolumeAccessMode]bool{}
 	for _, mode := range pvSpec.AccessModes {
 		m[mode] = true
 	}
 
-	// csiSource is passed separately for if PV source had to go through
-	// CSI translation for regular (non-inline) volumes
-	if csiSource == nil {
-		return nil, fmt.Errorf("CSI volume source was nil")
+	if pvSpec.CSI == nil {
+		return nil, errors.New("CSI volume source was nil")
 	}
 
 	var cap *csi.VolumeCapability
@@ -157,7 +156,7 @@ func GetVolumeCapabilities(pvSpec *v1.PersistentVolumeSpec, csiSource *v1.CSIPer
 		}
 
 	} else {
-		fsType := csiSource.FSType
+		fsType := pvSpec.CSI.FSType
 		if len(fsType) == 0 {
 			fsType = defaultFSType
 		}
diff --git a/pkg/controller/util_test.go b/pkg/controller/util_test.go
index 7cddbf319..f2b6a9bb4 100644
--- a/pkg/controller/util_test.go
+++ b/pkg/controller/util_test.go
@@ -1,3 +1,18 @@
+/*
+Copyright 2019 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
 package controller
 
 import (
@@ -195,7 +210,7 @@ func TestGetVolumeCapabilities(t *testing.T) {
 				},
 			},
 		}
-		cap, err := GetVolumeCapabilities(&pv.Spec, pv.Spec.CSI)
+		cap, err := GetVolumeCapabilities(&pv.Spec)
 
 		if err == nil && test.expectError {
 			t.Errorf("test %s: expected error, got none", test.name)