Commit 9a8f07dd authored by andyzhangx's avatar andyzhangx

remove VM API call dep in azure disk WaitForAttach

add comment add unit test for WaitForAttach fnc add unit test for WaitForAttach Func
parent df3e59fe
...@@ -60,6 +60,7 @@ filegroup( ...@@ -60,6 +60,7 @@ filegroup(
go_test( go_test(
name = "go_default_test", name = "go_default_test",
srcs = [ srcs = [
"attacher_test.go",
"azure_common_test.go", "azure_common_test.go",
"azure_dd_block_test.go", "azure_dd_block_test.go",
"azure_dd_test.go", "azure_dd_test.go",
......
...@@ -22,6 +22,7 @@ import ( ...@@ -22,6 +22,7 @@ import (
"path/filepath" "path/filepath"
"runtime" "runtime"
"strconv" "strconv"
"strings"
"time" "time"
"github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2019-03-01/compute" "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2019-03-01/compute"
...@@ -133,12 +134,14 @@ func (a *azureDiskAttacher) VolumesAreAttached(specs []*volume.Spec, nodeName ty ...@@ -133,12 +134,14 @@ func (a *azureDiskAttacher) VolumesAreAttached(specs []*volume.Spec, nodeName ty
} }
func (a *azureDiskAttacher) WaitForAttach(spec *volume.Spec, devicePath string, _ *v1.Pod, timeout time.Duration) (string, error) { func (a *azureDiskAttacher) WaitForAttach(spec *volume.Spec, devicePath string, _ *v1.Pod, timeout time.Duration) (string, error) {
volumeSource, _, err := getVolumeSource(spec) // devicePath could be a LUN number or
if err != nil { // "/dev/disk/azure/scsi1/lunx", "/dev/sdx" on Linux node
return "", err // "/dev/diskx" on Windows node
if strings.HasPrefix(devicePath, "/dev/") {
return devicePath, nil
} }
diskController, err := getDiskController(a.plugin.host) volumeSource, _, err := getVolumeSource(spec)
if err != nil { if err != nil {
return "", err return "", err
} }
...@@ -146,23 +149,9 @@ func (a *azureDiskAttacher) WaitForAttach(spec *volume.Spec, devicePath string, ...@@ -146,23 +149,9 @@ func (a *azureDiskAttacher) WaitForAttach(spec *volume.Spec, devicePath string,
nodeName := types.NodeName(a.plugin.host.GetHostName()) nodeName := types.NodeName(a.plugin.host.GetHostName())
diskName := volumeSource.DiskName diskName := volumeSource.DiskName
lun := int32(-1) lun, err := strconv.Atoi(devicePath)
if runtime.GOOS != "windows" {
// on Linux, usually devicePath is like "/dev/disk/azure/scsi1/lun2", get LUN directly
lun, err = getDiskLUN(devicePath)
if err != nil {
klog.V(2).Infof("azureDisk - WaitForAttach: getDiskLUN(%s) failed with error: %v", devicePath, err)
}
}
if lun < 0 {
klog.V(2).Infof("azureDisk - WaitForAttach: begin to GetDiskLun by diskName(%s), DataDiskURI(%s), nodeName(%s), devicePath(%s)",
diskName, volumeSource.DataDiskURI, nodeName, devicePath)
lun, err = diskController.GetDiskLun(diskName, volumeSource.DataDiskURI, nodeName)
if err != nil { if err != nil {
return "", err return "", fmt.Errorf("parse %s failed with error: %v, diskName: %s, nodeName: %s", devicePath, err, diskName, nodeName)
}
klog.V(2).Infof("azureDisk - WaitForAttach: GetDiskLun succeeded, got lun(%v)", lun)
} }
exec := a.plugin.host.GetExec(a.plugin.GetPluginName()) exec := a.plugin.host.GetExec(a.plugin.GetPluginName())
...@@ -249,6 +238,14 @@ func (attacher *azureDiskAttacher) MountDevice(spec *volume.Spec, devicePath str ...@@ -249,6 +238,14 @@ func (attacher *azureDiskAttacher) MountDevice(spec *volume.Spec, devicePath str
if notMnt { if notMnt {
diskMounter := util.NewSafeFormatAndMountFromHost(azureDataDiskPluginName, attacher.plugin.host) diskMounter := util.NewSafeFormatAndMountFromHost(azureDataDiskPluginName, attacher.plugin.host)
mountOptions := util.MountOptionFromSpec(spec, options...) mountOptions := util.MountOptionFromSpec(spec, options...)
if runtime.GOOS == "windows" {
// only parse devicePath on Windows node
diskNum, err := getDiskNum(devicePath)
if err != nil {
return err
}
devicePath = diskNum
}
err = diskMounter.FormatAndMount(devicePath, deviceMountPath, *volumeSource.FSType, mountOptions) err = diskMounter.FormatAndMount(devicePath, deviceMountPath, *volumeSource.FSType, mountOptions)
if err != nil { if err != nil {
if cleanErr := os.Remove(deviceMountPath); cleanErr != nil { if cleanErr := os.Remove(deviceMountPath); cleanErr != nil {
......
/*
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 azure_dd
import (
"fmt"
"testing"
"time"
"github.com/stretchr/testify/assert"
"k8s.io/api/core/v1"
"k8s.io/kubernetes/pkg/volume"
)
func createVolSpec(name string, readOnly bool) *volume.Spec {
return &volume.Spec{
Volume: &v1.Volume{
VolumeSource: v1.VolumeSource{
AzureDisk: &v1.AzureDiskVolumeSource{
DiskName: name,
ReadOnly: &readOnly,
},
},
},
}
}
func TestWaitForAttach(t *testing.T) {
tests := []struct {
devicePath string
expected string
expectError bool
}{
{
devicePath: "/dev/disk/azure/scsi1/lun0",
expected: "/dev/disk/azure/scsi1/lun0",
expectError: false,
},
{
devicePath: "/dev/sdc",
expected: "/dev/sdc",
expectError: false,
},
{
devicePath: "/dev/disk0",
expected: "/dev/disk0",
expectError: false,
},
}
attacher := azureDiskAttacher{}
spec := createVolSpec("fakedisk", false)
for _, test := range tests {
result, err := attacher.WaitForAttach(spec, test.devicePath, nil, 3000*time.Millisecond)
assert.Equal(t, result, test.expected)
assert.Equal(t, err != nil, test.expectError, fmt.Sprintf("error msg: %v", err))
}
}
...@@ -22,7 +22,6 @@ import ( ...@@ -22,7 +22,6 @@ import (
"os" "os"
"path/filepath" "path/filepath"
"regexp" "regexp"
"strconv"
libstrings "strings" libstrings "strings"
"github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2019-03-01/compute" "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2019-03-01/compute"
...@@ -62,7 +61,9 @@ var ( ...@@ -62,7 +61,9 @@ var (
string(api.AzureDedicatedBlobDisk), string(api.AzureDedicatedBlobDisk),
string(api.AzureManagedDisk)) string(api.AzureManagedDisk))
lunPathRE = regexp.MustCompile(`/dev/disk/azure/scsi(?:.*)/lun(.+)`) // only for Windows node
winDiskNumRE = regexp.MustCompile(`/dev/disk(.+)`)
winDiskNumFormat = "/dev/disk%d"
) )
func getPath(uid types.UID, volName string, host volume.VolumeHost) string { func getPath(uid types.UID, volName string, host volume.VolumeHost) string {
...@@ -206,24 +207,12 @@ func strFirstLetterToUpper(str string) string { ...@@ -206,24 +207,12 @@ func strFirstLetterToUpper(str string) string {
return libstrings.ToUpper(string(str[0])) + str[1:] return libstrings.ToUpper(string(str[0])) + str[1:]
} }
// getDiskLUN : deviceInfo could be a LUN number or a device path, e.g. /dev/disk/azure/scsi1/lun2 // getDiskNum : extract the disk num from a device path,
func getDiskLUN(deviceInfo string) (int32, error) { // deviceInfo format could be like this: e.g. /dev/disk2
var diskLUN string func getDiskNum(deviceInfo string) (string, error) {
if len(deviceInfo) <= 2 { matches := winDiskNumRE.FindStringSubmatch(deviceInfo)
diskLUN = deviceInfo
} else {
// extract the LUN num from a device path
matches := lunPathRE.FindStringSubmatch(deviceInfo)
if len(matches) == 2 { if len(matches) == 2 {
diskLUN = matches[1] return matches[1], nil
} else {
return -1, fmt.Errorf("cannot parse deviceInfo: %s", deviceInfo)
} }
} return "", fmt.Errorf("cannot parse deviceInfo: %s, correct format: /dev/disk?", deviceInfo)
lun, err := strconv.Atoi(diskLUN)
if err != nil {
return -1, err
}
return int32(lun), nil
} }
...@@ -183,57 +183,42 @@ func TestNormalizeStorageAccountType(t *testing.T) { ...@@ -183,57 +183,42 @@ func TestNormalizeStorageAccountType(t *testing.T) {
} }
} }
func TestGetDiskLUN(t *testing.T) { func TestGetDiskNum(t *testing.T) {
tests := []struct { tests := []struct {
deviceInfo string deviceInfo string
expectedLUN int32 expectedNum string
expectError bool expectError bool
}{ }{
{ {
deviceInfo: "0", deviceInfo: "/dev/disk0",
expectedLUN: 0, expectedNum: "0",
expectError: false, expectError: false,
}, },
{ {
deviceInfo: "10", deviceInfo: "/dev/disk99",
expectedLUN: 10, expectedNum: "99",
expectError: false, expectError: false,
}, },
{ {
deviceInfo: "11d",
expectedLUN: -1,
expectError: true,
},
{
deviceInfo: "999",
expectedLUN: -1,
expectError: true,
},
{
deviceInfo: "", deviceInfo: "",
expectedLUN: -1, expectedNum: "",
expectError: true, expectError: true,
}, },
{ {
deviceInfo: "/dev/disk/azure/scsi1/lun2", deviceInfo: "/dev/disk",
expectedLUN: 2, expectedNum: "",
expectError: false, expectError: true,
},
{
deviceInfo: "/dev/disk/azure/scsi0/lun12",
expectedLUN: 12,
expectError: false,
}, },
{ {
deviceInfo: "/dev/disk/by-id/scsi1/lun2", deviceInfo: "999",
expectedLUN: -1, expectedNum: "",
expectError: true, expectError: true,
}, },
} }
for _, test := range tests { for _, test := range tests {
result, err := getDiskLUN(test.deviceInfo) result, err := getDiskNum(test.deviceInfo)
assert.Equal(t, result, test.expectedLUN) assert.Equal(t, result, test.expectedNum)
assert.Equal(t, err != nil, test.expectError, fmt.Sprintf("error msg: %v", err)) assert.Equal(t, err != nil, test.expectError, fmt.Sprintf("error msg: %v", err))
} }
} }
...@@ -84,7 +84,7 @@ func findDiskByLun(lun int, iohandler ioHandler, exec mount.Exec) (string, error ...@@ -84,7 +84,7 @@ func findDiskByLun(lun int, iohandler ioHandler, exec mount.Exec) (string, error
if d, ok := v["number"]; ok { if d, ok := v["number"]; ok {
if diskNum, ok := d.(float64); ok { if diskNum, ok := d.(float64); ok {
klog.V(2).Infof("azureDisk Mount: got disk number(%d) by LUN(%d)", int(diskNum), lun) klog.V(2).Infof("azureDisk Mount: got disk number(%d) by LUN(%d)", int(diskNum), lun)
return strconv.Itoa(int(diskNum)), nil return fmt.Sprintf(winDiskNumFormat, int(diskNum)), nil
} }
klog.Warningf("LUN(%d) found, but could not get disk number(%q), location: %q", lun, d, location) klog.Warningf("LUN(%d) found, but could not get disk number(%q), location: %q", lun, d, location)
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment