feedc0de/my-rook-config
1
0
Fork 0
forked from rook/rook
Code Pull Requests Activity

exec: simplify the exec package from unused methods and logging

Browse Source 
The methods and arguments to the exec methods are not all used anymore.
This cleans up the methods to only what is necessary to improve
the readability and maintainability.

Signed-off-by: Travis Nielsen <tnielsen@redhat.com>
This commit is contained in:
Travis Nielsen
2020-03-19 07:49:53 -06:00
parent 18b0e7d295
commit 84b8cdcf75
4 changed files with 41 additions and 66 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/util/exec/exec.go
+38 -47
View File
@@ -31,39 +31,26 @@ import (
)
type Executor interface {
StartExecuteCommand(debug bool, actionName string, command string, arg ...string) (*exec.Cmd, error)
ExecuteCommand(debug bool, actionName string, command string, arg ...string) error
ExecuteCommandWithOutput(debug bool, actionName string, command string, arg ...string) (string, error)
ExecuteCommandWithCombinedOutput(debug bool, actionName string, command string, arg ...string) (string, error)
ExecuteCommandWithOutputFile(debug bool, actionName, command, outfileArg string, arg ...string) (string, error)
ExecuteCommandWithOutputFileTimeout(debug bool, timeout time.Duration, actionName, command, outfileArg string, arg ...string) (string, error)
ExecuteCommandWithTimeout(debug bool, timeout time.Duration, actionName string, command string, arg ...string) (string, error)
ExecuteCommand(suppressLogOutput bool, actionName string, command string, arg ...string) error
ExecuteCommandWithOutput(suppressLogCommand bool, actionName string, command string, arg ...string) (string, error)
ExecuteCommandWithCombinedOutput(suppressLogOutput bool, actionName string, command string, arg ...string) (string, error)
ExecuteCommandWithOutputFile(suppressLogOutput bool, actionName, command, outfileArg string, arg ...string) (string, error)
ExecuteCommandWithOutputFileTimeout(suppressLogOutput bool, timeout time.Duration, actionName, command, outfileArg string, arg ...string) (string, error)
ExecuteCommandWithTimeout(suppressLogOutput bool, timeout time.Duration, actionName string, command string, arg ...string) (string, error)
ExecuteStat(name string) (os.FileInfo, error)
}
type CommandExecutor struct {
}
// Start a process and return immediately
func (*CommandExecutor) StartExecuteCommand(debug bool, actionName string, command string, arg ...string) (*exec.Cmd, error) {
cmd, stdout, stderr, err := startCommand(debug, command, arg...)
if err != nil {
return cmd, createCommandError(err, actionName)
}
go logOutput(actionName, stdout, stderr)
return cmd, nil
}
// Start a process and wait for its completion
func (*CommandExecutor) ExecuteCommand(debug bool, actionName string, command string, arg ...string) error {
cmd, stdout, stderr, err := startCommand(debug, command, arg...)
func (*CommandExecutor) ExecuteCommand(suppressLogOutput bool, actionName string, command string, arg ...string) error {
cmd, stdout, stderr, err := startCommand(suppressLogOutput, command, arg...)
if err != nil {
return createCommandError(err, actionName)
}
logOutput(actionName, stdout, stderr)
logOutput(suppressLogOutput, actionName, stdout, stderr)
if err := cmd.Wait(); err != nil {
return createCommandError(err, actionName)
@@ -73,8 +60,8 @@ func (*CommandExecutor) ExecuteCommand(debug bool, actionName string, command st
}
// ExecuteCommandWithTimeout starts a process and wait for its completion with timeout.
func (*CommandExecutor) ExecuteCommandWithTimeout(debug bool, timeout time.Duration, actionName string, command string, arg ...string) (string, error) {
logCommand(debug, command, arg...)
func (*CommandExecutor) ExecuteCommandWithTimeout(suppressLogOutput bool, timeout time.Duration, actionName string, command string, arg ...string) (string, error) {
logCommand(suppressLogOutput, command, arg...)
cmd := exec.Command(command, arg...)
var b bytes.Buffer
@@ -90,11 +77,11 @@ func (*CommandExecutor) ExecuteCommandWithTimeout(debug bool, timeout time.Durat
done <- cmd.Wait()
}()
interrupSent := false
interruptSent := false
for {
select {
case <-time.After(timeout):
if interrupSent {
if interruptSent {
logger.Infof("Timeout waiting for process %s to return after interrupt signal was sent. Sending kill signal to the process", command)
var e error
if err := cmd.Process.Kill(); err != nil {
@@ -111,12 +98,12 @@ func (*CommandExecutor) ExecuteCommandWithTimeout(debug bool, timeout time.Durat
logger.Errorf("Failed to send interrupt signal to process %s: %+v", command, err)
// kill signal will be sent next loop
}
interrupSent = true
interruptSent = true
case err := <-done:
if err != nil {
return strings.TrimSpace(b.String()), createCommandError(err, command)
}
if interrupSent {
if interruptSent {
e := fmt.Errorf("Timeout waiting for the command %s to return", command)
return strings.TrimSpace(b.String()), createCommandError(e, command)
}
@@ -125,20 +112,20 @@ func (*CommandExecutor) ExecuteCommandWithTimeout(debug bool, timeout time.Durat
}
}
func (*CommandExecutor) ExecuteCommandWithOutput(debug bool, actionName string, command string, arg ...string) (string, error) {
logCommand(debug, command, arg...)
func (*CommandExecutor) ExecuteCommandWithOutput(suppressLogOutput bool, actionName string, command string, arg ...string) (string, error) {
logCommand(suppressLogOutput, command, arg...)
cmd := exec.Command(command, arg...)
return runCommandWithOutput(actionName, cmd, false)
}
func (*CommandExecutor) ExecuteCommandWithCombinedOutput(debug bool, actionName string, command string, arg ...string) (string, error) {
logCommand(debug, command, arg...)
func (*CommandExecutor) ExecuteCommandWithCombinedOutput(suppressLogOutput bool, actionName string, command string, arg ...string) (string, error) {
logCommand(suppressLogOutput, command, arg...)
cmd := exec.Command(command, arg...)
return runCommandWithOutput(actionName, cmd, true)
}
// Same as ExecuteCommandWithOutputFile but with a timeout limit.
func (*CommandExecutor) ExecuteCommandWithOutputFileTimeout(debug bool, timeout time.Duration, actionName string,
func (*CommandExecutor) ExecuteCommandWithOutputFileTimeout(suppressLogOutput bool, timeout time.Duration, actionName string,
command, outfileArg string, arg ...string) (string, error) {
outFile, err := ioutil.TempFile("", "")
@@ -149,7 +136,7 @@ func (*CommandExecutor) ExecuteCommandWithOutputFileTimeout(debug bool, timeout
defer os.Remove(outFile.Name())
arg = append(arg, outfileArg, outFile.Name())
logCommand(debug, command, arg...)
logCommand(suppressLogOutput, command, arg...)
ctx, cancel := context.WithTimeout(context.Background(), timeout)
defer cancel()
@@ -160,7 +147,7 @@ func (*CommandExecutor) ExecuteCommandWithOutputFileTimeout(debug bool, timeout
// if there was anything that went to stdout/stderr then log it, even before
// we return an error
if string(cmdOut) != "" {
logger.Info(string(cmdOut))
writeToLog(suppressLogOutput, string(cmdOut))
}
if ctx.Err() == context.DeadlineExceeded {
@@ -175,7 +162,7 @@ func (*CommandExecutor) ExecuteCommandWithOutputFileTimeout(debug bool, timeout
return string(fileOut), err
}
func (*CommandExecutor) ExecuteCommandWithOutputFile(debug bool, actionName string, command, outfileArg string, arg ...string) (string, error) {
func (*CommandExecutor) ExecuteCommandWithOutputFile(suppressLogOutput bool, actionName string, command, outfileArg string, arg ...string) (string, error) {
// create a temporary file to serve as the output file for the command to be run and ensure
// it is cleaned up after this function is done
@@ -189,12 +176,12 @@ func (*CommandExecutor) ExecuteCommandWithOutputFile(debug bool, actionName stri
// append the output file argument to the list or args
arg = append(arg, outfileArg, outFile.Name())
logCommand(debug, command, arg...)
logCommand(suppressLogOutput, command, arg...)
cmd := exec.Command(command, arg...)
cmdOut, err := cmd.CombinedOutput()
// if there was anything that went to stdout/stderr then log it, even before we return an error
if string(cmdOut) != "" {
logger.Debug(string(cmdOut))
writeToLog(suppressLogOutput, string(cmdOut))
}
if err != nil {
return string(cmdOut), err
@@ -205,8 +192,8 @@ func (*CommandExecutor) ExecuteCommandWithOutputFile(debug bool, actionName stri
return string(fileOut), err
}
func startCommand(debug bool, command string, arg ...string) (*exec.Cmd, io.ReadCloser, io.ReadCloser, error) {
logCommand(debug, command, arg...)
func startCommand(suppressLogOutput bool, command string, arg ...string) (*exec.Cmd, io.ReadCloser, io.ReadCloser, error) {
logCommand(suppressLogOutput, command, arg...)
cmd := exec.Command(command, arg...)
stdout, err := cmd.StdoutPipe()
@@ -228,16 +215,16 @@ func (*CommandExecutor) ExecuteStat(name string) (os.FileInfo, error) {
}
// read from reader line by line and write it to the log
func logFromReader(logger *capnslog.PackageLogger, reader io.ReadCloser) {
func logFromReader(suppressLogOutput bool, logger *capnslog.PackageLogger, reader io.ReadCloser) {
in := bufio.NewScanner(reader)
lastLine := ""
for in.Scan() {
lastLine = in.Text()
logger.Info(lastLine)
writeToLog(suppressLogOutput, lastLine)
}
}
func logOutput(name string, stdout, stderr io.ReadCloser) {
func logOutput(suppressLogOutput bool, name string, stdout, stderr io.ReadCloser) {
if stdout == nil || stderr == nil {
logger.Warningf("failed to collect stdout and stderr")
return
@@ -254,8 +241,8 @@ func logOutput(name string, stdout, stderr io.ReadCloser) {
}
}
go logFromReader(childLogger, stderr)
logFromReader(childLogger, stdout)
go logFromReader(suppressLogOutput, childLogger, stderr)
logFromReader(suppressLogOutput, childLogger, stdout)
}
func runCommandWithOutput(actionName string, cmd *exec.Cmd, combinedOutput bool) (string, error) {
@@ -277,9 +264,13 @@ func runCommandWithOutput(actionName string, cmd *exec.Cmd, combinedOutput bool)
return out, nil
}
func logCommand(debug bool, command string, arg ...string) {
func logCommand(suppressLogOutput bool, command string, arg ...string) {
msg := fmt.Sprintf("Running command: %s %s", command, strings.Join(arg, " "))
if debug {
writeToLog(suppressLogOutput, msg)
}
func writeToLog(suppressLogOutput bool, msg string) {
if suppressLogOutput {
logger.Debug(msg)
} else {
logger.Info(msg)
pkg/util/exec/test/mockexec.go
-9
View File
@@ -41,15 +41,6 @@ func (e *MockExecutor) ExecuteCommand(debug bool, actionName string, command str
return nil
}
func (e *MockExecutor) StartExecuteCommand(debug bool, actionName string, command string, arg ...string) (*exec.Cmd, error) {
if e.MockStartExecuteCommand != nil {
return e.MockStartExecuteCommand(debug, actionName, command, arg...)
}
args := []string{command}
return &exec.Cmd{Args: append(args, arg...)}, nil
}
func (e *MockExecutor) ExecuteCommandWithOutput(debug bool, actionName string, command string, arg ...string) (string, error) {
if e.MockExecuteCommandWithOutput != nil {
return e.MockExecuteCommandWithOutput(debug, actionName, command, arg...)
pkg/util/exec/translate_exec.go
-7
View File
@@ -18,7 +18,6 @@ package exec
import (
"fmt"
"os"
"os/exec"
"time"
)
@@ -34,12 +33,6 @@ type TranslateCommandExecutor struct {
Translator func(debug bool, actionName string, command string, arg ...string) (string, []string)
}
// StartExecuteCommand starts a process and return immediately
func (e *TranslateCommandExecutor) StartExecuteCommand(debug bool, actionName string, command string, arg ...string) (*exec.Cmd, error) {
transCommand, transArgs := e.Translator(debug, actionName, command, arg...)
return e.Executor.StartExecuteCommand(debug, actionName, transCommand, transArgs...)
}
// ExecuteCommand starts a process and wait for its completion
func (e *TranslateCommandExecutor) ExecuteCommand(debug bool, actionName string, command string, arg ...string) error {
transCommand, transArgs := e.Translator(debug, actionName, command, arg...)
tests/framework/utils/k8s_helper.go
+3 -3
View File
@@ -117,7 +117,7 @@ func (k8sh *K8sHelper) GetDockerImage(image string) error {
if dockercmd == "" {
dockercmd = "docker"
}
return k8sh.executor.ExecuteCommand(false, "", dockercmd, "pull", image)
return k8sh.executor.ExecuteCommand(false, dockercmd, "pull", image)
}
// SetDeploymentVersion sets the container version on the deployment. It is assumed to be the rook/ceph image.
@@ -162,9 +162,9 @@ func (k8sh *K8sHelper) KubectlWithStdin(stdin string, args ...string) (string, e
}
func getKubeConfig(executor exec.Executor) (*rest.Config, error) {
context, err := executor.ExecuteCommandWithOutput(false, "", "kubectl", "config", "view", "-o", "json")
context, err := executor.ExecuteCommandWithOutput(false, "kubectl", "config", "view", "-o", "json")
if err != nil {
k8slogger.Errorf("Errors Encountered while executing kubectl command : %v", err)
k8slogger.Errorf("failed to execute kubectl command. %v", err)
}
// Parse the kubectl context to get the settings for client connections