Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove auto option from setup.ilm.enabled and set the default value to true #28671

Merged
merged 4 commits into from
Oct 29, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.next.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d
- Enable IMDSv2 support for `add_cloud_metadata` processor on AWS. {issue}22101[22101] {pull}28285[28285]
- Update kubernetes.namespace from keyword to group field and add name, labels, annotations, uuid as its fields {pull}27917[27917]
- Previously, RE2 and thus Golang had a bug where `(|a)*` matched more characters than `(|a)+`. To stay consistent with PCRE, the bug was fixed. Configurations that rely on the old, buggy behaviour has to be adjusted. See more about Golang bug: https://github.com/golang/go/issues/46123 {pull}27543[27543]
- Remove `auto` from the available options of `setup.ilm.enabled` and set the default value to `true`. {pull}28671[28671]

*Auditbeat*

Expand Down
7 changes: 2 additions & 5 deletions libbeat/docs/shared-ilm.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ Example configuration:

["source","yaml",subs="attributes"]
----
setup.ilm.enabled: auto
setup.ilm.enabled: true
setup.ilm.rollover_alias: "{beatname_lc}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a follow up: As we only have data streams in 8.0, the rollover_alias config can be removed. Deprecate in 7.x?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

setup.ilm.pattern: "{now/d}-000001" <1>
----
Expand All @@ -47,10 +47,7 @@ You can specify the following settings in the `setup.ilm` section of the
==== `setup.ilm.enabled`

Enables or disables index lifecycle management on any new indices created by
{beatname_uc}. Valid values are `true`, `false`, and `auto`. When `auto` (the
default) is specified on version 7.0 and later, {beatname_uc} automatically uses
index lifecycle management if the feature is enabled in {es} and has the
required license; otherwise, {beatname_uc} creates daily indices.
{beatname_uc}. Valid values are `true` and `false`.

