From 7a60534c99cae44e9c14ff786c566a24936ce8aa Mon Sep 17 00:00:00 2001 From: Martin Polden Date: Mon, 10 Jun 2024 12:37:07 +0200 Subject: Ensure that fast wait does not return error for ongoing deployment --- client/go/internal/cli/cmd/curl.go | 2 +- client/go/internal/cli/cmd/deploy.go | 43 +++++++++++++++---------------- client/go/internal/cli/cmd/deploy_test.go | 29 ++++++++++++++++----- client/go/internal/cli/cmd/document.go | 32 +++++++++++++---------- client/go/internal/cli/cmd/feed.go | 10 +++---- client/go/internal/cli/cmd/prod.go | 2 +- client/go/internal/cli/cmd/query.go | 6 ++--- client/go/internal/cli/cmd/root.go | 5 +++- client/go/internal/cli/cmd/status.go | 6 ++--- client/go/internal/cli/cmd/test.go | 18 ++++++------- client/go/internal/cli/cmd/visit.go | 3 ++- client/go/internal/cli/cmd/visit_test.go | 2 +- client/go/internal/cli/cmd/waiter.go | 24 ++++++++++++++--- client/go/internal/mock/http.go | 7 +++++ client/go/internal/vespa/target.go | 11 +++++--- client/go/internal/vespa/target_cloud.go | 2 +- 16 files changed, 127 insertions(+), 75 deletions(-) (limited to 'client/go') diff --git a/client/go/internal/cli/cmd/curl.go b/client/go/internal/cli/cmd/curl.go index c7297574a32..63f6ef8c592 100644 --- a/client/go/internal/cli/cmd/curl.go +++ b/client/go/internal/cli/cmd/curl.go @@ -37,7 +37,7 @@ $ vespa curl -- -v --data-urlencode "yql=select * from music where album contain if err != nil { return err } - waiter := cli.waiter(time.Duration(waitSecs) * time.Second) + waiter := cli.waiter(time.Duration(waitSecs)*time.Second, cmd) service, err := waiter.Service(target, cli.config.cluster()) if err != nil { return err diff --git a/client/go/internal/cli/cmd/deploy.go b/client/go/internal/cli/cmd/deploy.go index 8372f6691dd..718e2b27b2b 100644 --- a/client/go/internal/cli/cmd/deploy.go +++ b/client/go/internal/cli/cmd/deploy.go @@ -72,21 +72,16 @@ $ vespa deploy -t cloud -z perf.aws-us-east-1c`, return err } } - if opts.Target.IsCloud() && waitSecs == 0 && !cmd.PersistentFlags().Changed("wait") { - // If --wait is not explicitly given, we always wait a few seconds in Cloud to catch fast failures, e.g. - // invalid application package - waitSecs = 2 - } - timeout := time.Duration(waitSecs) * time.Second - waiter := cli.waiter(timeout) + waiter := cli.waiter(time.Duration(waitSecs)*time.Second, cmd) if _, err := waiter.DeployService(target); err != nil { return err } var result vespa.PrepareResult - if err := cli.spinner(cli.Stderr, "Uploading application package...", func() error { + err = cli.spinner(cli.Stderr, "Uploading application package...", func() error { result, err = vespa.Deploy(opts) return err - }); err != nil { + }) + if err != nil { return err } log.Println() @@ -100,7 +95,7 @@ $ vespa deploy -t cloud -z perf.aws-us-east-1c`, log.Printf("\nUse %s for deployment status, or follow this deployment at", color.CyanString("vespa status deployment")) log.Print(color.CyanString(opts.Target.Deployment().System.ConsoleRunURL(opts.Target.Deployment(), result.ID))) } - return waitForDeploymentReady(cli, target, result.ID, timeout) + return waitForVespaReady(target, result.ID, waiter) }, } cmd.Flags().StringVarP(&logLevelArg, "log-level", "l", "error", `Log level for Vespa logs. Must be "error", "warning", "info" or "debug"`) @@ -162,8 +157,7 @@ func newActivateCmd(cli *CLI) *cobra.Command { if err != nil { return err } - timeout := time.Duration(waitSecs) * time.Second - waiter := cli.waiter(timeout) + waiter := cli.waiter(time.Duration(waitSecs)*time.Second, cmd) if _, err := waiter.DeployService(target); err != nil { return err } @@ -173,23 +167,28 @@ func newActivateCmd(cli *CLI) *cobra.Command { return err } cli.printSuccess("Activated application with session ", sessionID) - return waitForDeploymentReady(cli, target, sessionID, timeout) + return waitForVespaReady(target, sessionID, waiter) }, } cli.bindWaitFlag(cmd, 0, &waitSecs) return cmd } -func waitForDeploymentReady(cli *CLI, target vespa.Target, sessionOrRunID int64, timeout time.Duration) error { - if timeout == 0 { - return nil - } - waiter := cli.waiter(timeout) - if _, err := waiter.Deployment(target, sessionOrRunID); err != nil { - return err +func waitForVespaReady(target vespa.Target, sessionOrRunID int64, waiter *Waiter) error { + fastWait := waiter.FastWaitOn(target) + hasTimeout := waiter.Timeout > 0 + if fastWait || hasTimeout { + // Wait for deployment convergence + if _, err := waiter.Deployment(target, sessionOrRunID); err != nil { + return err + } + // Wait for healthy services + if hasTimeout { + _, err := waiter.Services(target) + return err + } } - _, err := waiter.Services(target) - return err + return nil } func printPrepareLog(stderr io.Writer, result vespa.PrepareResult) { diff --git a/client/go/internal/cli/cmd/deploy_test.go b/client/go/internal/cli/cmd/deploy_test.go index 9a046a984f6..00d540abdda 100644 --- a/client/go/internal/cli/cmd/deploy_test.go +++ b/client/go/internal/cli/cmd/deploy_test.go @@ -62,16 +62,12 @@ Hint: Pass --add-cert to use the certificate of the current application assert.Contains(t, stdout.String(), "Success: Triggered deployment") } -func TestDeployCloudDefaultWait(t *testing.T) { +func TestDeployCloudFastWait(t *testing.T) { pkgDir := filepath.Join(t.TempDir(), "app") createApplication(t, pkgDir, false, false) - cli, stdout, _ := newTestCLI(t, "CI=true") + cli, stdout, stderr := newTestCLI(t, "CI=true") httpClient := &mock.HTTPClient{} - httpClient.NextResponseString(200, `ok`) - httpClient.NextResponseString(200, `ok`) - httpClient.NextResponseString(200, `{"active": false, "status": "success"}`) - httpClient.NextResponseString(200, `{"endpoints": [{"url": "http://example.com", "scope": "zone", "cluster": "default"}]}`) cli.httpClient = httpClient app := vespa.ApplicationID{Tenant: "t1", Application: "a1", Instance: "i1"} @@ -80,6 +76,27 @@ func TestDeployCloudDefaultWait(t *testing.T) { assert.Nil(t, cli.Run("auth", "api-key")) assert.Nil(t, cli.Run("auth", "cert", pkgDir)) + // Deployment completes quickly + httpClient.NextResponseString(200, `ok`) + httpClient.NextResponseString(200, `{"active": false, "status": "success"}`) + require.Nil(t, cli.Run("deploy", pkgDir)) + assert.Contains(t, stdout.String(), "Success: Triggered deployment") + assert.True(t, httpClient.Consumed()) + + // Deployment fails quickly + stdout.Reset() + stderr.Reset() + httpClient.NextResponseString(200, `ok`) + httpClient.NextResponseString(200, `{"active": false, "status": "unsuccesful"}`) + require.NotNil(t, cli.Run("deploy", pkgDir)) + assert.Equal(t, stderr.String(), "Error: deployment run 0 incomplete after waiting up to 2s: aborting wait: deployment failed: run 0 ended with unsuccessful status: unsuccesful\n") + assert.True(t, httpClient.Consumed()) + + // Deployment which is running does not return error + stdout.Reset() + stderr.Reset() + httpClient.NextResponseString(200, `ok`) + httpClient.NextResponseString(200, `{"active": true, "status": "running"}`) require.Nil(t, cli.Run("deploy", pkgDir)) assert.Contains(t, stdout.String(), "Success: Triggered deployment") assert.True(t, httpClient.Consumed()) diff --git a/client/go/internal/cli/cmd/document.go b/client/go/internal/cli/cmd/document.go index 0393a9b2595..ea99488ee90 100644 --- a/client/go/internal/cli/cmd/document.go +++ b/client/go/internal/cli/cmd/document.go @@ -27,8 +27,8 @@ func addDocumentFlags(cli *CLI, cmd *cobra.Command, printCurl *bool, timeoutSecs cli.bindWaitFlag(cmd, 0, waitSecs) } -func documentClient(cli *CLI, timeoutSecs, waitSecs int, printCurl bool) (*document.Client, *vespa.Service, error) { - docService, err := documentService(cli, waitSecs) +func documentClient(cli *CLI, timeoutSecs int, waiter *Waiter, printCurl bool) (*document.Client, *vespa.Service, error) { + docService, err := documentService(cli, waiter) if err != nil { return nil, nil, err } @@ -47,8 +47,8 @@ func documentClient(cli *CLI, timeoutSecs, waitSecs int, printCurl bool) (*docum return client, docService, nil } -func sendOperation(op document.Operation, args []string, timeoutSecs, waitSecs int, printCurl bool, cli *CLI) error { - client, service, err := documentClient(cli, timeoutSecs, waitSecs, printCurl) +func sendOperation(op document.Operation, args []string, timeoutSecs int, waiter *Waiter, printCurl bool, cli *CLI) error { + client, service, err := documentClient(cli, timeoutSecs, waiter, printCurl) if err != nil { return err } @@ -91,8 +91,8 @@ func sendOperation(op document.Operation, args []string, timeoutSecs, waitSecs i return printResult(cli, operationResult(false, doc, service, result), false) } -func readDocument(id string, timeoutSecs, waitSecs int, printCurl bool, cli *CLI) error { - client, service, err := documentClient(cli, timeoutSecs, waitSecs, printCurl) +func readDocument(id string, timeoutSecs int, waiter *Waiter, printCurl bool, cli *CLI) error { + client, service, err := documentClient(cli, timeoutSecs, waiter, printCurl) if err != nil { return err } @@ -146,7 +146,8 @@ should be used instead of this.`, SilenceUsage: true, Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - return sendOperation(-1, args, timeoutSecs, waitSecs, printCurl, cli) + waiter := cli.waiter(time.Duration(waitSecs)*time.Second, cmd) + return sendOperation(-1, args, timeoutSecs, waiter, printCurl, cli) }, } addDocumentFlags(cli, cmd, &printCurl, &timeoutSecs, &waitSecs) @@ -171,7 +172,8 @@ $ vespa document put id:mynamespace:music::a-head-full-of-dreams src/test/resour DisableAutoGenTag: true, SilenceUsage: true, RunE: func(cmd *cobra.Command, args []string) error { - return sendOperation(document.OperationPut, args, timeoutSecs, waitSecs, printCurl, cli) + waiter := cli.waiter(time.Duration(waitSecs)*time.Second, cmd) + return sendOperation(document.OperationPut, args, timeoutSecs, waiter, printCurl, cli) }, } addDocumentFlags(cli, cmd, &printCurl, &timeoutSecs, &waitSecs) @@ -195,7 +197,8 @@ $ vespa document update id:mynamespace:music::a-head-full-of-dreams src/test/res DisableAutoGenTag: true, SilenceUsage: true, RunE: func(cmd *cobra.Command, args []string) error { - return sendOperation(document.OperationUpdate, args, timeoutSecs, waitSecs, printCurl, cli) + waiter := cli.waiter(time.Duration(waitSecs)*time.Second, cmd) + return sendOperation(document.OperationUpdate, args, timeoutSecs, waiter, printCurl, cli) }, } addDocumentFlags(cli, cmd, &printCurl, &timeoutSecs, &waitSecs) @@ -219,8 +222,9 @@ $ vespa document remove id:mynamespace:music::a-head-full-of-dreams`, DisableAutoGenTag: true, SilenceUsage: true, RunE: func(cmd *cobra.Command, args []string) error { + waiter := cli.waiter(time.Duration(waitSecs)*time.Second, cmd) if strings.HasPrefix(args[0], "id:") { - client, service, err := documentClient(cli, timeoutSecs, waitSecs, printCurl) + client, service, err := documentClient(cli, timeoutSecs, waiter, printCurl) if err != nil { return err } @@ -232,7 +236,7 @@ $ vespa document remove id:mynamespace:music::a-head-full-of-dreams`, result := client.Send(doc) return printResult(cli, operationResult(false, doc, service, result), false) } else { - return sendOperation(document.OperationRemove, args, timeoutSecs, waitSecs, printCurl, cli) + return sendOperation(document.OperationRemove, args, timeoutSecs, waiter, printCurl, cli) } }, } @@ -254,19 +258,19 @@ func newDocumentGetCmd(cli *CLI) *cobra.Command { SilenceUsage: true, Example: `$ vespa document get id:mynamespace:music::a-head-full-of-dreams`, RunE: func(cmd *cobra.Command, args []string) error { - return readDocument(args[0], timeoutSecs, waitSecs, printCurl, cli) + waiter := cli.waiter(time.Duration(waitSecs)*time.Second, cmd) + return readDocument(args[0], timeoutSecs, waiter, printCurl, cli) }, } addDocumentFlags(cli, cmd, &printCurl, &timeoutSecs, &waitSecs) return cmd } -func documentService(cli *CLI, waitSecs int) (*vespa.Service, error) { +func documentService(cli *CLI, waiter *Waiter) (*vespa.Service, error) { target, err := cli.target(targetOptions{}) if err != nil { return nil, err } - 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 69b847547a9..f69de6b3038 100644 --- a/client/go/internal/cli/cmd/feed.go +++ b/client/go/internal/cli/cmd/feed.go @@ -108,7 +108,7 @@ $ cat docs.jsonl | vespa feed -`, pprof.StartCPUProfile(f) defer pprof.StopCPUProfile() } - err := feed(args, options, cli) + err := feed(args, options, cli, cmd) if options.memprofile != "" { f, err := os.Create(options.memprofile) if err != nil { @@ -124,7 +124,7 @@ $ cat docs.jsonl | vespa feed -`, return cmd } -func createServices(n int, timeout time.Duration, waitSecs int, cli *CLI) ([]httputil.Client, string, error) { +func createServices(n int, timeout time.Duration, cli *CLI, waiter *Waiter) ([]httputil.Client, string, error) { if n < 1 { return nil, "", fmt.Errorf("need at least one client") } @@ -134,7 +134,6 @@ func createServices(n int, timeout time.Duration, waitSecs int, cli *CLI) ([]htt } services := make([]httputil.Client, 0, n) baseURL := "" - 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 { @@ -228,9 +227,10 @@ func enqueueAndWait(files []string, dispatcher *document.Dispatcher, options fee return fmt.Errorf("at least one file to feed from must specified") } -func feed(files []string, options feedOptions, cli *CLI) error { +func feed(files []string, options feedOptions, cli *CLI, cmd *cobra.Command) error { timeout := time.Duration(options.timeoutSecs) * time.Second - clients, baseURL, err := createServices(options.connections, timeout, options.waitSecs, cli) + waiter := cli.waiter(time.Duration(options.waitSecs)*time.Second, cmd) + clients, baseURL, err := createServices(options.connections, timeout, cli, waiter) if err != nil { return err } diff --git a/client/go/internal/cli/cmd/prod.go b/client/go/internal/cli/cmd/prod.go index 620ec055a1d..08d4086719d 100644 --- a/client/go/internal/cli/cmd/prod.go +++ b/client/go/internal/cli/cmd/prod.go @@ -430,6 +430,6 @@ func verifyTest(cli *CLI, testsParent string, suite string, required bool) error } return nil } - _, _, err = runTests(cli, testDirectory, true, 0) + _, _, err = runTests(cli, testDirectory, true, nil) return err } diff --git a/client/go/internal/cli/cmd/query.go b/client/go/internal/cli/cmd/query.go index db6dfa0a158..6feead66082 100644 --- a/client/go/internal/cli/cmd/query.go +++ b/client/go/internal/cli/cmd/query.go @@ -45,7 +45,8 @@ can be set by the syntax [parameter-name]=[value].`, SilenceUsage: true, Args: cobra.MinimumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - return query(cli, args, queryTimeoutSecs, waitSecs, printCurl, format, headers) + waiter := cli.waiter(time.Duration(waitSecs)*time.Second, cmd) + return query(cli, args, queryTimeoutSecs, printCurl, format, headers, waiter) }, } cmd.Flags().BoolVarP(&printCurl, "verbose", "v", false, "Print the equivalent curl command for the query") @@ -81,12 +82,11 @@ func parseHeaders(headers []string) (http.Header, error) { return h, nil } -func query(cli *CLI, arguments []string, timeoutSecs, waitSecs int, curl bool, format string, headers []string) error { +func query(cli *CLI, arguments []string, timeoutSecs int, curl bool, format string, headers []string, waiter *Waiter) error { target, err := cli.target(targetOptions{}) if err != nil { return err } - 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 8e0f3de4f72..6d0c747954a 100644 --- a/client/go/internal/cli/cmd/root.go +++ b/client/go/internal/cli/cmd/root.go @@ -54,6 +54,7 @@ type CLI struct { now func() time.Time retryInterval time.Duration + waitTimeout *time.Duration cmd *cobra.Command config *Config @@ -376,7 +377,9 @@ func (c *CLI) confirm(question string, confirmByDefault bool) (bool, error) { } } -func (c *CLI) waiter(timeout time.Duration) *Waiter { return &Waiter{Timeout: timeout, cli: c} } +func (c *CLI) waiter(timeout time.Duration, cmd *cobra.Command) *Waiter { + return &Waiter{Timeout: timeout, cli: c, cmd: cmd} +} // 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 e1c329ba993..d660e71abf9 100644 --- a/client/go/internal/cli/cmd/status.go +++ b/client/go/internal/cli/cmd/status.go @@ -50,7 +50,7 @@ $ vepsa status --format plain --cluster mycluster`, if err := verifyFormat(format); err != nil { return err } - waiter := cli.waiter(time.Duration(waitSecs) * time.Second) + waiter := cli.waiter(time.Duration(waitSecs)*time.Second, cmd) var failingContainers []*vespa.Service if cluster == "" { services, err := waiter.Services(t) @@ -126,7 +126,7 @@ func newStatusDeployCmd(cli *CLI) *cobra.Command { if err := verifyFormat(format); err != nil { return err } - waiter := cli.waiter(time.Duration(waitSecs) * time.Second) + waiter := cli.waiter(time.Duration(waitSecs)*time.Second, cmd) s, err := waiter.DeployService(t) if err != nil { return err @@ -174,7 +174,7 @@ $ vespa status deployment -t local [session-id] --wait 600 if err != nil { return err } - waiter := cli.waiter(time.Duration(waitSecs) * time.Second) + waiter := cli.waiter(time.Duration(waitSecs)*time.Second, cmd) id, err := waiter.Deployment(t, wantedID) if err != nil { var hints []string diff --git a/client/go/internal/cli/cmd/test.go b/client/go/internal/cli/cmd/test.go index 3bc78fc91c8..5144abe95b4 100644 --- a/client/go/internal/cli/cmd/test.go +++ b/client/go/internal/cli/cmd/test.go @@ -42,7 +42,8 @@ $ vespa test src/test/application/tests/system-test/feed-and-query.json`, DisableAutoGenTag: true, SilenceUsage: true, RunE: func(cmd *cobra.Command, args []string) error { - count, failed, err := runTests(cli, args[0], false, waitSecs) + waiter := cli.waiter(time.Duration(waitSecs)*time.Second, cmd) + count, failed, err := runTests(cli, args[0], false, waiter) if err != nil { return err } @@ -70,7 +71,7 @@ $ vespa test src/test/application/tests/system-test/feed-and-query.json`, return testCmd } -func runTests(cli *CLI, rootPath string, dryRun bool, waitSecs int) (int, []string, error) { +func runTests(cli *CLI, rootPath string, dryRun bool, waiter *Waiter) (int, []string, error) { count := 0 failed := make([]string, 0) if stat, err := os.Stat(rootPath); err != nil { @@ -89,7 +90,7 @@ func runTests(cli *CLI, rootPath string, dryRun bool, waitSecs int) (int, []stri fmt.Fprintln(cli.Stdout, "") previousFailed = false } - failure, err := runTest(testPath, context, waitSecs) + failure, err := runTest(testPath, context, waiter) if err != nil { return 0, nil, err } @@ -101,7 +102,7 @@ func runTests(cli *CLI, rootPath string, dryRun bool, waitSecs int) (int, []stri } } } else if strings.HasSuffix(stat.Name(), ".json") { - failure, err := runTest(rootPath, testContext{testsPath: filepath.Dir(rootPath), dryRun: dryRun, cli: cli, clusters: map[string]*vespa.Service{}}, waitSecs) + failure, err := runTest(rootPath, testContext{testsPath: filepath.Dir(rootPath), dryRun: dryRun, cli: cli, clusters: map[string]*vespa.Service{}}, waiter) if err != nil { return 0, nil, err } @@ -117,7 +118,7 @@ func runTests(cli *CLI, rootPath string, dryRun bool, waitSecs int) (int, []stri } // Runs the test at the given path, and returns the specified test name if the test fails -func runTest(testPath string, context testContext, waitSecs int) (string, error) { +func runTest(testPath string, context testContext, waiter *Waiter) (string, error) { var test test testBytes, err := os.ReadFile(testPath) if err != nil { @@ -150,7 +151,7 @@ func runTest(testPath string, context testContext, waitSecs int) (string, error) if step.Name != "" { stepName += ": " + step.Name } - failure, longFailure, err := verify(step, test.Defaults.Cluster, defaultParameters, context, waitSecs) + failure, longFailure, err := verify(step, test.Defaults.Cluster, defaultParameters, context, waiter) if err != nil { fmt.Fprintln(context.cli.Stderr) return "", errHint(fmt.Errorf("error in %s: %w", stepName, err), "See https://docs.vespa.ai/en/reference/testing") @@ -173,7 +174,7 @@ func runTest(testPath string, context testContext, waitSecs int) (string, error) } // Asserts specified response is obtained for request, or returns a failure message, or an error if this fails -func verify(step step, defaultCluster string, defaultParameters map[string]string, context testContext, waitSecs int) (string, string, error) { +func verify(step step, defaultCluster string, defaultParameters map[string]string, context testContext, waiter *Waiter) (string, string, error) { requestBody, err := getBody(step.Request.BodyRaw, context.testsPath) if err != nil { return "", "", err @@ -227,9 +228,8 @@ func verify(step step, defaultCluster string, defaultParameters map[string]strin } ok := false service, ok = context.clusters[cluster] - if !ok { + if !ok && waiter != nil { // Cache service so we don't have to discover it for every step - 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/visit.go b/client/go/internal/cli/cmd/visit.go index 963833337c2..7f99df2038e 100644 --- a/client/go/internal/cli/cmd/visit.go +++ b/client/go/internal/cli/cmd/visit.go @@ -109,7 +109,8 @@ $ vespa visit --field-set "[id]" # list document IDs if !result.Success { return fmt.Errorf("argument error: %s", result.Message) } - service, err := documentService(cli, vArgs.waitSecs) + waiter := cli.waiter(time.Duration(vArgs.waitSecs)*time.Second, cmd) + service, err := documentService(cli, waiter) if err != nil { return err } diff --git a/client/go/internal/cli/cmd/visit_test.go b/client/go/internal/cli/cmd/visit_test.go index 3053b987838..ae494059691 100644 --- a/client/go/internal/cli/cmd/visit_test.go +++ b/client/go/internal/cli/cmd/visit_test.go @@ -97,7 +97,7 @@ func withMockClient(t *testing.T, prepCli func(*mock.HTTPClient), runOp func(*ve prepCli(client) cli, _, _ := newTestCLI(t) cli.httpClient = client - service, err := documentService(cli, 0) + service, err := documentService(cli, &Waiter{cli: cli}) if err != nil { t.Fatal(err) } diff --git a/client/go/internal/cli/cmd/waiter.go b/client/go/internal/cli/cmd/waiter.go index d818359e61c..591cbf0c4ba 100644 --- a/client/go/internal/cli/cmd/waiter.go +++ b/client/go/internal/cli/cmd/waiter.go @@ -2,10 +2,12 @@ package cmd import ( + "errors" "fmt" "time" "github.com/fatih/color" + "github.com/spf13/cobra" "github.com/vespa-engine/vespa/client/go/internal/vespa" ) @@ -15,6 +17,7 @@ type Waiter struct { Timeout time.Duration // TODO(mpolden): Consider making this a budget cli *CLI + cmd *cobra.Command } // DeployService returns the service providing the deploy API on given target, @@ -81,10 +84,25 @@ func (w *Waiter) services(target vespa.Target) ([]*vespa.Service, error) { return target.ContainerServices(w.Timeout) } +// FastWaitOn returns whether we should use a short default timeout for given target. +func (w *Waiter) FastWaitOn(target vespa.Target) bool { + return target.IsCloud() && w.Timeout == 0 && !w.cmd.PersistentFlags().Changed("wait") +} + // Deployment waits for a deployment to become ready, returning the ID of the converged deployment. func (w *Waiter) Deployment(target vespa.Target, id int64) (int64, error) { - if w.Timeout > 0 { - w.cli.printInfo("Waiting up to ", color.CyanString(w.Timeout.String()), " for deployment to converge...") + timeout := w.Timeout + fastWait := w.FastWaitOn(target) + if timeout > 0 { + w.cli.printInfo("Waiting up to ", color.CyanString(timeout.String()), " for deployment to converge...") + } else if fastWait { + // If --wait is not explicitly given, we always wait a few seconds in Cloud to catch fast failures, e.g. + // invalid application package + timeout = 2 * time.Second + } + id, err := target.AwaitDeployment(id, timeout) + if fastWait && errors.Is(err, vespa.ErrWaitTimeout) { + return id, nil // Do not report fast wait timeout as an error } - return target.AwaitDeployment(id, w.Timeout) + return id, err } diff --git a/client/go/internal/mock/http.go b/client/go/internal/mock/http.go index 48835dd3338..8274f4113d3 100644 --- a/client/go/internal/mock/http.go +++ b/client/go/internal/mock/http.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "net/http" + "os" "strconv" "time" ) @@ -17,6 +18,9 @@ type HTTPClient struct { // The errors to return for future requests. If non-nil, these errors are returned before any responses in nextResponses. nextErrors []error + // LogRequests enables logging of all requests made through this client. + LogRequests bool + // LastRequest is the last HTTP request made through this. LastRequest *http.Request @@ -59,6 +63,9 @@ func (c *HTTPClient) NextResponseError(err error) { func (c *HTTPClient) Consumed() bool { return len(c.nextResponses) == 0 } func (c *HTTPClient) Do(request *http.Request, timeout time.Duration) (*http.Response, error) { + if c.LogRequests { + fmt.Fprintf(os.Stderr, "Sending request %+v\n", request) + } c.LastRequest = request if len(c.nextErrors) > 0 { err := c.nextErrors[0] diff --git a/client/go/internal/vespa/target.go b/client/go/internal/vespa/target.go index c1a7747f978..a35d8dd0741 100644 --- a/client/go/internal/vespa/target.go +++ b/client/go/internal/vespa/target.go @@ -36,11 +36,14 @@ const ( AnyDeployment int64 = -2 ) -var errWaitTimeout = errors.New("giving up") var errAuth = errors.New("auth failed") -// ErrDeployment is the error returned for terminal deployment failures. -var ErrDeployment = errors.New("deployment failed") +var ( + // ErrWaitTimeout is the error returned when waiting for something times out. + ErrWaitTimeout = errors.New("giving up") + // ErrDeployment is the error returned for terminal deployment failures. + ErrDeployment = errors.New("deployment failed") +) // Authenticator authenticates the given HTTP request. type Authenticator interface { @@ -293,7 +296,7 @@ func wait(service *Service, okFn responseFunc, reqFn requestFunc, timeout, retry time.Sleep(retryInterval) } if err == nil { - return status, errWaitTimeout + return status, ErrWaitTimeout } return status, err } diff --git a/client/go/internal/vespa/target_cloud.go b/client/go/internal/vespa/target_cloud.go index da57fc66807..d07b2c77ab4 100644 --- a/client/go/internal/vespa/target_cloud.go +++ b/client/go/internal/vespa/target_cloud.go @@ -233,7 +233,7 @@ func (t *cloudTarget) PrintLog(options LogOptions) error { timeout = math.MaxInt64 // No timeout } // Ignore wait error because logFunc has no concept of completion, we just want to print log entries until timeout is reached - if _, err := t.deployServiceWait(logFunc, requestFunc, timeout); err != nil && !errors.Is(err, errWaitTimeout) { + if _, err := t.deployServiceWait(logFunc, requestFunc, timeout); err != nil && !errors.Is(err, ErrWaitTimeout) { return fmt.Errorf("failed to read logs: %s", err) } return nil -- cgit v1.2.3