diff --git a/pkg/volume/azure_dd/BUILD b/pkg/volume/azure_dd/BUILD index 04688265332..ba57ee1ea6a 100644 --- a/pkg/volume/azure_dd/BUILD +++ b/pkg/volume/azure_dd/BUILD @@ -34,7 +34,6 @@ go_library( "//vendor/k8s.io/apimachinery/pkg/types:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/wait:go_default_library", - "//vendor/k8s.io/utils/exec:go_default_library", ], ) @@ -63,7 +62,5 @@ go_test( "//pkg/volume/testing:go_default_library", "//vendor/k8s.io/api/core/v1:go_default_library", "//vendor/k8s.io/client-go/util/testing:go_default_library", - "//vendor/k8s.io/utils/exec:go_default_library", - "//vendor/k8s.io/utils/exec/testing:go_default_library", ], ) diff --git a/pkg/volume/azure_dd/attacher.go b/pkg/volume/azure_dd/attacher.go index 9c6822d2a87..f473bc4ffb1 100644 --- a/pkg/volume/azure_dd/attacher.go +++ b/pkg/volume/azure_dd/attacher.go @@ -36,7 +36,6 @@ import ( "k8s.io/kubernetes/pkg/volume" volumeutil "k8s.io/kubernetes/pkg/volume/util" "k8s.io/kubernetes/pkg/volume/util/volumehelper" - "k8s.io/utils/exec" ) type azureDiskDetacher struct { @@ -171,9 +170,7 @@ func (a *azureDiskAttacher) WaitForAttach(spec *volume.Spec, devicePath string, newDevicePath := "" err = wait.Poll(1*time.Second, timeout, func() (bool, error) { - exe := exec.New() - - if newDevicePath, err = findDiskByLun(lun, io, exe); err != nil { + if newDevicePath, err = findDiskByLun(lun, io); err != nil { return false, fmt.Errorf("azureDisk - WaitForAttach ticker failed node (%s) disk (%s) lun(%v) err(%s)", nodeName, diskName, lun, err) } @@ -182,7 +179,7 @@ func (a *azureDiskAttacher) WaitForAttach(spec *volume.Spec, devicePath string, // the curent sequence k8s uses for unformated disk (check-disk, mount, fail, mkfs.extX) hangs on // Azure Managed disk scsi interface. this is a hack and will be replaced once we identify and solve // the root case on Azure. - formatIfNotFormatted(newDevicePath, *volumeSource.FSType) + formatIfNotFormatted(newDevicePath, *volumeSource.FSType, a.plugin.host.GetExec(a.plugin.GetPluginName())) return true, nil } diff --git a/pkg/volume/azure_dd/azure_common.go b/pkg/volume/azure_dd/azure_common.go index 7c36b3984a6..ec3d69fb768 100644 --- a/pkg/volume/azure_dd/azure_common.go +++ b/pkg/volume/azure_dd/azure_common.go @@ -21,7 +21,6 @@ import ( "io/ioutil" "os" "path" - "regexp" "strconv" libstrings "strings" @@ -35,7 +34,6 @@ import ( "k8s.io/kubernetes/pkg/util/mount" "k8s.io/kubernetes/pkg/util/strings" "k8s.io/kubernetes/pkg/volume" - "k8s.io/utils/exec" ) const ( @@ -158,6 +156,7 @@ type ioHandler interface { ReadDir(dirname string) ([]os.FileInfo, error) WriteFile(filename string, data []byte, perm os.FileMode) error Readlink(name string) (string, error) + ReadFile(filename string) ([]byte, error) } //TODO: check if priming the iscsi interface is actually needed @@ -176,6 +175,10 @@ func (handler *osIOHandler) Readlink(name string) (string, error) { return os.Readlink(name) } +func (handler *osIOHandler) ReadFile(filename string) ([]byte, error) { + return ioutil.ReadFile(filename) +} + // exclude those used by azure as resource and OS root in /dev/disk/azure func listAzureDiskPath(io ioHandler) []string { azureDiskPath := "/dev/disk/azure/" @@ -209,13 +212,13 @@ func scsiHostRescan(io ioHandler) { } } -func findDiskByLun(lun int, io ioHandler, exe exec.Interface) (string, error) { +func findDiskByLun(lun int, io ioHandler) (string, error) { azureDisks := listAzureDiskPath(io) - return findDiskByLunWithConstraint(lun, io, exe, azureDisks) + return findDiskByLunWithConstraint(lun, io, azureDisks) } // finds a device mounted to "current" node -func findDiskByLunWithConstraint(lun int, io ioHandler, exe exec.Interface, azureDisks []string) (string, error) { +func findDiskByLunWithConstraint(lun int, io ioHandler, azureDisks []string) (string, error) { var err error sys_path := "/sys/bus/scsi/devices" if dirs, err := io.ReadDir(sys_path); err == nil { @@ -237,18 +240,30 @@ func findDiskByLunWithConstraint(lun int, io ioHandler, exe exec.Interface, azur if lun == l { // find the matching LUN // read vendor and model to ensure it is a VHD disk - vendor := path.Join(sys_path, name, "vendor") - model := path.Join(sys_path, name, "model") - out, err := exe.Command("cat", vendor, model).CombinedOutput() + vendorPath := path.Join(sys_path, name, "vendor") + vendorBytes, err := io.ReadFile(vendorPath) if err != nil { - glog.V(4).Infof("azure disk - failed to cat device vendor and model, err: %v", err) + glog.Errorf("failed to read device vendor, err: %v", err) continue } - matched, err := regexp.MatchString("^MSFT[ ]{0,}\nVIRTUAL DISK[ ]{0,}\n$", libstrings.ToUpper(string(out))) - if err != nil || !matched { - glog.V(4).Infof("azure disk - doesn't match VHD, output %v, error %v", string(out), err) + vendor := libstrings.TrimSpace(string(vendorBytes)) + if libstrings.ToUpper(vendor) != "MSFT" { + glog.V(4).Infof("vendor doesn't match VHD, got %s", vendor) continue } + + modelPath := path.Join(sys_path, name, "model") + modelBytes, err := io.ReadFile(modelPath) + if err != nil { + glog.Errorf("failed to read device model, err: %v", err) + continue + } + model := libstrings.TrimSpace(string(modelBytes)) + if libstrings.ToUpper(model) != "VIRTUAL DISK" { + glog.V(4).Infof("model doesn't match VHD, got %s", model) + continue + } + // find a disk, validate name dir := path.Join(sys_path, name, "block") if dev, err := io.ReadDir(dir); err == nil { @@ -270,8 +285,8 @@ func findDiskByLunWithConstraint(lun int, io ioHandler, exe exec.Interface, azur return "", err } -func formatIfNotFormatted(disk string, fstype string) { - notFormatted, err := diskLooksUnformatted(disk) +func formatIfNotFormatted(disk string, fstype string, exec mount.Exec) { + notFormatted, err := diskLooksUnformatted(disk, exec) if err == nil && notFormatted { args := []string{disk} // Disk is unformatted so format it. @@ -283,9 +298,8 @@ func formatIfNotFormatted(disk string, fstype string) { args = []string{"-E", "lazy_itable_init=0,lazy_journal_init=0", "-F", disk} } glog.Infof("azureDisk - Disk %q appears to be unformatted, attempting to format as type: %q with options: %v", disk, fstype, args) - runner := exec.New() - cmd := runner.Command("mkfs."+fstype, args...) - _, err := cmd.CombinedOutput() + + _, err := exec.Run("mkfs."+fstype, args...) if err == nil { // the disk has been formatted successfully try to mount it again. glog.Infof("azureDisk - Disk successfully formatted (mkfs): %s - %s %s", fstype, disk, "tt") @@ -300,12 +314,10 @@ func formatIfNotFormatted(disk string, fstype string) { } } -func diskLooksUnformatted(disk string) (bool, error) { +func diskLooksUnformatted(disk string, exec mount.Exec) (bool, error) { args := []string{"-nd", "-o", "FSTYPE", disk} - runner := exec.New() - cmd := runner.Command("lsblk", args...) glog.V(4).Infof("Attempting to determine if disk %q is formatted using lsblk with args: (%v)", disk, args) - dataOut, err := cmd.CombinedOutput() + dataOut, err := exec.Run("lsblk", args...) if err != nil { glog.Errorf("Could not determine if disk %q is formatted (%v)", disk, err) return false, err diff --git a/pkg/volume/azure_dd/azure_common_test.go b/pkg/volume/azure_dd/azure_common_test.go index d026e2b92ee..00a35b98caa 100644 --- a/pkg/volume/azure_dd/azure_common_test.go +++ b/pkg/volume/azure_dd/azure_common_test.go @@ -19,11 +19,9 @@ package azure_dd import ( "fmt" "os" + "strings" "testing" "time" - - "k8s.io/utils/exec" - fakeexec "k8s.io/utils/exec/testing" ) type fakeFileInfo struct { @@ -107,27 +105,18 @@ func (handler *fakeIOHandler) Readlink(name string) (string, error) { return "/dev/azure/disk/sda", nil } +func (handler *fakeIOHandler) ReadFile(filename string) ([]byte, error) { + if strings.HasSuffix(filename, "vendor") { + return []byte("Msft \n"), nil + } + if strings.HasSuffix(filename, "model") { + return []byte("Virtual Disk \n"), nil + } + return nil, fmt.Errorf("unknown file") +} + func TestIoHandler(t *testing.T) { - var fcmd fakeexec.FakeCmd - fcmd = fakeexec.FakeCmd{ - CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{ - // cat - func() ([]byte, error) { - return []byte("Msft \nVirtual Disk \n"), nil - }, - // cat - func() ([]byte, error) { - return []byte("Msft \nVirtual Disk \n"), nil - }, - }, - } - fake := fakeexec.FakeExec{ - CommandScript: []fakeexec.FakeCommandAction{ - func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, - func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, - }, - } - disk, err := findDiskByLun(lun, &fakeIOHandler{}, &fake) + disk, err := findDiskByLun(lun, &fakeIOHandler{}) // if no disk matches lun, exit if disk != "/dev/"+devName || err != nil { t.Errorf("no data disk found: disk %v err %v", disk, err)