Merge pull request kubernetes#90162 from rfranzke/fix/azure-csi-translation

Fix flaws in Azure CSI translation

Author: k8s-ci-robot

staging/src/k8s.io/csi-translation-lib/CONTRIBUTING.md

@@ -2,6 +2,6 @@
 
 Do not open pull requests directly against this repository, they will be ignored. Instead, please open pull requests against [kubernetes/kubernetes](https://git.k8s.io/kubernetes/).  Please follow the same [contributing guide](https://git.k8s.io/kubernetes/CONTRIBUTING.md) you would follow for any other pull request made to kubernetes/kubernetes.
 
-This repository is published from [kubernetes/kubernetes/staging/src/k8s.io/csi-api](https://git.k8s.io/kubernetes/staging/src/k8s.io/csi-api) by the [kubernetes publishing-bot](https://git.k8s.io/publishing-bot).
+This repository is published from [kubernetes/kubernetes/staging/src/k8s.io/csi-translation-lib](https://git.k8s.io/kubernetes/staging/src/k8s.io/csi-translation-lib) by the [kubernetes publishing-bot](https://git.k8s.io/publishing-bot).
 
 Please see [Staging Directory and Publishing](https://git.k8s.io/community/contributors/devel/sig-architecture/staging.md) for more information.

staging/src/k8s.io/csi-translation-lib/go.mod

@@ -5,9 +5,11 @@ module k8s.io/csi-translation-lib
 go 1.13
 
 require (
+	github.com/stretchr/testify v1.4.0
 	k8s.io/api v0.0.0
 	k8s.io/apimachinery v0.0.0
 	k8s.io/cloud-provider v0.0.0
+	k8s.io/klog v1.0.0
 )
 
 replace (

staging/src/k8s.io/csi-translation-lib/plugins/BUILD

@@ -19,6 +19,7 @@ go_library(
         "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",
         "//staging/src/k8s.io/cloud-provider/volume:go_default_library",
+        "//vendor/k8s.io/klog:go_default_library",
     ],
 )
 
@@ -49,5 +50,7 @@ go_test(
     deps = [
         "//staging/src/k8s.io/api/core/v1:go_default_library",
         "//staging/src/k8s.io/api/storage/v1:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
+        "//vendor/github.com/stretchr/testify/assert:go_default_library",
     ],
 )

staging/src/k8s.io/csi-translation-lib/plugins/azure_disk.go

@@ -110,32 +110,36 @@ func (t *azureDiskCSITranslator) TranslateInTreePVToCSI(pv *v1.PersistentVolume)
 		return nil, fmt.Errorf("pv is nil or Azure Disk source not defined on pv")
 	}
 
-	azureSource := pv.Spec.PersistentVolumeSource.AzureDisk
-
-	// refer to https://github.com/kubernetes-sigs/azuredisk-csi-driver/blob/master/docs/driver-parameters.md
-	csiSource := &v1.CSIPersistentVolumeSource{
-		Driver:           AzureDiskDriverName,
-		VolumeHandle:     azureSource.DataDiskURI,
-		ReadOnly:         *azureSource.ReadOnly,
-		FSType:           *azureSource.FSType,
-		VolumeAttributes: map[string]string{azureDiskKind: "Managed"},
-	}
+	var (
+		azureSource = pv.Spec.PersistentVolumeSource.AzureDisk
+
+		// refer to https://github.com/kubernetes-sigs/azuredisk-csi-driver/blob/master/docs/driver-parameters.md
+		csiSource = &v1.CSIPersistentVolumeSource{
+			Driver:           AzureDiskDriverName,
+			VolumeAttributes: map[string]string{azureDiskKind: "Managed"},
+			VolumeHandle:     azureSource.DataDiskURI,
+		}
+	)
 
 	if azureSource.CachingMode != nil {
 		csiSource.VolumeAttributes[azureDiskCachingMode] = string(*azureSource.CachingMode)
 	}
 
 	if azureSource.FSType != nil {
+		csiSource.FSType = *azureSource.FSType
 		csiSource.VolumeAttributes[azureDiskFSType] = *azureSource.FSType
 	}
 
 	if azureSource.Kind != nil {
 		csiSource.VolumeAttributes[azureDiskKind] = string(*azureSource.Kind)
 	}
 
+	if azureSource.ReadOnly != nil {
+		csiSource.ReadOnly = *azureSource.ReadOnly
+	}
+
 	pv.Spec.PersistentVolumeSource.AzureDisk = nil
 	pv.Spec.PersistentVolumeSource.CSI = csiSource
-	pv.Spec.AccessModes = backwardCompatibleAccessModes(pv.Spec.AccessModes)
 
 	return pv, nil
 }

staging/src/k8s.io/csi-translation-lib/plugins/azure_disk_test.go

@@ -20,6 +20,9 @@ import (
 	"fmt"
 	"reflect"
 	"testing"
+
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 func TestIsManagedDisk(t *testing.T) {
@@ -91,3 +94,139 @@ func TestGetDiskName(t *testing.T) {
 		}
 	}
 }
+
+func TestTranslateAzureDiskInTreeStorageClassToCSI(t *testing.T) {
+	translator := NewAzureDiskCSITranslator()
+
+	cases := []struct {
+		name   string
+		volume *corev1.Volume
+		expVol *corev1.PersistentVolume
+		expErr bool
+	}{
+		{
+			name:   "empty volume",
+			expErr: true,
+		},
+		{
+			name:   "no azure disk volume",
+			volume: &corev1.Volume{},
+			expErr: true,
+		},
+		{
+			name: "azure disk volume",
+			volume: &corev1.Volume{
+				VolumeSource: corev1.VolumeSource{
+					AzureDisk: &corev1.AzureDiskVolumeSource{
+						DiskName:    "diskname",
+						DataDiskURI: "datadiskuri",
+					},
+				},
+			},
+			expVol: &corev1.PersistentVolume{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "disk.csi.azure.com-diskname",
+				},
+				Spec: corev1.PersistentVolumeSpec{
+					PersistentVolumeSource: corev1.PersistentVolumeSource{
+						CSI: &corev1.CSIPersistentVolumeSource{
+							Driver:           "disk.csi.azure.com",
+							VolumeHandle:     "datadiskuri",
+							VolumeAttributes: map[string]string{azureDiskKind: "Managed"},
+						},
+					},
+					AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce},
+				},
+			},
+		},
+	}
+
+	for _, tc := range cases {
+		t.Logf("Testing %v", tc.name)
+		got, err := translator.TranslateInTreeInlineVolumeToCSI(tc.volume)
+		if err != nil && !tc.expErr {
+			t.Errorf("Did not expect error but got: %v", err)
+		}
+
+		if err == nil && tc.expErr {
+			t.Errorf("Expected error, but did not get one.")
+		}
+
+		if !reflect.DeepEqual(got, tc.expVol) {
+			t.Errorf("Got parameters: %v, expected :%v", got, tc.expVol)
+		}
+	}
+}
+
+func TestTranslateAzureDiskInTreePVToCSI(t *testing.T) {
+	translator := NewAzureDiskCSITranslator()
+
+	cachingMode := corev1.AzureDataDiskCachingMode("cachingmode")
+	fsType := "fstype"
+	readOnly := true
+
+	cases := []struct {
+		name   string
+		volume *corev1.PersistentVolume
+		expVol *corev1.PersistentVolume
+		expErr bool
+	}{
+		{
+			name:   "empty volume",
+			expErr: true,
+		},
+		{
+			name:   "no azure file volume",
+			volume: &corev1.PersistentVolume{},
+			expErr: true,
+		},
+		{
+			name: "azure file volume",
+			volume: &corev1.PersistentVolume{
+				Spec: corev1.PersistentVolumeSpec{
+					PersistentVolumeSource: corev1.PersistentVolumeSource{
+						AzureDisk: &corev1.AzureDiskVolumeSource{
+							CachingMode: &cachingMode,
+							DataDiskURI: "datadiskuri",
+							FSType:      &fsType,
+							ReadOnly:    &readOnly,
+						},
+					},
+				},
+			},
+			expVol: &corev1.PersistentVolume{
+				Spec: corev1.PersistentVolumeSpec{
+					PersistentVolumeSource: corev1.PersistentVolumeSource{
+						CSI: &corev1.CSIPersistentVolumeSource{
+							Driver:   "disk.csi.azure.com",
+							FSType:   "fstype",
+							ReadOnly: true,
+							VolumeAttributes: map[string]string{
+								azureDiskCachingMode: "cachingmode",
+								azureDiskFSType:      fsType,
+								azureDiskKind:        "Managed",
+							},
+							VolumeHandle: "datadiskuri",
+						},
+					},
+				},
+			},
+		},
+	}
+
+	for _, tc := range cases {
+		t.Logf("Testing %v", tc.name)
+		got, err := translator.TranslateInTreePVToCSI(tc.volume)
+		if err != nil && !tc.expErr {
+			t.Errorf("Did not expect error but got: %v", err)
+		}
+
+		if err == nil && tc.expErr {
+			t.Errorf("Expected error, but did not get one.")
+		}
+
+		if !reflect.DeepEqual(got, tc.expVol) {
+			t.Errorf("Got parameters: %v, expected :%v", got, tc.expVol)
+		}
+	}
+}