Retrieve error handling (#453)

* Add errgroup dependency

Signed-off-by: liz <liz@heptio.com>

* 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>

Gopkg.toml

@@ -80,3 +80,7 @@
 [[constraint]]
  branch = "release-8.0"
  name = "k8s.io/client-go"
+
+[[constraint]]
+ branch = "master"
+ name = "golang.org/x/sync"

cmd/sonobuoy/app/retrieve.go

@@ -17,13 +17,16 @@ limitations under the License.
 package app
 
 import (
+ "fmt"
  "os"
  "path/filepath"
 
  "github.com/heptio/sonobuoy/pkg/client"
  "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)
  }
 }

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.

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 **/