Merge branch 'error-handling'

Co-authored-by: Jamie Klassen <[email protected]>

accounts/accountant.go

@@ -8,16 +8,18 @@ import (
 	"github.com/concourse/flag"
 )
 
-func NewDBAccountant(pgc flag.PostgresConfig) Accountant {
-	return &DBAccountant{postgresConfig: pgc}
+type AccountantFactory func(Command) Accountant
+
+var DefaultAccountantFactory = func(cmd Command) Accountant {
+	return &DBAccountant{PostgresConfig: cmd.Postgres}
 }
 
 type DBAccountant struct {
-	postgresConfig flag.PostgresConfig
+	PostgresConfig flag.PostgresConfig
 }
 
 func (da *DBAccountant) Account(containers []Container) ([]Sample, error) {
-	conn, err := sql.Open("postgres", da.postgresConfig.ConnectionString())
+	conn, err := sql.Open("postgres", da.PostgresConfig.ConnectionString())
 	if err != nil {
 		return nil, err
 	}

accounts/accountant_test.go

@@ -266,14 +266,16 @@ var _ = Describe("DBAccountant", func() {
 		}
 		createResources(resources)
 		checkResources()
-		accountant := accounts.NewDBAccountant(flag.PostgresConfig{
-			Host:     dbHost(),
-			Port:     5432,
-			User:     "postgres",
-			Password: "password",
-			Database: testDBName(),
-			SSLMode:  "disable",
-		})
+		accountant := &accounts.DBAccountant{
+			PostgresConfig: flag.PostgresConfig{
+				Host:     dbHost(),
+				Port:     5432,
+				User:     "postgres",
+				Password: "password",
+				Database: testDBName(),
+				SSLMode:  "disable",
+			},
+		}
 		Eventually(team.Containers).ShouldNot(BeEmpty())
 		containers := []accounts.Container{}
 		dbContainers, _ := team.Containers()
@@ -357,6 +359,14 @@ var _ = Describe("DBAccountant", func() {
 			}
 			return fakeProcess, nil
 		}
+		fakeGClientContainer.AttachStub = func(ctx context.Context, foo string, pi garden.ProcessIO) (garden.Process, error) {
+			fakeProcess := new(gardenfakes.FakeProcess)
+			fakeProcess.WaitStub = func() (int, error) {
+				io.WriteString(pi.Stdout, "[]")
+				return 0, nil
+			}
+			return fakeProcess, nil
+		}
 		fakeGClient.CreateReturns(fakeGClientContainer, nil)
 		fakeBaggageclaimClient := new(baggageclaimfakes.FakeClient)
 		fakeBaggageclaimVolume := new(baggageclaimfakes.FakeVolume)
@@ -382,14 +392,16 @@ var _ = Describe("DBAccountant", func() {
 
 		// TODO wait for build to complete?
 
-		accountant := accounts.NewDBAccountant(flag.PostgresConfig{
-			Host:     dbHost(),
-			Port:     5432,
-			User:     "postgres",
-			Password: "password",
-			Database: testDBName(),
-			SSLMode:  "disable",
-		})
+		accountant := &accounts.DBAccountant{
+			PostgresConfig: flag.PostgresConfig{
+				Host:     dbHost(),
+				Port:     5432,
+				User:     "postgres",
+				Password: "password",
+				Database: testDBName(),
+				SSLMode:  "disable",
+			},
+		}
 		Eventually(team.Containers).ShouldNot(BeEmpty())
 		containers := []accounts.Container{}
 		dbContainers, _ := team.Containers()

accounts/accounts.go

@@ -1,6 +1,93 @@
 package accounts
 
-import "github.com/concourse/concourse/atc/db"
+import (
+	"fmt"
+	"io"
+	"strings"
+
+	"github.com/concourse/concourse/atc/db"
+	"github.com/concourse/concourse/fly/ui"
+	"github.com/fatih/color"
+	"github.com/jessevdk/go-flags"
+)
+
+func Execute(workerFactory WorkerFactory, accountantFactory AccountantFactory, args []string, stdout io.Writer) int {
+	cmd, err := parseArgs(args)
+	if err != nil {
+		fmt.Fprintln(stdout, err.Error())
+		return flagErrorReturnCode(err)
+	}
+	worker, err := workerFactory(cmd)
+	if err != nil {
+		fmt.Fprintf(stdout, "configuration error: %s\n", err.Error())
+		return 1
+	}
+	accountant := accountantFactory(cmd)
+	containers, err := worker.Containers()
+	if err != nil {
+		fmt.Fprintf(stdout, "worker error: %s\n", err.Error())
+		return 1
+	}
+	samples, err := accountant.Account(containers)
+	if err != nil {
+		fmt.Fprintf(stdout, "accountant error: %s\n", err.Error())
+		return 1
+	}
+	err = printSamples(stdout, samples)
+	if err != nil {
+		return 1
+	}
+	return 0
+}
+
+func flagErrorReturnCode(err error) int {
+	ourErr, ok := err.(*flags.Error)
+	if ok && ourErr.Type == flags.ErrHelp {
+		return 0
+	}
+	return 1
+}
+
+func parseArgs(args []string) (Command, error) {
+	cmd := Command{}
+	parser := flags.NewParser(&cmd, flags.HelpFlag|flags.PassDoubleDash)
+	parser.NamespaceDelimiter = "-"
+	_, err := parser.ParseArgs(args)
+	return cmd, err
+}
+
+func printSamples(writer io.Writer, samples []Sample) error {
+	data := []ui.TableRow{}
+	for _, sample := range samples {
+		workloads := []string{}
+		for _, w := range sample.Labels.Workloads {
+			workloads = append(workloads, w.ToString())
+		}
+		data = append(data, ui.TableRow{
+			ui.TableCell{Contents: sample.Container.Handle},
+			ui.TableCell{Contents: string(sample.Labels.Type)},
+			ui.TableCell{Contents: strings.Join(workloads, ",")},
+		})
+	}
+	table := ui.Table{
+		Headers: ui.TableRow{
+			ui.TableCell{
+				Contents: "handle",
+				Color:    color.New(color.Bold),
+			},
+			ui.TableCell{
+				Contents: "type",
+				Color:    color.New(color.Bold),
+			},
+			ui.TableCell{
+				Contents: "workloads",
+				Color:    color.New(color.Bold),
+			},
+		},
+		Data: data,
+	}
+	return table.Render(writer, true)
+}
 
 type Sample struct {
 	Container Container
@@ -43,11 +130,3 @@ type Worker interface {
 }
 
 type StatsOption func()
-
-func Account(w Worker, a Accountant) ([]Sample, error) {
-	containers, err := w.Containers()
-	if err != nil {
-		return nil, err
-	}
-	return a.Account(containers)
-}

accounts/accounts_test.go

@@ -7,34 +7,110 @@ import (
 	"github.com/concourse/ft/accounts/accountsfakes"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
+	"github.com/onsi/gomega/gbytes"
 )
 
-var _ = Describe("Account", func() {
-	var (
-		worker     *accountsfakes.FakeWorker
-		accountant *accountsfakes.FakeAccountant
-	)
+//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 io.Writer
 
-	BeforeEach(func() {
-		worker = new(accountsfakes.FakeWorker)
-		accountant = new(accountsfakes.FakeAccountant)
-	})
+var _ = Describe("Accounts", func() {
+	Describe("#Execute", func() {
+		It("handles worker errors", func() {
+			buf := gbytes.NewBuffer()
+			fakeWorker := new(accountsfakes.FakeWorker)
+			fakeWorker.ContainersReturns([]accounts.Container{}, errors.New("pod not found"))
 
-	It("accounts the workloads for each container", func() {
-		container := accounts.Container{Handle: "abc123"}
-		containers := []accounts.Container{container}
-		worker.ContainersReturns(containers, nil)
+			returnCode := accounts.Execute(
+				func(accounts.Command) (accounts.Worker, error) {
+					return fakeWorker, nil
+				}, func(accounts.Command) accounts.Accountant {
+					return nil
+				},
+				[]string{},
+				buf,
+			)
 
-		accounts.Account(worker, accountant)
+			Expect(returnCode).To(Equal(1))
+			Expect(buf).To(gbytes.Say("worker error: pod not found\n"))
+		})
 
-		Expect(accountant.AccountArgsForCall(0)).To(Equal(containers))
-	})
+		It("prints help text when `-h` is passed", func() {
+			buf := gbytes.NewBuffer()
+			returnCode := accounts.Execute(nil, nil, []string{"-h"}, buf)
+			Expect(returnCode).To(Equal(0))
+			Expect(buf).To(gbytes.Say("Usage"))
+		})
+
+		It("fails on flag parsing errors", func() {
+			buf := gbytes.NewBuffer()
+			returnCode := accounts.Execute(nil, nil, []string{"--invalid-flag"}, buf)
+			Expect(returnCode).To(Equal(1))
+			Expect(buf).To(gbytes.Say("invalid-flag"))
+		})
+
+		It("fails on kubectl errors", func() {
+			buf := gbytes.NewBuffer()
+			returnCode := accounts.Execute(func(accounts.Command) (accounts.Worker, error) {
+				return nil, errors.New("error loading config file")
+			}, nil, []string{}, buf)
+
+			Expect(returnCode).To(Equal(1))
+			Expect(buf).To(gbytes.Say("configuration error: error loading config file\n"))
+		})
+
+		It("fails on accountant errors", func() {
+			buf := gbytes.NewBuffer()
+			fakeWorker := new(accountsfakes.FakeWorker)
+			container := accounts.Container{Handle: "abc123"}
+			containers := []accounts.Container{container}
+			fakeWorker.ContainersReturns(containers, nil)
+			fakeAccountant := new(accountsfakes.FakeAccountant)
+			fakeAccountant.AccountReturns(
+				nil,
+				errors.New("accountant error"),
+			)
+
+			returnCode := accounts.Execute(
+				func(accounts.Command) (accounts.Worker, error) {
+					return fakeWorker, nil
+				}, func(accounts.Command) accounts.Accountant {
+					return fakeAccountant
+				},
+				[]string{},
+				buf,
+			)
+
+			Expect(returnCode).To(Equal(1))
+			Expect(buf).To(gbytes.Say("accountant error: accountant error\n"))
+		})
 
-	It("surfaces worker errors", func() {
-		worker.ContainersReturns(nil, errors.New("HTTP error"))
+		It("fails on terminal io errors", func() {
+			fakeWriter := new(accountsfakes.FakeWriter)
+			fakeWriter.WriteReturns(0, errors.New("terminal io error"))
+			fakeWorker := new(accountsfakes.FakeWorker)
+			container := accounts.Container{Handle: "abc123"}
+			containers := []accounts.Container{container}
+			fakeWorker.ContainersReturns(containers, nil)
+			fakeAccountant := new(accountsfakes.FakeAccountant)
+			fakeAccountant.AccountReturns(
+				[]accounts.Sample{
+					accounts.Sample{
+						Container: container,
+					},
+				},
+				nil,
+			)
 
-		_, err := accounts.Account(worker, accountant)
+			returnCode := accounts.Execute(
+				func(accounts.Command) (accounts.Worker, error) {
+					return fakeWorker, nil
+				}, func(accounts.Command) accounts.Accountant {
+					return fakeAccountant
+				},
+				[]string{},
+				fakeWriter,
+			)
 
-		Expect(err).NotTo(BeNil())
+			Expect(returnCode).To(Equal(1))
+		})
 	})
 })