Wait also for CSI PVs during serial eviction

pkg/driver/driver_alicloud.go

@@ -36,6 +36,11 @@ import (
 	"github.com/aliyun/alibaba-cloud-sdk-go/services/ecs"
 )
 
+const (
+	// alicloudDriverName is the name of the CSI driver for Alibaba Cloud
+	alicloudDriverName = "diskplugin.csi.alibabacloud.com"
+)
+
 // AlicloudDriver is the driver struct for holding Alicloud machine information
 type AlicloudDriver struct {
 	AlicloudMachineClass *v1alpha1.AlicloudMachineClass
@@ -314,7 +319,7 @@ func (c *AlicloudDriver) GetVolNames(specs []corev1.PersistentVolumeSpec) ([]str
 			if name, ok := spec.FlexVolume.Options["volumeId"]; ok {
 				names = append(names, name)
 			}
-		} else if spec.CSI != nil && spec.CSI.Driver == "diskplugin.csi.alibabacloud.com" && spec.CSI.VolumeHandle != "" {
+		} else if spec.CSI != nil && spec.CSI.Driver == alicloudDriverName && spec.CSI.VolumeHandle != "" {
 			names = append(names, spec.CSI.VolumeHandle)
 		}
 	}

pkg/driver/driver_aws.go

@@ -21,6 +21,8 @@ import (
 	"encoding/base64"
 	"errors"
 	"fmt"
+	"net/url"
+	"regexp"
 	"strings"
 
 	v1alpha1 "github.com/gardener/machine-controller-manager/pkg/apis/machine/v1alpha1"
@@ -36,6 +38,14 @@ import (
 	"k8s.io/klog"
 )
 
+const (
+	// awsEBSDriverName is the name of the CSI driver for EBS
+	awsEBSDriverName = "ebs.csi.aws.com"
+
+	resourceTypeInstance = "instance"
+	resourceTypeVolume   = "volume"
+)
+
 // AWSDriver is the driver struct for holding AWS machine information
 type AWSDriver struct {
 	AWSMachineClass *v1alpha1.AWSMachineClass
@@ -45,11 +55,6 @@ type AWSDriver struct {
 	MachineName     string
 }
 
-const (
-	resourceTypeInstance = "instance"
-	resourceTypeVolume   = "volume"
-)
-
 // NewAWSDriver returns an empty AWSDriver object
 func NewAWSDriver(create func() (string, error), delete func() error, existing func() (string, error)) Driver {
 	return &AWSDriver{}
@@ -494,12 +499,18 @@ func (d *AWSDriver) GetVolNames(specs []corev1.PersistentVolumeSpec) ([]string,
 	names := []string{}
 	for i := range specs {
 		spec := &specs[i]
-		if spec.AWSElasticBlockStore == nil {
-			// Not an aws volume
-			continue
+		if spec.AWSElasticBlockStore != nil {
+			name, err := kubernetesVolumeIDToEBSVolumeID(spec.AWSElasticBlockStore.VolumeID)
+			if err != nil {
+				klog.Errorf("Failed to translate Kubernetes volume ID '%s' to EBS volume ID: %v", spec.AWSElasticBlockStore.VolumeID, err)
+				continue
+			}
+
+			names = append(names, name)
+		} else if spec.CSI != nil && spec.CSI.Driver == awsEBSDriverName && spec.CSI.VolumeHandle != "" {
+			name := spec.CSI.VolumeHandle
+			names = append(names, name)
 		}
-		name := spec.AWSElasticBlockStore.VolumeID
-		names = append(names, name)
 	}
 	return names, nil
 }
@@ -513,3 +524,47 @@ func (d *AWSDriver) GetUserData() string {
 func (d *AWSDriver) SetUserData(userData string) {
 	d.UserData = userData
 }
+
+// awsVolumeRegMatch represents Regex Match for AWS volume.
+var awsVolumeRegMatch = regexp.MustCompile("^vol-[^/]*$")
+
+// kubernetesVolumeIDToEBSVolumeID translates Kubernetes volume ID to EBS volume ID
+// KubernetsVolumeID forms:
+//  * aws://<zone>/<awsVolumeId>
+//  * aws:///<awsVolumeId>
+//  * <awsVolumeId>
+// EBS Volume ID form:
+//  * vol-<alphanumberic>
+func kubernetesVolumeIDToEBSVolumeID(kubernetesID string) (string, error) {
+	// name looks like aws://availability-zone/awsVolumeId
+
+	// The original idea of the URL-style name was to put the AZ into the
+	// host, so we could find the AZ immediately from the name without
+	// querying the API.  But it turns out we don't actually need it for
+	// multi-AZ clusters, as we put the AZ into the labels on the PV instead.
+	// However, if in future we want to support multi-AZ cluster
+	// volume-awareness without using PersistentVolumes, we likely will
+	// want the AZ in the host.
+	if !strings.HasPrefix(kubernetesID, "aws://") {
+		// Assume a bare aws volume id (vol-1234...)
+		return kubernetesID, nil
+	}
+	url, err := url.Parse(kubernetesID)
+	if err != nil {
+		return "", fmt.Errorf("invalid disk name (%s): %v", kubernetesID, err)
+	}
+	if url.Scheme != "aws" {
+		return "", fmt.Errorf("invalid scheme for AWS volume (%s)", kubernetesID)
+	}
+
+	awsID := url.Path
+	awsID = strings.Trim(awsID, "/")
+
+	// We sanity check the resulting volume; the two known formats are
+	// vol-12345678 and vol-12345678abcdef01
+	if !awsVolumeRegMatch.MatchString(awsID) {
+		return "", fmt.Errorf("invalid format for AWS volume (%s)", kubernetesID)
+	}
+
+	return awsID, nil
+}

pkg/driver/driver_aws_test.go

@@ -14,7 +14,6 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-// Package validation is used to validate all the machine CRD objects
 package driver
 
 import (
@@ -29,7 +28,7 @@ import (
 
 var _ = Describe("Driver AWS", func() {
 
-	Context("GenerateTags Driver AWS Spec", func() {
+	Context("#generateTags", func() {
 
 		It("should convert multiples tags successfully", func() {
 			awsDriver := &AWSDriver{
@@ -93,7 +92,7 @@ var _ = Describe("Driver AWS", func() {
 		})
 	})
 
-	Context("GenerateBlockDevices Driver AWS Spec", func() {
+	Context("#generateBlockDevices", func() {
 
 		It("should convert multiples blockDevices successfully", func() {
 			awsDriver := &AWSDriver{}
@@ -220,6 +219,74 @@ var _ = Describe("Driver AWS", func() {
 			Expect(disksGenerated).To(Equal(expectedDisks))
 			Expect(err).ToNot(HaveOccurred())
 		})
+	})
+
+	Context("#GetVolNames", func() {
+		var hostPathPVSpec = corev1.PersistentVolumeSpec{
+			PersistentVolumeSource: corev1.PersistentVolumeSource{
+				HostPath: &corev1.HostPathVolumeSource{
+					Path: "/mnt/data",
+				},
+			},
+		}
+
+		It("should handle in-tree PV (with .spec.awsElasticBlockStore)", func() {
+			driver := &AWSDriver{}
+			pvs := []corev1.PersistentVolumeSpec{
+				{
+					PersistentVolumeSource: corev1.PersistentVolumeSource{
+						AWSElasticBlockStore: &corev1.AWSElasticBlockStoreVolumeSource{
+							VolumeID: "aws://eu-west-1a/vol-1234",
+						},
+					},
+				},
+				{
+					PersistentVolumeSource: corev1.PersistentVolumeSource{
+						AWSElasticBlockStore: &corev1.AWSElasticBlockStoreVolumeSource{
+							VolumeID: "aws:///vol-2345",
+						},
+					},
+				},
+				{
+					PersistentVolumeSource: corev1.PersistentVolumeSource{
+						AWSElasticBlockStore: &corev1.AWSElasticBlockStoreVolumeSource{
+							VolumeID: "vol-3456",
+						},
+					},
+				},
+				hostPathPVSpec,
+			}
 
+			actual, err := driver.GetVolNames(pvs)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(actual).To(Equal([]string{"vol-1234", "vol-2345", "vol-3456"}))
+		})
+
+		It("should handle out-of-tree PV (with .spec.csi.volumeHandle)", func() {
+			driver := &AWSDriver{}
+			pvs := []corev1.PersistentVolumeSpec{
+				{
+					PersistentVolumeSource: corev1.PersistentVolumeSource{
+						CSI: &corev1.CSIPersistentVolumeSource{
+							Driver:       "io.kubernetes.storage.mock",
+							VolumeHandle: "vol-2345",
+						},
+					},
+				},
+				{
+					PersistentVolumeSource: corev1.PersistentVolumeSource{
+						CSI: &corev1.CSIPersistentVolumeSource{
+							Driver:       "ebs.csi.aws.com",
+							VolumeHandle: "vol-1234",
+						},
+					},
+				},
+				hostPathPVSpec,
+			}
+
+			actual, err := driver.GetVolNames(pvs)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(actual).To(Equal([]string{"vol-1234"}))
+		})
 	})
 })

pkg/driver/driver_azure.go

@@ -43,6 +43,11 @@ import (
 	"k8s.io/klog"
 )
 
+const (
+	// azureDiskDriverName is the name of the CSI driver for Azure Disk
+	azureDiskDriverName = "disk.csi.azure.com"
+)
+
 // AzureDriver is the driver struct for holding azure machine information
 type AzureDriver struct {
 	AzureMachineClass *v1alpha1.AzureMachineClass
@@ -1165,12 +1170,13 @@ func (d *AzureDriver) GetVolNames(specs []corev1.PersistentVolumeSpec) ([]string
 	names := []string{}
 	for i := range specs {
 		spec := &specs[i]
-		if spec.AzureDisk == nil {
-			// Not an azure volume
-			continue
+		if spec.AzureDisk != nil {
+			name := spec.AzureDisk.DiskName
+			names = append(names, name)
+		} else if spec.CSI != nil && spec.CSI.Driver == azureDiskDriverName && spec.CSI.VolumeHandle != "" {
+			name := spec.CSI.VolumeHandle
+			names = append(names, name)
 		}
-		name := spec.AzureDisk.DiskName
-		names = append(names, name)
 	}
 	return names, nil
 }

pkg/driver/driver_azure_test.go

@@ -17,15 +17,17 @@ limitations under the License.
 package driver
 
 import (
-	"github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2019-07-01/compute"
 	"github.com/gardener/machine-controller-manager/pkg/apis/machine/v1alpha1"
+
+	"github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2019-07-01/compute"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
+	corev1 "k8s.io/api/core/v1"
 )
 
 var _ = Describe("Driver Azure", func() {
 
-	Context("GenerateDataDisks Driver Azure Spec", func() {
+	Context("#generateDataDisks", func() {
 
 		It("should convert multiple dataDisks successfully", func() {
 			azureDriver := &AzureDriver{}
@@ -145,6 +147,60 @@ var _ = Describe("Driver Azure", func() {
 
 			Expect(disksGenerated).To(Equal(expectedDisks))
 		})
+	})
+
+	Context("#GetVolNames", func() {
+		var hostPathPVSpec = corev1.PersistentVolumeSpec{
+			PersistentVolumeSource: corev1.PersistentVolumeSource{
+				HostPath: &corev1.HostPathVolumeSource{
+					Path: "/mnt/data",
+				},
+			},
+		}
+
+		It("should handle in-tree PV (with .spec.azureDisk)", func() {
+			driver := &AzureDriver{}
+			pvs := []corev1.PersistentVolumeSpec{
+				{
+					PersistentVolumeSource: corev1.PersistentVolumeSource{
+						AzureDisk: &corev1.AzureDiskVolumeSource{
+							DiskName: "disk-1",
+						},
+					},
+				},
+				hostPathPVSpec,
+			}
+
+			actual, err := driver.GetVolNames(pvs)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(actual).To(Equal([]string{"disk-1"}))
+		})
 
+		It("should handle out-of-tree PV (with .spec.csi.volumeHandle)", func() {
+			driver := &AzureDriver{}
+			pvs := []corev1.PersistentVolumeSpec{
+				{
+					PersistentVolumeSource: corev1.PersistentVolumeSource{
+						CSI: &corev1.CSIPersistentVolumeSource{
+							Driver:       "io.kubernetes.storage.mock",
+							VolumeHandle: "vol-2",
+						},
+					},
+				},
+				{
+					PersistentVolumeSource: corev1.PersistentVolumeSource{
+						CSI: &corev1.CSIPersistentVolumeSource{
+							Driver:       "disk.csi.azure.com",
+							VolumeHandle: "vol-1",
+						},
+					},
+				},
+				hostPathPVSpec,
+			}
+
+			actual, err := driver.GetVolNames(pvs)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(actual).To(Equal([]string{"vol-1"}))
+		})
 	})
 })

pkg/driver/driver_gcp.go

@@ -37,6 +37,11 @@ import (
 	"k8s.io/klog"
 )
 
+const (
+	// gcePDDriverName is the name of the CSI driver for GCE PD
+	gcePDDriverName = "pd.csi.storage.gke.io"
+)
+
 // GCPDriver is the driver struct for holding GCP machine information
 type GCPDriver struct {
 	GCPMachineClass *v1alpha1.GCPMachineClass
@@ -366,12 +371,13 @@ func (d *GCPDriver) GetVolNames(specs []corev1.PersistentVolumeSpec) ([]string,
 	names := []string{}
 	for i := range specs {
 		spec := &specs[i]
-		if spec.GCEPersistentDisk == nil {
-			// Not a GCE volume
-			continue
+		if spec.GCEPersistentDisk != nil {
+			name := spec.GCEPersistentDisk.PDName
+			names = append(names, name)
+		} else if spec.CSI != nil && spec.CSI.Driver == gcePDDriverName && spec.CSI.VolumeHandle != "" {
+			name := spec.CSI.VolumeHandle
+			names = append(names, name)
 		}
-		name := spec.GCEPersistentDisk.PDName
-		names = append(names, name)
 	}
 	return names, nil
 }