From 348d8d83c3c823b64fc0692be607b1a5a8fac775 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20=C3=81lvarez?= Date: Thu, 12 Dec 2019 13:26:16 +0100 Subject: [PATCH] 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 | 20 ++++++------ beater/api/config/agent/handler_test.go | 4 +-- changelogs/7.5.asciidoc | 10 ++++++ kibana/connecting_client.go | 41 +++++++++++++++++++------ kibana/connecting_client_test.go | 6 ++-- tests/kibana.go | 5 ++- 6 files changed, 62 insertions(+), 24 deletions(-) diff --git a/beater/api/config/agent/handler.go b/beater/api/config/agent/handler.go index 7dd6da6e8b..769e2f3efe 100644 --- a/beater/api/config/agent/handler.go +++ b/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", diff --git a/beater/api/config/agent/handler_test.go b/beater/api/config/agent/handler_test.go index 3647143c1f..663738d5f8 100644 --- a/beater/api/config/agent/handler_test.go +++ b/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" diff --git a/changelogs/7.5.asciidoc b/changelogs/7.5.asciidoc index 2609086aa0..83e82306ca 100644 --- a/changelogs/7.5.asciidoc +++ b/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.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 diff --git a/kibana/connecting_client.go b/kibana/connecting_client.go index a60d126b40..9a00d26be2 100644 --- a/kibana/connecting_client.go +++ b/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 { diff --git a/kibana/connecting_client_test.go b/kibana/connecting_client_test.go index 74a16778b2..0e25b4c43b 100644 --- a/kibana/connecting_client_test.go +++ b/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) diff --git a/tests/kibana.go b/tests/kibana.go index 0bef37fac4..cebe631f0a 100644 --- a/tests/kibana.go +++ b/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 }