Wait for deploy service

9 files changed, 86 insertions, 46 deletions

diff --git a/client/go/internal/cli/cmd/deploy.go b/client/go/internal/cli/cmd/deploy.go
index 09ad06da627..b037a953b53 100644
--- a/client/go/internal/cli/cmd/deploy.go
+++ b/client/go/internal/cli/cmd/deploy.go
@@ -73,6 +73,9 @@ $ vespa deploy -t cloud -z perf.aws-us-east-1c`,
 					return err
 				}
 			}
+			if err := waitForDeployService(cli, target, timeout); err != nil {
+				return err
+			}
 			var result vespa.PrepareResult
 			if err := cli.spinner(cli.Stderr, "Uploading application package ...", func() error {
 				result, err = vespa.Deploy(opts)
@@ -84,7 +87,7 @@ $ vespa deploy -t cloud -z perf.aws-us-east-1c`,
 			if opts.Target.IsCloud() {
 				cli.printSuccess("Triggered deployment of ", color.CyanString(pkg.Path), " with run ID ", color.CyanString(strconv.FormatInt(result.ID, 10)))
 			} else {
-				cli.printSuccess("Deployed ", color.CyanString(pkg.Path))
+				cli.printSuccess("Deployed ", color.CyanString(pkg.Path), " with session ID ", color.CyanString(strconv.FormatInt(result.ID, 10)))
 				printPrepareLog(cli.Stderr, result)
 			}
 			if opts.Target.IsCloud() {
@@ -158,6 +161,9 @@ func newActivateCmd(cli *CLI) *cobra.Command {
 				return err
 			}
 			timeout := time.Duration(waitSecs) * time.Second
+			if err := waitForDeployService(cli, target, timeout); err != nil {
+				return err
+			}
 			opts := vespa.DeploymentOptions{Target: target, Timeout: timeout}
 			err = vespa.Activate(sessionID, opts)
 			if err != nil {
@@ -171,10 +177,23 @@ func newActivateCmd(cli *CLI) *cobra.Command {
 	return cmd
 }
 
+func waitForDeployService(cli *CLI, target vespa.Target, timeout time.Duration) error {
+	if timeout == 0 {
+		return nil
+	}
+	s, err := target.DeployService(0)
+	if err != nil {
+		return err
+	}
+	cli.printInfo("Waiting up to ", color.CyanString(timeout.String()), " for ", s.Description(), " to become ready ...")
+	return s.Wait(timeout)
+}
+
 func waitForContainerServices(cli *CLI, target vespa.Target, sessionOrRunID int64, timeout time.Duration) error {
 	if timeout == 0 {
 		return nil
 	}
+	cli.printInfo("Waiting up to ", color.CyanString(timeout.String()), " for deployment to converge  ...")
 	if _, err := target.AwaitDeployment(sessionOrRunID, timeout); err != nil {
 		return err
 	}
diff --git a/client/go/internal/cli/cmd/deploy_test.go b/client/go/internal/cli/cmd/deploy_test.go
index b72cf7bc714..35c9b05050c 100644
--- a/client/go/internal/cli/cmd/deploy_test.go
+++ b/client/go/internal/cli/cmd/deploy_test.go
@@ -6,6 +6,7 @@ package cmd
 
 import (
 	"bytes"
+	"io"
 	"path/filepath"
 	"strconv"
 	"strings"
@@ -61,6 +62,33 @@ Hint: Pass --add-cert to use the certificate of the current application
 	assert.Contains(t, stdout.String(), "Success: Triggered deployment")
 }
 
+func TestDeployWait(t *testing.T) {
+	cli, stdout, _ := newTestCLI(t)
+	client := &mock.HTTPClient{}
+	cli.httpClient = client
+	cli.retryInterval = 0
+	pkg := "testdata/applications/withSource/src/main/application"
+	// Deploy service is initially unavailable
+	client.NextResponseError(io.EOF)
+	client.NextStatus(500)
+	client.NextStatus(500)
+	// ... then becomes healthy
+	client.NextStatus(200)
+	client.NextStatus(200)
+	// Deployment succeeds
+	client.NextResponse(mock.HTTPResponse{
+		URI:    "/application/v2/tenant/default/prepareandactivate",
+		Status: 200,
+		Body:   []byte(`{"session-id": "1"}`),
+	})
+	mockServiceStatus(client, "foo") // Wait for deployment
+	mockServiceStatus(client, "foo") // Look up services
+	assert.Nil(t, cli.Run("deploy", "--wait=3", pkg))
+	assert.Equal(t,
+		"\nSuccess: Deployed "+pkg+" with session ID 1\n",
+		stdout.String())
+}
+
 func TestPrepareZip(t *testing.T) {
 	assertPrepare("testdata/applications/withTarget/target/application.zip",
 		[]string{"prepare", "testdata/applications/withTarget/target/application.zip"}, t)
@@ -85,7 +113,7 @@ func TestDeployZipWithURLTargetArgument(t *testing.T) {
 	cli.httpClient = client
 	assert.Nil(t, cli.Run(arguments...))
 	assert.Equal(t,
-		"\nSuccess: Deployed "+applicationPackage+"\n",
+		"\nSuccess: Deployed "+applicationPackage+" with session ID 0\n",
 		stdout.String())
 	assertDeployRequestMade("http://target:19071", client, t)
 }
@@ -161,7 +189,7 @@ func assertDeploy(applicationPackage string, arguments []string, t *testing.T) {
 	cli.httpClient = client
 	assert.Nil(t, cli.Run(arguments...))
 	assert.Equal(t,
-		"\nSuccess: Deployed "+applicationPackage+"\n",
+		"\nSuccess: Deployed "+applicationPackage+" with session ID 0\n",
 		stdout.String())
 	assertDeployRequestMade("http://127.0.0.1:19071", client, t)
 }
diff --git a/client/go/internal/cli/cmd/root.go b/client/go/internal/cli/cmd/root.go
index 2829dce2b21..b88d86ce453 100644
--- a/client/go/internal/cli/cmd/root.go
+++ b/client/go/internal/cli/cmd/root.go
@@ -43,6 +43,13 @@ type CLI struct {
 	Stdout      io.Writer
 	Stderr      io.Writer
 
+	exec       executor
+	isTerminal func() bool
+	spinner    func(w io.Writer, message string, fn func() error) error
+
+	now           func() time.Time
+	retryInterval time.Duration
+
 	cmd     *cobra.Command
 	config  *Config
 	version version.Version
@@ -51,10 +58,6 @@ type CLI struct {
 	httpClientFactory func(timeout time.Duration) util.HTTPClient
 	auth0Factory      auth0Factory
 	ztsFactory        ztsFactory
-	exec              executor
-	isTerminal        func() bool
-	spinner           func(w io.Writer, message string, fn func() error) error
-	now               func() time.Time
 }
 
 // ErrCLI is an error returned to the user. It wraps an exit status, a regular error and optional hints for resolving
@@ -134,12 +137,15 @@ For detailed description of flags and configuration, see 'vespa help config'.
 		Stdout:      stdout,
 		Stderr:      stderr,
 
-		version:           version,
-		cmd:               cmd,
+		exec:          &execSubprocess{},
+		now:           time.Now,
+		retryInterval: 2 * time.Second,
+
+		version: version,
+		cmd:     cmd,
+
 		httpClient:        httpClientFactory(time.Second * 10),
 		httpClientFactory: httpClientFactory,
-		exec:              &execSubprocess{},
-		now:               time.Now,
 		auth0Factory: func(httpClient util.HTTPClient, options auth0.Options) (vespa.Authenticator, error) {
 			return auth0.NewClient(httpClient, options)
 		},
@@ -296,7 +302,7 @@ func (c *CLI) printSuccess(msg ...interface{}) {
 }
 
 func (c *CLI) printInfo(msg ...interface{}) {
-	fmt.Fprintln(c.Stderr, "Info:", fmt.Sprint(msg...))
+	fmt.Fprintln(c.Stderr, fmt.Sprint(msg...))
 }
 
 func (c *CLI) printDebug(msg ...interface{}) {
@@ -405,9 +411,9 @@ func (c *CLI) createCustomTarget(targetType, customURL string) (vespa.Target, er
 	}
 	switch targetType {
 	case vespa.TargetLocal:
-		return vespa.LocalTarget(c.httpClient, tlsOptions), nil
+		return vespa.LocalTarget(c.httpClient, tlsOptions, c.retryInterval), nil
 	case vespa.TargetCustom:
-		return vespa.CustomTarget(c.httpClient, customURL, tlsOptions), nil
+		return vespa.CustomTarget(c.httpClient, customURL, tlsOptions, c.retryInterval), nil
 	default:
 		return nil, fmt.Errorf("invalid custom target: %s", targetType)
 	}
@@ -489,7 +495,7 @@ func (c *CLI) createCloudTarget(targetType string, opts targetOptions, customURL
 		Writer: c.Stdout,
 		Level:  vespa.LogLevel(logLevel),
 	}
-	return vespa.CloudTarget(c.httpClient, apiAuth, deploymentAuth, apiOptions, deploymentOptions, logOptions)
+	return vespa.CloudTarget(c.httpClient, apiAuth, deploymentAuth, apiOptions, deploymentOptions, logOptions, c.retryInterval)
 }
 
 // system returns the appropiate system for the target configured in this CLI.
diff --git a/client/go/internal/vespa/deploy.go b/client/go/internal/vespa/deploy.go
index 6577ac0d38b..8fd35e4a933 100644
--- a/client/go/internal/vespa/deploy.go
+++ b/client/go/internal/vespa/deploy.go
@@ -149,7 +149,7 @@ func Prepare(deployment DeploymentOptions) (PrepareResult, error) {
 		return PrepareResult{}, err
 	}
 	var jsonResponse struct {
-		SessionID string                   `json:"session-id"`
+		SessionID string                   `json:"session-id"` // API returns ID as string
 		Log       []LogLinePrepareResponse `json:"log"`
 	}
 	jsonDec := json.NewDecoder(response.Body)
@@ -384,7 +384,7 @@ func uploadApplicationPackage(url *url.URL, opts DeploymentOptions) (PrepareResu
 	defer response.Body.Close()
 
 	var jsonResponse struct {
-		SessionID string `json:"session-id"` // Config server
+		SessionID string `json:"session-id"` // Config server. API returns ID as string
 		RunID     int64  `json:"run"`        // Controller
 
 		Log []LogLinePrepareResponse `json:"log"`
diff --git a/client/go/internal/vespa/deploy_test.go b/client/go/internal/vespa/deploy_test.go
index 276e0f958d9..9addf81138a 100644
--- a/client/go/internal/vespa/deploy_test.go
+++ b/client/go/internal/vespa/deploy_test.go
@@ -19,7 +19,7 @@ import (
 
 func TestDeploy(t *testing.T) {
 	httpClient := mock.HTTPClient{}
-	target := LocalTarget(&httpClient, TLSOptions{})
+	target := LocalTarget(&httpClient, TLSOptions{}, 0)
 	appDir, _ := mock.ApplicationPackageDir(t, false, false)
 	opts := DeploymentOptions{
 		Target:             target,
@@ -170,7 +170,7 @@ func TestFindApplicationPackage(t *testing.T) {
 
 func TestDeactivate(t *testing.T) {
 	httpClient := mock.HTTPClient{}
-	target := LocalTarget(&httpClient, TLSOptions{})
+	target := LocalTarget(&httpClient, TLSOptions{}, 0)
 	opts := DeploymentOptions{Target: target}
 	require.Nil(t, Deactivate(opts))
 	assert.Equal(t, 1, len(httpClient.Requests))
diff --git a/client/go/internal/vespa/target.go b/client/go/internal/vespa/target.go
index df82cc435b2..bbee85217df 100644
--- a/client/go/internal/vespa/target.go
+++ b/client/go/internal/vespa/target.go
@@ -34,8 +34,6 @@ const (
 
 	// AnyDeployment waits for a deployment to converge on any generation
 	AnyDeployment int64 = -2
-
-	defaultRetryInterval = 2 * time.Second
 )
 
 var errWaitTimeout = errors.New("wait timed out")
diff --git a/client/go/internal/vespa/target_cloud.go b/client/go/internal/vespa/target_cloud.go
index 9b53180b7bb..14b7f1f4c52 100644
--- a/client/go/internal/vespa/target_cloud.go
+++ b/client/go/internal/vespa/target_cloud.go
@@ -72,7 +72,9 @@ type logMessage struct {
 }
 
 // CloudTarget creates a Target for the Vespa Cloud or hosted Vespa platform.
-func CloudTarget(httpClient util.HTTPClient, apiAuth Authenticator, deploymentAuth Authenticator, apiOptions APIOptions, deploymentOptions CloudDeploymentOptions, logOptions LogOptions) (Target, error) {
+func CloudTarget(httpClient util.HTTPClient, apiAuth Authenticator, deploymentAuth Authenticator,
+	apiOptions APIOptions, deploymentOptions CloudDeploymentOptions,
+	logOptions LogOptions, retryInterval time.Duration) (Target, error) {
 	return &cloudTarget{
 		httpClient:        httpClient,
 		apiOptions:        apiOptions,
@@ -80,7 +82,7 @@ func CloudTarget(httpClient util.HTTPClient, apiAuth Authenticator, deploymentAu
 		logOptions:        logOptions,
 		apiAuth:           apiAuth,
 		deploymentAuth:    deploymentAuth,
-		retryInterval:     defaultRetryInterval,
+		retryInterval:     retryInterval,
 	}, nil
 }
 
diff --git a/client/go/internal/vespa/target_custom.go b/client/go/internal/vespa/target_custom.go
index e6c13502430..a49654e3dff 100644
--- a/client/go/internal/vespa/target_custom.go
+++ b/client/go/internal/vespa/target_custom.go
@@ -35,24 +35,24 @@ type serviceInfo struct {
 }
 
 // LocalTarget creates a target for a Vespa platform running locally.
-func LocalTarget(httpClient util.HTTPClient, tlsOptions TLSOptions) Target {
+func LocalTarget(httpClient util.HTTPClient, tlsOptions TLSOptions, retryInterval time.Duration) Target {
 	return &customTarget{
 		targetType:    TargetLocal,
 		baseURL:       "http://127.0.0.1",
 		httpClient:    httpClient,
 		tlsOptions:    tlsOptions,
-		retryInterval: defaultRetryInterval,
+		retryInterval: retryInterval,
 	}
 }
 
 // CustomTarget creates a Target for a Vespa platform running at baseURL.
-func CustomTarget(httpClient util.HTTPClient, baseURL string, tlsOptions TLSOptions) Target {
+func CustomTarget(httpClient util.HTTPClient, baseURL string, tlsOptions TLSOptions, retryInterval time.Duration) Target {
 	return &customTarget{
 		targetType:    TargetCustom,
 		baseURL:       baseURL,
 		httpClient:    httpClient,
 		tlsOptions:    tlsOptions,
-		retryInterval: defaultRetryInterval,
+		retryInterval: retryInterval,
 	}
 }
 
diff --git a/client/go/internal/vespa/target_test.go b/client/go/internal/vespa/target_test.go
index 8fe424e897c..c9a5fc74de9 100644
--- a/client/go/internal/vespa/target_test.go
+++ b/client/go/internal/vespa/target_test.go
@@ -3,7 +3,6 @@ package vespa
 
 import (
 	"bytes"
-	"fmt"
 	"io"
 	"net/http"
 	"strings"
@@ -19,7 +18,7 @@ import (
 func TestLocalTarget(t *testing.T) {
 	// Local target uses discovery
 	client := &mock.HTTPClient{}
-	lt := LocalTarget(client, TLSOptions{})
+	lt := LocalTarget(client, TLSOptions{}, 0)
 	assertServiceURL(t, "http://127.0.0.1:19071", lt, "deploy")
 	for i := 0; i < 2; i++ {
 		response := `
@@ -67,31 +66,19 @@ func TestLocalTarget(t *testing.T) {
 	assertServiceURL(t, "http://127.0.0.1:8081", lt, "feed")
 }
 
-func setRetryInterval(target Target, interval time.Duration) {
-	switch t := target.(type) {
-	case *cloudTarget:
-		t.retryInterval = interval
-	case *customTarget:
-		t.retryInterval = interval
-	default:
-		panic(fmt.Sprintf("unexpected type %T", t))
-	}
-}
-
 func TestCustomTarget(t *testing.T) {
 	// Custom target always uses URL directly, without discovery
-	ct := CustomTarget(&mock.HTTPClient{}, "http://192.0.2.42", TLSOptions{})
+	ct := CustomTarget(&mock.HTTPClient{}, "http://192.0.2.42", TLSOptions{}, 0)
 	assertServiceURL(t, "http://192.0.2.42", ct, "deploy")
 	assertServiceURL(t, "http://192.0.2.42", ct, "")
-	ct2 := CustomTarget(&mock.HTTPClient{}, "http://192.0.2.42:60000", TLSOptions{})
+	ct2 := CustomTarget(&mock.HTTPClient{}, "http://192.0.2.42:60000", TLSOptions{}, 0)
 	assertServiceURL(t, "http://192.0.2.42:60000", ct2, "deploy")
 	assertServiceURL(t, "http://192.0.2.42:60000", ct2, "")
 }
 
 func TestCustomTargetWait(t *testing.T) {
 	client := &mock.HTTPClient{}
-	target := CustomTarget(client, "http://192.0.2.42", TLSOptions{})
-	setRetryInterval(target, 0)
+	target := CustomTarget(client, "http://192.0.2.42", TLSOptions{}, 0)
 	// Fails once
 	client.NextStatus(500)
 	assertService(t, true, target, "", 0)
@@ -107,7 +94,7 @@ func TestCustomTargetWait(t *testing.T) {
 
 func TestCustomTargetAwaitDeployment(t *testing.T) {
 	client := &mock.HTTPClient{}
-	target := CustomTarget(client, "http://192.0.2.42", TLSOptions{})
+	target := CustomTarget(client, "http://192.0.2.42", TLSOptions{}, 0)
 
 	// Not converged initially
 	_, err := target.AwaitDeployment(42, 0)
@@ -269,9 +256,9 @@ func createCloudTarget(t *testing.T, logWriter io.Writer) (Target, *mock.HTTPCli
 			},
 		},
 		LogOptions{Writer: logWriter},
+		0,
 	)
 	require.Nil(t, err)
-	setRetryInterval(target, 0)
 	return target, client
 }