From a20c1c437f5ac22854d05cea0ee0a9f2d338ed31 Mon Sep 17 00:00:00 2001 From: urso Date: Wed, 10 Mar 2021 19:41:40 +0100 Subject: [PATCH 1/9] Improve ILM policy and alias setup log output The idxmgmt has not all 'information' to properly log the outcome of the policy and status setup. The PR moves the reporting to the ILM package directly, and 'unifies' the logic of EnsurePolicy and EnsureAlias. --- libbeat/idxmgmt/ilm/ilm_test.go | 3 +- libbeat/idxmgmt/ilm/std.go | 71 ++++++++++++++++++++++++--------- libbeat/idxmgmt/std.go | 12 ++---- 3 files changed, 57 insertions(+), 29 deletions(-) diff --git a/libbeat/idxmgmt/ilm/ilm_test.go b/libbeat/idxmgmt/ilm/ilm_test.go index e47a9c0d06bc..884fb3741483 100644 --- a/libbeat/idxmgmt/ilm/ilm_test.go +++ b/libbeat/idxmgmt/ilm/ilm_test.go @@ -287,8 +287,9 @@ func TestDefaultSupport_Manager_EnsurePolicy(t *testing.T) { onHasILMPolicy(testPolicy.Name).Return(true, nil), }, }, - "overwrite existing": { + "overwrite": { overwrite: true, + create: true, calls: []onCall{ onCreateILMPolicy(testPolicy).Return(nil), }, diff --git a/libbeat/idxmgmt/ilm/std.go b/libbeat/idxmgmt/ilm/std.go index 0e33fc00f3ed..4fb3db739fe4 100644 --- a/libbeat/idxmgmt/ilm/std.go +++ b/libbeat/idxmgmt/ilm/std.go @@ -103,42 +103,75 @@ func (m *stdManager) CheckEnabled() (bool, error) { } func (m *stdManager) EnsureAlias() error { - if !m.checkExists { - return nil - } + log := m.log + overwrite := m.Overwrite() - b, err := m.client.HasAlias(m.alias.Name) - if err != nil { - return err + var exists bool + if m.checkExists && !overwrite { + var err error + exists, err = m.client.HasAlias(m.alias.Name) + if err != nil { + return err + } } - if b { + + switch { + case exists && !overwrite: + log.Infof("Index Alias %v exists already", m.alias.Name) return nil - } - // This always assume it's a date pattern by sourrounding it by <...> - return m.client.CreateAlias(m.alias) + case !exists || overwrite: + err := m.client.CreateAlias(m.alias) + if err != nil { + if ErrReason(err) != ErrAliasAlreadyExists { + log.Errorf("Index Alias %v setup failed: %v", m.alias.Name, err) + return err + } + log.Infof("Index Alias %v exists already", m.alias.Name) + return nil + } + + log.Info("Index Alias %v successfully created", m.alias.Name) + return nil + + default: + m.log.Infof("ILM index alias not created: exists=%v, overwrite=%v", exists, overwrite) + return nil + } } func (m *stdManager) EnsurePolicy(overwrite bool) (bool, error) { log := m.log overwrite = overwrite || m.Overwrite() - exists := true + var exists bool if m.checkExists && !overwrite { - b, err := m.client.HasILMPolicy(m.policy.Name) + var err error + exists, err = m.client.HasILMPolicy(m.policy.Name) if err != nil { return false, err } - exists = b } - if !exists || overwrite { - return !exists, m.client.CreateILMPolicy(m.policy) - } + switch { + case exists && !overwrite: + log.Infof("ILM policy %v exists already", m.policy) + return false, nil + + case !exists || overwrite: + err := m.client.CreateILMPolicy(m.policy) + if err != nil { + log.Errorf("ILM policy %v creation failed: %v", m.policy, err) + return false, err + } - log.Infof("do not generate ilm policy: exists=%v, overwrite=%v", - exists, overwrite) - return false, nil + log.Infof("ILM policy %v successfully created", m.policy) + return true, err + + default: + log.Infof("ILM policy not created: exists=%v, overwrite=%v", exists, overwrite) + return false, nil + } } func (c *infoCache) Valid() bool { diff --git a/libbeat/idxmgmt/std.go b/libbeat/idxmgmt/std.go index 538fc7d22624..722e1aa3a5ec 100644 --- a/libbeat/idxmgmt/std.go +++ b/libbeat/idxmgmt/std.go @@ -271,7 +271,6 @@ func (m *indexManager) Setup(loadTemplate, loadILM LoadMode) error { if err != nil { return err } - log.Info("ILM policy successfully loaded.") // The template should be updated if a new policy is created. if policyCreated && templateComponent.enabled { @@ -299,14 +298,9 @@ func (m *indexManager) Setup(loadTemplate, loadILM LoadMode) error { } if ilmComponent.load { - // ensure alias is created after the template is created - if err := m.ilm.EnsureAlias(); err != nil { - if ilm.ErrReason(err) != ilm.ErrAliasAlreadyExists { - return err - } - log.Info("Write alias exists already") - } else { - log.Info("Write alias successfully generated.") + _, err := m.ilm.EnsureAlias() + if err != nil { + return err } } From 3f61d7629cf567faa0b31c5629f38b8c710ad3f5 Mon Sep 17 00:00:00 2001 From: urso Date: Wed, 10 Mar 2021 19:58:59 +0100 Subject: [PATCH 2/9] Add changelog --- CHANGELOG.next.asciidoc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index 722fa5da14cf..26ec7e7e048b 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -240,6 +240,9 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d - Fix panic due to unhandled DeletedFinalStateUnknown in k8s OnDelete {pull}23419[23419] - Fix error loop with runaway CPU use when the Kafka output encounters some connection errors {pull}23484[23484] - Allow configuring credential_profile_name and shared_credential_file when using role_arn. {pull}24174[24174] +- Fix issue discovering docker containers and metadata after reconnections {pull}24318[24318] +- Fix ILM setup log reporting that a policy or an alias was created, even though the creation of any resource was disabled. {issue}24046[24046] {pull}24480[24480] +- Fix ILM alias not being created if `setup.ilm.check_exists: false` and `setup.ilm.overwrite: true` has been configured. {pull}24480[24480] *Auditbeat* From aba773ee5e03a1481b6eaae9f044799c653e4861 Mon Sep 17 00:00:00 2001 From: urso Date: Wed, 10 Mar 2021 20:14:39 +0100 Subject: [PATCH 3/9] Add EnsureAlias unit tests --- libbeat/idxmgmt/ilm/ilm_test.go | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/libbeat/idxmgmt/ilm/ilm_test.go b/libbeat/idxmgmt/ilm/ilm_test.go index 884fb3741483..8ca9b9e6c432 100644 --- a/libbeat/idxmgmt/ilm/ilm_test.go +++ b/libbeat/idxmgmt/ilm/ilm_test.go @@ -238,6 +238,27 @@ func TestDefaultSupport_Manager_EnsureAlias(t *testing.T) { }, fail: ErrRequestFailed, }, + "overwrite non-existent": { + calls: []onCall{ + onCreateAlias(alias).Return(nil), + }, + fail: nil, + cfg: map[string]interface{}{"check_exists": false, "overwrite": true}, + }, + "try overwrite existing": { + calls: []onCall{ + onCreateAlias(alias).Return(errOf(ErrAliasAlreadyExists)), + }, + fail: nil, // we detect that that the alias exists, and call it a day. + cfg: map[string]interface{}{"check_exists": false, "overwrite": true}, + }, + "fail to overwrite": { + calls: []onCall{ + onCreateAlias(alias).Return(errOf(ErrAliasCreateFailed)), + }, + fail: ErrAliasCreateFailed, + cfg: map[string]interface{}{"check_exists": false, "overwrite": true}, + }, } for name, test := range cases { @@ -283,6 +304,7 @@ func TestDefaultSupport_Manager_EnsurePolicy(t *testing.T) { }, }, "policy already exists": { + create: false, calls: []onCall{ onHasILMPolicy(testPolicy.Name).Return(true, nil), }, From c176ec789b78490eeba05cb755c9aedc2d4c3580 Mon Sep 17 00:00:00 2001 From: urso Date: Thu, 11 Mar 2021 16:59:23 +0100 Subject: [PATCH 4/9] fix build --- libbeat/idxmgmt/std.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libbeat/idxmgmt/std.go b/libbeat/idxmgmt/std.go index 722e1aa3a5ec..99cf5890353d 100644 --- a/libbeat/idxmgmt/std.go +++ b/libbeat/idxmgmt/std.go @@ -298,7 +298,7 @@ func (m *indexManager) Setup(loadTemplate, loadILM LoadMode) error { } if ilmComponent.load { - _, err := m.ilm.EnsureAlias() + err := m.ilm.EnsureAlias() if err != nil { return err } From 98ed72e5b868bcfba6869fcbc8d9666b7f04f90f Mon Sep 17 00:00:00 2001 From: urso Date: Wed, 24 Mar 2021 15:11:28 +0100 Subject: [PATCH 5/9] fix changelog --- CHANGELOG.next.asciidoc | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index 26ec7e7e048b..da51bdbef44a 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -240,7 +240,6 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d - Fix panic due to unhandled DeletedFinalStateUnknown in k8s OnDelete {pull}23419[23419] - Fix error loop with runaway CPU use when the Kafka output encounters some connection errors {pull}23484[23484] - Allow configuring credential_profile_name and shared_credential_file when using role_arn. {pull}24174[24174] -- Fix issue discovering docker containers and metadata after reconnections {pull}24318[24318] - Fix ILM setup log reporting that a policy or an alias was created, even though the creation of any resource was disabled. {issue}24046[24046] {pull}24480[24480] - Fix ILM alias not being created if `setup.ilm.check_exists: false` and `setup.ilm.overwrite: true` has been configured. {pull}24480[24480] From aedec9d9d2be4815b86613a57d853f13df150ef7 Mon Sep 17 00:00:00 2001 From: urso Date: Wed, 24 Mar 2021 17:21:53 +0100 Subject: [PATCH 6/9] fix ilm setup message formatting --- libbeat/idxmgmt/ilm/std.go | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/libbeat/idxmgmt/ilm/std.go b/libbeat/idxmgmt/ilm/std.go index 4fb3db739fe4..1f6a0151afcf 100644 --- a/libbeat/idxmgmt/ilm/std.go +++ b/libbeat/idxmgmt/ilm/std.go @@ -105,11 +105,12 @@ func (m *stdManager) CheckEnabled() (bool, error) { func (m *stdManager) EnsureAlias() error { log := m.log overwrite := m.Overwrite() + name := m.alias.Name var exists bool if m.checkExists && !overwrite { var err error - exists, err = m.client.HasAlias(m.alias.Name) + exists, err = m.client.HasAlias(name) if err != nil { return err } @@ -117,21 +118,21 @@ func (m *stdManager) EnsureAlias() error { switch { case exists && !overwrite: - log.Infof("Index Alias %v exists already", m.alias.Name) + log.Infof("Index Alias %v exists already.", name) return nil case !exists || overwrite: err := m.client.CreateAlias(m.alias) if err != nil { if ErrReason(err) != ErrAliasAlreadyExists { - log.Errorf("Index Alias %v setup failed: %v", m.alias.Name, err) + log.Errorf("Index Alias %v setup failed: %v.", name, err) return err } - log.Infof("Index Alias %v exists already", m.alias.Name) + log.Infof("Index Alias %v exists already.", name) return nil } - log.Info("Index Alias %v successfully created", m.alias.Name) + log.Infof("Index Alias %v successfully created.", name) return nil default: @@ -143,11 +144,12 @@ func (m *stdManager) EnsureAlias() error { func (m *stdManager) EnsurePolicy(overwrite bool) (bool, error) { log := m.log overwrite = overwrite || m.Overwrite() + name := m.policy.Name var exists bool if m.checkExists && !overwrite { var err error - exists, err = m.client.HasILMPolicy(m.policy.Name) + exists, err = m.client.HasILMPolicy(name) if err != nil { return false, err } @@ -155,21 +157,21 @@ func (m *stdManager) EnsurePolicy(overwrite bool) (bool, error) { switch { case exists && !overwrite: - log.Infof("ILM policy %v exists already", m.policy) + log.Infof("ILM policy %v exists already.", name) return false, nil case !exists || overwrite: err := m.client.CreateILMPolicy(m.policy) if err != nil { - log.Errorf("ILM policy %v creation failed: %v", m.policy, err) + log.Errorf("ILM policy %v creation failed: %v", name, err) return false, err } - log.Infof("ILM policy %v successfully created", m.policy) + log.Infof("ILM policy %v successfully created.", name) return true, err default: - log.Infof("ILM policy not created: exists=%v, overwrite=%v", exists, overwrite) + log.Infof("ILM policy not created: exists=%v, overwrite=%v.", exists, overwrite) return false, nil } } From b5d859535f2af53ab77dca42997267d0b07a324f Mon Sep 17 00:00:00 2001 From: urso Date: Wed, 24 Mar 2021 17:22:12 +0100 Subject: [PATCH 7/9] update ilm system tests --- libbeat/tests/system/test_ilm.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/libbeat/tests/system/test_ilm.py b/libbeat/tests/system/test_ilm.py index 2913893e11c2..3e238b8f2d04 100644 --- a/libbeat/tests/system/test_ilm.py +++ b/libbeat/tests/system/test_ilm.py @@ -3,6 +3,7 @@ import logging import os import pytest +import re import shutil import unittest @@ -286,12 +287,12 @@ def setUp(self): self.cmd = "ilm-policy" def assert_log_contains_policy(self): - assert self.log_contains('ILM policy successfully loaded.') + assert self.log_contains(re.compile('ILM policy .* successfully created.')) assert self.log_contains('"max_age": "30d"') assert self.log_contains('"max_size": "50gb"') def assert_log_contains_write_alias(self): - assert self.log_contains('Write alias successfully generated.') + assert self.log_contains(re.compile('Index Alias .* successfully created.')) def test_default(self): """ From ef43fd0aa86f1c45ef1795ebbec22f42cdbad3c2 Mon Sep 17 00:00:00 2001 From: urso Date: Wed, 24 Mar 2021 17:24:45 +0100 Subject: [PATCH 8/9] fix changelog #4 --- CHANGELOG.next.asciidoc | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index f55dc23bb3a8..76c9fd90a1c1 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -233,7 +233,6 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d - Add FAQ entry for madvdontneed variable {pull}23429[23429] - Fix panic due to unhandled DeletedFinalStateUnknown in k8s OnDelete {pull}23419[23419] - Fix error loop with runaway CPU use when the Kafka output encounters some connection errors {pull}23484[23484] -- Allow configuring credential_profile_name and shared_credential_file when using role_arn. {pull}24174[24174] - Fix ILM setup log reporting that a policy or an alias was created, even though the creation of any resource was disabled. {issue}24046[24046] {pull}24480[24480] - Fix ILM alias not being created if `setup.ilm.check_exists: false` and `setup.ilm.overwrite: true` has been configured. {pull}24480[24480] - Fix issue discovering docker containers and metadata after reconnections {pull}24318[24318] From becfee11e428c29f8a70518ba2aefb6b879aa556 Mon Sep 17 00:00:00 2001 From: urso Date: Wed, 24 Mar 2021 21:33:13 +0100 Subject: [PATCH 9/9] Fix system tests guarded by integration test --- libbeat/tests/system/test_ilm.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/libbeat/tests/system/test_ilm.py b/libbeat/tests/system/test_ilm.py index 3e238b8f2d04..12c79fad61d6 100644 --- a/libbeat/tests/system/test_ilm.py +++ b/libbeat/tests/system/test_ilm.py @@ -13,6 +13,9 @@ INTEGRATION_TESTS = os.environ.get('INTEGRATION_TESTS', False) +MSG_ILM_POLICY_LOADED = re.compile('ILM policy .* successfully created.') + + class TestRunILM(BaseTest): def setUp(self): @@ -47,7 +50,7 @@ def test_ilm_default(self): self.render_config() proc = self.start_beat() self.wait_until(lambda: self.log_contains("mockbeat start running.")) - self.wait_until(lambda: self.log_contains("ILM policy successfully loaded")) + self.wait_until(lambda: self.log_contains(MSG_ILM_POLICY_LOADED)) self.wait_until(lambda: self.log_contains("PublishEvents: 1 events have been published")) proc.check_kill_and_wait() @@ -85,7 +88,7 @@ def test_policy_name(self): proc = self.start_beat() self.wait_until(lambda: self.log_contains("mockbeat start running.")) - self.wait_until(lambda: self.log_contains("ILM policy successfully loaded")) + self.wait_until(lambda: self.log_contains(MSG_ILM_POLICY_LOADED)) self.wait_until(lambda: self.log_contains("PublishEvents: 1 events have been published")) proc.check_kill_and_wait() @@ -104,7 +107,7 @@ def test_rollover_alias(self): proc = self.start_beat() self.wait_until(lambda: self.log_contains("mockbeat start running.")) - self.wait_until(lambda: self.log_contains("ILM policy successfully loaded")) + self.wait_until(lambda: self.log_contains(MSG_ILM_POLICY_LOADED)) self.wait_until(lambda: self.log_contains("PublishEvents: 1 events have been published")) proc.check_kill_and_wait() @@ -124,7 +127,7 @@ def test_pattern(self): proc = self.start_beat() self.wait_until(lambda: self.log_contains("mockbeat start running.")) - self.wait_until(lambda: self.log_contains("ILM policy successfully loaded")) + self.wait_until(lambda: self.log_contains(MSG_ILM_POLICY_LOADED)) self.wait_until(lambda: self.log_contains("PublishEvents: 1 events have been published")) proc.check_kill_and_wait() @@ -144,7 +147,7 @@ def test_pattern_date(self): proc = self.start_beat() self.wait_until(lambda: self.log_contains("mockbeat start running.")) - self.wait_until(lambda: self.log_contains("ILM policy successfully loaded")) + self.wait_until(lambda: self.log_contains(MSG_ILM_POLICY_LOADED)) self.wait_until(lambda: self.log_contains("PublishEvents: 1 events have been published")) proc.check_kill_and_wait() @@ -287,7 +290,7 @@ def setUp(self): self.cmd = "ilm-policy" def assert_log_contains_policy(self): - assert self.log_contains(re.compile('ILM policy .* successfully created.')) + assert self.log_contains(MSG_ILM_POLICY_LOADED) assert self.log_contains('"max_age": "30d"') assert self.log_contains('"max_size": "50gb"')