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

Refactor export and idxmgmt handling. #11777

Merged
merged 13 commits into from
Apr 23, 2019
Merged

Refactor export and idxmgmt handling. #11777

merged 13 commits into from
Apr 23, 2019

Conversation

simitt
Copy link
Contributor

@simitt simitt commented Apr 12, 2019

At the moment one idxmgmt.Supporter and one ilm.Supporter can be defined for one beat, with the following interfaces:

idxmgmt.Supporter:

// Supporter provides index management and configuration related services
// throughout libbeat.
// The BuildSelector is used by the output to create an IndexSelector. The
// index selector will report the per event index name to be used.
// A manager instantiated via Supporter is responsible for instantiating/configuring
// the index throughout the Elastic Stack.
type Supporter interface {
	// Enalbed checks if index management is configured to configure templates,
	// ILM, or aliases.
	Enabled() bool

	// ILM provides access to the configured ILM support.
	ILM() ilm.Supporter

	// TemplateConfig returns the template configuration used by the index supporter.
	TemplateConfig(withILM bool) (template.TemplateConfig, error)

	// BuildSelector create an index selector.
	// The defaultIndex string is interpreted as format string. It is used
	// as default index if the configuration provided does not define an index or
	// has no default fallback if all indices are guarded by conditionals.
	BuildSelector(cfg *common.Config) (outputs.IndexSelector, error)

	// Manager creates a new manager that can be used to execute the required steps
	// for initializing an index, ILM policies, and write aliases.
	Manager(client ESClient, assets Asseter) Manager
}

ilm.Supporter and ilm.Manager:

// Supporter implements ILM support. For loading the policies and creating
// write alias a manager instance must be generated.
type Supporter interface {
	Mode() Mode
	Alias() Alias
	Policy() Policy
	Manager(h APIHandler) Manager
}

// Manager uses an APIHandler to install a policy.
type Manager interface {
	Enabled() (bool, error)

	EnsureAlias() 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)
}

Using these interfaces as is does not allow for having multiple templates nor multiple ILM policies for one beat. This is a requirement for APM though.

During some discussion with @urso we agreed on moving forward by allowing for having multiple ILM supporter per idxmgmt supporter, and abstracting away the template handling. This can be achieved by removing following from the idxmgmt.Supporter interface:

	// ILM provides access to the configured ILM support.
	ILM() ilm.Supporter

	// TemplateConfig returns the template configuration used by the index supporter.
	TemplateConfig(withILM bool) (template.TemplateConfig, error)

The main changes arising from this is related to the export commands, as they were using those methods.
Therefore this PR addresses two main points:

  • Change interface definition for idxmgmt.Supporter to allow for multiple ilm supporter and more flexible template handling
  • Reuse supporter logic for export template and export ilm-policy cmds and refactor export package to reuse and align more logic. Exporting templates and ilm-policy also allows to specify a --dir option now so one can write them to a file in a specified directory.

TODO:

  • Update changelog.

Change idxmgmt handling to work with esClient and stdoutClient, and
change supporter interface for more flexibility.
Refactor export cmds to reuse some logic.
@simitt simitt added the libbeat label Apr 12, 2019
@simitt simitt requested a review from urso April 12, 2019 05:53
@simitt simitt requested a review from a team as a code owner April 12, 2019 05:53
libbeat/idxmgmt/idxmgmt.go Outdated Show resolved Hide resolved
libbeat/idxmgmt/idxmgmt.go Outdated Show resolved Hide resolved
libbeat/idxmgmt/idxmgmt.go Outdated Show resolved Hide resolved
libbeat/idxmgmt/idxmgmt.go Outdated Show resolved Hide resolved
libbeat/idxmgmt/idxmgmt.go Outdated Show resolved Hide resolved
libbeat/idxmgmt/idxmgmt.go Outdated Show resolved Hide resolved
libbeat/cmd/export/config.go Show resolved Hide resolved
libbeat/cmd/instance/beat.go Show resolved Hide resolved
libbeat/cmd/export/config.go Outdated Show resolved Hide resolved
libbeat/cmd/export/config.go Outdated Show resolved Hide resolved
libbeat/tests/system/test_template.py Show resolved Hide resolved
libbeat/idxmgmt/idxmgmt_test.go Show resolved Hide resolved
libbeat/idxmgmt/ilm/client_handler.go Outdated Show resolved Hide resolved
libbeat/idxmgmt/ilm/client_handler.go Outdated Show resolved Hide resolved
libbeat/template/load.go Outdated Show resolved Hide resolved
libbeat/template/load.go Outdated Show resolved Hide resolved
@simitt
Copy link
Contributor Author

simitt commented Apr 15, 2019

@urso this is ready for another round of review.

libbeat/cmd/export/export.go Outdated Show resolved Hide resolved
libbeat/cmd/export/export.go Outdated Show resolved Hide resolved
libbeat/cmd/export/export.go Outdated Show resolved Hide resolved
libbeat/cmd/export/export.go Show resolved Hide resolved
libbeat/idxmgmt/client_handler.go Show resolved Hide resolved
libbeat/cmd/export/template.go Outdated Show resolved Hide resolved
libbeat/idxmgmt/ilm/client_handler_integration_test.go Outdated Show resolved Hide resolved
libbeat/template/client_handler.go Outdated Show resolved Hide resolved
libbeat/template/client_handler.go Outdated Show resolved Hide resolved
libbeat/template/template.go Outdated Show resolved Hide resolved
libbeat/idxmgmt/client_handler.go Outdated Show resolved Hide resolved
libbeat/idxmgmt/client_handler.go Outdated Show resolved Hide resolved
libbeat/idxmgmt/client_handler.go Outdated Show resolved Hide resolved
libbeat/cmd/export/config.go Outdated Show resolved Hide resolved
libbeat/cmd/export/config.go Outdated Show resolved Hide resolved
genTemplateConfigCmd.Flags().Bool("noilm", false, "Generate template with ILM disabled")
genTemplateConfigCmd.Flags().String("dir", "", "Specify directory for printing template files. By default templates are printed to stdout.")
Copy link

Choose a reason for hiding this comment

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

nice. Makes me wonder if it would make sense to have this as a positional parameter instead of a CLI flag. E.g. what happens when we introduce multiple templates support, where to write to if no dir is given?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If no dir is given, we write to stdout.

libbeat/template/template.go Outdated Show resolved Hide resolved
@urso
Copy link

urso commented Apr 18, 2019

Jenkins, please test this.

@urso
Copy link

urso commented Apr 18, 2019

Jenkins, test this.

@simitt
Copy link
Contributor Author

simitt commented Apr 23, 2019

jenkins, test this.

@simitt simitt added the needs_backport PR is waiting to be backported to other branches. label Apr 23, 2019
@simitt simitt merged commit 12fe5f8 into elastic:master Apr 23, 2019
@simitt simitt added the v7.2.0 label Apr 24, 2019
@simitt simitt deleted the adapt-ilm branch May 9, 2019 07:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libbeat needs_backport PR is waiting to be backported to other branches. v7.2.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants