Refactor index management #10347

urso · 2019-01-26T01:33:26Z

Changes:

introduce Index + ILM manager interfaces responsible for:
- create ILM policy
- create write alias
- create and load template
- separate interfaces for re-use: manager, apihandler
- provide default API handler for elasticsearch.Client
Index + ILM support interfaces, that can be swapped out with custom implementation (still WIP)
- Beat implementing custom managers have to decide on config file format
ILM settings have been moved to ilm.setup
Introduce setup.ilm.enabled: auto (new default)
- Index manager will detect on connect to Elasticsearch if ILM can be enabled or not
- Lazily chooses write alias or old-style index for publishing
- Index Manager does not check ILM availabilty on startup, but on first connect (so to not block beats startup)
- If Elasticsearch < 7.0, then auto defaults to false
- If Elasticsearch > 7.0, then auto checks if ILM is available and enbaled
- If setup.ilm.enabled: true, then fail check if version does not support ILM or ILM is disabled
Publisher pipeline does not use output package anymore to create an output, but a callback -> Beat instance itself creates output instances, passing IndexManager and other info to the output loader.
Outputs can use IndexManager in order to create the per event index selector
remove fragile manipulation of common.Config objects, that have been already partially parsed.
Ensure ILM can be setup if setup.ilm.enabled: true, but elasticsearch output is configured via Central Management

Followup: update docs

libbeat/publisher/pipeline/pipeline.go

libbeat/publisher/pipeline/module.go

libbeat/outputs/output_reg.go

libbeat/ilm/single.go

libbeat/ilm/eshandler.go

libbeat/ilm/error.go

libbeat/idxmgmt/ilm/single.go

libbeat/idxmgmt/ilm/noop.go

libbeat/idxmgmt/ilm/ilm.go

libbeat/idxmgmt/ilm/error.go

libbeat/idxmgmt/ilm/config.go

libbeat/common/reload/reload.go

libbeat/idxmgmt/ilm/single.go

libbeat/idxmgmt/ilm/ilm.go

libbeat/idxmgmt/idxmgmt.go

libbeat/idxmgmt/ilm/eshandler.go

libbeat/idxmgmt/idxmgmt.go

libbeat/idxmgmt/ilm/ilm.go

libbeat/idxmgmt/idxmgmt.go

libbeat/idxmgmt/assets.go

ph

@urso really good PR, I really like how you actually encapsulated all the logic inside the idxmgmt package, I found two things.

There was a problem with the integration suite with your code in the libbeat

command [go test -tags integration -cover -coverprofile /tmp/gotestcover-226034385 github.com/elastic/beats/libbeat/idxmgmt/ilm]: exit status 1
# github.com/elastic/beats/libbeat/idxmgmt/ilm
idxmgmt/ilm/eshandler_integration_test.go:28:2: cannot find package "github.com/satori/go.uuid" in any of:
	/go/src/github.com/elastic/beats/vendor/github.com/satori/go.uuid (vendor tree)
	/usr/local/go/src/github.com/satori/go.uuid (from $GOROOT)
	/go/src/github.com/satori/go.uuid (from $GOPATH)
FAIL	github.com/elastic/beats/libbeat/idxmgmt/ilm [setup failed]

I will fix the issue when the docker-compose logs is shallowing the previous error.

CHANGELOG-developer.next.asciidoc

auditbeat/auditbeat.reference.yml

libbeat/cmd/export/ilm_policy.go

libbeat/idxmgmt/ilm/eshandler.go

urso · 2019-01-29T09:48:18Z

There was a problem with the integration suite with your code in the libbeat

