ExecWithOption - get rid of stdout/stderr buffers (#2506)

* `ExecWithOptions` should not return stdout / stdin

Output could be obtained by writers passed via `ExecOptions`

* Adjust `PodCommandExecutor` and tests

pkg/kube/exec.go

@@ -81,14 +81,21 @@ type ExecOptions struct {
 // Exec is our version of the call to `kubectl exec` that does not depend on
 // k8s.io/kubernetes.
 func Exec(cli kubernetes.Interface, namespace, pod, container string, command []string, stdin io.Reader) (string, string, error) {
+	outbuf := &bytes.Buffer{}
+	errbuf := &bytes.Buffer{}
 	opts := ExecOptions{
 		Command:       command,
 		Namespace:     namespace,
 		PodName:       pod,
 		ContainerName: container,
 		Stdin:         stdin,
+		Stdout:        outbuf,
+		Stderr:        errbuf,
 	}
-	return ExecWithOptions(cli, opts)
+
+	err := ExecWithOptions(cli, opts)
+
+	return strings.TrimSpace(outbuf.String()), strings.TrimSpace(errbuf.String()), err
 }
 
 // ExecOutput is similar to Exec, except that inbound outputs are written to the
@@ -113,32 +120,20 @@ func ExecOutput(cli kubernetes.Interface, namespace, pod, container string, comm
 		},
 	}
 
-	_, _, err := ExecWithOptions(cli, opts)
-	return err
+	return ExecWithOptions(cli, opts)
 }
 
-// ExecWithOptions executes a command in the specified container,
-// returning stdout, stderr and error. `options` allowed for
-// additional parameters to be passed.
-func ExecWithOptions(kubeCli kubernetes.Interface, options ExecOptions) (string, string, error) {
+// ExecWithOptions executes a command in the specified container, returning an error.
+// `options` allowed for additional parameters to be passed.
+func ExecWithOptions(kubeCli kubernetes.Interface, options ExecOptions) error {
 	config, err := LoadConfig()
 	if err != nil {
-		return "", "", err
-	}
-
-	outbuf := &bytes.Buffer{}
-	if options.Stdout == nil {
-		options.Stdout = outbuf
-	}
-
-	errbuf := &bytes.Buffer{}
-	if options.Stderr == nil {
-		options.Stderr = errbuf
+		return err
 	}
 
 	errCh := execStream(kubeCli, config, options)
 	err = <-errCh
-	return strings.TrimSpace(outbuf.String()), strings.TrimSpace(errbuf.String()), errors.Wrap(err, "Failed to exec command in pod")
+	return errors.Wrap(err, "Failed to exec command in pod")
 }
 
 func execStream(kubeCli kubernetes.Interface, config *restclient.Config, options ExecOptions) chan error {

pkg/kube/exec_test.go

@@ -139,7 +139,7 @@ func (s *ExecSuite) TestExecWithWriterOptions(c *C) {
 			Stdout:        bufout,
 			Stderr:        buferr,
 		}
-		_, _, err := ExecWithOptions(s.cli, opts)
+		err := ExecWithOptions(s.cli, opts)
 		c.Assert(err, IsNil)
 		c.Assert(bufout.String(), Equals, testCase.expectedOut)
 		c.Assert(buferr.String(), Equals, testCase.expectedErr)
@@ -187,7 +187,7 @@ func (s *ExecSuite) TestErrorInExecWithOptions(c *C) {
 			ContainerName: "", // use default container
 			Stdin:         nil,
 		}
-		_, _, err1 := ExecWithOptions(s.cli, opts) // Output is not needed
+		err1 := ExecWithOptions(s.cli, opts)
 		c.Assert(err1, Not(IsNil))
 
 		var ee1 *ExecError
@@ -204,7 +204,7 @@ func (s *ExecSuite) TestErrorInExecWithOptions(c *C) {
 		opts.Stdout = &bufout
 		opts.Stderr = &buferr
 
-		_, _, err2 := ExecWithOptions(s.cli, opts) // Output is not needed
+		err2 := ExecWithOptions(s.cli, opts)
 		c.Assert(err2, Not(IsNil))
 
 		var ee2 *ExecError

pkg/kube/pod_command_executor.go

@@ -56,7 +56,7 @@ func (p *podCommandExecutor) Exec(ctx context.Context, command []string, stdin i
 	)
 
 	go func() {
-		_, _, err = p.pcep.ExecWithOptions(opts)
+		err = p.pcep.ExecWithOptions(opts)
 		close(cmdDone)
 	}()
 

pkg/kube/pod_command_executor_processor.go

@@ -21,7 +21,7 @@ import (
 // PodCommandExecutorProcessor is an interface wrapping kubernetes API invocation
 // it is purposed to be replaced by fake implementation in tests
 type PodCommandExecutorProcessor interface {
-	ExecWithOptions(opts ExecOptions) (string, string, error)
+	ExecWithOptions(opts ExecOptions) error
 }
 
 type podCommandExecutorProcessor struct {
@@ -30,6 +30,6 @@ type podCommandExecutorProcessor struct {
 
 // ExecWithOptions executes a command in the specified pod and container,
 // returning stdout, stderr and error.
-func (p *podCommandExecutorProcessor) ExecWithOptions(opts ExecOptions) (string, string, error) {
+func (p *podCommandExecutorProcessor) ExecWithOptions(opts ExecOptions) error {
 	return ExecWithOptions(p.cli, opts)
 }

pkg/kube/pod_command_executor_test.go

@@ -69,8 +69,6 @@ type fakePodCommandExecutorProcessor struct {
 	inExecWithOptionsOpts *ExecOptions
 	execWithOptionsStdout string
 	execWithOptionsStderr string
-	execWithOptionsRet1   string
-	execWithOptionsRet2   string
 	execWithOptionsErr    error
 
 	// Signal to `ExecWithOptions` to start "executing" command.
@@ -79,7 +77,7 @@ type fakePodCommandExecutorProcessor struct {
 	execWithOptionsSyncEnd   testBarrier
 }
 
-func (fprp *fakePodCommandExecutorProcessor) ExecWithOptions(opts ExecOptions) (string, string, error) {
+func (fprp *fakePodCommandExecutorProcessor) ExecWithOptions(opts ExecOptions) error {
 	fprp.inExecWithOptionsOpts = &opts
 	fprp.execWithOptionsSyncStart.SyncWithController()
 	if opts.Stdout != nil && len(fprp.execWithOptionsStdout) > 0 {
@@ -90,7 +88,7 @@ func (fprp *fakePodCommandExecutorProcessor) ExecWithOptions(opts ExecOptions) (
 	}
 	fprp.execWithOptionsSyncEnd.SyncWithController()
 
-	return fprp.execWithOptionsRet1, fprp.execWithOptionsRet2, fprp.execWithOptionsErr
+	return fprp.execWithOptionsErr
 }
 
 func (s *PodCommandExecutorTestSuite) TestPodRunnerExec(c *C) {