Merge pull request #31602 from vespa-engine/mpolden/vespa-log-local

Support non-cloud target in vespa log

7 files changed, 173 insertions, 79 deletions

diff --git a/client/go/internal/cli/cmd/log.go b/client/go/internal/cli/cmd/log.go
index 77ef7f68130..53b7079f428 100644
--- a/client/go/internal/cli/cmd/log.go
+++ b/client/go/internal/cli/cmd/log.go
@@ -6,6 +6,7 @@ import (
 	"time"
 
 	"github.com/spf13/cobra"
+	"github.com/vespa-engine/vespa/client/go/internal/version"
 	"github.com/vespa-engine/vespa/client/go/internal/vespa"
 )
 
@@ -34,7 +35,7 @@ $ vespa log --follow`,
 		SilenceUsage:      true,
 		Args:              cobra.MaximumNArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			target, err := cli.target(targetOptions{logLevel: levelArg, supportedType: cloudTargetOnly})
+			target, err := cli.target(targetOptions{logLevel: levelArg})
 			if err != nil {
 				return err
 			}
@@ -58,7 +59,12 @@ $ vespa log --follow`,
 				options.To = to
 			}
 			if err := target.PrintLog(options); err != nil {
-				return fmt.Errorf("could not retrieve logs: %w", err)
+				versionWithLogContainer := version.MustParse("8.359.0")
+				var hints []string
+				if err := target.CompatibleWith(versionWithLogContainer); err != nil {
+					hints = []string{fmt.Sprintf("This command requires a newer version of the Vespa platform: %s", err)}
+				}
+				return errHint(fmt.Errorf("could not retrieve logs: %w", err), hints...)
 			}
 			return nil
 		},
diff --git a/client/go/internal/cli/cmd/log_test.go b/client/go/internal/cli/cmd/log_test.go
index c1cab951793..e8e8a76b988 100644
--- a/client/go/internal/cli/cmd/log_test.go
+++ b/client/go/internal/cli/cmd/log_test.go
@@ -9,7 +9,7 @@ import (
 	"github.com/vespa-engine/vespa/client/go/internal/version"
 )
 
