diff --git a/pkg/util/exec/exec.go b/pkg/util/exec/exec.go index d99151b93..f64548a96 100644 --- a/pkg/util/exec/exec.go +++ b/pkg/util/exec/exec.go @@ -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) diff --git a/pkg/util/exec/test/mockexec.go b/pkg/util/exec/test/mockexec.go index 6ade83894..652b525a4 100644 --- a/pkg/util/exec/test/mockexec.go +++ b/pkg/util/exec/test/mockexec.go @@ -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...) diff --git a/pkg/util/exec/translate_exec.go b/pkg/util/exec/translate_exec.go index 06ae9a09b..68450d367 100644 --- a/pkg/util/exec/translate_exec.go +++ b/pkg/util/exec/translate_exec.go @@ -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...) diff --git a/tests/framework/utils/k8s_helper.go b/tests/framework/utils/k8s_helper.go index c9f87bc60..ee1df1722 100644 --- a/tests/framework/utils/k8s_helper.go +++ b/tests/framework/utils/k8s_helper.go @@ -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