From aa6953a32b7ca4cf3fc14363fbedb1d1ff8dd24d Mon Sep 17 00:00:00 2001 From: Steffen Siering Date: Fri, 5 Apr 2019 18:11:28 +0200 Subject: [PATCH] Cherry-pick #11671 to 7.0: Fix template/policy is always overwritten (#11672) Cherry-pick of PR #11671 to 7.0 branch. Original message: The force flag for policy and template overwrites have been set to true in the standard Elasticsearch callback. Due to this every reconnect will result in the policy and template being overwritten. This fix sets the flags to false and ensure that the template is only overwritten if a new policy is created in Elasticsearch. --- CHANGELOG.next.asciidoc | 2 ++ libbeat/cmd/instance/beat.go | 2 +- libbeat/idxmgmt/idxmgmt.go | 2 +- libbeat/idxmgmt/ilm/ilm.go | 8 +++++++- libbeat/idxmgmt/ilm/ilm_test.go | 6 +++++- libbeat/idxmgmt/ilm/noop.go | 6 +++--- libbeat/idxmgmt/ilm/std.go | 8 ++++---- libbeat/idxmgmt/mockilm_test.go | 4 ++-- libbeat/idxmgmt/std.go | 12 +++++++++--- 9 files changed, 34 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index 51ebb846371..3ae08217f45 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -32,6 +32,8 @@ https://github.com/elastic/beats/compare/v7.0.0-rc1...master[Check the HEAD diff - Relax validation of the X-Pack license UID value. {issue}11640[11640] - Fix a parsing error with the X-Pack license check on 32-bit system. {issue}11650[11650] +- Fix ILM policy always being overwritten. {pull}11671[11671] +- Fix template always being overwritten. {pull}11671[11671] *Auditbeat* diff --git a/libbeat/cmd/instance/beat.go b/libbeat/cmd/instance/beat.go index f76f2047cf1..9bb322df89b 100644 --- a/libbeat/cmd/instance/beat.go +++ b/libbeat/cmd/instance/beat.go @@ -763,7 +763,7 @@ func (b *Beat) registerESIndexManagement() error { func (b *Beat) indexSetupCallback() func(esClient *elasticsearch.Client) error { return func(esClient *elasticsearch.Client) error { m := b.index.Manager(esClient, idxmgmt.BeatsAssets(b.Fields)) - return m.Setup(true, true) + return m.Setup(false, false) } } diff --git a/libbeat/idxmgmt/idxmgmt.go b/libbeat/idxmgmt/idxmgmt.go index 6ee86354c3f..adda46d62c3 100644 --- a/libbeat/idxmgmt/idxmgmt.go +++ b/libbeat/idxmgmt/idxmgmt.go @@ -75,7 +75,7 @@ type ESClient interface { // Manager is used to initialize indices, ILM policies, and aliases within the // Elastic Stack. type Manager interface { - Setup(template, policy bool) error + Setup(forceTemplate, forcePolicy bool) error } // DefaultSupport initializes the default index management support used by most Beats. diff --git a/libbeat/idxmgmt/ilm/ilm.go b/libbeat/idxmgmt/ilm/ilm.go index 6ca833b978f..98cc77b70b2 100644 --- a/libbeat/idxmgmt/ilm/ilm.go +++ b/libbeat/idxmgmt/ilm/ilm.go @@ -45,8 +45,14 @@ type Supporter interface { // Manager uses an APIHandler to install a policy. type Manager interface { Enabled() (bool, error) + EnsureAlias() error - EnsurePolicy(overwrite bool) error + + // EnsurePolicy installs a policy if it does not exist. The policy is always + // written if overwrite is set. + // The created flag is set to true only if a new policy is created. `created` + // is false if an existing policy gets overwritten. + EnsurePolicy(overwrite bool) (created bool, err error) } // APIHandler defines the interface between a remote service and the Manager. diff --git a/libbeat/idxmgmt/ilm/ilm_test.go b/libbeat/idxmgmt/ilm/ilm_test.go index e4a6ccf2864..8abbb36c425 100644 --- a/libbeat/idxmgmt/ilm/ilm_test.go +++ b/libbeat/idxmgmt/ilm/ilm_test.go @@ -220,9 +220,11 @@ func TestDefaultSupport_Manager_EnsurePolicy(t *testing.T) { calls []onCall overwrite bool cfg map[string]interface{} + create bool fail error }{ "create new policy": { + create: true, calls: []onCall{ onHasILMPolicy(testPolicy.Name).Return(false, nil), onCreateILMPolicy(testPolicy).Return(nil), @@ -258,14 +260,16 @@ func TestDefaultSupport_Manager_EnsurePolicy(t *testing.T) { h := newMockHandler(test.calls...) m := createManager(t, h, test.cfg) - err := m.EnsurePolicy(test.overwrite) + created, err := m.EnsurePolicy(test.overwrite) if test.fail == nil { + assert.Equal(t, test.create, created) require.NoError(t, err) } else { require.Error(t, err) assert.Equal(t, test.fail, ErrReason(err)) } + h.AssertExpectations(t) }) } diff --git a/libbeat/idxmgmt/ilm/noop.go b/libbeat/idxmgmt/ilm/noop.go index 129ca4c9eab..c6479596aff 100644 --- a/libbeat/idxmgmt/ilm/noop.go +++ b/libbeat/idxmgmt/ilm/noop.go @@ -36,6 +36,6 @@ func (*noopSupport) Alias() Alias { return Alias{} } func (*noopSupport) Policy() Policy { return Policy{} } func (*noopSupport) Manager(_ APIHandler) Manager { return (*noopManager)(nil) } -func (*noopManager) Enabled() (bool, error) { return false, nil } -func (*noopManager) EnsureAlias() error { return errOf(ErrOpNotAvailable) } -func (*noopManager) EnsurePolicy(_ bool) error { return errOf(ErrOpNotAvailable) } +func (*noopManager) Enabled() (bool, error) { return false, nil } +func (*noopManager) EnsureAlias() error { return errOf(ErrOpNotAvailable) } +func (*noopManager) EnsurePolicy(_ bool) (bool, error) { return false, errOf(ErrOpNotAvailable) } diff --git a/libbeat/idxmgmt/ilm/std.go b/libbeat/idxmgmt/ilm/std.go index 8c55e471136..5709d03ca91 100644 --- a/libbeat/idxmgmt/ilm/std.go +++ b/libbeat/idxmgmt/ilm/std.go @@ -114,7 +114,7 @@ func (m *singlePolicyManager) EnsureAlias() error { return m.client.CreateAlias(m.alias) } -func (m *singlePolicyManager) EnsurePolicy(overwrite bool) error { +func (m *singlePolicyManager) EnsurePolicy(overwrite bool) (bool, error) { log := m.log overwrite = overwrite || m.overwrite @@ -122,18 +122,18 @@ func (m *singlePolicyManager) EnsurePolicy(overwrite bool) error { if m.checkExists && !overwrite { b, err := m.client.HasILMPolicy(m.policy.Name) if err != nil { - return err + return false, err } exists = b } if !exists || overwrite { - return m.client.CreateILMPolicy(m.policy) + return !exists, m.client.CreateILMPolicy(m.policy) } log.Infof("do not generate ilm policy: exists=%v, overwrite=%v", exists, overwrite) - return nil + return false, nil } func (c *infoCache) Valid() bool { diff --git a/libbeat/idxmgmt/mockilm_test.go b/libbeat/idxmgmt/mockilm_test.go index 5aac9419ace..305d910717f 100644 --- a/libbeat/idxmgmt/mockilm_test.go +++ b/libbeat/idxmgmt/mockilm_test.go @@ -86,9 +86,9 @@ func (m *mockILMSupport) EnsureAlias() error { } func onEnsurePolicy() onCall { return makeOnCall("EnsurePolicy") } -func (m *mockILMSupport) EnsurePolicy(overwrite bool) error { +func (m *mockILMSupport) EnsurePolicy(overwrite bool) (bool, error) { args := m.Called() - return args.Error(0) + return args.Bool(0), args.Error(1) } func makeOnCall(name string, args ...interface{}) onCall { diff --git a/libbeat/idxmgmt/std.go b/libbeat/idxmgmt/std.go index a2e8149e7bb..e97e0799fd2 100644 --- a/libbeat/idxmgmt/std.go +++ b/libbeat/idxmgmt/std.go @@ -200,8 +200,8 @@ func (s *indexSupport) BuildSelector(cfg *common.Config) (outputs.IndexSelector, }, nil } -func (m *indexManager) Setup(template, policy bool) error { - return m.load(template, policy) +func (m *indexManager) Setup(forceTemplate, forcePolicy bool) error { + return m.load(forceTemplate, forcePolicy) } func (m *indexManager) Load() error { @@ -231,10 +231,16 @@ func (m *indexManager) load(forceTemplate, forcePolicy bool) error { // install ilm policy if withILM { - if err := m.ilm.EnsurePolicy(forcePolicy); err != nil { + policyCreated, err := m.ilm.EnsurePolicy(forcePolicy) + if err != nil { return err } log.Info("ILM policy successfully loaded.") + + // The template should be updated if a new policy is created. + if policyCreated { + forceTemplate = true + } } // create and install template