goimports did select the wrong package :(. I fixed the import (and removed old dependency from my machine). Beats-CI and travis being green is somewhat worrysome.

kvch · 2019-01-29T09:55:34Z

libbeat/idxmgmt/ilm/error.go

@@ -0,0 +1,94 @@
+// Licensed to Elasticsearch B.V. under one or more contributor


I have seen similar functionality in go-txfile. It is definitely a better approach for error handling compared to what Golang provides by default. Do you have any plans to consolidate error handling? E.g moving the functions to libbeat and leaving the error variables here.

We also have some similar functionality in go-ucfg.

Do you have any plans to consolidate error handling?

Yes. See: https://github.com/urso/ecslog/tree/master/errx

kvch · 2019-01-29T10:11:31Z

libbeat/idxmgmt/ilm/eshandler.go

+	return status == 200, nil
+}
+
+func (h *esClientHandler) HasAlias(name string) (bool, error) {


I am looking forward to having an Elasticseach Golang client. I had to implement the same functions in the migration tool.

ruflin · 2019-01-29T12:22:31Z

CHANGELOG.next.asciidoc

@@ -21,6 +21,8 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d
 - Rename `process.exe` to `process.executable` in add_process_metadata to align with ECS. {pull}9949[9949]
 - Import ECS change https://github.com/elastic/ecs/pull/308[ecs#308]:
  leaf field `user.group` is now the `group` field set. {pull}10275[10275]
+- Move output.elasticsearch.ilm settings to setup.ilm. {pull}10347[10347]
+- ILM will be available by default if Elasticsearch > 7.0 is used. {pull}10347[10347]


What happens for the OSS users? Should this be >= 7?

I see, auto is default and not true ... I assume this does the trick.

yep, auto does the trick.

filebeat/tests/system/config/filebeat.yml.j2

ph · 2019-01-29T17:11:06Z

goimports did select the wrong package :(. I fixed the import (and removed old dependency from my machine). Beats-CI and travis being green is somewhat worrysome.

I have fixed that in #10380

ph · 2019-01-29T17:15:43Z

libbeat/idxmgmt/ilm/ilm.go

+type Alias struct {
+	Name    string
+	Pattern string
+}


urso · 2019-01-31T11:54:20Z

Jenkins, please test this.

urso · 2019-01-31T11:54:45Z

Jenkins, test this.

ph · 2019-01-31T13:53:35Z

The failures are not related to this change.

simitt · 2019-01-31T14:14:52Z

Thanks for putting this PR up @urso! First experiences trying this out showed that the integration with the new supporters is pretty smooth!

Changes: - introduce Index + ILM manager interfaces responsible for: - create ILM policy - create write alias - create and load template - separate interfaces for re-use: manager, apihandler - provide default API handler for elasticsearch.Client - Index + ILM support interfaces, that can be swapped out with custom implementation (still WIP) - Beat implementing custom managers have to decide on config file format - ILM settings have been moved to `ilm.setup` - Introduce `setup.ilm.enabled: auto` (new default) - Index manager will detect on connect to Elasticsearch if ILM can be enabled or not - Lazily chooses write alias or old-style index for publishing - Index Manager does not check ILM availabilty on startup, but on first connect (so to not block beats startup) - If Elasticsearch < 7.0, then `auto` defaults to false - If Elasticsearch > 7.0, then `auto` checks if ILM is available and enbaled - If `setup.ilm.enabled: true`, then fail check if version does not support ILM or ILM is disabled - Publisher pipeline does not use output package anymore to create an output, but a callback -> Beat instance itself creates output instances, passing IndexManager and other info to the output loader. - Outputs can use IndexManager in order to create the per event index selector - remove fragile manipulation of `common.Config` objects, that have been already partially parsed. - Ensure ILM can be setup if `setup.ilm.enabled: true`, but elasticsearch output is configured via Central Management Followup: update docs

urso requested a review from a team as a code owner January 26, 2019 01:33

houndci-bot reviewed Jan 26, 2019

View reviewed changes

urso requested a review from ph January 26, 2019 01:33

urso added the in progress Pull request is currently in progress. label Jan 26, 2019

houndci-bot reviewed Jan 26, 2019

View reviewed changes

libbeat/idxmgmt/ilm/eshandler.go Show resolved Hide resolved

libbeat/idxmgmt/ilm/eshandler.go Show resolved Hide resolved

libbeat/idxmgmt/idxmgmt.go Show resolved Hide resolved

houndci-bot reviewed Jan 26, 2019

View reviewed changes

libbeat/idxmgmt/ilm/ilm.go Show resolved Hide resolved

libbeat/idxmgmt/ilm/ilm.go Show resolved Hide resolved

libbeat/idxmgmt/idxmgmt.go Show resolved Hide resolved

urso force-pushed the refactor-index-management branch from a95cabd to 508e80a Compare January 27, 2019 16:13

houndci-bot reviewed Jan 28, 2019

View reviewed changes

libbeat/idxmgmt/assets.go Show resolved Hide resolved

urso requested review from a team as code owners January 28, 2019 06:59

urso removed request for a team January 28, 2019 07:17

urso added review libbeat and removed in progress Pull request is currently in progress. labels Jan 28, 2019

urso changed the title ~~[WIP] Refactor index management~~ Refactor index management Jan 28, 2019

ph approved these changes Jan 28, 2019

View reviewed changes

urso force-pushed the refactor-index-management branch from 1862a0f to 92a138b Compare January 29, 2019 09:52

kvch reviewed Jan 29, 2019

View reviewed changes

urso force-pushed the refactor-index-management branch from 92a138b to 004832d Compare January 29, 2019 09:57

kvch reviewed Jan 29, 2019

View reviewed changes

ruflin reviewed Jan 29, 2019

View reviewed changes

filebeat/tests/system/config/filebeat.yml.j2 Outdated Show resolved Hide resolved

simitt mentioned this pull request Jan 29, 2019

Disable ILM usage on APM elastic/apm-server#1852

Closed

urso force-pushed the refactor-index-management branch from 12cb23e to a15d913 Compare January 29, 2019 15:33

ph reviewed Jan 29, 2019

View reviewed changes

libbeat/idxmgmt/ilm/ilm.go

type Alias struct {

Name string

Pattern string

}

Copy link

Contributor

ph Jan 29, 2019

Choose a reason for hiding this comment

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

<3 ^

urso added 21 commits January 31, 2019 01:11

fix typo in setting name

f35e884

return error if ilm is required, but not available

1e13373

ilm package unit tests

20ffaa6

rename

01a1968

godoc

091c786

with withILM parameter

6da146b

mock ilm support for idxmgmt testing

208d8d6

init idxmgmt test

fd750df

fix goimports version

42dc1e3

Test event index selection

b8869ab

changelog

dbf2acc

review

e752dec

Replace TemplateSettings with Alias and Policy types

b57cc45

fix rebase

3eced98

rename setup.ilm.name to setup.ilm.policy_name

2da4f67

move template + es output config check to index supporter

2760ab0

pass default index name to BuildSelector

ff3f66e

Update tests

b7c0aa7

Move default index name creation to indexSupport

5dbe4c1

ups

99cba1d

fix rebase

8c29608

urso force-pushed the refactor-index-management branch from 7e51210 to 8c29608 Compare January 31, 2019 00:12

urso merged commit 2bbf828 into elastic:master Jan 31, 2019

urso deleted the refactor-index-management branch January 31, 2019 14:49

urso mentioned this pull request Feb 26, 2019

Enable ILM by default in 7.0 #9521

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor index management #10347

Refactor index management #10347

urso commented Jan 26, 2019 •

edited

Loading

ph left a comment

urso commented Jan 29, 2019

kvch Jan 29, 2019

urso Jan 29, 2019

kvch Jan 29, 2019

ruflin Jan 29, 2019

ruflin Jan 29, 2019

urso Jan 29, 2019

ph commented Jan 29, 2019

ph Jan 29, 2019

urso commented Jan 31, 2019

urso commented Jan 31, 2019

ph commented Jan 31, 2019

simitt commented Jan 31, 2019

		@@ -0,0 +1,94 @@
		// Licensed to Elasticsearch B.V. under one or more contributor

Refactor index management #10347

Refactor index management #10347

Conversation

urso commented Jan 26, 2019 • edited Loading

ph left a comment

Choose a reason for hiding this comment

urso commented Jan 29, 2019

kvch Jan 29, 2019

Choose a reason for hiding this comment

urso Jan 29, 2019

Choose a reason for hiding this comment

kvch Jan 29, 2019

Choose a reason for hiding this comment

ruflin Jan 29, 2019

Choose a reason for hiding this comment

ruflin Jan 29, 2019

Choose a reason for hiding this comment

urso Jan 29, 2019

Choose a reason for hiding this comment

ph commented Jan 29, 2019

ph Jan 29, 2019

Choose a reason for hiding this comment

urso commented Jan 31, 2019

urso commented Jan 31, 2019

ph commented Jan 31, 2019

simitt commented Jan 31, 2019

urso commented Jan 26, 2019 •

edited

Loading