Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Retire mount.Exec for k8s.io/utils/exec #85153

Merged
merged 1 commit into from Nov 14, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions pkg/controller/volume/attachdetach/BUILD
Expand Up @@ -46,6 +46,7 @@ go_library(
"//staging/src/k8s.io/client-go/util/workqueue:go_default_library",
"//staging/src/k8s.io/cloud-provider:go_default_library",
"//vendor/k8s.io/klog:go_default_library",
"//vendor/k8s.io/utils/exec:go_default_library",
],
)

Expand Down
Expand Up @@ -45,6 +45,8 @@ import (
"k8s.io/client-go/util/workqueue"
cloudprovider "k8s.io/cloud-provider"
"k8s.io/klog"
utilexec "k8s.io/utils/exec"

"k8s.io/kubernetes/pkg/controller/volume/attachdetach/cache"
"k8s.io/kubernetes/pkg/controller/volume/attachdetach/metrics"
"k8s.io/kubernetes/pkg/controller/volume/attachdetach/populator"
Expand Down Expand Up @@ -768,8 +770,8 @@ func (adc *attachDetachController) DeleteServiceAccountTokenFunc() func(types.UI
}
}

func (adc *attachDetachController) GetExec(pluginName string) mount.Exec {
return mount.NewOSExec()
func (adc *attachDetachController) GetExec(pluginName string) utilexec.Interface {
return utilexec.New()
}

func (adc *attachDetachController) addNodeToDswp(node *v1.Node, nodeName types.NodeName) {
Expand Down
1 change: 1 addition & 0 deletions pkg/controller/volume/expand/BUILD
Expand Up @@ -33,6 +33,7 @@ go_library(
"//staging/src/k8s.io/client-go/util/workqueue:go_default_library",
"//staging/src/k8s.io/cloud-provider:go_default_library",
"//vendor/k8s.io/klog:go_default_library",
"//vendor/k8s.io/utils/exec:go_default_library",
],
)

Expand Down
6 changes: 4 additions & 2 deletions pkg/controller/volume/expand/expand_controller.go
Expand Up @@ -41,6 +41,8 @@ import (
"k8s.io/client-go/tools/record"
"k8s.io/client-go/util/workqueue"
cloudprovider "k8s.io/cloud-provider"
utilexec "k8s.io/utils/exec"

v1helper "k8s.io/kubernetes/pkg/apis/core/v1/helper"
"k8s.io/kubernetes/pkg/controller/volume/events"
"k8s.io/kubernetes/pkg/util/mount"
Expand Down Expand Up @@ -380,8 +382,8 @@ func (expc *expandController) GetMounter(pluginName string) mount.Interface {
return nil
}

func (expc *expandController) GetExec(pluginName string) mount.Exec {
return mount.NewOSExec()
func (expc *expandController) GetExec(pluginName string) utilexec.Interface {
return utilexec.New()
}

func (expc *expandController) GetHostName() string {
Expand Down
1 change: 1 addition & 0 deletions pkg/controller/volume/persistentvolume/BUILD
Expand Up @@ -56,6 +56,7 @@ go_library(
"//staging/src/k8s.io/cloud-provider/volume/errors:go_default_library",
"//staging/src/k8s.io/csi-translation-lib:go_default_library",
"//vendor/k8s.io/klog:go_default_library",
"//vendor/k8s.io/utils/exec:go_default_library",
],
)

Expand Down
6 changes: 4 additions & 2 deletions pkg/controller/volume/persistentvolume/volume_host.go
Expand Up @@ -27,6 +27,8 @@ import (
"k8s.io/client-go/tools/record"
cloudprovider "k8s.io/cloud-provider"
"k8s.io/klog"
utilexec "k8s.io/utils/exec"

"k8s.io/kubernetes/pkg/util/mount"
vol "k8s.io/kubernetes/pkg/volume"
"k8s.io/kubernetes/pkg/volume/util/subpath"
Expand Down Expand Up @@ -116,8 +118,8 @@ func (ctrl *PersistentVolumeController) DeleteServiceAccountTokenFunc() func(typ
}
}

func (adc *PersistentVolumeController) GetExec(pluginName string) mount.Exec {
return mount.NewOSExec()
func (adc *PersistentVolumeController) GetExec(pluginName string) utilexec.Interface {
return utilexec.New()
}

func (ctrl *PersistentVolumeController) GetNodeLabels() (map[string]string, error) {
Expand Down
8 changes: 5 additions & 3 deletions pkg/kubelet/volume_host.go
Expand Up @@ -34,6 +34,8 @@ import (
"k8s.io/client-go/tools/cache"
"k8s.io/client-go/tools/record"
cloudprovider "k8s.io/cloud-provider"
utilexec "k8s.io/utils/exec"

"k8s.io/kubernetes/pkg/features"
"k8s.io/kubernetes/pkg/kubelet/configmap"
"k8s.io/kubernetes/pkg/kubelet/secret"
Expand Down Expand Up @@ -86,7 +88,7 @@ func NewInitializedVolumePluginMgr(
informerFactory: informerFactory,
csiDriverLister: csiDriverLister,
csiDriversSynced: csiDriversSynced,
exec: mount.NewOSExec(),
exec: utilexec.New(),
}

if err := kvh.volumePluginMgr.InitPlugins(plugins, prober, kvh); err != nil {
Expand Down Expand Up @@ -115,7 +117,7 @@ type kubeletVolumeHost struct {
informerFactory informers.SharedInformerFactory
csiDriverLister storagelisters.CSIDriverLister
csiDriversSynced cache.InformerSynced
exec mount.Exec
exec utilexec.Interface
}

func (kvh *kubeletVolumeHost) SetKubeletError(err error) {
Expand Down Expand Up @@ -271,6 +273,6 @@ func (kvh *kubeletVolumeHost) GetEventRecorder() record.EventRecorder {
return kvh.kubelet.recorder
}

func (kvh *kubeletVolumeHost) GetExec(pluginName string) mount.Exec {
func (kvh *kubeletVolumeHost) GetExec(pluginName string) utilexec.Interface {
return kvh.exec
}
2 changes: 1 addition & 1 deletion pkg/util/mount/BUILD
Expand Up @@ -4,7 +4,6 @@ go_library(
name = "go_default_library",
srcs = [
"doc.go",
"exec.go",
"fake_exec.go",
"fake_mounter.go",
"mount.go",
Expand Down Expand Up @@ -75,6 +74,7 @@ go_test(
],
embed = [":go_default_library"],
deps = [
"//vendor/k8s.io/utils/exec:go_default_library",
"//vendor/k8s.io/utils/exec/testing:go_default_library",
] + select({
"@io_bazel_rules_go//go/platform:windows": [
Expand Down
36 changes: 0 additions & 36 deletions pkg/util/mount/exec.go

This file was deleted.

11 changes: 3 additions & 8 deletions pkg/util/mount/mount.go
Expand Up @@ -23,6 +23,8 @@ import (
"os"
"path/filepath"
"strings"

utilexec "k8s.io/utils/exec"
)

const (
Expand Down Expand Up @@ -53,13 +55,6 @@ type Interface interface {
GetMountRefs(pathname string) ([]string, error)
}

// Exec is an interface for executing commands on systems.
type Exec interface {
// Run executes a command and returns its stdout + stderr combined in one
// stream.
Run(cmd string, args ...string) ([]byte, error)
}

// Compile-time check to ensure all Mounter implementations satisfy
// the mount interface.
var _ Interface = &Mounter{}
Expand All @@ -79,7 +74,7 @@ type MountPoint struct {
// mounts it otherwise the device is formatted first then mounted.
type SafeFormatAndMount struct {
Interface
Exec
Exec utilexec.Interface
}

// FormatAndMount formats the given disk, if needed, and mounts it.
Expand Down
6 changes: 3 additions & 3 deletions pkg/util/mount/mount_linux.go
Expand Up @@ -273,7 +273,7 @@ func (mounter *SafeFormatAndMount) formatAndMount(source string, target string,
// Run fsck on the disk to fix repairable issues, only do this for volumes requested as rw.
klog.V(4).Infof("Checking for issues with fsck on disk: %s", source)
args := []string{"-a", source}
out, err := mounter.Exec.Run("fsck", args...)
out, err := mounter.Exec.Command("fsck", args...).CombinedOutput()
if err != nil {
ee, isExitError := err.(utilexec.ExitError)
switch {
Expand Down Expand Up @@ -320,7 +320,7 @@ func (mounter *SafeFormatAndMount) formatAndMount(source string, target string,
}
}
klog.Infof("Disk %q appears to be unformatted, attempting to format as type: %q with options: %v", source, fstype, args)
_, err := mounter.Exec.Run("mkfs."+fstype, args...)
_, err := mounter.Exec.Command("mkfs."+fstype, args...).CombinedOutput()
if err == nil {
// the disk has been formatted successfully try to mount it again.
klog.Infof("Disk successfully formatted (mkfs): %s - %s %s", fstype, source, target)
Expand All @@ -344,7 +344,7 @@ func (mounter *SafeFormatAndMount) formatAndMount(source string, target string,
func (mounter *SafeFormatAndMount) GetDiskFormat(disk string) (string, error) {
args := []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", disk}
klog.V(4).Infof("Attempting to determine if disk %q is formatted using blkid with args: (%v)", disk, args)
dataOut, err := mounter.Exec.Run("blkid", args...)
dataOut, err := mounter.Exec.Command("blkid", args...).CombinedOutput()
output := string(dataOut)
klog.V(4).Infof("Output: %q, err: %v", output, err)

Expand Down
9 changes: 5 additions & 4 deletions pkg/util/mount/mount_windows.go
Expand Up @@ -26,6 +26,7 @@ import (
"strings"

"k8s.io/klog"
utilexec "k8s.io/utils/exec"
"k8s.io/utils/keymutex"
utilpath "k8s.io/utils/path"
)
Expand Down Expand Up @@ -219,7 +220,7 @@ func (mounter *SafeFormatAndMount) formatAndMount(source string, target string,
// format disk if it is unformatted(raw)
cmd := fmt.Sprintf("Get-Disk -Number %s | Where partitionstyle -eq 'raw' | Initialize-Disk -PartitionStyle MBR -PassThru"+
" | New-Partition -AssignDriveLetter -UseMaximumSize | Format-Volume -FileSystem %s -Confirm:$false", source, fstype)
if output, err := mounter.Exec.Run("powershell", "/c", cmd); err != nil {
if output, err := mounter.Exec.Command("powershell", "/c", cmd).CombinedOutput(); err != nil {
return fmt.Errorf("diskMount: format disk failed, error: %v, output: %q", err, string(output))
}
klog.V(4).Infof("diskMount: Disk successfully formatted, disk: %q, fstype: %q", source, fstype)
Expand All @@ -231,17 +232,17 @@ func (mounter *SafeFormatAndMount) formatAndMount(source string, target string,
driverPath := driveLetter + ":"
target = NormalizeWindowsPath(target)
klog.V(4).Infof("Attempting to formatAndMount disk: %s %s %s", fstype, driverPath, target)
if output, err := mounter.Exec.Run("cmd", "/c", "mklink", "/D", target, driverPath); err != nil {
if output, err := mounter.Exec.Command("cmd", "/c", "mklink", "/D", target, driverPath).CombinedOutput(); err != nil {
klog.Errorf("mklink failed: %v, output: %q", err, string(output))
return err
}
return nil
}

// Get drive letter according to windows disk number
func getDriveLetterByDiskNumber(diskNum string, exec Exec) (string, error) {
func getDriveLetterByDiskNumber(diskNum string, exec utilexec.Interface) (string, error) {
cmd := fmt.Sprintf("(Get-Partition -DiskNumber %s).DriveLetter", diskNum)
output, err := exec.Run("powershell", "/c", cmd)
output, err := exec.Command("powershell", "/c", cmd).CombinedOutput()
if err != nil {
return "", fmt.Errorf("azureMount: Get Drive Letter failed: %v, output: %q", err, string(output))
}
Expand Down
80 changes: 40 additions & 40 deletions pkg/util/mount/mount_windows_test.go
Expand Up @@ -24,10 +24,10 @@ import (
"os"
"os/exec"
"path/filepath"
"strings"
"testing"

"github.com/stretchr/testify/assert"
"k8s.io/utils/exec/testing"
)

func makeLink(link, target string) error {
Expand Down Expand Up @@ -224,61 +224,61 @@ func TestIsLikelyNotMountPoint(t *testing.T) {
}

func TestFormatAndMount(t *testing.T) {
fakeMounter := ErrorMounter{NewFakeMounter(nil), 0, nil}
execCallback := func(cmd string, args ...string) ([]byte, error) {
for j := range args {
if strings.Contains(args[j], "Get-Disk -Number") {
return []byte("0"), nil
}

if strings.Contains(args[j], "Get-Partition -DiskNumber") {
return []byte("0"), nil
}

if strings.Contains(args[j], "mklink") {
return nil, nil
}
}
return nil, fmt.Errorf("Unexpected cmd %s, args %v", cmd, args)
}
fakeExec := NewFakeExec(execCallback)

mounter := SafeFormatAndMount{
Interface: &fakeMounter,
Exec: fakeExec,
}

tests := []struct {
device string
target string
fstype string
execScripts []ExecArgs
mountOptions []string
expectError bool
}{
{
"0",
"disk",
"NTFS",
[]string{},
false,
device: "0",
target: "disk",
fstype: "NTFS",
execScripts: []ExecArgs{
{"powershell", []string{"/c", "Get-Disk", "-Number"}, "0", nil},
{"powershell", []string{"/c", "Get-Partition", "-DiskNumber"}, "0", nil},
{"cmd", []string{"/c", "mklink", "/D"}, "", nil},
},
mountOptions: []string{},
expectError: false,
},
{
"0",
"disk",
"",
[]string{},
false,
device: "0",
target: "disk",
fstype: "",
execScripts: []ExecArgs{
{"powershell", []string{"/c", "Get-Disk", "-Number"}, "0", nil},
{"powershell", []string{"/c", "Get-Partition", "-DiskNumber"}, "0", nil},
{"cmd", []string{"/c", "mklink", "/D"}, "", nil},
},
mountOptions: []string{},
expectError: false,
},
{
"invalidDevice",
"disk",
"NTFS",
[]string{},
true,
device: "invalidDevice",
target: "disk",
fstype: "NTFS",
mountOptions: []string{},
expectError: true,
},
}

for _, test := range tests {
fakeMounter := ErrorMounter{NewFakeMounter(nil), 0, nil}
fakeExec := &testingexec.FakeExec{}
for _, script := range test.execScripts {
fakeCmd := &testingexec.FakeCmd{}
cmdAction := makeFakeCmd(fakeCmd, script.command, script.args...)
outputAction := makeFakeOutput(script.output, script.err)
fakeCmd.CombinedOutputScript = append(fakeCmd.CombinedOutputScript, outputAction)
fakeExec.CommandScript = append(fakeExec.CommandScript, cmdAction)
}
mounter := SafeFormatAndMount{
Interface: &fakeMounter,
Exec: fakeExec,
}
base, err := ioutil.TempDir("", test.device)
if err != nil {
t.Fatalf(err.Error())
Expand Down