-func TestLog(t *testing.T) {
+func TestLogCloud(t *testing.T) {
 	_, pkgDir := mock.ApplicationPackageDir(t, false, false)
 	httpClient := &mock.HTTPClient{}
 	httpClient.NextResponseString(200, `1632738690.905535	host1a.dev.aws-us-east-1c	806/53	logserver-container	Container.com.yahoo.container.jdisc.ConfiguredApplication	info	Switching to the latest deployed set of configurations and components. Application config generation: 52532`)
@@ -30,14 +30,13 @@ func TestLog(t *testing.T) {
 	assert.Contains(t, stderr.String(), "Error: invalid period: cannot combine --from/--to with relative value: 1h\n")
 }
 
-func TestLogOldClient(t *testing.T) {
+func TestLogCloudIncompatible(t *testing.T) {
 	cli, _, stderr := newTestCLI(t)
 	cli.version = version.MustParse("7.0.0")
 
 	_, pkgDir := mock.ApplicationPackageDir(t, false, false)
 	httpClient := &mock.HTTPClient{}
 	httpClient.NextResponseString(200, `{"minVersion": "8.0.0"}`)
-	httpClient.NextResponseString(200, `1632738690.905535	host1a.dev.aws-us-east-1c	806/53	logserver-container	Container.com.yahoo.container.jdisc.ConfiguredApplication	info	Switching to the latest deployed set of configurations and components. Application config generation: 52532`)
 	cli.httpClient = httpClient
 
 	assert.Nil(t, cli.Run("config", "set", "application", "t1.a1.i1"))
@@ -46,6 +45,37 @@ func TestLogOldClient(t *testing.T) {
 	assert.Nil(t, cli.Run("auth", "cert", pkgDir))
 
 	assert.Nil(t, cli.Run("log"))
-	expected := "Warning: client version 7.0.0 is less than the minimum supported version: 8.0.0\nHint: This version may not work as expected\nHint: Try 'vespa version' to check for a new version\n"
+	expected := "Warning: client version 7.0.0 is less than the minimum supported version: 8.0.0\nHint: This version of CLI may not work as expected\nHint: Try 'vespa version' to check for a new version\n"
 	assert.Contains(t, stderr.String(), expected)
 }
+
+func TestLogLocal(t *testing.T) {
+	httpClient := &mock.HTTPClient{}
+	httpClient.NextResponseString(200, `1632738690.905535	localhost	806/53	logserver-container	Container.com.yahoo.container.jdisc.ConfiguredApplication	info	Switching to the latest deployed set of configurations and components. Application config generation: 52532`)
+	cli, stdout, stderr := newTestCLI(t)
+	cli.httpClient = httpClient
+
+	assert.Nil(t, cli.Run("log", "--from", "2021-09-27T10:00:00Z", "--to", "2021-09-27T11:00:00Z"))
+	expected := "[2021-09-27 10:31:30.905535] localhost info    logserver-container Container.com.yahoo.container.jdisc.ConfiguredApplication	Switching to the latest deployed set of configurations and components. Application config generation: 52532\n"
+	assert.Equal(t, expected, stdout.String())
+
+	assert.NotNil(t, cli.Run("log", "--from", "2021-09-27T13:12:49Z", "--to", "2021-09-27T13:15:00", "1h"))
+	assert.Contains(t, stderr.String(), "Error: invalid period: cannot combine --from/--to with relative value: 1h\n")
+}
+
+func TestLogLocalIncompatible(t *testing.T) {
+	httpClient := &mock.HTTPClient{}
+	httpClient.NextResponseString(404, `not found`)
+	httpClient.NextResponse(mock.HTTPResponse{
+		URI:    "/state/v1/version",
+		Status: 200,
+		Body:   []byte(`{"version": "8.358.0"}`),
+	})
+	cli, _, stderr := newTestCLI(t)
+	cli.httpClient = httpClient
+
+	assert.NotNil(t, cli.Run("log", "--from", "2021-09-27T10:00:00Z", "--to", "2021-09-27T11:00:00Z"))
+	assert.Equal(t, `Error: could not retrieve logs: failed to read logs: aborting wait: got status 404
+Hint: This command requires a newer version of the Vespa platform: platform version is older than required version: 8.358.0 < 8.359.0
+`, stderr.String())
+}
diff --git a/client/go/internal/cli/cmd/root.go b/client/go/internal/cli/cmd/root.go
index 5d5314d694f..c0f6f3af51e 100644
--- a/client/go/internal/cli/cmd/root.go
+++ b/client/go/internal/cli/cmd/root.go
@@ -405,9 +405,9 @@ func (c *CLI) target(opts targetOptions) (vespa.Target, error) {
 	if err != nil {
 		return nil, err
 	}
-	if !c.isCloudCI() { // Vespa Cloud always runs an up-to-date version
-		if err := target.CheckVersion(c.version); err != nil {
-			c.printWarning(err, "This version may not work as expected", "Try 'vespa version' to check for a new version")
+	if target.IsCloud() && !c.isCloudCI() { // Vespa Cloud always runs an up-to-date version
+		if err := target.CompatibleWith(c.version); err != nil {
+			c.printWarning(err, "This version of CLI may not work as expected", "Try 'vespa version' to check for a new version")
 		}
 	}
 	return target, nil
diff --git a/client/go/internal/vespa/target.go b/client/go/internal/vespa/target.go
index 5270b5669f9..674bedc9343 100644
--- a/client/go/internal/vespa/target.go
+++ b/client/go/internal/vespa/target.go
@@ -3,11 +3,14 @@
 package vespa
 
 import (
+	"bytes"
 	"crypto/tls"
 	"errors"
 	"fmt"
 	"io"
+	"math"
 	"net/http"
+	"strconv"
 	"strings"
 	"time"
 
@@ -120,8 +123,8 @@ type Target interface {
 	// PrintLog writes the logs of this deployment using given options to control output.
 	PrintLog(options LogOptions) error
 
-	// CheckVersion verifies whether clientVersion is compatible with this target.
-	CheckVersion(clientVersion version.Version) error
+	// CompatibleWith returns nil if target is compatible with the given version.
+	CompatibleWith(version version.Version) error
 }
 
 // TLSOptions holds the client certificate to use for cloud API or service requests.
@@ -252,6 +255,64 @@ func isOK(status int) (bool, error) {
 	}
 }
 
+func deployServiceWait(target Target, fn responseFunc, reqFn requestFunc, timeout, retryInterval time.Duration) (int, error) {
+	deployService, err := target.DeployService()
+	if err != nil {
+		return 0, err
+	}
+	return wait(deployService, fn, reqFn, timeout, retryInterval)
+}
+
+func pollLogs(target Target, logsURL string, options LogOptions, retryInterval time.Duration) error {
+	req, err := http.NewRequest("GET", logsURL, nil)
+	if err != nil {
+		return err
+	}
+	lastFrom := options.From
+	requestFunc := func() *http.Request {
+		fromMillis := lastFrom.Unix() * 1000
+		q := req.URL.Query()
+		q.Set("from", strconv.FormatInt(fromMillis, 10))
+		if !options.To.IsZero() {
+			toMillis := options.To.Unix() * 1000
+			q.Set("to", strconv.FormatInt(toMillis, 10))
+		}
+		req.URL.RawQuery = q.Encode()
+		return req
+	}
+	logFunc := func(status int, response []byte) (bool, error) {
+		if ok, err := isOK(status); !ok {
+			return ok, err
+		}
+		logEntries, err := ReadLogEntries(bytes.NewReader(response))
+		if err != nil {
+			return false, err
+		}
+		for _, le := range logEntries {
+			if !le.Time.After(lastFrom) {
+				continue
+			}
+			if LogLevel(le.Level) > options.Level {
+				continue
+			}
+			fmt.Fprintln(options.Writer, le.Format(options.Dequote))
+		}
+		if len(logEntries) > 0 {
+			lastFrom = logEntries[len(logEntries)-1].Time
+		}
+		return false, nil
+	}
+	var timeout time.Duration
+	if options.Follow {
+		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 := deployServiceWait(target, logFunc, requestFunc, timeout, retryInterval); err != nil && !errors.Is(err, ErrWaitTimeout) {
+		return fmt.Errorf("failed to read logs: %s", err)
+	}
+	return nil
+}
+
 // responseFunc returns whether a HTTP request is considered successful, based on its status and response data.
 // Returning false indicates that the operation should be retried. An error is returned if the response is considered
 // terminal and that the request should not be retried.
diff --git a/client/go/internal/vespa/target_cloud.go b/client/go/internal/vespa/target_cloud.go
index 6883515cee5..05d6bdd224e 100644
--- a/client/go/internal/vespa/target_cloud.go
+++ b/client/go/internal/vespa/target_cloud.go
@@ -2,11 +2,8 @@
 package vespa
 
 import (
-	"bytes"
 	"encoding/json"
-	"errors"
 	"fmt"
-	"math"
 	"net/http"
 	"sort"
 	"strconv"
@@ -148,7 +145,7 @@ func (t *cloudTarget) ContainerServices(timeout time.Duration) ([]*Service, erro
 	return services, nil
 }
 
-func (t *cloudTarget) CheckVersion(clientVersion version.Version) error {
+func (t *cloudTarget) CompatibleWith(clientVersion version.Version) error {
 	if clientVersion.IsZero() { // development version is always fine
 		return nil
 	}
@@ -190,61 +187,7 @@ func (t *cloudTarget) logsURL() string {
 }
 
 func (t *cloudTarget) PrintLog(options LogOptions) error {
-	req, err := http.NewRequest("GET", t.logsURL(), nil)
-	if err != nil {
-		return err
-	}
-	lastFrom := options.From
-	requestFunc := func() *http.Request {
-		fromMillis := lastFrom.Unix() * 1000
-		q := req.URL.Query()
-		q.Set("from", strconv.FormatInt(fromMillis, 10))
-		if !options.To.IsZero() {
-			toMillis := options.To.Unix() * 1000
-			q.Set("to", strconv.FormatInt(toMillis, 10))
-		}
-		req.URL.RawQuery = q.Encode()
-		return req
-	}
-	logFunc := func(status int, response []byte) (bool, error) {
-		if ok, err := isOK(status); !ok {
-			return ok, err
-		}
-		logEntries, err := ReadLogEntries(bytes.NewReader(response))
-		if err != nil {
-			return false, err
-		}
-		for _, le := range logEntries {
-			if !le.Time.After(lastFrom) {
-				continue
-			}
-			if LogLevel(le.Level) > options.Level {
-				continue
-			}
-			fmt.Fprintln(options.Writer, le.Format(options.Dequote))
-		}
-		if len(logEntries) > 0 {
-			lastFrom = logEntries[len(logEntries)-1].Time
-		}
-		return false, nil
-	}
-	var timeout time.Duration
-	if options.Follow {
-		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) {
-		return fmt.Errorf("failed to read logs: %s", err)
-	}
-	return nil
-}
-
-func (t *cloudTarget) deployServiceWait(fn responseFunc, reqFn requestFunc, timeout time.Duration) (int, error) {
-	deployService, err := t.DeployService()
-	if err != nil {
-		return 0, err
-	}
-	return wait(deployService, fn, reqFn, timeout, t.retryInterval)
+	return pollLogs(t, t.logsURL(), options, t.retryInterval)
 }
 
 func (t *cloudTarget) discoverLatestRun(timeout time.Duration) (int64, error) {
@@ -269,7 +212,7 @@ func (t *cloudTarget) discoverLatestRun(timeout time.Duration) (int64, error) {
 		}
 		return false, nil
 	}
-	_, err = t.deployServiceWait(jobsSuccessFunc, requestFunc, timeout)
+	_, err = deployServiceWait(t, jobsSuccessFunc, requestFunc, timeout, t.retryInterval)
 	return lastRunID, err
 }
 
@@ -314,7 +257,7 @@ func (t *cloudTarget) AwaitDeployment(runID int64, timeout time.Duration) (int64
 		success = true
 		return success, nil
 	}
-	_, err = t.deployServiceWait(jobSuccessFunc, requestFunc, timeout)
+	_, err = deployServiceWait(t, jobSuccessFunc, requestFunc, timeout, t.retryInterval)
 	if err != nil {
 		return 0, fmt.Errorf("deployment run %d not yet complete%s: %w", runID, waitDescription(timeout), err)
 	}
@@ -378,7 +321,7 @@ func (t *cloudTarget) discoverEndpoints(timeout time.Duration) (map[string]strin
 		}
 		return true, nil
 	}
-	if _, err := t.deployServiceWait(endpointFunc, func() *http.Request { return req }, timeout); err != nil {
+	if _, err := deployServiceWait(t, endpointFunc, func() *http.Request { return req }, timeout, t.retryInterval); err != nil {
 		return nil, fmt.Errorf("no endpoints found in zone %s%s: %w", t.deploymentOptions.Deployment.Zone, waitDescription(timeout), err)
 	}
 	if len(urlsByCluster) == 0 {
diff --git a/client/go/internal/vespa/target_custom.go b/client/go/internal/vespa/target_custom.go
index 9d62f7dc297..1f72308178a 100644
--- a/client/go/internal/vespa/target_custom.go
+++ b/client/go/internal/vespa/target_custom.go
@@ -64,10 +64,48 @@ func (t *customTarget) IsCloud() bool { return false }
 func (t *customTarget) Deployment() Deployment { return DefaultDeployment }
 
 func (t *customTarget) PrintLog(options LogOptions) error {
-	return fmt.Errorf("log access is only supported on cloud: run vespa-logfmt on the admin node instead, or export from a container image (here named 'vespa') using docker exec vespa vespa-logfmt")
+	deployService, err := t.DeployService()
+	if err != nil {
+		return err
+	}
+	logsURL := deployService.BaseURL + "/application/v2/tenant/default/application/default/environment/prod/region/default/instance/default/logs"
+	return pollLogs(t, logsURL, options, t.retryInterval)
 }
 
-func (t *customTarget) CheckVersion(version version.Version) error { return nil }
+func (t *customTarget) CompatibleWith(minVersion version.Version) error {
+	if minVersion.IsZero() { // development version is always fine
+		return nil
+	}
+	deployService, err := t.DeployService()
+	if err != nil {
+		return err
+	}
+	versionURL := deployService.BaseURL + "/state/v1/version"
+	req, err := http.NewRequest("GET", versionURL, nil)
+	if err != nil {
+		return err
+	}
+	var versionResponse struct {
+		Version string `json:"version"`
+	}
+	response, err := deployService.Do(req, 10*time.Second)
+	if err != nil {
+		return err
+	}
+	defer response.Body.Close()
+	dec := json.NewDecoder(response.Body)
+	if err := dec.Decode(&versionResponse); err != nil {
+		return err
+	}
+	targetVersion, err := version.Parse(versionResponse.Version)
+	if err != nil {
+		return err
+	}
+	if targetVersion.Less(minVersion) {
+		return fmt.Errorf("platform version is older than required version: %s < %s", targetVersion, minVersion)
+	}
+	return nil
+}
 
 func (t *customTarget) newService(url, name string, deployAPI bool) *Service {
 	return &Service{
diff --git a/client/go/internal/vespa/target_test.go b/client/go/internal/vespa/target_test.go
index 4c2fda8368e..f886c9117a9 100644
--- a/client/go/internal/vespa/target_test.go
+++ b/client/go/internal/vespa/target_test.go
@@ -117,6 +117,22 @@ func TestCustomTargetAwaitDeployment(t *testing.T) {
 	assert.Equal(t, int64(42), convergedID)
 }
 
+func TestCustomTargetCompatibleWith(t *testing.T) {
+	client := &mock.HTTPClient{}
+	target := CustomTarget(client, "http://192.0.2.42", TLSOptions{}, 0)
+	for i := 0; i < 3; i++ {
+		client.NextResponse(mock.HTTPResponse{
+			URI:    "/state/v1/version",
+			Status: 200,
+			Body:   []byte(`{"version": "1.2.3"}`),
+		})
+	}
+	assert.Nil(t, target.CompatibleWith(version.MustParse("1.2.2")))
+	assert.Nil(t, target.CompatibleWith(version.MustParse("1.2.3")))
+	assert.NotNil(t, target.CompatibleWith(version.MustParse("1.2.4")))
+	assert.True(t, client.Consumed())
+}
+
 func TestCloudTargetWait(t *testing.T) {
 	var logWriter bytes.Buffer
 	target, client := createCloudTarget(t, &logWriter)
@@ -231,14 +247,14 @@ func TestLog(t *testing.T) {
 	assert.Equal(t, expected, buf.String())
 }
 
-func TestCheckVersion(t *testing.T) {
+func TestCloudCompatibleWith(t *testing.T) {
 	target, client := createCloudTarget(t, io.Discard)
 	for i := 0; i < 3; i++ {
 		client.NextResponse(mock.HTTPResponse{URI: "/cli/v1/", Status: 200, Body: []byte(`{"minVersion":"8.0.0"}`)})
 	}
-	assert.Nil(t, target.CheckVersion(version.MustParse("8.0.0")))
-	assert.Nil(t, target.CheckVersion(version.MustParse("8.1.0")))
-	assert.NotNil(t, target.CheckVersion(version.MustParse("7.0.0")))
+	assert.Nil(t, target.CompatibleWith(version.MustParse("8.0.0")))
+	assert.Nil(t, target.CompatibleWith(version.MustParse("8.1.0")))
+	assert.NotNil(t, target.CompatibleWith(version.MustParse("7.0.0")))
 }
 
 func createCloudTarget(t *testing.T, logWriter io.Writer) (Target, *mock.HTTPClient) {