Print all endpoints independent of readiness

10 files changed, 69 insertions, 33 deletions

diff --git a/client/go/internal/cli/cmd/curl.go b/client/go/internal/cli/cmd/curl.go
index 3d81a5d2c0e..4a919d18d49 100644
--- a/client/go/internal/cli/cmd/curl.go
+++ b/client/go/internal/cli/cmd/curl.go
@@ -41,7 +41,7 @@ $ vespa curl -- -v --data-urlencode "yql=select * from music where album contain
 			}
 			var service *vespa.Service
 			useDeploy := curlService == "deploy"
-			waiter := cli.waiter(false, time.Duration(waitSecs)*time.Second)
+			waiter := cli.waiter(time.Duration(waitSecs) * time.Second)
 			if useDeploy {
 				if cli.config.cluster() != "" {
 					return fmt.Errorf("cannot specify cluster for service %s", curlService)
diff --git a/client/go/internal/cli/cmd/deploy.go b/client/go/internal/cli/cmd/deploy.go
index 0a4d72a8a48..aee26975901 100644
--- a/client/go/internal/cli/cmd/deploy.go
+++ b/client/go/internal/cli/cmd/deploy.go
@@ -73,7 +73,7 @@ $ vespa deploy -t cloud -z perf.aws-us-east-1c`,
 					return err
 				}
 			}
-			waiter := cli.waiter(false, timeout)
+			waiter := cli.waiter(timeout)
 			if _, err := waiter.DeployService(target); err != nil {
 				return err
 			}
@@ -158,7 +158,7 @@ func newActivateCmd(cli *CLI) *cobra.Command {
 				return err
 			}
 			timeout := time.Duration(waitSecs) * time.Second
-			waiter := cli.waiter(false, timeout)
+			waiter := cli.waiter(timeout)
 			if _, err := waiter.DeployService(target); err != nil {
 				return err
 			}
@@ -179,7 +179,7 @@ func waitForDeploymentReady(cli *CLI, target vespa.Target, sessionOrRunID int64,
 	if timeout == 0 {
 		return nil
 	}
-	waiter := cli.waiter(false, timeout)
+	waiter := cli.waiter(timeout)
 	if _, err := waiter.Deployment(target, sessionOrRunID); err != nil {
 		return err
 	}
diff --git a/client/go/internal/cli/cmd/document.go b/client/go/internal/cli/cmd/document.go
index 090060d578c..a98e9867d34 100644
--- a/client/go/internal/cli/cmd/document.go
+++ b/client/go/internal/cli/cmd/document.go
@@ -298,7 +298,7 @@ func documentService(cli *CLI, waitSecs int) (*vespa.Service, error) {
 	if err != nil {
 		return nil, err
 	}
-	waiter := cli.waiter(false, time.Duration(waitSecs)*time.Second)
+	waiter := cli.waiter(time.Duration(waitSecs) * time.Second)
 	return waiter.Service(target, cli.config.cluster())
 }
 
diff --git a/client/go/internal/cli/cmd/feed.go b/client/go/internal/cli/cmd/feed.go
index 5cdd0d7d63b..1a32ac7110d 100644
--- a/client/go/internal/cli/cmd/feed.go
+++ b/client/go/internal/cli/cmd/feed.go
@@ -108,7 +108,7 @@ func createServices(n int, timeout time.Duration, waitSecs int, cli *CLI) ([]uti
 	}
 	services := make([]util.HTTPClient, 0, n)
 	baseURL := ""
-	waiter := cli.waiter(false, time.Duration(waitSecs)*time.Second)
+	waiter := cli.waiter(time.Duration(waitSecs) * time.Second)
 	for i := 0; i < n; i++ {
 		service, err := waiter.Service(target, cli.config.cluster())
 		if err != nil {
diff --git a/client/go/internal/cli/cmd/query.go b/client/go/internal/cli/cmd/query.go
index 6fb571dc36b..bf2272ca981 100644
--- a/client/go/internal/cli/cmd/query.go
+++ b/client/go/internal/cli/cmd/query.go
@@ -64,7 +64,7 @@ func query(cli *CLI, arguments []string, timeoutSecs, waitSecs int, curl bool) e
 	if err != nil {
 		return err
 	}
-	waiter := cli.waiter(false, time.Duration(waitSecs)*time.Second)
+	waiter := cli.waiter(time.Duration(waitSecs) * time.Second)
 	service, err := waiter.Service(target, cli.config.cluster())
 	if err != nil {
 		return err
diff --git a/client/go/internal/cli/cmd/root.go b/client/go/internal/cli/cmd/root.go
index 352ddbcd152..068a7ed90b6 100644
--- a/client/go/internal/cli/cmd/root.go
+++ b/client/go/internal/cli/cmd/root.go
@@ -345,9 +345,7 @@ func (c *CLI) confirm(question string, confirmByDefault bool) (bool, error) {
 	}
 }
 
-func (c *CLI) waiter(once bool, timeout time.Duration) *Waiter {
-	return &Waiter{Once: once, Timeout: timeout, cli: c}
-}
+func (c *CLI) waiter(timeout time.Duration) *Waiter { return &Waiter{Timeout: timeout, cli: c} }
 
 // target creates a target according the configuration of this CLI and given opts.
 func (c *CLI) target(opts targetOptions) (vespa.Target, error) {
diff --git a/client/go/internal/cli/cmd/status.go b/client/go/internal/cli/cmd/status.go
index 001f4ba43f1..a0602494bff 100644
--- a/client/go/internal/cli/cmd/status.go
+++ b/client/go/internal/cli/cmd/status.go
@@ -42,7 +42,8 @@ $ vespa status --cluster mycluster --wait 600`,
 			if err != nil {
 				return err
 			}
-			waiter := cli.waiter(true, time.Duration(waitSecs)*time.Second)
+			waiter := cli.waiter(time.Duration(waitSecs) * time.Second)
+			var failingContainers []*vespa.Service
 			if cluster == "" {
 				services, err := waiter.Services(t)
 				if err != nil {
@@ -52,23 +53,41 @@ $ vespa status --cluster mycluster --wait 600`,
 					return errHint(fmt.Errorf("no services exist"), "Deployment may not be ready yet", "Try 'vespa status deployment'")
 				}
 				for _, s := range services {
-					printReadyService(s, cli)
+					if !printServiceStatus(s, waiter, cli) {
+						failingContainers = append(failingContainers, s)
+					}
 				}
-				return nil
 			} else {
 				s, err := waiter.Service(t, cluster)
 				if err != nil {
 					return err
 				}
-				printReadyService(s, cli)
-				return nil
+				if !printServiceStatus(s, waiter, cli) {
+					failingContainers = append(failingContainers, s)
+				}
 			}
+			return failingServicesErr(failingContainers...)
 		},
 	}
 	cli.bindWaitFlag(cmd, 0, &waitSecs)
 	return cmd
 }
 
+func failingServicesErr(services ...*vespa.Service) error {
+	if len(services) == 0 {
+		return nil
+	}
+	var nameOrURL []string
+	for _, s := range services {
+		if s.Name != "" {
+			nameOrURL = append(nameOrURL, s.Name)
+		} else {
+			nameOrURL = append(nameOrURL, s.BaseURL)
+		}
+	}
+	return fmt.Errorf("services not ready: %s", strings.Join(nameOrURL, ", "))
+}
+
 func newStatusDeployCmd(cli *CLI) *cobra.Command {
 	var waitSecs int
 	cmd := &cobra.Command{
@@ -83,12 +102,14 @@ func newStatusDeployCmd(cli *CLI) *cobra.Command {
 			if err != nil {
 				return err
 			}
-			waiter := cli.waiter(true, time.Duration(waitSecs)*time.Second)
+			waiter := cli.waiter(time.Duration(waitSecs) * time.Second)
 			s, err := waiter.DeployService(t)
 			if err != nil {
 				return err
 			}
-			printReadyService(s, cli)
+			if !printServiceStatus(s, waiter, cli) {
+				return failingServicesErr(s)
+			}
 			return nil
 		},
 	}
@@ -128,7 +149,7 @@ $ vespa status deployment -t local [session-id] --wait 600
 			if err != nil {
 				return err
 			}
-			waiter := cli.waiter(true, time.Duration(waitSecs)*time.Second)
+			waiter := cli.waiter(time.Duration(waitSecs) * time.Second)
 			id, err := waiter.Deployment(t, wantedID)
 			if err != nil {
 				return err
@@ -146,8 +167,19 @@ $ vespa status deployment -t local [session-id] --wait 600
 	return cmd
 }
 
-func printReadyService(s *vespa.Service, cli *CLI) {
+func printServiceStatus(s *vespa.Service, waiter *Waiter, cli *CLI) bool {
 	desc := s.Description()
 	desc = strings.ToUpper(string(desc[0])) + string(desc[1:])
-	log.Print(desc, " at ", color.CyanString(s.BaseURL), " is ", color.GreenString("ready"))
+	err := s.Wait(waiter.Timeout)
+	var sb strings.Builder
+	sb.WriteString(fmt.Sprintf("%s at %s is ", desc, color.CyanString(s.BaseURL)))
+	if err == nil {
+		sb.WriteString(color.GreenString("ready"))
+	} else {
+		sb.WriteString(color.RedString("not ready"))
+		sb.WriteString(": ")
+		sb.WriteString(err.Error())
+	}
+	fmt.Fprintln(cli.Stdout, sb.String())
+	return err == nil
 }
diff --git a/client/go/internal/cli/cmd/status_test.go b/client/go/internal/cli/cmd/status_test.go
index 394a0cd4ac4..1473e39ff54 100644
--- a/client/go/internal/cli/cmd/status_test.go
+++ b/client/go/internal/cli/cmd/status_test.go
@@ -38,12 +38,18 @@ func TestStatusCommandMultiCluster(t *testing.T) {
 	mockServiceStatus(client)
 	assert.NotNil(t, cli.Run("status"))
 	assert.Equal(t, "Error: no services exist\nHint: Deployment may not be ready yet\nHint: Try 'vespa status deployment'\n", stderr.String())
+	stderr.Reset()
 
 	mockServiceStatus(client, "foo", "bar")
-	assert.Nil(t, cli.Run("status"))
+	client.NextStatus(200)
+	client.NextStatus(400) // One cluster is unavilable
+	assert.NotNil(t, cli.Run("status"))
 	assert.Equal(t, `Container bar at http://127.0.0.1:8080 is ready
-Container foo at http://127.0.0.1:8080 is ready
+Container foo at http://127.0.0.1:8080 is not ready: unhealthy container foo: status 400 at http://127.0.0.1:8080/status.html: aborting wait: got status 400
 `, stdout.String())
+	assert.Equal(t,
+		"Error: services not ready: foo\n",
+		stderr.String())
 
 	stdout.Reset()
 	mockServiceStatus(client, "foo", "bar")
@@ -75,18 +81,25 @@ func TestStatusError(t *testing.T) {
 	client := &mock.HTTPClient{}
 	mockServiceStatus(client, "default")
 	client.NextStatus(500)
-	cli, _, stderr := newTestCLI(t)
+	cli, stdout, stderr := newTestCLI(t)
 	cli.httpClient = client
 	assert.NotNil(t, cli.Run("status", "container"))
 	assert.Equal(t,
-		"Error: unhealthy container default: status 500 at http://127.0.0.1:8080/status.html: wait timed out\n",
+		"Container default at http://127.0.0.1:8080 is not ready: unhealthy container default: status 500 at http://127.0.0.1:8080/status.html: wait timed out\n",
+		stdout.String())
+	assert.Equal(t,
+		"Error: services not ready: default\n",
 		stderr.String())
 
+	stdout.Reset()
 	stderr.Reset()
 	client.NextResponseError(io.EOF)
 	assert.NotNil(t, cli.Run("status", "container", "-t", "http://example.com"))
 	assert.Equal(t,
-		"Error: unhealthy container at http://example.com/status.html: EOF\n",
+		"Container at http://example.com is not ready: unhealthy container at http://example.com/status.html: EOF\n",
+		stdout.String())
+	assert.Equal(t,
+		"Error: services not ready: http://example.com\n",
 		stderr.String())
 }
 
diff --git a/client/go/internal/cli/cmd/test.go b/client/go/internal/cli/cmd/test.go
index e842cab606e..f0432dd4f70 100644
--- a/client/go/internal/cli/cmd/test.go
+++ b/client/go/internal/cli/cmd/test.go
@@ -220,7 +220,7 @@ func verify(step step, defaultCluster string, defaultParameters map[string]strin
 		service, ok = context.clusters[cluster]
 		if !ok {
 			// Cache service so we don't have to discover it for every step
-			waiter := context.cli.waiter(false, time.Duration(waitSecs)*time.Second)
+			waiter := context.cli.waiter(time.Duration(waitSecs) * time.Second)
 			service, err = waiter.Service(target, cluster)
 			if err != nil {
 				return "", "", err
diff --git a/client/go/internal/cli/cmd/waiter.go b/client/go/internal/cli/cmd/waiter.go
index 302bc679885..0cfb3aa76d5 100644
--- a/client/go/internal/cli/cmd/waiter.go
+++ b/client/go/internal/cli/cmd/waiter.go
@@ -11,17 +11,12 @@ import (
 
 // Waiter waits for Vespa services to become ready, within a timeout.
 type Waiter struct {
-	// Once species whether we should wait at least one time, irregardless of timeout.
-	Once bool
-
 	// Timeout specifies how long we should wait for an operation to complete.
 	Timeout time.Duration // TODO(mpolden): Consider making this a budget
 
 	cli *CLI
 }
 
-func (w *Waiter) wait() bool { return w.Once || w.Timeout > 0 }
-
 // DeployService returns the service providing the deploy API on given target,
 func (w *Waiter) DeployService(target vespa.Target) (*vespa.Service, error) {
 	s, err := target.DeployService()
@@ -74,8 +69,6 @@ func (w *Waiter) Services(target vespa.Target) ([]*vespa.Service, error) {
 func (w *Waiter) maybeWaitFor(service *vespa.Service) error {
 	if w.Timeout > 0 {
 		w.cli.printInfo("Waiting up to ", color.CyanString(w.Timeout.String()), " for ", service.Description(), "...")
-	}
-	if w.wait() {
 		return service.Wait(w.Timeout)
 	}
 	return nil