From f3e7eeca8c2fca89486c2cd059d4c06bcbcb22e0 Mon Sep 17 00:00:00 2001
From: David Zhu <dyzz@google.com>
Date: Mon, 10 Jun 2019 15:34:45 -0700
Subject: [PATCH] Add more backward compatible access mode logic to remove
 ReadOnlyMany access mode when ReadWriteOnce,ReadOnlyMany specified

---
 .../csi-translation-lib/plugins/gce_pd.go     | 33 +++++++++++--
 .../plugins/gce_pd_test.go                    | 47 +++++++++++++++++--
 2 files changed, 71 insertions(+), 9 deletions(-)

diff --git a/staging/src/k8s.io/csi-translation-lib/plugins/gce_pd.go b/staging/src/k8s.io/csi-translation-lib/plugins/gce_pd.go
index 1a9b11a13ab..4dee96ef9fe 100644
--- a/staging/src/k8s.io/csi-translation-lib/plugins/gce_pd.go
+++ b/staging/src/k8s.io/csi-translation-lib/plugins/gce_pd.go
@@ -141,19 +141,44 @@ func (g *gcePersistentDiskCSITranslator) TranslateInTreeStorageClassToCSI(sc *st
 // plugin never supported ReadWriteMany but also did not validate or enforce
 // this access mode for pre-provisioned volumes. The GCE PD CSI Driver validates
 // and enforces (fails) ReadWriteMany. Therefore we treat all in-tree
-// ReadWriteMany as ReadWriteOnce volumes to not break legacy volumes.
+// ReadWriteMany as ReadWriteOnce volumes to not break legacy volumes. It also
+// takes [ReadWriteOnce, ReadOnlyMany] and makes it ReadWriteOnce. This is
+// because the in-tree plugin does not enforce access modes and just attaches
+// the disk in ReadWriteOnce mode; however, the CSI external-attacher will fail
+// this combination because technically [ReadWriteOnce, ReadOnlyMany] is not
+// supportable on an attached volume
+// See: https://github.com/kubernetes-csi/external-attacher/issues/153
 func backwardCompatibleAccessModes(ams []v1.PersistentVolumeAccessMode) []v1.PersistentVolumeAccessMode {
 	if ams == nil {
 		return nil
 	}
-	newAM := []v1.PersistentVolumeAccessMode{}
+
+	s := map[v1.PersistentVolumeAccessMode]bool{}
+	var newAM []v1.PersistentVolumeAccessMode
+
 	for _, am := range ams {
 		if am == v1.ReadWriteMany {
-			newAM = append(newAM, v1.ReadWriteOnce)
+			// ReadWriteMany is unsupported in CSI, but in-tree did no
+			// validation and treated it as ReadWriteOnce
+			s[v1.ReadWriteOnce] = true
 		} else {
-			newAM = append(newAM, am)
+			s[am] = true
 		}
 	}
+
+	switch {
+	case s[v1.ReadOnlyMany] && s[v1.ReadWriteOnce]:
+		// ROX,RWO is unsupported in CSI, but in-tree did not validation and
+		// treated it as ReadWriteOnce
+		newAM = []v1.PersistentVolumeAccessMode{v1.ReadWriteOnce}
+	case s[v1.ReadWriteOnce]:
+		newAM = []v1.PersistentVolumeAccessMode{v1.ReadWriteOnce}
+	case s[v1.ReadOnlyMany]:
+		newAM = []v1.PersistentVolumeAccessMode{v1.ReadOnlyMany}
+	default:
+		newAM = []v1.PersistentVolumeAccessMode{v1.ReadWriteOnce}
+	}
+
 	return newAM
 }
 
diff --git a/staging/src/k8s.io/csi-translation-lib/plugins/gce_pd_test.go b/staging/src/k8s.io/csi-translation-lib/plugins/gce_pd_test.go
index cd64ec92275..3df9b0242af 100644
--- a/staging/src/k8s.io/csi-translation-lib/plugins/gce_pd_test.go
+++ b/staging/src/k8s.io/csi-translation-lib/plugins/gce_pd_test.go
@@ -211,18 +211,16 @@ func TestBackwardCompatibleAccessModes(t *testing.T) {
 		expAccessModes []v1.PersistentVolumeAccessMode
 	}{
 		{
-			name: "multiple normals",
+			name: "ROX",
 			accessModes: []v1.PersistentVolumeAccessMode{
 				v1.ReadOnlyMany,
-				v1.ReadWriteOnce,
 			},
 			expAccessModes: []v1.PersistentVolumeAccessMode{
 				v1.ReadOnlyMany,
-				v1.ReadWriteOnce,
 			},
 		},
 		{
-			name: "one normal",
+			name: "RWO",
 			accessModes: []v1.PersistentVolumeAccessMode{
 				v1.ReadWriteOnce,
 			},
@@ -231,13 +229,52 @@ func TestBackwardCompatibleAccessModes(t *testing.T) {
 			},
 		},
 		{
-			name: "some readwritemany",
+			name: "RWX",
+			accessModes: []v1.PersistentVolumeAccessMode{
+				v1.ReadWriteMany,
+			},
+			expAccessModes: []v1.PersistentVolumeAccessMode{
+				v1.ReadWriteOnce,
+			},
+		},
+		{
+			name: "RWO, ROX",
+			accessModes: []v1.PersistentVolumeAccessMode{
+				v1.ReadOnlyMany,
+				v1.ReadWriteOnce,
+			},
+			expAccessModes: []v1.PersistentVolumeAccessMode{
+				v1.ReadWriteOnce,
+			},
+		},
+		{
+			name: "RWO, RWX",
 			accessModes: []v1.PersistentVolumeAccessMode{
 				v1.ReadWriteOnce,
 				v1.ReadWriteMany,
 			},
 			expAccessModes: []v1.PersistentVolumeAccessMode{
 				v1.ReadWriteOnce,
+			},
+		},
+		{
+			name: "RWX, ROX",
+			accessModes: []v1.PersistentVolumeAccessMode{
+				v1.ReadWriteMany,
+				v1.ReadOnlyMany,
+			},
+			expAccessModes: []v1.PersistentVolumeAccessMode{
+				v1.ReadWriteOnce,
+			},
+		},
+		{
+			name: "RWX, ROX, RWO",
+			accessModes: []v1.PersistentVolumeAccessMode{
+				v1.ReadWriteMany,
+				v1.ReadWriteOnce,
+				v1.ReadOnlyMany,
+			},
+			expAccessModes: []v1.PersistentVolumeAccessMode{
 				v1.ReadWriteOnce,
 			},
 		},