Retry connection when Kibana version is not compatible. (#3031) (#3041)

* Retry connection when Kibana version is not compatible.

Fixes #3022

* Update changelog

beater/api/config/agent/handler.go

@@ -117,15 +117,17 @@ func validateClient(c *request.Context, client kibana.Client, withAuth bool) boo
 			errMsgKibanaDisabled)
 		return false
 	}
-	if !client.Connected() {
-		c.Result.Set(request.IDResponseErrorsServiceUnavailable,
-			http.StatusServiceUnavailable,
-			msgNoKibanaConnection,
-			msgNoKibanaConnection,
-			errMsgNoKibanaConnection)
-		return false
-	}
-	if supported, _ := client.SupportsVersion(minKibanaVersion); !supported {
+
+	if supported, err := client.SupportsVersion(minKibanaVersion, true); !supported {
+		if err != nil {
+			c.Result.Set(request.IDResponseErrorsServiceUnavailable,
+				http.StatusServiceUnavailable,
+				msgNoKibanaConnection,
+				msgNoKibanaConnection,
+				errMsgNoKibanaConnection)
+			return false
+		}
+
 		version, _ := client.GetVersion()
 
 		errMsg := fmt.Sprintf("%s: min version %+v, configured version %+v",

beater/api/config/agent/handler_test.go

@@ -27,10 +27,10 @@ import (
 	"testing"
 	"time"
 
-	"golang.org/x/time/rate"
-
 	"github.com/elastic/apm-server/agentcfg"
 
+	"golang.org/x/time/rate"
+
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 

changelogs/7.5.asciidoc

@@ -4,6 +4,16 @@
 https://github.com/elastic/apm-server/compare/7.4\...7.5[View commits]
 
 * <<release-notes-7.5.0>>
+* <<release-notes-7.5.1>>
+
+[[release-notes-7.5.1]]
+=== APM Server version 7.5.1
+
+https://github.com/elastic/apm-server/compare/v7.5.0\...v7.5.1[View commits]
+
+[float]
+==== Bug fixes
+- Update Kibana client when its version becomes stale {pull}3031[3031].
 
 [[release-notes-7.5.0]]
 === APM Server version 7.5.0

kibana/connecting_client.go

@@ -50,14 +50,14 @@ type Client interface {
 	// Connected indicates whether or not a connection to Kibana has been established
 	Connected() bool
 	// SupportsVersion compares given version to version of connected Kibana instance
-	SupportsVersion(v *common.Version) (bool, error)
+	SupportsVersion(*common.Version, bool) (bool, error)
 }
 
 // ConnectingClient implements Client interface
 type ConnectingClient struct {
 	client *kibana.Client
 	cfg    *common.Config
-	m      sync.Mutex
+	m      sync.RWMutex
 }
 
 // NewConnectingClient returns instance of ConnectingClient and starts a background routine trying to connect
@@ -88,8 +88,9 @@ func NewConnectingClient(cfg *common.Config) Client {
 // If no connection is established an error is returned
 func (c *ConnectingClient) Send(method, extraPath string, params url.Values,
 	headers http.Header, body io.Reader) (*http.Response, error) {
-
-	if !c.Connected() {
+	c.m.RLock()
+	defer c.m.RUnlock()
+	if c.client == nil {
 		return nil, errNotConnected
 	}
 	return c.client.Send(method, extraPath, params, headers, body)
@@ -98,22 +99,44 @@ func (c *ConnectingClient) Send(method, extraPath string, params url.Values,
 // GetVersion returns Kibana version or an error
 // If no connection is established an error is returned
 func (c *ConnectingClient) GetVersion() (common.Version, error) {
-	if !c.Connected() {
+	c.m.RLock()
+	defer c.m.RUnlock()
+	if c.client == nil {
 		return common.Version{}, errNotConnected
 	}
 	return c.client.GetVersion(), nil
 }
 
 // Connected checks if a connection has been established
-func (c *ConnectingClient) Connected() bool { return c.client != nil }
+func (c *ConnectingClient) Connected() bool {
+	c.m.RLock()
+	defer c.m.RUnlock()
+	return c.client != nil
+}
 
 // SupportsVersion checks if connected Kibana instance is compatible to given version
 // If no connection is established an error is returned
-func (c *ConnectingClient) SupportsVersion(v *common.Version) (bool, error) {
-	if !c.Connected() {
+func (c *ConnectingClient) SupportsVersion(v *common.Version, retry bool) (bool, error) {
+	log := logp.NewLogger(logs.Kibana)
+	c.m.RLock()
+	if c.client == nil && !retry {
+		c.m.RUnlock()
 		return false, errNotConnected
 	}
-	return v.LessThanOrEqual(false, &c.client.Version), nil
+	upToDate := c.client != nil && v.LessThanOrEqual(false, &c.client.Version)
+	c.m.RUnlock()
+	if !retry || upToDate {
+		return upToDate, nil
+	}
+	client, err := kibana.NewKibanaClient(c.cfg)
+	if err != nil {
+		log.Errorf("failed to obtain connection to Kibana: %s", err.Error())
+		return upToDate, err
+	}
+	c.m.Lock()
+	c.client = client
+	c.m.Unlock()
+	return c.SupportsVersion(v, false)
 }
 
 func (c *ConnectingClient) connect() error {

kibana/connecting_client_test.go

@@ -76,20 +76,20 @@ func TestConnectingClient_GetVersion(t *testing.T) {
 func TestConnectingClient_SupportsVersion(t *testing.T) {
 	t.Run("SupportsVersionTrue", func(t *testing.T) {
 		c := mockClient()
-		s, err := c.SupportsVersion(common.MustNewVersion("7.3.0"))
+		s, err := c.SupportsVersion(common.MustNewVersion("7.3.0"), false)
 		require.NoError(t, err)
 		assert.True(t, s)
 	})
 	t.Run("SupportsVersionFalse", func(t *testing.T) {
 		c := mockClient()
-		s, err := c.SupportsVersion(common.MustNewVersion("7.4.0"))
+		s, err := c.SupportsVersion(common.MustNewVersion("7.4.0"), false)
 		require.NoError(t, err)
 		assert.False(t, s)
 	})
 
 	t.Run("SupportsVersionError", func(t *testing.T) {
 		c := NewConnectingClient(mockCfg)
-		s, err := c.SupportsVersion(common.MustNewVersion("7.3.0"))
+		s, err := c.SupportsVersion(common.MustNewVersion("7.3.0"), false)
 		require.Error(t, err)
 		assert.Equal(t, err, errNotConnected)
 		assert.False(t, s)

tests/kibana.go

@@ -58,7 +58,10 @@ func (c *MockKibanaClient) GetVersion() (common.Version, error) {
 func (c *MockKibanaClient) Connected() bool { return c.connected }
 
 // SupportsVersion returns whether or not mock client is compatible with given version
-func (c *MockKibanaClient) SupportsVersion(v *common.Version) (bool, error) {
+func (c *MockKibanaClient) SupportsVersion(v *common.Version, _ bool) (bool, error) {
+	if !c.connected {
+		return false, errors.New("unable to retrieve connection to Kibana")
+	}
 	return v.LessThanOrEqual(true, &c.v), nil
 }