[float]
[[setup-ilm-rollover_alias-option]]
Expand Down
52 changes: 16 additions & 36 deletions libbeat/idxmgmt/ilm/client_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import (

// ClientHandler defines the interface between a remote service and the Manager.
type ClientHandler interface {
CheckILMEnabled(Mode) (bool, error)
CheckILMEnabled(bool) (bool, error)

HasAlias(name string) (bool, error)
CreateAlias(alias Alias) error
Expand Down Expand Up @@ -92,40 +92,29 @@ func NewFileClientHandler(c FileClient) *FileClientHandler {
}

// CheckILMEnabled indicates whether or not ILM is supported for the configured mode and ES instance.
func (h *ESClientHandler) CheckILMEnabled(mode Mode) (bool, error) {
if mode == ModeDisabled {
func (h *ESClientHandler) CheckILMEnabled(enabled bool) (bool, error) {
if !enabled {
return false, nil
}

avail, probe := checkILMVersion(mode, h.client.GetVersion())
if !avail {
if mode == ModeEnabled {
ver := h.client.GetVersion()
return false, errf(ErrESVersionNotSupported,
"Elasticsearch %v does not support ILM", ver.String())
}
return false, nil
}

if !probe {
// version potentially supports ILM, but mode + version indicates that we
// want to disable ILM support.
return false, nil
ver := h.client.GetVersion()
ruflin marked this conversation as resolved.
Show resolved Hide resolved
if !checkILMVersion(h.client.GetVersion()) {
return false, errf(ErrESVersionNotSupported, "Elasticsearch %v does not support ILM", ver.String())
}

avail, enabled, err := h.checkILMSupport()
avail, enabledILM, err := h.checkILMSupport()
if err != nil {
return false, err
}

if !avail {
if mode == ModeEnabled {
if enabledILM {
return false, errOf(ErrESVersionNotSupported)
}
return false, nil
}

if !enabled && mode == ModeEnabled {
if !enabledILM && enabled {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if enabled == false we should never get to this line of code because of line 96?

return false, errOf(ErrESILMDisabled)
}
return enabled, nil
Expand Down Expand Up @@ -244,18 +233,14 @@ func (h *ESClientHandler) queryFeatures(to interface{}) (int, error) {
}

// CheckILMEnabled indicates whether or not ILM is supported for the configured mode and client version.
func (h *FileClientHandler) CheckILMEnabled(mode Mode) (bool, error) {
if mode == ModeDisabled {
return false, nil
}
avail, probe := checkILMVersion(mode, h.client.GetVersion())
if avail {
return probe, nil
}
if mode != ModeEnabled {
func (h *FileClientHandler) CheckILMEnabled(enabled bool) (bool, error) {
if !enabled {
return false, nil
}
version := h.client.GetVersion()
if checkILMVersion(version) {
return enabled, nil
}
return false, errf(ErrESVersionNotSupported,
"Elasticsearch %v does not support ILM", version.String())
}
Expand Down Expand Up @@ -287,11 +272,6 @@ func (h *FileClientHandler) HasAlias(name string) (bool, error) {
// avail: indicates whether version supports ILM
// probe: in case version potentially supports ILM, check the combination of mode + version
// to indicate whether or not ILM support should be enabled or disabled
func checkILMVersion(mode Mode, ver common.Version) (avail, probe bool) {
avail = !ver.LessThan(esMinILMVersion)
if avail {
probe = (mode == ModeEnabled) ||
(mode == ModeAuto && !ver.LessThan(esMinDefaultILMVersion))
}
return avail, probe
func checkILMVersion(ver common.Version) (avail bool) {
return !ver.LessThan(esMinILMVersion)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just checked in the code what esMinILMVersion and it is 6.6.0, so we will also be able to get rid of this code 🥳

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So let's clean this up 👍🏼

}
42 changes: 13 additions & 29 deletions libbeat/idxmgmt/ilm/client_handler_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,21 +48,14 @@ const (
func TestESClientHandler_CheckILMEnabled(t *testing.T) {
t.Run("no ilm if disabled", func(t *testing.T) {
h := newESClientHandler(t)
b, err := h.CheckILMEnabled(ilm.ModeDisabled)
b, err := h.CheckILMEnabled(false)
assert.NoError(t, err)
assert.False(t, b)
})

t.Run("with ilm if auto", func(t *testing.T) {
h := newESClientHandler(t)
b, err := h.CheckILMEnabled(ilm.ModeAuto)
assert.NoError(t, err)
assert.True(t, b)
})

t.Run("with ilm if enabled", func(t *testing.T) {
h := newESClientHandler(t)
b, err := h.CheckILMEnabled(ilm.ModeEnabled)
b, err := h.CheckILMEnabled(true)
assert.NoError(t, err)
assert.True(t, b)
})
Expand Down Expand Up @@ -268,38 +261,29 @@ func getEnv(name, def string) string {

func TestFileClientHandler_CheckILMEnabled(t *testing.T) {
for name, test := range map[string]struct {
m ilm.Mode
version string
enabled bool
err bool
enabled bool
version string
ilmEnabled bool
err bool
}{
"ilm enabled": {
m: ilm.ModeEnabled,
enabled: true,
},
"ilm auto": {
m: ilm.ModeAuto,
enabled: true,
enabled: true,
ilmEnabled: true,
},
"ilm disabled": {
m: ilm.ModeDisabled,
enabled: false,
enabled: false,
ilmEnabled: false,
},
"ilm enabled, version too old": {
m: ilm.ModeEnabled,
enabled: true,
version: "5.0.0",
err: true,
},
"ilm auto, version too old": {
m: ilm.ModeAuto,
version: "5.0.0",
enabled: false,
},
} {
t.Run(name, func(t *testing.T) {
h := ilm.NewFileClientHandler(newMockClient(test.version))
b, err := h.CheckILMEnabled(test.m)
assert.Equal(t, test.enabled, b)
b, err := h.CheckILMEnabled(test.enabled)
assert.Equal(t, test.ilmEnabled, b)
if test.err {
assert.Error(t, err)
} else {
Expand Down
44 changes: 3 additions & 41 deletions libbeat/idxmgmt/ilm/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ package ilm

import (
"fmt"
"strconv"
"strings"

"github.com/elastic/beats/v7/libbeat/beat"
"github.com/elastic/beats/v7/libbeat/common"
Expand All @@ -29,7 +27,7 @@ import (

// Config is used for unpacking a common.Config.
type Config struct {
Mode Mode `config:"enabled"`
Enabled bool `config:"enabled"`
PolicyName fmtstr.EventFormatString `config:"policy_name"`
PolicyFile string `config:"policy_file"`
RolloverAlias fmtstr.EventFormatString `config:"rollover_alias"`
Expand All @@ -44,20 +42,6 @@ type Config struct {
Overwrite bool `config:"overwrite"`
}

//Mode is used for enumerating the ilm mode.
type Mode uint8

const (
//ModeAuto enum 'auto'
ModeAuto Mode = iota

//ModeEnabled enum 'true'
ModeEnabled

//ModeDisabled enum 'false'
ModeDisabled
)

const ilmDefaultPattern = "{now/d}-000001"

// DefaultPolicy defines the default policy to be used if no custom policy is
Expand All @@ -79,31 +63,9 @@ var DefaultPolicy = common.MapStr{
},
}

//Unpack creates enumeration value true, false or auto
func (m *Mode) Unpack(in string) error {
in = strings.ToLower(in)

if in == "auto" {
*m = ModeAuto
return nil
}

b, err := strconv.ParseBool(in)
if err != nil {
return fmt.Errorf("ilm.enabled` mode '%v' is invalid (try auto, true, false)", in)
}

if b {
*m = ModeEnabled
} else {
*m = ModeDisabled
}
return nil
}

//Validate verifies that expected config options are given and valid
func (cfg *Config) Validate() error {
if cfg.RolloverAlias.IsEmpty() && cfg.Mode != ModeDisabled {
if cfg.RolloverAlias.IsEmpty() && cfg.Enabled {
return fmt.Errorf("rollover_alias must be set when ILM is not disabled")
}
return nil
Expand All @@ -115,7 +77,7 @@ func defaultConfig(info beat.Info) Config {
policyFmt := fmtstr.MustCompileEvent(info.Beat)

return Config{
Mode: ModeAuto,
Enabled: true,
PolicyName: *policyFmt,
RolloverAlias: *aliasFmt,
Pattern: ilmDefaultPattern,
Expand Down
6 changes: 3 additions & 3 deletions libbeat/idxmgmt/ilm/ilm.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ type SupportFactory func(*logp.Logger, beat.Info, *common.Config) (Supporter, er
// write alias a manager instance must be generated.
type Supporter interface {
// Query settings
Mode() Mode
Enabled() bool
ruflin marked this conversation as resolved.
Show resolved Hide resolved
Alias() Alias
Policy() Policy
Overwrite() bool
Expand Down Expand Up @@ -82,7 +82,7 @@ func DefaultSupport(log *logp.Logger, info beat.Info, config *common.Config) (Su
}
}

if cfg.Mode == ModeDisabled {
if !cfg.Enabled {
return NewNoopSupport(info, config)
}

Expand Down Expand Up @@ -137,7 +137,7 @@ func StdSupport(log *logp.Logger, info beat.Info, config *common.Config) (Suppor
policy.Body = body
}

return NewStdSupport(log, cfg.Mode, alias, policy, cfg.Overwrite, cfg.CheckExists), nil
return NewStdSupport(log, cfg.Enabled, alias, policy, cfg.Overwrite, cfg.CheckExists), nil
}

// NoopSupport configures a new noop ILM support implementation,
Expand Down
Loading