Add ILM support to Beats #7963
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| // Licensed to Elasticsearch B.V. under one or more contributor | ||
| // license agreements. See the NOTICE file distributed with | ||
| // this work for additional information regarding copyright | ||
| // ownership. Elasticsearch B.V. licenses this file to you under | ||
| // the Apache License, Version 2.0 (the "License"); you may | ||
| // not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, | ||
| // software distributed under the License is distributed on an | ||
| // "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
| // KIND, either express or implied. See the License for the | ||
| // specific language governing permissions and limitations | ||
| // under the License. | ||
|
|
||
| package export | ||
|
|
||
| import ( | ||
| "fmt" | ||
|
|
||
| "github.com/spf13/cobra" | ||
|
|
||
| "github.com/elastic/beats/libbeat/cmd/instance" | ||
| ) | ||
|
|
||
| // GenGetILMPolicyCmd is the command used to export the ilm policy. | ||
| func GenGetILMPolicyCmd() *cobra.Command { | ||
| genTemplateConfigCmd := &cobra.Command{ | ||
| Use: "ilm-policy", | ||
| Short: "Export ILM policy", | ||
| Run: func(cmd *cobra.Command, args []string) { | ||
| fmt.Println(instance.ILMPolicy.StringToPrint()) | ||
| }, | ||
| } | ||
|
|
||
| return genTemplateConfigCmd | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -105,6 +105,9 @@ type beatConfig struct { | |
| Dashboards *common.Config `config:"setup.dashboards"` | ||
| Template *common.Config `config:"setup.template"` | ||
| Kibana *common.Config `config:"setup.kibana"` | ||
|
|
||
| // ILM Config options | ||
| ILM *common.Config `config:"output.elasticsearch.ilm"` | ||
| } | ||
|
|
||
| var ( | ||
|
|
@@ -430,7 +433,7 @@ func (b *Beat) TestConfig(bt beat.Creator) error { | |
| } | ||
|
|
||
| // Setup registers ES index template, kibana dashboards, ml jobs and pipelines. | ||
| func (b *Beat) Setup(bt beat.Creator, template, setupDashboards, machineLearning, pipelines, policy bool) error { | ||
| return handleError(func() error { | ||
| err := b.Init() | ||
| if err != nil { | ||
|
|
@@ -509,6 +512,13 @@ func (b *Beat) Setup(bt beat.Creator, template, setupDashboards, machineLearning | |
| fmt.Println("Loaded Ingest pipelines") | ||
| } | ||
|
|
||
| if policy { | ||
| if err := b.loadILMPolicy(); err != nil { | ||
| return err | ||
| } | ||
| fmt.Println("Loaded Index Lifecycle Management (ILM) policy") | ||
| } | ||
|
|
||
| return nil | ||
| }()) | ||
| } | ||
|
|
@@ -719,11 +729,11 @@ func (b *Beat) loadDashboards(ctx context.Context, force bool) error { | |
| // the elasticsearch output. It is important the the registration happens before | ||
| // the publisher is created. | ||
| func (b *Beat) registerTemplateLoading() error { | ||
| var templateCfg template.TemplateConfig | ||
|
|
||
| // Check if outputting to file is enabled, and output to file if it is | ||
| if b.Config.Template.Enabled() { | ||
| err := b.Config.Template.Unpack(&templateCfg) | ||
| if err != nil { | ||
| return fmt.Errorf("unpacking template config fails: %v", err) | ||
| } | ||
|
|
@@ -741,8 +751,82 @@ func (b *Beat) registerTemplateLoading() error { | |
| return err | ||
| } | ||
|
|
||
| if esCfg.Index != "" && | ||
| (templateCfg.Name == "" || templateCfg.Pattern == "") && | ||
| (b.Config.Template == nil || b.Config.Template.Enabled()) { | ||
| return errors.New("setup.template.name and setup.template.pattern have to be set if index name is modified") | ||
| } | ||
|
|
||
| if b.Config.ILM.Enabled() { | ||
| cfgwarn.Beta("Index lifecycle management is enabled which is in beta.") | ||
|
|
||
| ilmCfg, err := getILMConfig(b) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| // In case no template settings are set, config must be created | ||
| if b.Config.Template == nil { | ||
| b.Config.Template = common.NewConfig() | ||
| } | ||
| // Template name and pattern can't be configure when using ILM | ||
| logp.Info("Set setup.template.name to '%s' as ILM is enabled.", ilmCfg.RolloverAlias) | ||
| err = b.Config.Template.SetString("name", -1, ilmCfg.RolloverAlias) | ||
| if err != nil { | ||
| return errw.Wrap(err, "error setting setup.template.name") | ||
| } | ||
| pattern := fmt.Sprintf("%s-*", ilmCfg.RolloverAlias) | ||
| logp.Info("Set setup.template.pattern to '%s' as ILM is enabled.", pattern) | ||
| err = b.Config.Template.SetString("pattern", -1, pattern) | ||
| if err != nil { | ||
| return errw.Wrap(err, "error setting setup.template.pattern") | ||
| } | ||
|
|
||
| // rollover_alias and lifecycle.name can't be configured and will be overwritten | ||
| logp.Info("Set settings.index.lifecycle.rollover_alias in template to %s as ILM is enabled.", ilmCfg.RolloverAlias) | ||
| err = b.Config.Template.SetString("settings.index.lifecycle.rollover_alias", -1, ilmCfg.RolloverAlias) | ||
| if err != nil { | ||
| return errw.Wrap(err, "error setting settings.index.lifecycle.rollover_alias") | ||
| } | ||
| logp.Info("Set settings.index.lifecycle.name in template to %s as ILM is enabled.", ILMPolicyName) | ||
| err = b.Config.Template.SetString("settings.index.lifecycle.name", -1, ILMPolicyName) | ||
| if err != nil { | ||
| return errw.Wrap(err, "error setting settings.index.lifecycle.name") | ||
| } | ||
|
There was a problem hiding this comment. Why are there two different configuration settings ( There was a problem hiding this comment. The values you see above are put into the index template. These "could" be set through Perhaps I miss something above as I didn't fully understand the question. |
||
|
|
||
| // Set the ingestion index to the rollover alias | ||
| logp.Info("Set output.elasticsearch.index to '%s' as ILM is enabled.", ilmCfg.RolloverAlias) | ||
| esCfg.Index = ilmCfg.RolloverAlias | ||
|
There was a problem hiding this comment. What if There was a problem hiding this comment. It's an issue that bugs me and I don't have good answer for it yet. The problem: Assuming we allow index alias to be based on fields of events like we do at the moment for indices (for example APM ...), how do we know when to create an index alias? It would mean we create multiple index alias but not at startup as we don't know yet on startup. At the same time we need to know if an index alias already exist to make sure we create it before the first event is created. We could keep some state on the Beats side to figure out which aliases are already created and exist. An alternative could be to disable automatic index creation: https://www.elastic.co/guide/en/elasticsearch/reference/current/docs-index_.html#index-creation But I think this would require a config change. There are some discussions to allow this on a cluster level. Assuming this would be enabled, we would get an error back and could create the index write alias. Other ideas / thoughts? To keep complexity low I would also be ok to ship a first version of ILM without this capability as it could be manually configured the right way assuming something creates the correct write aliases in advance. In the APM case it's 3 predefined ones, so it could even be hardcoded. There was a problem hiding this comment. We will need to be able to use multiple indices for APM Server. I am fine with dealing with that in a separate PR though. There was a problem hiding this comment. This is a long standing issue. Settings in outputs allow for multiple indices, but setup is focused on one index only. I don't think we can't fix it here. Ultimately I would like to combine index selection with index/template/ilm setup. Right now a many settings are all over the place + must be configured appropriately. There was a problem hiding this comment. I agree with @urso, we do not have a good story about setup and multiples indices, before we always focused into having one indice per beat type, but in the real world it's not completely true since users can make the indice dynamic. For the risk of not making everyone happy, for me having a global It is certainly a part of a bigger discussion that we need to have, but I think one thing that confused me is that we do a lot of coupling from different parts of beats to the Elasticsearch output, we should instead try to decouple and encapsulate specific logic that we have. For me ILM, Ingest pipelines and Template are specific to ES output and should be be scope in the elasticsearch package / output. I would like to see that the ES outputs config contains the information about the template, ILM and we have some kind of I think I will drop my ideas in a doc and share it. |
||
| err = b.Config.Output.Config().SetString("index", -1, ilmCfg.RolloverAlias) | ||
| if err != nil { | ||
| return errw.Wrap(err, "error setting output.elasticsearch.index") | ||
| } | ||
|
|
||
| writeAliasCallback, err := b.writeAliasLoadingCallback() | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| // Load write alias already on | ||
| esConfig := b.Config.Output.Config() | ||
|
|
||
| // Check that ILM is enabled and the right elasticsearch version exists | ||
| esClient, err := elasticsearch.NewConnectedClient(esConfig) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| err = checkElasticsearchVersionIlm(esClient) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| err = checkILMFeatureEnabled(esClient) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| elasticsearch.RegisterConnectCallback(writeAliasCallback) | ||
| } | ||
|
|
||
| if b.Config.Template == nil || (b.Config.Template != nil && b.Config.Template.Enabled()) { | ||
|
|
@@ -754,6 +838,8 @@ func (b *Beat) registerTemplateLoading() error { | |
| return err | ||
| } | ||
| elasticsearch.RegisterConnectCallback(callback) | ||
| } else if b.Config.ILM.Enabled() { | ||
| return errors.New("templates cannot be disable when using ILM") | ||
|
There was a problem hiding this comment. Why throwing an error here instead of also overwriting the There was a problem hiding this comment. The difference between the two for me is that the template settings I must overwrite / set to create a valid template. In most cases I do not expect users that they modified the template name / pattern but I still need to overwrite it. If the template is disabled, the user modified it on purpose and I rather abort and tell the user to fix it. Based on the above it could be argued that I should check if the defaults for modified for the name or pattern. But it can't really made a difference if the default value applies or if it was uncommented. This also applies to your above comment that I should only error out of ILM is not enabled. I rather have users that made special configs disable these configs first and then start using ILM. |
||
| } | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we create a Setting or Config type instead of adding a new argument? I would prefer if we would stabilize this interface?
Maybe just add your policy to this new type and create a new issue to refactor the other arguments into it.
Or another way would be to accept a variadic arguments which would be a function operating on a config object.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree that we should generalise this but not as part of this PR as I want to keep the scope of it as small as possible. It will also be backported to 6.x
Issue for refactoring can be found here: #9342
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, to not do a scope creep here, I would have still preferred to move to a struct because we actually break the developer contract in 6.x by adding a new parameter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I took another look, we don't use this method outside of libbeat. So I am OK to break it.