Merge pull request #795 from bryanv/bryanv/pvc-bound-check-requested-…

…zones

🌱 Better handle bound PVC placement with multiple accessible zones

pkg/util/kube/storage.go

@@ -69,9 +69,15 @@ func GetPVCZoneConstraints(
 
 		switch pvc.Status.Phase {
 		case corev1.ClaimBound:
-			// Easy case: if this PVC is already bound then just get its accessible zones, if any
-			// (the PVC's StorageClass should be Immediate).
 			z = getPVCAccessibleZones(pvc)
+			if z.Len() > 1 {
+				if reqZones := getPVCRequestedZones(pvc); reqZones.Len() > 0 {
+					// A PVC's accessible zones are all the zones that the PV datastore is accessible on,
+					// which may include zones that were not requested. Constrain our candidates to the
+					// accessible zones which were also requested.
+					z = z.Intersection(reqZones)
+				}
+			}
 
 		case corev1.ClaimPending:
 			var isImmediate bool

pkg/util/kube/storage_test.go

@@ -183,7 +183,7 @@ var _ = Describe("GetPVCZoneConstraints", func() {
 		})
 	})
 
-	Context("Bound PVC with Immediate StorageClass and Unbound PVC with WFFC StorageClass ", func() {
+	Context("Bound PVC with Immediate StorageClass and Unbound PVC with WFFC StorageClass", func() {
 
 		It("Returns success with common zones", func() {
 			immdSC := *builder.DummyStorageClass()
@@ -232,6 +232,39 @@ var _ = Describe("GetPVCZoneConstraints", func() {
 			Expect(zones.UnsortedList()).To(ConsistOf("zone1"))
 		})
 	})
+
+	Context("Bound PVC with accessible zones that include zones not requested", func() {
+
+		It("Returns success with common zones", func() {
+			immdSC := *builder.DummyStorageClass()
+			immdSC.VolumeBindingMode = ptr.To(storagev1.VolumeBindingImmediate)
+
+			storageClasses := map[string]storagev1.StorageClass{
+				immdSC.Name: immdSC,
+			}
+
+			pvcs := make([]corev1.PersistentVolumeClaim, 1)
+			pvcs[0] = corev1.PersistentVolumeClaim{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "bound-pvc",
+					Annotations: map[string]string{
+						"csi.vsphere.volume-requested-topology":  `[{"topology.kubernetes.io/zone":"zone1"},{"topology.kubernetes.io/zone":"zone2"},{"topology.kubernetes.io/zone":"zone3"}]`,
+						"csi.vsphere.volume-accessible-topology": `[{"topology.kubernetes.io/zone":"zone1"},{"topology.kubernetes.io/zone":"zoneX"}]`,
+					},
+				},
+				Spec: corev1.PersistentVolumeClaimSpec{
+					StorageClassName: &immdSC.Name,
+				},
+				Status: corev1.PersistentVolumeClaimStatus{
+					Phase: corev1.ClaimBound,
+				},
+			}
+
+			zones, err := kubeutil.GetPVCZoneConstraints(storageClasses, pvcs)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(zones.UnsortedList()).To(ConsistOf("zone1"))
+		})
+	})
 })
 
 var _ = DescribeTableSubtree("GetPVCZoneConstraints Table",