From 61b1e29b93816bf62e912528c93f7878903b79c7 Mon Sep 17 00:00:00 2001
From: liz <liz@heptio.com>
Date: Wed, 27 Jun 2018 16:04:18 -0400
Subject: [PATCH] client.Retrieve now returns an error channel instead of error

sonobuoy retrieve now has better, more user-friendly error handling

closes #407

Signed-off-by: liz <liz@heptio.com>
---
 cmd/sonobuoy/app/retrieve.go | 19 ++++++++++++++----
 pkg/client/interfaces.go     |  2 +-
 pkg/client/retrieve.go       | 37 +++++++++++++++++++-----------------
 3 files changed, 36 insertions(+), 22 deletions(-)

diff --git a/cmd/sonobuoy/app/retrieve.go b/cmd/sonobuoy/app/retrieve.go
index 026283cfd..afd411c39 100644
--- a/cmd/sonobuoy/app/retrieve.go
+++ b/cmd/sonobuoy/app/retrieve.go
@@ -17,6 +17,7 @@ limitations under the License.
 package app
 
 import (
+	"fmt"
 	"os"
 	"path/filepath"
 
@@ -24,6 +25,8 @@ import (
 	"github.com/heptio/sonobuoy/pkg/errlog"
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
+	"golang.org/x/sync/errgroup"
+	"k8s.io/client-go/util/exec"
 )
 
 var (
@@ -70,15 +73,23 @@ func retrieveResults(cmd *cobra.Command, args []string) {
 	}
 
 	// Get a reader that contains the tar output of the results directory.
-	reader, err := sbc.RetrieveResults(&client.RetrieveConfig{Namespace: rcvFlags.namespace})
+	reader, ec := sbc.RetrieveResults(&client.RetrieveConfig{Namespace: rcvFlags.namespace})
 	if err != nil {
 		errlog.LogError(err)
 		os.Exit(1)
 	}
 
-	// Extract the tar output into a local directory under the prefix.
-	err = client.UntarAll(reader, outDir, prefix)
-	if err != nil {
+	eg := &errgroup.Group{}
+	eg.Go(func() error { return <-ec })
+	eg.Go(func() error { return client.UntarAll(reader, outDir, prefix) })
+
+	err = eg.Wait()
+	if _, ok := err.(exec.CodeExitError); ok {
+		fmt.Fprintln(os.Stderr, "Results not ready yet. Check `sonobuoy status` for status.")
 		os.Exit(1)
+
+	} else if err != nil {
+		fmt.Fprintf(os.Stderr, "error retrieving results: %v\n", err)
+		os.Exit(2)
 	}
 }
diff --git a/pkg/client/interfaces.go b/pkg/client/interfaces.go
index b50a9ab81..34627b108 100644
--- a/pkg/client/interfaces.go
+++ b/pkg/client/interfaces.go
@@ -128,7 +128,7 @@ type Interface interface {
 	// GenerateManifest fills in a template with a Sonobuoy config
 	GenerateManifest(cfg *GenConfig) ([]byte, error)
 	// RetrieveResults copies results from a sonobuoy run into a Reader in tar format.
-	RetrieveResults(cfg *RetrieveConfig) (io.Reader, error)
+	RetrieveResults(cfg *RetrieveConfig) (io.Reader, <-chan error)
 	// GetStatus determines the status of the sonobuoy run in order to assist the user.
 	GetStatus(namespace string) (*aggregation.Status, error)
 	// LogReader returns a reader that contains a merged stream of sonobuoy logs.
diff --git a/pkg/client/retrieve.go b/pkg/client/retrieve.go
index c284aec8d..507ecd9f0 100644
--- a/pkg/client/retrieve.go
+++ b/pkg/client/retrieve.go
@@ -24,8 +24,6 @@ import (
 	"path"
 	"path/filepath"
 
-	"github.com/sirupsen/logrus"
-
 	"github.com/heptio/sonobuoy/pkg/config"
 	"github.com/pkg/errors"
 
@@ -34,10 +32,19 @@ import (
 	"k8s.io/client-go/tools/remotecommand"
 )
 
-func (c *SonobuoyClient) RetrieveResults(cfg *RetrieveConfig) (io.Reader, error) {
+var tarCommand = []string{
+	"/usr/bin/env",
+	"bash",
+	"-c",
+	fmt.Sprintf("tar cf - %s/*.tar.gz", config.MasterResultsPath),
+}
+
+func (c *SonobuoyClient) RetrieveResults(cfg *RetrieveConfig) (io.Reader, <-chan error) {
+	ec := make(chan error, 1)
 	client, err := c.Client()
 	if err != nil {
-		return nil, err
+		ec <- err
+		return nil, ec
 	}
 	restClient := client.CoreV1().RESTClient()
 	req := restClient.Post().
@@ -48,34 +55,30 @@ func (c *SonobuoyClient) RetrieveResults(cfg *RetrieveConfig) (io.Reader, error)
 		Param("container", config.MasterContainerName)
 	req.VersionedParams(&corev1.PodExecOptions{
 		Container: config.MasterContainerName,
-		Command:   []string{"tar", "cf", "-", config.MasterResultsPath},
+		Command:   tarCommand,
 		Stdin:     false,
 		Stdout:    true,
-		Stderr:    true,
+		Stderr:    false,
 	}, scheme.ParameterCodec)
 	executor, err := remotecommand.NewSPDYExecutor(c.RestConfig, "POST", req.URL())
 	if err != nil {
-		return nil, err
+		ec <- err
+		return nil, ec
 	}
 	reader, writer := io.Pipe()
-	go func(writer *io.PipeWriter) {
+	go func(writer *io.PipeWriter, ec chan error) {
 		defer writer.Close()
+		defer close(ec)
 		err = executor.Stream(remotecommand.StreamOptions{
 			Stdout: writer,
-			Stderr: os.Stderr,
 			Tty:    false,
 		})
 		if err != nil {
-			// Since this function returns an io.Reader to the consumer and does
-			// not buffer the entire (potentially large) output, RetrieveResults
-			// has to return the reader first to be read from. This means we
-			// either lose this error (easy) or provide a significantly more
-			// complex error mechanism for the consumer (hard).
-			logrus.Error(err)
+			ec <- err
 		}
-	}(writer)
+	}(writer, ec)
 
-	return reader, nil
+	return reader, ec
 }
 
 /** Everything below this marker has been copy/pasta'd from k8s/k8s. The only modification is exporting UntarAll **/