Check for compatible version in log command

7 files changed, 100 insertions, 15 deletions

diff --git a/client/go/internal/cli/cmd/log.go b/client/go/internal/cli/cmd/log.go
index 73117bc7ab4..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"
 )
 
@@ -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 90aecabb65f..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"))
@@ -49,3 +48,34 @@ func TestLogOldClient(t *testing.T) {
 	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 e7a15682ba4..674bedc9343 100644
--- a/client/go/internal/vespa/target.go
+++ b/client/go/internal/vespa/target.go
@@ -123,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.
diff --git a/client/go/internal/vespa/target_cloud.go b/client/go/internal/vespa/target_cloud.go
index 06fad31bd66..05d6bdd224e 100644
--- a/client/go/internal/vespa/target_cloud.go
+++ b/client/go/internal/vespa/target_cloud.go
@@ -145,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
 	}
diff --git a/client/go/internal/vespa/target_custom.go b/client/go/internal/vespa/target_custom.go
index a5e0c2ee2a0..1f72308178a 100644
--- a/client/go/internal/vespa/target_custom.go
+++ b/client/go/internal/vespa/target_custom.go
@@ -72,7 +72,40 @@ func (t *customTarget) PrintLog(options LogOptions) error {
 	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) {