diff options
32 files changed, 568 insertions, 448 deletions
diff --git a/client/go/cmd/api_key.go b/client/go/cmd/api_key.go index 0f665007107..faab67781d1 100644 --- a/client/go/cmd/api_key.go +++ b/client/go/cmd/api_key.go @@ -54,8 +54,9 @@ var apiKeyCmd = &cobra.Command{ Long: apiKeyLongDoc, Example: apiKeyExample(), DisableAutoGenTag: true, + SilenceUsage: true, Args: cobra.ExactArgs(0), - Run: doApiKey, + RunE: doApiKey, } var deprecatedApiKeyCmd = &cobra.Command{ @@ -64,29 +65,32 @@ var deprecatedApiKeyCmd = &cobra.Command{ Long: apiKeyLongDoc, Example: apiKeyExample(), DisableAutoGenTag: true, + SilenceUsage: true, Args: cobra.ExactArgs(0), Hidden: true, Deprecated: "use 'vespa auth api-key' instead", - Run: doApiKey, + RunE: doApiKey, } -func doApiKey(_ *cobra.Command, _ []string) { +func doApiKey(_ *cobra.Command, _ []string) error { cfg, err := LoadConfig() if err != nil { - fatalErr(err, "Could not load config") - return + return fmt.Errorf("could not load config: %w", err) + } + app, err := getApplication() + if err != nil { + return err } - app := getApplication() apiKeyFile := cfg.APIKeyPath(app.Tenant) if util.PathExists(apiKeyFile) && !overwriteKey { - printErrHint(fmt.Errorf("File %s already exists", apiKeyFile), "Use -f to overwrite it") + err := fmt.Errorf("refusing to overwrite %s", apiKeyFile) + printErrHint(err, "Use -f to overwrite it") printPublicKey(apiKeyFile, app.Tenant) - return + return ErrCLI{error: err, quiet: true} } apiKey, err := vespa.CreateAPIKey() if err != nil { - fatalErr(err, "Could not create API key") - return + return fmt.Errorf("could not create api key: %w", err) } if err := ioutil.WriteFile(apiKeyFile, apiKey, 0600); err == nil { printSuccess("API private key written to ", apiKeyFile) @@ -94,41 +98,40 @@ func doApiKey(_ *cobra.Command, _ []string) { if vespa.Auth0AccessTokenEnabled() { if err == nil { if err := cfg.Set(cloudAuthFlag, "api-key"); err != nil { - fatalErr(err, "Could not write config") + return fmt.Errorf("could not write config: %w", err) } if err := cfg.Write(); err != nil { - fatalErr(err) + return err } } } + return nil } else { - fatalErr(err, "Failed to write ", apiKeyFile) + return fmt.Errorf("failed to write: %s: %w", apiKeyFile, err) } } -func printPublicKey(apiKeyFile, tenant string) { +func printPublicKey(apiKeyFile, tenant string) error { pemKeyData, err := ioutil.ReadFile(apiKeyFile) if err != nil { - fatalErr(err, "Failed to read ", apiKeyFile) - return + return fmt.Errorf("failed to read: %s: %w", apiKeyFile, err) } key, err := vespa.ECPrivateKeyFrom(pemKeyData) if err != nil { - fatalErr(err, "Failed to load key") - return + return fmt.Errorf("failed to load key: %w", err) } pemPublicKey, err := vespa.PEMPublicKeyFrom(key) if err != nil { - fatalErr(err, "Failed to extract public key") - return + return fmt.Errorf("failed to extract public key: %w", err) } fingerprint, err := vespa.FingerprintMD5(pemPublicKey) if err != nil { - fatalErr(err, "Failed to extract fingerprint") + return fmt.Errorf("failed to extract fingerprint: %w", err) } log.Printf("\nThis is your public key:\n%s", color.Green(pemPublicKey)) log.Printf("Its fingerprint is:\n%s\n", color.Cyan(fingerprint)) log.Print("\nTo use this key in Vespa Cloud click 'Add custom key' at") log.Printf(color.Cyan("%s/tenant/%s/keys").String(), getConsoleURL(), tenant) log.Print("and paste the entire public key including the BEGIN and END lines.") + return nil } diff --git a/client/go/cmd/api_key_test.go b/client/go/cmd/api_key_test.go index b08758ae21d..df3cf144180 100644 --- a/client/go/cmd/api_key_test.go +++ b/client/go/cmd/api_key_test.go @@ -18,6 +18,6 @@ func TestAPIKey(t *testing.T) { assert.Contains(t, out, "Success: API private key written to "+keyFile+"\n") out, outErr := execute(command{args: []string{"api-key", "-a", "t1.a1.i1"}, homeDir: homeDir}, t, nil) - assert.Contains(t, outErr, "Error: File "+keyFile+" already exists\nHint: Use -f to overwrite it\n") + assert.Contains(t, outErr, "Error: refusing to overwrite "+keyFile+"\nHint: Use -f to overwrite it\n") assert.Contains(t, out, "This is your public key") } diff --git a/client/go/cmd/auth.go b/client/go/cmd/auth.go index 9322f8d0808..68d7fa00fdf 100644 --- a/client/go/cmd/auth.go +++ b/client/go/cmd/auth.go @@ -1,6 +1,8 @@ package cmd import ( + "fmt" + "github.com/spf13/cobra" "github.com/vespa-engine/vespa/client/go/vespa" ) @@ -21,14 +23,13 @@ func init() { } var authCmd = &cobra.Command{ - Use: "auth", - Short: "Manage Vespa Cloud credentials", - Long: `Manage Vespa Cloud credentials.`, - + Use: "auth", + Short: "Manage Vespa Cloud credentials", + Long: `Manage Vespa Cloud credentials.`, DisableAutoGenTag: true, - Run: func(cmd *cobra.Command, args []string) { - // Root command does nothing - cmd.Help() - exitFunc(1) + SilenceUsage: false, + Args: cobra.MinimumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + return fmt.Errorf("invalid command: %s", args[0]) }, } diff --git a/client/go/cmd/cert.go b/client/go/cmd/cert.go index 9319c799ae5..f338a1ba81a 100644 --- a/client/go/cmd/cert.go +++ b/client/go/cmd/cert.go @@ -59,8 +59,9 @@ var certCmd = &cobra.Command{ Long: longDoc, Example: certExample(), DisableAutoGenTag: true, + SilenceUsage: true, Args: cobra.MaximumNArgs(1), - Run: doCert, + RunE: doCert, } var deprecatedCertCmd = &cobra.Command{ @@ -69,85 +70,76 @@ var deprecatedCertCmd = &cobra.Command{ Long: longDoc, Example: "$ vespa cert -a my-tenant.my-app.my-instance", DisableAutoGenTag: true, + SilenceUsage: true, Args: cobra.MaximumNArgs(1), Deprecated: "use 'vespa auth cert' instead", Hidden: true, - Run: doCert, + RunE: doCert, } -func doCert(_ *cobra.Command, args []string) { - app := getApplication() +func doCert(_ *cobra.Command, args []string) error { + app, err := getApplication() + if err != nil { + return err + } pkg, err := vespa.FindApplicationPackage(applicationSource(args), false) if err != nil { - fatalErr(err) - return + return err } cfg, err := LoadConfig() if err != nil { - fatalErr(err) - return + return err } privateKeyFile, err := cfg.PrivateKeyPath(app) if err != nil { - fatalErr(err) - return + return err } certificateFile, err := cfg.CertificatePath(app) if err != nil { - fatalErr(err) - return + return err } if !overwriteCertificate { hint := "Use -f flag to force overwriting" if pkg.HasCertificate() { - fatalErrHint(fmt.Errorf("Application package %s already contains a certificate", pkg.Path), hint) - return + return errHint(fmt.Errorf("application package %s already contains a certificate", pkg.Path), hint) } if util.PathExists(privateKeyFile) { - fatalErrHint(fmt.Errorf("Private key %s already exists", color.Cyan(privateKeyFile)), hint) - return + return errHint(fmt.Errorf("private key %s already exists", color.Cyan(privateKeyFile)), hint) } if util.PathExists(certificateFile) { - fatalErrHint(fmt.Errorf("Certificate %s already exists", color.Cyan(certificateFile)), hint) - return + return errHint(fmt.Errorf("certificate %s already exists", color.Cyan(certificateFile)), hint) } } if pkg.IsZip() { - var msg string + var hint string if vespa.Auth0AccessTokenEnabled() { - msg = "Try running 'mvn clean' before 'vespa auth cert', and then 'mvn package'" + hint = "Try running 'mvn clean' before 'vespa auth cert', and then 'mvn package'" } else { - msg = "Try running 'mvn clean' before 'vespa cert', and then 'mvn package'" + hint = "Try running 'mvn clean' before 'vespa cert', and then 'mvn package'" } - fatalErrHint(fmt.Errorf("Cannot add certificate to compressed application package %s", pkg.Path), - msg) - return + return errHint(fmt.Errorf("cannot add certificate to compressed application package %s", pkg.Path), hint) } keyPair, err := vespa.CreateKeyPair() if err != nil { - fatalErr(err, "Could not create key pair") - return + return err } pkgCertificateFile := filepath.Join(pkg.Path, "security", "clients.pem") if err := os.MkdirAll(filepath.Dir(pkgCertificateFile), 0755); err != nil { - fatalErr(err, "Could not create security directory") - return + return fmt.Errorf("could not create security directory: %w", err) } if err := keyPair.WriteCertificateFile(pkgCertificateFile, overwriteCertificate); err != nil { - fatalErr(err, "Could not write certificate") - return + return fmt.Errorf("could not write certificate to application package: %w", err) } if err := keyPair.WriteCertificateFile(certificateFile, overwriteCertificate); err != nil { - fatalErr(err, "Could not write certificate") - return + return fmt.Errorf("could not write certificate: %w", err) } if err := keyPair.WritePrivateKeyFile(privateKeyFile, overwriteCertificate); err != nil { - fatalErr(err, "Could not write private key") - return + return fmt.Errorf("could not write private key: %w", err) } printSuccess("Certificate written to ", color.Cyan(pkgCertificateFile)) printSuccess("Certificate written to ", color.Cyan(certificateFile)) printSuccess("Private key written to ", color.Cyan(privateKeyFile)) + return nil } diff --git a/client/go/cmd/cert_test.go b/client/go/cmd/cert_test.go index 96b626b5c98..aae40db43a6 100644 --- a/client/go/cmd/cert_test.go +++ b/client/go/cmd/cert_test.go @@ -29,7 +29,7 @@ func TestCert(t *testing.T) { assert.Equal(t, fmt.Sprintf("Success: Certificate written to %s\nSuccess: Certificate written to %s\nSuccess: Private key written to %s\n", pkgCertificate, certificate, privateKey), out) _, outErr := execute(command{args: []string{"cert", "-a", "t1.a1.i1", pkgDir}, homeDir: homeDir}, t, nil) - assert.Contains(t, outErr, fmt.Sprintf("Error: Application package %s already contains a certificate", appDir)) + assert.Contains(t, outErr, fmt.Sprintf("Error: application package %s already contains a certificate", appDir)) } func TestCertCompressedPackage(t *testing.T) { @@ -42,7 +42,7 @@ func TestCertCompressedPackage(t *testing.T) { assert.Nil(t, err) _, outErr := execute(command{args: []string{"cert", "-a", "t1.a1.i1", pkgDir}, homeDir: homeDir}, t, nil) - assert.Contains(t, outErr, "Error: Cannot add certificate to compressed application package") + assert.Contains(t, outErr, "Error: cannot add certificate to compressed application package") err = os.Remove(zipFile) assert.Nil(t, err) diff --git a/client/go/cmd/clone.go b/client/go/cmd/clone.go index 1ba95668d29..7a25613947a 100644 --- a/client/go/cmd/clone.go +++ b/client/go/cmd/clone.go @@ -45,34 +45,35 @@ directory can be overriden by setting the VESPA_CLI_CACHE_DIR environment variable.`, Example: "$ vespa clone vespa-cloud/album-recommendation my-app", DisableAutoGenTag: true, - Run: func(cmd *cobra.Command, args []string) { + SilenceUsage: true, + RunE: func(cmd *cobra.Command, args []string) error { if listApps { apps, err := listSampleApps() if err != nil { - fatalErr(err, "Could not list sample applications") - return + return fmt.Errorf("could not list sample applications: %w", err) } for _, app := range apps { log.Print(app) } - } else { - if len(args) != 2 { - fatalErr(nil, "Expected exactly 2 arguments") - return - } - cloneApplication(args[0], args[1]) + return nil + } + if len(args) != 2 { + return fmt.Errorf("expected exactly 2 arguments, got %d", len(args)) } + return cloneApplication(args[0], args[1]) }, } -func cloneApplication(applicationName string, applicationDir string) { - zipFile := getSampleAppsZip() +func cloneApplication(applicationName string, applicationDir string) error { + zipFile, err := getSampleAppsZip() + if err != nil { + return err + } defer zipFile.Close() r, err := zip.OpenReader(zipFile.Name()) if err != nil { - fatalErr(err, "Could not open sample apps zip '", color.Cyan(zipFile.Name()), "'") - return + return fmt.Errorf("could not open sample apps zip '%s': %w", color.Cyan(zipFile.Name()), err) } defer r.Close() @@ -83,23 +84,22 @@ func cloneApplication(applicationName string, applicationDir string) { if !found { // Create destination directory lazily when source is found createErr := os.Mkdir(applicationDir, 0755) if createErr != nil { - fatalErr(createErr, "Could not create directory '", color.Cyan(applicationDir), "'") - return + return fmt.Errorf("could not create directory '%s': %w", color.Cyan(applicationDir), createErr) } } found = true if err := copy(f, applicationDir, dirPrefix); err != nil { - fatalErr(err, "Could not copy zip entry '", color.Cyan(f.Name), "' to ", color.Cyan(applicationDir)) - return + return fmt.Errorf("could not copy zip entry '%s': %w", color.Cyan(f.Name), err) } } } if !found { - fatalErrHint(fmt.Errorf("Could not find source application '%s'", color.Cyan(applicationName)), "Use -f to ignore the cache") + return errHint(fmt.Errorf("could not find source application '%s'", color.Cyan(applicationName)), "Use -f to ignore the cache") } else { log.Print("Created ", color.Cyan(applicationDir)) } + return nil } func openOutputFile() (*os.File, error) { @@ -125,50 +125,41 @@ func useCache(cacheFile *os.File) (bool, error) { return stat.Size() > 0 && time.Now().Before(expiry), nil } -func getSampleAppsZip() *os.File { +func getSampleAppsZip() (*os.File, error) { f, err := openOutputFile() if err != nil { - fatalErr(err, "Could not determine location of cache file") - return nil + return nil, fmt.Errorf("could not determine location of cache file: %w", err) } useCache, err := useCache(f) if err != nil { - fatalErr(err, "Could not determine cache status", "Try ignoring the cache with the -f flag") - return nil + return nil, errHint(fmt.Errorf("could not determine cache status: %w", err), "Try ignoring the cache with the -f flag") } if useCache { log.Print(color.Yellow("Using cached sample apps ...")) - return f + return f, nil } - err = util.Spinner(color.Yellow("Downloading sample apps ...").String(), func() error { request, err := http.NewRequest("GET", "https://github.com/vespa-engine/sample-apps/archive/refs/heads/master.zip", nil) if err != nil { - fatalErr(err, "Invalid URL") - return nil + return fmt.Errorf("invalid url: %w", err) } response, err := util.HttpDo(request, time.Minute*60, "GitHub") if err != nil { - fatalErr(err, "Could not download sample apps from GitHub") - return nil + return fmt.Errorf("could not download sample apps: %w", err) } defer response.Body.Close() if response.StatusCode != 200 { - fatalErr(nil, "Could not download sample apps from GitHub: ", response.StatusCode) - return nil + return fmt.Errorf("could not download sample apps: github returned status %d", response.StatusCode) } if err := f.Truncate(0); err != nil { - fatalErr(err, "Could not truncate sample apps file: ", f.Name()) - return nil + return fmt.Errorf("could not truncate sample apps file: %s: %w", f.Name(), err) } if _, err := io.Copy(f, response.Body); err != nil { - fatalErr(err, "Could not write sample apps to file: ", f.Name()) - return nil + return fmt.Errorf("could not write sample apps to file: %s: %w", f.Name(), err) } - return err + return nil }) - - return f + return f, err } func copy(f *zip.File, destinationDir string, zipEntryPrefix string) error { diff --git a/client/go/cmd/command_tester.go b/client/go/cmd/command_tester.go index eb55021b536..135dda895d4 100644 --- a/client/go/cmd/command_tester.go +++ b/client/go/cmd/command_tester.go @@ -63,9 +63,6 @@ func execute(cmd command, t *testing.T, client *mockHttpClient) (string, string) rootCmd.Flags().VisitAll(resetFlag) documentCmd.Flags().VisitAll(resetFlag) - // Do not exit in tests - exitFunc = func(code int) {} - // Capture stdout and execute command var capturedOut bytes.Buffer var capturedErr bytes.Buffer @@ -79,7 +76,7 @@ func execute(cmd command, t *testing.T, client *mockHttpClient) (string, string) // Execute command and return output rootCmd.SetArgs(append(cmd.args, cmd.moreArgs...)) - rootCmd.Execute() + Execute() return capturedOut.String(), capturedErr.String() } diff --git a/client/go/cmd/config.go b/client/go/cmd/config.go index bd8353cad3f..a195a3e1975 100644 --- a/client/go/cmd/config.go +++ b/client/go/cmd/config.go @@ -46,10 +46,10 @@ instead. Configuration is written to $HOME/.vespa by default. This path can be overridden by setting the VESPA_CLI_HOME environment variable.`, DisableAutoGenTag: true, - Run: func(cmd *cobra.Command, args []string) { - // Root command does nothing - cmd.Help() - exitFunc(1) + SilenceUsage: false, + Args: cobra.MinimumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + return fmt.Errorf("invalid command: %s", args[0]) }, } @@ -58,20 +58,17 @@ var setConfigCmd = &cobra.Command{ Short: "Set a configuration option.", Example: "$ vespa config set target cloud", DisableAutoGenTag: true, + SilenceUsage: true, Args: cobra.ExactArgs(2), - Run: func(cmd *cobra.Command, args []string) { + RunE: func(cmd *cobra.Command, args []string) error { cfg, err := LoadConfig() if err != nil { - fatalErr(err, "Could not load config") - return + return err } if err := cfg.Set(args[0], args[1]); err != nil { - fatalErr(err) - } else { - if err := cfg.Write(); err != nil { - fatalErr(err) - } + return err } + return cfg.Write() }, } @@ -82,13 +79,12 @@ var getConfigCmd = &cobra.Command{ $ vespa config get target`, Args: cobra.MaximumNArgs(1), DisableAutoGenTag: true, - Run: func(cmd *cobra.Command, args []string) { + SilenceUsage: true, + RunE: func(cmd *cobra.Command, args []string) error { cfg, err := LoadConfig() if err != nil { - fatalErr(err, "Could not load config") - return + return err } - if len(args) == 0 { // Print all values var flags []string for flag := range flagToConfigBindings { @@ -101,6 +97,7 @@ $ vespa config get target`, } else { printOption(cfg, args[0]) } + return nil }, } @@ -118,11 +115,11 @@ type KeyPair struct { func LoadConfig() (*Config, error) { home, err := vespaCliHome() if err != nil { - return nil, err + return nil, fmt.Errorf("could not detect config directory: %w", err) } c := &Config{Home: home, createDirs: true} if err := c.load(); err != nil { - return nil, err + return nil, fmt.Errorf("could not load config: %w", err) } return c, nil } diff --git a/client/go/cmd/config_test.go b/client/go/cmd/config_test.go index 0e74e53c5e5..ba0df781715 100644 --- a/client/go/cmd/config_test.go +++ b/client/go/cmd/config_test.go @@ -10,7 +10,7 @@ import ( func TestConfig(t *testing.T) { homeDir := filepath.Join(t.TempDir(), ".vespa") - assertConfigCommandErr(t, "invalid option or value: \"foo\": \"bar\"\n", homeDir, "config", "set", "foo", "bar") + assertConfigCommandErr(t, "Error: invalid option or value: \"foo\": \"bar\"\n", homeDir, "config", "set", "foo", "bar") assertConfigCommand(t, "foo = <unset>\n", homeDir, "config", "get", "foo") assertConfigCommand(t, "target = local\n", homeDir, "config", "get", "target") assertConfigCommand(t, "", homeDir, "config", "set", "target", "cloud") @@ -19,7 +19,7 @@ func TestConfig(t *testing.T) { assertConfigCommand(t, "", homeDir, "config", "set", "target", "https://127.0.0.1") assertConfigCommand(t, "target = https://127.0.0.1\n", homeDir, "config", "get", "target") - assertConfigCommandErr(t, "invalid application: \"foo\"\n", homeDir, "config", "set", "application", "foo") + assertConfigCommandErr(t, "Error: invalid application: \"foo\"\n", homeDir, "config", "set", "application", "foo") assertConfigCommand(t, "application = <unset>\n", homeDir, "config", "get", "application") assertConfigCommand(t, "", homeDir, "config", "set", "application", "t1.a1.i1") assertConfigCommand(t, "application = t1.a1.i1\n", homeDir, "config", "get", "application") @@ -27,7 +27,7 @@ func TestConfig(t *testing.T) { assertConfigCommand(t, "application = t1.a1.i1\ncolor = auto\nquiet = false\ntarget = https://127.0.0.1\nwait = 0\n", homeDir, "config", "get") assertConfigCommand(t, "", homeDir, "config", "set", "wait", "60") - assertConfigCommandErr(t, "wait option must be an integer >= 0, got \"foo\"\n", homeDir, "config", "set", "wait", "foo") + assertConfigCommandErr(t, "Error: wait option must be an integer >= 0, got \"foo\"\n", homeDir, "config", "set", "wait", "foo") assertConfigCommand(t, "wait = 60\n", homeDir, "config", "get", "wait") } diff --git a/client/go/cmd/curl.go b/client/go/cmd/curl.go index 41b1a7ccc58..073e415da76 100644 --- a/client/go/cmd/curl.go +++ b/client/go/cmd/curl.go @@ -2,6 +2,7 @@ package cmd import ( + "fmt" "log" "os" "strings" @@ -30,31 +31,34 @@ For a more high-level interface to query and feeding, see the 'query' and 'docum $ vespa curl -- -X POST -H "Content-Type:application/json" --data-binary @src/test/resources/A-Head-Full-of-Dreams.json /document/v1/namespace/music/docid/1 $ vespa curl -- -v --data-urlencode "yql=select * from music where album contains 'head';" /search/\?hits=5`, DisableAutoGenTag: true, + SilenceUsage: true, Args: cobra.MinimumNArgs(1), - Run: func(cmd *cobra.Command, args []string) { + RunE: func(cmd *cobra.Command, args []string) error { cfg, err := LoadConfig() if err != nil { - fatalErr(err, "Could not load config") - return + return err + } + app, err := getApplication() + if err != nil { + return err } - app := getApplication() privateKeyFile, err := cfg.PrivateKeyPath(app) if err != nil { - fatalErr(err) - return + return err } certificateFile, err := cfg.CertificatePath(app) if err != nil { - fatalErr(err) - return + return err + } + service, err := getService("query", 0, "") + if err != nil { + return err } - service := getService("query", 0, "") url := joinURL(service.BaseURL, args[len(args)-1]) rawArgs := args[:len(args)-1] c, err := curl.RawArgs(url, rawArgs...) if err != nil { - fatalErr(err) - return + return err } c.PrivateKey = privateKeyFile c.Certificate = certificateFile @@ -63,10 +67,10 @@ $ vespa curl -- -v --data-urlencode "yql=select * from music where album contain log.Print(c.String()) } else { if err := c.Run(os.Stdout, os.Stderr); err != nil { - fatalErr(err, "Failed to run curl") - return + return fmt.Errorf("failed to execute curl: %w", err) } } + return nil }, } diff --git a/client/go/cmd/deploy.go b/client/go/cmd/deploy.go index ae39afc3773..e35188933e1 100644 --- a/client/go/cmd/deploy.go +++ b/client/go/cmd/deploy.go @@ -18,9 +18,8 @@ const ( ) var ( - zoneArg string - logLevelArg string - sessionOrRunID int64 + zoneArg string + logLevelArg string ) func init() { @@ -51,39 +50,45 @@ $ vespa deploy -t cloud -z dev.aws-us-east-1c # -z can be omitted here as this $ vespa deploy -t cloud -z perf.aws-us-east-1c`, Args: cobra.MaximumNArgs(1), DisableAutoGenTag: true, - Run: func(cmd *cobra.Command, args []string) { + SilenceUsage: true, + RunE: func(cmd *cobra.Command, args []string) error { pkg, err := vespa.FindApplicationPackage(applicationSource(args), true) if err != nil { - fatalErr(nil, err.Error()) - return + return err } cfg, err := LoadConfig() if err != nil { - fatalErr(err, "Could not load config") - return + return err + } + target, err := getTarget() + if err != nil { + return err + } + opts, err := getDeploymentOpts(cfg, pkg, target) + if err != nil { + return err + } + sessionOrRunID, err := vespa.Deploy(opts) + if err != nil { + return err } - target := getTarget() - opts := getDeploymentOpts(cfg, pkg, target) - if sessionOrRunID, err := vespa.Deploy(opts); err == nil { - fmt.Print("\n") - if opts.IsCloud() { - printSuccess("Triggered deployment of ", color.Cyan(pkg.Path), " with run ID ", color.Cyan(sessionOrRunID)) - } else { - printSuccess("Deployed ", color.Cyan(pkg.Path)) - } - if opts.IsCloud() { - log.Printf("\nUse %s for deployment status, or follow this deployment at", color.Cyan("vespa status")) - log.Print(color.Cyan(fmt.Sprintf("%s/tenant/%s/application/%s/dev/instance/%s/job/%s-%s/run/%d", - getConsoleURL(), - opts.Deployment.Application.Tenant, opts.Deployment.Application.Application, opts.Deployment.Application.Instance, - opts.Deployment.Zone.Environment, opts.Deployment.Zone.Region, - sessionOrRunID))) - } - waitForQueryService(sessionOrRunID) + fmt.Print("\n") + if opts.IsCloud() { + printSuccess("Triggered deployment of ", color.Cyan(pkg.Path), " with run ID ", color.Cyan(sessionOrRunID)) } else { - fatalErr(nil, err.Error()) + printSuccess("Deployed ", color.Cyan(pkg.Path)) } + if opts.IsCloud() { + log.Printf("\nUse %s for deployment status, or follow this deployment at", color.Cyan("vespa status")) + log.Print(color.Cyan(fmt.Sprintf("%s/tenant/%s/application/%s/dev/instance/%s/job/%s-%s/run/%d", + getConsoleURL(), + opts.Deployment.Application.Tenant, opts.Deployment.Application.Application, opts.Deployment.Application.Instance, + opts.Deployment.Zone.Environment, opts.Deployment.Zone.Region, + sessionOrRunID))) + } + waitForQueryService(sessionOrRunID) + return nil }, } @@ -92,31 +97,32 @@ var prepareCmd = &cobra.Command{ Short: "Prepare an application package for activation", Args: cobra.MaximumNArgs(1), DisableAutoGenTag: true, - Run: func(cmd *cobra.Command, args []string) { + SilenceUsage: true, + RunE: func(cmd *cobra.Command, args []string) error { pkg, err := vespa.FindApplicationPackage(applicationSource(args), true) if err != nil { - fatalErr(err, "Could not find application package") - return + return fmt.Errorf("could not find application package: %w", err) } cfg, err := LoadConfig() if err != nil { - fatalErr(err, "Could not load config") - return + return err + } + target, err := getTarget() + if err != nil { + return err } - target := getTarget() sessionID, err := vespa.Prepare(vespa.DeploymentOpts{ ApplicationPackage: pkg, Target: target, }) - if err == nil { - if err := cfg.WriteSessionID(vespa.DefaultApplication, sessionID); err != nil { - fatalErr(err, "Could not write session ID") - return - } - printSuccess("Prepared ", color.Cyan(pkg.Path), " with session ", sessionID) - } else { - fatalErr(nil, err.Error()) + if err != nil { + return err + } + if err := cfg.WriteSessionID(vespa.DefaultApplication, sessionID); err != nil { + return fmt.Errorf("could not write session id: %w", err) } + printSuccess("Prepared ", color.Cyan(pkg.Path), " with session ", sessionID) + return nil }, } @@ -125,33 +131,34 @@ var activateCmd = &cobra.Command{ Short: "Activate (deploy) a previously prepared application package", Args: cobra.MaximumNArgs(1), DisableAutoGenTag: true, - Run: func(cmd *cobra.Command, args []string) { + SilenceUsage: true, + RunE: func(cmd *cobra.Command, args []string) error { pkg, err := vespa.FindApplicationPackage(applicationSource(args), true) if err != nil { - fatalErr(err, "Could not find application package") - return + return fmt.Errorf("could not find application package: %w", err) } cfg, err := LoadConfig() if err != nil { - fatalErr(err, "Could not load config") - return + return err } sessionID, err := cfg.ReadSessionID(vespa.DefaultApplication) if err != nil { - fatalErr(err, "Could not read session ID") - return + return fmt.Errorf("could not read session id: %w", err) + } + target, err := getTarget() + if err != nil { + return err } - target := getTarget() err = vespa.Activate(sessionID, vespa.DeploymentOpts{ ApplicationPackage: pkg, Target: target, }) - if err == nil { - printSuccess("Activated ", color.Cyan(pkg.Path), " with session ", sessionID) - waitForQueryService(sessionID) - } else { - fatalErr(nil, err.Error()) + if err != nil { + return err } + printSuccess("Activated ", color.Cyan(pkg.Path), " with session ", sessionID) + waitForQueryService(sessionID) + return nil }, } diff --git a/client/go/cmd/deploy_test.go b/client/go/cmd/deploy_test.go index 5bb45e70fad..a37a433397f 100644 --- a/client/go/cmd/deploy_test.go +++ b/client/go/cmd/deploy_test.go @@ -164,7 +164,7 @@ func assertApplicationPackageError(t *testing.T, cmd string, status int, expecte client.NextResponse(status, returnBody) _, outErr := execute(command{args: []string{cmd, "testdata/applications/withTarget/target/application.zip"}}, t, client) assert.Equal(t, - "Error: Invalid application package (Status "+strconv.Itoa(status)+")\n\n"+expectedMessage+"\n", + "Error: invalid application package (Status "+strconv.Itoa(status)+")\n"+expectedMessage+"\n", outErr) } @@ -173,6 +173,6 @@ func assertDeployServerError(t *testing.T, status int, errorMessage string) { client.NextResponse(status, errorMessage) _, outErr := execute(command{args: []string{"deploy", "testdata/applications/withTarget/target/application.zip"}}, t, client) assert.Equal(t, - "Error: Error from deploy service at 127.0.0.1:19071 (Status "+strconv.Itoa(status)+"):\n"+errorMessage+"\n", + "Error: error from deploy service at 127.0.0.1:19071 (Status "+strconv.Itoa(status)+"):\n"+errorMessage+"\n", outErr) } diff --git a/client/go/cmd/document.go b/client/go/cmd/document.go index 84c384e701e..8dab813ec68 100644 --- a/client/go/cmd/document.go +++ b/client/go/cmd/document.go @@ -46,9 +46,14 @@ To feed with high throughput, https://docs.vespa.ai/en/vespa-feed-client.html should be used instead of this.`, Example: `$ vespa document src/test/resources/A-Head-Full-of-Dreams.json`, DisableAutoGenTag: true, + SilenceUsage: true, Args: cobra.ExactArgs(1), - Run: func(cmd *cobra.Command, args []string) { - printResult(vespa.Send(args[0], documentService(), operationOptions()), false) + RunE: func(cmd *cobra.Command, args []string) error { + service, err := documentService() + if err != nil { + return err + } + return printResult(vespa.Send(args[0], service, operationOptions()), false) }, } @@ -62,11 +67,16 @@ If the document id is specified both as an argument and in the file the argument Example: `$ vespa document put src/test/resources/A-Head-Full-of-Dreams.json $ vespa document put id:mynamespace:music::a-head-full-of-dreams src/test/resources/A-Head-Full-of-Dreams.json`, DisableAutoGenTag: true, - Run: func(cmd *cobra.Command, args []string) { + SilenceUsage: true, + RunE: func(cmd *cobra.Command, args []string) error { + service, err := documentService() + if err != nil { + return err + } if len(args) == 1 { - printResult(vespa.Put("", args[0], documentService(), operationOptions()), false) + return printResult(vespa.Put("", args[0], service, operationOptions()), false) } else { - printResult(vespa.Put(args[0], args[1], documentService(), operationOptions()), false) + return printResult(vespa.Put(args[0], args[1], service, operationOptions()), false) } }, } @@ -80,11 +90,16 @@ If the document id is specified both as an argument and in the file the argument Example: `$ vespa document update src/test/resources/A-Head-Full-of-Dreams-Update.json $ vespa document update id:mynamespace:music::a-head-full-of-dreams src/test/resources/A-Head-Full-of-Dreams.json`, DisableAutoGenTag: true, - Run: func(cmd *cobra.Command, args []string) { + SilenceUsage: true, + RunE: func(cmd *cobra.Command, args []string) error { + service, err := documentService() + if err != nil { + return err + } if len(args) == 1 { - printResult(vespa.Update("", args[0], documentService(), operationOptions()), false) + return printResult(vespa.Update("", args[0], service, operationOptions()), false) } else { - printResult(vespa.Update(args[0], args[1], documentService(), operationOptions()), false) + return printResult(vespa.Update(args[0], args[1], service, operationOptions()), false) } }, } @@ -98,11 +113,16 @@ If the document id is specified both as an argument and in the file the argument Example: `$ vespa document remove src/test/resources/A-Head-Full-of-Dreams-Remove.json $ vespa document remove id:mynamespace:music::a-head-full-of-dreams`, DisableAutoGenTag: true, - Run: func(cmd *cobra.Command, args []string) { + SilenceUsage: true, + RunE: func(cmd *cobra.Command, args []string) error { + service, err := documentService() + if err != nil { + return err + } if strings.HasPrefix(args[0], "id:") { - printResult(vespa.RemoveId(args[0], documentService(), operationOptions()), false) + return printResult(vespa.RemoveId(args[0], service, operationOptions()), false) } else { - printResult(vespa.RemoveOperation(args[0], documentService(), operationOptions()), false) + return printResult(vespa.RemoveOperation(args[0], service, operationOptions()), false) } }, } @@ -112,13 +132,18 @@ var documentGetCmd = &cobra.Command{ Short: "Gets a document", Args: cobra.ExactArgs(1), DisableAutoGenTag: true, + SilenceUsage: true, Example: `$ vespa document get id:mynamespace:music::a-head-full-of-dreams`, - Run: func(cmd *cobra.Command, args []string) { - printResult(vespa.Get(args[0], documentService(), operationOptions()), true) + RunE: func(cmd *cobra.Command, args []string) error { + service, err := documentService() + if err != nil { + return err + } + return printResult(vespa.Get(args[0], service, operationOptions()), true) }, } -func documentService() *vespa.Service { return getService("document", 0, "") } +func documentService() (*vespa.Service, error) { return getService("document", 0, "") } func operationOptions() vespa.OperationOptions { return vespa.OperationOptions{ @@ -134,7 +159,7 @@ func curlOutput() io.Writer { return ioutil.Discard } -func printResult(result util.OperationResult, payloadOnlyOnSuccess bool) { +func printResult(result util.OperationResult, payloadOnlyOnSuccess bool) error { out := stdout if !result.Success { out = stderr @@ -158,6 +183,9 @@ func printResult(result util.OperationResult, payloadOnlyOnSuccess bool) { } if !result.Success { - exitFunc(1) + err := errHint(fmt.Errorf("document operation failed")) + err.quiet = true + return err } + return nil } diff --git a/client/go/cmd/document_test.go b/client/go/cmd/document_test.go index f3a5fbe9543..2b596e9893b 100644 --- a/client/go/cmd/document_test.go +++ b/client/go/cmd/document_test.go @@ -97,7 +97,10 @@ func TestDocumentGet(t *testing.T) { func assertDocumentSend(arguments []string, expectedOperation string, expectedMethod string, expectedDocumentId string, expectedPayloadFile string, t *testing.T) { client := &mockHttpClient{} - documentURL := documentServiceURL(client) + documentURL, err := documentServiceURL(client) + if err != nil { + t.Fatal(err) + } expectedPath, _ := vespa.IdToURLPath(expectedDocumentId) expectedURL := documentURL + "/document/v1/" + expectedPath out, errOut := execute(command{args: arguments}, t, client) @@ -123,7 +126,10 @@ func assertDocumentSend(arguments []string, expectedOperation string, expectedMe func assertDocumentGet(arguments []string, documentId string, t *testing.T) { client := &mockHttpClient{} - documentURL := documentServiceURL(client) + documentURL, err := documentServiceURL(client) + if err != nil { + t.Fatal(err) + } client.NextResponse(200, "{\"fields\":{\"foo\":\"bar\"}}") assert.Equal(t, `{ @@ -160,6 +166,10 @@ func assertDocumentServerError(t *testing.T, status int, errorMessage string) { outErr) } -func documentServiceURL(client *mockHttpClient) string { - return getService("document", 0, "").BaseURL +func documentServiceURL(client *mockHttpClient) (string, error) { + service, err := getService("document", 0, "") + if err != nil { + return "", err + } + return service.BaseURL, nil } diff --git a/client/go/cmd/helpers.go b/client/go/cmd/helpers.go index b3a25c577e8..03cdaecbfce 100644 --- a/client/go/cmd/helpers.go +++ b/client/go/cmd/helpers.go @@ -16,34 +16,15 @@ import ( "github.com/vespa-engine/vespa/client/go/vespa" ) -var exitFunc = os.Exit // To allow overriding Exit in tests - -func fatalErrHint(err error, hints ...string) { - printErrHint(err, hints...) - exitFunc(1) -} - -func fatalErr(err error, msg ...interface{}) { - printErr(err, msg...) - exitFunc(1) -} - func printErrHint(err error, hints ...string) { - if err != nil { - printErr(nil, err.Error()) - } + printErr(err) for _, hint := range hints { fmt.Fprintln(stderr, color.Cyan("Hint:"), hint) } } -func printErr(err error, msg ...interface{}) { - if len(msg) > 0 { - fmt.Fprintln(stderr, color.Red("Error:"), fmt.Sprint(msg...)) - } - if err != nil { - fmt.Fprintln(stderr, color.Yellow(err)) - } +func printErr(err error) { + fmt.Fprintln(stderr, color.Red("Error:"), err) } func printSuccess(msg ...interface{}) { @@ -80,13 +61,16 @@ func vespaCliCacheDir() (string, error) { return cacheDir, nil } -func deploymentFromArgs() vespa.Deployment { +func deploymentFromArgs() (vespa.Deployment, error) { zone, err := vespa.ZoneFromString(zoneArg) if err != nil { - fatalErrHint(err, "Zone format is <env>.<region>") + return vespa.Deployment{}, err } - app := getApplication() - return vespa.Deployment{Application: app, Zone: zone} + app, err := getApplication() + if err != nil { + return vespa.Deployment{}, err + } + return vespa.Deployment{Application: app, Zone: zone}, nil } func applicationSource(args []string) string { @@ -96,49 +80,48 @@ func applicationSource(args []string) string { return "." } -func getApplication() vespa.ApplicationID { +func getApplication() (vespa.ApplicationID, error) { cfg, err := LoadConfig() if err != nil { - fatalErr(err, "Could not load config") - return vespa.ApplicationID{} + return vespa.ApplicationID{}, err } app, err := cfg.Get(applicationFlag) if err != nil { - fatalErrHint(err, "No application specified. Try the --"+applicationFlag+" flag") - return vespa.ApplicationID{} + return vespa.ApplicationID{}, errHint(fmt.Errorf("no application specified: %w", err), "Try the --"+applicationFlag+" flag") } application, err := vespa.ApplicationFromString(app) if err != nil { - fatalErrHint(err, "Application format is <tenant>.<app>.<instance>") - return vespa.ApplicationID{} + return vespa.ApplicationID{}, errHint(err, "application format is <tenant>.<app>.<instance>") } - return application + return application, nil } -func getTargetType() string { +func getTargetType() (string, error) { cfg, err := LoadConfig() if err != nil { - fatalErr(err, "Could not load config") - return "" + return "", err } target, err := cfg.Get(targetFlag) if err != nil { - fatalErr(err, "A valid target must be specified") + return "", fmt.Errorf("invalid target: %w", err) } - return target + return target, nil } -func getService(service string, sessionOrRunID int64, cluster string) *vespa.Service { - t := getTarget() +func getService(service string, sessionOrRunID int64, cluster string) (*vespa.Service, error) { + t, err := getTarget() + if err != nil { + return nil, err + } timeout := time.Duration(waitSecsArg) * time.Second if timeout > 0 { log.Printf("Waiting up to %d %s for %s service to become available ...", color.Cyan(waitSecsArg), color.Cyan("seconds"), color.Cyan(service)) } s, err := t.Service(service, timeout, sessionOrRunID, cluster) if err != nil { - fatalErr(err, "Service not found: ", service) + return nil, fmt.Errorf("service %s not found: %w", service, err) } - return s + return s, nil } func getEndpointsOverride() string { return os.Getenv("VESPA_CLI_ENDPOINTS") } @@ -167,39 +150,47 @@ func getApiURL() string { return "https://api.vespa-external.aws.oath.cloud:4443" } -func getTarget() vespa.Target { - targetType := getTargetType() +func getTarget() (vespa.Target, error) { + targetType, err := getTargetType() + if err != nil { + return nil, err + } if strings.HasPrefix(targetType, "http") { - return vespa.CustomTarget(targetType) + return vespa.CustomTarget(targetType), nil } switch targetType { case "local": - return vespa.LocalTarget() + return vespa.LocalTarget(), nil case "cloud": cfg, err := LoadConfig() if err != nil { - fatalErr(err, "Could not load config") - return nil + return nil, err + } + deployment, err := deploymentFromArgs() + if err != nil { + return nil, err + } + endpoints, err := getEndpointsFromEnv() + if err != nil { + return nil, err } - deployment := deploymentFromArgs() - endpoints := getEndpointsFromEnv() var apiKey []byte = nil apiKey, err = cfg.ReadAPIKey(deployment.Application.Tenant) if !vespa.Auth0AccessTokenEnabled() && endpoints == nil { if err != nil { - fatalErrHint(err, "Deployment to cloud requires an API key. Try 'vespa api-key'") + return nil, errHint(err, "Deployment to cloud requires an API key. Try 'vespa api-key'") } } kp, err := cfg.X509KeyPair(deployment.Application) if err != nil { - var msg string + var hint string if vespa.Auth0AccessTokenEnabled() { - msg = "Deployment to cloud requires a certificate. Try 'vespa auth cert'" + hint = "Deployment to cloud requires a certificate. Try 'vespa auth cert'" } else { - msg = "Deployment to cloud requires a certificate. Try 'vespa cert'" + hint = "Deployment to cloud requires a certificate. Try 'vespa cert'" } - fatalErrHint(err, msg) + return nil, errHint(err, hint) } var cloudAuth string if vespa.Auth0AccessTokenEnabled() { @@ -215,7 +206,10 @@ func getTarget() vespa.Target { cloudAuth = "" } - return vespa.CloudTarget(getApiURL(), deployment, apiKey, + return vespa.CloudTarget( + getApiURL(), + deployment, + apiKey, vespa.TLSOptions{ KeyPair: kp.KeyPair, CertificateFile: kp.CertificateFile, @@ -228,14 +222,17 @@ func getTarget() vespa.Target { cfg.AuthConfigPath(), getSystemName(), cloudAuth, - endpoints) + endpoints, + ), nil } - fatalErrHint(fmt.Errorf("Invalid target: %s", targetType), "Valid targets are 'local', 'cloud' or an URL") - return nil + return nil, errHint(fmt.Errorf("invalid target: %s", targetType), "Valid targets are 'local', 'cloud' or an URL") } -func waitForService(service string, sessionOrRunID int64) { - s := getService(service, sessionOrRunID, "") +func waitForService(service string, sessionOrRunID int64) error { + s, err := getService(service, sessionOrRunID, "") + if err != nil { + return err + } timeout := time.Duration(waitSecsArg) * time.Second if timeout > 0 { log.Printf("Waiting up to %d %s for service to become ready ...", color.Cyan(waitSecsArg), color.Cyan("seconds")) @@ -245,57 +242,58 @@ func waitForService(service string, sessionOrRunID int64) { log.Print(s.Description(), " at ", color.Cyan(s.BaseURL), " is ", color.Green("ready")) } else { if err == nil { - err = fmt.Errorf("Status %d", status) + err = fmt.Errorf("status %d", status) } - fatalErr(err, s.Description(), " at ", color.Cyan(s.BaseURL), " is ", color.Red("not ready")) + return fmt.Errorf("%s at %s is %s: %w", s.Description(), color.Cyan(s.BaseURL), color.Red("not ready"), err) } + return nil } -func getDeploymentOpts(cfg *Config, pkg vespa.ApplicationPackage, target vespa.Target) vespa.DeploymentOpts { +func getDeploymentOpts(cfg *Config, pkg vespa.ApplicationPackage, target vespa.Target) (vespa.DeploymentOpts, error) { opts := vespa.DeploymentOpts{ApplicationPackage: pkg, Target: target} if opts.IsCloud() { - deployment := deploymentFromArgs() + deployment, err := deploymentFromArgs() + if err != nil { + return vespa.DeploymentOpts{}, err + } if !opts.ApplicationPackage.HasCertificate() { - var msg string + var hint string if vespa.Auth0AccessTokenEnabled() { - msg = "Try 'vespa auth cert'" + hint = "Try 'vespa auth cert'" } else { - msg = "Try 'vespa cert'" + hint = "Try 'vespa cert'" } - fatalErrHint(fmt.Errorf("Missing certificate in application package"), "Applications in Vespa Cloud require a certificate", msg) - return opts + return vespa.DeploymentOpts{}, errHint(fmt.Errorf("missing certificate in application package"), "Applications in Vespa Cloud require a certificate", hint) } - var err error opts.APIKey, err = cfg.ReadAPIKey(deployment.Application.Tenant) if !vespa.Auth0AccessTokenEnabled() { if err != nil { - fatalErrHint(err, "Deployment to cloud requires an API key. Try 'vespa api-key'") - return opts + return vespa.DeploymentOpts{}, errHint(err, "Deployment to cloud requires an API key. Try 'vespa api-key'") } } opts.Deployment = deployment } - return opts + return opts, nil } -func getEndpointsFromEnv() map[string]string { +func getEndpointsFromEnv() (map[string]string, error) { endpointsString := getEndpointsOverride() if endpointsString == "" { - return nil + return nil, nil } var endpoints endpoints urlsByCluster := make(map[string]string) if err := json.Unmarshal([]byte(endpointsString), &endpoints); err != nil { - fatalErrHint(err, "Endpoints must be valid JSON") + return nil, fmt.Errorf("endpoints must be valid json: %w", err) } if len(endpoints.Endpoints) == 0 { - fatalErr(fmt.Errorf("endpoints must be non-empty")) + return nil, fmt.Errorf("endpoints must be non-empty") } for _, endpoint := range endpoints.Endpoints { urlsByCluster[endpoint.Cluster] = endpoint.URL } - return urlsByCluster + return urlsByCluster, nil } type endpoints struct { diff --git a/client/go/cmd/log.go b/client/go/cmd/log.go index 7d3f6e95cd8..d61eaecf35b 100644 --- a/client/go/cmd/log.go +++ b/client/go/cmd/log.go @@ -40,9 +40,13 @@ $ vespa log --nldequote=false 10m $ vespa log --from 2021-08-25T15:00:00Z --to 2021-08-26T02:00:00Z $ vespa log --follow`, DisableAutoGenTag: true, + SilenceUsage: true, Args: cobra.MaximumNArgs(1), - Run: func(cmd *cobra.Command, args []string) { - target := getTarget() + RunE: func(cmd *cobra.Command, args []string) error { + target, err := getTarget() + if err != nil { + return err + } options := vespa.LogOptions{ Level: vespa.LogLevel(levelArg), Follow: followArg, @@ -51,21 +55,21 @@ $ vespa log --follow`, } if options.Follow { if fromArg != "" || toArg != "" || len(args) > 0 { - fatalErr(fmt.Errorf("cannot combine --from/--to or relative time with --follow"), "Could not follow logs") + return fmt.Errorf("cannot combine --from/--to or relative time with --follow") } options.From = time.Now().Add(-5 * time.Minute) } else { from, to, err := parsePeriod(args) if err != nil { - fatalErr(err, "Invalid period") - return + return fmt.Errorf("invalid period: %w", err) } options.From = from options.To = to } if err := target.PrintLog(options); err != nil { - fatalErr(err, "Could not retrieve logs") + return fmt.Errorf("could not retrieve logs: %w", err) } + return nil }, } diff --git a/client/go/cmd/log_test.go b/client/go/cmd/log_test.go index f239bebc488..4b32927ca17 100644 --- a/client/go/cmd/log_test.go +++ b/client/go/cmd/log_test.go @@ -24,5 +24,5 @@ func TestLog(t *testing.T) { assert.Equal(t, expected, out) _, errOut := execute(command{homeDir: homeDir, args: []string{"log", "--from", "2021-09-27T13:12:49Z", "--to", "2021-09-27T13:15:00", "1h"}}, t, httpClient) - assert.Equal(t, "Error: Invalid period\ncannot combine --from/--to with relative value: 1h\n", errOut) + assert.Equal(t, "Error: invalid period: cannot combine --from/--to with relative value: 1h\n", errOut) } diff --git a/client/go/cmd/login.go b/client/go/cmd/login.go index 5011b290b9f..f4438cdbb24 100644 --- a/client/go/cmd/login.go +++ b/client/go/cmd/login.go @@ -12,6 +12,7 @@ var loginCmd = &cobra.Command{ Short: "Authenticate the Vespa CLI", Example: "$ vespa auth login", DisableAutoGenTag: true, + SilenceUsage: true, RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() cfg, err := LoadConfig() @@ -29,7 +30,7 @@ var loginCmd = &cobra.Command{ return err } if err := cfg.Write(); err != nil { - fatalErr(err) + return err } } } diff --git a/client/go/cmd/man.go b/client/go/cmd/man.go index d90898117de..01fffd38a32 100644 --- a/client/go/cmd/man.go +++ b/client/go/cmd/man.go @@ -2,6 +2,8 @@ package cmd import ( + "fmt" + "github.com/spf13/cobra" "github.com/spf13/cobra/doc" ) @@ -16,13 +18,14 @@ var manCmd = &cobra.Command{ Args: cobra.ExactArgs(1), Hidden: true, // Not intended to be called by users DisableAutoGenTag: true, - Run: func(cmd *cobra.Command, args []string) { + SilenceUsage: true, + RunE: func(cmd *cobra.Command, args []string) error { dir := args[0] err := doc.GenManTree(rootCmd, nil, dir) if err != nil { - fatalErr(err, "Failed to write man pages") - return + return fmt.Errorf("failed to write man pages: %w", err) } printSuccess("Man pages written to ", dir) + return nil }, } diff --git a/client/go/cmd/prod.go b/client/go/cmd/prod.go index 62cf659362b..89f401a356e 100644 --- a/client/go/cmd/prod.go +++ b/client/go/cmd/prod.go @@ -33,10 +33,10 @@ Configure and deploy your application package to production in Vespa Cloud.`, Example: `$ vespa prod init $ vespa prod submit`, DisableAutoGenTag: true, - Run: func(cmd *cobra.Command, args []string) { - // Root command does nothing - cmd.Help() - exitFunc(1) + SilenceUsage: false, + Args: cobra.MinimumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + return fmt.Errorf("invalid command: %s", args[0]) }, } @@ -53,28 +53,25 @@ Reference: https://cloud.vespa.ai/en/reference/services https://cloud.vespa.ai/en/reference/deployment`, DisableAutoGenTag: true, - Run: func(cmd *cobra.Command, args []string) { + SilenceUsage: true, + RunE: func(cmd *cobra.Command, args []string) error { appSource := applicationSource(args) pkg, err := vespa.FindApplicationPackage(appSource, false) if err != nil { - fatalErr(err) - return + return err } if pkg.IsZip() { - fatalErrHint(fmt.Errorf("Cannot modify compressed application package %s", pkg.Path), + return errHint(fmt.Errorf("cannot modify compressed application package %s", pkg.Path), "Try running 'mvn clean' and run this command again") - return } deploymentXML, err := readDeploymentXML(pkg) if err != nil { - fatalErr(err, "Could not read deployment.xml") - return + return fmt.Errorf("could not read deployment.xml: %w", err) } servicesXML, err := readServicesXML(pkg) if err != nil { - fatalErr(err, "A services.xml declaring your cluster(s) must exist") - return + return fmt.Errorf("a services.xml declaring your cluster(s) must exist: %w", err) } fmt.Fprint(stdout, "This will modify any existing ", color.Yellow("deployment.xml"), " and ", color.Yellow("services.xml"), @@ -83,18 +80,23 @@ https://cloud.vespa.ai/en/reference/deployment`, fmt.Fprint(stdout, "Abort the configuration at any time by pressing Ctrl-C. The\nfiles will remain untouched.\n\n") fmt.Fprint(stdout, "See this guide for sizing a Vespa deployment:\n", color.Green("https://docs.vespa.ai/en/performance/sizing-search.html\n\n")) r := bufio.NewReader(stdin) - deploymentXML = updateRegions(r, deploymentXML) - servicesXML = updateNodes(r, servicesXML) + deploymentXML, err = updateRegions(r, deploymentXML) + if err != nil { + return err + } + servicesXML, err = updateNodes(r, servicesXML) + if err != nil { + return err + } fmt.Fprintln(stdout) if err := writeWithBackup(pkg, "deployment.xml", deploymentXML.String()); err != nil { - fatalErr(err) - return + return err } if err := writeWithBackup(pkg, "services.xml", servicesXML.String()); err != nil { - fatalErr(err) - return + return err } + return nil }, } @@ -117,34 +119,33 @@ For more information about production deployments in Vespa Cloud see: https://cloud.vespa.ai/en/getting-to-production https://cloud.vespa.ai/en/automated-deployments`, DisableAutoGenTag: true, + SilenceUsage: true, Example: `$ mvn package # when adding custom Java components $ vespa prod submit`, - Run: func(cmd *cobra.Command, args []string) { - target := getTarget() + RunE: func(cmd *cobra.Command, args []string) error { + target, err := getTarget() + if err != nil { + return err + } if target.Type() != "cloud" { - fatalErr(fmt.Errorf("%s target cannot deploy to Vespa Cloud", target.Type())) - return + return fmt.Errorf("%s target cannot deploy to Vespa Cloud", target.Type()) } appSource := applicationSource(args) pkg, err := vespa.FindApplicationPackage(appSource, true) if err != nil { - fatalErr(err) - return + return err } cfg, err := LoadConfig() if err != nil { - fatalErr(err, "Could not load config") - return + return err } if !pkg.HasDeployment() { - fatalErrHint(fmt.Errorf("No deployment.xml found"), "Try creating one with vespa prod init") - return + return errHint(fmt.Errorf("no deployment.xml found"), "Try creating one with vespa prod init") } if pkg.TestPath == "" { - fatalErrHint(fmt.Errorf("No tests found"), + return errHint(fmt.Errorf("no tests found"), "The application must be a Java maven project, or include basic HTTP tests under src/test/application/", "See https://cloud.vespa.ai/en/getting-to-production") - return } // TODO: Always verify tests. Do it before packaging, when running Maven from this CLI. if !pkg.IsZip() { @@ -155,14 +156,18 @@ $ vespa prod submit`, fmt.Fprintln(stderr, color.Yellow("Warning:"), "We recommend doing this only from a CD job") printErrHint(nil, "See https://cloud.vespa.ai/en/getting-to-production") } - opts := getDeploymentOpts(cfg, pkg, target) + opts, err := getDeploymentOpts(cfg, pkg, target) + if err != nil { + return err + } if err := vespa.Submit(opts); err != nil { - fatalErr(err, "Could not submit application for deployment") + return fmt.Errorf("could not submit application for deployment: %w", err) } else { printSuccess("Submitted ", color.Cyan(pkg.Path), " for deployment") log.Printf("See %s for deployment progress\n", color.Cyan(fmt.Sprintf("%s/tenant/%s/application/%s/prod/deployment", getConsoleURL(), opts.Deployment.Application.Tenant, opts.Deployment.Application.Application))) } + return nil }, } @@ -197,24 +202,27 @@ func writeWithBackup(pkg vespa.ApplicationPackage, filename, contents string) er return ioutil.WriteFile(dst, []byte(contents), 0644) } -func updateRegions(r *bufio.Reader, deploymentXML xml.Deployment) xml.Deployment { - regions := promptRegions(r, deploymentXML) +func updateRegions(r *bufio.Reader, deploymentXML xml.Deployment) (xml.Deployment, error) { + regions, err := promptRegions(r, deploymentXML) + if err != nil { + return xml.Deployment{}, err + } parts := strings.Split(regions, ",") regionElements := xml.Regions(parts...) if err := deploymentXML.Replace("prod", "region", regionElements); err != nil { - fatalErr(err, "Could not update region elements in deployment.xml") + return xml.Deployment{}, fmt.Errorf("could not update region elements in deployment.xml: %w", err) } // TODO: Some sample apps come with production <test> elements, but not necessarily working production tests, we // therefore remove <test> elements here. // This can be improved by supporting <test> elements in xml package and allow specifying testing as part of // region prompt, e.g. region1;test,region2 if err := deploymentXML.Replace("prod", "test", nil); err != nil { - fatalErr(err, "Could not remove test elements in deployment.xml") + return xml.Deployment{}, fmt.Errorf("could not remove test elements in deployment.xml: %w", err) } - return deploymentXML + return deploymentXML, nil } -func promptRegions(r *bufio.Reader, deploymentXML xml.Deployment) string { +func promptRegions(r *bufio.Reader, deploymentXML xml.Deployment) (string, error) { fmt.Fprintln(stdout, color.Cyan("> Deployment regions")) fmt.Fprintf(stdout, "Documentation: %s\n", color.Green("https://cloud.vespa.ai/en/reference/zones")) fmt.Fprintf(stdout, "Example: %s\n\n", color.Yellow("aws-us-east-1c,aws-us-west-2a")) @@ -239,47 +247,56 @@ func promptRegions(r *bufio.Reader, deploymentXML xml.Deployment) string { return prompt(r, "Which regions do you wish to deploy in?", strings.Join(currentRegions, ","), validator) } -func updateNodes(r *bufio.Reader, servicesXML xml.Services) xml.Services { +func updateNodes(r *bufio.Reader, servicesXML xml.Services) (xml.Services, error) { for _, c := range servicesXML.Container { - nodes := promptNodes(r, c.ID, c.Nodes) + nodes, err := promptNodes(r, c.ID, c.Nodes) + if err != nil { + return xml.Services{}, err + } if err := servicesXML.Replace("container#"+c.ID, "nodes", nodes); err != nil { - fatalErr(err) - return xml.Services{} + return xml.Services{}, err } } for _, c := range servicesXML.Content { - nodes := promptNodes(r, c.ID, c.Nodes) + nodes, err := promptNodes(r, c.ID, c.Nodes) + if err != nil { + return xml.Services{}, err + } if err := servicesXML.Replace("content#"+c.ID, "nodes", nodes); err != nil { - fatalErr(err) - return xml.Services{} + return xml.Services{}, err } } - return servicesXML + return servicesXML, nil } -func promptNodes(r *bufio.Reader, clusterID string, defaultValue xml.Nodes) xml.Nodes { - count := promptNodeCount(r, clusterID, defaultValue.Count) +func promptNodes(r *bufio.Reader, clusterID string, defaultValue xml.Nodes) (xml.Nodes, error) { + count, err := promptNodeCount(r, clusterID, defaultValue.Count) + if err != nil { + return xml.Nodes{}, err + } const autoSpec = "auto" defaultSpec := autoSpec resources := defaultValue.Resources if resources != nil { defaultSpec = defaultValue.Resources.String() } - spec := promptResources(r, clusterID, defaultSpec) + spec, err := promptResources(r, clusterID, defaultSpec) + if err != nil { + return xml.Nodes{}, err + } if spec == autoSpec { resources = nil } else { r, err := xml.ParseResources(spec) if err != nil { - fatalErr(err) // Should not happen as resources have already been validated - return xml.Nodes{} + return xml.Nodes{}, err // Should not happen as resources have already been validated } resources = &r } - return xml.Nodes{Count: count, Resources: resources} + return xml.Nodes{Count: count, Resources: resources}, nil } -func promptNodeCount(r *bufio.Reader, clusterID string, nodeCount string) string { +func promptNodeCount(r *bufio.Reader, clusterID string, nodeCount string) (string, error) { fmt.Fprintln(stdout, color.Cyan("\n> Node count: "+clusterID+" cluster")) fmt.Fprintf(stdout, "Documentation: %s\n", color.Green("https://cloud.vespa.ai/en/reference/services")) fmt.Fprintf(stdout, "Example: %s\nExample: %s\n\n", color.Yellow("4"), color.Yellow("[2,8]")) @@ -290,7 +307,7 @@ func promptNodeCount(r *bufio.Reader, clusterID string, nodeCount string) string return prompt(r, fmt.Sprintf("How many nodes should the %s cluster have?", color.Cyan(clusterID)), nodeCount, validator) } -func promptResources(r *bufio.Reader, clusterID string, resources string) string { +func promptResources(r *bufio.Reader, clusterID string, resources string) (string, error) { fmt.Fprintln(stdout, color.Cyan("\n> Node resources: "+clusterID+" cluster")) fmt.Fprintf(stdout, "Documentation: %s\n", color.Green("https://cloud.vespa.ai/en/reference/services")) fmt.Fprintf(stdout, "Example: %s\nExample: %s\n\n", color.Yellow("auto"), color.Yellow("vcpu=4,memory=8Gb,disk=100Gb")) @@ -325,7 +342,7 @@ func readServicesXML(pkg vespa.ApplicationPackage) (xml.Services, error) { return xml.ReadServices(f) } -func prompt(r *bufio.Reader, question, defaultAnswer string, validator func(input string) error) string { +func prompt(r *bufio.Reader, question, defaultAnswer string, validator func(input string) error) (string, error) { var input string for input == "" { fmt.Fprint(stdout, question) @@ -337,8 +354,7 @@ func prompt(r *bufio.Reader, question, defaultAnswer string, validator func(inpu var err error input, err = r.ReadString('\n') if err != nil { - fatalErr(err) - return "" + return "", err } input = strings.TrimSpace(input) if input == "" { @@ -351,7 +367,7 @@ func prompt(r *bufio.Reader, question, defaultAnswer string, validator func(inpu input = "" } } - return input + return input, nil } func verifyTests(testsParent string, target vespa.Target) { @@ -361,20 +377,20 @@ func verifyTests(testsParent string, target vespa.Target) { verifyTest(testsParent, "production-test", target, false) } -func verifyTest(testsParent string, suite string, target vespa.Target, required bool) { +func verifyTest(testsParent string, suite string, target vespa.Target, required bool) error { testDirectory := filepath.Join(testsParent, "tests", suite) _, err := os.Stat(testDirectory) if err != nil { if required { if errors.Is(err, os.ErrNotExist) { - fatalErrHint(fmt.Errorf("No %s tests found", suite), + return errHint(fmt.Errorf("no %s tests found: %w", suite, err), fmt.Sprintf("No such directory: %s", testDirectory), "See https://cloud.vespa.ai/en/reference/testing") } - fatalErrHint(err, "See https://cloud.vespa.ai/en/reference/testing") + return errHint(err, "See https://cloud.vespa.ai/en/reference/testing") } - return + return nil } - - runTests(testDirectory, true) + _, _, err = runTests(testDirectory, true) + return err } diff --git a/client/go/cmd/query.go b/client/go/cmd/query.go index 86e99e24052..8ee022d7061 100644 --- a/client/go/cmd/query.go +++ b/client/go/cmd/query.go @@ -33,14 +33,18 @@ Any parameter from https://docs.vespa.ai/en/reference/query-api-reference.html can be set by the syntax [parameter-name]=[value].`, // TODO: Support referencing a query json file DisableAutoGenTag: true, + SilenceUsage: true, Args: cobra.MinimumNArgs(1), - Run: func(cmd *cobra.Command, args []string) { - query(args) + RunE: func(cmd *cobra.Command, args []string) error { + return query(args) }, } -func query(arguments []string) { - service := getService("query", 0, "") +func query(arguments []string) error { + service, err := getService("query", 0, "") + if err != nil { + return err + } url, _ := url.Parse(service.BaseURL + "/search/") urlQuery := url.Query() for i := 0; i < len(arguments); i++ { @@ -56,23 +60,22 @@ func query(arguments []string) { url.RawQuery = urlQuery.Encode() deadline, err := time.ParseDuration(queryTimeout) if err != nil { - fatalErr(err, "Invalid query timeout") - return + return fmt.Errorf("invalid query timeout: %w", err) } response, err := service.Do(&http.Request{URL: url}, deadline+time.Second) // Slightly longer than query timeout if err != nil { - fatalErr(nil, "Request failed: ", err) - return + return fmt.Errorf("request failed: %w", err) } defer response.Body.Close() if response.StatusCode == 200 { log.Print(util.ReaderToJSON(response.Body)) } else if response.StatusCode/100 == 4 { - fatalErr(nil, "Invalid query: ", response.Status, "\n", util.ReaderToJSON(response.Body)) + return fmt.Errorf("invalid query: %s\n%s", response.Status, util.ReaderToJSON(response.Body)) } else { - fatalErr(nil, response.Status, " from container at ", color.Cyan(url.Host), "\n", util.ReaderToJSON(response.Body)) + return fmt.Errorf("%s from container at %s\n%s", response.Status, color.Cyan(url.Host), util.ReaderToJSON(response.Body)) } + return nil } func splitArg(argument string) (string, string) { diff --git a/client/go/cmd/query_test.go b/client/go/cmd/query_test.go index 57c309c8cb3..09b1afb2b84 100644 --- a/client/go/cmd/query_test.go +++ b/client/go/cmd/query_test.go @@ -50,7 +50,10 @@ func assertQuery(t *testing.T, expectedQuery string, query ...string) { "{\n \"query\": \"result\"\n}\n", executeCommand(t, client, []string{"query"}, query), "query output") - queryURL := queryServiceURL(client) + queryURL, err := queryServiceURL(client) + if err != nil { + t.Fatal(err) + } assert.Equal(t, queryURL+"/search/"+expectedQuery, client.lastRequest.URL.String()) } @@ -59,7 +62,7 @@ func assertQueryError(t *testing.T, status int, errorMessage string) { client.NextResponse(status, errorMessage) _, outErr := execute(command{args: []string{"query", "yql=select from sources * where title contains 'foo'"}}, t, client) assert.Equal(t, - "Error: Invalid query: Status "+strconv.Itoa(status)+"\n"+errorMessage+"\n", + "Error: invalid query: Status "+strconv.Itoa(status)+"\n"+errorMessage+"\n", outErr, "error output") } @@ -74,6 +77,10 @@ func assertQueryServiceError(t *testing.T, status int, errorMessage string) { "error output") } -func queryServiceURL(client *mockHttpClient) string { - return getService("query", 0, "").BaseURL +func queryServiceURL(client *mockHttpClient) (string, error) { + service, err := getService("query", 0, "") + if err != nil { + return "", err + } + return service.BaseURL, nil } diff --git a/client/go/cmd/root.go b/client/go/cmd/root.go index 7b69d544ac8..e25c5cb2d0d 100644 --- a/client/go/cmd/root.go +++ b/client/go/cmd/root.go @@ -17,6 +17,15 @@ import ( "github.com/spf13/cobra" ) +// ErrCLI is an error returned to the user. It wraps an exit status, a regular error and optional hints for resolving +// the error. +type ErrCLI struct { + Status int + quiet bool + hints []string + error +} + var ( rootCmd = &cobra.Command{ Use: "vespa command-name", @@ -28,14 +37,14 @@ Prefer web service API's to this in production. Vespa documentation: https://docs.vespa.ai`, DisableAutoGenTag: true, - PersistentPreRun: func(cmd *cobra.Command, args []string) { - configureOutput() + SilenceErrors: true, // We have our own error printing + SilenceUsage: false, + PersistentPreRunE: func(cmd *cobra.Command, args []string) error { + return configureOutput() }, Args: cobra.MinimumNArgs(1), - Run: func(cmd *cobra.Command, args []string) { - // Root command does nothing - cmd.Help() - exitFunc(1) + RunE: func(cmd *cobra.Command, args []string) error { + return fmt.Errorf("invalid command: %s", args[0]) }, } @@ -70,7 +79,7 @@ func isTerminal() bool { return false } -func configureOutput() { +func configureOutput() error { if quietArg { stdout = ioutil.Discard } @@ -79,11 +88,11 @@ func configureOutput() { config, err := LoadConfig() if err != nil { - fatalErr(err, "Could not load config") + return err } colorValue, err := config.Get(colorFlag) if err != nil { - fatalErr(err) + return err } colorize := false @@ -94,9 +103,10 @@ func configureOutput() { colorize = true case "never": default: - fatalErrHint(fmt.Errorf("Invalid value for %s option", colorFlag), "Must be \"auto\", \"never\" or \"always\"") + return errHint(fmt.Errorf("invalid value for %s option", colorFlag), "Must be \"auto\", \"never\" or \"always\"") } color = aurora.NewAurora(colorize) + return nil } func init() { @@ -112,5 +122,20 @@ func init() { bindFlagToConfig(quietFlag, rootCmd) } -// Execute executes the root command. -func Execute() error { return rootCmd.Execute() } +// errHint creates a new CLI error, with optional hints that will be printed after the error +func errHint(err error, hints ...string) ErrCLI { return ErrCLI{Status: 1, hints: hints, error: err} } + +// Execute executes command and prints any errors. +func Execute() error { + err := rootCmd.Execute() + if err != nil { + if cliErr, ok := err.(ErrCLI); ok { + if !cliErr.quiet { + printErrHint(cliErr, cliErr.hints...) + } + } else { + printErr(err) + } + } + return err +} diff --git a/client/go/cmd/status.go b/client/go/cmd/status.go index c72df481547..711dba4aa9d 100644 --- a/client/go/cmd/status.go +++ b/client/go/cmd/status.go @@ -20,9 +20,10 @@ var statusCmd = &cobra.Command{ Short: "Verify that a service is ready to use (query by default)", Example: `$ vespa status query`, DisableAutoGenTag: true, + SilenceUsage: true, Args: cobra.MaximumNArgs(1), - Run: func(cmd *cobra.Command, args []string) { - waitForService("query", 0) + RunE: func(cmd *cobra.Command, args []string) error { + return waitForService("query", 0) }, } @@ -31,9 +32,10 @@ var statusQueryCmd = &cobra.Command{ Short: "Verify that the query service is ready to use (default)", Example: `$ vespa status query`, DisableAutoGenTag: true, + SilenceUsage: true, Args: cobra.ExactArgs(0), - Run: func(cmd *cobra.Command, args []string) { - waitForService("query", 0) + RunE: func(cmd *cobra.Command, args []string) error { + return waitForService("query", 0) }, } @@ -42,9 +44,10 @@ var statusDocumentCmd = &cobra.Command{ Short: "Verify that the document service is ready to use", Example: `$ vespa status document`, DisableAutoGenTag: true, + SilenceUsage: true, Args: cobra.ExactArgs(0), - Run: func(cmd *cobra.Command, args []string) { - waitForService("document", 0) + RunE: func(cmd *cobra.Command, args []string) error { + return waitForService("document", 0) }, } @@ -53,8 +56,9 @@ var statusDeployCmd = &cobra.Command{ Short: "Verify that the deploy service is ready to use", Example: `$ vespa status deploy`, DisableAutoGenTag: true, + SilenceUsage: true, Args: cobra.ExactArgs(0), - Run: func(cmd *cobra.Command, args []string) { - waitForService("deploy", 0) + RunE: func(cmd *cobra.Command, args []string) error { + return waitForService("deploy", 0) }, } diff --git a/client/go/cmd/status_test.go b/client/go/cmd/status_test.go index 757ef5f3b06..631aa511459 100644 --- a/client/go/cmd/status_test.go +++ b/client/go/cmd/status_test.go @@ -82,7 +82,7 @@ func assertQueryStatusError(target string, args []string, t *testing.T) { cmd = append(cmd, args...) _, outErr := execute(command{args: cmd}, t, client) assert.Equal(t, - "Error: Container (query API) at "+target+" is not ready\nStatus 500\n", + "Error: Container (query API) at "+target+" is not ready: status 500\n", outErr, "vespa status container") } diff --git a/client/go/cmd/test.go b/client/go/cmd/test.go index 262b57eff33..179a8043a2a 100644 --- a/client/go/cmd/test.go +++ b/client/go/cmd/test.go @@ -9,9 +9,6 @@ import ( "crypto/tls" "encoding/json" "fmt" - "github.com/spf13/cobra" - "github.com/vespa-engine/vespa/client/go/util" - "github.com/vespa-engine/vespa/client/go/vespa" "io/ioutil" "math" "net/http" @@ -20,6 +17,10 @@ import ( "path/filepath" "strings" "time" + + "github.com/spf13/cobra" + "github.com/vespa-engine/vespa/client/go/util" + "github.com/vespa-engine/vespa/client/go/vespa" ) func init() { @@ -39,8 +40,13 @@ See https://cloud.vespa.ai/en/reference/testing.html for details.`, $ vespa test src/test/application/tests/system-test/feed-and-query.json`, Args: cobra.ExactArgs(1), DisableAutoGenTag: true, - Run: func(cmd *cobra.Command, args []string) { - if count, failed := runTests(args[0], false); len(failed) != 0 { + SilenceUsage: true, + RunE: func(cmd *cobra.Command, args []string) error { + count, failed, err := runTests(args[0], false) + if err != nil { + return err + } + if len(failed) != 0 { plural := "s" if count == 1 { plural = "" @@ -49,26 +55,27 @@ $ vespa test src/test/application/tests/system-test/feed-and-query.json`, for _, test := range failed { fmt.Fprintln(stdout, test) } - exitFunc(3) + return ErrCLI{Status: 3, error: fmt.Errorf("tests failed"), quiet: true} } else { plural := "s" if count == 1 { plural = "" } fmt.Fprintf(stdout, "\n%s %d test%s OK\n", color.Green("Success:"), count, plural) + return nil } }, } -func runTests(rootPath string, dryRun bool) (int, []string) { +func runTests(rootPath string, dryRun bool) (int, []string, error) { count := 0 failed := make([]string, 0) if stat, err := os.Stat(rootPath); err != nil { - fatalErrHint(err, "See https://cloud.vespa.ai/en/reference/testing") + return 0, nil, errHint(err, "See https://cloud.vespa.ai/en/reference/testing") } else if stat.IsDir() { tests, err := ioutil.ReadDir(rootPath) // TODO: Use os.ReadDir when >= 1.16 is required. if err != nil { - fatalErrHint(err, "See https://cloud.vespa.ai/en/reference/testing") + return 0, nil, errHint(err, "See https://cloud.vespa.ai/en/reference/testing") } context := testContext{testsPath: rootPath, dryRun: dryRun} previousFailed := false @@ -79,7 +86,10 @@ func runTests(rootPath string, dryRun bool) (int, []string) { fmt.Fprintln(stdout, "") previousFailed = false } - failure := runTest(testPath, context) + failure, err := runTest(testPath, context) + if err != nil { + return 0, nil, err + } if failure != "" { failed = append(failed, failure) previousFailed = true @@ -88,27 +98,30 @@ func runTests(rootPath string, dryRun bool) (int, []string) { } } } else if strings.HasSuffix(stat.Name(), ".json") { - failure := runTest(rootPath, testContext{testsPath: filepath.Dir(rootPath), dryRun: dryRun}) + failure, err := runTest(rootPath, testContext{testsPath: filepath.Dir(rootPath), dryRun: dryRun}) + if err != nil { + return 0, nil, err + } if failure != "" { failed = append(failed, failure) } count++ } if count == 0 { - fatalErrHint(fmt.Errorf("Failed to find any tests at %s", rootPath), "See https://cloud.vespa.ai/en/reference/testing") + return 0, nil, errHint(fmt.Errorf("failed to find any tests at %s", rootPath), "See https://cloud.vespa.ai/en/reference/testing") } - return count, failed + return count, failed, nil } // Runs the test at the given path, and returns the specified test name if the test fails -func runTest(testPath string, context testContext) string { +func runTest(testPath string, context testContext) (string, error) { var test test testBytes, err := ioutil.ReadFile(testPath) if err != nil { - fatalErrHint(err, "See https://cloud.vespa.ai/en/reference/testing") + return "", errHint(err, "See https://cloud.vespa.ai/en/reference/testing") } if err = json.Unmarshal(testBytes, &test); err != nil { - fatalErrHint(err, fmt.Sprintf("Failed parsing test at %s", testPath), "See https://cloud.vespa.ai/en/reference/testing") + return "", errHint(fmt.Errorf("failed parsing test at %s: %w", testPath, err), "See https://cloud.vespa.ai/en/reference/testing") } testName := test.Name @@ -122,12 +135,12 @@ func runTest(testPath string, context testContext) string { defaultParameters, err := getParameters(test.Defaults.ParametersRaw, filepath.Dir(testPath)) if err != nil { fmt.Fprintln(stderr) - fatalErrHint(err, fmt.Sprintf("Invalid default parameters for %s", testName), "See https://cloud.vespa.ai/en/reference/testing") + return "", errHint(fmt.Errorf("invalid default parameters for %s: %w", testName, err), "See https://cloud.vespa.ai/en/reference/testing") } if len(test.Steps) == 0 { fmt.Fprintln(stderr) - fatalErrHint(fmt.Errorf("a test must have at least one step, but none were found in %s", testPath), "See https://cloud.vespa.ai/en/reference/testing") + return "", errHint(fmt.Errorf("a test must have at least one step, but none were found in %s", testPath), "See https://cloud.vespa.ai/en/reference/testing") } for i, step := range test.Steps { stepName := fmt.Sprintf("Step %d", i+1) @@ -137,12 +150,12 @@ func runTest(testPath string, context testContext) string { failure, longFailure, err := verify(step, test.Defaults.Cluster, defaultParameters, context) if err != nil { fmt.Fprintln(stderr) - fatalErrHint(err, fmt.Sprintf("Error in %s", stepName), "See https://cloud.vespa.ai/en/reference/testing") + return "", errHint(fmt.Errorf("error in %s: %w", stepName, err), "See https://cloud.vespa.ai/en/reference/testing") } if !context.dryRun { if failure != "" { fmt.Fprintf(stdout, " %s\n%s:\n%s\n", color.Red("failed"), stepName, longFailure) - return fmt.Sprintf("%s: %s: %s", testName, stepName, failure) + return fmt.Sprintf("%s: %s: %s", testName, stepName, failure), nil } if i == 0 { fmt.Fprintf(stdout, " ") @@ -153,7 +166,7 @@ func runTest(testPath string, context testContext) string { if !context.dryRun { fmt.Fprintln(stdout, color.Green(" OK")) } - return "" + return "", nil } // Asserts specified response is obtained for request, or returns a failure message, or an error if this fails @@ -194,7 +207,11 @@ func verify(step step, defaultCluster string, defaultParameters map[string]strin } externalEndpoint := requestUrl.IsAbs() if !externalEndpoint && !context.dryRun { - service, err = context.target().Service("query", 0, 0, cluster) + target, err := context.target() + if err != nil { + return "", "", err + } + service, err = target.Service("query", 0, 0, cluster) if err != nil { return "", "", err } @@ -453,9 +470,13 @@ type testContext struct { dryRun bool } -func (t *testContext) target() vespa.Target { +func (t *testContext) target() (vespa.Target, error) { if t.lazyTarget == nil { - t.lazyTarget = getTarget() + target, err := getTarget() + if err != nil { + return nil, err + } + t.lazyTarget = target } - return t.lazyTarget + return t.lazyTarget, nil } diff --git a/client/go/cmd/test_test.go b/client/go/cmd/test_test.go index 490527df224..2c59bbbc030 100644 --- a/client/go/cmd/test_test.go +++ b/client/go/cmd/test_test.go @@ -50,7 +50,7 @@ func TestIllegalFileReference(t *testing.T) { client.NextStatus(200) _, errBytes := execute(command{args: []string{"test", "testdata/tests/production-test/illegal-reference.json"}}, t, client) assertRequests([]*http.Request{createRequest("GET", "http://127.0.0.1:8080/search/", "{}")}, client, t) - assert.Equal(t, "\nError: path may not point outside src/test/application, but 'foo/../../../../this-is-not-ok.json' does\nHint: Error in Step 2\nHint: See https://cloud.vespa.ai/en/reference/testing\n", errBytes) + assert.Equal(t, "\nError: error in Step 2: path may not point outside src/test/application, but 'foo/../../../../this-is-not-ok.json' does\nHint: See https://cloud.vespa.ai/en/reference/testing\n", errBytes) } func TestProductionTest(t *testing.T) { @@ -71,7 +71,7 @@ func TestTestWithoutAssertions(t *testing.T) { func TestSuiteWithoutTests(t *testing.T) { client := &mockHttpClient{} _, errBytes := execute(command{args: []string{"test", "testdata/tests/staging-test"}}, t, client) - assert.Equal(t, "Error: Failed to find any tests at testdata/tests/staging-test\nHint: See https://cloud.vespa.ai/en/reference/testing\n", errBytes) + assert.Equal(t, "Error: failed to find any tests at testdata/tests/staging-test\nHint: See https://cloud.vespa.ai/en/reference/testing\n", errBytes) } func TestSingleTest(t *testing.T) { diff --git a/client/go/cmd/version.go b/client/go/cmd/version.go index d2760402851..2660dfe7d61 100644 --- a/client/go/cmd/version.go +++ b/client/go/cmd/version.go @@ -46,14 +46,14 @@ var versionCmd = &cobra.Command{ Use: "version", Short: "Show current version and check for updates", DisableAutoGenTag: true, + SilenceUsage: true, Args: cobra.ExactArgs(0), - Run: func(cmd *cobra.Command, args []string) { + RunE: func(cmd *cobra.Command, args []string) error { log.Printf("vespa version %s compiled with %v on %v/%v", build.Version, runtime.Version(), runtime.GOOS, runtime.GOARCH) if !skipVersionCheck && sp.isTerminal() { - if err := checkVersion(); err != nil { - fatalErr(err) - } + return checkVersion() } + return nil }, } diff --git a/client/go/cmd/vespa/main.go b/client/go/cmd/vespa/main.go index 32828b15aa4..f7ce064f3a5 100644 --- a/client/go/cmd/vespa/main.go +++ b/client/go/cmd/vespa/main.go @@ -5,12 +5,17 @@ package main import ( - "github.com/vespa-engine/vespa/client/go/cmd" "os" + + "github.com/vespa-engine/vespa/client/go/cmd" ) func main() { if err := cmd.Execute(); err != nil { - os.Exit(1) + if cliErr, ok := err.(cmd.ErrCLI); ok { + os.Exit(cliErr.Status) + } else { + os.Exit(1) + } } } diff --git a/client/go/util/http.go b/client/go/util/http.go index d5b8e3128ff..f95d52dabad 100644 --- a/client/go/util/http.go +++ b/client/go/util/http.go @@ -49,7 +49,7 @@ func CreateClient(timeout time.Duration) HttpClient { func HttpGet(host string, path string, description string) (*http.Response, error) { url, err := url.Parse(host + path) if err != nil { - return nil, fmt.Errorf("Invalid target URL: %s: %w", host+path, err) + return nil, fmt.Errorf("invalid target url: %s: %w", host+path, err) } return HttpDo(&http.Request{URL: url}, time.Second*10, description) } diff --git a/client/go/vespa/deploy.go b/client/go/vespa/deploy.go index d52fc969c37..6ef4995e181 100644 --- a/client/go/vespa/deploy.go +++ b/client/go/vespa/deploy.go @@ -133,7 +133,7 @@ func (ap *ApplicationPackage) zipReader(test bool) (io.ReadCloser, error) { if !ap.IsZip() { tempZip, err := ioutil.TempFile("", "vespa") if err != nil { - return nil, fmt.Errorf("Could not create a temporary zip file for the application package: %w", err) + return nil, fmt.Errorf("could not create a temporary zip file for the application package: %w", err) } defer func() { tempZip.Close() @@ -146,7 +146,7 @@ func (ap *ApplicationPackage) zipReader(test bool) (io.ReadCloser, error) { } f, err := os.Open(zipFile) if err != nil { - return nil, fmt.Errorf("Could not open application package at %s: %w", ap.Path, err) + return nil, fmt.Errorf("could not open application package at %s: %w", ap.Path, err) } return f, nil } @@ -403,9 +403,9 @@ func uploadApplicationPackage(url *url.URL, opts DeploymentOpts) (int64, error) func checkResponse(req *http.Request, response *http.Response, serviceDescription string) error { if response.StatusCode/100 == 4 { - return fmt.Errorf("Invalid application package (%s)\n\n%s", response.Status, extractError(response.Body)) + return fmt.Errorf("invalid application package (%s)\n%s", response.Status, extractError(response.Body)) } else if response.StatusCode != 200 { - return fmt.Errorf("Error from %s at %s (%s):\n%s", strings.ToLower(serviceDescription), req.URL.Host, response.Status, util.ReaderToJSON(response.Body)) + return fmt.Errorf("error from %s at %s (%s):\n%s", strings.ToLower(serviceDescription), req.URL.Host, response.Status, util.ReaderToJSON(response.Body)) } return nil } diff --git a/client/go/vespa/target.go b/client/go/vespa/target.go index 99da625cf0b..f50716a5a3a 100644 --- a/client/go/vespa/target.go +++ b/client/go/vespa/target.go @@ -291,6 +291,9 @@ func (t *cloudTarget) PrepareApiRequest(req *http.Request, sigKeyId string) erro func (t *cloudTarget) addAuth0AccessToken(request *http.Request) error { a, err := auth0.GetAuth0(t.authConfigPath, t.systemName, t.apiURL) + if err != nil { + return err + } system, err := a.PrepareSystem(auth0.ContextWithCancel()) if err != nil { return err @@ -562,7 +565,7 @@ func wait(fn responseFunc, reqFn requestFunc, certificate *tls.Certificate, time return statusCode, nil } } - timeLeft := deadline.Sub(time.Now()) + timeLeft := time.Until(deadline) if loopOnce || timeLeft < retryInterval { break } |